diff --git a/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java b/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java index b4566179b9..c609806819 100644 --- a/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java +++ b/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java @@ -24,11 +24,11 @@ import javax.inject.Provider; import org.jclouds.collect.Memoized; import org.jclouds.compute.domain.Hardware; -import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import com.google.common.base.Supplier; @@ -36,7 +36,7 @@ import com.google.common.base.Supplier; public class CloudSigmaTemplateBuilderImpl extends TemplateBuilderImpl { @Inject public CloudSigmaTemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> hardwares, + ImageCacheSupplier images, @Memoized Supplier> hardwares, Supplier defaultLocation2, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy) { super(locations, images, hardwares, defaultLocation2, optionsProvider, defaultTemplateProvider, getImageStrategy); 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 73188c9e4b..180bd62f1e 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 @@ -33,6 +33,7 @@ import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.util.Throwables2; @@ -49,7 +50,7 @@ public class EC2TemplateBuilderImpl extends TemplateBuilderImpl { @Inject protected EC2TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> sizes, + ImageCacheSupplier images, @Memoized Supplier> sizes, Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy, Supplier> imageMap) { 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 ca642417a2..0d2150052e 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 @@ -49,6 +49,7 @@ import org.jclouds.compute.domain.Template; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; @@ -225,7 +226,7 @@ public class EC2TemplateBuilderTest { m1_small().build(), m1_xlarge().build(), m2_xlarge().build(), m2_2xlarge().build(), m2_4xlarge().build(), g2_2xlarge().build(), CC1_4XLARGE)); - return new EC2TemplateBuilderImpl(locations, images, sizes, Suppliers.ofInstance(location), optionsProvider, + return new EC2TemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60), sizes, Suppliers.ofInstance(location), optionsProvider, templateBuilderProvider, getImageStrategy, imageCache) { }; } 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 b82d16507f..9c7de8db44 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 @@ -38,6 +38,7 @@ import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.domain.internal.TemplateBuilderImplTest; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; @@ -84,7 +85,7 @@ public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest { ImagesToRegionAndIdMap.imagesToMap(images.get())))); } - return new EC2TemplateBuilderImpl(locations, images, sizes, Suppliers.ofInstance(defaultLocation), + return new EC2TemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60), sizes, Suppliers.ofInstance(defaultLocation), optionsProvider, templateBuilderProvider, getImageStrategy, Suppliers.>ofInstance(imageMap)); } @@ -219,5 +220,11 @@ public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest { public void testFindImageWithIdDefaultToGetImageStrategy() { } - + + // The EC2 provider already overrides the getImage method so this test is not useful for EC2 + @Override + public void testFindImageWithIdDefaultToGetImageStrategyAndPopulatesTheCache() { + + } + } diff --git a/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java b/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java index bc20c610c5..8304466318 100644 --- a/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java +++ b/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java @@ -24,11 +24,11 @@ import javax.inject.Provider; import org.jclouds.collect.Memoized; import org.jclouds.compute.domain.Hardware; -import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import com.google.common.base.Supplier; @@ -37,7 +37,7 @@ public class VCloudTemplateBuilderImpl extends TemplateBuilderImpl { @Inject protected VCloudTemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> sizes, + ImageCacheSupplier images, @Memoized Supplier> sizes, Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy) { super(locations, images, sizes, defaultLocation, optionsProvider, defaultTemplateProvider, getImageStrategy); diff --git a/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java b/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java index 7cf1142ecd..d4498f6e35 100644 --- a/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java +++ b/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java @@ -53,6 +53,7 @@ import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.compute.strategy.CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap; import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.config.ValueOfConfigurationKeyOrNull; import org.jclouds.domain.LoginCredentials; import org.jclouds.json.Json; @@ -111,6 +112,9 @@ public abstract class BaseComputeServiceContextModule extends AbstractModule { }, InitializeRunScriptOnNodeOrPlaceInBadMap.class).build(InitializeRunScriptOnNodeOrPlaceInBadMap.Factory.class)); install(new FactoryModuleBuilder().build(BlockUntilInitScriptStatusIsZeroThenReturnOutput.Factory.class)); + + bind(new TypeLiteral>>() { + }).annotatedWith(Memoized.class).to(ImageCacheSupplier.class); } protected void bindCredentialsOverriderFunction() { @@ -229,7 +233,7 @@ public abstract class BaseComputeServiceContextModule extends AbstractModule { @Provides @Singleton - @Memoized + @Named("imageCache") protected Supplier> supplyImageCache(AtomicReference authException, @Named(PROPERTY_SESSION_INTERVAL) long seconds, final Supplier> imageSupplier, Injector injector) { if (shouldEagerlyParseImages(injector)) { diff --git a/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java b/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java index 9f937f3fe7..8287e41fb5 100644 --- a/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java +++ b/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java @@ -54,6 +54,7 @@ import org.jclouds.compute.domain.TemplateBuilderSpec; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.logging.Logger; @@ -77,7 +78,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; - protected final Supplier> images; + protected final ImageCacheSupplier images; protected final Supplier> hardwares; protected final Supplier> locations; protected final Supplier defaultLocation; @@ -130,13 +131,13 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Inject protected TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> hardwares, - Supplier defaultLocation2, @Named("DEFAULT") Provider optionsProvider, + ImageCacheSupplier images, @Memoized Supplier> hardwares, + Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy) { this.locations = checkNotNull(locations, "locations"); - this.images = checkNotNull(images, "locations"); + this.images = checkNotNull(images, "images"); this.hardwares = checkNotNull(hardwares, "hardwares"); - this.defaultLocation = checkNotNull(defaultLocation2, "defaultLocation2"); + this.defaultLocation = checkNotNull(defaultLocation, "defaultLocation"); this.optionsProvider = checkNotNull(optionsProvider, "optionsProvider"); this.defaultTemplateProvider = checkNotNull(defaultTemplateProvider, "defaultTemplateProvider"); this.getImageStrategy = checkNotNull(getImageStrategy, "getImageStrategy"); @@ -727,21 +728,26 @@ public class TemplateBuilderImpl implements TemplateBuilder { } private Image findImageWithId(Set images) { - // Try find the image in the cache and fallback to the GetImageStrategy + // Try to find the image in the cache and fallback to the GetImageStrategy // see https://issues.apache.org/jira/browse/JCLOUDS-570 Optional image = tryFind(images, idPredicate); - if (!image.isPresent()) { - logger.warn("Image %s not found in the image cache. Trying to get it directly...", imageId); - // Note that this might generate make a call to the provider instead of using a cache, but - // this will be executed rarely, only when an image is not present in the image list but - // it actually exists in the provider. It shouldn't be an expensive call so using a cache just for - // this corner case is overkill. - image = Optional.fromNullable(getImageStrategy.getImage(imageId)); - if (!image.isPresent()) { - throwNoSuchElementExceptionAfterLoggingImageIds(format("%s not found", idPredicate), images); - } + if (image.isPresent()) { + return image.get(); } - return image.get(); + + logger.info("Image %s not found in the image cache. Trying to get it from the provider...", imageId); + // Note that this will generate make a call to the provider instead of using a cache, but + // this will be executed rarely, only when an image is not present in the image list but + // it actually exists in the provider. It shouldn't be an expensive call so using a cache just for + // this corner case is overkill. + Image imageFromProvider = getImageStrategy.getImage(imageId); + if (imageFromProvider == null) { + throwNoSuchElementExceptionAfterLoggingImageIds(format("%s not found", idPredicate), images); + } + // Register the just found image in the image cache, so subsequent uses of the TemplateBuilder and + // the ComptueService find it. + this.images.registerImage(imageFromProvider); + return imageFromProvider; } private Hardware findHardwareWithId(Set hardwaresToSearch) { diff --git a/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java b/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java index ea493abbf2..db24331f04 100644 --- a/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java +++ b/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java @@ -34,6 +34,7 @@ import javax.inject.Singleton; import org.jclouds.compute.ComputeServiceAdapter; import org.jclouds.compute.ComputeServiceAdapter.NodeAndInitialCredentials; +import org.jclouds.compute.config.ComputeServiceAdapterContextModule.AddDefaultCredentialsToImage; import org.jclouds.compute.domain.ComputeMetadata; import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.NodeMetadata; @@ -74,12 +75,13 @@ public class AdaptingComputeServiceStrategies implements CreateNodeW private final ComputeServiceAdapter client; private final Function nodeMetadataAdapter; private final Function imageAdapter; + private final AddDefaultCredentialsToImage addDefaultCredentialsToImage; @Inject public AdaptingComputeServiceStrategies(Map credentialStore, PrioritizeCredentialsFromTemplate prioritizeCredentialsFromTemplate, ComputeServiceAdapter client, Function nodeMetadataAdapter, - Function imageAdapter) { + Function imageAdapter, AddDefaultCredentialsToImage addDefaultCredentialsToImage) { this.credentialStore = checkNotNull(credentialStore, "credentialStore"); this.prioritizeCredentialsFromTemplate = checkNotNull(prioritizeCredentialsFromTemplate, "prioritizeCredentialsFromTemplate"); @@ -87,6 +89,7 @@ public class AdaptingComputeServiceStrategies implements CreateNodeW this.nodeMetadataAdapter = Functions.compose(addLoginCredentials, checkNotNull(nodeMetadataAdapter, "nodeMetadataAdapter")); this.imageAdapter = checkNotNull(imageAdapter, "imageAdapter"); + this.addDefaultCredentialsToImage = checkNotNull(addDefaultCredentialsToImage, "addDefaultCredentialsToImage"); } private final Function addLoginCredentials = new Function() { @@ -124,7 +127,9 @@ public class AdaptingComputeServiceStrategies implements CreateNodeW I image = client.getImage(checkNotNull(id, "id")); if (image == null) return null; - return imageAdapter.apply(image); + // The image supplier configured in the ComputeServiceAdapterContextModule also adds the default credentials to + // each image in the image list. When getting a single image, the behavior must be the same. + return addDefaultCredentialsToImage.apply(imageAdapter.apply(image)); } @Override diff --git a/compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java b/compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java new file mode 100644 index 0000000000..400525bec0 --- /dev/null +++ b/compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java @@ -0,0 +1,84 @@ +/* + * 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.compute.suppliers; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.Iterables.concat; +import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL; + +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import javax.inject.Named; +import javax.inject.Singleton; + +import org.jclouds.compute.domain.Image; + +import com.google.common.base.Supplier; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.collect.ImmutableSet; +import com.google.inject.Inject; + +/** + * Image supplier that allows new images to be registered. + *

+ * This is a wrapper for the memoized image supplier (the actual image cache), to provide a way to register new images as + * needed. Once a new image is created by the {@link org.jclouds.compute.extensions.ImageExtension}, or discovered by + * other means (see https://issues.apache.org/jira/browse/JCLOUDS-570) this supplier will allow the image to be appended + * to the cached list, so it can be properly used normally. + */ +@Singleton +public class ImageCacheSupplier implements Supplier> { + + private final Supplier> imageCache; + + private final Cache uncachedImages; + + @Inject + public ImageCacheSupplier(@Named("imageCache") Supplier> imageCache, + @Named(PROPERTY_SESSION_INTERVAL) long sessionIntervalSeconds) { + this.imageCache = checkNotNull(imageCache, "imageCache"); + // We use a cache to let the entries in the "uncached" set expire as soon as the image cache expires. We want the + // uncached set to be regenerated when the original cache is also regenerated. + this.uncachedImages = CacheBuilder.newBuilder().expireAfterWrite(sessionIntervalSeconds, TimeUnit.SECONDS) + .build(); + } + + @Override + public Set get() { + return ImmutableSet.copyOf(concat(imageCache.get(), uncachedImages.asMap().values())); + } + + /** + * Registers a new image in the image cache. + *

+ * This method should be called to register new images into the image cache, when some image that is known to exist + * in the provider is still not cached. For example, this can happen when an image is created after the image cache + * has been populated for the first time. + *

+ * Note that this method does not check if the image is already cached, to avoid loading all images if the image + * cache is still not populated. + * + * @param image The image to be registered to the cache. + */ + public void registerImage(Image image) { + checkNotNull(image, "image"); + uncachedImages.put(image.getId(), image); + } + +} 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 40fb4fefed..3c476f6ff5 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 @@ -22,6 +22,7 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; @@ -45,6 +46,7 @@ import org.jclouds.compute.domain.Volume; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.predicates.ImagePredicates; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; @@ -466,7 +468,7 @@ public class TemplateBuilderImplTest { Supplier> images, Supplier> hardwares, Location defaultLocation, Provider optionsProvider, Provider templateBuilderProvider, GetImageStrategy getImageStrategy) { - TemplateBuilderImpl template = new TemplateBuilderImpl(locations, images, hardwares, Suppliers + TemplateBuilderImpl template = new TemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60), hardwares, Suppliers .ofInstance(defaultLocation), optionsProvider, templateBuilderProvider, getImageStrategy); return template; } @@ -824,6 +826,82 @@ public class TemplateBuilderImplTest { verify(getImageStrategy); } + @Test + public void testFindImageWithIdDefaultToGetImageStrategyAndPopulatesTheCache() { + final Supplier> locations = Suppliers.> ofInstance(ImmutableSet + . of(region)); + final Supplier> images = Suppliers.> ofInstance(ImmutableSet + . of( + new ImageBuilder() + .ids("Ubuntu 11.04 x64") + .name("Ubuntu 11.04 x64") + .description("Ubuntu 11.04 x64") + .location(region) + .status(Status.AVAILABLE) + .operatingSystem( + OperatingSystem.builder().name("Ubuntu 11.04 x64").description("Ubuntu 11.04 x64") + .is64Bit(true).version("11.04").family(OsFamily.UBUNTU).build()).build(), + new ImageBuilder() + .ids("Ubuntu 11.04 64-bit") + .name("Ubuntu 11.04 64-bit") + .description("Ubuntu 11.04 64-bit") + .location(region) + .status(Status.AVAILABLE) + .operatingSystem( + OperatingSystem.builder().name("Ubuntu 11.04 64-bit").description("Ubuntu 11.04 64-bit") + .is64Bit(true).version("11.04").family(OsFamily.UBUNTU).build()).build())); + + final Supplier> hardwares = Suppliers.> ofInstance(ImmutableSet + . of( + new HardwareBuilder() + .ids(String.format("datacenter(%s)platform(%s)cpuCores(%d)memorySizeMB(%d)diskSizeGB(%d)", + "Falkenberg", "Xen", 1, 512, 5)).ram(512) + .processors(ImmutableList.of(new Processor(1, 1.0))) + .volumes(ImmutableList. of(new VolumeImpl((float) 5, true, true))).hypervisor("Xen") + .location(region) + .supportsImage(ImagePredicates.idEquals(image.getId())).build())); + + final Provider optionsProvider = new Provider() { + @Override + public TemplateOptions get() { + return new TemplateOptions(); + } + }; + + final GetImageStrategy getImageStrategy = createMock(GetImageStrategy.class); + + expect(getImageStrategy.getImage(image.getId())).andReturn(image); + replay(getImageStrategy); + + Provider templateBuilderProvider = new Provider() { + @Override + public TemplateBuilder get() { + return createTemplateBuilder(null, locations, images, hardwares, region, optionsProvider, this, getImageStrategy); + } + }; + + TemplateBuilder templateBuilder = templateBuilderProvider.get(); + + try { + // First call searching for the image properties will fail, as the image is not in the cache + templateBuilder.osNameMatches(image.getOperatingSystem().getName()).build(); + fail("Image should not exist in the cache"); + } catch (Exception ex) { + // Expected path + } + + // A second call using the imageId will fallback to the GetImageStrategy and populate the image in the cache. + assertNotNull(templateBuilder.imageId(image.getId()).build()); + + // The third call will succeed, as the previous one should have populated the image in the cache. + templateBuilder.imageId(null); // Clear all criteria + Template template = templateBuilder.osNameMatches(image.getOperatingSystem().getName()).build(); + assertEquals(template.getImage().getId(), image.getId()); + + // Verify this is called only once, as the third call will already find the image in the cache + verify(getImageStrategy); + } + @SuppressWarnings("unchecked") @Test public void testHardwareIdNullsHypervisor() { diff --git a/compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java b/compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java new file mode 100644 index 0000000000..29a7caf3d0 --- /dev/null +++ b/compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java @@ -0,0 +1,77 @@ +/* + * 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.compute.suppliers; + +import static org.testng.Assert.assertEquals; + +import java.util.Set; + +import org.jclouds.compute.domain.Image; +import org.jclouds.compute.domain.ImageBuilder; +import org.jclouds.compute.domain.OperatingSystem; +import org.jclouds.domain.Location; +import org.jclouds.domain.LocationBuilder; +import org.jclouds.domain.LocationScope; +import org.testng.annotations.Test; + +import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableSet; + +/** + * Unit tests for the {@link ImageCacheSupplier} class. + */ +@Test(groups = "unit", testName = "ImageCacheSupplierTest") +public class ImageCacheSupplierTest { + + private Location location = new LocationBuilder().scope(LocationScope.PROVIDER).id("location") + .description("location").build(); + + private OperatingSystem os = OperatingSystem.builder().name("osName").version("osVersion") + .description("osDescription").arch("X86_32").build(); + + private Image image = new ImageBuilder().id("imageId").providerId("imageId").name("imageName") + .description("imageDescription").version("imageVersion").operatingSystem(os).status(Image.Status.AVAILABLE) + .location(location).build(); + + private Set images = ImmutableSet.of(image); + + @Test(expectedExceptions = NullPointerException.class) + public void testRegisterNullImageIsNotAllowed() { + ImageCacheSupplier imageCache = new ImageCacheSupplier(Suppliers.> ofInstance(images), 60); + imageCache.registerImage(null); + } + + @Test + public void testRegisterImageIgnoresDuplicates() { + ImageCacheSupplier imageCache = new ImageCacheSupplier(Suppliers.> ofInstance(images), 60); + assertEquals(imageCache.get().size(), 1); + + imageCache.registerImage(image); + + assertEquals(imageCache.get().size(), 1); + } + + @Test + public void testRegisterNewImage() { + ImageCacheSupplier imageCache = new ImageCacheSupplier(Suppliers.> ofInstance(images), 60); + assertEquals(imageCache.get().size(), 1); + + imageCache.registerImage(ImageBuilder.fromImage(image).id("newimage").build()); + + assertEquals(imageCache.get().size(), 2); + } +} diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java index ef2eac423e..160c37f9cf 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java @@ -28,6 +28,7 @@ import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.internal.EC2TemplateBuilderImpl; @@ -39,7 +40,7 @@ public class AWSEC2TemplateBuilderImpl extends EC2TemplateBuilderImpl { @Inject protected AWSEC2TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> sizes, + ImageCacheSupplier images, @Memoized Supplier> sizes, Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy, Supplier> imageMap) {