Issue 763: code tidy, to use new ImagesToRegionAndIdMap.imagesToMap(Set<Image>)

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<Iterable<? extends Image>, Map<RegionAndName, ? extends Image>> {
+
+   public static Map<RegionAndName, ? extends Image> imagesToMap(Iterable<? extends Image> input) {
+      return new ImagesToRegionAndIdMap().apply(input);
+   }
+   
+   @Override
+   public Map<RegionAndName, ? extends Image> apply(Iterable<? extends Image> input) {
+      return uniqueIndex(input, new Function<Image, RegionAndName>() {
+         
+         @Override
+         public RegionAndName apply(Image from) {
+            return new RegionAndName(from.getLocation().getId(), from.getProviderId());
+         }
+         
+      });
+   }
+
+
+}

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()));

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<Set<? extends Image>> {
          Iterable<? extends Image> parsedImages = ImmutableSet.copyOf(filter(transform(describer.apply(queries), parser), Predicates
                   .notNull()));
 
-         ImmutableMap<RegionAndName, ? extends Image> imageMap = uniqueIndex(parsedImages, new Function<Image, RegionAndName>() {
+         Map<RegionAndName, ? extends Image> imageMap = ImagesToRegionAndIdMap.imagesToMap(parsedImages);
-
-            @Override
-            public RegionAndName apply(Image from) {
-               return new RegionAndName(from.getLocation().getId(), from.getProviderId());
-            }
-
-         });
-         
          cache.get().invalidateAll();
          cache.get().asMap().putAll((Map)imageMap);
          logger.debug("<< images(%d)", imageMap.size());

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<RegionAndName, Image> imageMap = (ImmutableMap<RegionAndName, Image>) uniqueIndex(images.get(), new Function<Image, RegionAndName>() {
+      ImmutableMap<RegionAndName, Image> imageMap = (ImmutableMap<RegionAndName, Image>) ImagesToRegionAndIdMap.imagesToMap(images.get());
-         @Override
-         public RegionAndName apply(Image from) {
-            return new RegionAndName(from.getLocation().getId(), from.getProviderId());
-         }
-      });
       Supplier<Cache<RegionAndName, ? extends Image>> imageCache = Suppliers.<Cache<RegionAndName, ? extends Image>> ofInstance(
                CacheBuilder.newBuilder().<RegionAndName,Image>build(CacheLoader.from(Functions.forMap(imageMap))));
 

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<Image, RegionAndName>() {
+            return ImagesToRegionAndIdMap.imagesToMap(images).get(from);
-
-               @Override
-               public RegionAndName apply(Image from) {
-                  return new RegionAndName(from.getLocation().getId(), from.getProviderId());
-               }
-
-            }).get(from);
          }
       };
       Cache<RegionAndName, Image> instanceToImage = CacheBuilder.newBuilder().build(getRealImage);

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<RegionAndName, Image>() {
+         imageMap = CacheBuilder.newBuilder().build(CacheLoader.from(Functions.forMap(
-            @Override
+                  ImagesToRegionAndIdMap.imagesToMap(images.get()))));
-            public Image load(RegionAndName from) {
-               return uniqueIndex(images.get(), new Function<Image, RegionAndName>() {
-                  
-                  @Override
-                  public RegionAndName apply(Image from) {
-                     return new RegionAndName(from.getLocation().getId(), from.getProviderId());
-                  }
-                  
-               }).get(from);
-            }
-   
-         });
       }
 
       return new EC2TemplateBuilderImpl(locations, images, sizes, Suppliers.ofInstance(defaultLocation),

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();

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<Set<? extends Image>> {
          throw Throwables.propagate(e);
       }
 
-      // TODO Need to clear out old entries; previously it was a new cache object every time 
+      final Map<RegionAndName, ? extends Image> imageMap = ImagesToRegionAndIdMap.imagesToMap(parsedImages);
-      // (and enclosed within the cache provider so didn't risk someone getting the cache while it was empty)!
-      ImmutableMap<RegionAndName, ? extends Image> imageMap = uniqueIndex(parsedImages, new Function<Image, RegionAndName>() {
-
-         @Override
-         public RegionAndName apply(Image from) {
-            return new RegionAndName(from.getLocation().getId(), from.getProviderId());
-         }
-
-      });
       cache.get().invalidateAll();
       cache.get().asMap().putAll((Map)imageMap);
       logger.debug("<< images(%d)", imageMap.size());

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<String, Credentials> credentialStore) {
       Map<InstanceState, NodeState> instanceToNodeState = EC2ComputeServiceDependenciesModule.instanceToNodeState;
 
-      final ImmutableMap<RegionAndName, Image> backing = Maps.uniqueIndex(images, new Function<Image, RegionAndName>() {
+      final Map<RegionAndName, ? extends Image> backing = ImagesToRegionAndIdMap.imagesToMap(images);
 
-         @Override
-         public RegionAndName apply(Image from) {
-            return new RegionAndName(from.getLocation().getId(), from.getProviderId());
-         }
-
-      });
-      
       Cache<RegionAndName, Image> instanceToImage = CacheBuilder.newBuilder().build(new CacheLoader<RegionAndName, Image> (){
     
          @Override