From 12d359f4bd8af438569df9a8c0a970e883036275 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Thu, 28 Oct 2010 23:44:04 -0700 Subject: [PATCH] Issue 381: fixed incorrect widening of scope --- .../domain/internal/TemplateBuilderImpl.java | 95 ++++++++------- .../internal/TemplateBuilderImplTest.java | 71 ++++++++++- ...kVCloudExpressTemplateBuilderLiveTest.java | 113 ++++++++++++++++++ 3 files changed, 233 insertions(+), 46 deletions(-) create mode 100644 vcloud/terremark/src/test/java/org/jclouds/vcloud/terremark/compute/TerremarkVCloudExpressTemplateBuilderLiveTest.java 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 cd2deda64e..41e8a56da7 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 @@ -116,9 +116,9 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Inject protected TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> hardwares, - Supplier defaultLocation2, Provider optionsProvider, - @Named("DEFAULT") Provider defaultTemplateProvider) { + @Memoized Supplier> images, @Memoized Supplier> hardwares, + Supplier defaultLocation2, Provider optionsProvider, + @Named("DEFAULT") Provider defaultTemplateProvider) { this.locations = locations; this.images = images; this.hardwares = hardwares; @@ -140,7 +140,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { boolean returnVal = true; if (location != null && input.getLocation() != null) returnVal = location.equals(input.getLocation()) || location.getParent() != null - && location.getParent().equals(input.getLocation()); + && location.getParent().equals(input.getLocation()); return returnVal; } @@ -214,7 +214,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { returnVal = false; else returnVal = input.getDescription().contains(osDescription) - || input.getDescription().matches(osDescription); + || input.getDescription().matches(osDescription); } return returnVal; } @@ -328,8 +328,8 @@ public class TemplateBuilderImpl implements TemplateBuilder { returnVal = false; else returnVal = input.getDescription().equals(imageDescription) - || input.getDescription().contains(imageDescription) - || input.getDescription().matches(imageDescription); + || input.getDescription().contains(imageDescription) + || input.getDescription().matches(imageDescription); } return returnVal; } @@ -384,12 +384,12 @@ public class TemplateBuilderImpl implements TemplateBuilder { } }; private final Predicate hardwarePredicate = and(hardwareIdPredicate, locationPredicate, - hardwareCoresPredicate, hardwareRamPredicate); + hardwareCoresPredicate, hardwareRamPredicate); static final Ordering DEFAULT_SIZE_ORDERING = new Ordering() { public int compare(Hardware left, Hardware right) { return ComparisonChain.start().compare(getCores(left), getCores(right)).compare(left.getRam(), right.getRam()) - .compare(getSpace(left), getSpace(right)).result(); + .compare(getSpace(left), getSpace(right)).result(); } }; static final Ordering BY_CORES_ORDERING = new Ordering() { @@ -399,16 +399,16 @@ public class TemplateBuilderImpl implements TemplateBuilder { }; static final Ordering DEFAULT_IMAGE_ORDERING = new Ordering() { public int compare(Image left, Image right) { - return ComparisonChain.start() - .compare(left.getName(), right.getName(), Ordering. natural().nullsLast()) - .compare(left.getVersion(), right.getVersion(), Ordering. natural().nullsLast()) - .compare(left.getOperatingSystem().getName(), right.getOperatingSystem().getName(),// - Ordering. natural().nullsLast()) - .compare(left.getOperatingSystem().getVersion(), right.getOperatingSystem().getVersion(),// - Ordering. natural().nullsLast()) - .compare(left.getOperatingSystem().getDescription(), right.getOperatingSystem().getDescription(),// - Ordering. natural().nullsLast()) - .compare(left.getOperatingSystem().getArch(), right.getOperatingSystem().getArch()).result(); + return ComparisonChain.start().compare(left.getName(), right.getName(), + Ordering. natural().nullsLast()).compare(left.getVersion(), right.getVersion(), + Ordering. natural().nullsLast()).compare(left.getOperatingSystem().getName(), + right.getOperatingSystem().getName(),// + Ordering. natural().nullsLast()).compare(left.getOperatingSystem().getVersion(), + right.getOperatingSystem().getVersion(),// + Ordering. natural().nullsLast()).compare(left.getOperatingSystem().getDescription(), + right.getOperatingSystem().getDescription(),// + Ordering. natural().nullsLast()).compare(left.getOperatingSystem().getArch(), + right.getOperatingSystem().getArch()).result(); } }; @@ -427,19 +427,23 @@ public class TemplateBuilderImpl implements TemplateBuilder { */ @Override public TemplateBuilder fromHardware(Hardware hardware) { - if (hardware.getLocation() != null) + if (currentLocationWiderThan(hardware.getLocation())) this.location = hardware.getLocation(); this.minCores = getCores(hardware); this.minRam = hardware.getRam(); return this; } + private boolean currentLocationWiderThan(Location location) { + return this.location == null || (location != null && this.location.getScope().compareTo(location.getScope()) < 0); + } + /** * {@inheritDoc} */ @Override public TemplateBuilder fromImage(Image image) { - if (image.getLocation() != null) + if (currentLocationWiderThan(image.getLocation())) this.location = image.getLocation(); if (image.getOperatingSystem().getFamily() != null) this.osFamily = image.getOperatingSystem().getFamily(); @@ -539,7 +543,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { Iterable supportedImages = filter(images, buildImagePredicate()); if (Iterables.size(supportedImages) == 0) throw new NoSuchElementException(String.format( - "no image matched predicate %s images that didn't match below:\n%s", imagePredicate, images)); + "no image matched predicate %s images that didn't match below:\n%s", imagePredicate, images)); Hardware hardware = resolveSize(hardwareSorter(), supportedImages); Image image = resolveImage(hardware, supportedImages); logger.debug("<< matched image(%s)", image); @@ -552,29 +556,29 @@ public class TemplateBuilderImpl implements TemplateBuilder { Hardware hardware; try { Iterable hardwaresThatAreCompatibleWithOurImages = filter(hardwaresl, - new Predicate() { - @Override - public boolean apply(final Hardware hardware) { - return Iterables.any(images, new Predicate() { + new Predicate() { + @Override + public boolean apply(final Hardware hardware) { + return Iterables.any(images, new Predicate() { - @Override - public boolean apply(Image input) { - return hardware.supportsImage().apply(input); - } + @Override + public boolean apply(Image input) { + return hardware.supportsImage().apply(input); + } - @Override - public String toString() { - return "hardware(" + hardware + ").supportsImage()"; - } + @Override + public String toString() { + return "hardware(" + hardware + ").supportsImage()"; + } - }); + }); - } - }); + } + }); hardware = hardwareOrdering.max(filter(hardwaresThatAreCompatibleWithOurImages, hardwarePredicate)); } catch (NoSuchElementException exception) { throw new NoSuchElementException("hardwares don't support any images: " + toString() + "\n" + hardwaresl - + "\n" + images); + + "\n" + images); } logger.debug("<< matched hardware(%s)", hardware); return hardware; @@ -682,7 +686,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { // looks verbose, but explicit type needed for this to compile // properly Predicate imagePredicate = predicates.size() == 1 ? Iterables.> get(predicates, 0) - : Predicates. and(predicates); + : Predicates. and(predicates); return imagePredicate; } @@ -830,8 +834,9 @@ public class TemplateBuilderImpl implements TemplateBuilder { @VisibleForTesting boolean nothingChangedExceptOptions() { return osFamily == null && location == null && imageId == null && hardwareId == null && osName == null - && osDescription == null && imageVersion == null && osVersion == null && osArch == null && os64Bit == null - && imageName == null && imageDescription == null && minCores == 0 && minRam == 0 && !biggest && !fastest; + && osDescription == null && imageVersion == null && osVersion == null && osArch == null + && os64Bit == null && imageName == null && imageDescription == null && minCores == 0 && minRam == 0 + && !biggest && !fastest; } /** @@ -845,10 +850,10 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Override public String toString() { return "[biggest=" + biggest + ", fastest=" + fastest + ", imageName=" + imageName + ", imageDescription=" - + imageDescription + ", imageId=" + imageId + ", imageVersion=" + imageVersion + ", location=" + location - + ", minCores=" + minCores + ", minRam=" + minRam + ", osFamily=" + osFamily + ", osName=" + osName - + ", osDescription=" + osDescription + ", osVersion=" + osVersion + ", osArch=" + osArch + ", os64Bit=" - + os64Bit + ", hardwareId=" + hardwareId + "]"; + + imageDescription + ", imageId=" + imageId + ", imageVersion=" + imageVersion + ", location=" + + location + ", minCores=" + minCores + ", minRam=" + minRam + ", osFamily=" + osFamily + ", osName=" + + osName + ", osDescription=" + osDescription + ", osVersion=" + osVersion + ", osArch=" + osArch + + ", os64Bit=" + os64Bit + ", hardwareId=" + hardwareId + "]"; } @Override 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 f2253a1243..cab0f53cd0 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 @@ -39,6 +39,7 @@ import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.predicates.ImagePredicates; import org.jclouds.domain.Location; +import org.jclouds.domain.LocationScope; import org.testng.annotations.Test; import com.google.common.base.Supplier; @@ -54,7 +55,7 @@ public class TemplateBuilderImplTest { @SuppressWarnings("unchecked") @Test - public void tesResolveImages() { + public void testResolveImages() { Location defaultLocation = createMock(Location.class); Image image = createMock(Image.class); OperatingSystem os = createMock(OperatingSystem.class); @@ -202,6 +203,8 @@ public class TemplateBuilderImplTest { expect(os.getArch()).andReturn(null).atLeastOnce(); expect(os.is64Bit()).andReturn(false).atLeastOnce(); + expect(defaultLocation.getScope()).andReturn(LocationScope.PROVIDER).atLeastOnce(); + replay(image); replay(os); replay(defaultTemplate); @@ -257,6 +260,8 @@ public class TemplateBuilderImplTest { expect(os.getArch()).andReturn(null).atLeastOnce(); expect(os.is64Bit()).andReturn(false).atLeastOnce(); + expect(defaultLocation.getScope()).andReturn(LocationScope.PROVIDER).atLeastOnce(); + replay(image); replay(os); replay(defaultTemplate); @@ -359,6 +364,70 @@ public class TemplateBuilderImplTest { return template; } + @SuppressWarnings("unchecked") + @Test + public void testSuppliedImageLocationWiderThanDefault() { + TemplateOptions from = provideTemplateOptions(); + + Location defaultLocation = createMock(Location.class); + Image image = createMock(Image.class); + + Hardware hardware = new HardwareBuilder().id("hardwareId").supportsImage(ImagePredicates.idEquals("foo")).build(); + + Supplier> locations = Suppliers.> ofInstance(ImmutableSet + . of(defaultLocation)); + Supplier> images = Suppliers.> ofInstance(ImmutableSet + . of(image)); + Supplier> hardwares = Suppliers.> ofInstance(ImmutableSet + . of(hardware)); + Location imageLocation = createMock(Location.class); + OperatingSystem os = createMock(OperatingSystem.class); + + Provider optionsProvider = createMock(Provider.class); + Provider templateBuilderProvider = createMock(Provider.class); + TemplateOptions defaultOptions = createMock(TemplateOptions.class); + expect(optionsProvider.get()).andReturn(from).atLeastOnce(); + + expect(defaultLocation.getId()).andReturn("location").atLeastOnce(); + + expect(image.getId()).andReturn("foo").atLeastOnce(); + expect(image.getLocation()).andReturn(defaultLocation).atLeastOnce(); + expect(image.getOperatingSystem()).andReturn(os).atLeastOnce(); + expect(image.getName()).andReturn(null).atLeastOnce(); + expect(image.getDescription()).andReturn(null).atLeastOnce(); + expect(image.getVersion()).andReturn(null).atLeastOnce(); + + expect(os.getName()).andReturn(null).atLeastOnce(); + expect(os.getVersion()).andReturn(null).atLeastOnce(); + expect(os.getFamily()).andReturn(null).atLeastOnce(); + expect(os.getDescription()).andReturn(null).atLeastOnce(); + expect(os.getArch()).andReturn(null).atLeastOnce(); + expect(os.is64Bit()).andReturn(false).atLeastOnce(); + + expect(defaultLocation.getScope()).andReturn(LocationScope.HOST).atLeastOnce(); + + replay(defaultOptions); + replay(imageLocation); + replay(image); + replay(os); + replay(defaultLocation); + replay(optionsProvider); + replay(templateBuilderProvider); + + TemplateBuilderImpl template = createTemplateBuilder(null, locations, images, hardwares, defaultLocation, + optionsProvider, templateBuilderProvider); + + assertEquals(template.imageId("foo").locationId("location").build().getLocation(), defaultLocation); + + verify(defaultOptions); + verify(imageLocation); + verify(image); + verify(os); + verify(defaultLocation); + verify(optionsProvider); + verify(templateBuilderProvider); + } + @SuppressWarnings("unchecked") @Test public void testSuppliedLocationWithNoOptions() { diff --git a/vcloud/terremark/src/test/java/org/jclouds/vcloud/terremark/compute/TerremarkVCloudExpressTemplateBuilderLiveTest.java b/vcloud/terremark/src/test/java/org/jclouds/vcloud/terremark/compute/TerremarkVCloudExpressTemplateBuilderLiveTest.java new file mode 100644 index 0000000000..e138e298d2 --- /dev/null +++ b/vcloud/terremark/src/test/java/org/jclouds/vcloud/terremark/compute/TerremarkVCloudExpressTemplateBuilderLiveTest.java @@ -0,0 +1,113 @@ +/** + * + * Copyright (C) 2010 Cloud Conscious, LLC. + * + * ==================================================================== + * Licensed 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.vcloud.terremark.compute; + +import static com.google.common.base.Preconditions.checkNotNull; +import static org.jclouds.compute.util.ComputeServiceUtils.getCores; +import static org.testng.Assert.assertEquals; + +import java.io.IOException; +import java.util.Properties; + +import org.jclouds.Constants; +import org.jclouds.compute.ComputeServiceContext; +import org.jclouds.compute.ComputeServiceContextFactory; +import org.jclouds.compute.domain.OsFamily; +import org.jclouds.compute.domain.Template; +import org.jclouds.logging.log4j.config.Log4JLoggingModule; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableSet; +import com.google.inject.Module; + +/** + * + * @author Adrian Cole + */ +@Test(groups = "live", testName = "terremark.TerremarkVCloudExpressTemplateBuilderLiveTest") +public class TerremarkVCloudExpressTemplateBuilderLiveTest { + protected String provider = "trmk-vcloudexpress"; + protected String identity; + protected String credential; + protected String endpoint; + protected String apiversion; + + @BeforeClass + protected void setupCredentials() { + identity = checkNotNull(System.getProperty("test." + provider + ".identity"), "test." + provider + ".identity"); + credential = System.getProperty("test." + provider + ".credential"); + endpoint = System.getProperty("test." + provider + ".endpoint"); + apiversion = System.getProperty("test." + provider + ".apiversion"); + } + + protected Properties setupProperties() { + Properties overrides = new Properties(); + overrides.setProperty(Constants.PROPERTY_TRUST_ALL_CERTS, "true"); + overrides.setProperty(Constants.PROPERTY_RELAX_HOSTNAME, "true"); + overrides.setProperty(provider + ".identity", identity); + if (credential != null) + overrides.setProperty(provider + ".credential", credential); + if (endpoint != null) + overrides.setProperty(provider + ".endpoint", endpoint); + if (apiversion != null) + overrides.setProperty(provider + ".apiversion", apiversion); + return overrides; + } + + @Test + public void testTemplateBuilderCanUseImageId() { + ComputeServiceContext newContext = null; + try { + newContext = new ComputeServiceContextFactory().createContext(provider, ImmutableSet + . of(new Log4JLoggingModule()), setupProperties()); + + Template defaultTemplate = newContext.getComputeService().templateBuilder().build(); + + Template template = newContext.getComputeService().templateBuilder().imageId( + defaultTemplate.getImage().getId()).build(); + assertEquals(template, defaultTemplate); + + } finally { + if (newContext != null) + newContext.close(); + } + } + + @Test + public void testDefaultTemplateBuilder() throws IOException { + ComputeServiceContext newContext = null; + try { + newContext = new ComputeServiceContextFactory().createContext(provider, ImmutableSet + . of(new Log4JLoggingModule()), setupProperties()); + + Template defaultTemplate = newContext.getComputeService().templateBuilder().build(); + assertEquals(defaultTemplate.getImage().getOperatingSystem().getVersion(), null); + assertEquals(defaultTemplate.getImage().getOperatingSystem().is64Bit(), true); + assertEquals(defaultTemplate.getImage().getOperatingSystem().getFamily(), OsFamily.UBUNTU); + assertEquals(getCores(defaultTemplate.getHardware()), 1.0d); + + } finally { + if (newContext != null) + newContext.close(); + } + } + +} \ No newline at end of file