Skip to content

Commit

Permalink
working version for basic case of multiple paths
Browse files Browse the repository at this point in the history
  • Loading branch information
jrhee17 committed Oct 3, 2019
1 parent 9fb403e commit 781d06a
Show file tree
Hide file tree
Showing 12 changed files with 431 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,12 @@ private static void addMethodInfo(Map<Class<?>, Set<MethodInfo>> methodInfos,
final TypeSignature returnTypeSignature = toTypeSignature(method.getGenericReturnType());
final List<FieldInfo> fieldInfos = fieldInfos(service.annotatedValueResolvers());
final Class<?> clazz = service.object().getClass();
route.methods().forEach(
httpMethod -> {
final MethodInfo methodInfo = new MethodInfo(
name, returnTypeSignature, fieldInfos, ImmutableList.of(), // Ignore exceptions.
ImmutableList.of(endpoint), httpMethod, findDescription(method));
methodInfos.computeIfAbsent(clazz, unused -> new HashSet<>()).add(methodInfo);
});
route.methods().forEach(httpMethod -> {
final MethodInfo methodInfo = new MethodInfo(
name, returnTypeSignature, fieldInfos, ImmutableList.of(), // Ignore exceptions.
ImmutableList.of(endpoint), httpMethod, findDescription(method));
methodInfos.computeIfAbsent(clazz, unused -> new HashSet<>()).add(methodInfo);
});
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public class AnnotatedHttpService implements HttpService {
responseConverter = responseConverter(
method, requireNonNull(responseConverters, "responseConverters"), exceptionHandler);
aggregationStrategy = AggregationStrategy.from(resolvers);

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

this.defaultHttpHeaders = requireNonNull(defaultHttpHeaders, "defaultHttpHeaders");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,9 @@ public static List<AnnotatedHttpServiceElement> find(String pathPrefix, Object o

final List<Method> methods = requestMappingMethods(object);
return methods.stream()
.map((Method method) -> create(pathPrefix, object, method, exceptionHandlerFunctions,
requestConverterFunctions, responseConverterFunctions))
.flatMap((Method method) ->
create(pathPrefix, object, method, exceptionHandlerFunctions,
requestConverterFunctions, responseConverterFunctions).stream())
.collect(toImmutableList());
}

Expand Down Expand Up @@ -255,14 +256,14 @@ private static <T extends Annotation> void setAdditionalHeader(HttpHeadersBuilde
}

/**
* Returns an {@link AnnotatedHttpService} instance defined to {@code method} of {@code object} using
* {@link Path} annotation.
* Returns an list of {@link AnnotatedHttpService} instances defined to {@code method} of {@code object}
* using {@link Path} annotation.
*/
@VisibleForTesting
static AnnotatedHttpServiceElement create(String pathPrefix, Object object, Method method,
List<ExceptionHandlerFunction> baseExceptionHandlers,
List<RequestConverterFunction> baseRequestConverters,
List<ResponseConverterFunction> baseResponseConverters) {
static List<AnnotatedHttpServiceElement> create(String pathPrefix, Object object, Method method,
List<ExceptionHandlerFunction> baseExceptionHandlers,
List<RequestConverterFunction> baseRequestConverters,
List<ResponseConverterFunction> baseResponseConverters) {

final Set<Annotation> methodAnnotations = httpMethodAnnotations(method);
if (methodAnnotations.isEmpty()) {
Expand All @@ -275,15 +276,16 @@ static AnnotatedHttpServiceElement create(String pathPrefix, Object object, Meth
" must have an HTTP method annotation.");
}
final Class<?> clazz = object.getClass();
final String pattern = findPattern(method, methodAnnotations);
final Set<String> patterns = findPatterns(method, methodAnnotations);
final String computedPathPrefix = computePathPrefix(clazz, pathPrefix);

final Route route = Route.builder()
.path(computedPathPrefix, pattern)
.methods(methods)
.consumes(consumableMediaTypes(method, clazz))
.produces(producibleMediaTypes(method, clazz))
.build();
final List<Route> routes = patterns.stream().map(
pattern -> Route.builder()
.path(computedPathPrefix, pattern)
.methods(methods)
.consumes(consumableMediaTypes(method, clazz))
.produces(producibleMediaTypes(method, clazz))
.build()).collect(Collectors.toList());

final List<ExceptionHandlerFunction> eh =
getAnnotatedInstances(method, clazz, ExceptionHandler.class, ExceptionHandlerFunction.class)
Expand All @@ -295,41 +297,6 @@ static AnnotatedHttpServiceElement create(String pathPrefix, Object object, Meth
getAnnotatedInstances(method, clazz, ResponseConverter.class, ResponseConverterFunction.class)
.addAll(baseResponseConverters).build();

List<AnnotatedValueResolver> resolvers;
try {
resolvers = AnnotatedValueResolver.ofServiceMethod(method, route.paramNames(),
toRequestObjectResolvers(req));
} catch (NoParameterException ignored) {
// Allow no parameter like below:
//
// @Get("/")
// public String method1() { ... }
//
resolvers = ImmutableList.of();
}

final Set<String> expectedParamNames = route.paramNames();
final Set<String> requiredParamNames =
resolvers.stream()
.filter(AnnotatedValueResolver::isPathVariable)
.map(AnnotatedValueResolver::httpElementName)
.collect(Collectors.toSet());

if (!expectedParamNames.containsAll(requiredParamNames)) {
final Set<String> missing = Sets.difference(requiredParamNames, expectedParamNames);
throw new IllegalArgumentException("cannot find path variables: " + missing);
}

// Warn unused path variables only if there's no '@RequestObject' annotation.
if (resolvers.stream().noneMatch(r -> r.annotationType() == RequestObject.class) &&
!requiredParamNames.containsAll(expectedParamNames)) {
final Set<String> missing = Sets.difference(expectedParamNames, requiredParamNames);
logger.warn("Some path variables of the method '" + method.getName() +
"' of the class '" + clazz.getName() +
"' do not have their corresponding parameters annotated with @Param. " +
"They would not be automatically injected: " + missing);
}

final Optional<HttpStatus> defaultResponseStatus = findFirst(method, StatusCode.class)
.map(code -> {
final int statusCode = code.value();
Expand Down Expand Up @@ -376,11 +343,61 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc
}
};
}
return new AnnotatedHttpServiceElement(route, new AnnotatedHttpService(object, method, resolvers,
eh, res, route,
defaultHeaders.build(),
defaultTrailers.build()),
decorator(method, clazz, initialDecorator));

final ResponseHeaders responseHeaders = defaultHeaders.build();
final HttpHeaders responseTrailers = defaultTrailers.build();

return routes.stream().map(route -> {
final List<AnnotatedValueResolver> resolvers =
getAnnotatedValueResolvers(req, route, method, clazz);
return new AnnotatedHttpServiceElement(route,
new AnnotatedHttpService(object, method,
resolvers, eh, res, route,
responseHeaders,
responseTrailers),
decorator(method, clazz, initialDecorator));
}).collect(Collectors.toList());
}

private static List<AnnotatedValueResolver> getAnnotatedValueResolvers(List<RequestConverterFunction> req,
Route route, Method method,
Class<?> clazz) {
final Set<String> expectedParamNames = route.paramNames();
List<AnnotatedValueResolver> resolvers;
try {
resolvers = AnnotatedValueResolver.ofServiceMethod(method, expectedParamNames,
toRequestObjectResolvers(req));
} catch (NoParameterException ignored) {
// Allow no parameter like below:
//
// @Get("/")
// public String method1() { ... }
//
resolvers = ImmutableList.of();
}

final Set<String> requiredParamNames =
resolvers.stream()
.filter(AnnotatedValueResolver::isPathVariable)
.map(AnnotatedValueResolver::httpElementName)
.collect(Collectors.toSet());

if (!expectedParamNames.containsAll(requiredParamNames)) {
final Set<String> missing = Sets.difference(requiredParamNames, expectedParamNames);
throw new IllegalArgumentException("cannot find path variables: " + missing);
}

// Warn unused path variables only if there's no '@RequestObject' annotation.
if (resolvers.stream().noneMatch(r -> r.annotationType() == RequestObject.class) &&
!requiredParamNames.containsAll(expectedParamNames)) {
final Set<String> missing = Sets.difference(expectedParamNames, requiredParamNames);
logger.warn("Some path variables of the method '" + method.getName() +
"' of the class '" + clazz.getName() +
"' do not have their corresponding parameters annotated with @Param. " +
"They would not be automatically injected: " + missing);
}

return resolvers;
}

/**
Expand Down Expand Up @@ -512,23 +529,20 @@ private static Set<MediaType> listToSet(List<MediaType> types, Class<?> annotati
* Returns a specified path pattern. The path pattern might be specified by {@link Path} or
* HTTP method annotations such as {@link Get} and {@link Post}.
*/
private static String findPattern(Method method, Set<Annotation> methodAnnotations) {
String pattern = findFirst(method, Path.class).map(Path::value)
.orElse(null);
for (Annotation a : methodAnnotations) {
final String p = (String) invokeValueMethod(a);
if (DefaultValues.isUnspecified(p)) {
continue;
}
checkArgument(pattern == null,
"Only one path can be specified. (" + pattern + ", " + p + ')');
pattern = p;
}
if (pattern == null || pattern.isEmpty()) {
private static Set<String> findPatterns(Method method, Set<Annotation> methodAnnotations) {
final Set<String> patterns = findAll(method, Path.class).stream().map(Path::value)
.collect(Collectors.toSet());
final Set<String> httpMethodPatterns =
methodAnnotations.stream()
.map(annotation -> (String) invokeValueMethod(annotation))
.filter(DefaultValues::isSpecified)
.collect(Collectors.toSet());
patterns.addAll(httpMethodPatterns);
if (patterns.isEmpty()) {
throw new IllegalArgumentException(
"A path pattern should be specified by @Path or HTTP method annotations.");
}
return pattern;
return patterns;
}

/**
Expand Down
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();
}
Loading

0 comments on commit 781d06a

Please sign in to comment.