From 7672ee36660796fd08a235d737317339a433dbe6 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 21 Sep 2017 12:13:19 +0100 Subject: [PATCH] Fix hystrix command keys in tests to be unique --- .../cloud/netflix/AdhocTestSuite.java | 8 +- ...onCommandCauseFallbackPropagationTest.java | 466 +++++++++--------- ...ibbonCommandHystrixThreadPoolKeyTests.java | 67 ++- 3 files changed, 296 insertions(+), 245 deletions(-) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java index 888c4063..6f20fb93 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java @@ -21,8 +21,8 @@ import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; -import org.springframework.cloud.netflix.zuul.FormZuulServletProxyApplicationTests; -import org.springframework.cloud.netflix.zuul.filters.route.LazyLoadOfZuulConfigurationTests; +import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandCauseFallbackPropagationTest; +import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandHystrixThreadPoolKeyTests; /** * A test suite for probing weird ordering problems in the tests. @@ -30,8 +30,8 @@ import org.springframework.cloud.netflix.zuul.filters.route.LazyLoadOfZuulConfig * @author Dave Syer */ @RunWith(Suite.class) -@SuiteClasses({ FormZuulServletProxyApplicationTests.class, - LazyLoadOfZuulConfigurationTests.class }) +@SuiteClasses({ RibbonCommandHystrixThreadPoolKeyTests.class, + RibbonCommandCauseFallbackPropagationTest.class }) @Ignore public class AdhocTestSuite { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java index b0cc25d6..f7d62131 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandCauseFallbackPropagationTest.java @@ -17,6 +17,11 @@ package org.springframework.cloud.netflix.zuul.filters.route.support; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.UUID; + import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientException; import com.netflix.client.ClientRequest; @@ -26,18 +31,16 @@ import com.netflix.client.config.IClientConfig; import com.netflix.client.http.HttpResponse; import com.netflix.hystrix.HystrixCommandProperties; import com.netflix.hystrix.exception.HystrixTimeoutException; + import org.junit.Test; + import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; -import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.cloud.netflix.zuul.filters.route.FallbackProvider; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpResponse; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; - import static org.assertj.core.api.Assertions.assertThat; /** @@ -45,220 +48,241 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class RibbonCommandCauseFallbackPropagationTest { - @Test - public void providerIsCalledInCaseOfException() throws Exception { - TestZuulFallbackProviderWithoutCause provider = new TestZuulFallbackProviderWithoutCause(HttpStatus.INTERNAL_SERVER_ERROR); - RuntimeException exception = new RuntimeException("Failed!"); - TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), provider); - - ClientHttpResponse response = testCommand.execute(); - - assertThat(response).isNotNull(); - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); - } - - @Test - public void causeIsProvidedForNewInterface() throws Exception { - TestFallbackProvider provider = TestFallbackProvider.withResponse(HttpStatus.NOT_FOUND); - RuntimeException exception = new RuntimeException("Failed!"); - TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), provider); - - ClientHttpResponse response = testCommand.execute(); - - assertThat(response).isNotNull(); - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); - Throwable cause = provider.getCause(); - assertThat(cause.getClass()).isEqualTo(exception.getClass()); - assertThat(cause.getMessage()).isEqualTo(exception.getMessage()); - } - - @Test - public void executionExceptionIsUsedInsteadWhenFailedExceptionIsNull() throws Exception { - TestFallbackProvider provider = TestFallbackProvider.withResponse(HttpStatus.BAD_GATEWAY); - final RuntimeException exception = new RuntimeException("Failed!"); - TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), provider) { - @Override - public Throwable getFailedExecutionException() { - return null; - } - - @Override - public Throwable getExecutionException() { - return exception; - } - }; - - ClientHttpResponse response = testCommand.execute(); - - assertThat(response).isNotNull(); - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_GATEWAY); - } - - @Test - public void timeoutExceptionIsPropagated() throws Exception { - TestFallbackProvider provider = TestFallbackProvider.withResponse(HttpStatus.CONFLICT); - RuntimeException exception = new RuntimeException("Failed!"); - TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), provider, 1) { - @Override - protected ClientRequest createRequest() throws Exception { - Thread.sleep(5); - return super.createRequest(); - } - }; - - ClientHttpResponse response = testCommand.execute(); - - assertThat(response).isNotNull(); - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); - assertThat(provider.getCause()).isNotNull(); - assertThat(provider.getCause().getClass()).isEqualTo(HystrixTimeoutException.class); - } - - public static class TestRibbonCommand - extends AbstractRibbonCommand, ClientRequest, HttpResponse> { - - public TestRibbonCommand(AbstractLoadBalancerAwareClient client, ZuulFallbackProvider fallbackProvider) { - this(client, new ZuulProperties(), fallbackProvider); - } - - public TestRibbonCommand(AbstractLoadBalancerAwareClient client, - ZuulProperties zuulProperties, - ZuulFallbackProvider fallbackProvider) { - super("testCommand", client, null, zuulProperties, fallbackProvider); - } - - public TestRibbonCommand(AbstractLoadBalancerAwareClient client, - ZuulFallbackProvider fallbackProvider, - int timeout) { - // different name is used because of properties caching - super(getSetter("testCommand2", new ZuulProperties()).andCommandPropertiesDefaults(defauts(timeout)), - client, null, fallbackProvider, null); - } - - private static HystrixCommandProperties.Setter defauts(final int timeout) { - return HystrixCommandProperties.Setter().withExecutionTimeoutEnabled(true) - .withExecutionIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD) - .withExecutionTimeoutInMilliseconds(timeout); - } - - @Override - protected ClientRequest createRequest() throws Exception { - return null; - } - } - - public static class TestClient extends AbstractLoadBalancerAwareClient { - - private final RuntimeException exception; - - public TestClient(RuntimeException exception) { - super(null); - this.exception = exception; - } - - @Override - public IResponse executeWithLoadBalancer(final ClientRequest request, final IClientConfig requestConfig) throws ClientException { - throw exception; - } - - @Override - public RequestSpecificRetryHandler getRequestSpecificRetryHandler(final ClientRequest clientRequest, final IClientConfig iClientConfig) { - return null; - } - - @Override - public IResponse execute(final ClientRequest clientRequest, final IClientConfig iClientConfig) throws Exception { - return null; - } - } - - public static class TestFallbackProvider implements FallbackProvider { - - private final ClientHttpResponse response; - private Throwable cause; - - public TestFallbackProvider(final ClientHttpResponse response) { - this.response = response; - } - - @Override - public ClientHttpResponse fallbackResponse(final Throwable cause) { - this.cause = cause; - return response; - } - - @Override - public String getRoute() { - return "test-route"; - } - - @Override - public ClientHttpResponse fallbackResponse() { - throw new UnsupportedOperationException("fallback without cause is not supported"); - } - - public Throwable getCause() { - return cause; - } - - public static TestFallbackProvider withResponse(final HttpStatus status) { - return new TestFallbackProvider(getClientHttpResponse(status)); - } - } - - public static class TestZuulFallbackProviderWithoutCause implements ZuulFallbackProvider { - - private final ClientHttpResponse response; - - public TestZuulFallbackProviderWithoutCause(final ClientHttpResponse response) { - this.response = response; - } - - public TestZuulFallbackProviderWithoutCause(final HttpStatus status) { - this(getClientHttpResponse(status)); - } - - @Override - public String getRoute() { - return "test-route"; - } - - @Override - public ClientHttpResponse fallbackResponse() { - return response; - } - } - - private static ClientHttpResponse getClientHttpResponse(final HttpStatus status) { - return new ClientHttpResponse() { - @Override - public HttpStatus getStatusCode() throws IOException { - return status; - } - - @Override - public int getRawStatusCode() throws IOException { - return getStatusCode().value(); - } - - @Override - public String getStatusText() throws IOException { - return getStatusCode().getReasonPhrase(); - } - - @Override - public void close() { - } - - @Override - public InputStream getBody() throws IOException { - return new ByteArrayInputStream("test".getBytes()); - } - - @Override - public HttpHeaders getHeaders() { - return new HttpHeaders(); - } - }; - } + @Test + public void providerIsCalledInCaseOfException() throws Exception { + TestZuulFallbackProviderWithoutCause provider = new TestZuulFallbackProviderWithoutCause( + HttpStatus.INTERNAL_SERVER_ERROR); + RuntimeException exception = new RuntimeException("Failed!"); + TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), + provider); + + ClientHttpResponse response = testCommand.execute(); + + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + } + + @Test + public void causeIsProvidedForNewInterface() throws Exception { + TestFallbackProvider provider = TestFallbackProvider + .withResponse(HttpStatus.NOT_FOUND); + RuntimeException exception = new RuntimeException("Failed!"); + TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), + provider); + + ClientHttpResponse response = testCommand.execute(); + + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + Throwable cause = provider.getCause(); + assertThat(cause.getClass()).isEqualTo(exception.getClass()); + assertThat(cause.getMessage()).isEqualTo(exception.getMessage()); + } + + @Test + public void executionExceptionIsUsedInsteadWhenFailedExceptionIsNull() + throws Exception { + TestFallbackProvider provider = TestFallbackProvider + .withResponse(HttpStatus.BAD_GATEWAY); + final RuntimeException exception = new RuntimeException("Failed!"); + TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), + provider) { + @Override + public Throwable getFailedExecutionException() { + return null; + } + + @Override + public Throwable getExecutionException() { + return exception; + } + }; + + ClientHttpResponse response = testCommand.execute(); + + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_GATEWAY); + } + + @Test + public void timeoutExceptionIsPropagated() throws Exception { + TestFallbackProvider provider = TestFallbackProvider + .withResponse(HttpStatus.CONFLICT); + RuntimeException exception = new RuntimeException("Failed!"); + TestRibbonCommand testCommand = new TestRibbonCommand(new TestClient(exception), + provider, 1) { + @Override + protected ClientRequest createRequest() throws Exception { + Thread.sleep(5); + return super.createRequest(); + } + }; + + ClientHttpResponse response = testCommand.execute(); + + assertThat(response).isNotNull(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(provider.getCause()).isNotNull(); + assertThat(provider.getCause().getClass()) + .isEqualTo(HystrixTimeoutException.class); + } + + public static class TestRibbonCommand extends + AbstractRibbonCommand, ClientRequest, HttpResponse> { + + public TestRibbonCommand( + AbstractLoadBalancerAwareClient client, + ZuulFallbackProvider fallbackProvider) { + this(client, new ZuulProperties(), fallbackProvider); + } + + public TestRibbonCommand( + AbstractLoadBalancerAwareClient client, + ZuulProperties zuulProperties, ZuulFallbackProvider fallbackProvider) { + super("testCommand" + UUID.randomUUID(), client, null, zuulProperties, + fallbackProvider); + } + + public TestRibbonCommand( + AbstractLoadBalancerAwareClient client, + ZuulFallbackProvider fallbackProvider, int timeout) { + // different name is used because of properties caching + super(getSetter("testCommand" + UUID.randomUUID(), new ZuulProperties()) + .andCommandPropertiesDefaults(defauts(timeout)), client, null, + fallbackProvider, null); + } + + private static HystrixCommandProperties.Setter defauts(final int timeout) { + return HystrixCommandProperties.Setter().withExecutionTimeoutEnabled(true) + .withExecutionIsolationStrategy( + HystrixCommandProperties.ExecutionIsolationStrategy.THREAD) + .withExecutionTimeoutInMilliseconds(timeout); + } + + @Override + protected ClientRequest createRequest() throws Exception { + return null; + } + } + + @SuppressWarnings("rawtypes") + public static class TestClient extends AbstractLoadBalancerAwareClient { + + private final RuntimeException exception; + + public TestClient(RuntimeException exception) { + super(null); + this.exception = exception; + } + + @Override + public IResponse executeWithLoadBalancer(final ClientRequest request, + final IClientConfig requestConfig) throws ClientException { + throw exception; + } + + @Override + public RequestSpecificRetryHandler getRequestSpecificRetryHandler( + final ClientRequest clientRequest, final IClientConfig iClientConfig) { + return null; + } + + @Override + public IResponse execute(final ClientRequest clientRequest, + final IClientConfig iClientConfig) throws Exception { + return null; + } + } + + public static class TestFallbackProvider implements FallbackProvider { + + private final ClientHttpResponse response; + private Throwable cause; + + public TestFallbackProvider(final ClientHttpResponse response) { + this.response = response; + } + + @Override + public ClientHttpResponse fallbackResponse(final Throwable cause) { + this.cause = cause; + return response; + } + + @Override + public String getRoute() { + return "test-route"; + } + + @Override + public ClientHttpResponse fallbackResponse() { + throw new UnsupportedOperationException( + "fallback without cause is not supported"); + } + + public Throwable getCause() { + return cause; + } + + public static TestFallbackProvider withResponse(final HttpStatus status) { + return new TestFallbackProvider(getClientHttpResponse(status)); + } + } + + public static class TestZuulFallbackProviderWithoutCause + implements ZuulFallbackProvider { + + private final ClientHttpResponse response; + + public TestZuulFallbackProviderWithoutCause(final ClientHttpResponse response) { + this.response = response; + } + + public TestZuulFallbackProviderWithoutCause(final HttpStatus status) { + this(getClientHttpResponse(status)); + } + + @Override + public String getRoute() { + return "test-route"; + } + + @Override + public ClientHttpResponse fallbackResponse() { + return response; + } + } + + private static ClientHttpResponse getClientHttpResponse(final HttpStatus status) { + return new ClientHttpResponse() { + @Override + public HttpStatus getStatusCode() throws IOException { + return status; + } + + @Override + public int getRawStatusCode() throws IOException { + return getStatusCode().value(); + } + + @Override + public String getStatusText() throws IOException { + return getStatusCode().getReasonPhrase(); + } + + @Override + public void close() { + } + + @Override + public InputStream getBody() throws IOException { + return new ByteArrayInputStream("test".getBytes()); + } + + @Override + public HttpHeaders getHeaders() { + return new HttpHeaders(); + } + }; + } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java index 05ab8d4f..2003c5ae 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java @@ -20,8 +20,12 @@ import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientRequest; import com.netflix.client.http.HttpResponse; import com.netflix.hystrix.HystrixCommandProperties; +import com.netflix.hystrix.strategy.HystrixPlugins; + +import org.junit.After; import org.junit.Before; import org.junit.Test; + import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import static org.assertj.core.api.Assertions.assertThat; @@ -38,25 +42,38 @@ public class RibbonCommandHystrixThreadPoolKeyTests { zuulProperties = new ZuulProperties(); } + @After + public void tearDown() throws Exception { + HystrixPlugins.reset(); + } + @Test public void testDefaultHystrixThreadPoolKey() throws Exception { - zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); + zuulProperties.setRibbonIsolationStrategy( + HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); - TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); - TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", + zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", + zuulProperties); // CommandGroupKey should be used as ThreadPoolKey as default. - assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(ribbonCommand1.getCommandGroup().name()); - assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(ribbonCommand2.getCommandGroup().name()); + assertThat(ribbonCommand1.getThreadPoolKey().name()) + .isEqualTo(ribbonCommand1.getCommandGroup().name()); + assertThat(ribbonCommand2.getThreadPoolKey().name()) + .isEqualTo(ribbonCommand2.getCommandGroup().name()); } @Test public void testUseSeparateThreadPools() throws Exception { - zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); + zuulProperties.setRibbonIsolationStrategy( + HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); zuulProperties.getThreadPool().setUseSeparateThreadPools(true); - TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); - TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", + zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", + zuulProperties); assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo("testCommand1"); assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo("testCommand2"); @@ -66,35 +83,45 @@ public class RibbonCommandHystrixThreadPoolKeyTests { public void testThreadPoolKeyPrefix() throws Exception { final String prefix = "zuulgw-"; - zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); + zuulProperties.setRibbonIsolationStrategy( + HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); zuulProperties.getThreadPool().setUseSeparateThreadPools(true); zuulProperties.getThreadPool().setThreadPoolKeyPrefix(prefix); - TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); - TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", + zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", + zuulProperties); - assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(prefix + "testCommand1"); - assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(prefix + "testCommand2"); + assertThat(ribbonCommand1.getThreadPoolKey().name()) + .isEqualTo(prefix + "testCommand1"); + assertThat(ribbonCommand2.getThreadPoolKey().name()) + .isEqualTo(prefix + "testCommand2"); } @Test public void testNoSideEffectOnSemaphoreIsolation() throws Exception { final String prefix = "zuulgw-"; - zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE); + zuulProperties.setRibbonIsolationStrategy( + HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE); zuulProperties.getThreadPool().setUseSeparateThreadPools(true); zuulProperties.getThreadPool().setThreadPoolKeyPrefix(prefix); - TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); - TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", + zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", + zuulProperties); // There should be no side effect on semaphore isolation - assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(ribbonCommand1.getCommandGroup().name()); - assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(ribbonCommand2.getCommandGroup().name()); + assertThat(ribbonCommand1.getThreadPoolKey().name()) + .isEqualTo(ribbonCommand1.getCommandGroup().name()); + assertThat(ribbonCommand2.getThreadPoolKey().name()) + .isEqualTo(ribbonCommand2.getCommandGroup().name()); } - public static class TestRibbonCommand - extends AbstractRibbonCommand, ClientRequest, HttpResponse> { + public static class TestRibbonCommand extends + AbstractRibbonCommand, ClientRequest, HttpResponse> { public TestRibbonCommand(String commandKey, ZuulProperties zuulProperties) { super(commandKey, null, null, zuulProperties); }