Issue 381: fixed incorrect widening of scope

compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java

@@ -399,16 +399,16 @@ public class TemplateBuilderImpl implements TemplateBuilder {
    };
    static final Ordering<Image> DEFAULT_IMAGE_ORDERING = new Ordering<Image>() {
       public int compare(Image left, Image right) {
-         return ComparisonChain.start()
+         return ComparisonChain.start().compare(left.getName(), right.getName(),
-               .compare(left.getName(), right.getName(), Ordering.<String> natural().nullsLast())
+                  Ordering.<String> natural().nullsLast()).compare(left.getVersion(), right.getVersion(),
-               .compare(left.getVersion(), right.getVersion(), Ordering.<String> natural().nullsLast())
+                  Ordering.<String> natural().nullsLast()).compare(left.getOperatingSystem().getName(),
-               .compare(left.getOperatingSystem().getName(), right.getOperatingSystem().getName(),//
+                  right.getOperatingSystem().getName(),//
-                     Ordering.<String> natural().nullsLast())
+                  Ordering.<String> natural().nullsLast()).compare(left.getOperatingSystem().getVersion(),
-               .compare(left.getOperatingSystem().getVersion(), right.getOperatingSystem().getVersion(),//
+                  right.getOperatingSystem().getVersion(),//
-                     Ordering.<String> natural().nullsLast())
+                  Ordering.<String> natural().nullsLast()).compare(left.getOperatingSystem().getDescription(),
-               .compare(left.getOperatingSystem().getDescription(), right.getOperatingSystem().getDescription(),//
+                  right.getOperatingSystem().getDescription(),//
-                     Ordering.<String> natural().nullsLast())
+                  Ordering.<String> natural().nullsLast()).compare(left.getOperatingSystem().getArch(),
-               .compare(left.getOperatingSystem().getArch(), right.getOperatingSystem().getArch()).result();
+                  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();
@@ -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
+               && osDescription == null && imageVersion == null && osVersion == null && osArch == null
-            && imageName == null && imageDescription == null && minCores == 0 && minRam == 0 && !biggest && !fastest;
+               && 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
+               + imageDescription + ", imageId=" + imageId + ", imageVersion=" + imageVersion + ", location="
-            + ", minCores=" + minCores + ", minRam=" + minRam + ", osFamily=" + osFamily + ", osName=" + osName
+               + location + ", minCores=" + minCores + ", minRam=" + minRam + ", osFamily=" + osFamily + ", osName="
-            + ", osDescription=" + osDescription + ", osVersion=" + osVersion + ", osArch=" + osArch + ", os64Bit="
+               + osName + ", osDescription=" + osDescription + ", osVersion=" + osVersion + ", osArch=" + osArch
-            + os64Bit + ", hardwareId=" + hardwareId + "]";
+               + ", os64Bit=" + os64Bit + ", hardwareId=" + hardwareId + "]";
    }
 
    @Override

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<Set<? extends Location>> locations = Suppliers.<Set<? extends Location>> ofInstance(ImmutableSet
+               .<Location> of(defaultLocation));
+      Supplier<Set<? extends Image>> images = Suppliers.<Set<? extends Image>> ofInstance(ImmutableSet
+               .<Image> of(image));
+      Supplier<Set<? extends Hardware>> hardwares = Suppliers.<Set<? extends Hardware>> ofInstance(ImmutableSet
+               .<Hardware> of(hardware));
+      Location imageLocation = createMock(Location.class);
+      OperatingSystem os = createMock(OperatingSystem.class);
+
+      Provider<TemplateOptions> optionsProvider = createMock(Provider.class);
+      Provider<TemplateBuilder> 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() {

vcloud/terremark/src/test/java/org/jclouds/vcloud/terremark/compute/TerremarkVCloudExpressTemplateBuilderLiveTest.java

@@ -0,0 +1,113 @@
+/**
+ *
+ * Copyright (C) 2010 Cloud Conscious, LLC. <info@cloudconscious.com>
+ *
+ * ====================================================================
+ * 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
+                  .<Module> 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
+                  .<Module> 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();
+      }
+   }
+
+}