feat: adapt TimeLimit schema to query-lib Attribute and Index VOs - #124
feat: adapt TimeLimit schema to query-lib Attribute and Index VOs#124abnegate wants to merge 20 commits into
Conversation
|
utopia-php/database feat-query-lib (and Appwrite/Cloud) require PHP >=8.5. Advertising >=8.4.1 made Composer reject the graph for 8.4 consumers while CI only passed 8.4 via ignore-platform-reqs.
|
@greptile-apps review |
Schema setup with a known column or index type now uses Attribute::string() / Index::key() instead of the generic constructors plus ColumnType / IndexType.
Asterisk wildcards on utopia-php packages are replaced with equivalent caret constraints so Composer ranges stay consistent.
Keep composer.json and composer.lock in sync so `composer validate` passes, and pin utopia-php/database to the current query-lib HEAD.
|
@greptileai review |
|
@greptile-apps review Force re-review of HEAD |
Database::createCollection no longer accepts a string id.
Database feat-query-lib now requires query ^0.5 after nested joins.
|
@greptile-apps review |
The branch requires query "^0.4 || ^0.5" while the database branch it
depends on moved to "0.6.*". Composer cannot satisfy both, and audit's
branch is on 0.6 as well, so anything requiring abuse and audit together --
appwrite does -- cannot resolve either.
CI has not been showing this because the committed lock still pins a
database commit from before that bump, so nothing has re-resolved. Running
composer update against the current branch head fails outright:
utopia-php/database dev-feat-query-lib requires utopia-php/query 0.6.*
-> found utopia-php/query[0.6.0] but it conflicts with your root
composer.json require (^0.4 || ^0.5).
The lock now tracks the database branch head rather than a commit two
constraint changes behind it.
Nothing in src or tests touches the parts of the query API that changed
between 0.5 and 0.6. phpstan at level max and pint are both clean, and the
suite returns exactly the same counts before and after the bump on the same
containers -- 113 tests, 54 errors, 2 failures, 39 skipped, all of them the
uninitialised local redis cluster rather than anything this changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
utopia-php/database#823 moved its ORM, repository/seeder and migration-runner layers into stacked PRs, so its branch head moved. Nothing here referenced any of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lock still pinned 6cc5f8be, two reconciles behind: the branch's green CI was proving a database revision that will not ship. a137475 carries the merge with main and the batched upsert read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guard exists because loading the backend file from the fixture test hid every backend case from PHPUnit's own scan. Pinning the ten method names and the fixture's case count made a rename or an added test fail a check about discovery, so the assertions mirrored the suite instead of describing it. Verified the guard still bites: hoisting the require_once in TablesDBFixtureTest back to file scope fails on the missing class key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@greptileai review |
database's feat-query-lib merged main (1ea2f35), which raised utopia-php/validators to ^1.0. The lock was still on a137475 with validators 0.6.0, so CI here was proving the branch against a base the rest of the train no longer uses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| $this->assertMatchesRegularExpression('/^abuse-[a-f0-9]{24}$/', $this->read()['deletes'][0]); | ||
| AppwriteTablesDBTest::tearDownAfterClass(); | ||
| $this->assertCount(1, $this->read()['deletes']); | ||
| } |
There was a problem hiding this comment.
Identifier Format Mirrors Implementation
This assertion hard-codes the abuse- prefix and the exact 24-character hexadecimal encoding produced by bin2hex(random_bytes(12)). It tests an internal formatting choice rather than fixture lifecycle behavior, so a harmless identifier refactor would break the test. This violates the repository directive against mirroring source details in assertions and must be addressed before merging.
Context Used: Call out and harshly judge implementation-coupled ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/Abuse/Appwrite/TablesDBFixtureTest.php
Line: 121
Comment:
**Identifier Format Mirrors Implementation**
This assertion hard-codes the `abuse-` prefix and the exact 24-character hexadecimal encoding produced by `bin2hex(random_bytes(12))`. It tests an internal formatting choice rather than fixture lifecycle behavior, so a harmless identifier refactor would break the test. This violates the repository directive against mirroring source details in assertions and must be addressed before merging.
**Context Used:** Call out and harshly judge implementation-coupled ... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Picks up the read-replica write-hook sync and the schema-bound aggregate validators, so CI here proves this branch against the base the rest of the train now uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adapts the TimeLimit schema to the query-lib
AttributeandIndexvalue objects.Why this approach
Query-lib
createCollection()takes aCollectionvalue object only, so the TimeLimit adapter now buildsnew Collection(id:, attributes:, indexes:)from the typed factories (Attribute::string(),Index::unique(), …) withColumnType/IndexTyperather thanDatabase::VAR_*strings.The column set is unchanged:
key/time/count, a unique(key, time)and an index ontime.utopia-php/databaseis pinned todev-feat-query-lib as 7.0.0with caret ranges (^, not*), and the lock is re-pinned to that branch's head whenever it moves.Chain
Landing order, bottom up:
Stacked on #823 but not part of it, and not required by anything above: #947 (ORM), #948 (repositories and seeding), #949 (migration runner and schema differ).
Every
dev-feat-query-libpin in this train is re-pinned to its branch head whenever one of them moves, so each PR's CI runs against what the others actually contain.Verified
Not verified
tests/Abuse/Appwrite/TablesDBTest.phpdoes not run — it needsAPPWRITE_*CI secrets this repository does not haveutopia-php/databasedependency is still a branch pin. It becomes a released tag only once #823 merges, and this PR should not land before that.