Add support for RFC 10008 (The HTTP QUERY Method) - #6861
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds HTTP ChangesHTTP QUERY method support
Netty version update
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WebClient
participant HttpServer
participant AnnotatedServiceFactory
participant QueryHandler
Client->>WebClient: send QUERY request
WebClient->>HttpServer: execute QUERY with path and body
HttpServer->>AnnotatedServiceFactory: resolve QUERY route
AnnotatedServiceFactory->>QueryHandler: invoke `@Query` handler
QueryHandler-->>Client: return QUERY response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/src/main/java/com/linecorp/armeria/client/WebClient.java`:
- Around line 603-671: Mark all eight new query(...) overloads in
core/src/main/java/com/linecorp/armeria/client/WebClient.java:603-671 with
`@UnstableApi` and retain or add complete Javadoc. Also mark query(String path)
with `@UnstableApi` and add Javadoc in
core/src/main/java/com/linecorp/armeria/common/AbstractHttpRequestBuilder.java:78-82,
core/src/main/java/com/linecorp/armeria/common/HttpRequestBuilder.java:67-71,
and
core/src/main/java/com/linecorp/armeria/client/WebClientRequestPreparation.java:455-459.
In `@core/src/main/java/com/linecorp/armeria/server/annotation/Query.java`:
- Around line 31-33: Add the existing `@UnstableApi` annotation to the public
Query annotation declaration, including the required import, while preserving
its current retention and target metadata.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66c09182-7cd5-494e-b561-b141bc7483d0
📒 Files selected for processing (24)
annotation-processor/src/main/java/com/linecorp/armeria/server/annotation/processor/DocumentationProcessor.javacore/src/main/java/com/linecorp/armeria/client/BlockingWebClient.javacore/src/main/java/com/linecorp/armeria/client/BlockingWebClientRequestPreparation.javacore/src/main/java/com/linecorp/armeria/client/ClientHttp1ObjectEncoder.javacore/src/main/java/com/linecorp/armeria/client/FutureTransformingRequestPreparation.javacore/src/main/java/com/linecorp/armeria/client/RestClient.javacore/src/main/java/com/linecorp/armeria/client/TransformingRequestPreparation.javacore/src/main/java/com/linecorp/armeria/client/WebClient.javacore/src/main/java/com/linecorp/armeria/client/WebClientRequestPreparation.javacore/src/main/java/com/linecorp/armeria/common/AbstractHttpRequestBuilder.javacore/src/main/java/com/linecorp/armeria/common/HttpHeaderNames.javacore/src/main/java/com/linecorp/armeria/common/HttpMethod.javacore/src/main/java/com/linecorp/armeria/common/HttpRequestBuilder.javacore/src/main/java/com/linecorp/armeria/common/RequestMethodSetters.javacore/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceFactory.javacore/src/main/java/com/linecorp/armeria/server/AbstractHttpService.javacore/src/main/java/com/linecorp/armeria/server/annotation/Query.javacore/src/main/resources/META-INF/native-image/com.linecorp.armeria/armeria/reflect-config.jsoncore/src/test/java/com/linecorp/armeria/common/HttpHeaderNamesTest.javacore/src/test/java/com/linecorp/armeria/common/HttpMethodTest.javacore/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceFactoryTest.javacore/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.javascala/scala_2.13/src/main/scala/com/linecorp/armeria/client/scala/ScalaRestClient.scalascala/scala_2.13/src/test/scala/com/linecorp/armeria/client/scala/RestClientSuite.scala
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6861 +/- ##
============================================
+ Coverage 74.46% 75.12% +0.66%
- Complexity 22234 25419 +3185
============================================
Files 1963 2265 +302
Lines 82437 94389 +11952
Branches 10764 12340 +1576
============================================
+ Hits 61385 70912 +9527
- Misses 15918 17620 +1702
- Partials 5134 5857 +723 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jrhee17
left a comment
There was a problem hiding this comment.
Changes look reasonable to me 👍
Motivation:
Implement Support for RFC 10008
Modifications:
HttpMethod.QUERYsupport in core method handling.QUERYenum constant and RFC reference inHttpMethod.QUERYto supported method parsing and lookup paths.QUERYto the idempotent methods set.Accept-Queryheader constant.HttpHeaderNames.ACCEPT_QUERY.query(String path)support in request builders.RequestMethodSetters.AbstractHttpRequestBuilderand overridden inHttpRequestBuilder.QUERY.query(...)overloads inWebClientandBlockingWebClient.query(String path)inRestClient.query(String path)in request preparation/decorator classes.QUERY.ClientHttp1ObjectEncodersoQUERYfollows the same zero-length body handling path as POST/PUT/PATCH.QUERY.QUERYdispatch anddoQuery(...)hook inAbstractHttpService.@Queryannotation (core/.../server/annotation/Query.java).@Query -> HttpMethod.QUERYinAnnotatedServiceFactory.DocumentationProcessor.@Query.com.linecorp.armeria.server.annotation.Queryentry inreflect-config.json.HttpMethod.QUERYparse/support/idempotency.HttpHeaderNames.ACCEPT_QUERY.@Queryrouting and handling.@Queryuse in annotated endpoints.query(pathPattern)inScalaRestClient.Result:
QUERYrequests through Java and Scala high-level clients.QUERYvia bothAbstractHttpService#doQuery(...)and@Queryannotated methods.Accept-Queryheader usage is now exposed as a standard constant.@Queryannotation.This is part of an ongoing effort of yours truly to enable QUERY method usage across the JVM ecosystem.