Conversation
📝 WalkthroughWalkthroughThis PR removes two legacy console controllers and updates service configuration to delegate console handling to a new modular action system. It adds multiple Console module action classes: API and Web init handlers, a streaming assistant endpoint, a variables API, and several redirect handlers (root, auth, invite, login, mfa, card, recover, register). The Http service registration is updated to register these new actions and redirects. Overall, functionality from the deleted controllers is reimplemented as discrete, registered module actions. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous 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). (3)
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! |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/Appwrite/Platform/Modules/Console/Http/Assistant/Create.php (1)
71-82: Remove unused parameter in header callback.The
$curlparameter in the header function callback is unused and flagged by static analysis.🔎 Proposed fix
-curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) { +curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($ch, $header) use (&$responseHeaders) { $len = strlen($header); $header = explode(':', $header, 2); if (count($header) < 2) { // ignore invalid headers return $len; } $responseHeaders[strtolower(trim($header[0]))] = trim($header[1]); return $len; });src/Appwrite/Platform/Modules/Console/Http/Init/Web.php (1)
23-28: Consider removing the deprecated X-XSS-Protection header.The
X-XSS-Protectionheader on Line 26 has been deprecated and is ignored by modern browsers (Chrome removed it in 2019, Edge deprecated it, and Firefox never supported it). Modern security practices recommend using Content Security Policy (CSP) instead.📋 Suggested refactor
$response ->addHeader('X-Frame-Options', 'SAMEORIGIN') // Avoid console and homepage from showing in iframes - ->addHeader('X-XSS-Protection', '1; mode=block; report=/v1/xss?url=' . \urlencode($request->getURI())) ->addHeader('X-UA-Compatible', 'IE=Edge') // Deny IE browsers from going into quirks mode ;Alternatively, if XSS protection reporting is still needed, consider implementing a proper CSP policy instead.
src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php (1)
78-80: Consider using explicit type casting for clarity.Lines 78-80 use the unary
+operator for type coercion (e.g.,+System::getEnv('_APP_STORAGE_LIMIT')). While valid, this is non-idiomatic in PHP. The more conventional approach is to use explicit casting like(int)orintval()for better readability and maintainability.🔎 Suggested refactor
'_APP_DOMAIN_TARGET_CAA' => '0 issue "' . System::getEnv('_APP_DOMAIN_TARGET_CAA') . '"', -'_APP_STORAGE_LIMIT' => +System::getEnv('_APP_STORAGE_LIMIT'), -'_APP_COMPUTE_BUILD_TIMEOUT' => +System::getEnv('_APP_COMPUTE_BUILD_TIMEOUT'), -'_APP_COMPUTE_SIZE_LIMIT' => +System::getEnv('_APP_COMPUTE_SIZE_LIMIT'), +'_APP_STORAGE_LIMIT' => (int)System::getEnv('_APP_STORAGE_LIMIT'), +'_APP_COMPUTE_BUILD_TIMEOUT' => (int)System::getEnv('_APP_COMPUTE_BUILD_TIMEOUT'), +'_APP_COMPUTE_SIZE_LIMIT' => (int)System::getEnv('_APP_COMPUTE_SIZE_LIMIT'), '_APP_USAGE_STATS' => System::getEnv('_APP_USAGE_STATS'),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/config/services.phpapp/controllers/api/console.phpapp/controllers/web/console.phpsrc/Appwrite/Platform/Modules/Console/Http/Assistant/Create.phpsrc/Appwrite/Platform/Modules/Console/Http/Init/API.phpsrc/Appwrite/Platform/Modules/Console/Http/Init/Web.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Base.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.phpsrc/Appwrite/Platform/Modules/Console/Http/Variables/Get.phpsrc/Appwrite/Platform/Modules/Console/Services/Http.php
💤 Files with no reviewable changes (2)
- app/controllers/api/console.php
- app/controllers/web/console.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-10T04:12:13.037Z
Learnt from: ItzNotABug
Repo: appwrite/appwrite PR: 9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Indexes/Delete.php:85-86
Timestamp: 2025-05-10T04:12:13.037Z
Learning: The class `Appwrite\Platform\Modules\Databases\Http\Databases\Collections\Indexes\Action` defines a method `getGrantParentNotFoundException()` which is used to handle cases where a parent resource (like a database collection) is not found.
Applied to files:
src/Appwrite/Platform/Modules/Console/Http/Init/API.php
🧬 Code graph analysis (12)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (5)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (2)
Base(10-51)getPath(22-22)src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (8)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (2)
Base(10-51)getPath(22-22)src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (8)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (2)
Base(10-51)getPath(22-22)src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (8)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (2)
Base(10-51)getPath(22-22)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (7)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (8)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (1)
getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (1)
getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php (2)
src/Appwrite/Utopia/Response.php (2)
Response(159-983)dynamic(703-736)src/Appwrite/Utopia/Response/Model/Document.php (1)
Document(8-115)
src/Appwrite/Platform/Modules/Console/Services/Http.php (14)
src/Appwrite/Platform/Appwrite.php (1)
Appwrite(17-32)src/Appwrite/Platform/Modules/Console/Http/Assistant/Create.php (3)
Create(14-92)__construct(23-50)getName(18-21)src/Appwrite/Platform/Modules/Console/Http/Init/API.php (3)
API(9-28)__construct(16-27)getName(11-14)src/Appwrite/Platform/Modules/Console/Http/Init/Web.php (3)
Web(9-31)__construct(16-30)getName(11-14)src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (2)
Get(7-18)getName(9-12)src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php (3)
Get(17-93)__construct(26-50)getName(21-24)src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (1)
__construct(24-35)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (1)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (1)
Base(10-51)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (8)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (2)
Base(10-51)getPath(22-22)src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
src/Appwrite/Platform/Modules/Console/Http/Init/Web.php (1)
src/Appwrite/Platform/Modules/Console/Http/Init/API.php (2)
getName(11-14)__construct(16-27)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (8)
src/Appwrite/Platform/Modules/Console/Http/Redirects/Base.php (2)
Base(10-51)getPath(22-22)src/Appwrite/Platform/Modules/Console/Http/Redirects/Auth/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (3)
Get(7-18)getName(9-12)getPath(14-17)
🪛 PHPMD (2.15.0)
src/Appwrite/Platform/Modules/Console/Http/Assistant/Create.php
71-71: Avoid unused parameters such as '$curl'. (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). (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
- GitHub Check: Setup & Build Appwrite Image
🔇 Additional comments (11)
app/config/services.php (1)
23-23: LGTM! Configuration correctly reflects the module-based architecture.The controller entries are appropriately set to empty strings with clear comments indicating module usage, aligning with the removal of legacy controller files and the introduction of the new Console module structure.
Also applies to: 273-275
src/Appwrite/Platform/Modules/Console/Http/Redirects/Login/Get.php (1)
7-18: LGTM! Clean implementation.The redirect handler correctly extends the Base class and follows the established pattern for console redirects.
src/Appwrite/Platform/Modules/Console/Http/Redirects/Card/Get.php (1)
7-18: LGTM! Implementation follows the established pattern.The redirect handler correctly extends the Base class and uses the wildcard path pattern appropriately for card routes.
src/Appwrite/Platform/Modules/Console/Services/Http.php (1)
22-43: LGTM! Service wiring correctly registers all console module actions.The HTTP service properly initializes:
- API and Web init hooks for security and access control
- Console variables and assistant endpoints
- All redirect handlers for console paths
The organization with comments clearly delineates the different types of actions being registered.
src/Appwrite/Platform/Modules/Console/Http/Redirects/MFA/Get.php (1)
7-18: LGTM! Implementation is consistent.The MFA redirect handler correctly extends the Base class and follows the established pattern for console redirects.
src/Appwrite/Platform/Modules/Console/Http/Redirects/Invite/Get.php (1)
7-18: LGTM! Clean and consistent implementation.The invite redirect handler correctly extends the Base class and follows the established pattern for console redirects.
src/Appwrite/Platform/Modules/Console/Http/Redirects/Recover/Get.php (1)
7-18: LGTM!The redirect handler correctly implements the Base contract with appropriate naming and path configuration. Implementation is consistent with other console redirects.
src/Appwrite/Platform/Modules/Console/Http/Redirects/Register/Get.php (1)
7-18: LGTM!The register redirect handler follows the established pattern and correctly uses wildcard routing with a leading slash.
src/Appwrite/Platform/Modules/Console/Http/Redirects/Root/Get.php (1)
7-18: LGTM!The root redirect handler is correctly implemented and follows the pattern established by other console redirects.
src/Appwrite/Platform/Modules/Console/Http/Init/API.php (1)
16-27: LGTM!The console API init handler correctly restricts access to the console project ID. The security check is appropriate and the exception handling is correct.
src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php (1)
26-50: LGTM!The endpoint configuration is well-structured with appropriate HTTP method, path, SDK metadata, and dependency injection. The action wiring is correct.
✨ Benchmark results
⚡ Benchmark Comparison
|
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
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