Disable filters for platform and project databases - Stat resources#11027
Disable filters for platform and project databases - Stat resources#11027ChiragAgg5k merged 15 commits into1.8.xfrom
Conversation
- Disable filters for platform and project databases - Stat resources
📝 WalkthroughWalkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-11T10:32:38.142ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (2)
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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Platform/Workers/StatsResources.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
📚 Learning: 2025-08-11T10:32:38.142Z
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
Applied to files:
src/Appwrite/Platform/Workers/StatsResources.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Platform/Workers/StatsResources.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
📚 Learning: 2025-08-11T10:32:38.142Z
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
Applied to files:
src/Appwrite/Platform/Workers/StatsResources.php
🪛 GitHub Actions: Linter
src/Appwrite/Platform/Workers/StatsResources.php
[error] 1-1: PSR-12 linting failed: 892 files, 1 style issue. no_whitespace_in_blank_lines (style issue in src/Appwrite/Platform/Workers/StatsResources.php).
[error] 1-1: Pint lint run failed. Command 'vendor/bin/pint --test --config pint.json' exited with code 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Databases/Legacy)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: Benchmark
🔇 Additional comments (1)
src/Appwrite/Platform/Workers/StatsResources.php (1)
103-104: The review comment is based on incorrect code identification.The code at lines 103-104 does not contain
disableFilters()calls. Line 103 contains$this->disableSubqueries();instead. A search of the entire file confirms there are nodisableFilters()orenableFilters()calls anywhere insrc/Appwrite/Platform/Workers/StatsResources.php. The concerns raised about global side effects on injected database instances do not apply to this code.Likely an incorrect or invalid review comment.
…anidamodar-patch-2
…nidamodar-patch-2
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Appwrite/Platform/Workers/StatsResources.php (1)
102-102: Remove trailing whitespace from blank line.Line 102 contains trailing whitespace, causing the PSR-12 linting failure (
no_whitespace_in_blank_lines).🔎 Proposed fix
- +
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Appwrite/Platform/Action.phpsrc/Appwrite/Platform/Workers/StatsResources.php
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10546
File: src/Appwrite/Platform/Workers/Migrations.php:144-148
Timestamp: 2025-10-23T08:06:38.889Z
Learning: In the Appwrite codebase, migration workers receive already-validated data from queued jobs. Query validation using Query::parseQueries() happens at the API endpoint level (with try-catch for QueryException) before jobs are queued, so workers in src/Appwrite/Platform/Workers/Migrations.php don't need redundant validation.
📚 Learning: 2025-08-11T10:32:38.142Z
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
Applied to files:
src/Appwrite/Platform/Workers/StatsResources.phpsrc/Appwrite/Platform/Action.php
📚 Learning: 2025-10-02T10:16:48.075Z
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10023
File: src/Appwrite/Databases/TransactionState.php:174-219
Timestamp: 2025-10-02T10:16:48.075Z
Learning: In `src/Appwrite/Databases/TransactionState.php`, the `listDocuments` method intentionally applies queries only to the initial database fetch of committed documents. Transaction state changes (creates, updates, deletes) are then overlaid on top of the filtered committed set without re-evaluating the queries against the final merged result. This is the intended behavior for transaction-aware document listing.
Applied to files:
src/Appwrite/Platform/Action.php
🧬 Code graph analysis (1)
src/Appwrite/Platform/Workers/StatsResources.php (1)
src/Appwrite/Platform/Action.php (1)
disableSubqueries(110-127)
🪛 GitHub Actions: Linter
src/Appwrite/Platform/Workers/StatsResources.php
[error] 1-1: PSR-12: no_whitespace_in_blank_lines issue detected by Pint linter in StatsResources.php. Run 'vendor/bin/pint --write' to fix code style issues.
🪛 PHPMD (2.15.0)
src/Appwrite/Platform/Action.php
49-49: Avoid unused parameters such as '$filters'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (VCS)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Databases/Legacy)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E Service Test (Site Screenshots)
- GitHub Check: Unit Test
- GitHub Check: E2E General Test
- GitHub Check: scan
🔇 Additional comments (2)
src/Appwrite/Platform/Workers/StatsResources.php (2)
103-103: Verify necessity ofdisableSubqueries()call.Based on the past discussion where you acknowledged that
skipFiltersmight makedisableSubqueries()unnecessary, this global filter modification is still present. ThedisableSubqueries()method adds global filters that affect all subsequent database operations in this worker, which differs from the scopedskipFilters()pattern used on Line 202.Can you clarify:
- Is this global subquery disabling intentional for all operations in this worker?
- Or should specific operations use scoped
skipFilters()instead, similar to Line 202?
202-202: Good use of scopedskipFilterspattern.The scoped
skipFilters()closure correctly ensures filters are automatically restored after the operation, preventing global side effects. This aligns with the pattern used elsewhere in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Appwrite/Platform/Workers/StatsResources.php (1)
103-204: Inconsistent application ofskipFilters– other database operations remain unprotected.Only the call to
countForSitesAndFunctions(line 201) is wrapped withskipFilters, while all other database operations in the same method (lines 107-155) lack this protection. According to the PR title "Disable filters for platform and project databases - Stat resources", filters should be disabled for ALL database operations in this worker, not just one.The following operations also need to be wrapped with
skipFilters:
- Lines 107-120:
$dbForPlatform->count()operations for platforms, webhooks, keys, and domains- Lines 123-155:
$dbForProject->count()operations for databases, buckets, users, teams, functions, messages, providers, topics, and targets🔎 Example of applying skipFilters consistently
try { $region = $project->getAttribute('region'); - $platforms = $dbForPlatform->count('platforms', [ + $platforms = $dbForPlatform->skipFilters(fn() => $dbForPlatform->count('platforms', [ Query::equal('projectInternalId', [$project->getSequence()]) - ]); + ]), $this->filters); - $webhooks = $dbForPlatform->count('webhooks', [ + $webhooks = $dbForPlatform->skipFilters(fn() => $dbForPlatform->count('webhooks', [ Query::equal('projectInternalId', [$project->getSequence()]) - ]); + ]), $this->filters);Apply this pattern to all remaining
count()andsum()operations on both$dbForPlatformand$dbForProjectthroughout the method.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Appwrite/Platform/Action.phpsrc/Appwrite/Platform/Workers/StatsResources.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
📚 Learning: 2025-08-11T10:32:38.142Z
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10297
File: app/controllers/api/users.php:649-657
Timestamp: 2025-08-11T10:32:38.142Z
Learning: In Appwrite's utopia-php/database (version 0.77.* and later), the database layer internally handles filter separation for count operations. When using `$dbForProject->count()` within a `skipFilters` closure, there's no need to manually separate filter queries using `Query::groupByType($queries)['filters']` as the database handles this automatically.
Applied to files:
src/Appwrite/Platform/Action.phpsrc/Appwrite/Platform/Workers/StatsResources.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (VCS)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Users)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Tokens)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Site Screenshots)
- GitHub Check: Unit Test
- GitHub Check: E2E General Test
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: scan
🔇 Additional comments (1)
src/Appwrite/Platform/Action.php (1)
24-28: The reference toStatsResources.phpline 201 is incorrect—this file does not exist in the codebase. Additionally, no code selectively applies filters by the comment grouping in the array. ThedisableSubqueries()method in Action.php either uses the entire$filtersarray or selectively uses filters by explicit name in configuration files, not by comment-based groups. The reorganization ofsubQueryProjectVariablesfrom the Project group to the Sites/Functions group is safe and aligns with its actual usage (thevarsProjectfield configuration in the projects collection).Likely an incorrect or invalid review comment.
4 participants
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist