Skip to content

Replace deprecated route method access - #12867

Merged
ChiragAgg5k merged 2 commits into
mainfrom
fix-route-method-deprecation
Jul 14, 2026
Merged

Replace deprecated route method access#12867
ChiragAgg5k merged 2 commits into
mainfrom
fix-route-method-deprecation

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

What does this PR do?

Replaces deprecated Utopia\Http\Route::getMethod() calls with the multi-method getMethods() API.

Route methods are preserved rather than defaulting to the first entry: logs and mock output use comma-separated methods, GraphQL resolvers receive the concrete registered method, and OpenAPI emits an operation for every method. This removes PHP 8.5 deprecation warnings and keeps multi-method routes accurate.

Test Plan

  • Run composer lint on all changed PHP files.
  • Run docker compose exec appwrite test tests/unit/SDK/Specification/FormatTest.php.
  • Run docker compose exec appwrite test tests/unit/GraphQL.
  • Run docker compose exec appwrite test tests/e2e/Services/GraphQL/PresenceTest.php.

Related PRs and Issues

  • #XXXX

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes all PHP 8.5 deprecation warnings caused by Route::getMethod() and replaces them with the multi-method getMethods() API across error handlers, the mock shutdown hook, GraphQL resolvers/schema, and the OpenAPI 3 formatter.

  • general.php and http.php now log comma-joined methods; mock.php stores per-method result keys and the previous review comment about key format has been addressed.
  • OpenAPI3.php is refactored to collect parameters into $parameterNodes first and then emit a separate operation per HTTP verb, with a unique operationId suffix when a route has more than one method — addressing the prior duplicate-operationId finding. A new unit test covers the two-method (GET+POST) path.
  • Schema.php / Resolvers.php / Mapper.php thread the concrete $httpMethod from getRoutes()'s outer key through to each GraphQL resolver so requests are dispatched with the right verb.

Confidence Score: 5/5

Safe to merge — all changes are mechanical replacements of the deprecated single-method API, and no multi-method routes currently exist in Appwrite, so the one latent issue in Schema.php has no runtime impact today.

All eight changed files correctly replace getMethod() with getMethods(). The only non-trivial concern is that Schema.php now processes a multi-method route once per method key returned by getRoutes(), which would silently add the same operation to both GraphQL queries and mutations for a GET+POST route. Because Appwrite currently has no multi-method routes, this does not affect running code; it is a latent issue for future route registrations and does not block this change.

src/Appwrite/GraphQL/Schema.php — the outer-loop $httpMethod key approach works today but will produce duplicate query+mutation entries if a multi-method route is ever registered.

Important Files Changed

Filename Overview
src/Appwrite/GraphQL/Schema.php Uses $httpMethod key from getRoutes() outer loop; correct for single-method routes, but multi-method routes would be processed once per method key (query AND mutation with same name), which may be unintentional
src/Appwrite/SDK/Specification/Format/OpenAPI3.php Refactors param collection into $parameterNodes and iterates getMethods() to emit a separate OpenAPI operation per HTTP verb, with unique operationId suffix when methods > 1; addresses prior review finding
app/controllers/mock.php Stores test-coverage results under per-method keys (addressing previous review comment) and uses comma-joined methods in the response body; intentional design
tests/unit/SDK/Specification/FormatTest.php Adds testMultiMethodRouteEmitsEveryOperation verifying GET and POST produce distinct operations with correct operationId, parameter placement, and request body handling

Reviews (2): Last reviewed commit: "fix: handle multi-method route outputs" | Re-trigger Greptile

Comment thread src/Appwrite/SDK/Specification/Format/OpenAPI3.php Outdated
Comment thread app/controllers/mock.php Outdated
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit aca1dfc - 1 flaky test
Test Retries Total Time Details
FunctionsCustomServerTest::testGetDeployment 1 22ms Logs
Commit 10d2465 - 4 flaky tests
Test Retries Total Time Details
RealtimeConsoleClientTest::testIndexesTablesAPI 1 497ms Logs
RealtimeConsoleClientTest::testDeleteAttributeCollectionsAPI 1 245ms Logs
RealtimeConsoleClientTest::testDeleteAttributeTablesAPI 1 238ms Logs
RealtimeConsoleClientTest::testPing 1 23ms Logs

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

✨ Benchmark results

Comparing main (before) → fix-route-method-deprecation (after).

Metric Before After Change
🚀 Requests/sec 201.96 200.88 -0.5%
⏱️ Latency P50 59.53 ms 61.26 ms +2.9%
⏱️ Latency P95 290.84 ms 283.98 ms -2.4%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 61.26 283.98 12,540 200.88 -6.86
Account 94.09 288.42 660 11.07 -3.1
TablesDB 51.84 249.73 6,820 110.17 +1.48
Storage 60.13 324.27 3,300 54.7 -25.75
Functions 88.91 316.81 1,760 30.23 +2.06

Top API waits (after)

API request Max wait (ms)
storage.buckets.create 1,127.51
tablesdb.rows.increment 942.67
storage.files.delete 918.41
functions.create 917.52
storage.buckets.delete 841.9

@ChiragAgg5k
ChiragAgg5k merged commit 17ea6d4 into main Jul 14, 2026
42 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the fix-route-method-deprecation branch July 14, 2026 05:39
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.

2 participants