Skip to content

Commit

Permalink
Support for HTTP GET map parameters (#6072)
Browse files Browse the repository at this point in the history
## 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<String, Object>` to handle all query parameters.
  • Loading branch information
kwondh5217 authored Feb 21, 2025
1 parent 242d029 commit b1a09d4
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
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;
import java.nio.charset.StandardCharsets;
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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -628,6 +631,14 @@ public String paramPrecedence(RequestContext ctx,
validateContext(ctx);
return username + '/' + password;
}

@Get("/param/map")
public String map(RequestContext ctx, @Param Map<String, Object> map) {
validateContext(ctx);
return map.isEmpty() ? "empty" : map.entrySet().stream()
.map(entry -> entry.getKey() + '=' + entry.getValue())
.collect(Collectors.joining(", "));
}
}

@ResponseConverter(UnformattedStringConverterFunction.class)
Expand Down Expand Up @@ -925,6 +936,13 @@ public ResponseEntity<HttpResponse> responseEntityHttpResponse(RequestContext ct
}
}

public static class MyAnnotationService16 {
@Get("/param/map-invalid")
public String invalidMapParam(@Param("param") Map<String, String> param) {
return "Should not reach here";
}
}

@Test
void testAnnotatedService() throws Exception {
try (CloseableHttpClient hc = HttpClients.createMinimal()) {
Expand Down Expand Up @@ -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");
}
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, AttributeKey<?>> successExpectAttrKeys;
static Map<String, AttributeKey<?>> failExpectAttrKeys;

Expand Down Expand Up @@ -363,14 +364,24 @@ 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<Object>) value).hasSize(1)
.containsOnly(resolver.defaultValue());
assertThat(((List<Object>) value).get(0).getClass())
.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());
}
Expand Down Expand Up @@ -447,6 +458,7 @@ void method1(@Param String var1,
@Param @Default Integer emptyParam2,
@Param @Default List<String> emptyParam3,
@Param @Default List<Integer> emptyParam4,
@Param Map<String, Object> queryParamMap,
@Header List<String> header1,
@Header("header1") Optional<List<ValueEnum>> optionalHeader1,
@Header String header2,
Expand Down

0 comments on commit b1a09d4

Please sign in to comment.