From b1a09d48e1186d9ffa7e867ab376e559f04f853c Mon Sep 17 00:00:00 2001 From: Daeho Kwon <100909553+kwondh5217@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:01:36 +0900 Subject: [PATCH] Support for HTTP GET map parameters (#6072) ## 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 #6058 - Users can now define a method parameter annotated with `@Param` as `Map` to handle all query parameters. --- .../annotation/AnnotatedValueResolver.java | 32 +++++++++++++++++ .../annotation/AnnotatedServiceTest.java | 35 +++++++++++++++++++ .../AnnotatedValueResolverTest.java | 14 +++++++- 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java b/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java index 546c4f443a0..0e93c707a8e 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java @@ -48,6 +48,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.ServiceLoader; @@ -458,6 +459,18 @@ private static AnnotatedValueResolver of(AnnotatedElement annotatedElement, final Param param = annotatedElement.getAnnotation(Param.class); if (param != null) { final String name = findName(typeElement, param.value()); + // If the parameter is of type Map and the @Param annotation does not specify a value, + // map all query parameters into the Map. + if (Map.class.isAssignableFrom(type)) { + if (DefaultValues.isSpecified(param.value())) { + throw new IllegalArgumentException( + String.format("Invalid @Param annotation on Map parameter: '%s'. " + + "The @Param annotation specifies a value ('%s'), " + + "which is not allowed. ", + annotatedElement, param.value())); + } + return ofQueryParamMap(name, annotatedElement, typeElement, type, description); + } if (type == File.class || type == Path.class || type == MultipartFile.class) { return ofFileParam(name, annotatedElement, typeElement, type, description); } @@ -585,6 +598,25 @@ private static AnnotatedValueResolver ofQueryParam(String name, .build(); } + private static AnnotatedValueResolver ofQueryParamMap(String name, + AnnotatedElement annotatedElement, + AnnotatedElement typeElement, Class type, + DescriptionInfo description) { + + return new Builder(annotatedElement, type, name) + .annotationType(Param.class) + .typeElement(typeElement) + .description(description) + .aggregation(AggregationStrategy.FOR_FORM_DATA) + .resolver((resolver, ctx) -> ctx.queryParams().stream() + .collect(toImmutableMap( + Entry::getKey, + Entry::getValue, + (existing, replacement) -> replacement + ))) + .build(); + } + private static AnnotatedValueResolver ofFileParam(String name, AnnotatedElement annotatedElement, AnnotatedElement typeElement, Class type, diff --git a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java index bb5f68a8403..f32ff8afd5f 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java @@ -19,6 +19,7 @@ import static org.apache.hc.core5.http.HttpHeaders.CONTENT_TYPE; import static org.apache.hc.core5.http.HttpHeaders.IF_MATCH; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.nio.charset.Charset; @@ -26,6 +27,7 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; @@ -75,6 +77,7 @@ import com.linecorp.armeria.internal.testing.AnticipatedException; import com.linecorp.armeria.internal.testing.GenerateNativeImageTrace; import com.linecorp.armeria.server.HttpStatusException; +import com.linecorp.armeria.server.Server; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.TestConverters.NaiveIntConverterFunction; @@ -628,6 +631,14 @@ public String paramPrecedence(RequestContext ctx, validateContext(ctx); return username + '/' + password; } + + @Get("/param/map") + public String map(RequestContext ctx, @Param Map map) { + validateContext(ctx); + return map.isEmpty() ? "empty" : map.entrySet().stream() + .map(entry -> entry.getKey() + '=' + entry.getValue()) + .collect(Collectors.joining(", ")); + } } @ResponseConverter(UnformattedStringConverterFunction.class) @@ -925,6 +936,13 @@ public ResponseEntity responseEntityHttpResponse(RequestContext ct } } + public static class MyAnnotationService16 { + @Get("/param/map-invalid") + public String invalidMapParam(@Param("param") Map param) { + return "Should not reach here"; + } + } + @Test void testAnnotatedService() throws Exception { try (CloseableHttpClient hc = HttpClients.createMinimal()) { @@ -1057,6 +1075,11 @@ void testParam() throws Exception { testStatusCode(hc, get("/7/param/default2"), 400); testBody(hc, get("/7/param/default_null"), "(null)"); + + // Case all query parameters test map + testBody(hc, get("/7/param/map?key1=value1&key2=value2"), + "key1=value1, key2=value2"); + testBody(hc, get("/7/param/map"), "empty"); } } @@ -1353,6 +1376,18 @@ void testResponseEntity() throws Exception { } } + @Test + void testInvalidParamAnnotationUsageOnMap() { + assertThatThrownBy(() -> + Server.builder() + .annotatedService() + .build(new MyAnnotationService16()) + .build() + ) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid @Param annotation on Map parameter"); + } + private enum UserLevel { LV1, LV2 diff --git a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java index 7fd3fe0b472..d3acd4c0422 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java @@ -112,6 +112,7 @@ class AnnotatedValueResolverTest { static final ServiceRequestContext context; static final HttpRequest request; static final RequestHeaders originalHeaders; + static final String QUERY_PARAM_MAP = "queryParamMap"; static Map> successExpectAttrKeys; static Map> failExpectAttrKeys; @@ -363,7 +364,11 @@ private static void testResolver(AnnotatedValueResolver resolver) { } } } else { - assertThat(resolver.defaultValue()).isNotNull(); + if (QUERY_PARAM_MAP.equals(resolver.httpElementName())) { + assertThat(resolver.defaultValue()).isNull(); + } else { + assertThat(resolver.defaultValue()).isNotNull(); + } if (resolver.hasContainer() && List.class.isAssignableFrom(resolver.containerType())) { assertThat((List) value).hasSize(1) .containsOnly(resolver.defaultValue()); @@ -371,6 +376,12 @@ private static void testResolver(AnnotatedValueResolver resolver) { .isEqualTo(resolver.elementType()); } else if (resolver.shouldWrapValueAsOptional()) { assertThat(value).isEqualTo(Optional.of(resolver.defaultValue())); + } else if (QUERY_PARAM_MAP.equals(resolver.httpElementName())) { + assertThat(value).isNotNull(); + assertThat(value).isInstanceOf(Map.class); + assertThat((Map) value).size() + .isEqualTo(existingHttpParameters.size() + + existingWithoutValueParameters.size()); } else { assertThat(value).isEqualTo(resolver.defaultValue()); } @@ -447,6 +458,7 @@ void method1(@Param String var1, @Param @Default Integer emptyParam2, @Param @Default List emptyParam3, @Param @Default List emptyParam4, + @Param Map queryParamMap, @Header List header1, @Header("header1") Optional> optionalHeader1, @Header String header2,