From 5f54110a47db423536807a63399e8492bb735db7 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 12 Jul 2011 00:16:02 -0700 Subject: [PATCH] Issue 623:clean up exception messages when templateBuilder fails to match an image --- .../internal/EC2TemplateBuilderImpl.java | 4 +- .../internal/EC2TemplateBuilderImplTest.java | 2 +- .../domain/internal/TemplateBuilderImpl.java | 78 +++++++++++++------ .../StubTemplateBuilderIntegrationTest.java | 1 + .../internal/TemplateBuilderImplTest.java | 44 +++++++++++ 5 files changed, 103 insertions(+), 26 deletions(-) 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 413a8fe19d..e77527f2c1 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 @@ -69,9 +69,9 @@ public class EC2TemplateBuilderImpl extends TemplateBuilderImpl { try { return imageMap.get(key); } catch (NullPointerException nex) { - throw new NoSuchElementException(String.format("image %s/%s not found", key.getRegion(), key.getName())); + throw new NoSuchElementException(String.format("imageId(%s/%s) not found", key.getRegion(), key.getName())); } catch (ComputationException nex) { - throw new NoSuchElementException(String.format("image %s/%s not found", key.getRegion(), key.getName())); + throw new NoSuchElementException(String.format("imageId(%s/%s) not found", key.getRegion(), key.getName())); } } return null; 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 8b05766f1d..a58441ed1e 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 @@ -57,7 +57,7 @@ import com.google.common.collect.Sets; * * @author Adrian Cole */ -@Test(groups = "unit", sequential = true) +@Test(groups = "unit", singleThreaded = true) public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest { @Override 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 4c9c44949a..40d23b0a51 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 @@ -21,11 +21,14 @@ package org.jclouds.compute.domain.internal; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.and; import static com.google.common.collect.Iterables.filter; +import static com.google.common.collect.Iterables.find; +import static com.google.common.collect.Iterables.size; +import static com.google.common.collect.Iterables.transform; import static com.google.common.collect.Lists.newArrayList; import static org.jclouds.compute.util.ComputeServiceUtils.getCores; import static org.jclouds.compute.util.ComputeServiceUtils.getCoresAndSpeed; import static org.jclouds.compute.util.ComputeServiceUtils.getSpace; - +import static java.lang.String.format; import java.util.List; import java.util.NoSuchElementException; import java.util.Set; @@ -50,6 +53,7 @@ import org.jclouds.logging.Logger; import org.jclouds.util.Lists2; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.base.Supplier; @@ -145,7 +149,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Override public String toString() { - return "location(" + location + ")"; + return location == null ? "anyLocation()" : "locationEqualsOrChildOf(" + location.getId() + ")"; } }; @@ -500,7 +504,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { public TemplateBuilder locationId(final String locationId) { Set locations = this.locations.get(); try { - this.location = Iterables.find(locations, new Predicate() { + this.location = find(locations, new Predicate() { @Override public boolean apply(Location input) { @@ -514,7 +518,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { }); } catch (NoSuchElementException e) { - throw new NoSuchElementException(String.format("location id %s not found in: %s", locationId, locations)); + throw new NoSuchElementException(format("location id %s not found in: %s", locationId, locations)); } return this; } @@ -528,6 +532,15 @@ public class TemplateBuilderImpl implements TemplateBuilder { return this; } + private static final Function imageToId = new Function() { + + @Override + public String apply(Image arg0) { + return arg0.getId(); + } + + }; + /** * {@inheritDoc} */ @@ -547,9 +560,14 @@ public class TemplateBuilderImpl implements TemplateBuilder { Set images = getImages(); Predicate imagePredicate = buildImagePredicate(); 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)); + if (size(supportedImages) == 0) { + if (imagePredicate == idPredicate) { + throw new NoSuchElementException(format("%s not found", idPredicate)); + } else { + throwNoSuchElementExceptionAfterLoggingImageIds(format("no image matched predicate: %s", imagePredicate), + images); + } + } Hardware hardware = resolveSize(hardwareSorter(), supportedImages); Image image = resolveImage(hardware, supportedImages); logger.debug("<< matched image(%s)", image); @@ -557,6 +575,13 @@ public class TemplateBuilderImpl implements TemplateBuilder { return new TemplateImpl(image, hardware, location, options); } + protected void throwNoSuchElementExceptionAfterLoggingImageIds(String message, Iterable images) { + NoSuchElementException exception = new NoSuchElementException(message); + if (logger.isTraceEnabled()) + logger.warn(exception, "image ids that didn't match: %s", transform(images, imageToId)); + throw exception; + } + protected Hardware resolveSize(Ordering hardwareOrdering, final Iterable images) { Set hardwarel = hardwares.get(); Hardware hardware; @@ -583,8 +608,11 @@ public class TemplateBuilderImpl implements TemplateBuilder { }); hardware = hardwareOrdering.max(filter(hardwaresThatAreCompatibleWithOurImages, hardwarePredicate)); } catch (NoSuchElementException exception) { - throw new NoSuchElementException("hardware don't support any images: " + toString() + "\n" + hardwarel - + "\n" + images); + String message = format("no hardware profiles support images matching params: %s", toString()); + exception = new NoSuchElementException(message); + if (logger.isTraceEnabled()) + logger.warn(exception, "hardware profiles %s\nimage ids %s", hardwarel, transform(images, imageToId)); + throw exception; } logger.debug("<< matched hardware(%s)", hardware); return hardware; @@ -622,13 +650,16 @@ public class TemplateBuilderImpl implements TemplateBuilder { try { Iterable matchingImages = filter(supportedImages, imagePredicate); if (logger.isTraceEnabled()) - logger.trace("<< matched images(%s)", matchingImages); + logger.trace("<< matched images(%s)", transform(matchingImages, imageToId)); List maxImages = Lists2.multiMax(DEFAULT_IMAGE_ORDERING, matchingImages); if (logger.isTraceEnabled()) - logger.trace("<< best images(%s)", maxImages); + logger.trace("<< best images(%s)", transform(maxImages, imageToId)); return maxImages.get(maxImages.size() - 1); } catch (NoSuchElementException exception) { - throw new NoSuchElementException("image didn't match: " + toString() + "\n" + supportedImages); + throwNoSuchElementExceptionAfterLoggingImageIds(format("no image matched params: %s", toString()), + supportedImages); + assert false; + return null; } } @@ -651,7 +682,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Override public String toString() { - return "location(" + location + ")"; + return locationPredicate.toString(); } }); @@ -668,19 +699,20 @@ public class TemplateBuilderImpl implements TemplateBuilder { osPredicates.add(os64BitPredicate); if (osArch != null) osPredicates.add(osArchPredicate); - predicates.add(new Predicate() { + if (osPredicates.size() > 0) + predicates.add(new Predicate() { - @Override - public boolean apply(Image input) { - return Predicates.and(osPredicates).apply(input.getOperatingSystem()); - } + @Override + public boolean apply(Image input) { + return and(osPredicates).apply(input.getOperatingSystem()); + } - @Override - public String toString() { - return Predicates.and(osPredicates).toString(); - } + @Override + public String toString() { + return and(osPredicates).toString(); + } - }); + }); if (imageVersion != null) predicates.add(imageVersionPredicate); if (imageName != null) diff --git a/compute/src/test/java/org/jclouds/compute/StubTemplateBuilderIntegrationTest.java b/compute/src/test/java/org/jclouds/compute/StubTemplateBuilderIntegrationTest.java index efd3a6f0ad..f57fb5ae1a 100644 --- a/compute/src/test/java/org/jclouds/compute/StubTemplateBuilderIntegrationTest.java +++ b/compute/src/test/java/org/jclouds/compute/StubTemplateBuilderIntegrationTest.java @@ -46,4 +46,5 @@ public class StubTemplateBuilderIntegrationTest extends BaseTemplateBuilderLiveT protected Set getIso3166Codes() { return ImmutableSet. of(); } + } \ 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 0eee4197f7..641a0d1c37 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 @@ -274,6 +274,10 @@ public class TemplateBuilderImplTest { template.imageId("notImageId").build(); assert false; } catch (NoSuchElementException e) { + // make sure big data is not in the exception message + assertEquals( + e.getMessage(), + "no hardware profiles support images matching params: [biggest=false, fastest=false, imageName=null, imageDescription=null, imageId=notImageId, imageVersion=null, location=EasyMock for interface org.jclouds.domain.Location, minCores=0.0, minRam=0, osFamily=null, osName=null, osDescription=null, osVersion=null, osArch=null, os64Bit=false, hardwareId=null]"); verify(image); verify(os); verify(defaultTemplate); @@ -523,7 +527,47 @@ public class TemplateBuilderImplTest { template.imageId("region/ami").build(); assert false; } catch (NoSuchElementException e) { + // make sure big data is not in the exception message + assertEquals(e.getMessage(), "imageId(region/ami) not found"); + } + verify(defaultOptions); + verify(defaultLocation); + verify(optionsProvider); + verify(templateBuilderProvider); + } + + @SuppressWarnings("unchecked") + @Test + public void testDefaultLocationWithUnmatchedPredicateExceptionMessage() { + Supplier> locations = Suppliers.> ofInstance(ImmutableSet + . of()); + Supplier> images = Suppliers.> ofInstance(ImmutableSet. of()); + Supplier> hardwares = Suppliers.> ofInstance(ImmutableSet + . of()); + Location defaultLocation = createMock(Location.class); + Provider optionsProvider = createMock(Provider.class); + Provider templateBuilderProvider = createMock(Provider.class); + TemplateOptions defaultOptions = createMock(TemplateOptions.class); + + expect(defaultLocation.getId()).andReturn("us-east-1"); + + expect(optionsProvider.get()).andReturn(defaultOptions); + + replay(defaultOptions); + replay(defaultLocation); + replay(optionsProvider); + replay(templateBuilderProvider); + + TemplateBuilderImpl template = createTemplateBuilder(null, locations, images, hardwares, defaultLocation, + optionsProvider, templateBuilderProvider); + + try { + template.imageDescriptionMatches("region/ami").build(); + assert false; + } catch (NoSuchElementException e) { + // make sure big data is not in the exception message + assertEquals(e.getMessage(), "no image matched predicate: And(locationEqualsOrChildOf(us-east-1),imageDescription(region/ami))"); } verify(defaultOptions);