Make RequestTemplate an abstract class. Added test to make sure ClientOptions...

Make RequestTemplate an abstract class. Added test to make sure ClientOptions and Hystrix properties set correctly. Fix the problem that DefaultClientConfigImpl.getPropertyWithType() does not honor archaius property.

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

@@ -11,47 +11,15 @@ import com.netflix.ribbonclientextensions.hystrix.FallbackHandler;
  */
 public abstract class RequestTemplate<T, R> {
     
-    private final String name;
-    private final ResourceGroup<?> group;
-    private final Class<? extends T> classType;
-    private FallbackHandler<T> fallbackHandler;
-
-    public RequestTemplate(String name,
-            ResourceGroup<?> group,
-            Class<? extends T> classType) {
-        super();
-        this.name = name;
-        this.group = group;
-        this.classType = classType;
-    }
-    
-    public final String name() {
-        return name;
-    }
-
-    public final ResourceGroup<?> group() {
-        return group;
-    }
-
-    public final Class<? extends T> classType() {
-        return classType;
-    }
-
-
     public abstract RequestBuilder<T> requestBuilder();
     
+    public abstract String name();
+    
     public abstract RequestTemplate<T, R> copy(String name);
         
-    public RequestTemplate<T, R> withFallbackProvider(FallbackHandler<T> fallbackHandler) {
-        this.fallbackHandler = fallbackHandler;
-        return this;
-    }
+    public abstract RequestTemplate<T, R> withFallbackProvider(FallbackHandler<T> fallbackProvider);
     
-    public FallbackHandler<T> fallbackHandler() {
-        return fallbackHandler;
-    }
-
-    public abstract RequestTemplate<T, R> withResponseValidator(ResponseValidator<R> validator);
+    public abstract RequestTemplate<T, R> withResponseValidator(ResponseValidator<R> transformer);
         
     /**
      * Calling this method will enable both Hystrix request cache and supplied external cache providers  

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;
 
@@ -74,7 +66,7 @@ class HttpRequest<T> implements RibbonRequest<T> {
     
     RibbonHystrixObservableCommand<T> createHystrixCommand() {
         return new RibbonHystrixObservableCommand<T>(client, httpRequest, hystrixCacheKey, cacheProviders, requestProperties, template.fallbackHandler(), 
-                template.responseValidator(), template.classType(), template.hystrixProperties());
+                template.responseValidator(), template.getClassType(), template.hystrixProperties());
     }
     
     @Override

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

@@ -34,11 +34,14 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
     private ParsedTemplate parsedUriTemplate;
     private ResponseValidator<HttpClientResponse<ByteBuf>> validator;
     private HttpMethod method;
+    private final String name;
     private final List<CacheProviderWithKeyTemplate<T>> cacheProviders;
     private ParsedTemplate hystrixCacheKeyTemplate;
     private Map<String, ParsedTemplate> parsedTemplates;
+    private final Class<? extends T> classType;
     private final int concurrentRequestLimit;
     private final HttpHeaders headers;
+    private final HttpResourceGroup group;
     
     static class CacheProviderWithKeyTemplate<T> {
         private ParsedTemplate keyTemplate;
@@ -58,8 +61,8 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
     }
     
     public HttpRequestTemplate(String name, HttpResourceGroup group, Class<? extends T> classType) {
-        super(name, group, classType);
         this.client = group.getClient();
+        this.classType = classType;
         if (client instanceof LoadBalancingRxClient) {
             LoadBalancingRxClient<?, ? ,?> ribbonClient = (LoadBalancingRxClient<?, ? ,?>) client;
             maxResponseTime = ribbonClient.getResponseTimeOut();
@@ -70,6 +73,8 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
             maxResponseTime = -1;
             concurrentRequestLimit = -1;
         }
+        this.name = name;
+        this.group = group;
         method = HttpMethod.GET;
         headers = new DefaultHttpHeaders();
         headers.add(group.getHeaders());
@@ -79,7 +84,8 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
     
     @Override
     public HttpRequestTemplate<T> withFallbackProvider(FallbackHandler<T> fallbackHandler) {
-        return (HttpRequestTemplate<T>) super.withFallbackProvider(fallbackHandler);
+        this.fallbackHandler = fallbackHandler;
+        return this;
     }
 
     @Override
@@ -149,7 +155,11 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
     ResponseValidator<HttpClientResponse<ByteBuf>> responseValidator() {
         return validator;
     }
-        
+    
+    FallbackHandler<T> fallbackHandler() {
+        return fallbackHandler;
+    }
+    
     ParsedTemplate uriTemplate() {
         return parsedUriTemplate;
     }
@@ -157,11 +167,20 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
     HttpMethod method() {
         return method;
     }
-        
+    
+    Class<? extends T> getClassType() {
+        return this.classType;
+    }
+    
     HttpHeaders getHeaders() {
         return this.headers;
     }
-        
+    
+    @Override
+    public String name() {
+        return name;
+    }
+    
     @Override
     public HttpRequestTemplate<T> withResponseValidator(
             ResponseValidator<HttpClientResponse<ByteBuf>> validator) {
@@ -171,7 +190,7 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
 
     @Override
     public HttpRequestTemplate<T> copy(String name) {
-        HttpRequestTemplate<T> newTemplate = new HttpRequestTemplate<T>(name, (HttpResourceGroup) this.group(), 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);
@@ -198,5 +217,5 @@ public class HttpRequestTemplate<T> extends RequestTemplate<T, HttpClientRespons
     HttpClient<ByteBuf, ByteBuf> getClient() {
         return this.client;
     }
+    
 }
-

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