From 63320f15b61466c66cdbd8ba96a420641371c7a4 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 15 Jan 2012 11:30:22 -0800 Subject: [PATCH] Issue 731: unravel dependency cycle to only use interfaces --- ...BaseCloudLoadBalancersAsyncClientTest.java | 3 +- .../config/OpenStackAuthenticationModule.java | 79 +++++------ .../openstack/handlers/RetryOnRenew.java | 15 +-- .../openstack/handlers/RetryOnRenewTest.java | 15 ++- .../RetryOnTimeOutExceptionFunction.java | 77 +++++++++++ .../RetryOnTimeOutExceptionFunctionTest.java | 126 ++++++++++++++++++ 6 files changed, 256 insertions(+), 59 deletions(-) create mode 100644 core/src/main/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunction.java create mode 100644 core/src/test/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunctionTest.java diff --git a/apis/cloudloadbalancers/src/test/java/org/jclouds/cloudloadbalancers/features/BaseCloudLoadBalancersAsyncClientTest.java b/apis/cloudloadbalancers/src/test/java/org/jclouds/cloudloadbalancers/features/BaseCloudLoadBalancersAsyncClientTest.java index 05280b785e..a619a26b87 100644 --- a/apis/cloudloadbalancers/src/test/java/org/jclouds/cloudloadbalancers/features/BaseCloudLoadBalancersAsyncClientTest.java +++ b/apis/cloudloadbalancers/src/test/java/org/jclouds/cloudloadbalancers/features/BaseCloudLoadBalancersAsyncClientTest.java @@ -30,6 +30,7 @@ import org.jclouds.cloudloadbalancers.CloudLoadBalancersClient; import org.jclouds.cloudloadbalancers.config.CloudLoadBalancersRestClientModule; import org.jclouds.cloudloadbalancers.functions.ConvertLB; import org.jclouds.cloudloadbalancers.reference.Region; +import org.jclouds.domain.Credentials; import org.jclouds.http.HttpRequest; import org.jclouds.http.RequiresHttp; import org.jclouds.internal.ClassMethodArgs; @@ -78,7 +79,7 @@ public abstract class BaseCloudLoadBalancersAsyncClientTest extends RestClien install(new OpenStackAuthenticationModule() { @Override protected Supplier provideAuthenticationResponseSupplier( - final LoadingCache cache) { + LoadingCache cache, Credentials in) { return Suppliers.ofInstance(new AuthenticationResponse("token", ImmutableMap. of())); } }); diff --git a/common/openstack/src/main/java/org/jclouds/openstack/config/OpenStackAuthenticationModule.java b/common/openstack/src/main/java/org/jclouds/openstack/config/OpenStackAuthenticationModule.java index e7f180d312..0d260b205b 100644 --- a/common/openstack/src/main/java/org/jclouds/openstack/config/OpenStackAuthenticationModule.java +++ b/common/openstack/src/main/java/org/jclouds/openstack/config/OpenStackAuthenticationModule.java @@ -29,14 +29,17 @@ import javax.inject.Named; import javax.inject.Singleton; import org.jclouds.Constants; -import org.jclouds.concurrent.RetryOnTimeOutExceptionSupplier; +import org.jclouds.concurrent.RetryOnTimeOutExceptionFunction; import org.jclouds.date.TimeStamp; +import org.jclouds.domain.Credentials; import org.jclouds.http.RequiresHttp; +import org.jclouds.location.Provider; import org.jclouds.openstack.Authentication; import org.jclouds.openstack.OpenStackAuthAsyncClient; import org.jclouds.openstack.OpenStackAuthAsyncClient.AuthenticationResponse; import org.jclouds.rest.AsyncClientFactory; +import com.google.common.base.Function; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import com.google.common.base.Throwables; @@ -46,6 +49,7 @@ import com.google.common.cache.LoadingCache; import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.inject.AbstractModule; import com.google.inject.Provides; +import com.google.inject.TypeLiteral; /** * Configures the Rackspace authentication service connection, including logging and http transport. @@ -57,6 +61,8 @@ public class OpenStackAuthenticationModule extends AbstractModule { @Override protected void configure() { + bind(new TypeLiteral>() { + }).to(GetAuthenticationResponse.class); } /** @@ -74,68 +80,57 @@ public class OpenStackAuthenticationModule extends AbstractModule { }; } + @Provides + @Provider + protected Credentials provideAuthenticationCredentials(@Named(Constants.PROPERTY_IDENTITY) String user, + @Named(Constants.PROPERTY_CREDENTIAL) String key) { + return new Credentials(user, key); + } + @Singleton - public static class GetAuthenticationResponse implements Supplier { - protected final OpenStackAuthAsyncClient client; - protected final String user; - protected final String key; + public static class GetAuthenticationResponse extends + RetryOnTimeOutExceptionFunction { @Inject - public GetAuthenticationResponse(AsyncClientFactory factory, @Named(Constants.PROPERTY_IDENTITY) String user, - @Named(Constants.PROPERTY_CREDENTIAL) String key) { - this.client = factory.create(OpenStackAuthAsyncClient.class); - this.user = user; - this.key = key; - } + public GetAuthenticationResponse(final AsyncClientFactory factory) { + super(new Function() { - @Override - public AuthenticationResponse get() { - try { - Future response = authenticate(); - return response.get(30, TimeUnit.SECONDS); - } catch (Exception e) { - Throwables.propagate(e); - assert false : e; - return null; - } - } + @Override + public AuthenticationResponse apply(Credentials input) { + try { + Future response = factory.create(OpenStackAuthAsyncClient.class) + .authenticate(input.identity, input.credential); + return response.get(30, TimeUnit.SECONDS); + } catch (Exception e) { + throw Throwables.propagate(e); + } + } + }); - protected Future authenticate() { - return client.authenticate(user, key); } - } @Provides @Singleton - public LoadingCache provideAuthenticationResponseCache2( - final GetAuthenticationResponse getAuthenticationResponse) { - - final RetryOnTimeOutExceptionSupplier delegate = - new RetryOnTimeOutExceptionSupplier(getAuthenticationResponse); + public LoadingCache provideAuthenticationResponseCache2( + final Function getAuthenticationResponse, + @Provider final Credentials creds) { + + LoadingCache cache = CacheBuilder.newBuilder().expireAfterWrite(23, + TimeUnit.HOURS).build(CacheLoader.from(getAuthenticationResponse)); - CacheLoader cacheLoader = new CacheLoader() { - @Override - public AuthenticationResponse load(String key) throws Exception { - return delegate.get(); - } - }; - - LoadingCache cache = CacheBuilder.newBuilder().expireAfterWrite(23, TimeUnit.HOURS) - .build(cacheLoader); - return cache; } @Provides @Singleton protected Supplier provideAuthenticationResponseSupplier( - final LoadingCache cache) { + final LoadingCache cache, @Provider final Credentials creds) { return new Supplier() { @Override public AuthenticationResponse get() { try { - return cache.get("key"); + return cache.get(creds); } catch (UncheckedExecutionException e) { throw Throwables.propagate(e.getCause()); } catch (ExecutionException e) { 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 edb39e1a38..dc0cb7e6da 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 @@ -24,6 +24,7 @@ import java.io.IOException; import javax.annotation.Resource; +import org.jclouds.domain.Credentials; import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpRetryHandler; @@ -48,12 +49,8 @@ public class RetryOnRenew implements HttpRetryHandler { @Resource protected Logger logger = Logger.NULL; - // This doesn't work yet -// @Inject -// Supplier providedAuthenticationResponseCache; - @Inject - LoadingCache authenticationResponseCache; + LoadingCache authenticationResponseCache; @Override public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { @@ -63,8 +60,8 @@ public class RetryOnRenew implements HttpRetryHandler { case 401: // Do not retry on 401 from authentication request Multimap headers = command.getCurrentRequest().getHeaders(); - if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) && headers.containsKey(AuthHeaders.AUTH_KEY) && - !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { + if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER) + && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { retry = false; } else { String content = parsePayloadOrNull(response); @@ -79,12 +76,12 @@ public class RetryOnRenew implements HttpRetryHandler { break; } return retry; - + } finally { releasePayload(response); } } - + String parsePayloadOrNull(HttpResponse response) { if (response.getPayload() != null) { try { diff --git a/common/openstack/src/test/java/org/jclouds/openstack/handlers/RetryOnRenewTest.java b/common/openstack/src/test/java/org/jclouds/openstack/handlers/RetryOnRenewTest.java index 0aa578f3e4..c59629cfd2 100644 --- a/common/openstack/src/test/java/org/jclouds/openstack/handlers/RetryOnRenewTest.java +++ b/common/openstack/src/test/java/org/jclouds/openstack/handlers/RetryOnRenewTest.java @@ -18,13 +18,14 @@ */ package org.jclouds.openstack.handlers; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.classextension.EasyMock.createMock; -import static org.easymock.classextension.EasyMock.replay; -import static org.easymock.classextension.EasyMock.verify; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; import static org.testng.Assert.assertTrue; +import org.jclouds.domain.Credentials; import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; @@ -47,23 +48,23 @@ public class RetryOnRenewTest { HttpRequest request = createMock(HttpRequest.class); HttpResponse response = createMock(HttpResponse.class); @SuppressWarnings("unchecked") - LoadingCache cache = createMock(LoadingCache.class); + LoadingCache cache = createMock(LoadingCache.class); expect(command.getCurrentRequest()).andReturn(request); cache.invalidateAll(); expectLastCall(); - + expect(response.getPayload()).andReturn(Payloads.newStringPayload("token expired, please renew")).anyTimes(); expect(response.getStatusCode()).andReturn(401).atLeastOnce(); replay(command); replay(response); replay(cache); - + RetryOnRenew retry = new RetryOnRenew(); retry.authenticationResponseCache = cache; - + assertTrue(retry.shouldRetryRequest(command, response)); verify(command); diff --git a/core/src/main/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunction.java b/core/src/main/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunction.java new file mode 100644 index 0000000000..c2fd9f9c5b --- /dev/null +++ b/core/src/main/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunction.java @@ -0,0 +1,77 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jclouds.concurrent; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Throwables.propagate; + +import java.util.concurrent.TimeoutException; + +import org.jclouds.util.Throwables2; + +import com.google.common.base.Function; +import com.google.common.collect.ForwardingObject; + +/** + * + * @author Adrian Cole + */ +public class RetryOnTimeOutExceptionFunction extends ForwardingObject implements Function{ + private final Function delegate; + + public RetryOnTimeOutExceptionFunction(Function delegate) { + this.delegate = checkNotNull(delegate, "delegate"); + } + + //TODO: backoff limited retry handler + @Override + public V apply(K key) { + TimeoutException ex = null; + for (int i = 0; i < 3; i++) { + try { + ex = null; + return delegate().apply(key); + } catch (Exception e) { + if ((ex = Throwables2.getFirstThrowableOfType(e, TimeoutException.class)) != null) + continue; + throw propagate(e); + } + } + if (ex != null) + throw propagate(ex); + assert false; + return null; + } + + @Override + public boolean equals(Object obj) { + return delegate.equals(obj); + } + + @Override + public int hashCode() { + return delegate.hashCode(); + } + + @Override + protected Function delegate() { + return delegate; + } + +} \ No newline at end of file diff --git a/core/src/test/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunctionTest.java b/core/src/test/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunctionTest.java new file mode 100644 index 0000000000..4b2136e3a1 --- /dev/null +++ b/core/src/test/java/org/jclouds/concurrent/RetryOnTimeOutExceptionFunctionTest.java @@ -0,0 +1,126 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jclouds.concurrent; + +import static org.easymock.EasyMock.expect; +import static org.easymock.classextension.EasyMock.createMock; +import static org.easymock.classextension.EasyMock.replay; +import static org.easymock.classextension.EasyMock.verify; +import static org.testng.Assert.assertEquals; + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeoutException; + +import org.jclouds.rest.AuthorizationException; +import org.testng.annotations.Test; + +import com.google.common.base.Function; + +/** + * Tests behavior of RetryOnTimeOutExceptionFunction + * + * @author Adrian Cole + */ +@Test(groups = "unit", singleThreaded = true, testName = "RetryOnTimeOutExceptionFunctionTest") +public class RetryOnTimeOutExceptionFunctionTest { + ExecutorService executorService = MoreExecutors.sameThreadExecutor(); + + @SuppressWarnings("unchecked") + @Test + public void testGetThrowsOriginalExceptionButRetriesOnTimeoutException() throws InterruptedException, ExecutionException { + Function delegate = createMock(Function.class); + TimeoutException timeout = createMock(TimeoutException.class); + RuntimeException throwable = new RuntimeException(timeout); + + expect(delegate.apply("baz")).andThrow(throwable); + expect(timeout.getCause()).andReturn(null).anyTimes(); + expect(delegate.apply("baz")).andThrow(throwable); + expect(delegate.apply("baz")).andThrow(throwable); + + replay(delegate); + replay(timeout); + + RetryOnTimeOutExceptionFunction supplier = new RetryOnTimeOutExceptionFunction( + delegate); + try { + supplier.apply("baz"); + assert false; + } catch (RuntimeException e) { + assertEquals(e.getCause(), timeout); + } + + verify(delegate); + verify(timeout); + + } + + @SuppressWarnings("unchecked") + @Test + public void testGetAllowsTwoFailuresOnTimeoutException() throws InterruptedException, ExecutionException { + Function delegate = createMock(Function.class); + TimeoutException timeout = createMock(TimeoutException.class); + RuntimeException throwable = new RuntimeException(timeout); + + expect(delegate.apply("baz")).andThrow(throwable); + expect(timeout.getCause()).andReturn(null).anyTimes(); + expect(delegate.apply("baz")).andThrow(throwable); + expect(delegate.apply("baz")).andReturn("foo"); + + replay(delegate); + replay(timeout); + + RetryOnTimeOutExceptionFunction supplier = new RetryOnTimeOutExceptionFunction( + delegate); + assertEquals(supplier.apply("baz"), "foo"); + + verify(delegate); + verify(timeout); + } + + @SuppressWarnings("unchecked") + @Test + public void testGetAllowsNoFailuresOnOtherExceptions() throws InterruptedException, ExecutionException { + Function delegate = createMock(Function.class); + AuthorizationException auth = createMock(AuthorizationException.class); + RuntimeException throwable = new RuntimeException(auth); + + expect(delegate.apply("baz")).andThrow(throwable); + expect(auth.getCause()).andReturn(null).anyTimes(); + + + replay(delegate); + replay(auth); + + RetryOnTimeOutExceptionFunction supplier = new RetryOnTimeOutExceptionFunction( + delegate); + + try { + supplier.apply("baz"); + assert false; + } catch (RuntimeException e) { + assertEquals(e.getCause(), auth); + } + + verify(delegate); + verify(auth); + + } + +}