Repository navigation
chore: 8.1.2 review cleanup - #1014
Merged
Merged
Conversation
Database::rawQuery() duplicated query() with a Statement; the permission event classes were never dispatched; AllowNullUid only wrapped Column\AllowNull; getRelationshipHook(), getCacheWriterTimeout() and the index-length, varchar and datetime getters had no consumers and repeat what limits() and the caller already hold. Removing them shrinks the 8.x API back to what callers use. Feature\RawQuery, query()/mutate() and the kept limit getters stay. Tests observe the relationship hook through snapshot() and the hook's own scoped state instead of the removed getter, and the Mirror writer timeout through the query cache scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A hostname given to Pool::setHostname() was ignored: hostname() always asked the borrowed adapter, so Database::getHostname() and the cache keys built from it named the pooled host. Appwrite subclassed Pool only to work around that and to reach the protected $pool. hostname() now returns the set hostname when it is not empty and delegates otherwise, and getPool() returns the utopia-php/pools pool, so the subclass can go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
read() routed a list to find() or aggregate() by inspecting its queries, which is a caller's decision: appwrite's list endpoints now call find() or aggregate() explicitly, so the router leaves the library before anyone else depends on it. fetch() and nextPageCheck() share ParsedQuery::isAggregation() instead of repeating the aggregations/groupBy check; find() still refuses aggregate and groupBy queries exactly as before. The isAggregation() assertions move from the deleted ReadTest into QueryTest, and the join tests read their ungrouped count through aggregate(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 8.0 to 8.1 guide pointed callers at read(), which 8.1.2 removes, so it now points at find()/aggregate() and isAggregation(), and a self-contained 8.1.1 to 8.1.2 section explains the replacement and the cache-key effect of a pool hostname. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ating 7.4.1 The round-fractions mode reproduced each engine's 7.4.1 arithmetic through Adapter::arithmetic(), the Arithmetic classes and a locked read of every stored value, for a behaviour the user summed up as "it's just rounding". Integer attributes now take one rule, always on and applied once in PHP before validation: a fractional increase/decrease change or increment/decrement amount rounds half away from zero, a fractional max is floored and a fractional min ceiled. Decimal text rounds exactly at any size. A counter change that rounds to zero writes nothing and fires no event. Fractional multiply, divide, modulo and power stay refused, and float attributes are unchanged. This removes setRoundFractions()/isRoundingFractions()/withRoundFractions(), the Mirror overrides, the snapshot flag, Adapter::arithmetic() and its overrides and pool cache, the validators' $fractions argument and the Fractions trait, and restores the single 8.0.3 counter path and the single updateDocuments() write. IntegerRoundingTest (Memory, schemaless Memory, SQLite) and the IntegerRoundingTests e2e scope replace the emulation tests, keeping the whole-bound cases of FractionalBoundTest and FractionalOperatorLimitTest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Callers of setRoundFractions() and Adapter::arithmetic() need to know they are gone and what replaced them, and the 8.0 to 8.1 guide must no longer send readers to the removed toggle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The adapted and new tests still carried the removed mode's vocabulary in their names; naming them after the rounding rule keeps a search for the removed subsystem pointing only at what is really gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AttributeUpdate, CollectionUpdate and RelationshipUpdate repeated the name of the model they update in the class name. They now live beside that model as Attribute\Update, Collection\Update and Relationship\Update, so the namespace carries the context. There is no class_alias and no composer conflict entry: consumers switch their imports in the releases that follow (migration 3.1.2, then appwrite and cloud). Files that use more than one of the models import them under aliases so their call sites stay unchanged; files that use one import it as Update. The historical CHANGELOG and UPGRADE sections keep the names those releases shipped with; the new 8.1.2 sections map old to new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # UPGRADE.md
# Conflicts: # CHANGELOG.md # UPGRADE.md
Each branch added its own 8.1.2 CHANGELOG section and 8.1.1 -> 8.1.2 UPGRADE section. This merge folds the four into one of each: a single CHANGELOG 8.1.2 entry (Removed (breaking) / Changed (breaking) / Changed / Added / Fixed) and a single upgrade guide whose Removed API table names every removal with its replacement, followed by the detail sections for read(), the update models, integer rounding and the pool hostname. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ReadWritePool caches the write pool's hostname on first use to avoid a borrow per cache key. The cache outlived setHostname(), so a hostname set after the first answer was ignored and cache keys kept naming the pooled adapter's host, the very bug 8.1.2 fixes for Pool. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deleting AllowNullUidTest with the wrapper dropped the only test that the AllowNull constructor refuses an injected column name; wrap() was covered but the constructor validates separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wholeNumber() took a closure over three anonymous booleans to say when to move away from zero, and three wrappers named the modes. An internal Rounding enum names them and a match picks the direction, so a reader sees the rule in one place instead of decoding closure arguments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The decimal pattern wanted digits on both sides of the point and no sign or whitespace, so '.5', '1.', '+1.5' and ' 1.5' passed the numeric check and were then refused as "must be an integer", which named the wrong problem. Text with an exponent stays refused: it is not plain decimal and rounding it exactly would need a parser for scientific notation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
increaseDocumentAttribute(0.4) leaves the document alone, but increment(0.4) given to updateDocument() still counted as a change and moved $updatedAt, and updateDocuments() and upserts wrote every row. roundOperators() now reports the attributes whose fractional amount rounds to 0 and the caller keeps their stored value, so the existing no-change handling of each write applies: updateDocument() keeps $updatedAt, updateDocuments() with nothing else returns 0, and an upsert of a stored document skips it. An upsert that creates the document keeps the rounded operator, so it stores what increment(0) stores on insert. An amount given as the integer 0 is not a rounding case and behaves as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the cases the rounding review found missing: changes that round to nothing on every write path with their events and $updatedAt, plain decimal text with signs, whitespace and a bare point, exponent text, text operator amounts and fractional integer-array items. The adapters move to an Engine enum and the e2e closure to a typed helper, which clears the PHPStan match.unhandled and argument.type errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sses States that a change rounding to 0 skips the max/min check and what each write path does with it, widens the decimal text description to match the parser, qualifies "still refused" as true only without setRoundFractions(true), lists the two behaviours that opt-in users lose, and notes the ReadWritePool hostname fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
updateDocuments returned 0 for updates whose only change rounded to nothing but left the rounded increment(0) in the caller's document. The same updates given again then counted as a change and bumped $updatedAt, and Mirror, which replicates a clone of those updates, wrote that change to the destination while the source wrote nothing. The rounded keys are now removed before the empty check, so the caller's document carries no change and replication has nothing to apply. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rounding decimal text adds one to its whole part with BigInt::add, whose string helpers built the result by prepending a digit at a time, so the work grew with the square of the length: a million-digit amount took about 14 seconds before the range check refused it. The helpers now pad both numbers to whole 18-digit chunks, work chunk by chunk in native integers and join the reversed chunks once, which keeps results exact without capping the length; the same amount is refused in milliseconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
removeAttribute changes the document but returns static, so PHPStan took it for a possibly pure call and kept remembering an earlier isEmpty() result across it. updateDocuments checks isEmpty() again after removing the operators that round to nothing, which PHPStan then reported as always false. Declaring the mutation lets analysis see the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 150 files, which is 50 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing: 150
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Patch release 8.1.2: a review cleanup of what #823 (8.x query-lib adaptation) added beyond what the 8.x API needs.
Arithmetic*,Trait\Fractions,setRoundFractions(),Adapter::arithmetic()) is gone. Increment/decrement amounts on integer attributes now round half away from zero, once, in PHP, on every adapter: +1.5 → +2, −2.5 → −3, 0.4 → no-op. Fractional max bounds are floored, min bounds ceiled. Fractional multiply/divide/modulo/power stay refused.Database::read()(usefind()/aggregate()),Database::rawQuery()(usequery()),getRelationshipHook(), four limit getters,getCacheWriterTimeout(), the never-dispatched permission events,AllowNullUid.AttributeUpdate/CollectionUpdate/RelationshipUpdate→Attribute\Update/Collection\Update/Relationship\Update.hostname()honourssetHostname()(also inReadWritePool); newgetPool().Every breaking item is listed in CHANGELOG
## 8.1.2with its replacement in UPGRADE "Upgrading from 8.1.1 to 8.1.2". Kept on purpose:DispatcherHook,Profiler,ReadWritePool,Filter\Callback, adapter-levelrawQuery().Tests
Fail-first:
IntegerRoundingTest(Memory, schemaless Memory, SQLite),PoolHostnameTest,MirrorTestzero-rounding replication, BigInt long-number arithmetic. New e2e scopeIntegerRoundingTestsreplacesFractionalIntegerTests. Unit suite and both PHPStan configs pass locally.🤖 Generated with Claude Code