diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/config/CloudStackRestClientModule.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/config/CloudStackRestClientModule.java index 9ddd95e128..2374cf0293 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/config/CloudStackRestClientModule.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/config/CloudStackRestClientModule.java @@ -122,7 +122,7 @@ import org.jclouds.cloudstack.filters.AuthenticationFilter; import org.jclouds.cloudstack.filters.QuerySigner; import org.jclouds.cloudstack.functions.LoginWithPasswordCredentials; import org.jclouds.cloudstack.handlers.CloudStackErrorHandler; -import org.jclouds.cloudstack.handlers.RetryOnRenewAndLogoutOnClose; +import org.jclouds.cloudstack.handlers.InvalidateSessionAndRetryOn401AndLogoutOnClose; import org.jclouds.concurrent.RetryOnTimeOutExceptionFunction; import org.jclouds.domain.Credentials; import org.jclouds.http.HttpErrorHandler; @@ -242,7 +242,7 @@ public class CloudStackRestClientModule extends RestClientModule authenticationResponseCache, - SessionClient sessionClient) { + protected InvalidateSessionAndRetryOn401AndLogoutOnClose(LoadingCache authenticationResponseCache, + SessionClient sessionClient) { this.authenticationResponseCache = authenticationResponseCache; this.sessionClient = sessionClient; } @Override public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { - boolean retry = false; // default try { switch (response.getStatusCode()) { case 401: - byte[] content = closeClientButKeepContentStream(response); - if (new String(content).equals("TODO: What state can we retry?")) { - logger.debug("invalidating session"); - authenticationResponseCache.invalidateAll(); - retry = true; - } + authenticationResponseCache.invalidateAll(); + return super.shouldRetryRequest(command, response); } - return retry; + return false; + } finally { releasePayload(response); } diff --git a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/CloudStackErrorHandlerTest.java b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/CloudStackErrorHandlerTest.java index 36ea5eaacb..8e404804d8 100644 --- a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/CloudStackErrorHandlerTest.java +++ b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/CloudStackErrorHandlerTest.java @@ -18,14 +18,7 @@ */ package org.jclouds.cloudstack.handlers; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.reportMatcher; -import static org.easymock.classextension.EasyMock.createMock; -import static org.easymock.classextension.EasyMock.replay; -import static org.easymock.classextension.EasyMock.verify; - -import java.net.URI; - +import com.google.inject.Guice; import org.easymock.IArgumentMatcher; import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpRequest; @@ -36,54 +29,59 @@ import org.jclouds.rest.ResourceNotFoundException; import org.jclouds.util.Strings2; import org.testng.annotations.Test; -import com.google.inject.Guice; +import java.net.URI; + +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.reportMatcher; +import static org.easymock.classextension.EasyMock.createMock; +import static org.easymock.classextension.EasyMock.replay; +import static org.easymock.classextension.EasyMock.verify; /** - * * @author Adrian Cole */ -@Test(groups = { "unit" }) +@Test(groups = {"unit"}) public class CloudStackErrorHandlerTest { @Test public void test400MakesIllegalArgumentException() { assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 400, "", "Bad Request", - IllegalArgumentException.class); + IllegalArgumentException.class); } @Test public void test401MakesAuthorizationException() { assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 401, "", "Unauthorized", - AuthorizationException.class); + AuthorizationException.class); } @Test public void test404MakesResourceNotFoundException() { assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 404, "", "Not Found", - ResourceNotFoundException.class); + ResourceNotFoundException.class); } @Test public void test405MakesIllegalArgumentException() { assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 405, "", "Method Not Allowed", - IllegalArgumentException.class); + IllegalArgumentException.class); } @Test public void test431MakesIllegalStateException() { assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 431, "", "Method Not Allowed", - IllegalStateException.class); + IllegalStateException.class); } @Test public void test431MakesResourceNotFoundExceptionOnDelete() { assertCodeMakes( - "GET", - URI.create("https://api.ninefold.com/compute/v1.0/?response=json&command=deleteSSHKeyPair"), - 431, - "", - "{ \"deletekeypairresponse\" : {\"errorcode\" : 431, \"errortext\" : \"A key pair with name 'adriancole-adapter-test-keypair' does not exist for account jclouds in domain id=457\"} }", - ResourceNotFoundException.class); + "GET", + URI.create("https://api.ninefold.com/compute/v1.0/?response=json&command=deleteSSHKeyPair"), + 431, + "", + "{ \"deletekeypairresponse\" : {\"errorcode\" : 431, \"errortext\" : \"A key pair with name 'adriancole-adapter-test-keypair' does not exist for account jclouds in domain id=457\"} }", + ResourceNotFoundException.class); } @Test @@ -91,30 +89,37 @@ public class CloudStackErrorHandlerTest { assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 409, "", "Conflict", IllegalStateException.class); } - @Test void test537MakesIllegalStateException() { + @Test + public void test531MakesAuthorizationException() { + assertCodeMakes("GET", URI.create("https://cloudstack.com/foo"), 531, "", "Unauthoized", + AuthorizationException.class); + } + + @Test + void test537MakesIllegalStateException() { assertCodeMakes( - "GET", - URI.create("http://10.26.26.155:8080/client/api?response=json&command=createIpForwardingRule&ipaddressid=37&startport=22&protocol=tcp"), - 537, - "", - "{ \"createipforwardingruleresponse\" : {\"errorcode\" : 537, \"errortext\" : \"There is already firewall rule specified for the ip address id=37\"} }", - IllegalStateException.class); + "GET", + URI.create("http://10.26.26.155:8080/client/api?response=json&command=createIpForwardingRule&ipaddressid=37&startport=22&protocol=tcp"), + 537, + "", + "{ \"createipforwardingruleresponse\" : {\"errorcode\" : 537, \"errortext\" : \"There is already firewall rule specified for the ip address id=37\"} }", + IllegalStateException.class); } private void assertCodeMakes(String method, URI uri, int statusCode, String message, String content, - Class expected) { + Class expected) { assertCodeMakes(method, uri, statusCode, message, "text/xml", content, expected); } private void assertCodeMakes(String method, URI uri, int statusCode, String message, String contentType, - String content, Class expected) { + String content, Class expected) { CloudStackErrorHandler function = Guice.createInjector().getInstance(CloudStackErrorHandler.class); HttpCommand command = createMock(HttpCommand.class); HttpRequest request = new HttpRequest(method, uri); HttpResponse response = new HttpResponse(statusCode, message, Payloads.newInputStreamPayload(Strings2 - .toInputStream(content))); + .toInputStream(content))); response.getPayload().getContentMetadata().setContentType(contentType); expect(command.getCurrentRequest()).andReturn(request).atLeastOnce(); diff --git a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/RetryOnRenewAndLogoutOnCloseTest.java b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/InvalidateSessionAndRetryOn401AndLogoutOnCloseTest.java similarity index 55% rename from apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/RetryOnRenewAndLogoutOnCloseTest.java rename to apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/InvalidateSessionAndRetryOn401AndLogoutOnCloseTest.java index 36622d8448..2166402701 100644 --- a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/RetryOnRenewAndLogoutOnCloseTest.java +++ b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/handlers/InvalidateSessionAndRetryOn401AndLogoutOnCloseTest.java @@ -18,12 +18,8 @@ */ package org.jclouds.cloudstack.handlers; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.verify; -import static org.testng.Assert.assertTrue; - +import com.google.common.cache.LoadingCache; +import org.easymock.IAnswer; import org.jclouds.cloudstack.domain.LoginResponse; import org.jclouds.cloudstack.features.SessionClient; import org.jclouds.domain.Credentials; @@ -32,33 +28,61 @@ import org.jclouds.http.HttpResponse; import org.jclouds.io.Payloads; import org.testng.annotations.Test; -import com.google.common.cache.LoadingCache; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; /** - * Tests behavior of {@code RetryOnRenewAndLogoutOnClose} handler - * + * Tests behavior of {@code InvalidateSessionAndRetryOn401AndLogoutOnClose} handler + * * @author grkvlt@apache.org */ -@Test(groups = "unit", testName = "RetryOnRenewAndLogoutOnCloseTest") -public class RetryOnRenewAndLogoutOnCloseTest { +@Test(groups = "unit", testName = "InvalidateSessionAndRetryOn401AndLogoutOnCloseTest") +public class InvalidateSessionAndRetryOn401AndLogoutOnCloseTest { + @SuppressWarnings("unchecked") @Test - public void test401ShouldRetry() { + public void test401ShouldRetryAndFailAfterFiveAttempts() { HttpCommand command = createMock(HttpCommand.class); SessionClient sessionClient = createMock(SessionClient.class); LoadingCache cache = createMock(LoadingCache.class); cache.invalidateAll(); - expectLastCall(); + expectLastCall().anyTimes(); + + final AtomicInteger counter = new AtomicInteger(); + expect(command.incrementFailureCount()).andAnswer(new IAnswer() { + @Override + public Integer answer() throws Throwable { + return counter.incrementAndGet(); + } + }).anyTimes(); + expect(command.isReplayable()).andReturn(true).anyTimes(); + expect(command.getFailureCount()).andAnswer(new IAnswer() { + @Override + public Integer answer() throws Throwable { + return counter.get(); + } + }).anyTimes(); replay(cache, command); HttpResponse response = HttpResponse.builder().payload( - Payloads.newStringPayload("TODO: What state can we retry?")).statusCode(401).build(); + Payloads.newStringPayload("Not relevant")).statusCode(401).build(); - RetryOnRenewAndLogoutOnClose retry = new RetryOnRenewAndLogoutOnClose(cache, sessionClient); + InvalidateSessionAndRetryOn401AndLogoutOnClose retry = + new InvalidateSessionAndRetryOn401AndLogoutOnClose(cache, sessionClient); - assertTrue(retry.shouldRetryRequest(command, response)); + for (int i = 0; i < 5; i++) { + assertTrue(retry.shouldRetryRequest(command, response)); + } + assertFalse(retry.shouldRetryRequest(command, response)); verify(cache, command); }