Merge remote-tracking branch 'netflix/2.x-commonclient' into 2.x-commonclient

ribbon-client-extensions/src/main/java/com/netflix/ribbonclientextensions/RequestTemplate.java

@@ -9,17 +9,17 @@ import com.netflix.ribbonclientextensions.hystrix.FallbackHandler;
  * @param <T> response entity type
  * @param <R> response meta data, e.g. HttpClientResponse
  */
-public interface RequestTemplate<T, R> {
+public abstract class RequestTemplate<T, R> {
     
-    RequestBuilder<T> requestBuilder();
+    public abstract RequestBuilder<T> requestBuilder();
     
-    String name();
+    public abstract String name();
     
-    RequestTemplate<T, R> copy(String name);
+    public abstract RequestTemplate<T, R> copy(String name);
         
-    RequestTemplate<T, R> withFallbackProvider(FallbackHandler<T> fallbackProvider);
+    public abstract RequestTemplate<T, R> withFallbackProvider(FallbackHandler<T> fallbackProvider);
     
-    RequestTemplate<T, R> withResponseValidator(ResponseValidator<R> transformer);
+    public abstract RequestTemplate<T, R> withResponseValidator(ResponseValidator<R> transformer);
         
     /**
      * Calling this method will enable both Hystrix request cache and supplied external cache providers  
@@ -29,13 +29,13 @@ public interface RequestTemplate<T, R> {
      * @param cacheKeyTemplate
      * @return
      */
-    RequestTemplate<T, R> withRequestCacheKey(String cacheKeyTemplate);
+    public abstract RequestTemplate<T, R> withRequestCacheKey(String cacheKeyTemplate);
 
-    RequestTemplate<T, R> addCacheProvider(String cacheKeyTemplate, CacheProvider<T> cacheProvider);
+    public abstract RequestTemplate<T, R> addCacheProvider(String cacheKeyTemplate, CacheProvider<T> cacheProvider);
     
-    RequestTemplate<T, R> withHystrixProperties(HystrixObservableCommand.Setter setter);
+    public abstract RequestTemplate<T, R> withHystrixProperties(HystrixObservableCommand.Setter setter);
     
-    public abstract class RequestBuilder<T> {
+    public static abstract class RequestBuilder<T> {
         public abstract RequestBuilder<T> withRequestProperty(String key, Object value);
         
         public abstract RibbonRequest<T> build();

ribbon-client-extensions/src/main/java/com/netflix/ribbonclientextensions/http/HttpRequest.java

@@ -3,7 +3,6 @@ package com.netflix.ribbonclientextensions.http;
 import io.netty.buffer.ByteBuf;
 import io.reactivex.netty.protocol.http.client.HttpClient;
 import io.reactivex.netty.protocol.http.client.HttpClientRequest;
-import io.reactivex.netty.protocol.http.client.HttpClientResponse;
 
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -12,18 +11,11 @@ import java.util.Map;
 import java.util.concurrent.Future;
 
 import rx.Observable;
-import rx.Observable.OnSubscribe;
-import rx.Subscriber;
 
-import com.netflix.hystrix.HystrixExecutableInfo;
-import com.netflix.hystrix.HystrixObservableCommand;
 import com.netflix.ribbonclientextensions.CacheProvider;
-import com.netflix.ribbonclientextensions.ResponseValidator;
-import com.netflix.ribbonclientextensions.RibbonRequest;
 import com.netflix.ribbonclientextensions.RequestWithMetaData;
-import com.netflix.ribbonclientextensions.RibbonResponse;
+import com.netflix.ribbonclientextensions.RibbonRequest;
 import com.netflix.ribbonclientextensions.http.HttpRequestTemplate.CacheProviderWithKeyTemplate;
-import com.netflix.ribbonclientextensions.hystrix.FallbackHandler;
 import com.netflix.ribbonclientextensions.template.TemplateParser;
 import com.netflix.ribbonclientextensions.template.TemplateParsingException;
 

ribbon-client-extensions/src/main/java/com/netflix/ribbonclientextensions/http/HttpRequestBuilder.java

 package com.netflix.ribbonclientextensions.http;
 
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-
 import io.netty.buffer.ByteBuf;
-import io.reactivex.netty.protocol.http.client.HttpClient;
 import io.reactivex.netty.protocol.http.client.HttpClientRequest;
 import io.reactivex.netty.protocol.http.client.RawContentSource;
 import io.reactivex.netty.protocol.http.client.RepeatableContentHttpRequest;
 
-import com.netflix.hystrix.HystrixObservableCommand;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
 import com.netflix.hystrix.exception.HystrixBadRequestException;
 import com.netflix.ribbonclientextensions.RequestTemplate.RequestBuilder;
 import com.netflix.ribbonclientextensions.RibbonRequest;

ribbon-client-extensions/src/main/java/com/netflix/ribbonclientextensions/http/HttpRequestTemplate.java

@@ -24,7 +24,7 @@ import com.netflix.ribbonclientextensions.ResponseValidator;
 import com.netflix.ribbonclientextensions.hystrix.FallbackHandler;
 import com.netflix.ribbonclientextensions.template.ParsedTemplate;
 
-public class HttpRequestTemplate<T> implements RequestTemplate<T, HttpClientResponse<ByteBuf>> {
+public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientResponse<ByteBuf>> {
 
     private final HttpClient<ByteBuf, ByteBuf> client;
     private final String clientName;
@@ -60,8 +60,8 @@ public class HttpRequestTemplate<T> implements RequestTemplate<T, HttpClientResp
         }
     }
     
-    public HttpRequestTemplate(String name, HttpResourceGroup group, HttpClient<ByteBuf, ByteBuf> client, Class<? extends T> classType) {
-        this.client = client;
+    public HttpRequestTemplate(String name, HttpResourceGroup group, Class<? extends T> classType) {
+        this.client = group.getClient();
         this.classType = classType;
         if (client instanceof LoadBalancingRxClient) {
             LoadBalancingRxClient<?, ? ,?> ribbonClient = (LoadBalancingRxClient<?, ? ,?>) client;
@@ -93,14 +93,14 @@ public class HttpRequestTemplate<T> implements RequestTemplate<T, HttpClientResp
         if (setter == null) {
             setter = HystrixObservableCommand.Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(clientName))
                     .andCommandKey(HystrixCommandKey.Factory.asKey(name()));
+            HystrixCommandProperties.Setter commandProps = HystrixCommandProperties.Setter();
             if (maxResponseTime > 0) {
-                    setter.andCommandPropertiesDefaults(
-                            HystrixCommandProperties.Setter().withExecutionIsolationThreadTimeoutInMilliseconds(maxResponseTime));
+               commandProps.withExecutionIsolationThreadTimeoutInMilliseconds(maxResponseTime);
             }
             if (concurrentRequestLimit > 0) {
-                setter.andCommandPropertiesDefaults(
-                        HystrixCommandProperties.Setter().withExecutionIsolationSemaphoreMaxConcurrentRequests(concurrentRequestLimit));                
+                commandProps.withExecutionIsolationSemaphoreMaxConcurrentRequests(concurrentRequestLimit);                
             }
+            setter.andCommandPropertiesDefaults(commandProps);
         }
         return new HttpRequestBuilder<T>(this);
     }
@@ -190,7 +190,7 @@ public class HttpRequestTemplate<T> implements RequestTemplate<T, HttpClientResp
 
     @Override
     public HttpRequestTemplate<T> copy(String name) {
-        HttpRequestTemplate<T> newTemplate = new HttpRequestTemplate<T>(name, this.group, this.client, this.classType);
+        HttpRequestTemplate<T> newTemplate = new HttpRequestTemplate<T>(name, this.group, this.classType);
         newTemplate.cacheProviders.addAll(this.cacheProviders);
         newTemplate.method = this.method;
         newTemplate.headers.add(this.headers);
@@ -217,5 +217,5 @@ public class HttpRequestTemplate<T> implements RequestTemplate<T, HttpClientResp
     HttpClient<ByteBuf, ByteBuf> getClient() {
         return this.client;
     }
+    
 }
-

ribbon-client-extensions/src/main/java/com/netflix/ribbonclientextensions/http/HttpResourceGroup.java

@@ -37,7 +37,7 @@ public class HttpResourceGroup extends ResourceGroup<HttpRequestTemplate<?>> {
     @Override
     public <T> HttpRequestTemplate<T> newRequestTemplate(String name,
             Class<? extends T> classType) {
-        return new HttpRequestTemplate<T>(name, HttpResourceGroup.this, client, classType);
+        return new HttpRequestTemplate<T>(name, this, classType);
     }
     
     public HttpRequestTemplate<ByteBuf> newRequestTemplate(String name) {
@@ -48,4 +48,7 @@ public class HttpResourceGroup extends ResourceGroup<HttpRequestTemplate<?>> {
         return headers;
     }
     
+    HttpClient<ByteBuf, ByteBuf> getClient() {
+        return client;
+    }
 }

ribbon-client-extensions/src/test/java/com/netflix/ribbonclientextensions/RibbonTest.java

@@ -11,6 +11,8 @@ import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -61,12 +63,15 @@ public class RibbonTest {
     
     
     @Test
-    public void testCommandWithMetaData() throws IOException {
+    public void testCommandWithMetaData() throws IOException, InterruptedException, ExecutionException {
         // LogManager.getRootLogger().setLevel((Level)Level.DEBUG);
         MockWebServer server = new MockWebServer();
         String content = "Hello world";
+        server.enqueue(new MockResponse().setResponseCode(200).setHeader("Content-type", "text/plain")
+                .setBody(content));
         server.enqueue(new MockResponse().setResponseCode(200).setHeader("Content-type", "text/plain")
                 .setBody(content));       
+
         server.play();
         
         HttpResourceGroup group = Ribbon.createHttpResourceGroup("myclient", ClientOptions.create()
@@ -80,12 +85,11 @@ public class RibbonTest {
                     public Observable<ByteBuf> get(String key, Map<String, Object> vars) {
                         return Observable.error(new Exception("Cache miss"));
                     }
-                }).withHystrixProperties(HystrixObservableCommand.Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("group"))
-                        .andCommandPropertiesDefaults(HystrixCommandProperties.Setter().withRequestCacheEnabled(false))
-                        )
+                })
                 .requestBuilder().build();
         final AtomicBoolean success = new AtomicBoolean(false);
-        Observable<String> result = request.withMetadata().toObservable().flatMap(new Func1<RibbonResponse<Observable<ByteBuf>>, Observable<String>>(){
+        RequestWithMetaData<ByteBuf> metaRequest = request.withMetadata();
+        Observable<String> result = metaRequest.toObservable().flatMap(new Func1<RibbonResponse<Observable<ByteBuf>>, Observable<String>>(){
             @Override
             public Observable<String> call(
                     final RibbonResponse<Observable<ByteBuf>> response) {
@@ -101,6 +105,12 @@ public class RibbonTest {
         String s = result.toBlocking().single();
         assertEquals(content, s);
         assertTrue(success.get());
+        
+        Future<RibbonResponse<ByteBuf>> future = metaRequest.queue();
+        RibbonResponse<ByteBuf> response = future.get();
+        assertEquals(content, response.content().toString(Charset.defaultCharset()));
+        assertTrue(future.isDone());
+        assertTrue(response.getHystrixInfo().isSuccessfulExecution());
     }
 
    
@@ -222,9 +232,6 @@ public class RibbonTest {
                     }
                 })
                 .withUriTemplate("/")
-                .withHystrixProperties(HystrixObservableCommand.Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("group"))
-                        .andCommandPropertiesDefaults(HystrixCommandProperties.Setter().withRequestCacheEnabled(false))
-                        )
                 .requestBuilder().build();
         String result = request.execute().toString(Charset.defaultCharset());
         assertEquals(content, result);

ribbon-client-extensions/src/test/java/com/netflix/ribbonclientextensions/http/TemplateBuilderTest.java

@@ -12,7 +12,11 @@ import org.junit.Test;
 
 import rx.Observable;
 
+import com.netflix.hystrix.HystrixCommandProperties;
+import com.netflix.hystrix.HystrixObservableCommand;
+import com.netflix.hystrix.HystrixObservableCommand.Setter;
 import com.netflix.ribbonclientextensions.CacheProvider;
+import com.netflix.ribbonclientextensions.ClientOptions;
 import com.netflix.ribbonclientextensions.Ribbon;
 import com.netflix.ribbonclientextensions.RibbonRequest;
 
@@ -41,7 +45,7 @@ public class TemplateBuilderTest {
     public void testVarReplacement() {
         HttpResourceGroup group = Ribbon.createHttpResourceGroup("test");
         
-        HttpRequestTemplate<ByteBuf> template = group.newRequestTemplate("resource1", ByteBuf.class);
+        HttpRequestTemplate<ByteBuf> template = group.newRequestTemplate("testVarReplacement", ByteBuf.class);
         template.withUriTemplate("/foo/{id}?name={name}");
         HttpClientRequest<ByteBuf> request = template
                 .requestBuilder()
@@ -55,7 +59,7 @@ public class TemplateBuilderTest {
     public void testCacheKeyTemplates() {
         HttpResourceGroup group = Ribbon.createHttpResourceGroup("test");
         
-        HttpRequestTemplate<String> template = group.newRequestTemplate("resource1", String.class);
+        HttpRequestTemplate<String> template = group.newRequestTemplate("testCacheKeyTemplates", String.class);
         template.withUriTemplate("/foo/{id}")
             .addCacheProvider("cache.{id}", new FakeCacheProvider("cache.3"))
             .addCacheProvider("/cache/{id}", new FakeCacheProvider("/cache/5"));
@@ -73,7 +77,7 @@ public class TemplateBuilderTest {
         HttpResourceGroup group = Ribbon.createHttpResourceGroup("test");
         group.withCommonHeader("header1", "group");
         
-        HttpRequestTemplate<String> template = group.newRequestTemplate("resource1", String.class);
+        HttpRequestTemplate<String> template = group.newRequestTemplate("testHttpHeaders", String.class);
         template.withUriTemplate("/foo/bar")
             .withHeader("header2", "template")
             .withHeader("header1", "template");
@@ -88,5 +92,24 @@ public class TemplateBuilderTest {
         assertEquals(1, header2.size());
         assertEquals("template", header2.get(0));
     }
+    
+    @Test
+    public void testHystrixProperties() {
+        ClientOptions clientOptions = ClientOptions.create()
+                .withMaxAutoRetriesNextServer(1)
+                .withMaxAutoRetries(1)
+                .withConnectTimeout(1000)
+                .withMaxTotalConnections(400)
+                .withReadTimeout(2000);
+        HttpResourceGroup group = Ribbon.createHttpResourceGroup("test", clientOptions);
+        HttpRequestTemplate<String> template = group.newRequestTemplate("testHystrixProperties", String.class);
+        HttpRequest<String> request = (HttpRequest<String>) template.withMethod("GET")
+            .withUriTemplate("/foo/bar")
+            .requestBuilder().build();
+        HystrixObservableCommand<String> command = request.createHystrixCommand();
+        HystrixCommandProperties props = command.getProperties();
+        assertEquals(400, props.executionIsolationSemaphoreMaxConcurrentRequests().get().intValue());
+        assertEquals(12000, props.executionIsolationThreadTimeoutInMilliseconds().get().intValue());
+    }
 }
 

ribbon-core/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java

@@ -858,7 +858,7 @@ public class DefaultClientConfigImpl implements IClientConfig {
     @SuppressWarnings("unchecked")
     @Override
     public <T> T getPropertyWithType(IClientConfigKey<T> key) {
-        Object obj = properties.get(key.key());
+        Object obj = getProperty(key.key());
         if (obj == null) {
             return null;
         }

ribbon-core/src/test/java/com/netflix/client/config/DefaultClientConfigImplTest.java

@@ -24,7 +24,8 @@ public class DefaultClientConfigImplTest {
         assertEquals("1000", config.getProperty(CommonClientConfigKey.ConnectTimeout));
         assertEquals(1000, config.getPropertyWithType(CommonClientConfigKey.ConnectTimeout).intValue());
         config.setPropertyWithType(CommonClientConfigKey.ConnectTimeout, 2000);
-        assertEquals(2000, config.getPropertyWithType(CommonClientConfigKey.ConnectTimeout).intValue());
+        // The archaius property should override code override
+        assertEquals(1000, config.getPropertyWithType(CommonClientConfigKey.ConnectTimeout).intValue());
     }
     
     @Test