From 684210d8d80cf4e4804b6eedcf66d81c23f111d7 Mon Sep 17 00:00:00 2001
From: Adrian Cole <adrian@jclouds.org>
Date: Tue, 20 Mar 2012 09:34:26 -0700
Subject: [PATCH] cleaner impl of LocationPredicate, which doesn't eagerly
 fetch current location on toString()

---
 .../domain/internal/LocationPredicate.java    | 45 ++++++++++++-------
 .../internal/TemplateBuilderImplTest.java     |  5 +--
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/compute/src/main/java/org/jclouds/compute/domain/internal/LocationPredicate.java b/compute/src/main/java/org/jclouds/compute/domain/internal/LocationPredicate.java
index afb13aff78..4c9edee7d7 100644
--- a/compute/src/main/java/org/jclouds/compute/domain/internal/LocationPredicate.java
+++ b/compute/src/main/java/org/jclouds/compute/domain/internal/LocationPredicate.java
@@ -17,7 +17,10 @@
  * under the License.
  */
 package org.jclouds.compute.domain.internal;
-import static com.google.common.base.Preconditions.*;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.base.Predicates.equalTo;
+import static com.google.common.base.Predicates.or;
 
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.domain.Location;
@@ -25,6 +28,8 @@ import org.jclouds.domain.LocationScope;
 
 import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSet.Builder;
 
 /**
  * If the current location id is null, then we don't care where to launch a node.
@@ -42,33 +47,43 @@ public class LocationPredicate implements Predicate<ComputeMetadata> {
 
    @Override
    public boolean apply(ComputeMetadata input) {
-      Location location = locationSupplier.get();
-      if (location == null)
+      Location current = locationSupplier.get();
+      if (current == null)
          return true;
       
-      if (location.equals(input.getLocation()))
-         return true;
-      
-      checkArgument(
-            location.getParent() != null || location.getScope() == LocationScope.PROVIDER,
-            "only locations of scope PROVIDER can have a null parent; arg: %s",
-            location);
-      
       if (input.getLocation() == null)
          return true;
       
+      Location parent = current.getParent();
+      checkArgument(
+            parent != null || current.getScope() == LocationScope.PROVIDER,
+            "only locations of scope PROVIDER can have a null parent; arg: %s",
+            current);
+      
       checkState(
             input.getLocation().getParent() != null || input.getLocation().getScope() == LocationScope.PROVIDER,
             "only locations of scope PROVIDER can have a null parent; input: %s",
             input.getLocation());
+
+      Builder<Predicate<Location>> predicates = ImmutableSet.<Predicate<Location>>builder();
+
+      predicates.add(equalTo(current));
       
-      return location.equals(input.getLocation()) || location.getParent() != null
-               && location.getParent().equals(input.getLocation()) || location.getParent().getParent() != null
-               && location.getParent().getParent().equals(input.getLocation());
+      if (parent != null) {
+         predicates.add(equalTo(parent));
+         
+         Location grandparent = parent.getParent();
+         if (grandparent != null)
+            predicates.add(equalTo(grandparent));
+      }
+      
+      return or(predicates.build()).apply(input.getLocation());
+
    }
 
    @Override
    public String toString() {
-      return locationSupplier.get() == null ? "anyLocation()" : "locationEqualsParentOrGrandparentOf(" + locationSupplier.get().getId() + ")";
+      // not calling .get() here, as it could accidentally cause eager api fetch
+      return "equalsParentOrGrandparentOfCurrentLocation()";
    }
 }
\ No newline at end of file
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 8b71cf222c..3ad50a7b4f 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
@@ -627,7 +627,7 @@ public class TemplateBuilderImplTest {
 
    @SuppressWarnings("unchecked")
    @Test
-   public void testDefaultLocationWithUnmatchedPredicateExceptionMessage() {
+   public void testDefaultLocationWithUnmatchedPredicateExceptionMessageAndLocationNotCalled() {
       Supplier<Set<? extends Location>> locations = Suppliers.<Set<? extends Location>> ofInstance(ImmutableSet
                .<Location> of());
       Supplier<Set<? extends Image>> images = Suppliers.<Set<? extends Image>> ofInstance(ImmutableSet.<Image> of());
@@ -639,7 +639,6 @@ public class TemplateBuilderImplTest {
       TemplateOptions defaultOptions = createMock(TemplateOptions.class);
 
       expect(optionsProvider.get()).andReturn(defaultOptions);
-      expect(defaultLocation.getId()).andReturn("us-east-1");
 
       replay(defaultOptions);
       replay(defaultLocation);
@@ -654,7 +653,7 @@ public class TemplateBuilderImplTest {
          assert false;
       } catch (NoSuchElementException e) {
          // make sure big data is not in the exception message
-         assertEquals(e.getMessage(), "no image matched predicate: And(locationEqualsParentOrGrandparentOf(us-east-1),imageDescription(description))");
+         assertEquals(e.getMessage(), "no image matched predicate: And(equalsParentOrGrandparentOfCurrentLocation(),imageDescription(description))");
       }
 
       verify(defaultOptions);