From a45e16ebb35c3d77425e020eefb1d6b1fe17c55b Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Tue, 6 Dec 2011 23:41:19 +0000 Subject: [PATCH] Issue 763: code tidy, to use new ImagesToRegionAndIdMap.imagesToMap(Set) --- .../functions/ImagesToRegionAndIdMap.java | 33 +++++++++++++++++++ .../internal/EC2TemplateBuilderImpl.java | 6 ++++ .../compute/suppliers/EC2ImageSupplier.java | 13 ++------ .../ec2/compute/EC2TemplateBuilderTest.java | 9 ++--- .../RunningInstanceToNodeMetadataTest.java | 11 +------ .../internal/EC2TemplateBuilderImplTest.java | 20 +++-------- .../internal/TemplateBuilderImplTest.java | 13 ++++++-- .../suppliers/AWSEC2ImageSupplier.java | 14 ++------ .../AWSRunningInstanceToNodeMetadataTest.java | 12 ++----- 9 files changed, 63 insertions(+), 68 deletions(-) create mode 100644 apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/ImagesToRegionAndIdMap.java diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/ImagesToRegionAndIdMap.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/ImagesToRegionAndIdMap.java new file mode 100644 index 0000000000..0e4a4e6667 --- /dev/null +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/ImagesToRegionAndIdMap.java @@ -0,0 +1,33 @@ +package org.jclouds.ec2.compute.functions; + +import static com.google.common.collect.Maps.uniqueIndex; + +import java.util.Map; + +import org.jclouds.compute.domain.Image; +import org.jclouds.ec2.compute.domain.RegionAndName; + +import com.google.common.base.Function; +import com.google.inject.Singleton; + +@Singleton +public class ImagesToRegionAndIdMap implements Function, Map> { + + public static Map imagesToMap(Iterable input) { + return new ImagesToRegionAndIdMap().apply(input); + } + + @Override + public Map apply(Iterable input) { + return uniqueIndex(input, new Function() { + + @Override + public RegionAndName apply(Image from) { + return new RegionAndName(from.getLocation().getId(), from.getProviderId()); + } + + }); + } + + +} diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java index 149104c0c6..bf89da6181 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java @@ -36,6 +36,7 @@ import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; +import org.jclouds.util.Throwables2; import com.google.common.base.Supplier; import com.google.common.cache.Cache; @@ -73,6 +74,11 @@ public class EC2TemplateBuilderImpl extends TemplateBuilderImpl { } catch (ExecutionException e) { throw new NoSuchElementException(String.format("could not get imageId(%s/%s)", key.getRegion(), key.getName())); } catch (UncheckedExecutionException e) { + // Primarily for testing: if cache is backed by a map, can get IllegalArgumentException instead of NPE + IllegalArgumentException e2 = Throwables2.getFirstThrowableOfType(e, IllegalArgumentException.class); + if (e2 != null && e2.getMessage() != null && e2.getMessage().contains("not present in")) { + throw new NoSuchElementException(String.format("imageId(%s/%s) not found", key.getRegion(), key.getName())); + } throw new NoSuchElementException(String.format("could not get imageId(%s/%s)", key.getRegion(), key.getName())); } catch (NullPointerException nex) { throw new NoSuchElementException(String.format("imageId(%s/%s) not found", key.getRegion(), key.getName())); diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/suppliers/EC2ImageSupplier.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/suppliers/EC2ImageSupplier.java index 08be1975fb..2cd9a5d2de 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/suppliers/EC2ImageSupplier.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/suppliers/EC2ImageSupplier.java @@ -20,7 +20,6 @@ package org.jclouds.ec2.compute.suppliers; import static com.google.common.collect.Iterables.filter; import static com.google.common.collect.Iterables.transform; -import static com.google.common.collect.Maps.uniqueIndex; import static org.jclouds.ec2.options.DescribeImagesOptions.Builder.ownedBy; import static org.jclouds.ec2.reference.EC2Constants.PROPERTY_EC2_AMI_OWNERS; @@ -38,12 +37,12 @@ import org.jclouds.compute.domain.Image; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.functions.EC2ImageParser; +import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; import org.jclouds.ec2.compute.strategy.DescribeImagesParallel; import org.jclouds.ec2.options.DescribeImagesOptions; import org.jclouds.location.Region; import org.jclouds.logging.Logger; -import com.google.common.base.Function; import com.google.common.base.Predicates; import com.google.common.base.Supplier; import com.google.common.cache.Cache; @@ -95,15 +94,7 @@ public class EC2ImageSupplier implements Supplier> { Iterable parsedImages = ImmutableSet.copyOf(filter(transform(describer.apply(queries), parser), Predicates .notNull())); - ImmutableMap imageMap = uniqueIndex(parsedImages, new Function() { - - @Override - public RegionAndName apply(Image from) { - return new RegionAndName(from.getLocation().getId(), from.getProviderId()); - } - - }); - + Map imageMap = ImagesToRegionAndIdMap.imagesToMap(parsedImages); cache.get().invalidateAll(); cache.get().asMap().putAll((Map)imageMap); logger.debug("<< images(%d)", imageMap.size()); diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java index 666d1347d8..0c765f3f3d 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java @@ -18,7 +18,6 @@ */ package org.jclouds.ec2.compute; -import static com.google.common.collect.Maps.uniqueIndex; import static java.lang.String.format; import static org.easymock.EasyMock.expect; import static org.easymock.classextension.EasyMock.createMock; @@ -56,6 +55,7 @@ import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; import org.jclouds.domain.LoginCredentials; import org.jclouds.ec2.compute.domain.RegionAndName; +import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; import org.jclouds.ec2.compute.internal.EC2TemplateBuilderImpl; import org.testng.annotations.Test; @@ -196,12 +196,7 @@ public class EC2TemplateBuilderTest { // weird compilation error means have to cast this - see https://bugs.eclipse.org/bugs/show_bug.cgi?id=365818 @SuppressWarnings("unchecked") - ImmutableMap imageMap = (ImmutableMap) uniqueIndex(images.get(), new Function() { - @Override - public RegionAndName apply(Image from) { - return new RegionAndName(from.getLocation().getId(), from.getProviderId()); - } - }); + ImmutableMap imageMap = (ImmutableMap) ImagesToRegionAndIdMap.imagesToMap(images.get()); Supplier> imageCache = Suppliers.> ofInstance( CacheBuilder.newBuilder().build(CacheLoader.from(Functions.forMap(imageMap)))); diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java index cc8fd7bd67..127f716aca 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java @@ -45,7 +45,6 @@ import org.jclouds.ec2.xml.DescribeInstancesResponseHandlerTest; import org.jclouds.javax.annotation.Nullable; import org.testng.annotations.Test; -import com.google.common.base.Function; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import com.google.common.cache.Cache; @@ -54,7 +53,6 @@ import com.google.common.cache.CacheLoader; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Maps; /** * @author Adrian Cole @@ -234,14 +232,7 @@ public class RunningInstanceToNodeMetadataTest { @Override public Image load(@Nullable RegionAndName from) { - return Maps.uniqueIndex(images, new Function() { - - @Override - public RegionAndName apply(Image from) { - return new RegionAndName(from.getLocation().getId(), from.getProviderId()); - } - - }).get(from); + return ImagesToRegionAndIdMap.imagesToMap(images).get(from); } }; Cache instanceToImage = CacheBuilder.newBuilder().build(getRealImage); diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java index 3a848f5176..0b1593402f 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java @@ -18,7 +18,6 @@ */ package org.jclouds.ec2.compute.internal; -import static com.google.common.collect.Maps.uniqueIndex; import static org.easymock.EasyMock.expect; import static org.easymock.classextension.EasyMock.createMock; import static org.easymock.classextension.EasyMock.replay; @@ -43,10 +42,11 @@ import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; import org.jclouds.ec2.compute.domain.RegionAndName; +import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; import org.jclouds.ec2.compute.options.EC2TemplateOptions; import org.testng.annotations.Test; -import com.google.common.base.Function; +import com.google.common.base.Functions; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import com.google.common.cache.Cache; @@ -86,20 +86,8 @@ public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest { }); } else { - imageMap = CacheBuilder.newBuilder().build(new CacheLoader() { - @Override - public Image load(RegionAndName from) { - return uniqueIndex(images.get(), new Function() { - - @Override - public RegionAndName apply(Image from) { - return new RegionAndName(from.getLocation().getId(), from.getProviderId()); - } - - }).get(from); - } - - }); + imageMap = CacheBuilder.newBuilder().build(CacheLoader.from(Functions.forMap( + ImagesToRegionAndIdMap.imagesToMap(images.get())))); } return new EC2TemplateBuilderImpl(locations, images, sizes, Suppliers.ofInstance(defaultLocation), diff --git a/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java b/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java index 06032c3881..260e09d493 100644 --- a/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java +++ b/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java @@ -79,6 +79,10 @@ public class TemplateBuilderImplTest { expect(image2.getVersion()).andReturn("imageVersion"); expect(image.getOperatingSystem()).andReturn(os).atLeastOnce(); expect(image2.getOperatingSystem()).andReturn(os2).atLeastOnce(); + expect(image.getLocation()).andReturn(defaultLocation).anyTimes(); + expect(image2.getLocation()).andReturn(defaultLocation).anyTimes(); + expect(image.getProviderId()).andReturn("imageId").anyTimes(); + expect(image2.getProviderId()).andReturn("imageId2").anyTimes(); expect(os.getName()).andReturn("osName"); expect(os2.getName()).andReturn("osName"); expect(os.getVersion()).andReturn("osVersion"); @@ -87,7 +91,8 @@ public class TemplateBuilderImplTest { expect(os2.getDescription()).andReturn("osDescription"); expect(os.getArch()).andReturn("X86_64").atLeastOnce(); expect(os2.getArch()).andReturn("X86_64").atLeastOnce(); - + expect(defaultLocation.getId()).andReturn("location").anyTimes(); + replay(image); replay(image2); replay(os); @@ -135,11 +140,15 @@ public class TemplateBuilderImplTest { expect(optionsProvider.get()).andReturn(new TemplateOptions()); + expect(defaultLocation.getId()).andReturn("myregion").anyTimes(); + expect(image.getLocation()).andReturn(defaultLocation).atLeastOnce(); expect(image2.getLocation()).andReturn(defaultLocation).atLeastOnce(); expect(image.getOperatingSystem()).andReturn(os).atLeastOnce(); expect(image2.getOperatingSystem()).andReturn(os2).atLeastOnce(); - expect(image.getId()).andReturn("myregion/1"); + expect(image.getId()).andReturn("myregion/1").atLeastOnce(); + expect(image.getProviderId()).andReturn("1").anyTimes(); + expect(image2.getProviderId()).andReturn("2").anyTimes(); expect(os.getArch()).andReturn("X86_32").atLeastOnce(); expect(os2.getArch()).andReturn("X86_64").atLeastOnce(); diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/suppliers/AWSEC2ImageSupplier.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/suppliers/AWSEC2ImageSupplier.java index 9fa2531d86..f585583529 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/suppliers/AWSEC2ImageSupplier.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/suppliers/AWSEC2ImageSupplier.java @@ -20,7 +20,6 @@ package org.jclouds.aws.ec2.compute.suppliers; import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Iterables.transform; -import static com.google.common.collect.Maps.uniqueIndex; import static org.jclouds.aws.ec2.reference.AWSEC2Constants.PROPERTY_EC2_AMI_QUERY; import static org.jclouds.aws.ec2.reference.AWSEC2Constants.PROPERTY_EC2_CC_AMI_QUERY; import static org.jclouds.aws.ec2.reference.AWSEC2Constants.PROPERTY_EC2_CC_REGIONS; @@ -40,6 +39,7 @@ import org.jclouds.aws.ec2.compute.config.ClusterCompute; import org.jclouds.compute.domain.Image; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.ec2.compute.domain.RegionAndName; +import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; import org.jclouds.location.Region; import org.jclouds.logging.Logger; @@ -48,7 +48,6 @@ import com.google.common.base.Splitter; import com.google.common.base.Supplier; import com.google.common.base.Throwables; import com.google.common.cache.Cache; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -122,16 +121,7 @@ public class AWSEC2ImageSupplier implements Supplier> { throw Throwables.propagate(e); } - // TODO Need to clear out old entries; previously it was a new cache object every time - // (and enclosed within the cache provider so didn't risk someone getting the cache while it was empty)! - ImmutableMap imageMap = uniqueIndex(parsedImages, new Function() { - - @Override - public RegionAndName apply(Image from) { - return new RegionAndName(from.getLocation().getId(), from.getProviderId()); - } - - }); + final Map imageMap = ImagesToRegionAndIdMap.imagesToMap(parsedImages); cache.get().invalidateAll(); cache.get().asMap().putAll((Map)imageMap); logger.debug("<< images(%d)", imageMap.size()); diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java index 5cf6939768..b98faea803 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java @@ -36,6 +36,7 @@ import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; import org.jclouds.ec2.compute.config.EC2ComputeServiceDependenciesModule; import org.jclouds.ec2.compute.domain.RegionAndName; +import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; import org.jclouds.ec2.domain.Attachment; import org.jclouds.ec2.domain.BlockDevice; import org.jclouds.ec2.domain.InstanceState; @@ -43,7 +44,6 @@ import org.jclouds.ec2.domain.RootDeviceType; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; -import com.google.common.base.Function; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import com.google.common.cache.Cache; @@ -52,7 +52,6 @@ import com.google.common.cache.CacheLoader; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Maps; import com.google.inject.Guice; /** @@ -159,15 +158,8 @@ public class AWSRunningInstanceToNodeMetadataTest { Map credentialStore) { Map instanceToNodeState = EC2ComputeServiceDependenciesModule.instanceToNodeState; - final ImmutableMap backing = Maps.uniqueIndex(images, new Function() { + final Map backing = ImagesToRegionAndIdMap.imagesToMap(images); - @Override - public RegionAndName apply(Image from) { - return new RegionAndName(from.getLocation().getId(), from.getProviderId()); - } - - }); - Cache instanceToImage = CacheBuilder.newBuilder().build(new CacheLoader (){ @Override