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 03fadb8172..066204bb10 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 @@ -16,8 +16,12 @@ */ package org.jclouds.aws.ec2.compute.suppliers; +import static com.google.common.base.Predicates.in; +import static com.google.common.base.Throwables.propagate; import static com.google.common.collect.Iterables.concat; +import static com.google.common.collect.Iterables.filter; import static com.google.common.collect.Iterables.transform; +import static org.jclouds.Constants.PROPERTY_USER_THREADS; 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; @@ -28,9 +32,7 @@ import java.util.Set; import javax.annotation.Resource; import javax.inject.Inject; import javax.inject.Named; -import javax.inject.Singleton; -import org.jclouds.Constants; import org.jclouds.aws.ec2.compute.config.ClusterCompute; import org.jclouds.aws.ec2.compute.config.ImageQuery; import org.jclouds.compute.domain.Image; @@ -43,7 +45,6 @@ import org.jclouds.logging.Logger; import com.google.common.base.Function; import com.google.common.base.Splitter; import com.google.common.base.Supplier; -import com.google.common.base.Throwables; import com.google.common.cache.LoadingCache; import com.google.common.collect.ForwardingSet; import com.google.common.collect.ImmutableMultimap; @@ -54,18 +55,11 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; -/** - * - * @author Adrian Cole - */ -@Singleton -public class AWSEC2ImageSupplier implements Supplier> { - - // TODO could/should this sub-class EC2ImageSupplier? Or does that confuse guice? - +public final class AWSEC2ImageSupplier implements Supplier> { + @Resource @Named(ComputeServiceConstants.COMPUTE_LOGGER) - protected Logger logger = Logger.NULL; + private Logger logger = Logger.NULL; private final Set clusterComputeIds; private final CallForImages.Factory factory; @@ -76,12 +70,11 @@ public class AWSEC2ImageSupplier implements Supplier> { private final Iterable clusterRegions; private final Supplier> cache; - @Inject - protected AWSEC2ImageSupplier(@Region Supplier> regions, - @ImageQuery Map queries, @Named(PROPERTY_EC2_CC_REGIONS) String clusterRegions, - Supplier> cache, - CallForImages.Factory factory, @ClusterCompute Set clusterComputeIds, - @Named(Constants.PROPERTY_USER_THREADS) ListeningExecutorService userExecutor) { + @Inject AWSEC2ImageSupplier(@Region Supplier> regions, @ImageQuery Map queries, + @Named(PROPERTY_EC2_CC_REGIONS) String clusterRegions, + Supplier> cache, CallForImages.Factory factory, + @ClusterCompute Set clusterComputeIds, + @Named(PROPERTY_USER_THREADS) ListeningExecutorService userExecutor) { this.factory = factory; this.regions = regions; this.queries = queries; @@ -97,13 +90,16 @@ public class AWSEC2ImageSupplier implements Supplier> { String amiQuery = queries.get(PROPERTY_EC2_AMI_QUERY); String ccAmiQuery = queries.get(PROPERTY_EC2_CC_AMI_QUERY); - ListenableFuture> normalImages = images(regions.get(), amiQuery, PROPERTY_EC2_AMI_QUERY); + Set regionIds = regions.get(); + + ListenableFuture> normalImages = images(regionIds, amiQuery, PROPERTY_EC2_AMI_QUERY); ImmutableSet clusterImages; try { - clusterImages = ImmutableSet.copyOf(images(clusterRegions, ccAmiQuery, PROPERTY_EC2_CC_AMI_QUERY).get()); + clusterImages = ImmutableSet + .copyOf(images(filter(clusterRegions, in(regionIds)), ccAmiQuery, PROPERTY_EC2_CC_AMI_QUERY).get()); } catch (Exception e) { logger.warn(e, "Error parsing images in query %s", ccAmiQuery); - throw Throwables.propagate(e); + throw propagate(e); } Iterables.addAll(clusterComputeIds, transform(clusterImages, new Function() { @@ -113,20 +109,21 @@ public class AWSEC2ImageSupplier implements Supplier> { } })); + Iterable parsedImages; try { parsedImages = ImmutableSet.copyOf(concat(clusterImages, normalImages.get())); } catch (Exception e) { logger.warn(e, "Error parsing images in query %s", amiQuery); - throw Throwables.propagate(e); + throw propagate(e); } - final Map imageMap = ImagesToRegionAndIdMap.imagesToMap(parsedImages); + Map imageMap = ImagesToRegionAndIdMap.imagesToMap(parsedImages); cache.get().invalidateAll(); - cache.get().asMap().putAll(Map.class.cast(imageMap)); + cache.get().putAll(Map.class.cast(imageMap)); logger.debug("<< images(%d)", imageMap.size()); - - // TODO Used to be mutable; was this assumed anywhere? + + // Forwarding so that later changes to the underlying cache are visible. return new ForwardingSet() { protected Set delegate() { return ImmutableSet.copyOf(cache.get().asMap().values());