From 9f10d5207fd39e130875d788f5ddecf50680e66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20D=C3=B6rnbrack?= Date: Fri, 9 Jun 2017 10:12:24 +0200 Subject: [PATCH 1/2] fixes a problem with whitespaces in list of retryable status codes --- .../ribbon/RibbonLoadBalancedRetryPolicy.java | 14 +++++++------- .../RibbonLoadBalancedRetryPolicyFactoryTest.java | 5 +++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java index 4e062ae5..874215f8 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java @@ -18,17 +18,17 @@ package org.springframework.cloud.netflix.ribbon; -import java.util.ArrayList; -import java.util.List; +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.IClientConfig; +import com.netflix.client.config.IClientConfigKey; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; -import org.springframework.core.env.Environment; import org.springframework.http.HttpMethod; import org.springframework.util.StringUtils; -import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.IClientConfig; -import com.netflix.client.config.IClientConfigKey; + +import java.util.ArrayList; +import java.util.List; /** * {@link LoadBalancedRetryPolicy} for Ribbon clients. @@ -60,7 +60,7 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy { for(String code : retryableStatusCodesArray) { if(!StringUtils.isEmpty(code)) { try { - retryableStatusCodes.add(Integer.valueOf(code)); + retryableStatusCodes.add(Integer.valueOf(code.trim())); } catch (NumberFormatException e) { //TODO log } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java index 3fedccea..595de862 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java @@ -201,7 +201,7 @@ public class RibbonLoadBalancedRetryPolicyFactoryTest { } @Test - public void testRetryableStatusCodest() throws Exception { + public void testRetryableStatusCodes() throws Exception { int sameServer = 3; int nextServer = 3; RibbonServer server = getRibbonServer(); @@ -210,7 +210,7 @@ public class RibbonLoadBalancedRetryPolicyFactoryTest { doReturn(nextServer).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); doReturn(false).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); doReturn(config).when(clientFactory).getClientConfig(eq(server.getServiceId())); - doReturn("404,502,foo, ,").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn("404,502, 418,foo, ,").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); clientFactory.getLoadBalancerContext(server.getServiceId()).setRetryHandler(new DefaultLoadBalancerRetryHandler(config)); RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server); RibbonLoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); @@ -220,6 +220,7 @@ public class RibbonLoadBalancedRetryPolicyFactoryTest { assertThat(policy.retryableStatusCode(400), is(false)); assertThat(policy.retryableStatusCode(404), is(true)); assertThat(policy.retryableStatusCode(502), is(true)); + assertThat(policy.retryableStatusCode(418), is(true)); } protected RibbonLoadBalancerClient getRibbonLoadBalancerClient( From 4f148de74d7b91f2916c0897ab883717939c8565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20D=C3=B6rnbrack?= Date: Fri, 9 Jun 2017 10:12:24 +0200 Subject: [PATCH 2/2] code cleanup --- .../ribbon/RibbonLoadBalancedRetryPolicy.java | 14 +++++++------- .../RibbonLoadBalancedRetryPolicyFactoryTest.java | 5 +++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java index 4e062ae5..874215f8 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicy.java @@ -18,17 +18,17 @@ package org.springframework.cloud.netflix.ribbon; -import java.util.ArrayList; -import java.util.List; +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.IClientConfig; +import com.netflix.client.config.IClientConfigKey; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; -import org.springframework.core.env.Environment; import org.springframework.http.HttpMethod; import org.springframework.util.StringUtils; -import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.IClientConfig; -import com.netflix.client.config.IClientConfigKey; + +import java.util.ArrayList; +import java.util.List; /** * {@link LoadBalancedRetryPolicy} for Ribbon clients. @@ -60,7 +60,7 @@ public class RibbonLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy { for(String code : retryableStatusCodesArray) { if(!StringUtils.isEmpty(code)) { try { - retryableStatusCodes.add(Integer.valueOf(code)); + retryableStatusCodes.add(Integer.valueOf(code.trim())); } catch (NumberFormatException e) { //TODO log } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java index 3fedccea..90c5739c 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancedRetryPolicyFactoryTest.java @@ -201,7 +201,7 @@ public class RibbonLoadBalancedRetryPolicyFactoryTest { } @Test - public void testRetryableStatusCodest() throws Exception { + public void testRetryableStatusCodes() throws Exception { int sameServer = 3; int nextServer = 3; RibbonServer server = getRibbonServer(); @@ -210,7 +210,7 @@ public class RibbonLoadBalancedRetryPolicyFactoryTest { doReturn(nextServer).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); doReturn(false).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); doReturn(config).when(clientFactory).getClientConfig(eq(server.getServiceId())); - doReturn("404,502,foo, ,").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn("404, 418,502,foo, ,").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); clientFactory.getLoadBalancerContext(server.getServiceId()).setRetryHandler(new DefaultLoadBalancerRetryHandler(config)); RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server); RibbonLoadBalancedRetryPolicyFactory factory = new RibbonLoadBalancedRetryPolicyFactory(clientFactory); @@ -219,6 +219,7 @@ public class RibbonLoadBalancedRetryPolicyFactoryTest { doReturn(HttpMethod.GET).when(request).getMethod(); assertThat(policy.retryableStatusCode(400), is(false)); assertThat(policy.retryableStatusCode(404), is(true)); + assertThat(policy.retryableStatusCode(418), is(true)); assertThat(policy.retryableStatusCode(502), is(true)); }