diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index 5dd75399..75cc6c57 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -118,7 +118,7 @@ public class FeignAutoConfiguration { ApacheHttpClientConnectionManagerFactory connectionManagerFactory, FeignHttpClientProperties httpClientProperties) { final HttpClientConnectionManager connectionManager = connectionManagerFactory - .newConnectionManager(false, httpClientProperties.getMaxConnections(), + .newConnectionManager(httpClientProperties.isDisableSslValidation(), httpClientProperties.getMaxConnections(), httpClientProperties.getMaxConnectionsPerRoute(), httpClientProperties.getTimeToLive(), httpClientProperties.getTimeToLiveUnit(), registryBuilder); @@ -184,7 +184,8 @@ public class FeignAutoConfiguration { ConnectionPool connectionPool, FeignHttpClientProperties httpClientProperties) { Boolean followRedirects = httpClientProperties.isFollowRedirects(); Integer connectTimeout = httpClientProperties.getConnectionTimeout(); - this.okHttpClient = httpClientFactory.createBuilder(false). + Boolean disableSslValidation = httpClientProperties.isDisableSslValidation(); + this.okHttpClient = httpClientFactory.createBuilder(disableSslValidation). connectTimeout(connectTimeout, TimeUnit.MILLISECONDS). followRedirects(followRedirects). connectionPool(connectionPool).build(); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java index 3930a663..3bb051ce 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.feign.ribbon; import java.util.Map; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; @@ -34,12 +35,14 @@ import com.netflix.loadbalancer.ILoadBalancer; * @author Spencer Gibb * @author Dave Syer * @author Ryan Baxter + * @author Gang Li */ public class CachingSpringLoadBalancerFactory { private final SpringClientFactory factory; private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private final LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; + private final LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; private boolean enableRetry = false; private volatile Map cache = new ConcurrentReferenceHashMap<>(); @@ -48,6 +51,7 @@ public class CachingSpringLoadBalancerFactory { this.factory = factory; this.loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(factory); this.loadBalancedBackOffPolicyFactory = null; + this.loadBalancedRetryListenerFactory = null; } @Deprecated @@ -57,6 +61,7 @@ public class CachingSpringLoadBalancerFactory { this.factory = factory; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.loadBalancedBackOffPolicyFactory = null; + this.loadBalancedRetryListenerFactory = null; } @Deprecated @@ -67,14 +72,28 @@ public class CachingSpringLoadBalancerFactory { this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.enableRetry = enableRetry; this.loadBalancedBackOffPolicyFactory = null; + this.loadBalancedRetryListenerFactory = null; } + @Deprecated + //TODO remove in 2.0.0x public CachingSpringLoadBalancerFactory(SpringClientFactory factory, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { this.factory = factory; this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = null; + this.enableRetry = true; + } + + public CachingSpringLoadBalancerFactory(SpringClientFactory factory, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + this.factory = factory; + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory; this.enableRetry = true; } @@ -86,7 +105,7 @@ public class CachingSpringLoadBalancerFactory { ILoadBalancer lb = this.factory.getLoadBalancer(clientName); ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class); FeignLoadBalancer client = enableRetry ? new RetryableFeignLoadBalancer(lb, config, serverIntrospector, - loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory) : new FeignLoadBalancer(lb, config, serverIntrospector); + loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory) : new FeignLoadBalancer(lb, config, serverIntrospector); this.cache.put(clientName, client); return client; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index c9ca5d33..d8981442 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -22,6 +22,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.feign.support.FeignHttpClientProperties; @@ -65,10 +66,11 @@ public class FeignRibbonClientAutoConfiguration { @Primary @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") public CachingSpringLoadBalancerFactory retryabeCachingLBClientFactory( - SpringClientFactory factory, - LoadBalancedRetryPolicyFactory retryPolicyFactory, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { - return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, loadBalancedBackOffPolicyFactory); + SpringClientFactory factory, + LoadBalancedRetryPolicyFactory retryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); } @Bean diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java index 04bc00c6..cb5aea92 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancer.java @@ -25,6 +25,7 @@ import java.io.IOException; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -34,6 +35,7 @@ import org.springframework.cloud.netflix.ribbon.RibbonProperties; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -47,11 +49,13 @@ import com.netflix.loadbalancer.Server; /** * A {@link FeignLoadBalancer} that leverages Spring Retry to retry failed requests. * @author Ryan Baxter + * @author Gang Li */ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements ServiceInstanceChooser { private final LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private final LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; + private final LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; @Deprecated //TODO remove in 2.0.x @@ -61,8 +65,11 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.loadBalancedBackOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + this.loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); } + @Deprecated + //TODO remove in 2.0.x public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { @@ -70,7 +77,21 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory == null ? - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); + } + + public RetryableFeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + super(lb, clientConfig, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.setRetryHandler(new DefaultLoadBalancerRetryHandler(clientConfig)); + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory == null ? + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory() : loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory == null ? + new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory() : loadBalancedRetryListenerFactory; } @Override @@ -90,6 +111,10 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser RetryTemplate retryTemplate = new RetryTemplate(); BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); + RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + if (retryListeners != null && retryListeners.length != 0) { + retryTemplate.setListeners(retryListeners); + } retryTemplate.setRetryPolicy(retryPolicy == null ? new NeverRetryPolicy() : new FeignRetryPolicy(request.toHttpRequest(), retryPolicy, this, this.getClientName())); return retryTemplate.execute(new RetryCallback() { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientConfigurationTests.java new file mode 100644 index 00000000..43f388a8 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientConfigurationTests.java @@ -0,0 +1,90 @@ +/* + * Copyright 2013-2018 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; + +import java.lang.reflect.Field; +import javax.net.ssl.SSLContextSpi; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.X509TrustManager; +import org.apache.http.config.Lookup; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.conn.socket.ConnectionSocketFactory; +import org.apache.http.impl.conn.DefaultHttpClientConnectionOperator; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; +import org.springframework.cloud.test.ClassPathExclusions; +import org.springframework.cloud.test.ModifiedClassPathRunner; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.util.ReflectionUtils; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +/** + * @author Ryan Baxter + */ +@RunWith(ModifiedClassPathRunner.class) +@ClassPathExclusions({ "ribbon-loadbalancer-{version:\\d.*}.jar" }) +public class FeignHttpClientConfigurationTests { + + private ConfigurableApplicationContext context; + + @Before + public void setUp() { + context = new SpringApplicationBuilder().properties("debug=true","feign.httpclient.disableSslValidation=true").web(false) + .sources(HttpClientConfiguration.class, FeignAutoConfiguration.class).run(); + } + + @After + public void tearDown() { + if(context != null) { + context.close(); + } + } + + @Test + public void disableSslTest() throws Exception { + HttpClientConnectionManager connectionManager = context.getBean(HttpClientConnectionManager.class); + Lookup socketFactoryRegistry = getConnectionSocketFactoryLookup(connectionManager); + assertNotNull(socketFactoryRegistry.lookup("https")); + assertNull(this.getX509TrustManager(socketFactoryRegistry).getAcceptedIssuers()); + } + + private Lookup getConnectionSocketFactoryLookup(HttpClientConnectionManager connectionManager) { + DefaultHttpClientConnectionOperator connectionOperator = (DefaultHttpClientConnectionOperator)this.getField(connectionManager, "connectionOperator"); + return (Lookup)this.getField(connectionOperator, "socketFactoryRegistry"); + } + + private X509TrustManager getX509TrustManager(Lookup socketFactoryRegistry) { + ConnectionSocketFactory connectionSocketFactory = (ConnectionSocketFactory)socketFactoryRegistry.lookup("https"); + SSLSocketFactory sslSocketFactory = (SSLSocketFactory)this.getField(connectionSocketFactory, "socketfactory"); + SSLContextSpi sslContext = (SSLContextSpi)this.getField(sslSocketFactory, "context"); + return (X509TrustManager)this.getField(sslContext, "trustManager"); + } + + protected Object getField(Object target, String name) { + Field field = ReflectionUtils.findField(target.getClass(), name); + ReflectionUtils.makeAccessible(field); + Object value = ReflectionUtils.getField(field, target); + return value; + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignOkHttpConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignOkHttpConfigurationTests.java new file mode 100644 index 00000000..a55d758c --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignOkHttpConfigurationTests.java @@ -0,0 +1,72 @@ +/* + * Copyright 2013-2018 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; + +import okhttp3.OkHttpClient; + +import java.lang.reflect.Field; +import javax.net.ssl.HostnameVerifier; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; +import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; +import org.springframework.cloud.test.ClassPathExclusions; +import org.springframework.cloud.test.ModifiedClassPathRunner; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.util.ReflectionUtils; + +/** + * @author Ryan Baxter + */ +@RunWith(ModifiedClassPathRunner.class) +@ClassPathExclusions({ "ribbon-loadbalancer-{version:\\d.*}.jar" }) +public class FeignOkHttpConfigurationTests { + + private ConfigurableApplicationContext context; + + @Before + public void setUp() { + context = new SpringApplicationBuilder().properties("debug=true","feign.httpclient.disableSslValidation=true", + "feign.okhttp.enabled=true", "feign.httpclient.enabled=false").web(false) + .sources(HttpClientConfiguration.class, FeignAutoConfiguration.class).run(); + } + + @After + public void tearDown() { + if(context != null) { + context.close(); + } + } + + @Test + public void disableSslTest() throws Exception { + OkHttpClient httpClient = context.getBean(OkHttpClient.class); + HostnameVerifier hostnameVerifier = (HostnameVerifier)this.getField(httpClient, "hostnameVerifier"); + Assert.assertTrue(OkHttpClientFactory.TrustAllHostnames.class.isInstance(hostnameVerifier)); + } + + protected Object getField(Object target, String name) { + Field field = ReflectionUtils.findField(target.getClass(), name); + ReflectionUtils.makeAccessible(field); + Object value = ReflectionUtils.getField(field, target); + return value; + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java index 358c4004..d86ed087 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java @@ -23,6 +23,8 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -42,6 +44,12 @@ public class CachingSpringLoadBalancerFactoryTests { @Mock private RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; + @Mock + private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory; + + @Mock + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory; + private CachingSpringLoadBalancerFactory factory; @Before @@ -78,4 +86,51 @@ public class CachingSpringLoadBalancerFactoryTests { verify(this.delegate, times(1)).getClientConfig("client2"); } + @Test + public void delegateCreatesWithNoRetry() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory(this.delegate); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + + @Test + public void delegateCreatesWithRetry() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory( + this.delegate, loadBalancedRetryPolicyFactory, false); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + + @Test + public void delegateCreatesWithBackOff() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory( + this.delegate, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + + @Test + public void delegateCreatesWithRetryListener() { + IClientConfig config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.ConnectTimeout, 1000); + config.set(CommonClientConfigKey.ReadTimeout, 500); + when(this.delegate.getClientConfig("retry")).thenReturn(config); + CachingSpringLoadBalancerFactory factory = new CachingSpringLoadBalancerFactory( + this.delegate, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); + FeignLoadBalancer client = this.factory.create("retry"); + assertNotNull("client was null", client); + } + } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java index 0149e30a..96f9d302 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RetryableFeignLoadBalancerTests.java @@ -33,6 +33,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; @@ -43,7 +44,10 @@ import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerContext; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.http.HttpRequest; +import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; +import org.springframework.retry.TerminatedRetryException; import org.springframework.retry.backoff.BackOffContext; import org.springframework.retry.backoff.BackOffInterruptedException; import org.springframework.retry.backoff.BackOffPolicy; @@ -77,6 +81,7 @@ import static org.mockito.Mockito.when; /** * @author Ryan Baxter + * @author Gang Li */ public class RetryableFeignLoadBalancerTests { @Mock @@ -287,6 +292,95 @@ public class RetryableFeignLoadBalancerTests { } + @Test + public void retryListenerTest() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://listener")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + doThrow(new IOException("boom")).doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListeners myRetryListeners = new MyRetryListeners(); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, + backOffPolicyFactory, myRetryListeners); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(200, ribbonResponse.toResponse().status()); + verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); + assertEquals(1, backOffPolicyFactory.getCount()); + assertEquals(1, myRetryListeners.getOnError()); + } + + @Test(expected = TerminatedRetryException.class) + public void retryListenerTestNoRetry() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://listener")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListenersNotRetry myRetryListenersNotRetry = new MyRetryListenersNotRetry(); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, + backOffPolicyFactory, myRetryListenersNotRetry); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + } + + @Test + public void retryWithDefaultConstructorTest() throws Exception { + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + RibbonLoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://listener", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://listener")); + Response response = Response.builder().status(200).headers(new HashMap>()).build(); + doThrow(new IOException("boom")).doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + MyBackOffPolicyFactory backOffPolicyFactory = new MyBackOffPolicyFactory(); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryPolicyFactory, + backOffPolicyFactory); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(200, ribbonResponse.toResponse().status()); + verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); + assertEquals(1, backOffPolicyFactory.getCount()); + } + class MyBackOffPolicyFactory implements LoadBalancedBackOffPolicyFactory, BackOffPolicy { private int count = 0; @@ -311,4 +405,56 @@ public class RetryableFeignLoadBalancerTests { } } + class MyRetryListeners implements LoadBalancedRetryListenerFactory { + + private int onError = 0; + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return true; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + onError++; + } + }}; + } + + public int getOnError() { + return onError; + } + } + + class MyRetryListenersNotRetry implements LoadBalancedRetryListenerFactory { + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return false; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + }}; + } + } + } \ No newline at end of file diff --git a/spring-cloud-netflix-hystrix-contract/pom.xml b/spring-cloud-netflix-hystrix-contract/pom.xml index e8d38e46..893aba80 100644 --- a/spring-cloud-netflix-hystrix-contract/pom.xml +++ b/spring-cloud-netflix-hystrix-contract/pom.xml @@ -85,6 +85,14 @@ false + + spring-releases + Spring Releases + https://repo.spring.io/libs-release-local + + false + + diff --git a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java index e4ad0bb3..d6e5f275 100644 --- a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java +++ b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java @@ -35,6 +35,7 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; @@ -108,6 +109,13 @@ public class RibbonAutoConfiguration { return new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); } + @Bean + @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") + @ConditionalOnMissingBean + public LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory() { + return new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); + } + @Bean @ConditionalOnMissingBean public PropertiesFactory propertiesFactory() { diff --git a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java index 3ade9e73..f05ac558 100644 --- a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java +++ b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/HttpClientRibbonConfiguration.java @@ -33,6 +33,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; @@ -119,10 +120,9 @@ public class HttpClientRibbonConfiguration { @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") public RibbonLoadBalancingHttpClient ribbonLoadBalancingHttpClient( - IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler, CloseableHttpClient httpClient) { - RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient( - httpClient, config, serverIntrospector); + IClientConfig config, ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, RetryHandler retryHandler, CloseableHttpClient httpClient) { + RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient(httpClient, config, serverIntrospector); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -133,13 +133,14 @@ public class HttpClientRibbonConfiguration { @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") public RetryableRibbonLoadBalancingHttpClient retryableRibbonLoadBalancingHttpClient( - IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, CloseableHttpClient httpClient, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { + IClientConfig config, ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, RetryHandler retryHandler, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, CloseableHttpClient httpClient, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient( - httpClient, config, serverIntrospector, loadBalancedRetryPolicyFactory, - loadBalancedBackOffPolicyFactory); + httpClient, config, serverIntrospector, loadBalancedRetryPolicyFactory, + loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); diff --git a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java index 8e859db9..9ef78acf 100644 --- a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java @@ -26,6 +26,7 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -34,6 +35,7 @@ import org.springframework.cloud.netflix.ribbon.RibbonProperties; import org.springframework.cloud.netflix.ribbon.support.RibbonRetryPolicy; import org.springframework.http.HttpRequest; import org.springframework.retry.RetryCallback; +import org.springframework.retry.RetryListener; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -48,12 +50,15 @@ import com.netflix.loadbalancer.Server; /** * An Apache HTTP client which leverages Spring Retry to retry failed requests. * @author Ryan Baxter + * @author Gang Li */ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient implements ServiceInstanceChooser { private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = + new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); @Deprecated //TODO remove in 2.0.x @@ -73,6 +78,8 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } + @Deprecated + //TODO remove in 2.0.x public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, @@ -82,6 +89,17 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; } + public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, + IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + super(delegate, config, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory; + } + @Override public RibbonApacheHttpResponse execute(final RibbonApacheHttpRequest request, final IClientConfig configOverride) throws Exception { final RequestConfig.Builder builder = RequestConfig.custom(); @@ -130,6 +148,10 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH : new RetryPolicy(request, retryPolicy, this, this.getClientName())); BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); + RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + if (retryListeners != null && retryListeners.length != 0) { + retryTemplate.setListeners(retryListeners); + } return retryTemplate.execute(callback); } diff --git a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java index c84819ac..d3e5d3fb 100644 --- a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java +++ b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonConfiguration.java @@ -26,6 +26,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.OkHttpClientConnectionPoolFactory; import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; @@ -92,19 +93,20 @@ public class OkHttpRibbonConfiguration { } } - @Bean @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") - public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient(IClientConfig config, - ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, - RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, - OkHttpClient delegate, - LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { + public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient( + IClientConfig config, + ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, + RetryHandler retryHandler, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + OkHttpClient delegate, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient(delegate, config, - serverIntrospector, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory); + serverIntrospector, loadBalancedRetryPolicyFactory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -114,9 +116,10 @@ public class OkHttpRibbonConfiguration { @Bean @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") - public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient(IClientConfig config, - ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, - RetryHandler retryHandler, OkHttpClient delegate) { + public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient( + IClientConfig config, + ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, + RetryHandler retryHandler, OkHttpClient delegate) { OkHttpLoadBalancingClient client = new OkHttpLoadBalancingClient(delegate, config, serverIntrospector); client.setLoadBalancer(loadBalancer); diff --git a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java index ad18830f..3895e425 100644 --- a/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java @@ -24,6 +24,7 @@ import org.apache.commons.lang.BooleanUtils; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -32,6 +33,7 @@ import org.springframework.cloud.netflix.ribbon.support.RibbonRetryPolicy; import org.springframework.http.HttpRequest; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -47,12 +49,15 @@ import com.netflix.loadbalancer.Server; /** * An OK HTTP client which leverages Spring Retry to retry failed request. * @author Ryan Baxter + * @author Gang Li */ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClient implements ServiceInstanceChooser { private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = - new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = + new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); @Deprecated //TODO remove in 2.0.x @@ -62,6 +67,8 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } + @Deprecated + //TODO remove in 2.0.x public RetryableOkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory) { @@ -70,12 +77,25 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; } + public RetryableOkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) { + super(delegate, config, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + this.loadBalancedBackOffPolicyFactory = loadBalancedBackOffPolicyFactory; + this.loadBalancedRetryListenerFactory = loadBalancedRetryListenerFactory; + } + private OkHttpRibbonResponse executeWithRetry(OkHttpRibbonRequest request, LoadBalancedRetryPolicy retryPolicy, - RetryCallback callback) - throws Exception { + RetryCallback callback) throws Exception { RetryTemplate retryTemplate = new RetryTemplate(); BackOffPolicy backOffPolicy = loadBalancedBackOffPolicyFactory.createBackOffPolicy(this.getClientName()); retryTemplate.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); + RetryListener[] retryListeners = this.loadBalancedRetryListenerFactory.createRetryListeners(this.getClientName()); + if (retryListeners != null && retryListeners.length != 0) { + retryTemplate.setListeners(retryListeners); + } boolean retryable = request.getContext() == null ? true : BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true); retryTemplate.setRetryPolicy(retryPolicy == null || !retryable ? new NeverRetryPolicy() diff --git a/spring-cloud-netflix-ribbon/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java b/spring-cloud-netflix-ribbon/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java index 415d3f79..228a5be8 100644 --- a/spring-cloud-netflix-ribbon/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java +++ b/spring-cloud-netflix-ribbon/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java @@ -35,6 +35,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.springframework.cloud.client.loadbalancer.LoadBalancedBackOffPolicyFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryListenerFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; @@ -49,7 +50,10 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpMethod; +import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; +import org.springframework.retry.TerminatedRetryException; import org.springframework.retry.backoff.BackOffContext; import org.springframework.retry.backoff.BackOffInterruptedException; import org.springframework.retry.backoff.BackOffPolicy; @@ -82,11 +86,13 @@ import static org.mockito.Mockito.when; /** * @author Sébastien Nussbaumer * @author Ryan Baxter + * @author Gang Li */ public class RibbonLoadBalancingHttpClientTests { private ILoadBalancer loadBalancer; private LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + private LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory = new LoadBalancedRetryListenerFactory.DefaultRetryListenerFactory(); @Before public void setup() { @@ -247,6 +253,34 @@ public class RibbonLoadBalancingHttpClientTests { return client; } + private RetryableRibbonLoadBalancingHttpClient setupClientForRetry(int retriesNextServer, int retriesSameServer, + boolean retryable, boolean retryOnAllOps, + String serviceName, String host, int port, + CloseableHttpClient delegate, ILoadBalancer lb, String statusCodes, + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory, + boolean isSecure, LoadBalancedRetryListenerFactory loadBalancedRetryListenerFactory) throws Exception { + ServerIntrospector introspector = mock(ServerIntrospector.class); + RetryHandler retryHandler = new DefaultLoadBalancerRetryHandler(retriesSameServer, retriesNextServer, retryable); + doReturn(new Server(host, port)).when(lb).chooseServer(eq(serviceName)); + DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl(); + clientConfig.set(CommonClientConfigKey.OkToRetryOnAllOperations, retryOnAllOps); + clientConfig.set(CommonClientConfigKey.MaxAutoRetriesNextServer, retriesNextServer); + clientConfig.set(CommonClientConfigKey.MaxAutoRetries, retriesSameServer); + clientConfig.set(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES, statusCodes); + clientConfig.set(CommonClientConfigKey.IsSecure, isSecure); + clientConfig.setClientName(serviceName); + RibbonLoadBalancerContext context = new RibbonLoadBalancerContext(lb, clientConfig, retryHandler); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + doReturn(context).when(clientFactory).getLoadBalancerContext(eq(serviceName)); + doReturn(clientConfig).when(clientFactory).getClientConfig(eq(serviceName)); + LoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); + RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient(delegate, clientConfig, + introspector, factory, loadBalancedBackOffPolicyFactory, loadBalancedRetryListenerFactory); + client.setLoadBalancer(lb); + ReflectionTestUtils.setField(client, "delegate", delegate); + return client; + } + @Test public void testRetrySameServerOnly() throws Exception { int retriesNextServer = 0; @@ -500,6 +534,148 @@ public class RibbonLoadBalancingHttpClientTests { assertEquals(1, myBackOffPolicyFactory.getCount()); } + @Test + public void retryListenerTest() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListeners myRetryListeners = new MyRetryListeners(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false, myRetryListeners); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + verify(response, times(0)).close(); + verify(delegate, times(3)).execute(any(HttpUriRequest.class)); + verify(lb, times(1)).chooseServer(eq(serviceName)); + assertEquals(2, myBackOffPolicyFactory.getCount()); + assertEquals(2, myRetryListeners.getOnError()); + } + + @Test + public void retryDefaultListenerTest() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListeners myRetryListeners = new MyRetryListeners(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false, loadBalancedRetryListenerFactory); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + verify(response, times(0)).close(); + verify(delegate, times(3)).execute(any(HttpUriRequest.class)); + verify(lb, times(1)).chooseServer(eq(serviceName)); + assertEquals(2, myBackOffPolicyFactory.getCount()); + assertEquals(0, myRetryListeners.getOnError()); + } + + @Test(expected = TerminatedRetryException.class) + public void retryListenerTestNoRetry() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + MyRetryListenersNotRetry myRetryListenersNotRetry = new MyRetryListenersNotRetry(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false, myRetryListenersNotRetry); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + + } + + @Test + public void retryWithOriginalConstructorTest() throws Exception { + int retriesNextServer = 1; + int retriesSameServer = 1; + boolean retryable = true; + boolean retryOnAllOps = true; + String serviceName = "listener"; + String host = serviceName; + int port = 80; + HttpMethod method = HttpMethod.POST; + URI uri = new URI("http://" + host + ":" + port); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). + when(delegate).execute(any(HttpUriRequest.class)); + ILoadBalancer lb = mock(ILoadBalancer.class); + MyBackOffPolicyFactory myBackOffPolicyFactory = new MyBackOffPolicyFactory(); + RetryableRibbonLoadBalancingHttpClient client = setupClientForRetry(retriesNextServer, retriesSameServer, retryable, retryOnAllOps, + serviceName, host, port, delegate, lb, "", myBackOffPolicyFactory, false); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + doReturn(method).when(request).getMethod(); + doReturn(uri).when(request).getURI(); + doReturn(request).when(request).withNewUri(any(URI.class)); + HttpUriRequest uriRequest = mock(HttpUriRequest.class); + doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); + RibbonApacheHttpResponse returnedResponse = client.execute(request, null); + verify(response, times(0)).close(); + verify(delegate, times(3)).execute(any(HttpUriRequest.class)); + verify(lb, times(1)).chooseServer(eq(serviceName)); + assertEquals(2, myBackOffPolicyFactory.getCount()); + } + @Configuration protected static class UseDefaults { @@ -613,4 +789,56 @@ public class RibbonLoadBalancingHttpClientTests { } } + class MyRetryListeners implements LoadBalancedRetryListenerFactory { + + private int onError = 0; + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return true; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + onError++; + } + }}; + } + + public int getOnError() { + return onError; + } + } + + class MyRetryListenersNotRetry implements LoadBalancedRetryListenerFactory { + + @Override + public RetryListener[] createRetryListeners(String service) { + return new RetryListener[] {new RetryListener() { + @Override + public boolean open(RetryContext context, RetryCallback callback) { + return false; + } + + @Override + public void close(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + + @Override + public void onError(RetryContext context, RetryCallback callback, Throwable throwable) { + + } + }}; + } + } + } diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java index 273f7eab..07fdfb97 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java @@ -138,7 +138,10 @@ public class SimpleRouteLocator implements RouteLocator, Ordered { } String targetPath = path; String prefix = this.properties.getPrefix(); - if (path.startsWith(prefix) && this.properties.isStripPrefix()) { + if(prefix.endsWith("/")) { + prefix = prefix.substring(0, prefix.length() - 1); + } + if (path.startsWith(prefix + "/") && this.properties.isStripPrefix()) { targetPath = path.substring(prefix.length()); } if (route.isStripPrefix()) { diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index 6b7565c9..bbc5b682 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -144,7 +144,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter { private void initialize() { if(!customHttpClient) { this.connectionManager = connectionManagerFactory.newConnectionManager( - this.sslHostnameValidationEnabled, + !this.sslHostnameValidationEnabled, this.hostProperties.getMaxTotalConnections(), this.hostProperties.getMaxPerRouteConnections(), this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index 01e23553..33b0aa79 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -17,6 +17,8 @@ package org.springframework.cloud.netflix.zuul.filters.route.support; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; @@ -27,6 +29,7 @@ import org.springframework.http.client.ClientHttpResponse; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientRequest; import com.netflix.client.config.IClientConfig; +import com.netflix.client.config.IClientConfigKey; import com.netflix.client.http.HttpResponse; import com.netflix.config.DynamicIntProperty; import com.netflix.config.DynamicPropertyFactory; @@ -45,6 +48,7 @@ import com.netflix.zuul.context.RequestContext; public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> extends HystrixCommand implements RibbonCommand { + private static final Log LOGGER = LogFactory.getLog(AbstractRibbonCommand.class); protected final LBC client; protected RibbonCommandContext context; protected FallbackProvider zuulFallbackProvider; @@ -69,7 +73,7 @@ public abstract class AbstractRibbonCommand 0) { + hystrixTimeout = commandHystrixTimeout; + } + else if( defaultHystrixTimeout > 0) { + hystrixTimeout = defaultHystrixTimeout; + } else { + hystrixTimeout = ribbonTimeout; + } + if(hystrixTimeout < ribbonTimeout) { + LOGGER.warn("The Hystrix timeout of " + hystrixTimeout + "ms for the command " + commandKey + + " is set lower than the combination of the Ribbon read and connect timeout, " + ribbonTimeout + "ms."); + } + return HystrixCommandProperties.Setter().withExecutionIsolationStrategy( + zuulProperties.getRibbonIsolationStrategy()).withExecutionTimeoutInMilliseconds(hystrixTimeout); + } + + @Deprecated + //TODO remove in 2.0.x + protected static Setter getSetter(final String commandKey, ZuulProperties zuulProperties) { + return getSetter(commandKey, zuulProperties, null); + } + protected static Setter getSetter(final String commandKey, - ZuulProperties zuulProperties) { + ZuulProperties zuulProperties, IClientConfig config) { // @formatter:off Setter commandSetter = Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand")) .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey)); - - final HystrixCommandProperties.Setter setter = HystrixCommandProperties.Setter() - .withExecutionIsolationStrategy(zuulProperties.getRibbonIsolationStrategy()).withExecutionTimeoutInMilliseconds( - RibbonClientConfiguration.DEFAULT_CONNECT_TIMEOUT + RibbonClientConfiguration.DEFAULT_READ_TIMEOUT); + final HystrixCommandProperties.Setter setter = createSetter(config, commandKey, zuulProperties); if (zuulProperties.getRibbonIsolationStrategy() == ExecutionIsolationStrategy.SEMAPHORE){ final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; // we want to default to semaphore-isolation since this wraps diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocatorTests.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocatorTests.java index 0ba3d486..2f4ef1f0 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocatorTests.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocatorTests.java @@ -24,6 +24,7 @@ import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.collection.IsCollectionWithSize.hasSize; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import org.junit.Test; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; @@ -58,6 +59,25 @@ public class SimpleRouteLocatorTests { assertThat(routes, hasSize(1)); } + @Test + public void testStripPrefix() { + ZuulProperties properties = new ZuulProperties(); + properties.setPrefix("/test"); + properties.setStripPrefix(true); + RouteLocator locator = new FilteringRouteLocator("/", properties); + properties.getRoutes().put("testservicea", new ZuulRoute("/testservicea/**", "testservicea")); + assertEquals("/test/testservicea/**", locator.getRoutes().get(0).getFullPath()); + } + + @Test + public void testPrefix() { + ZuulProperties properties = new ZuulProperties(); + properties.setPrefix("/test/"); + RouteLocator locator = new FilteringRouteLocator("/", properties); + properties.getRoutes().put("testservicea", new ZuulRoute("/testservicea/**", "testservicea")); + assertEquals("/test/testservicea/**", locator.getRoutes().get(0).getFullPath()); + } + @Test public void test_getMatchingRouteFilterRouteAcceptor() { RouteLocator locator = new FilteringRouteLocator("/", this.zuul); diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactoryTest.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactoryTest.java index 65646bfd..e37616b3 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactoryTest.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactoryTest.java @@ -1,6 +1,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import java.util.HashSet; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -11,6 +12,9 @@ import org.springframework.cloud.netflix.zuul.filters.route.FallbackProvider; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; +import com.netflix.client.config.IClientConfigKey; +import com.netflix.config.ConfigurationManager; +import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyString; @@ -39,6 +43,12 @@ public class HttpClientRibbonCommandFactoryTest { this.ribbonCommandFactory = new HttpClientRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet()); } + @After + public void after() { + ConfigurationManager.getConfigInstance().clear(); + HystrixPropertiesFactory.reset(); + } + @Test public void testHystrixTimeoutValue() throws Exception { RibbonCommandContext context = mock(RibbonCommandContext.class); @@ -47,4 +57,50 @@ public class HttpClientRibbonCommandFactoryTest { assertEquals(2000, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); } + @Test + public void testHystrixTimeoutValueSetting() throws Exception { + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 50); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context); + assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + + @Test + public void testHystrixTimeoutValueCommandSetting() throws Exception { + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context); + assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + + @Test + public void testHystrixTimeoutValueCommandAndDefaultSetting() throws Exception { + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 30); + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + HttpClientRibbonCommand ribbonCommand = this.ribbonCommandFactory.create(context); + assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + + @Test + public void testHystrixTimeoutValueRibbonTimeouts() throws Exception { + SpringClientFactory springClientFactory = mock(SpringClientFactory.class); + ZuulProperties zuulProperties = new ZuulProperties(); + RibbonLoadBalancingHttpClient loadBalancingHttpClient = mock(RibbonLoadBalancingHttpClient.class); + IClientConfig clientConfig = new DefaultClientConfigImpl(); + clientConfig.set(IClientConfigKey.Keys.ConnectTimeout, 100); + clientConfig.set(IClientConfigKey.Keys.ReadTimeout, 500); + doReturn(loadBalancingHttpClient).when(springClientFactory).getClient(anyString(), + eq(RibbonLoadBalancingHttpClient.class)); + doReturn(clientConfig).when(springClientFactory).getClientConfig(anyString()); + HttpClientRibbonCommandFactory ribbonCommandFactory = new HttpClientRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet()); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + HttpClientRibbonCommand ribbonCommand = ribbonCommandFactory.create(context); + assertEquals(600, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + } \ No newline at end of file diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java index a6288a18..82536644 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactoryTest.java @@ -1,6 +1,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import java.util.HashSet; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -11,6 +12,9 @@ import org.springframework.cloud.netflix.zuul.filters.route.FallbackProvider; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; +import com.netflix.client.config.IClientConfigKey; +import com.netflix.config.ConfigurationManager; +import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyString; @@ -39,6 +43,12 @@ public class OkHttpRibbonCommandFactoryTest { commandFactory = new OkHttpRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet()); } + @After + public void after() { + ConfigurationManager.getConfigInstance().clear(); + HystrixPropertiesFactory.reset(); + } + @Test public void testHystrixTimeoutValue() throws Exception { RibbonCommandContext context = mock(RibbonCommandContext.class); @@ -47,4 +57,50 @@ public class OkHttpRibbonCommandFactoryTest { assertEquals(2000, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); } + @Test + public void testHystrixTimeoutValueSetting() throws Exception { + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 50); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context); + assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + + @Test + public void testHystrixTimeoutValueCommandSetting() throws Exception { + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context); + assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + + @Test + public void testHystrixTimeoutValueCommandAndDefaultSetting() throws Exception { + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds", 30); + ConfigurationManager.getConfigInstance().setProperty("hystrix.command.service.execution.isolation.thread.timeoutInMilliseconds", 50); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + OkHttpRibbonCommand ribbonCommand = this.commandFactory.create(context); + assertEquals(50, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + + @Test + public void testHystrixTimeoutValueRibbonTimeouts() throws Exception { + SpringClientFactory springClientFactory = mock(SpringClientFactory.class); + ZuulProperties zuulProperties = new ZuulProperties(); + OkHttpLoadBalancingClient loadBalancingHttpClient = mock(OkHttpLoadBalancingClient.class); + IClientConfig clientConfig = new DefaultClientConfigImpl(); + clientConfig.set(IClientConfigKey.Keys.ConnectTimeout, 100); + clientConfig.set(IClientConfigKey.Keys.ReadTimeout, 500); + doReturn(loadBalancingHttpClient).when(springClientFactory).getClient(anyString(), + eq(OkHttpLoadBalancingClient.class)); + doReturn(clientConfig).when(springClientFactory).getClientConfig(anyString()); + OkHttpRibbonCommandFactory commandFactory = new OkHttpRibbonCommandFactory(springClientFactory, zuulProperties, new HashSet()); + RibbonCommandContext context = mock(RibbonCommandContext.class); + doReturn("service").when(context).getServiceId(); + OkHttpRibbonCommand ribbonCommand = commandFactory.create(context); + assertEquals(600, ribbonCommand.getProperties().executionTimeoutInMilliseconds().get().intValue()); + } + } \ No newline at end of file diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java index 9ef46f59..c4b2162f 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java @@ -27,6 +27,7 @@ import com.netflix.client.ClientException; import com.netflix.client.ClientRequest; import com.netflix.client.IResponse; import com.netflix.client.RequestSpecificRetryHandler; +import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; import com.netflix.client.http.HttpResponse; import com.netflix.hystrix.HystrixCommandProperties; @@ -158,7 +159,7 @@ public class RibbonCommandCauseFallbackPropagationTest { AbstractLoadBalancerAwareClient client, FallbackProvider fallbackProvider, int timeout, RibbonCommandContext context) { // different name is used because of properties caching - super(getSetter("testCommand" + UUID.randomUUID(), new ZuulProperties()) + super(getSetter("testCommand" + UUID.randomUUID(), new ZuulProperties(), new DefaultClientConfigImpl()) .andCommandPropertiesDefaults(defauts(timeout)), client, context, fallbackProvider, null); }