Pass LB IClientConfig to executeWithLoadBalancer

fixes 208

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClient.java

@@ -23,6 +23,7 @@
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 
+import lombok.Data;
 import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
 import org.springframework.util.ReflectionUtils;
 
@@ -35,6 +36,8 @@
 import feign.Request;
 import feign.Response;
 
+import static org.springframework.cloud.netflix.feign.ribbon.RibbonLoadBalancer.RibbonRequest;
+
 /**
  * @author Julien Roy
  * @author Spencer Gibb
@@ -57,9 +60,10 @@ public Response execute(Request request, Request.Options options) throws IOExcep
 			String clientName = asUri.getHost();
 			URI uriWithoutSchemeAndPort = URI.create(request.url().replace(
 					asUri.getScheme() + "://" + asUri.getHost(), ""));
-			RibbonLoadBalancer.RibbonRequest ribbonRequest = new RibbonLoadBalancer.RibbonRequest(
+			RibbonLoadBalancer.RibbonRequest ribbonRequest = new RibbonRequest(
 					request, uriWithoutSchemeAndPort);
-			return lbClient(clientName).executeWithLoadBalancer(ribbonRequest)
+			LBClient client = getClient(clientName);
+			return client.getLoadBalancer().executeWithLoadBalancer(ribbonRequest, client.config)
 					.toResponse();
 		}
 		catch (ClientException ex) {
@@ -71,10 +75,17 @@ public Response execute(Request request, Request.Options options) throws IOExcep
 		}
 	}
 
-	private RibbonLoadBalancer lbClient(String clientName) {
+	@Data
+	private class LBClient {
+		private final IClientConfig config;
+		private final RibbonLoadBalancer loadBalancer;
+	}
+
+	private LBClient getClient(String clientName) {
 		IClientConfig config = this.factory.getClientConfig(clientName);
 		ILoadBalancer lb = this.factory.getLoadBalancer(clientName);
-		return new RibbonLoadBalancer(this.defaultClient, lb, config);
+		RibbonLoadBalancer loadBalancer = new RibbonLoadBalancer(this.defaultClient, lb, config);
+		return new LBClient(config, loadBalancer);
 	}
 
 	public void setDefaultClient(Client defaultClient) {

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientRetryTests.java

@@ -0,0 +1,159 @@
+/*
+ * Copyright 2013-2015 the original author or authors.
+ *
+ * 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 org.springframework.cloud.netflix.feign.ribbon;
+
+import static org.junit.Assert.*;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Proxy;
+import java.util.Arrays;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.boot.test.IntegrationTest;
+import org.springframework.boot.test.SpringApplicationConfiguration;
+import org.springframework.cloud.netflix.feign.EnableFeignClients;
+import org.springframework.cloud.netflix.feign.FeignClient;
+import org.springframework.cloud.netflix.ribbon.RibbonClient;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RestController;
+
+import com.netflix.loadbalancer.BaseLoadBalancer;
+import com.netflix.loadbalancer.ILoadBalancer;
+import com.netflix.loadbalancer.Server;
+
+/**
+ * @author Spencer Gibb
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@SpringApplicationConfiguration(classes = FeignRibbonClientRetryTests.Application.class)
+@WebAppConfiguration
+@IntegrationTest({ "server.port=0", "spring.application.name=feignclienttest",
+		"localapp.ribbon.MaxAutoRetries=5",
+		"localapp.ribbon.MaxAutoRetriesNextServer=5",
+		"localapp.ribbon.OkToRetryOnAllOperations=true",
+})
+@DirtiesContext
+public class FeignRibbonClientRetryTests {
+
+	@Value("${local.server.port}")
+	private int port = 0;
+
+	@Autowired
+	private TestClient testClient;
+
+	// @FeignClient(value = "http://localhost:9876", loadbalance = false)
+	@FeignClient("localapp")
+	protected static interface TestClient {
+		@RequestMapping(method = RequestMethod.GET, value = "/hello")
+		public Hello getHello();
+
+		@RequestMapping(method = RequestMethod.GET, value = "/retryme")
+		public int retryMe();
+	}
+
+	@Configuration
+	@EnableAutoConfiguration
+	@RestController
+	@EnableFeignClients
+	@RibbonClient(name = "localapp", configuration = LocalRibbonClientConfiguration.class)
+	public static class Application {
+
+		private AtomicInteger retries = new AtomicInteger(1);
+
+		@RequestMapping(method = RequestMethod.GET, value = "/hello")
+		public Hello getHello() {
+			return new Hello("hello world 1");
+		}
+
+		@RequestMapping(method = RequestMethod.GET, value = "/retryme")
+		public int retryMe() {
+			return retries.getAndIncrement();
+		}
+
+		public static void main(String[] args) throws InterruptedException {
+			ConfigurableApplicationContext context = new SpringApplicationBuilder(Application.class).properties(
+					"spring.application.name=feignclienttest",
+					"localapp.ribbon.MaxAutoRetries=5",
+					"localapp.ribbon.MaxAutoRetriesNextServer=5",
+					"localapp.ribbon.OkToRetryOnAllOperations=true",
+					"management.contextPath=/admin"
+					//,"local.server.port=9999"
+					).run(args);
+		}
+	}
+
+	@Test
+	public void testClient() {
+		assertNotNull("testClient was null", this.testClient);
+		assertTrue("testClient is not a java Proxy",
+				Proxy.isProxyClass(this.testClient.getClass()));
+		InvocationHandler invocationHandler = Proxy.getInvocationHandler(this.testClient);
+		assertNotNull("invocationHandler was null", invocationHandler);
+	}
+
+	@Test
+	public void testRetries() {
+		int retryMe = this.testClient.retryMe();
+		assertEquals("retryCount didn't match", retryMe, 1);
+		//TODO: not sure how to verify retry happens.  Debugging through it, it works
+		//maybe the assertEquals above is enough because of the bogus servers
+	}
+
+	@Data
+	@AllArgsConstructor
+	@NoArgsConstructor
+	public static class Hello {
+		private String message;
+	}
+}
+
+// Load balancer with fixed server list for "local" pointing to localhost
+// some bogus servers are thrown in to test retry
+@Configuration
+class LocalRibbonClientConfiguration {
+
+	@Value("${local.server.port}")
+	private int port = 0;
+
+	@Bean
+	public ILoadBalancer ribbonLoadBalancer() {
+		BaseLoadBalancer balancer = new BaseLoadBalancer();
+		balancer.setServersList(Arrays.asList(new Server("___mybadhost__", 10001),
+				new Server("___mybadhost2__", 10002),
+				new Server("___mybadhost3__", 10003),
+				new Server("localhost", this.port)));
+		return balancer;
+	}
+
+}