Support for HTTP GET map parameters #6072

kwondh5217 · 2025-01-19T17:07:53Z

Motivation:

This pull request addresses #6058

Currently, @Param cannot be mapped to a Map, but this change enables that functionality, allowing query parameters to be handled as a Map.

Modifications:

Introduced logic in AnnotatedValueResolver to detect when a parameter is of type Map and the @Param annotation has an unspecified value.
- Future enhancements may include supporting the mapping of specific values to a Map when the @Param value is explicitly specified.
Added a new method ofQueryParamMap to handle the creation of an AnnotatedValueResolver for mapping all query parameters into a Map.
- Query parameters are collected into a Map using the stream-based collector.
Updated the existing logic to ensure this behavior only applies when the @Param value is unspecified and the parameter type is Map.

Result:

Closes Support for HTTP GET map parameters? #6058
Users can now define a method parameter annotated with @Param as Map<String, Object> to handle all query parameters.

github-actions · 2025-01-19T17:30:29Z

🔍 Build Scan® (commit: `51cce0f`)

Job name	Status	Build Scan®
build-ubicloud-standard-16-jdk-8	✅	https://ge.armeria.dev/s/fohrhw4bvjjz4
build-ubicloud-standard-16-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/zzlgo2ewhvpzs
build-ubicloud-standard-16-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/l3mvuvu7zgzga
build-ubicloud-standard-16-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/fp427wbizdrso
build-ubicloud-standard-16-jdk-17-leak	✅	https://ge.armeria.dev/s/uzpku6sgszio2
build-ubicloud-standard-16-jdk-11	✅	https://ge.armeria.dev/s/er3k6fvczgy5e
build-macos-latest-jdk-21	✅	https://ge.armeria.dev/s/rxqbnraw2bcme

minwoox

HI, @kwondh5217, thanks for the PR. 😉
Would you add an e2e test that verifies if this change works?

GET http://127.0.0.1:xxx/map?a=b&c=d

@Get("/map")
public HttpResponse map(@Params Map<String, Object> map) {...}

kwondh5217 · 2025-01-21T04:54:43Z

@minwoox Thanks for your comment!
I've added a test in AnnotatedServiceTest to verify that this change works.
Please take a look when you have time! 🙇

minwoox

Basically looks good. Left just nits. 👍

core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java

...rc/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java

kwondh5217 · 2025-02-04T11:54:49Z

@minwoox !
I’ve made the requested changes. Please take a look when you have time. 🙏

minwoox

Nice work, @kwondh5217! 😄

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java

kwondh5217 · 2025-02-10T07:14:58Z

@jrhee17 !
I have set up the IDE and reformatted all files. Thank you for your review!

Signed-off-by: Daeho Kwon <trewq231@naver.com>

minwoox · 2025-02-21T05:01:41Z

Nice work, @kwondh5217! 👏 👏 👏

kwondh5217 · 2025-02-21T05:25:28Z

@minwoox Thanks! 😄
I'll submit another PR to support Map<String, List> for handling multi-value query parameters.
Looking forward to your feedback on that as well! 🚀

minwoox · 2025-02-21T05:28:24Z

@kwondh5217 I can't wait to see that. 😆

kwondh5217 · 2025-02-23T14:40:17Z

Hi @minwoox ,
I've submitted a PR to support Map<String, List<?>> for handling multi-value query parameters.
When you have some time, could you take a look? 🙇‍♂️

kwondh5217 requested review from ikhoon, jrhee17, minwoox and trustin as code owners January 19, 2025 17:07

minwoox reviewed Jan 21, 2025

View reviewed changes

minwoox added this to the 1.32.0 milestone Jan 21, 2025

minwoox added the new feature label Jan 21, 2025

minwoox reviewed Jan 21, 2025

View reviewed changes

core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java Outdated Show resolved Hide resolved

...rc/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java Outdated Show resolved Hide resolved

minwoox reviewed Jan 21, 2025

View reviewed changes

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java Outdated Show resolved Hide resolved

kwondh5217 requested a review from minwoox February 5, 2025 01:39

minwoox approved these changes Feb 5, 2025

View reviewed changes

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java Outdated Show resolved Hide resolved

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java Outdated Show resolved Hide resolved

minwoox mentioned this pull request Feb 5, 2025

Support for HTTP GET map parameters? #6058

Closed

jrhee17 approved these changes Feb 10, 2025

View reviewed changes

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java Show resolved Hide resolved

ikhoon approved these changes Feb 19, 2025

View reviewed changes

kwondh5217 added 9 commits February 21, 2025 12:28

Add ofQueryParamMap method to mapping all query parameters to a Map

974daac

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Add ofQueryParamMap method to mapping all query parameters to a Map

8d48b4d

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Add tests for @Param Map<String, Object>

6c3a691

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Add tests for @Param Map<String, Object>

8920d8b

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Add tests for @Param Map<String, Object>

fd257ff

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Fix @Param handling to raise exception for Map with specified value

4d81e89

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Polish AnnotatedValueResolver

60f572c

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Style reformat code according to LINE OSS guidelines

43c879b

Signed-off-by: Daeho Kwon <trewq231@naver.com>

Style reformat code according to LINE OSS guidelines

Loading
Loading status checks…

51cce0f

Signed-off-by: Daeho Kwon <trewq231@naver.com>

kwondh5217 force-pushed the main branch from 702ecf7 to 51cce0f Compare February 21, 2025 03:41

minwoox merged commit b1a09d4 into line:main Feb 21, 2025
14 checks passed

kwondh5217 mentioned this pull request Feb 23, 2025

Support multi-value query parameters #6118

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for HTTP GET map parameters #6072

Support for HTTP GET map parameters #6072

kwondh5217 commented Jan 19, 2025 •

edited

Loading

github-actions bot commented Jan 19, 2025 •

edited

Loading

minwoox left a comment

kwondh5217 commented Jan 21, 2025

minwoox left a comment

kwondh5217 commented Feb 4, 2025

minwoox left a comment

kwondh5217 commented Feb 10, 2025

minwoox commented Feb 21, 2025

kwondh5217 commented Feb 21, 2025

minwoox commented Feb 21, 2025

kwondh5217 commented Feb 23, 2025

Support for HTTP GET map parameters #6072

Support for HTTP GET map parameters #6072

Conversation

kwondh5217 commented Jan 19, 2025 • edited Loading

Motivation:

Modifications:

Result:

github-actions bot commented Jan 19, 2025 • edited Loading

🔍 Build Scan® (commit: 51cce0f)

minwoox left a comment

Choose a reason for hiding this comment

kwondh5217 commented Jan 21, 2025

minwoox left a comment

Choose a reason for hiding this comment

kwondh5217 commented Feb 4, 2025

minwoox left a comment

Choose a reason for hiding this comment

kwondh5217 commented Feb 10, 2025

minwoox commented Feb 21, 2025

kwondh5217 commented Feb 21, 2025

minwoox commented Feb 21, 2025

kwondh5217 commented Feb 23, 2025

kwondh5217 commented Jan 19, 2025 •

edited

Loading

github-actions bot commented Jan 19, 2025 •

edited

Loading

🔍 Build Scan® (commit: `51cce0f`)