Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for HTTP GET map parameters #6072

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,17 @@ 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 +597,25 @@ private static AnnotatedValueResolver ofQueryParam(String name,
.build();
}

private static AnnotatedValueResolver ofQueryParamMap(String name,
kwondh5217 marked this conversation as resolved.
Show resolved Hide resolved
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,23 @@ 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 +457,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
Loading