Skip to content

feat: adapt TimeLimit schema to query-lib Attribute and Index VOs - #124

Open
abnegate wants to merge 9 commits into
mainfrom
feat-query-lib
Open

feat: adapt TimeLimit schema to query-lib Attribute and Index VOs#124
abnegate wants to merge 9 commits into
mainfrom
feat-query-lib

Conversation

@abnegate

@abnegate abnegate commented Aug 21, 2026

Copy link
Copy Markdown
Member

Adapts the TimeLimit schema to the query-lib Attribute and Index value objects.

Why this approach

Query-lib createCollection() takes a Collection value object only, so the TimeLimit adapter now builds new Collection(id:, attributes:, indexes:) from the typed factories (Attribute::string(), Index::unique(), …) with ColumnType / IndexType rather than Database::VAR_* strings.

The column set is unchanged: key / time / count, a unique (key, time) and an index on time.

utopia-php/database is pinned to dev-feat-query-lib as 7.0.0 with caret ranges (^, not *), and the lock is re-pinned to that branch's head whenever it moves.

Chain

Landing order, bottom up:

  1. utopia-php/database#823 — the query-lib migration itself
  2. utopia-php/abuse#124, utopia-php/audit#133, utopia-php/migration#222 — the schema call sites in the libraries
  3. appwrite/appwrite#11649
  4. appwrite-labs/cloud#5410

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-lib pin 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

  • Linter, CodeQL, Tests (8.5) and Benchmarks all green on this head
  • Greptile 5/5, no unresolved threads
  • Pint on the Collection wrap

Not verified

  • tests/Abuse/Appwrite/TablesDBTest.php does not run — it needs APPWRITE_* CI secrets this repository does not have
  • The utopia-php/database dependency is still a branch pin. It becomes a released tag only once #823 merges, and this PR should not land before that.

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

Greptile Summary

The PR migrates the database-backed TimeLimit setup to query-lib collection, attribute, and index value objects while preserving the existing schema. It also adopts the query-lib database branch and aligns the package, containers, and CI with its PHP 8.5 requirement.

  • Constructs the abuse collection through typed Collection, Attribute, and Index value objects.
  • Pins the query-lib database development branch and updates its dependency graph.
  • Raises the supported PHP floor to 8.5 and removes the PHP 8.4 CI and container target.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/Abuse/Adapters/TimeLimit/Database.php Replaces document-array schema definitions with typed query-lib value objects while retaining the key, time, and count fields and required indexes.
composer.json Selects the query-lib database branch, adds its query dependency and VCS repositories, and correctly raises the advertised PHP floor to 8.5.
composer.lock Locks the query-lib dependency graph and records the database branch alias and PHP 8.5 platform requirement.
.github/workflows/tests.yml Removes the incompatible PHP 8.4 matrix entry so CI tests only the newly supported PHP 8.5 runtime.
docker-compose.yml Changes the default test container target to the existing PHP 8.5 Dockerfile.

Reviews (11): Last reviewed commit: "chore(deps): re-pin the database lock to..." | Re-trigger Greptile

Comment thread composer.json
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.
@abnegate

Copy link
Copy Markdown
Member Author

@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.
@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptile-apps review

Force re-review of HEAD 99414ff. Description updated for factories and caret lock refresh.

Database::createCollection no longer accepts a string id.
Database feat-query-lib now requires query ^0.5 after nested joins.
@abnegate

Copy link
Copy Markdown
Member Author

@greptile-apps review

abnegate and others added 2 commits August 26, 2026 19:47
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant