Skip to content

Commit

Permalink
Support multiple Path annotations
Browse files Browse the repository at this point in the history
Motivation:
A user might want to specify multiple paths for annotated http services. 

Modification:
- Allow repeatable annotations for `@Path` annotation.
- Different behavior in path declaration (described below)
- One `AnnotatedHttpService` is created for each `(httpMethod, pathMapping)`
    - It is difficult to accommodate multiple pathMappings using a single `AnnotatedHttpService` because `AnnotatedValueResolver` may be different
    - It may be possible to optimize this by serving multiple  `httpMethod`s that have the same `pathMapping` using a single `AnnotatedHttpService` at the cost of code complexiy
- Merge `MethodInfo` endpoints with same (`httpMethod`, `methodName`)
- Known issue: overloaded methods for `AnnotatedHttpService` are not displayed in docs. I'm thinking this can be tackled in a separate PR.

#### Breaking change in path declaration behavior
**AS-IS**
```
@get("/")
@post
public void method1() {} // accepts both Get("/") and Post("/")
```
**TO-BE**
```
@get("/")
@post
public void method1() {} // exception thrown. Path declared for Get not applied to Post
```
  • Loading branch information
jrhee17 authored and minwoox committed Oct 23, 2019
1 parent cca8e3c commit c760adf
Show file tree
Hide file tree
Showing 13 changed files with 733 additions and 167 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016 LINE Corporation
* Copyright 2019 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
Expand All @@ -17,6 +17,7 @@
package com.linecorp.armeria.server.annotation;

import java.lang.annotation.ElementType;
import java.lang.annotation.Repeatable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
Expand All @@ -26,6 +27,7 @@
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
@Repeatable(value = Paths.class)
public @interface Path {

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright 2019 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.server.annotation;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* The containing annotation type for {@link Path}.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface Paths {
/**
* An array of {@link Path}s.
*/
Path[] value();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;

import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

Expand All @@ -29,12 +31,15 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Sets;

import com.linecorp.armeria.common.HttpHeaders;
import com.linecorp.armeria.common.HttpMethod;
import com.linecorp.armeria.server.Service;

/**
Expand Down Expand Up @@ -74,11 +79,7 @@ public ServiceInfo(String name,
@Nullable String docString) {

this.name = requireNonNull(name, "name");

requireNonNull(methods, "methods");

this.methods = ImmutableSortedSet.copyOf(comparing(MethodInfo::name)
.thenComparing(MethodInfo::httpMethod), methods);
this.methods = mergeEndpoints(requireNonNull(methods));
this.exampleHttpHeaders = ImmutableList.copyOf(requireNonNull(exampleHttpHeaders,
"exampleHttpHeaders"));
this.docString = Strings.emptyToNull(docString);
Expand All @@ -100,6 +101,37 @@ public Set<MethodInfo> methods() {
return methods;
}

/**
* Merges the {@link MethodInfo}s with the same method name and {@link HttpMethod} pair
* into a single {@link MethodInfo}. Note that only the {@link EndpointInfo}s are merged
* because the {@link MethodInfo}s being merged always have the same
* {@code exampleHttpHeaders} and {@code exampleRequests}.
*/
@VisibleForTesting
static Set<MethodInfo> mergeEndpoints(Iterable<MethodInfo> methodInfos) {
final Map<List<Object>, MethodInfo> methodInfoMap = new HashMap<>();
for (MethodInfo methodInfo : methodInfos) {
final List<Object> mergeKey = ImmutableList.of(methodInfo.name(), methodInfo.httpMethod());
methodInfoMap.compute(mergeKey, (key, value) -> {
if (value == null) {
return methodInfo;
} else {
final Set<EndpointInfo> endpointInfos =
Sets.union(value.endpoints(), methodInfo.endpoints());
return new MethodInfo(value.name(), value.returnTypeSignature(),
value.parameters(), value.exceptionTypeSignatures(),
endpointInfos, value.exampleHttpHeaders(),
value.exampleRequests(), value.httpMethod(),
value.docString());
}
});
}
return ImmutableSortedSet
.orderedBy(comparing(MethodInfo::name).thenComparing(MethodInfo::httpMethod))
.addAll(methodInfoMap.values())
.build();
}

/**
* Returns all enum, struct and exception {@link TypeSignature}s referred to by this service.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.linecorp.armeria.internal.annotation.AnnotatedHttpDocServicePlugin.BEAN;
import static com.linecorp.armeria.internal.annotation.AnnotatedHttpDocServicePlugin.INT;
import static com.linecorp.armeria.internal.annotation.AnnotatedHttpDocServicePlugin.LONG;
Expand All @@ -31,6 +32,7 @@
import static com.linecorp.armeria.server.docs.FieldRequirement.REQUIRED;
import static org.assertj.core.api.Assertions.assertThat;

import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
Expand All @@ -54,9 +56,11 @@
import com.linecorp.armeria.server.Route;
import com.linecorp.armeria.server.RouteBuilder;
import com.linecorp.armeria.server.Server;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.annotation.Get;
import com.linecorp.armeria.server.annotation.Header;
import com.linecorp.armeria.server.annotation.Param;
import com.linecorp.armeria.server.annotation.Path;
import com.linecorp.armeria.server.annotation.RequestObject;
import com.linecorp.armeria.server.docs.DocServiceFilter;
import com.linecorp.armeria.server.docs.EndpointInfo;
Expand Down Expand Up @@ -218,7 +222,9 @@ private static Route withMethodAndTypes(RouteBuilder builder) {
@Test
public void testGenerateSpecification() {
final Map<String, ServiceInfo> services = services((plugin, service, method) -> true,
(plugin, service, method) -> false);
(plugin, service, method) -> false,
new FooClass(),
new BarClass());

assertThat(services).containsOnlyKeys(FOO_NAME, BAR_NAME);
checkFooService(services.get(FOO_NAME));
Expand All @@ -237,12 +243,12 @@ public void include() {
// 1. Nothing specified.
DocServiceFilter include = (plugin, service, method) -> true;
DocServiceFilter exclude = (plugin, service, method) -> false;
Map<String, ServiceInfo> services = services(include, exclude);
Map<String, ServiceInfo> services = services(include, exclude, new FooClass(), new BarClass());
assertThat(services).containsOnlyKeys(FOO_NAME, BAR_NAME);

// 2. Exclude specified.
exclude = DocServiceFilter.ofMethodName(FOO_NAME, "fooMethod");
services = services(include, exclude);
services = services(include, exclude, new FooClass(), new BarClass());
assertThat(services).containsOnlyKeys(FOO_NAME, BAR_NAME);

List<String> methods = methods(services);
Expand All @@ -252,15 +258,15 @@ public void include() {
include = DocServiceFilter.ofServiceName(FOO_NAME);
// Set the exclude to the default.
exclude = (plugin, service, method) -> false;
services = services(include, exclude);
services = services(include, exclude, new FooClass(), new BarClass());
assertThat(services).containsOnlyKeys(FOO_NAME);

methods = methods(services);
assertThat(methods).containsExactlyInAnyOrder("fooMethod", "foo2Method");

// 3-2. Include methodName specified.
include = DocServiceFilter.ofMethodName(FOO_NAME, "fooMethod");
services = services(include, exclude);
services = services(include, exclude, new FooClass(), new BarClass());
assertThat(services).containsOnlyKeys(FOO_NAME);

methods = methods(services);
Expand All @@ -269,7 +275,7 @@ public void include() {
// 4-1. Include and exclude specified.
include = DocServiceFilter.ofServiceName(FOO_NAME);
exclude = DocServiceFilter.ofMethodName(FOO_NAME, "fooMethod");
services = services(include, exclude);
services = services(include, exclude, new FooClass());
assertThat(services).containsOnlyKeys(FOO_NAME);

methods = methods(services);
Expand All @@ -278,10 +284,22 @@ public void include() {
// 4-2. Include and exclude specified.
include = DocServiceFilter.ofMethodName(FOO_NAME, "fooMethod");
exclude = DocServiceFilter.ofServiceName(FOO_NAME);
services = services(include, exclude);
services = services(include, exclude, new FooClass(), new BarClass());
assertThat(services.size()).isZero();
}

@Test
public void testMultiPath() {
final Map<String, ServiceInfo> services = services(new MultiPathClass());
final Map<String, MethodInfo> methods =
services.get(MultiPathClass.class.getName()).methods().stream()
.collect(toImmutableMap(MethodInfo::name, Function.identity()));
final Set<String> paths = methods.get("multiGet").endpoints()
.stream().map(EndpointInfo::pathMapping)
.collect(toImmutableSet());
assertThat(paths).containsOnly("exact:/path1", "exact:/path2");
}

private static void checkFooService(ServiceInfo fooServiceInfo) {
assertThat(fooServiceInfo.exampleHttpHeaders()).isEmpty();
final Map<String, MethodInfo> methods =
Expand Down Expand Up @@ -330,11 +348,18 @@ private static void checkBarService(ServiceInfo barServiceInfo) {
assertFieldInfos(fieldInfos, ImmutableList.of(bar, compositeBean()));
}

private static Map<String, ServiceInfo> services(DocServiceFilter include, DocServiceFilter exclude) {
final Server server = Server.builder()
.annotatedService(new FooClass())
.annotatedService(new BarClass())
.build();
private static Map<String, ServiceInfo> services(Object... services) {
final DocServiceFilter include = (plugin, service, method) -> true;
final DocServiceFilter exclude = (plugin, service, method) -> false;
return services(include, exclude, services);
}

private static Map<String, ServiceInfo> services(DocServiceFilter include,
DocServiceFilter exclude,
Object... services) {
final ServerBuilder builder = Server.builder();
Arrays.stream(services).forEach(builder::annotatedService);
final Server server = builder.build();
final ServiceSpecification specification =
plugin.generateSpecification(ImmutableSet.copyOf(server.serviceConfigs()),
unifyFilter(include, exclude));
Expand Down Expand Up @@ -412,6 +437,13 @@ private static class BarClass {
public void barMethod(@Param String bar, CompositeBean compositeBean) {}
}

private static class MultiPathClass {
@Get
@Path("/path1")
@Path("/path2")
public void multiGet() {}
}

static class CompositeBean {
@RequestObject
private RequestBean1 bean1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public void jsonSpecification() throws InterruptedException {
addPrefixMethodInfo(methodInfos);
addConsumesMethodInfo(methodInfos);
addBeanMethodInfo(methodInfos);
addMultiMethodInfo(methodInfos);
final Map<Class<?>, String> serviceDescription = ImmutableMap.of(MyService.class, "My service class");

final JsonNode expectedJson = mapper.valueToTree(AnnotatedHttpDocServicePlugin.generate(
Expand Down Expand Up @@ -245,6 +246,17 @@ private static void addBeanMethodInfo(Map<Class<?>, Set<MethodInfo>> methodInfos
methodInfos.computeIfAbsent(MyService.class, unused -> new HashSet<>()).add(methodInfo);
}

private static void addMultiMethodInfo(Map<Class<?>, Set<MethodInfo>> methodInfos) {
final EndpointInfo endpoint1 = new EndpointInfoBuilder("*", "exact:/service/multi")
.availableMimeTypes(MediaType.JSON_UTF_8).build();
final EndpointInfo endpoint2 = new EndpointInfoBuilder("*", "prefix:/service/multi2/")
.availableMimeTypes(MediaType.JSON_UTF_8).build();
final MethodInfo methodInfo = new MethodInfo(
"multi", TypeSignature.ofBase("HttpResponse"), ImmutableList.of(), ImmutableList.of(),
ImmutableList.of(endpoint1, endpoint2), HttpMethod.GET, null);
methodInfos.computeIfAbsent(MyService.class, unused -> new HashSet<>()).add(methodInfo);
}

private static void addExamples(JsonNode json) {
// Add the global example.
((ArrayNode) json.get("exampleHttpHeaders")).add(mapper.valueToTree(EXAMPLE_HEADERS_ALL));
Expand Down Expand Up @@ -372,6 +384,13 @@ public HttpResponse exclude1() {
public HttpResponse exclude2() {
return HttpResponse.of(200);
}

@Get
@Path("/multi")
@Path("prefix:/multi2")
public HttpResponse multi() {
return HttpResponse.of(200);
}
}

private enum MyEnum {
Expand Down
Loading

0 comments on commit c760adf

Please sign in to comment.