From b3fc37ac978aca7c0a30486ce1edc3b5433d2651 Mon Sep 17 00:00:00 2001 From: Allen Wang Date: Mon, 25 Aug 2014 10:46:27 -0700 Subject: [PATCH] Changes according to code review comments. --- build.gradle | 1 + .../ribbon/proxy/EvCacheAnnotationTest.java | 5 +- .../ribbon/proxy/sample/HystrixHandlers.java | 50 ----- .../netflix/ribbon/proxy/sample/Movie.java | 8 - .../proxy/sample/MovieServiceInterfaces.java | 185 ------------------ .../ribbon/proxy/sample/MovieTransformer.java | 31 --- .../proxy/sample/ResourceGroupClasses.java | 14 -- .../sample/SampleCacheProviderFactory.java | 42 ---- .../sample/SampleMovieServiceWithEVCache.java | 55 ++++++ .../client/netty/LoadBalancingRxClient.java | 14 -- .../netflix/ribbon/RibbonResourceFactory.java | 3 +- 11 files changed, 60 insertions(+), 348 deletions(-) delete mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/HystrixHandlers.java delete mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/Movie.java delete mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieServiceInterfaces.java delete mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieTransformer.java delete mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/ResourceGroupClasses.java delete mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleCacheProviderFactory.java create mode 100644 ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleMovieServiceWithEVCache.java diff --git a/build.gradle b/build.gradle index 29e6a751..1fc60051 100644 --- a/build.gradle +++ b/build.gradle @@ -159,6 +159,7 @@ project(':ribbon-evcache') { compile project(':ribbon') compile 'com.netflix.evcache:evcache-client:1.0.5' testCompile project(':ribbon-test') + testCompile project(':ribbon').sourceSets.test.output } } diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/EvCacheAnnotationTest.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/EvCacheAnnotationTest.java index 28e8f760..594937db 100644 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/EvCacheAnnotationTest.java +++ b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/EvCacheAnnotationTest.java @@ -25,7 +25,7 @@ import com.netflix.ribbon.proxy.processor.AnnotationProcessorsProvider; import com.netflix.ribbon.proxy.sample.HystrixHandlers.MovieFallbackHandler; import com.netflix.ribbon.proxy.sample.HystrixHandlers.SampleHttpResponseValidator; -import com.netflix.ribbon.proxy.sample.MovieServiceInterfaces.SampleMovieService; +import com.netflix.ribbon.proxy.sample.SampleMovieServiceWithEVCache; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -91,11 +91,10 @@ public void testGetQueryWithDomainObjectResult() throws Exception { expect(httpRequestTemplateBuilderMock.withResponseValidator(anyObject(SampleHttpResponseValidator.class))).andReturn(httpRequestTemplateBuilderMock); expect(httpRequestTemplateBuilderMock.withCacheProvider(anyObject(String.class), anyObject(CacheProvider.class))).andReturn(httpRequestTemplateBuilderMock); expect(httpRequestTemplateBuilderMock.withCacheProvider(anyObject(String.class), anyObject(EvCacheProvider.class))).andReturn(httpRequestTemplateBuilderMock); - // expect(evCacheProviderPoolMock.getMatching(anyObject(EvCacheOptions.class))).andReturn(evCacheProviderMock); replayAll(); - MethodTemplateExecutor executor = createExecutor(SampleMovieService.class, "findMovieById"); + MethodTemplateExecutor executor = createExecutor(SampleMovieServiceWithEVCache.class, "findMovieById"); RibbonRequest ribbonRequest = executor.executeFromTemplate(new Object[]{"id123"}); verifyAll(); diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/HystrixHandlers.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/HystrixHandlers.java deleted file mode 100644 index 34eb802a..00000000 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/HystrixHandlers.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * 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.ribbon.proxy.sample; - -import com.netflix.hystrix.HystrixExecutableInfo; -import com.netflix.ribbon.ServerError; -import com.netflix.ribbon.UnsuccessfulResponseException; -import com.netflix.ribbon.http.HttpResponseValidator; -import com.netflix.ribbon.hystrix.FallbackHandler; - -import io.netty.buffer.ByteBuf; -import io.reactivex.netty.protocol.http.client.HttpClientResponse; -import rx.Observable; - -import java.util.Map; - -/** - * @author Tomasz Bak - */ -public class HystrixHandlers { - - public static class SampleHttpResponseValidator implements HttpResponseValidator { - - @Override - public void validate(HttpClientResponse response) throws UnsuccessfulResponseException, ServerError { - } - } - - public static class MovieFallbackHandler implements FallbackHandler { - - @Override - public Observable getFallback(HystrixExecutableInfo hystrixInfo, Map requestProperties) { - return null; - } - } -} diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/Movie.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/Movie.java deleted file mode 100644 index 780fd7f0..00000000 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/Movie.java +++ /dev/null @@ -1,8 +0,0 @@ -package com.netflix.ribbon.proxy.sample; - -/** - * @author Tomasz Bak - */ -public class Movie { - -} diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieServiceInterfaces.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieServiceInterfaces.java deleted file mode 100644 index 4ae398b9..00000000 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieServiceInterfaces.java +++ /dev/null @@ -1,185 +0,0 @@ -package com.netflix.ribbon.proxy.sample; - -import com.netflix.ribbon.RibbonRequest; -import com.netflix.ribbon.proxy.annotation.CacheProvider; -import com.netflix.ribbon.proxy.annotation.Content; -import com.netflix.ribbon.proxy.annotation.ContentTransformerClass; -import com.netflix.ribbon.proxy.annotation.EvCache; -import com.netflix.ribbon.proxy.annotation.Http; -import com.netflix.ribbon.proxy.annotation.Http.Header; -import com.netflix.ribbon.proxy.annotation.Http.HttpMethod; -import com.netflix.ribbon.proxy.annotation.Hystrix; -import com.netflix.ribbon.proxy.annotation.ResourceGroup; -import com.netflix.ribbon.proxy.annotation.TemplateName; -import com.netflix.ribbon.proxy.annotation.Var; -import com.netflix.ribbon.proxy.sample.EvCacheClasses.SampleEVCacheTranscoder; -import com.netflix.ribbon.proxy.sample.HystrixHandlers.MovieFallbackHandler; -import com.netflix.ribbon.proxy.sample.HystrixHandlers.SampleHttpResponseValidator; -import io.netty.buffer.ByteBuf; -import rx.Observable; - -import java.util.concurrent.atomic.AtomicReference; - -import static com.netflix.ribbon.proxy.sample.ResourceGroupClasses.SampleHttpResourceGroup; - -/** - * @author Tomasz Bak - */ -public class MovieServiceInterfaces { - - public static interface SampleMovieService { - - @TemplateName("findMovieById") - @Http( - method = HttpMethod.GET, - uri = "/movies/{id}", - headers = { - @Header(name = "X-MyHeader1", value = "value1.1"), - @Header(name = "X-MyHeader1", value = "value1.2"), - @Header(name = "X-MyHeader2", value = "value2") - }) - @Hystrix( - cacheKey = "findMovieById/{id}", - validator = SampleHttpResponseValidator.class, - fallbackHandler = MovieFallbackHandler.class) - @CacheProvider(key = "findMovieById_{id}", provider = SampleCacheProviderFactory.class) - - @EvCache(name = "movie-cache", appName = "movieService", key = "movie-{id}", ttl = 50, - enableZoneFallback = true, transcoder = SampleEVCacheTranscoder.class) - RibbonRequest findMovieById(@Var("id") String id); - - @TemplateName("findRawMovieById") - @Http(method = HttpMethod.GET, uri = "/rawMovies/{id}") - RibbonRequest findRawMovieById(@Var("id") String id); - - @TemplateName("findMovie") - @Http(method = HttpMethod.GET, uri = "/movies?name={name}&author={author}") - RibbonRequest findMovie(@Var("name") String name, @Var("author") String author); - - @TemplateName("registerMovie") - @Http(method = HttpMethod.POST, uri = "/movies") - @Hystrix(cacheKey = "registerMovie", fallbackHandler = MovieFallbackHandler.class) - @ContentTransformerClass(MovieTransformer.class) - RibbonRequest registerMovie(@Content Movie movie); - - @Http(method = HttpMethod.PUT, uri = "/movies/{id}") - @ContentTransformerClass(MovieTransformer.class) - RibbonRequest updateMovie(@Var("id") String id, @Content Movie movie); - - @Http(method = HttpMethod.PATCH, uri = "/movies/{id}") - @ContentTransformerClass(MovieTransformer.class) - RibbonRequest updateMoviePartial(@Var("id") String id, @Content Movie movie); - - @TemplateName("registerTitle") - @Http(method = HttpMethod.POST, uri = "/titles") - @Hystrix(cacheKey = "registerTitle", fallbackHandler = MovieFallbackHandler.class) - RibbonRequest registerTitle(@Content String title); - - @TemplateName("registerByteBufBinary") - @Http(method = HttpMethod.POST, uri = "/binaries/byteBuf") - @Hystrix(cacheKey = "registerByteBufBinary", fallbackHandler = MovieFallbackHandler.class) - RibbonRequest registerByteBufBinary(@Content ByteBuf binary); - - @TemplateName("registerByteArrayBinary") - @Http(method = HttpMethod.POST, uri = "/binaries/byteArray") - @Hystrix(cacheKey = "registerByteArrayBinary", fallbackHandler = MovieFallbackHandler.class) - RibbonRequest registerByteArrayBinary(@Content byte[] binary); - - @TemplateName("deleteMovie") - @Http(method = HttpMethod.DELETE, uri = "/movies/{id}") - RibbonRequest deleteMovie(@Var("id") String id); - } - - public static interface ShortMovieService { - @TemplateName("findMovieById") - @Http(method = HttpMethod.GET, uri = "/movies/{id}") - RibbonRequest findMovieById(@Var("id") String id); - - @TemplateName("findMovieById") - @Http(method = HttpMethod.GET, uri = "/movies") - RibbonRequest findAll(); - } - - public static interface BrokenMovieService { - - @Http(method = HttpMethod.GET) - Movie returnTypeNotRibbonRequest(); - - Movie missingHttpAnnotation(); - - @Http(method = HttpMethod.GET) - RibbonRequest multipleContentParameters(@Content Movie content1, @Content Movie content2); - } - - @ResourceGroup(name = "testResourceGroup") - public static interface SampleMovieServiceWithResourceGroupNameAnnotation { - - } - - @ResourceGroup(resourceGroupClass = SampleHttpResourceGroup.class) - public static interface SampleMovieServiceWithResourceGroupClassAnnotation { - - } - - @ResourceGroup(name = "testResourceGroup", resourceGroupClass = SampleHttpResourceGroup.class) - public static interface BrokenMovieServiceWithResourceGroupNameAndClassAnnotation { - - } - - @ResourceGroup(name = "testResourceGroup") - public static interface TemplateNameDerivedFromMethodName { - @Http(method = HttpMethod.GET, uri = "/template") - RibbonRequest myTemplateName(); - } - - @ResourceGroup(name = "testResourceGroup") - public static interface HystrixOptionalAnnotationValues { - - @TemplateName("hystrix1") - @Http(method = HttpMethod.GET, uri = "/hystrix/1") - @Hystrix(cacheKey = "findMovieById/{id}") - RibbonRequest hystrixWithCacheKeyOnly(); - - @TemplateName("hystrix2") - @Http(method = HttpMethod.GET, uri = "/hystrix/2") - @Hystrix(validator = SampleHttpResponseValidator.class) - RibbonRequest hystrixWithValidatorOnly(); - - @TemplateName("hystrix3") - @Http(method = HttpMethod.GET, uri = "/hystrix/3") - @Hystrix(fallbackHandler = MovieFallbackHandler.class) - RibbonRequest hystrixWithFallbackHandlerOnly(); - - } - - @ResourceGroup(name = "testResourceGroup") - public static interface PostsWithDifferentContentTypes { - - @TemplateName("rawContentSource") - @Http(method = HttpMethod.POST, uri = "/content/rawContentSource") - @ContentTransformerClass(MovieTransformer.class) - RibbonRequest postwithRawContentSource(AtomicReference arg1, int arg2, @Content Observable movie); - - @TemplateName("byteBufContent") - @Http(method = HttpMethod.POST, uri = "/content/byteBufContent") - RibbonRequest postwithByteBufContent(@Content ByteBuf byteBuf); - - @TemplateName("byteArrayContent") - @Http(method = HttpMethod.POST, uri = "/content/byteArrayContent") - RibbonRequest postwithByteArrayContent(@Content byte[] bytes); - - @TemplateName("stringContent") - @Http(method = HttpMethod.POST, uri = "/content/stringContent") - RibbonRequest postwithStringContent(@Content String content); - - @TemplateName("movieContent") - @Http(method = HttpMethod.POST, uri = "/content/movieContent") - @ContentTransformerClass(MovieTransformer.class) - RibbonRequest postwithMovieContent(@Content Movie movie); - - @TemplateName("movieContentBroken") - @Http(method = HttpMethod.POST, uri = "/content/movieContentBroken") - RibbonRequest postwithMovieContentBroken(@Content Movie movie); - - } -} diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieTransformer.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieTransformer.java deleted file mode 100644 index af0a4bcd..00000000 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/MovieTransformer.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * 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.ribbon.proxy.sample; - -import io.netty.buffer.ByteBuf; -import io.netty.buffer.ByteBufAllocator; -import io.reactivex.netty.channel.ContentTransformer; - -/** - * @author Tomasz Bak - */ -public class MovieTransformer implements ContentTransformer { - @Override - public ByteBuf call(MovieTransformer toTransform, ByteBufAllocator byteBufAllocator) { - return null; - } -} diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/ResourceGroupClasses.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/ResourceGroupClasses.java deleted file mode 100644 index 18e96b3f..00000000 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/ResourceGroupClasses.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.netflix.ribbon.proxy.sample; - -import com.netflix.ribbon.http.HttpResourceGroup; - -/** - * @author Tomasz Bak - */ -public class ResourceGroupClasses { - public static class SampleHttpResourceGroup extends HttpResourceGroup { - public SampleHttpResourceGroup() { - super("myTestGroup"); - } - } -} diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleCacheProviderFactory.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleCacheProviderFactory.java deleted file mode 100644 index 645cbbc2..00000000 --- a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleCacheProviderFactory.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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.ribbon.proxy.sample; - -import com.netflix.ribbon.CacheProvider; -import com.netflix.ribbon.CacheProviderFactory; -import rx.Observable; - -import java.util.Map; - -/** - * @author Tomasz Bak - */ -public class SampleCacheProviderFactory implements CacheProviderFactory { - - @Override - public CacheProvider createCacheProvider() { - return new SampleCacheProvider(); - } - - public static class SampleCacheProvider implements CacheProvider { - - @Override - public Observable get(String key, Map requestProperties) { - return null; - } - } -} diff --git a/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleMovieServiceWithEVCache.java b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleMovieServiceWithEVCache.java new file mode 100644 index 00000000..142c7c19 --- /dev/null +++ b/ribbon-evcache/src/test/java/com/netflix/ribbon/proxy/sample/SampleMovieServiceWithEVCache.java @@ -0,0 +1,55 @@ +/* + * 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.ribbon.proxy.sample; + +import com.netflix.ribbon.RibbonRequest; +import com.netflix.ribbon.proxy.annotation.CacheProvider; +import com.netflix.ribbon.proxy.annotation.EvCache; +import com.netflix.ribbon.proxy.annotation.Http; +import com.netflix.ribbon.proxy.annotation.Http.Header; +import com.netflix.ribbon.proxy.annotation.Http.HttpMethod; +import com.netflix.ribbon.proxy.annotation.Hystrix; +import com.netflix.ribbon.proxy.annotation.TemplateName; +import com.netflix.ribbon.proxy.annotation.Var; +import com.netflix.ribbon.proxy.sample.EvCacheClasses.SampleEVCacheTranscoder; +import com.netflix.ribbon.proxy.sample.HystrixHandlers.MovieFallbackHandler; +import com.netflix.ribbon.proxy.sample.HystrixHandlers.SampleHttpResponseValidator; +import com.netflix.ribbon.proxy.sample.MovieServiceInterfaces.SampleMovieService; +import io.netty.buffer.ByteBuf; + +/** + * @author Allen Wang + */ +public interface SampleMovieServiceWithEVCache extends SampleMovieService { + @TemplateName("findMovieById") + @Http( + method = HttpMethod.GET, + uri = "/movies/{id}", + headers = { + @Header(name = "X-MyHeader1", value = "value1.1"), + @Header(name = "X-MyHeader1", value = "value1.2"), + @Header(name = "X-MyHeader2", value = "value2") + }) + @Hystrix( + cacheKey = "findMovieById/{id}", + validator = SampleHttpResponseValidator.class, + fallbackHandler = MovieFallbackHandler.class) + @CacheProvider(key = "findMovieById_{id}", provider = SampleCacheProviderFactory.class) + + @EvCache(name = "movie-cache", appName = "movieService", key = "movie-{id}", ttl = 50, + enableZoneFallback = true, transcoder = SampleEVCacheTranscoder.class) + RibbonRequest findMovieById(@Var("id") String id); +} diff --git a/ribbon-transport/src/main/java/com/netflix/client/netty/LoadBalancingRxClient.java b/ribbon-transport/src/main/java/com/netflix/client/netty/LoadBalancingRxClient.java index 4026a891..42b8869a 100644 --- a/ribbon-transport/src/main/java/com/netflix/client/netty/LoadBalancingRxClient.java +++ b/ribbon-transport/src/main/java/com/netflix/client/netty/LoadBalancingRxClient.java @@ -43,7 +43,6 @@ import org.slf4j.LoggerFactory; import rx.Observable; import rx.Subscription; -import rx.functions.Func1; import javax.annotation.Nullable; import java.io.File; @@ -118,19 +117,6 @@ public IClientConfig getClientConfig() { return clientConfig; } - public Observable connectWithAction(final Func1, Observable> action, RetryHandler retryHandler) { - return lbExecutor.create(new LoadBalancerObservableCommand() { - @Override - public Observable run(Server server) { - return getRxClient(server.getHost(), server.getPort()).connect().flatMap(action); - } - }, retryHandler == null ? this.retryHandler : retryHandler); - } - - public Observable connectWithAction(final Func1, Observable> action) { - return connectWithAction(action, retryHandler); - } - public int getResponseTimeOut() { int maxRetryNextServer = 0; int maxRetrySameServer = 0; diff --git a/ribbon/src/main/java/com/netflix/ribbon/RibbonResourceFactory.java b/ribbon/src/main/java/com/netflix/ribbon/RibbonResourceFactory.java index c2f2dd43..bb8669a7 100644 --- a/ribbon/src/main/java/com/netflix/ribbon/RibbonResourceFactory.java +++ b/ribbon/src/main/java/com/netflix/ribbon/RibbonResourceFactory.java @@ -22,7 +22,8 @@ import com.netflix.ribbon.proxy.processor.AnnotationProcessorsProvider; /** - * Factory for creating an HttpResourceGroup. For DI either bind DefaultHttpResourceGroupFactory + * Factory for creating an HttpResourceGroup or dynamic proxy from an annotated interface. + * For DI either bind DefaultHttpResourceGroupFactory * or implement your own to customize or override HttpResourceGroup. * * @author elandau