From 95141ff8770f373ff1b77365381cadf79dea2803 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 16 Nov 2014 12:26:35 -0800 Subject: [PATCH] Revert 428b2bd2ea2d73354e15ffad52516105b15576cd as this hides inconsistency between regions and zones. --- apis/ec2/pom.xml | 12 ++ .../DescribeAvailabilityZonesInRegion.java | 57 ++------ .../ec2/internal/BaseEC2ApiMockTest.java | 133 ++++++++++++++++++ ...ribeAvailabilityZonesInRegionMockTest.java | 75 ++++++++++ ...DescribeAvailabilityZonesInRegionTest.java | 124 ---------------- 5 files changed, 232 insertions(+), 169 deletions(-) create mode 100644 apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ApiMockTest.java create mode 100644 apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionMockTest.java delete mode 100644 apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionTest.java diff --git a/apis/ec2/pom.xml b/apis/ec2/pom.xml index fb29552b0d..2a1b89d31d 100644 --- a/apis/ec2/pom.xml +++ b/apis/ec2/pom.xml @@ -91,6 +91,18 @@ ${project.version} test + + com.squareup.okhttp + mockwebserver + test + + + + org.bouncycastle + bcprov-jdk15on + + + diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegion.java b/apis/ec2/src/main/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegion.java index f97e962f95..67cb55d192 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegion.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegion.java @@ -19,74 +19,41 @@ package org.jclouds.ec2.suppliers; import java.util.Map; import java.util.Set; -import javax.annotation.Resource; import javax.inject.Inject; -import javax.inject.Singleton; import org.jclouds.ec2.EC2Api; import org.jclouds.ec2.domain.AvailabilityZoneInfo; import org.jclouds.ec2.features.AvailabilityZoneAndRegionApi; -import org.jclouds.http.HttpResponseException; import org.jclouds.location.Region; import org.jclouds.location.suppliers.RegionIdToZoneIdsSupplier; -import org.jclouds.logging.Logger; -import org.jclouds.util.Suppliers2; -import com.google.common.base.Function; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.common.collect.Maps; -@Singleton -public class DescribeAvailabilityZonesInRegion implements RegionIdToZoneIdsSupplier { - @Resource - protected Logger logger = Logger.NULL; - - private final AvailabilityZoneAndRegionApi client; +public final class DescribeAvailabilityZonesInRegion implements RegionIdToZoneIdsSupplier { + private final EC2Api api; private final Supplier> regions; @Inject - public DescribeAvailabilityZonesInRegion(EC2Api client, @Region Supplier> regions) { - this.client = client.getAvailabilityZoneAndRegionApi().get(); + DescribeAvailabilityZonesInRegion(EC2Api api, @Region Supplier> regions) { + this.api = api; this.regions = regions; } @Override public Map>> get() { - Builder> map = ImmutableMap.builder(); - HttpResponseException exception = null; - // TODO: this should be parallel + AvailabilityZoneAndRegionApi zoneApi = api.getAvailabilityZoneAndRegionApi().get(); + Builder>> map = ImmutableMap.builder(); for (String region : regions.get()) { - try { - ImmutableSet zones = ImmutableSet.copyOf(Iterables.transform(client - .describeAvailabilityZonesInRegion(region), new Function() { - - @Override - public String apply(AvailabilityZoneInfo arg0) { - return arg0.getZone(); - } - - })); - if (zones.size() > 0) - map.put(region, zones); - } catch (HttpResponseException e) { - // TODO: this should be in retry handler, not here. - if (e.getMessage().contains("Unable to tunnel through proxy")) { - exception = e; - logger.error(e, "Could not describe availability zones in Region: %s", region); - } else { - throw e; - } + ImmutableSet.Builder zoneBuilder = ImmutableSet.builder(); + for (AvailabilityZoneInfo zone : zoneApi.describeAvailabilityZonesInRegion(region)) { + zoneBuilder.add(zone.getZone()); } + map.put(region, Suppliers.>ofInstance(zoneBuilder.build())); } - ImmutableMap> result = map.build(); - if (result.isEmpty() && exception != null) { - throw exception; - } - return Maps.transformValues(result, Suppliers2.> ofInstanceFunction()); + return map.build(); } - } diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ApiMockTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ApiMockTest.java new file mode 100644 index 0000000000..7834ba0869 --- /dev/null +++ b/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ApiMockTest.java @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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.ec2.internal; + +import static com.google.common.base.Throwables.propagate; +import static com.google.common.net.HttpHeaders.CONTENT_TYPE; +import static com.google.common.util.concurrent.MoreExecutors.sameThreadExecutor; +import static javax.ws.rs.core.MediaType.APPLICATION_XML; +import static org.jclouds.util.Strings2.toStringAndClose; +import static org.testng.Assert.assertEquals; + +import java.io.IOException; +import java.util.Map; +import java.util.Properties; +import java.util.Set; + +import org.jclouds.Constants; +import org.jclouds.ContextBuilder; +import org.jclouds.concurrent.config.ExecutorServiceModule; +import org.jclouds.ec2.EC2Api; +import org.jclouds.ec2.EC2ApiMetadata; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; + +import com.google.common.base.Charsets; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.google.inject.Module; +import com.squareup.okhttp.mockwebserver.MockResponse; +import com.squareup.okhttp.mockwebserver.MockWebServer; +import com.squareup.okhttp.mockwebserver.RecordedRequest; + +/** + * Tests need to run {@code singleThreaded = true) as otherwise tests will clash on the regionToServers field. + * Sharing the regionToServers field means less code to write. + */ +public class BaseEC2ApiMockTest { + protected static final String DEFAULT_REGION = "us-east-1"; + + // Example keys from http://docs.aws.amazon.com/general/latest/gr/signature-version-2.html + private static final String ACCESS_KEY = "AKIAIOSFODNN7EXAMPLE"; + private static final String SECRET_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"; + + private Map regionToServers = Maps.newLinkedHashMap(); + + protected EC2Api api() { + return builder().buildApi(EC2Api.class); + } + + protected ContextBuilder builder() { + Properties overrides = new Properties(); + overrides.setProperty(Constants.PROPERTY_MAX_RETRIES, "1"); + return ContextBuilder.newBuilder(new EC2ApiMetadata()).credentials(ACCESS_KEY, SECRET_KEY) + .endpoint("http://localhost:" + regionToServers.get(DEFAULT_REGION).getPort()).overrides(overrides) + .modules(modules); + } + + private final Set modules = ImmutableSet.of(new ExecutorServiceModule(sameThreadExecutor())); + + @BeforeMethod + public void start() throws IOException { + MockWebServer server = new MockWebServer(); + server.play(); + regionToServers.put(DEFAULT_REGION, server); + } + + @AfterMethod(alwaysRun = true) + public void stop() throws IOException { + for (MockWebServer server : regionToServers.values()) { + server.shutdown(); + } + } + + protected void enqueue(String region, MockResponse response) { + regionToServers.get(region).enqueue(response); + } + + protected void enqueueRegions(String... regions) throws IOException { + StringBuilder describeRegionsResponse = new StringBuilder(); + describeRegionsResponse.append(""); + for (String region : regions) { + describeRegionsResponse.append(""); + describeRegionsResponse.append("").append(region).append(""); + if (!regionToServers.containsKey(region)) { + MockWebServer server = new MockWebServer(); + server.play(); + regionToServers.put(region, server); + } + String regionEndpoint = "http://localhost:" + regionToServers.get(region).getPort(); + describeRegionsResponse.append("").append(regionEndpoint).append(""); + describeRegionsResponse.append(""); + } + describeRegionsResponse.append(""); + enqueue(DEFAULT_REGION, + new MockResponse().addHeader(CONTENT_TYPE, APPLICATION_XML).setBody(describeRegionsResponse.toString())); + } + + protected void enqueueXml(String region, String resource) { + enqueue(region, + new MockResponse().addHeader(CONTENT_TYPE, APPLICATION_XML).setBody(stringFromResource(resource))); + } + + protected String stringFromResource(String resourceName) { + try { + return toStringAndClose(getClass().getResourceAsStream(resourceName)); + } catch (IOException e) { + throw propagate(e); + } + } + + /** Stripping out authorization, ensures the following post params were sent. */ + protected RecordedRequest assertPosted(String region, String postParams) throws InterruptedException { + RecordedRequest request = regionToServers.get(region).takeRequest(); + assertEquals(request.getMethod(), "POST"); + assertEquals(request.getPath(), "/"); + assertEquals(new String(request.getBody(), Charsets.UTF_8).replaceAll("&Signature.*", ""), postParams); + return request; + } +} diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionMockTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionMockTest.java new file mode 100644 index 0000000000..11702245a1 --- /dev/null +++ b/apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionMockTest.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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.ec2.suppliers; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; + +import java.util.Map; +import java.util.Set; + +import org.jclouds.ec2.internal.BaseEC2ApiMockTest; +import org.jclouds.rest.AuthorizationException; +import org.testng.annotations.Test; + +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableSet; +import com.squareup.okhttp.mockwebserver.MockResponse; + +@Test(groups = "unit", testName = "DescribeAvailabilityZonesInRegionMockTest", singleThreaded = true) +public class DescribeAvailabilityZonesInRegionMockTest extends BaseEC2ApiMockTest { + + public void onlySendsRequestsToConfiguredRegions() throws Exception { + enqueueRegions("us-east-1"); + enqueueXml("us-east-1", "/availabilityZones.xml"); + + Map>> result = new DescribeAvailabilityZonesInRegion(api(), + supplyRegionIds("us-east-1")).get(); + + assertEquals(result.size(), 1); + assertEquals(result.get("us-east-1").get(), + ImmutableSet.of("us-east-1a", "us-east-1b", "us-east-1c", "us-east-1d")); + + assertPosted("us-east-1", "Action=DescribeRegions"); + assertPosted("us-east-1", "Action=DescribeAvailabilityZones"); + } + + public void failsOnAuthorizationErrorToAnyRegion() throws Exception { + enqueueRegions("us-east-1", "eu-central-1"); + enqueueXml("us-east-1", "/availabilityZones.xml"); + enqueue("eu-central-1", new MockResponse().setResponseCode(401)); + + DescribeAvailabilityZonesInRegion supplier = new DescribeAvailabilityZonesInRegion(api(), + supplyRegionIds("us-east-1", "eu-central-1")); + + try { + supplier.get(); + fail(); + } catch (AuthorizationException e){ + + } + + assertPosted("us-east-1", "Action=DescribeRegions"); + assertPosted("us-east-1", "Action=DescribeAvailabilityZones"); + assertPosted("eu-central-1", "Action=DescribeAvailabilityZones"); + } + + private static Supplier> supplyRegionIds(String... regionIds) { + return Suppliers.>ofInstance(ImmutableSet.copyOf(regionIds)); + } +} diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionTest.java deleted file mode 100644 index 89148056d6..0000000000 --- a/apis/ec2/src/test/java/org/jclouds/ec2/suppliers/DescribeAvailabilityZonesInRegionTest.java +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF 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.ec2.suppliers; - -import static org.easymock.EasyMock.expect; -import static org.easymock.classextension.EasyMock.createControl; -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.fail; - -import java.util.Map; -import java.util.Set; - -import org.easymock.classextension.IMocksControl; -import org.jclouds.ec2.EC2Api; -import org.jclouds.ec2.domain.AvailabilityZoneInfo; -import org.jclouds.ec2.features.AvailabilityZoneAndRegionApi; -import org.jclouds.http.HttpCommand; -import org.jclouds.http.HttpResponseException; -import org.testng.annotations.Test; - -import com.google.common.base.Optional; -import com.google.common.base.Suppliers; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; - -/** - * A test for {@link DescribeAvailabilityZonesInRegion}. - * - * @author Eric Pabst (pabstec@familysearch.org) - */ -public class DescribeAvailabilityZonesInRegionTest { - @Test - public void testDescribeAvailabilityZonesInRegion_BestEffort() { - IMocksControl control = createControl(); - EC2Api client = control.createMock(EC2Api.class); - AvailabilityZoneAndRegionApi regionClient = control.createMock(AvailabilityZoneAndRegionApi.class); - AvailabilityZoneInfo info1 = control.createMock(AvailabilityZoneInfo.class); - AvailabilityZoneInfo info2 = control.createMock(AvailabilityZoneInfo.class); - HttpCommand command = control.createMock(HttpCommand.class); - HttpResponseException exception = new HttpResponseException("Error: Unable to tunnel through proxy: ...", - command, null); - - expect(client.getAvailabilityZoneAndRegionApi()).andStubReturn((Optional) Optional.of(regionClient)); - expect(regionClient.describeAvailabilityZonesInRegion("accessibleRegion1")).andReturn( - ImmutableSet.of(info1)); - expect(regionClient.describeAvailabilityZonesInRegion("inaccessibleRegion")).andThrow(exception); - expect(regionClient.describeAvailabilityZonesInRegion("accessibleRegion2")).andReturn( - ImmutableSet.of(info2)); - expect(info1.getZone()).andStubReturn("zone1"); - expect(info2.getZone()).andStubReturn("zone2"); - - Set regions = ImmutableSet.of("accessibleRegion1", "inaccessibleRegion", "accessibleRegion2"); - control.replay(); - - Map> expectedResult = ImmutableMap.> builder().put("accessibleRegion1", - ImmutableSet.of("zone1")).put("accessibleRegion2", ImmutableSet.of("zone2")).build(); - - DescribeAvailabilityZonesInRegion regionIdToZoneId = new DescribeAvailabilityZonesInRegion(client, Suppliers - .ofInstance(regions)); - assertEquals(Maps.transformValues(regionIdToZoneId.get(), Suppliers.> supplierFunction()), - expectedResult); - control.verify(); - } - - @Test - public void testDescribeAvailabilityZonesInRegion_RethrowIfNoneFound() { - IMocksControl control = createControl(); - EC2Api client = control.createMock(EC2Api.class); - AvailabilityZoneAndRegionApi regionClient = control.createMock(AvailabilityZoneAndRegionApi.class); - HttpCommand command = control.createMock(HttpCommand.class); - HttpResponseException exception = new HttpResponseException("Error: Unable to tunnel through proxy: ...", - command, null); - - expect(client.getAvailabilityZoneAndRegionApi()).andStubReturn((Optional) Optional.of(regionClient)); - expect(regionClient.describeAvailabilityZonesInRegion("inaccessibleRegion")).andThrow(exception); - - Set regions = ImmutableSet.of("inaccessibleRegion"); - control.replay(); - - DescribeAvailabilityZonesInRegion regionIdToZoneId = new DescribeAvailabilityZonesInRegion(client, Suppliers - .ofInstance(regions)); - try { - regionIdToZoneId.get(); - fail("expected exception"); - } catch (HttpResponseException e) { - assertEquals(e, exception); - } - control.verify(); - } - - @Test - public void testDescribeAvailabilityZonesInRegion_NoZones() { - IMocksControl control = createControl(); - EC2Api client = control.createMock(EC2Api.class); - AvailabilityZoneAndRegionApi regionClient = control.createMock(AvailabilityZoneAndRegionApi.class); - - expect(client.getAvailabilityZoneAndRegionApi()).andStubReturn((Optional) Optional.of(regionClient)); - expect(regionClient.describeAvailabilityZonesInRegion("emptyRegion")).andReturn( - ImmutableSet. of()); - - Set regions = ImmutableSet.of("emptyRegion"); - control.replay(); - - DescribeAvailabilityZonesInRegion regionIdToZoneId = new DescribeAvailabilityZonesInRegion(client, Suppliers - .ofInstance(regions)); - assertEquals(regionIdToZoneId.get(), ImmutableMap. of()); - control.verify(); - } -}