Conversation
WalkthroughThe updates introduce enhanced handling of selection queries in database document endpoints, ensuring explicit control over relationship traversal and attribute inclusion. A new request filter for version 1.8.0 is added to maintain legacy behavior with related documents. The request filter base class is extended to support project database and route context. End-to-end tests are refined to use explicit selection queries, and test stability is improved with added waits. Minor CI workflow cleanup is applied. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Controller
participant RequestFilterV20
participant ProjectDB
participant Database
Client->>API_Controller: GET /databases/{dbId}/collections/{colId}/documents
API_Controller->>RequestFilterV20: Apply filter (with ProjectDB, Route)
RequestFilterV20->>ProjectDB: Fetch related collection keys (if needed)
RequestFilterV20->>API_Controller: Return modified queries with legacy related keys
API_Controller->>Database: find/getDocument (with/without skipRelationships)
Database-->>API_Controller: Documents
API_Controller-->>Client: Response (with selected fields and relationships)
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (19)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
✨ Benchmark results
⚡ Benchmark Comparison
|
…zy-load-relationships
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/Appwrite/Utopia/Request/Filter.php (2)
10-12: Consider giving typed properties an explicit default valueAlthough the constructor normally initialises
$routeand$dbForProject, it is still possible for an extending class to forget callingparent::__construct(), leaving the properties un-initialised and triggering aTyped property must not be accessed before initializationerror. Adding a= nulldefault makes the class more defensive at virtually zero cost.- private ?Route $route; - private ?Database $dbForProject; + private ?Route $route = null; + private ?Database $dbForProject = null;
47-56: Broaden the caught throwable & remove redundant null-coalescing
Route::getParamValue()may throw anyThrowable, not onlyException(e.g.Error). Catching\Throwableis safer.
Also, the null-coalescing operator already covers the route isnullcase, so the surroundingtry/catchonly needs to protect against the method throwing. This tiny refactor keeps the intent but avoids double work:- try { - $value = $this->route?->getParamValue($key) ?? $default; - } catch (\Exception $e) { - $value = $default; - } + try { + return $this->route?->getParamValue($key) ?? $default; + } catch (\Throwable) { + return $default; + } - - return $value;src/Appwrite/Utopia/Request/Filters/V19.php (1)
30-36: Redundant prefix check
$isDatabaseModelis always true when$modelequals eitherdatabases.listDocumentsordatabases.getDocument. Removing the extrastr_starts_withsimplifies the guard clause:- $isDatabaseModel = \str_starts_with($model, 'databases.'); - $isTargetOperation = \in_array($model, ['databases.listDocuments', 'databases.getDocument']); - - if (! $isDatabaseModel || ! $isTargetOperation) { + if (!\in_array($model, ['databases.listDocuments', 'databases.getDocument'], true)) { return $content; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
app/controllers/api/databases.php(2 hunks)app/controllers/general.php(1 hunks)composer.json(3 hunks)src/Appwrite/Utopia/Request/Filter.php(2 hunks)src/Appwrite/Utopia/Request/Filters/V19.php(1 hunks)
🔇 Additional comments (7)
app/controllers/general.php (1)
833-834:Details
✅ Verification successful
Dependency injection improves request filter capabilities for handling relationship queries.
The change injects the project database instance and route object into the RequestV19 filter, enabling it to access collection metadata and modify query selections to control relationship loading behavior. This aligns with the PR objective of implementing lazy loading for relationships, ensuring backward compatibility for versions before 1.7.0.
🏁 Script executed:
#!/bin/bash # Verify how the V19 filter class uses these dependencies # Look for the V19 filter class implementation echo "Searching for V19 filter implementation..." rg --type php -A 15 "class V19" src/Length of output: 3774
🏁 Script executed:
#!/bin/bash # Display the full V19 request filter implementation to verify constructor and manageSelectQueries usage rg --type php -A200 "class V19 extends Filter" src/Appwrite/Utopia/Request/Filters/V19.phpLength of output: 3500
🏁 Script executed:
#!/bin/bash # Verify Filter base class constructor signature and dependency handling rg --type php -A200 "class Filter" src/Appwrite/Utopia/Request/Filters/Filter.phpLength of output: 170
🏁 Script executed:
#!/bin/bash # Search for base Filter class definition across the src directory rg --type php -n "class Filter" src/Length of output: 165
🏁 Script executed:
#!/bin/bash # Show the Request Filter base class implementation sed -n '1,200p' src/Appwrite/Utopia/Request/Filter.php # Check for constructor definition in Filter rg --type php -n "__construct" src/Appwrite/Utopia/Request/Filter.phpLength of output: 1457
Dependency injection correctly enhances RequestV19 filter
Injecting the project’s Database instance and the Route into the RequestV19 filter aligns perfectly with how the base
Filterconstructor is defined (__construct(Database $dbForProject = null, Route $route = null)). This grants the filter access to:
getDbForProject()for resolving collection metadatagetParamValue()for extractingdatabaseIdandcollectionIdfrom the routewhich it uses in
manageSelectQueries()to preserve 1.6.x behavior (including related documents by default) while maintaining lazy-loading support for 1.7.x+. This change meets the PR’s goal of backward-compatible relationship loading.composer.json (3)
52-52: Cache dependency updated.The
utopia-php/cachepackage has been updated to version 0.13.*, which likely includes improvements or features needed to support the lazy-loading relationship functionality.
97-102: Added custom repository source for the database package.This configuration directs Composer to fetch the
utopia-php/databasepackage directly from the GitHub repository instead of Packagist, which is necessary to use the development branch specified above.
56-56:Details
❓ Verification inconclusive
Using a development branch for database package.
The dependency on
utopia-php/databasehas been changed from a stable version to a development branch specifically created for handling wildcards in queries. This branch is required for implementing the relationship lazy-loading functionality described in the PR.
🏁 Script executed:
#!/bin/bash # Check if there's a reference to a related PR for the database package # Look for references to the related database PR in the current repo echo "Searching for references to the utopia-php/database PR..." rg -i "utopia.*database.*PR" --type mdLength of output: 165
Please verify the “dev-manage-wildcards” alias for utopia-php/database
It looks like you’ve switched
utopia-php/databaseto a development branch alias:• composer.json: line 56
"utopia-php/database": "dev-manage-wildcards as 0.66.0",Before we merge, confirm that:
- The
dev-manage-wildcardsbranch exists and is up-to-date in the utopia-php/database repo.- That branch has been published (or is accessible) via Packagist under the correct version alias.
- Any consumers of this package in the repo will still resolve correctly to the expected API.
app/controllers/api/databases.php (2)
3513-3522: Well-implemented optimization of database queries!This change implements the lazy-loading of relationships by checking for selection queries before deciding how to fetch documents. When no selections are present, the code skips loading relationships, which should significantly improve performance for documents with many relationships.
3673-3684: Consistent implementation of lazy-loading in the getDocument endpoint.Similar to the change in listDocuments, this implements lazy-loading for single document retrieval. The conditional relationship loading is well-structured and mirrors the implementation in the list endpoint, ensuring consistent behavior throughout the API.
src/Appwrite/Utopia/Request/Filters/V19.php (1)
43-45: Verify key used inQuery::groupByType()resultYou access the grouped array via
['selections'].
In Utopia Database, the key is usually derived from the constant (Query::TYPE_SELECT), so it may be'selects'. A mismatch yields an empty$selections, making the wildcard-detection loop ineffective when explicit selects are supplied.Please double-check the constant name or update the index:
$groups = Query::groupByType($parsed); $selections = $groups[Query::TYPE_SELECT.'s'] ?? [];(Adjust if the library already uses the plural form you expect.)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/tests.yml (1)
362-362: Add newline at end of file
YAML files should end with a newline to satisfy POSIX standards and avoid thenew-line-at-end-of-filelint error.Apply this diff to add the missing newline:
--- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -362 +363 +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 362-362: no new line character at the end of file
(new-line-at-end-of-file)
src/Appwrite/Utopia/Request/Filters/V20.php (2)
35-41: Harden handling of thequeriespayload
manageSelectQueries()assumes thatqueriesis either absent or already an array of query-strings.
A single scalar (e.g."limit(50)") will makeQuery::parseQueries()throw, falling into the generic catch and silently disabling the filter.+ // Normalise – always work with an array + if (isset($content['queries']) && !\is_array($content['queries'])) { + $content['queries'] = [$content['queries']]; + }
61-76: Consider caching relationship keys forlistDocuments
getRelatedCollectionKeys()is invoked on every wildcardlistDocumentsrequest and performs two DB reads.
If the same collection is queried frequently, memoising the keys (e.g. static cache keyed by$collectionId) would cut DB round-trips and latency without affecting correctness.src/Appwrite/Utopia/Response/Filters/V20.php (1)
7-13: Implement or remove placeholder filter
V20returns the payload untouched.
If no transformation is planned, drop the class to avoid maintenance overhead; otherwise add a TODO explaining the forthcoming mapping logic.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/tests.yml(3 hunks)app/controllers/general.php(3 hunks)src/Appwrite/Utopia/Request/Filters/V19.php(1 hunks)src/Appwrite/Utopia/Request/Filters/V20.php(1 hunks)src/Appwrite/Utopia/Response/Filters/V20.php(1 hunks)tests/e2e/Services/Databases/DatabasesBase.php(7 hunks)tests/e2e/Services/Databases/DatabasesCustomServerTest.php(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/Appwrite/Utopia/Request/Filters/V19.php
🚧 Files skipped from review as they are similar to previous changes (3)
- app/controllers/general.php
- tests/e2e/Services/Databases/DatabasesBase.php
- tests/e2e/Services/Databases/DatabasesCustomServerTest.php
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/tests.yml
[error] 276-276: trailing spaces
(trailing-spaces)
[error] 362-362: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (2)
.github/workflows/tests.yml (2)
121-125: Clean up trailing whitespace
Removed unnecessary spaces after thesleep 10command in the E2E General Test job to eliminate diff noise and satisfy YAML lint rules.
273-277: Clean up trailing whitespace
Removed extraneous spaces at the end of the conditional block in the Shared Mode Service Test job to improve readability and avoid lint warnings.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 276-276: trailing spaces
(trailing-spaces)
2 participants
What does this PR do?
Skips fetching related documents unless explicitly requested via
selectqueries.Default behavior
GET
Response
{ "title": "Avengers", "$id": "avengers", "$permissions": [], "$databaseId": "theatre", "$collectionId": "movies", "$createdAt": "2025-04-19T11:59:18.339+00:00", "$updatedAt": "2025-04-19T11:59:18.339+00:00" }With nested relationship selects
GET
PARAMS
Response
{ "title": "Avengers", "actors": [ { "name": "Chris Evans", "role": "Captain America", "$databaseId": "theatre", "$collectionId": "actors" }, ... ], "ratings": [ { "score": 9, "$databaseId": "theatre", "$collectionId": "ratings" } ], "$databaseId": "theatre", "$collectionId": "movies" }Test Plan
Manual.
Related PRs and Issues
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores