mirror of https://github.com/apache/jclouds.git
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
This commit is contained in:
parent
69aa5d6426
commit
9b3e6d91bf
|
@ -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);
|
||||
authenticationResponseCache.invalidateAll();
|
||||
retry = true;
|
||||
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 {
|
||||
// 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;
|
||||
}
|
||||
// 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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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();
|
||||
|
||||
|
|
|
@ -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);
|
||||
authenticationResponseCache.invalidateAll();
|
||||
retry = true;
|
||||
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 {
|
||||
// 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;
|
||||
}
|
||||
// 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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
authenticationResponseCache.invalidateAll();
|
||||
retry = true;
|
||||
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 {
|
||||
// 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;
|
||||
}
|
||||
// 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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
|
Loading…
Reference in New Issue