提交 fc04feb1 编写于 作者: T Tomasz Bak

Minor update according to the code review comments.

上级 ed01590c
......@@ -26,7 +26,7 @@ import com.netflix.ribbonclientextensions.proxy.annotation.Http;
import com.netflix.ribbonclientextensions.proxy.annotation.Http.Header;
import com.netflix.ribbonclientextensions.proxy.annotation.Http.HttpMethod;
import com.netflix.ribbonclientextensions.proxy.annotation.Hystrix;
import com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroupSpec;
import com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroup;
import com.netflix.ribbonclientextensions.proxy.annotation.TemplateName;
import com.netflix.ribbonclientextensions.proxy.annotation.Var;
import io.netty.buffer.ByteBuf;
......@@ -34,7 +34,7 @@ import io.netty.buffer.ByteBuf;
/**
* @author Tomasz Bak
*/
@ResourceGroupSpec(name = "movieServiceGroup")
@ResourceGroup(name = "movieServiceGroup")
public interface MovieService {
@TemplateName("recommendationsByUserId")
......
......@@ -16,9 +16,9 @@
package com.netflix.ribbonclientextensions.proxy;
import com.netflix.ribbonclientextensions.http.HttpResourceGroup;
import com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroupSpec;
import com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroup;
import static com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroupSpec.*;
import static com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroup.*;
/**
* @author Tomasz Bak
......@@ -31,7 +31,7 @@ class ClassTemplate<T> {
ClassTemplate(Class<T> clientInterface) {
this.clientInterface = clientInterface;
ResourceGroupSpec annotation = clientInterface.getAnnotation(ResourceGroupSpec.class);
ResourceGroup annotation = clientInterface.getAnnotation(ResourceGroup.class);
if (annotation != null) {
String name = annotation.name().trim();
resourceGroupName = name.isEmpty() ? null : annotation.name();
......
......@@ -33,6 +33,8 @@ import com.netflix.ribbonclientextensions.proxy.annotation.Http.HttpMethod;
import com.netflix.ribbonclientextensions.proxy.annotation.Hystrix;
import com.netflix.ribbonclientextensions.proxy.annotation.TemplateName;
import com.netflix.ribbonclientextensions.proxy.annotation.Var;
import io.netty.buffer.ByteBuf;
import io.reactivex.netty.protocol.http.client.RawContentSource;
import io.reactivex.netty.serialization.ContentTransformer;
import java.lang.annotation.Annotation;
......@@ -56,11 +58,12 @@ import static java.lang.String.*;
* @author Tomasz Bak
*/
class MethodTemplate {
private final Method method;
private final String templateName;
private final Http.HttpMethod httpMethod;
private final String path;
private final Map<String, String> headers;
private final Map<String, List<String>> headers;
private final String[] paramNames;
private final int[] valueIdxs;
private final int contentArgPosition;
......@@ -69,7 +72,7 @@ class MethodTemplate {
private final String hystrixCacheKey;
private final FallbackHandler<?> hystrixFallbackHandler;
private final HttpResponseValidator hystrixResponseValidator;
private final Map<String, CacheProvider<?>> cacheProviders;
private final List<CacheProviderEntry> cacheProviders;
private final EvCacheOptions evCacheOptions;
MethodTemplate(Method method) {
......@@ -87,7 +90,7 @@ class MethodTemplate {
hystrixCacheKey = values.hystrixCacheKey;
hystrixFallbackHandler = values.hystrixFallbackHandler;
hystrixResponseValidator = values.hystrixResponseValidator;
cacheProviders = Collections.unmodifiableMap(values.cacheProviders);
cacheProviders = Collections.unmodifiableList(values.cacheProviders);
evCacheOptions = values.evCacheOptions;
}
......@@ -107,7 +110,7 @@ class MethodTemplate {
return path;
}
public Map<String, String> getHeaders() {
public Map<String, List<String>> getHeaders() {
return headers;
}
......@@ -147,7 +150,7 @@ class MethodTemplate {
return hystrixResponseValidator;
}
public Map<String, CacheProvider<?>> getCacheProviders() {
public List<CacheProviderEntry> getCacheProviders() {
return cacheProviders;
}
......@@ -163,6 +166,24 @@ class MethodTemplate {
return list.toArray(new MethodTemplate[list.size()]);
}
static class CacheProviderEntry {
private final String key;
private final CacheProvider<?> cacheProvider;
CacheProviderEntry(String key, CacheProvider<?> cacheProvider) {
this.key = key;
this.cacheProvider = cacheProvider;
}
public String getKey() {
return key;
}
public CacheProvider<?> getCacheProvider() {
return cacheProvider;
}
}
private static class MethodAnnotationValues {
private final Method method;
......@@ -177,8 +198,8 @@ class MethodTemplate {
public String hystrixCacheKey;
private FallbackHandler<?> hystrixFallbackHandler;
private HttpResponseValidator hystrixResponseValidator;
public final Map<String, String> headers = new HashMap<String, String>();
private final Map<String, CacheProvider<?>> cacheProviders = new HashMap<String, CacheProvider<?>>();
public final Map<String, List<String>> headers = new HashMap<String, List<String>>();
private final List<CacheProviderEntry> cacheProviders = new ArrayList<CacheProviderEntry>();
private EvCacheOptions evCacheOptions;
private MethodAnnotationValues(Method method) {
......@@ -200,7 +221,7 @@ class MethodTemplate {
for (Provider provider : annotation.value()) {
Class<? extends CacheProviderFactory<?>> providerClass = provider.provider();
CacheProviderFactory<?> factory = Utils.newInstance(providerClass);
cacheProviders.put(provider.key(), factory.createCacheProvider());
cacheProviders.add(new CacheProviderEntry(provider.key(), factory.createCacheProvider()));
}
}
}
......@@ -225,14 +246,18 @@ class MethodTemplate {
private void extractHttpAnnotation() {
Http annotation = method.getAnnotation(Http.class);
if (null == annotation) {
throw new IllegalArgumentException(format(
"Method %s.%s misses @Http annotation",
method.getDeclaringClass().getSimpleName(), method.getName()));
throw new ProxyAnnotationException(format("Method %s misses @Http annotation", methodName()));
}
httpMethod = annotation.method();
path = annotation.path();
for (Header h : annotation.headers()) {
headers.put(h.name(), h.value());
if (!headers.containsKey(h.name())) {
ArrayList<String> values = new ArrayList<String>();
values.add(h.value());
headers.put(h.name(), values);
} else {
headers.get(h.name()).add(h.value());
}
}
}
......@@ -271,33 +296,42 @@ class MethodTemplate {
}
}
if (count > 1) {
throw new IllegalArgumentException(format(
"Method %s.%s annotates multiple parameters as @Content - at most one is allowed ",
method.getDeclaringClass().getSimpleName(), method.getName()));
throw new ProxyAnnotationException(format("Method %s annotates multiple parameters as @Content - at most one is allowed ", methodName()));
}
contentArgPosition = pos;
}
private void extractTemplateName() {
TemplateName annotation = method.getAnnotation(TemplateName.class);
if (null != annotation) {
templateName = annotation.value();
private void extractContentTransformerClass() {
ContentTransformerClass annotation = method.getAnnotation(ContentTransformerClass.class);
if (contentArgPosition == -1) {
if (annotation != null) {
throw new ProxyAnnotationException(format("ContentTransformClass defined on method %s with no @Content parameter", method.getName()));
}
return;
}
if (annotation == null) {
Class<?> contentType = method.getParameterTypes()[contentArgPosition];
if (RawContentSource.class.isAssignableFrom(contentType) || ByteBuf.class.isAssignableFrom(contentType) || String.class.isAssignableFrom(contentType)) {
return;
}
throw new ProxyAnnotationException(format("ContentTransformerClass annotation missing for content type %s in method %s",
contentType.getName(), methodName()));
}
contentTansformerClass = annotation.value();
}
private void extractContentTransformerClass() {
ContentTransformerClass annotation = method.getAnnotation(ContentTransformerClass.class);
private void extractTemplateName() {
TemplateName annotation = method.getAnnotation(TemplateName.class);
if (null != annotation) {
contentTansformerClass = annotation.value();
templateName = annotation.value();
}
}
private void extractResultType() {
Class<?> returnClass = method.getReturnType();
if (!returnClass.isAssignableFrom(RibbonRequest.class)) {
throw new IllegalArgumentException(format(
"Method %s.%s must return Void or RibbonRequest<T> type not %s",
method.getDeclaringClass().getSimpleName(), method.getName(), returnClass.getSimpleName()));
throw new ProxyAnnotationException(format("Method %s must return Void or RibbonRequest<T> type not %s",
methodName(), returnClass.getSimpleName()));
}
ParameterizedType returnType = (ParameterizedType) method.getGenericReturnType();
resultType = (Class<?>) returnType.getActualTypeArguments()[0];
......@@ -314,7 +348,7 @@ class MethodTemplate {
if (transcoderClasses.length == 0) {
transcoder = null;
} else if (transcoderClasses.length > 1) {
throw new RibbonProxyException("Multiple transcoders defined on method " + method.getName());
throw new ProxyAnnotationException("Multiple transcoders defined on method " + methodName());
} else {
transcoder = Utils.newInstance(transcoderClasses[0]);
}
......@@ -327,5 +361,9 @@ class MethodTemplate {
transcoder,
annotation.cacheKeyTemplate());
}
private String methodName() {
return method.getDeclaringClass().getSimpleName() + '.' + method.getName();
}
}
}
......@@ -31,7 +31,9 @@ import io.reactivex.netty.serialization.StringTransformer;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
/**
* @author Tomasz Bak
......@@ -96,8 +98,11 @@ class MethodTemplateExecutor {
}
private void withHttpHeaders(HttpRequestTemplate<?> httpRequestTemplate) {
for (Map.Entry<String, String> header : methodTemplate.getHeaders().entrySet()) {
httpRequestTemplate.withHeader(header.getKey(), header.getValue());
for (Entry<String, List<String>> header : methodTemplate.getHeaders().entrySet()) {
String key = header.getKey();
for (String value : header.getValue()) {
httpRequestTemplate.withHeader(key, value);
}
}
}
......@@ -115,8 +120,8 @@ class MethodTemplateExecutor {
@SuppressWarnings({"rawtypes", "unchecked", "OverlyStrongTypeCast"})
private void withCacheProviders(HttpRequestTemplate<?> httpRequestTemplate) {
if (methodTemplate.getCacheProviders() != null) {
for (Map.Entry<String, CacheProvider<?>> entry : methodTemplate.getCacheProviders().entrySet()) {
httpRequestTemplate.addCacheProvider(entry.getKey(), (CacheProvider) entry.getValue());
for (MethodTemplate.CacheProviderEntry entry : methodTemplate.getCacheProviders()) {
httpRequestTemplate.addCacheProvider(entry.getKey(), (CacheProvider) entry.getCacheProvider());
}
}
EvCacheOptions evCacheOptions = methodTemplate.getEvCacheOptions();
......
/*
* Copyright 2014 Netflix, Inc.
*
* Licensed 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
*
* http://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.netflix.ribbonclientextensions.proxy;
/**
* @author Tomasz Bak
*/
public class ProxyAnnotationException extends RibbonProxyException {
private static final long serialVersionUID = 1584867992816375583L;
public ProxyAnnotationException(String message) {
super(message);
}
}
......@@ -24,7 +24,7 @@ import java.lang.annotation.Target;
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface ResourceGroupSpec {
public @interface ResourceGroup {
String name() default "";
Class<? extends HttpResourceGroup> resourceGroupClass() default UndefHttpResourceGroup.class;
......
......@@ -68,7 +68,8 @@ public class MethodTemplateExecutorTest {
expect(requestBuilderMock.withRequestProperty("id", "id123")).andReturn(requestBuilderMock);
expect(httpResourceGroupMock.newRequestTemplate("findMovieById", Movie.class)).andReturn(httpRequestTemplateMock);
expect(httpRequestTemplateMock.withHeader("X-MyHeader1", "value1")).andReturn(httpRequestTemplateMock);
expect(httpRequestTemplateMock.withHeader("X-MyHeader1", "value1.1")).andReturn(httpRequestTemplateMock);
expect(httpRequestTemplateMock.withHeader("X-MyHeader1", "value1.2")).andReturn(httpRequestTemplateMock);
expect(httpRequestTemplateMock.withHeader("X-MyHeader2", "value2")).andReturn(httpRequestTemplateMock);
expect(httpRequestTemplateMock.withRequestCacheKey("findMovieById/{id}")).andReturn(httpRequestTemplateMock);
expect(httpRequestTemplateMock.withFallbackProvider(anyObject(MovieFallbackHandler.class))).andReturn(httpRequestTemplateMock);
......
package com.netflix.ribbonclientextensions.proxy;
import com.netflix.ribbonclientextensions.CacheProvider;
import com.netflix.ribbonclientextensions.evache.EvCacheOptions;
import com.netflix.ribbonclientextensions.proxy.MethodTemplate.CacheProviderEntry;
import com.netflix.ribbonclientextensions.proxy.sample.EvCacheClasses.SampleEVCacheTranscoder;
import com.netflix.ribbonclientextensions.proxy.sample.Movie;
import com.netflix.ribbonclientextensions.proxy.sample.MovieServiceInterfaces.BrokenMovieService;
import com.netflix.ribbonclientextensions.proxy.sample.MovieServiceInterfaces.HystrixOptionalAnnotationValues;
import com.netflix.ribbonclientextensions.proxy.sample.MovieServiceInterfaces.PostsWithDifferentContentTypes;
import com.netflix.ribbonclientextensions.proxy.sample.MovieServiceInterfaces.SampleMovieService;
import com.netflix.ribbonclientextensions.proxy.sample.MovieTransformer;
import com.netflix.ribbonclientextensions.proxy.sample.SampleCacheProviderFactory.SampleCacheProvider;
import org.junit.Test;
......@@ -26,8 +28,9 @@ public class MethodTemplateTest {
assertEquals("findMovieById", template.getTemplateName());
assertEquals("/movies/{id}", template.getPath());
assertTrue("value1".equals(template.getHeaders().get("X-MyHeader1")));
assertTrue("value2".equals(template.getHeaders().get("X-MyHeader2")));
assertTrue("value1.1".equals(template.getHeaders().get("X-MyHeader1").get(0)));
assertTrue("value1.2".equals(template.getHeaders().get("X-MyHeader1").get(1)));
assertTrue("value2".equals(template.getHeaders().get("X-MyHeader2").get(0)));
assertEquals(0, template.getParamPosition(0));
assertEquals(template.getResultType(), Movie.class);
......@@ -36,9 +39,9 @@ public class MethodTemplateTest {
assertNotNull(template.getHystrixFallbackHandler());
assertNotNull(template.getHystrixResponseValidator());
CacheProvider cacheProvider = template.getCacheProviders().get("findMovieById_{id}");
assertNotNull(cacheProvider);
assertTrue(cacheProvider instanceof SampleCacheProvider);
CacheProviderEntry cacheProviderEntry = template.getCacheProviders().get(0);
assertNotNull(cacheProviderEntry);
assertTrue(cacheProviderEntry.getCacheProvider() instanceof SampleCacheProvider);
EvCacheOptions evOpts = template.getEvCacheOptions();
assertNotNull(evOpts);
......@@ -79,23 +82,61 @@ public class MethodTemplateTest {
assertNotNull(template.getHystrixFallbackHandler());
}
@Test
public void testWithRawContentSourceContent() throws Exception {
MethodTemplate methodTemplate = new MethodTemplate(methodByName(PostsWithDifferentContentTypes.class, "postwithRawContentSource"));
assertEquals(0, methodTemplate.getContentArgPosition());
assertNull(methodTemplate.getContentTransformerClass());
}
@Test
public void testWithByteBufContent() throws Exception {
MethodTemplate methodTemplate = new MethodTemplate(methodByName(PostsWithDifferentContentTypes.class, "postwithByteBufContent"));
assertEquals(0, methodTemplate.getContentArgPosition());
assertNull(methodTemplate.getContentTransformerClass());
}
@Test
public void testWithStringContent() throws Exception {
MethodTemplate methodTemplate = new MethodTemplate(methodByName(PostsWithDifferentContentTypes.class, "postwithStringContent"));
assertEquals(0, methodTemplate.getContentArgPosition());
assertNull(methodTemplate.getContentTransformerClass());
}
@Test
public void testWithUserClassContent() throws Exception {
MethodTemplate methodTemplate = new MethodTemplate(methodByName(PostsWithDifferentContentTypes.class, "postwithMovieContent"));
assertEquals(0, methodTemplate.getContentArgPosition());
assertNotNull(methodTemplate.getContentTransformerClass());
assertTrue(MovieTransformer.class.equals(methodTemplate.getContentTransformerClass()));
}
@Test(expected = ProxyAnnotationException.class)
public void testWithUserClassContentAndNotDefinedContentTransformer() {
new MethodTemplate(methodByName(PostsWithDifferentContentTypes.class, "postwithMovieContentBroken"));
}
@Test
public void testFromFactory() throws Exception {
MethodTemplate[] methodTemplates = MethodTemplate.from(SampleMovieService.class);
assertEquals(SampleMovieService.class.getMethods().length, methodTemplates.length);
}
@Test(expected = IllegalArgumentException.class)
@Test(expected = ProxyAnnotationException.class)
public void testDetectsInvalidResultType() throws Exception {
new MethodTemplate(methodByName(BrokenMovieService.class, "returnTypeNotRibbonRequest"));
}
@Test(expected = IllegalArgumentException.class)
@Test(expected = ProxyAnnotationException.class)
public void testMissingHttpMethod() throws Exception {
new MethodTemplate(methodByName(BrokenMovieService.class, "missingHttpAnnotation"));
}
@Test(expected = IllegalArgumentException.class)
@Test(expected = ProxyAnnotationException.class)
public void testMultipleContentParameters() throws Exception {
new MethodTemplate(methodByName(BrokenMovieService.class, "multipleContentParameters"));
}
......
......@@ -10,7 +10,7 @@ import com.netflix.ribbonclientextensions.proxy.annotation.Http;
import com.netflix.ribbonclientextensions.proxy.annotation.Http.Header;
import com.netflix.ribbonclientextensions.proxy.annotation.Http.HttpMethod;
import com.netflix.ribbonclientextensions.proxy.annotation.Hystrix;
import com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroupSpec;
import com.netflix.ribbonclientextensions.proxy.annotation.ResourceGroup;
import com.netflix.ribbonclientextensions.proxy.annotation.TemplateName;
import com.netflix.ribbonclientextensions.proxy.annotation.Var;
import com.netflix.ribbonclientextensions.proxy.sample.EvCacheClasses.SampleEVCacheTranscoder;
......@@ -33,7 +33,8 @@ public class MovieServiceInterfaces {
method = HttpMethod.GET,
path = "/movies/{id}",
headers = {
@Header(name = "X-MyHeader1", value = "value1"),
@Header(name = "X-MyHeader1", value = "value1.1"),
@Header(name = "X-MyHeader1", value = "value1.2"),
@Header(name = "X-MyHeader2", value = "value2")
})
@Hystrix(
......@@ -59,9 +60,11 @@ public class MovieServiceInterfaces {
RibbonRequest<Void> registerMovie(@Content Movie movie);
@Http(method = HttpMethod.PUT, path = "/movies/{id}")
@ContentTransformerClass(MovieTransformer.class)
RibbonRequest<Void> updateMovie(@Var("id") String id, @Content Movie movie);
@Http(method = HttpMethod.PATCH, path = "/movies/{id}")
@ContentTransformerClass(MovieTransformer.class)
RibbonRequest<Void> updateMoviePartial(@Var("id") String id, @Content Movie movie);
@TemplateName("registerMovieRaw")
......@@ -91,20 +94,35 @@ public class MovieServiceInterfaces {
RibbonRequest<ByteBuf> findAll();
}
@ResourceGroupSpec(name = "testResourceGroup")
public static interface BrokenMovieService {
@Http(method = HttpMethod.GET)
Movie returnTypeNotRibbonRequest();
Movie missingHttpAnnotation();
@Http(method = HttpMethod.GET)
RibbonRequest<Void> multipleContentParameters(@Content Movie content1, @Content Movie content2);
}
@ResourceGroup(name = "testResourceGroup")
public static interface SampleMovieServiceWithResourceGroupNameAnnotation {
}
@ResourceGroupSpec(resourceGroupClass = SampleHttpResourceGroup.class)
@ResourceGroup(resourceGroupClass = SampleHttpResourceGroup.class)
public static interface SampleMovieServiceWithResourceGroupClassAnnotation {
}
@ResourceGroupSpec(name = "testResourceGroup", resourceGroupClass = SampleHttpResourceGroup.class)
@ResourceGroup(name = "testResourceGroup", resourceGroupClass = SampleHttpResourceGroup.class)
public static interface BrokenMovieServiceWithResourceGroupNameAndClassAnnotation {
}
@ResourceGroupSpec(name = "testResourceGroup")
@ResourceGroup(name = "testResourceGroup")
public static interface HystrixOptionalAnnotationValues {
@TemplateName("hystrix1")
@Http(method = HttpMethod.GET, path = "/hystrix/1")
@Hystrix(cacheKey = "findMovieById/{id}")
......@@ -119,17 +137,32 @@ public class MovieServiceInterfaces {
@Http(method = HttpMethod.GET, path = "/hystrix/3")
@Hystrix(fallbackHandler = MovieFallbackHandler.class)
RibbonRequest<Void> hystrixWithFallbackHandlerOnly();
}
public static interface BrokenMovieService {
@ResourceGroup(name = "testResourceGroup")
public static interface PostsWithDifferentContentTypes {
@Http(method = HttpMethod.GET)
Movie returnTypeNotRibbonRequest();
@TemplateName("rawContentSource")
@Http(method = HttpMethod.POST, path = "/content/rawContentSource")
RibbonRequest<Void> postwithRawContentSource(@Content RawContentSource<Movie> movie);
Movie missingHttpAnnotation();
@TemplateName("byteBufContent")
@Http(method = HttpMethod.POST, path = "/content/byteBufContent")
RibbonRequest<Void> postwithByteBufContent(@Content ByteBuf byteBuf);
@Http(method = HttpMethod.GET)
RibbonRequest<Void> multipleContentParameters(@Content Movie content1, @Content Movie content2);
@TemplateName("stringContent")
@Http(method = HttpMethod.POST, path = "/content/stringContent")
RibbonRequest<Void> postwithStringContent(@Content String content);
@TemplateName("movieContent")
@Http(method = HttpMethod.POST, path = "/content/movieContent")
@ContentTransformerClass(MovieTransformer.class)
RibbonRequest<Void> postwithMovieContent(@Content Movie movie);
@TemplateName("movieContentBroken")
@Http(method = HttpMethod.POST, path = "/content/movieContentBroken")
RibbonRequest<Void> postwithMovieContentBroken(@Content Movie movie);
}
}
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册