Repository: jclouds Updated Branches: refs/heads/master 69aa5d642 -> 9b3e6d91b
Fix retryOnRenew classes These classes should not close the release the payload as they are not reading it. - fix swift - fix openstack-swift v1 and v2 - fix RetryOnRenewTest for v1 and v2 Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/9b3e6d91 Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/9b3e6d91 Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/9b3e6d91 Branch: refs/heads/master Commit: 9b3e6d91bff99b39699a333eec798d1f2cd5cded Parents: 69aa5d6 Author: Andrea Turli <[email protected]> Authored: Wed Oct 26 16:55:21 2016 +0200 Committer: Andrea Turli <[email protected]> Committed: Fri Oct 28 17:29:05 2016 +0200 ---------------------------------------------------------------------- .../keystone/v2_0/handlers/RetryOnRenew.java | 77 +++++++++----------- .../v2_0/handlers/RetryOnRenewTest.java | 2 - .../openstack/handlers/RetryOnRenew.java | 71 ++++++++---------- .../keystone/v1_1/handlers/RetryOnRenew.java | 75 +++++++++---------- .../v1_1/handlers/RetryOnRenewTest.java | 4 - 5 files changed, 97 insertions(+), 132 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/9b3e6d91/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java ---------------------------------------------------------------------- diff --git a/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java b/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java index 07983e9..6364465 100644 --- a/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java +++ b/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java @@ -16,8 +16,6 @@ */ package org.jclouds.openstack.keystone.v2_0.handlers; -import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream; - import java.util.concurrent.TimeUnit; import javax.annotation.Resource; @@ -79,54 +77,45 @@ public class RetryOnRenew implements HttpRetryHandler { @Override public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { boolean retry = false; // default - try { - switch (response.getStatusCode()) { - case 401: - // Do not retry on 401 from authentication request - Multimap<String, String> headers = command.getCurrentRequest().getHeaders(); - if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) - && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { - retry = false; - } else { - closeClientButKeepContentStream(response); - // This is not an authentication request returning 401 - // Check if we already had seen this request - Integer count = retryCountMap.getIfPresent(command); + switch (response.getStatusCode()) { + case 401: + // Do not retry on 401 from authentication request + Multimap<String, String> headers = command.getCurrentRequest().getHeaders(); + if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) + && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { + retry = false; + } else { + // This is not an authentication request returning 401 + // Check if we already had seen this request + Integer count = retryCountMap.getIfPresent(command); - if (count == null) { - // First time this non-authentication request failed - logger.debug("invalidating authentication token - first time for %s", command); - retryCountMap.put(command, 1); + if (count == null) { + // First time this non-authentication request failed + logger.debug("invalidating authentication token - first time for %s", command); + retryCountMap.put(command, 1); + authenticationResponseCache.invalidateAll(); + retry = true; + } else { + // This request has failed before + if (count + 1 >= NUM_RETRIES) { + logger.debug("too many 401s - giving up after: %s for %s", count, command); + retry = false; + } else { + // Retry just in case + logger.debug("invalidating authentication token - retry %s for %s", count, command); + retryCountMap.put(command, count + 1); + // Wait between retries authenticationResponseCache.invalidateAll(); + Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS); retry = true; - } else { - // This request has failed before - if (count + 1 >= NUM_RETRIES) { - logger.debug("too many 401s - giving up after: %s for %s", count, command); - retry = false; - } else { - // Retry just in case - logger.debug("invalidating authentication token - retry %s for %s", count, command); - retryCountMap.put(command, count + 1); - // Wait between retries - authenticationResponseCache.invalidateAll(); - Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS); - retry = true; - } } } - break; - case 408: - return backoffHandler.shouldRetryRequest(command, response); - } - return retry; - } finally { - // If the request is failed and is not going to be retried, the - // ErrorHandler will be invoked and it might need to read the payload. - // For some kind of payload sources, such as the OkHttp Source, if the - // payload is released, the upcoming operations will fail. - closeClientButKeepContentStream(response); + } + break; + case 408: + return backoffHandler.shouldRetryRequest(command, response); } + return retry; } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/9b3e6d91/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java ---------------------------------------------------------------------- diff --git a/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java b/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java index badf120..840cda3 100644 --- a/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java +++ b/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java @@ -111,8 +111,6 @@ public class RetryOnRenewTest { LoadingCache<Credentials, Access> cache = createMock(LoadingCache.class); BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class); - expect(response.getPayload()).andReturn(Payloads.newStringPayload( - "The server has waited too long for the request to be sent by the client.")).times(3); expect(backoffHandler.shouldRetryRequest(command, response)).andReturn(true).once(); expect(response.getStatusCode()).andReturn(408).once(); http://git-wip-us.apache.org/repos/asf/jclouds/blob/9b3e6d91/common/openstack/src/main/java/org/jclouds/openstack/handlers/RetryOnRenew.java ---------------------------------------------------------------------- diff --git a/common/openstack/src/main/java/org/jclouds/openstack/handlers/RetryOnRenew.java b/common/openstack/src/main/java/org/jclouds/openstack/handlers/RetryOnRenew.java index 1d71d7f..19ce4a3 100644 --- a/common/openstack/src/main/java/org/jclouds/openstack/handlers/RetryOnRenew.java +++ b/common/openstack/src/main/java/org/jclouds/openstack/handlers/RetryOnRenew.java @@ -16,9 +16,6 @@ */ package org.jclouds.openstack.handlers; -import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream; -import static org.jclouds.http.HttpUtils.releasePayload; - import java.util.concurrent.TimeUnit; import javax.annotation.Resource; @@ -75,49 +72,43 @@ public class RetryOnRenew implements HttpRetryHandler { @Override public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { boolean retry = false; // default - try { - switch (response.getStatusCode()) { - case 401: - // Do not retry on 401 from authentication request - Multimap<String, String> headers = command.getCurrentRequest().getHeaders(); - if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) - && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { - retry = false; - } else { - closeClientButKeepContentStream(response); - // This is not an authentication request returning 401 - // Check if we already had seen this request - Integer count = retryCountMap.getIfPresent(command); + switch (response.getStatusCode()) { + case 401: + // Do not retry on 401 from authentication request + Multimap<String, String> headers = command.getCurrentRequest().getHeaders(); + if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) + && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { + retry = false; + } else { + // This is not an authentication request returning 401 + // Check if we already had seen this request + Integer count = retryCountMap.getIfPresent(command); - if (count == null) { - // First time this non-authentication request failed - logger.debug("invalidating authentication token - first time for %s", command); - retryCountMap.put(command, 1); + if (count == null) { + // First time this non-authentication request failed + logger.debug("invalidating authentication token - first time for %s", command); + retryCountMap.put(command, 1); + authenticationResponseCache.invalidateAll(); + retry = true; + } else { + // This request has failed before + if (count + 1 >= NUM_RETRIES) { + logger.debug("too many 401s - giving up after: %s for %s", count, command); + retry = false; + } else { + // Retry just in case + logger.debug("invalidating authentication token - retry %s for %s", count, command); + retryCountMap.put(command, count + 1); + // Wait between retries authenticationResponseCache.invalidateAll(); + Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS); retry = true; - } else { - // This request has failed before - if (count + 1 >= NUM_RETRIES) { - logger.debug("too many 401s - giving up after: %s for %s", count, command); - retry = false; - } else { - // Retry just in case - logger.debug("invalidating authentication token - retry %s for %s", count, command); - retryCountMap.put(command, count + 1); - // Wait between retries - authenticationResponseCache.invalidateAll(); - Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS); - retry = true; - } } } - break; - } - return retry; - - } finally { - releasePayload(response); + } + break; } + return retry; } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/9b3e6d91/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java ---------------------------------------------------------------------- diff --git a/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java b/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java index 47eb259..e03926f 100644 --- a/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java +++ b/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java @@ -16,9 +16,6 @@ */ package org.jclouds.openstack.keystone.v1_1.handlers; -import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream; -import static org.jclouds.http.HttpUtils.releasePayload; - import java.util.concurrent.TimeUnit; import javax.annotation.Resource; @@ -80,51 +77,45 @@ public class RetryOnRenew implements HttpRetryHandler { @Override public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { boolean retry = false; // default - try { - switch (response.getStatusCode()) { - case 401: - // Do not retry on 401 from authentication request - Multimap<String, String> headers = command.getCurrentRequest().getHeaders(); - if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) - && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { - retry = false; - } else { - closeClientButKeepContentStream(response); - // This is not an authentication request returning 401 - // Check if we already had seen this request - Integer count = retryCountMap.getIfPresent(command); + switch (response.getStatusCode()) { + case 401: + // Do not retry on 401 from authentication request + Multimap<String, String> headers = command.getCurrentRequest().getHeaders(); + if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) + && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { + retry = false; + } else { + // This is not an authentication request returning 401 + // Check if we already had seen this request + Integer count = retryCountMap.getIfPresent(command); - if (count == null) { - // First time this non-authentication request failed - logger.debug("invalidating authentication token - first time for %s", command); - retryCountMap.put(command, 1); + if (count == null) { + // First time this non-authentication request failed + logger.debug("invalidating authentication token - first time for %s", command); + retryCountMap.put(command, 1); + authenticationResponseCache.invalidateAll(); + retry = true; + } else { + // This request has failed before + if (count + 1 >= NUM_RETRIES) { + logger.debug("too many 401s - giving up after: %s for %s", count, command); + retry = false; + } else { + // Retry just in case + logger.debug("invalidating authentication token - retry %s for %s", count, command); + retryCountMap.put(command, count + 1); + // Wait between retries authenticationResponseCache.invalidateAll(); + Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS); retry = true; - } else { - // This request has failed before - if (count + 1 >= NUM_RETRIES) { - logger.debug("too many 401s - giving up after: %s for %s", count, command); - retry = false; - } else { - // Retry just in case - logger.debug("invalidating authentication token - retry %s for %s", count, command); - retryCountMap.put(command, count + 1); - // Wait between retries - authenticationResponseCache.invalidateAll(); - Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS); - retry = true; - } } } - break; - case 408: - return backoffHandler.shouldRetryRequest(command, response); - } - return retry; - - } finally { - releasePayload(response); + } + break; + case 408: + return backoffHandler.shouldRetryRequest(command, response); } + return retry; } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/9b3e6d91/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java ---------------------------------------------------------------------- diff --git a/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java b/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java index ec3d88d..8ee89f1 100644 --- a/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java +++ b/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java @@ -110,14 +110,11 @@ public class RetryOnRenewTest { @Test public void test408ShouldRetry() { HttpCommand command = createMock(HttpCommand.class); - HttpRequest request = createMock(HttpRequest.class); HttpResponse response = createMock(HttpResponse.class); @SuppressWarnings("unchecked") LoadingCache<Credentials, Auth> cache = createMock(LoadingCache.class); BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class); - expect(response.getPayload()).andReturn(Payloads.newStringPayload( - "The server has waited too long for the request to be sent by the client.")).times(2); expect(backoffHandler.shouldRetryRequest(command, response)).andReturn(true).once(); expect(response.getStatusCode()).andReturn(408).once(); @@ -145,7 +142,6 @@ public class RetryOnRenewTest { LoadingCache<Credentials, Auth> cache = createMock(LoadingCache.class); BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class); - expect(response.getPayload()).andReturn(Payloads.newStringPayload("")).times(2); expect(response.getStatusCode()).andReturn(404).once(); replay(command);
