Conversation
Contributor
📝 WalkthroughWalkthroughThe PR removes the deprecation-warning assembly and emission logic from app/controllers/general.php (the code that computed SDK-derived deprecation details and appended them to $warnings, including the corresponding block in App::get('/robots.txt')). The deprecation pathway that would emit X-Appwrite-Warning is no longer executed; no other routing, error handling, or response logic was changed. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (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). (16)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/controllers/general.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/controllers/general.php (2)
src/Appwrite/SDK/Method.php (1)
getDeprecated(191-194)src/Appwrite/SDK/Deprecated.php (2)
getReplaceWith(28-31)getSince(20-23)
🔇 Additional comments (1)
app/controllers/general.php (1)
1081-1084: No multi-dotreplaceWithpatterns found. All currentreplaceWithvalues contain exactly one dot; no changes required.
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
✨ Benchmark results
⚡ Benchmark Comparison
|
ItzNotABug
suggested changes
Aug 30, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/controllers/general.php (1)
1078-1088: Avoid empty “Please use …” and incorrect docs path when headers are absent; build message/link conditionally with safe fallbacksUsing default 'rest'/'server' makes the “Route …” prefix always chosen and can yield a broken link when replaceWith is empty. Build the anchor and link only when replacement exists, detect header presence with empty defaults, fall back to server-rest, and sanitize the final header value.
- $deprecatedMethod = $sdkItems[0]->getDeprecated(); - - $replaceWith = $deprecatedMethod->getReplaceWith(); - if ($replaceWith) { - $replaceWith = preg_replace('/\./', '#', $replaceWith, 1); - } - $sdkName = $request->getHeader('x-sdk-name', 'rest'); - $sdkPlatform = $request->getHeader('x-sdk-platform', 'server'); - $deprecatedReplaceWithLink = 'https://appwrite.io/docs/references/cloud/' . $sdkPlatform . '-' . strtolower($sdkName) . '/' . $replaceWith; - - $deprecationWarning = (!empty($sdkName) ? 'Route ' . $route->getPath() : 'Method `' . $sdkItems[0]->getNamespace() . '.' . $sdkItems[0]->getMethodName() . '`') . ' is deprecated since ' . $deprecatedMethod->getSince() . '. Please use `' . $deprecatedMethod->getReplaceWith() . '` instead. See: ' . $deprecatedReplaceWithLink; + $deprecated = $sdkItems[0]->getDeprecated(); + + $replaceWithRaw = $deprecated->getReplaceWith(); + $anchor = $replaceWithRaw ? preg_replace('/\./', '#', $replaceWithRaw, 1) : ''; + + // Detect actual header presence; use sane fallback for docs section + $sdkName = $request->getHeader('x-sdk-name', ''); + $sdkPlatform = $request->getHeader('x-sdk-platform', ''); + $docsSection = (!empty($sdkName) && !empty($sdkPlatform)) + ? ($sdkPlatform . '-' . strtolower($sdkName)) + : 'server-rest'; + + $deprecatedReplaceWithLink = 'https://appwrite.io/docs/references/cloud/' . $docsSection . ($anchor ? '/' . $anchor : ''); + + $prefix = !empty($sdkName) + ? ('Route ' . $route->getPath()) + : ('Method `' . $sdkItems[0]->getNamespace() . '.' . $sdkItems[0]->getMethodName() . '`'); + + $deprecationWarning = $prefix . ' is deprecated since ' . $deprecated->getSince() . '.'; + if (!empty($replaceWithRaw)) { + $deprecationWarning .= ' Please use `' . $replaceWithRaw . '` instead.'; + } + $deprecationWarning .= ' See: ' . $deprecatedReplaceWithLink; + // Defensive: prevent header injection via unexpected control chars in metadata + $deprecationWarning = preg_replace('/[\r\n]+/', ' ', $deprecationWarning);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/controllers/general.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In app/controllers/general.php, the deprecation warning logic is safely implemented: it first checks isDeprecated() for all SDK items (which returns $this->deprecated !== null), and only when all are deprecated does it call getDeprecated(). This logical flow guarantees that getDeprecated() cannot return null in that context, making additional null checks unnecessary.
📚 Learning: 2025-08-29T15:29:23.250Z
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In app/controllers/general.php, the deprecation warning logic is safely implemented: it first checks isDeprecated() for all SDK items (which returns $this->deprecated !== null), and only when all are deprecated does it call getDeprecated(). This logical flow guarantees that getDeprecated() cannot return null in that context, making additional null checks unnecessary.
Applied to files:
app/controllers/general.php
📚 Learning: 2025-08-29T15:29:23.250Z
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In the Appwrite codebase, when SDK methods are marked as deprecated (isDeprecated() returns true), the getDeprecated() method is guaranteed to return valid deprecation metadata objects, not null. The deprecation checking logic first verifies isDeprecated() before accessing getDeprecated(), providing implicit null-safety.
Applied to files:
app/controllers/general.php
🧬 Code graph analysis (1)
app/controllers/general.php (3)
src/Appwrite/SDK/Method.php (3)
getDeprecated(191-194)getNamespace(128-131)getMethodName(138-141)src/Appwrite/SDK/Deprecated.php (2)
getReplaceWith(28-31)getSince(20-23)src/Appwrite/Utopia/Request.php (1)
getHeader(195-199)
⏰ 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). (18)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Tokens)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Site Screenshots)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: Unit Test
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: scan
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/controllers/general.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In app/controllers/general.php, the deprecation warning logic is safely implemented: it first checks isDeprecated() for all SDK items (which returns $this->deprecated !== null), and only when all are deprecated does it call getDeprecated(). This logical flow guarantees that getDeprecated() cannot return null in that context, making additional null checks unnecessary.
📚 Learning: 2025-08-29T15:29:23.250Z
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In app/controllers/general.php, the deprecation warning logic is safely implemented: it first checks isDeprecated() for all SDK items (which returns $this->deprecated !== null), and only when all are deprecated does it call getDeprecated(). This logical flow guarantees that getDeprecated() cannot return null in that context, making additional null checks unnecessary.
Applied to files:
app/controllers/general.php
📚 Learning: 2025-08-29T15:29:23.250Z
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In the Appwrite codebase, when SDK methods are marked as deprecated (isDeprecated() returns true), the getDeprecated() method is guaranteed to return valid deprecation metadata objects, not null. The deprecation checking logic first verifies isDeprecated() before accessing getDeprecated(), providing implicit null-safety.
Applied to files:
app/controllers/general.php
🧬 Code graph analysis (1)
app/controllers/general.php (3)
src/Appwrite/SDK/Method.php (3)
getDeprecated(191-194)getNamespace(128-131)getMethodName(138-141)src/Appwrite/SDK/Deprecated.php (2)
getReplaceWith(28-31)getSince(20-23)src/Appwrite/Utopia/Request.php (1)
getHeader(195-199)
⏰ 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). (17)
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Users)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Databases/Legacy)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: E2E General Test
ItzNotABug
approved these changes
Aug 30, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/controllers/general.php (2)
1055-1055: Expose X-Appwrite-Warning for browsers (CORS).Without exposing this header, web clients can’t read the deprecation warning.
Apply:
- ->addHeader('Access-Control-Expose-Headers', 'X-Appwrite-Session, X-Fallback-Cookies') + ->addHeader('Access-Control-Expose-Headers', 'X-Appwrite-Session, X-Fallback-Cookies, X-Appwrite-Warning')
1165-1169: Mirror CORS expose for OPTIONS preflight.Keep OPTIONS consistent so UAs know the warning header is readable.
Apply:
- ->addHeader('Access-Control-Expose-Headers', 'X-Appwrite-Session, X-Fallback-Cookies') + ->addHeader('Access-Control-Expose-Headers', 'X-Appwrite-Session, X-Fallback-Cookies, X-Appwrite-Warning')
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/controllers/general.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In app/controllers/general.php, the deprecation warning logic is safely implemented: it first checks isDeprecated() for all SDK items (which returns $this->deprecated !== null), and only when all are deprecated does it call getDeprecated(). This logical flow guarantees that getDeprecated() cannot return null in that context, making additional null checks unnecessary.
📚 Learning: 2025-08-29T15:29:23.250Z
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In app/controllers/general.php, the deprecation warning logic is safely implemented: it first checks isDeprecated() for all SDK items (which returns $this->deprecated !== null), and only when all are deprecated does it call getDeprecated(). This logical flow guarantees that getDeprecated() cannot return null in that context, making additional null checks unnecessary.
Applied to files:
app/controllers/general.php
📚 Learning: 2025-08-29T15:29:23.250Z
Learnt from: ChiragAgg5k
PR: appwrite/appwrite#10408
File: app/controllers/general.php:0-0
Timestamp: 2025-08-29T15:29:23.250Z
Learning: In the Appwrite codebase, when SDK methods are marked as deprecated (isDeprecated() returns true), the getDeprecated() method is guaranteed to return valid deprecation metadata objects, not null. The deprecation checking logic first verifies isDeprecated() before accessing getDeprecated(), providing implicit null-safety.
Applied to files:
app/controllers/general.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). (19)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E Service Test (Databases/Legacy)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: E2E Service Test (Site Screenshots)
- GitHub Check: E2E General Test
- GitHub Check: Unit Test
- GitHub Check: scan
app/controllers/general.php
Outdated
Comment on lines
+1067
to
+1102
| // $sdk = $route->getLabel('sdk', false); | ||
| // $sdkItems = is_array($sdk) ? $sdk : (!empty($sdk) ? [$sdk] : []); | ||
| // if (!empty($sdkItems) && count($sdkItems) > 0) { | ||
| // $allDeprecated = true; | ||
| // foreach ($sdkItems as $sdkItem) { | ||
| // if (!$sdkItem->isDeprecated()) { | ||
| // $allDeprecated = false; | ||
| // break; | ||
| // } | ||
| // } | ||
| // if ($allDeprecated) { | ||
| // $deprecatedMethod = $sdkItems[0]->getDeprecated(); | ||
|
|
||
| // $replaceWith = $deprecatedMethod->getReplaceWith(); | ||
| // if ($replaceWith) { | ||
| // $replaceWith = preg_replace('/\./', '#', $replaceWith, 1); | ||
| // } | ||
|
|
||
| // $sdkNameHeader = strtolower($request->getHeader('x-sdk-name', '')); | ||
| // $sdkPlatformHeader = strtolower($request->getHeader('x-sdk-platform', '')); | ||
|
|
||
| // $sdkExists = !empty($sdkNameHeader); | ||
| // $sdkName = $sdkExists ? $sdkNameHeader : 'rest'; | ||
| // $sdkPlatform = !empty($sdkPlatformHeader) ? $sdkPlatformHeader : 'server'; | ||
|
|
||
| // $deprecatedReplaceWithLink = 'https://appwrite.io/docs/references/cloud/' . $sdkPlatform . '-' . $sdkName . '/' . $replaceWith; | ||
|
|
||
| // $deprecationWarning = ( | ||
| // !$sdkExists | ||
| // ? 'Route ' . $route->getPath() | ||
| // : 'Method `' . $sdkItems[0]->getNamespace() . '.' . $sdkItems[0]->getMethodName() . '`' | ||
| // ) . ' is deprecated since ' . $deprecatedMethod->getSince() . '. Please use `' . $deprecatedMethod->getReplaceWith() . '` instead. See: ' . $deprecatedReplaceWithLink; | ||
|
|
||
| // $warnings[] = $deprecationWarning; | ||
| // } | ||
| // } |
Contributor
There was a problem hiding this comment.
Deprecation warnings are fully disabled; PR objective not met. Reinstate warning for the specific route.
Commenting out this block stops emitting any X-Appwrite-Warning. Add a minimal, explicit warning for the target route to satisfy the PR description.
Apply:
- /** @var \Appwrite\SDK\Method $sdk */
- // $sdk = $route->getLabel('sdk', false);
- // $sdkItems = is_array($sdk) ? $sdk : (!empty($sdk) ? [$sdk] : []);
- // if (!empty($sdkItems) && count($sdkItems) > 0) {
- // $allDeprecated = true;
- // foreach ($sdkItems as $sdkItem) {
- // if (!$sdkItem->isDeprecated()) {
- // $allDeprecated = false;
- // break;
- // }
- // }
- // if ($allDeprecated) {
- // $deprecatedMethod = $sdkItems[0]->getDeprecated();
-
- // $replaceWith = $deprecatedMethod->getReplaceWith();
- // if ($replaceWith) {
- // $replaceWith = preg_replace('/\./', '#', $replaceWith, 1);
- // }
-
- // $sdkNameHeader = strtolower($request->getHeader('x-sdk-name', ''));
- // $sdkPlatformHeader = strtolower($request->getHeader('x-sdk-platform', ''));
-
- // $sdkExists = !empty($sdkNameHeader);
- // $sdkName = $sdkExists ? $sdkNameHeader : 'rest';
- // $sdkPlatform = !empty($sdkPlatformHeader) ? $sdkPlatformHeader : 'server';
-
- // $deprecatedReplaceWithLink = 'https://appwrite.io/docs/references/cloud/' . $sdkPlatform . '-' . $sdkName . '/' . $replaceWith;
-
- // $deprecationWarning = (
- // !$sdkExists
- // ? 'Route ' . $route->getPath()
- // : 'Method `' . $sdkItems[0]->getNamespace() . '.' . $sdkItems[0]->getMethodName() . '`'
- // ) . ' is deprecated since ' . $deprecatedMethod->getSince() . '. Please use `' . $deprecatedMethod->getReplaceWith() . '` instead. See: ' . $deprecatedReplaceWithLink;
-
- // $warnings[] = $deprecationWarning;
- // }
- // }
+ // Emit explicit deprecation warning for legacy Documents list route
+ $routePath = $route->getPath();
+ $deprecatedRoutePatterns = [
+ '/v1/databases/{databaseId}/collections/{collectionId}/documents',
+ '/v1/databases/:databaseId/collections/:collectionId/documents',
+ ];
+ if (\in_array($routePath, $deprecatedRoutePatterns, true)) {
+ $warnings[] = 'Route /v1/databases/:databaseId/collections/:collectionId/documents is deprecated since 1.8.0. Please use `tablesDB.listRows` instead. See: https://appwrite.io/docs/references/cloud/server-rest/tablesDB#listRows';
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/controllers/general.php around lines 1067-1102 the entire deprecation
warning block was commented out, disabling X-Appwrite-Warning; restore a minimal
explicit warning for the current route by re-enabling logic that inspects the
route's 'sdk' label, checks if all listed SDK items are deprecated, retrieves
the deprecated metadata (since and replaceWith), normalizes x-sdk-name /
x-sdk-platform headers (lowercase, default to 'rest'/'server' when missing),
build the replacement doc link (replace first '.' with '#' in replaceWith) and
append a single concise deprecation string to $warnings[] (use route path when
no SDK header, otherwise include SDK namespace.method), keeping the code minimal
and only adding the one warning for the target route.
abnegate
reviewed
Sep 1, 2025
app/controllers/general.php
Outdated
Comment on lines
+1067
to
+1078
| // $sdk = $route->getLabel('sdk', false); | ||
| // $sdkItems = is_array($sdk) ? $sdk : (!empty($sdk) ? [$sdk] : []); | ||
| // if (!empty($sdkItems) && count($sdkItems) > 0) { | ||
| // $allDeprecated = true; | ||
| // foreach ($sdkItems as $sdkItem) { | ||
| // if (!$sdkItem->isDeprecated()) { | ||
| // $allDeprecated = false; | ||
| // break; | ||
| // } | ||
| // } | ||
| // if ($allDeprecated) { | ||
| // $deprecatedMethod = $sdkItems[0]->getDeprecated(); |
Member
There was a problem hiding this comment.
Let's remove code instead of leaving it commented, if we need it back, we can get it from git history
abnegate
approved these changes
Sep 1, 2025
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
3 participants
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 does this PR do?
new warning:
Test Plan
Related PRs and Issues
Checklist