You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2016/02/17 01:31:40 UTC
[3/7] brooklyn-server git commit: better error messages,
especially in ui when submitting bad plans
better error messages, especially in ui when submitting bad plans
longer-term we'd like to include the location in the file, but for now at least we show rather more readable errors for common problems
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/c2fe5e91
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/c2fe5e91
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/c2fe5e91
Branch: refs/heads/master
Commit: c2fe5e916fa82cf0b89943ad8cb31c0d5bf8dcdc
Parents: 495b1d3
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Tue Feb 9 13:49:14 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Tue Feb 16 17:53:03 2016 +0000
----------------------------------------------------------------------
.../brooklyn/camp/spi/pdp/DeploymentPlan.java | 10 +-
.../brooklyn/camp/spi/resolve/PdpProcessor.java | 3 +-
.../spi/creation/BrooklynEntityMatcher.java | 3 +-
.../creation/BrooklynYamlLocationResolver.java | 25 +++--
.../catalog/internal/BasicBrooklynCatalog.java | 11 +-
.../core/typereg/BasicBrooklynTypeRegistry.java | 6 +-
.../brooklyn/util/core/ResourceUtils.java | 2 +-
.../brooklyn/util/core/flags/TypeCoercions.java | 15 ++-
.../apache/brooklyn/rest/domain/ApiError.java | 14 +--
.../rest/resources/ApplicationResource.java | 103 +++++++++++--------
.../rest/resources/CatalogResource.java | 14 +--
.../rest/util/DefaultExceptionMapper.java | 11 +-
.../brooklyn/rest/util/WebResourceUtils.java | 46 +++++++--
.../brooklyn/util/exceptions/Exceptions.java | 87 ++++++++++++----
.../exceptions/PropagatedRuntimeException.java | 11 +-
.../exceptions/RuntimeInterruptedException.java | 8 +-
.../brooklyn/util/javalang/JavaClassNames.java | 24 ++++-
.../java/org/apache/brooklyn/util/net/Urls.java | 32 +++++-
.../util/exceptions/ExceptionsTest.java | 42 ++++++++
.../util/javalang/JavaClassNamesTest.java | 17 ++-
20 files changed, 367 insertions(+), 117 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/pdp/DeploymentPlan.java
----------------------------------------------------------------------
diff --git a/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/pdp/DeploymentPlan.java b/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/pdp/DeploymentPlan.java
index 7ad3164..f7dbaf2 100644
--- a/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/pdp/DeploymentPlan.java
+++ b/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/pdp/DeploymentPlan.java
@@ -24,7 +24,9 @@ import java.util.Map;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.javalang.JavaClassNames;
import org.apache.commons.lang3.builder.ToStringBuilder;
public class DeploymentPlan {
@@ -60,12 +62,12 @@ public class DeploymentPlan {
if (service instanceof Map) {
result.services.add(Service.of((Map<String,Object>) service));
} else {
- throw new IllegalArgumentException("service should be map, not "+service.getClass());
+ throw new UserFacingException("Services list should have a map for each entry, not "+JavaClassNames.superSimpleClassName(service));
}
}
} else if (services!=null) {
// TODO "map" short form
- throw new IllegalArgumentException("artifacts body should be iterable, not "+services.getClass());
+ throw new UserFacingException("Services block should be a list, not "+JavaClassNames.superSimpleClassName(services));
}
result.artifacts = new ArrayList<Artifact>();
@@ -75,12 +77,12 @@ public class DeploymentPlan {
if (artifact instanceof Map) {
result.artifacts.add(Artifact.of((Map<String,Object>) artifact));
} else {
- throw new IllegalArgumentException("artifact should be map, not "+artifact.getClass());
+ throw new UserFacingException("Artifacts list should contain map items, not "+JavaClassNames.superSimpleClassName(artifact));
}
}
} else if (artifacts!=null) {
// TODO "map" short form
- throw new IllegalArgumentException("artifacts body should be iterable, not "+artifacts.getClass());
+ throw new UserFacingException("Artifacts block should contain a list, not "+JavaClassNames.superSimpleClassName(artifacts));
}
result.customAttributes = attrs;
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/resolve/PdpProcessor.java
----------------------------------------------------------------------
diff --git a/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/resolve/PdpProcessor.java b/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/resolve/PdpProcessor.java
index ae42ee7..e1e69cf 100644
--- a/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/resolve/PdpProcessor.java
+++ b/camp/camp-base/src/main/java/org/apache/brooklyn/camp/spi/resolve/PdpProcessor.java
@@ -35,6 +35,7 @@ import org.apache.brooklyn.camp.spi.resolve.interpret.PlanInterpretationContext;
import org.apache.brooklyn.camp.spi.resolve.interpret.PlanInterpretationNode;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.stream.Streams;
import org.apache.brooklyn.util.yaml.Yamls;
import org.apache.commons.compress.archivers.ArchiveEntry;
@@ -163,7 +164,7 @@ public class PdpProcessor {
return;
}
}
- throw new IllegalArgumentException("Deployment plan item cannot be matched. Please check your YAML. Item: "+deploymentPlanItem);
+ throw new UserFacingException("Unable to match plan item: "+deploymentPlanItem);
}
// ----------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java
index 91a490a..b0356c3 100644
--- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java
+++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java
@@ -32,6 +32,7 @@ import org.apache.brooklyn.camp.spi.resolve.PdpMatcher;
import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -62,7 +63,7 @@ public class BrooklynEntityMatcher implements PdpMatcher {
Service service = (Service)deploymentPlanItem;
String serviceType = service.getServiceType();
- if (serviceType==null) throw new NullPointerException("Service must declare a type ("+service+")");
+ if (serviceType==null) throw new UserFacingException("Service must declare a type: "+service);
BrooklynClassLoadingContext loader = BasicBrooklynCatalog.BrooklynLoaderTracker.getLoader();
if (loader == null) loader = JavaBrooklynClassLoadingContext.create(mgmt);
if (BrooklynComponentTemplateResolver.Factory.newInstance(loader, serviceType).canResolve())
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlLocationResolver.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlLocationResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlLocationResolver.java
index 2f8dd99..7f2e056 100644
--- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlLocationResolver.java
+++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlLocationResolver.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.camp.brooklyn.spi.creation;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
+import java.util.NoSuchElementException;
import org.apache.brooklyn.api.location.Location;
import org.apache.brooklyn.api.location.LocationDefinition;
@@ -28,6 +29,7 @@ import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.guava.Maybe.Absent;
import org.apache.brooklyn.util.text.Strings;
@@ -60,22 +62,22 @@ public class BrooklynYamlLocationResolver {
} else if (location instanceof Map) {
locationFromString = resolveLocationFromMap((Map<?,?>)location);
} else {
- throw new IllegalStateException("Illegal parameter for 'location'; must be a string or map (but got "+location+")");
+ throw new UserFacingException("Illegal parameter for 'location'; must be a string or map (but got "+location+")");
}
}
if (locations!=null) {
if (!(locations instanceof Iterable))
- throw new IllegalStateException("Illegal parameter for 'locations'; must be an iterable (but got "+locations+")");
+ throw new UserFacingException("Illegal parameter for 'locations'; must be an iterable (but got "+locations+")");
locationsFromList = resolveLocations( (Iterable<Object>)locations );
}
if (locationFromString!=null && locationsFromList!=null) {
if (locationsFromList.size() != 1)
- throw new IllegalStateException("Conflicting 'location' and 'locations' ("+location+" and "+locations+"); "
+ throw new UserFacingException("Conflicting 'location' and 'locations' ("+location+" and "+locations+"); "
+ "if both are supplied the list must have exactly one element being the same");
if (!locationFromString.equals( Iterables.getOnlyElement(locationsFromList) ))
- throw new IllegalStateException("Conflicting 'location' and 'locations' ("+location+" and "+locations+"); "
+ throw new UserFacingException("Conflicting 'location' and 'locations' ("+location+" and "+locations+"); "
+ "different location specified in each");
} else if (locationFromString!=null) {
locationsFromList = Arrays.asList(locationFromString);
@@ -100,7 +102,7 @@ public class BrooklynYamlLocationResolver {
return resolveLocationFromMap((Map<?,?>)location);
}
// could support e.g. location definition
- throw new IllegalStateException("Illegal parameter for 'location' ("+location+"); must be a string or map");
+ throw new UserFacingException("Illegal parameter for 'location' ("+location+"); must be a string or map");
}
/** resolves the location from the given spec string, either "Named Location", or "named:Named Location" format;
@@ -112,16 +114,16 @@ public class BrooklynYamlLocationResolver {
public Location resolveLocationFromMap(Map<?,?> location) {
if (location.size() > 1) {
- throw new IllegalStateException("Illegal parameter for 'location'; expected a single entry in map ("+location+")");
+ throw new UserFacingException("Illegal parameter for 'location'; expected a single entry in map ("+location+")");
}
Object key = Iterables.getOnlyElement(location.keySet());
Object value = location.get(key);
if (!(key instanceof String)) {
- throw new IllegalStateException("Illegal parameter for 'location'; expected String key ("+location+")");
+ throw new UserFacingException("Illegal parameter for 'location'; expected String key ("+location+")");
}
if (!(value instanceof Map)) {
- throw new IllegalStateException("Illegal parameter for 'location'; expected config map ("+location+")");
+ throw new UserFacingException("Illegal parameter for 'location'; expected config map ("+location+")");
}
return resolveLocation((String)key, (Map<?,?>)value);
}
@@ -136,7 +138,12 @@ public class BrooklynYamlLocationResolver {
if (l.isPresent()) return l.get();
RuntimeException exception = ((Absent<?>)l).getException();
- throw new IllegalStateException("Illegal parameter for 'location' ("+spec+"); not resolvable: "+
+ if (exception instanceof NoSuchElementException &&
+ exception.getMessage().contains("Unknown location")) {
+ // common case
+ throw new UserFacingException(exception.getMessage(), exception.getCause());
+ }
+ throw new UserFacingException("Illegal parameter for 'location' ("+spec+"); not resolvable: "+
Exceptions.collapseText( exception ), exception);
}
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
index 8306168..34b905b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
@@ -50,8 +50,10 @@ import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.AggregateClassLoader;
+import org.apache.brooklyn.util.javalang.JavaClassNames;
import org.apache.brooklyn.util.javalang.LoadedClassLoader;
import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
@@ -459,7 +461,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
if (items!=null) {
int count = 0;
- for (Map<?,?> i: ((List<Map<?,?>>)items)) {
+ for (Object ii: checkType(items, "items", List.class)) {
+ Map<?,?> i = checkType(ii, "entry in items list", Map.class);
collectCatalogItems(Yamls.getTextOfYamlAtPath(sourceYaml, "items", count).getMatchedYamlTextOrWarn(),
i, result, catalogMetadata);
count++;
@@ -601,6 +604,12 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
result.add(dto);
}
+ @SuppressWarnings("unchecked")
+ private <T> T checkType(Object x, String description, Class<T> type) {
+ if (type.isInstance(x)) return (T)x;
+ throw new UserFacingException("Expected "+JavaClassNames.superSimpleClassName(type)+" for "+description+", not "+JavaClassNames.superSimpleClassName(x));
+ }
+
private String setFromItemIfUnset(String oldValue, Map<?,?> item, String fieldAttr) {
if (Strings.isNonBlank(oldValue)) return oldValue;
if (item!=null) {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
index 5d4bbf6..e28c7b2 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
@@ -228,9 +228,11 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry {
// put the legacy exception first if none of the new transformers support the type
// (until the new transformer is the primary pathway)
exceptionsInOrder.add(e);
+ exceptionsInOrder.add(e0);
+ } else {
+ exceptionsInOrder.add(e0);
+ exceptionsInOrder.add(e);
}
- exceptionsInOrder.add(e0);
- exceptionsInOrder.add(e);
throw Exceptions.create("Unable to instantiate "+(symbolicName==null ? "item" : symbolicName), exceptionsInOrder);
}
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
index aac7f7b..e7eb5ac 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
@@ -293,7 +293,7 @@ public class ResourceUtils {
throw new IOException("'"+orig+"' not found on classpath or filesystem");
} catch (Exception e) {
if (context!=null) {
- throw new RuntimeException("Error getting resource '"+url+"' for "+context+": "+e, e);
+ throw Exceptions.propagate("Error getting resource '"+url+"' for "+context, e);
} else {
throw Exceptions.propagate(e);
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
index 872e713..4029a71 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
@@ -31,6 +31,7 @@ import java.math.BigInteger;
import java.net.InetAddress;
import java.net.URI;
import java.net.URL;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.List;
@@ -59,11 +60,12 @@ import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.Enums;
+import org.apache.brooklyn.util.javalang.JavaClassNames;
import org.apache.brooklyn.util.net.Cidr;
import org.apache.brooklyn.util.net.Networking;
import org.apache.brooklyn.util.net.UserAndHostAndPort;
-import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
import org.apache.brooklyn.util.time.Time;
import org.apache.brooklyn.util.yaml.Yamls;
@@ -269,6 +271,17 @@ public class TypeCoercions {
}
//not found
+ if (targetType.isEnum()) {
+ try {
+ throw new ClassCoercionException("Invalid value '"+value+"' for "+JavaClassNames.simpleClassName(targetType)+"; expected one of "+
+ Arrays.asList((Object[])targetType.getMethod("values").invoke(null)));
+ } catch (ClassCoercionException e) {
+ throw e;
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ // fall back to below
+ }
+ }
throw new ClassCoercionException("Cannot coerce type "+value.getClass()+" to "+targetType.getCanonicalName()+" ("+value+"): no adapter known");
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java
----------------------------------------------------------------------
diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java
index 89ee744..a13f619 100644
--- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java
+++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java
@@ -27,11 +27,11 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
+import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.text.Strings;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import com.google.common.base.Optional;
import com.google.common.base.Throwables;
public class ApiError implements Serializable {
@@ -63,8 +63,7 @@ public class ApiError implements Serializable {
*/
public static Builder builderFromThrowable(Throwable t) {
checkNotNull(t, "throwable");
- String message = Optional.fromNullable(t.getMessage())
- .or(t.getClass().getName());
+ String message = Exceptions.collapseText(t);
return builder()
.message(message)
.details(Throwables.getStackTraceAsString(t));
@@ -100,11 +99,14 @@ public class ApiError implements Serializable {
}
/** puts a prefix in front of the message, with the given separator if there is already a message;
- * if there is no message, it simply sets the prefix as the message
+ * if there is no message, it simply sets the prefix as the message.
+ * if no prefix or blank, does nothing.
*/
public Builder prefixMessage(String prefix, String separatorIfMessageNotBlank) {
- if (Strings.isBlank(message)) message(prefix);
- else message(prefix+separatorIfMessageNotBlank+message);
+ if (Strings.isNonBlank(prefix)) {
+ if (Strings.isBlank(message)) message(prefix);
+ else message(prefix+separatorIfMessageNotBlank+message);
+ }
return this;
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
index 1dbf1a8..a74ad66 100644
--- a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
+++ b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
@@ -18,11 +18,24 @@
*/
package org.apache.brooklyn.rest.resources;
-import com.google.common.base.Throwables;
-import com.google.common.collect.FluentIterable;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static javax.ws.rs.core.Response.created;
+import static javax.ws.rs.core.Response.status;
+import static javax.ws.rs.core.Response.Status.ACCEPTED;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.ResponseBuilder;
+import javax.ws.rs.core.UriInfo;
+
import org.apache.brooklyn.api.entity.Application;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntitySpec;
@@ -66,26 +79,17 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.JavaClassNames;
+import org.apache.brooklyn.util.net.Urls;
import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import javax.ws.rs.core.Context;
-import javax.ws.rs.core.Response;
-import javax.ws.rs.core.Response.ResponseBuilder;
-import javax.ws.rs.core.UriInfo;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.ws.rs.core.Response.Status.ACCEPTED;
-import static javax.ws.rs.core.Response.created;
-import static javax.ws.rs.core.Response.status;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Throwables;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
@HaHotStateRequired
public class ApplicationResource extends AbstractBrooklynRestResource implements ApplicationApi {
@@ -235,27 +239,49 @@ public class ApplicationResource extends AbstractBrooklynRestResource implements
@Override
public Response createFromYaml(String yaml) {
// First of all, see if it's a URL
- URI uri;
+ Preconditions.checkNotNull(yaml, "Blueprint cannot be null");
+ URI uri = null;
try {
- uri = new URI(yaml);
+ String yamlUrl = yaml.trim();
+ if (Urls.isUrlWithProtocol(yamlUrl)) {
+ uri = new URI(yamlUrl);
+ }
} catch (URISyntaxException e) {
// It's not a URI then...
uri = null;
}
if (uri != null) {
log.debug("Create app called with URI; retrieving contents: {}", uri);
- yaml = ResourceUtils.create(mgmt()).getResourceAsString(uri.toString());
+ try {
+ yaml = ResourceUtils.create(mgmt()).getResourceAsString(uri.toString());
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ throw new UserFacingException("Cannot resolve URL: "+uri, e);
+ }
}
log.debug("Creating app from yaml:\n{}", yaml);
- EntitySpec<? extends Application> spec = createEntitySpecForApplication(yaml);
+ EntitySpec<? extends Application> spec;
+ try {
+ spec = createEntitySpecForApplication(yaml);
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ Throwable root = Throwables.getRootCause(e);
+ if (root instanceof UserFacingException) throw (UserFacingException) root;
+ throw WebResourceUtils.badRequest(e, "Error in blueprint");
+ }
+
if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.DEPLOY_APPLICATION, spec)) {
throw WebResourceUtils.forbidden("User '%s' is not authorized to start application %s",
Entitlements.getEntitlementContext().user(), yaml);
}
- return launch(yaml, spec);
+ try {
+ return launch(yaml, spec);
+ } catch (Exception e) {
+ throw WebResourceUtils.badRequest(e, "Error launching blueprint");
+ }
}
private Response launch(String yaml, EntitySpec<? extends Application> spec) {
@@ -308,14 +334,20 @@ public class ApplicationResource extends AbstractBrooklynRestResource implements
//TODO infer encoding from request
String potentialYaml = new String(inputToAutodetectType);
- EntitySpec<? extends Application> spec = createEntitySpecForApplication(potentialYaml);
+ EntitySpec<? extends Application> spec;
+ try {
+ spec = createEntitySpecForApplication(potentialYaml);
+ } catch (Exception e) {
+ // TODO if not yaml/json - try ZIP, etc
+
+ throw WebResourceUtils.badRequest(e, "Error in blueprint");
+ }
- // TODO not json - try ZIP, etc
if (spec != null) {
return launch(potentialYaml, spec);
} else if (looksLikeLegacy) {
- throw Throwables.propagate(legacyFormatException);
+ throw Exceptions.propagate(legacyFormatException);
} else {
return Response.serverError().entity("Unsupported format; not able to autodetect.").build();
}
@@ -341,18 +373,7 @@ public class ApplicationResource extends AbstractBrooklynRestResource implements
}
private EntitySpec<? extends Application> createEntitySpecForApplication(String potentialYaml) {
- try {
- return EntityManagementUtils.createEntitySpecForApplication(mgmt(), potentialYaml);
- } catch (Exception e) {
- // An IllegalArgumentException for creating the entity spec gets wrapped in a ISE, and possibly a Compound.
- // But we want to return a 400 rather than 500, so ensure we throw IAE.
- IllegalArgumentException iae = (IllegalArgumentException) Exceptions.getFirstThrowableOfType(e, IllegalArgumentException.class);
- if (iae != null) {
- throw new IllegalArgumentException("Cannot create spec for app: "+iae.getMessage(), e);
- } else {
- throw Exceptions.propagate(e);
- }
- }
+ return EntityManagementUtils.createEntitySpecForApplication(mgmt(), potentialYaml);
}
private void checkApplicationTypesAreValid(ApplicationSpec applicationSpec) {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
index a26f1a1..7494fc4 100644
--- a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
+++ b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
@@ -112,11 +112,9 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat
Iterable<? extends CatalogItem<?, ?>> items;
try {
items = brooklyn().getCatalog().addItems(yaml);
- } catch (IllegalArgumentException e) {
- return Response.status(Status.BAD_REQUEST)
- .type(MediaType.APPLICATION_JSON)
- .entity(ApiError.of(e))
- .build();
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ return ApiError.of(e).asBadRequestResponseJson();
}
log.info("REST created catalog items: "+items);
@@ -128,7 +126,11 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat
result.put(item.getId(), CatalogTransformer.catalogItemSummary(brooklyn(), item));
} catch (Throwable t) {
log.warn("Error loading catalog item '"+item+"' (rethrowing): "+t);
- throw Exceptions.propagate(t);
+ // unfortunately items are already added to the catalog and hard to remove,
+ // but at least let the user know;
+ // happens eg if a class refers to a missing class, like
+ // loading nosql items including mongo without the mongo bson class on the classpath
+ throw Exceptions.propagateAnnotated("At least one unusable item was added ("+item.getId()+")", t);
}
}
return Response.status(Status.CREATED).entity(result).build();
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
index 978755b..d11149c 100644
--- a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
+++ b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
@@ -39,6 +39,8 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.text.Strings;
+import com.google.common.base.Throwables;
+
@Provider
public class DefaultExceptionMapper implements ExceptionMapper<Throwable> {
@@ -55,7 +57,6 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable> {
*/
@Override
public Response toResponse(Throwable throwable1) {
-
LOG.debug("REST request running as {} threw: {}", Entitlements.getEntitlementContext(),
Exceptions.collapse(throwable1));
if (LOG.isTraceEnabled()) {
@@ -92,13 +93,17 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable> {
LOG.warn("REST call generated exception type "+throwable2.getClass()+" unrecognized in "+getClass()+" (subsequent occurrences will be logged debug only): " + throwable2, throwable2);
}
}
+
+ // Before saying server error, look for a user-facing exception anywhere in the hierarchy
+ Throwable root = Throwables.getRootCause(throwable1);
+ if (root instanceof UserFacingException) {
+ return ApiError.of(root.getMessage()).asBadRequestResponseJson();
+ }
Throwable throwable3 = Exceptions.collapse(throwable2);
Builder rb = ApiError.builderFromThrowable(throwable3);
if (Strings.isBlank(rb.getMessage()))
rb.message("Internal error. Contact server administrator to consult logs for more details.");
- if (Exceptions.isPrefixImportant(throwable3))
- rb.message(Exceptions.getPrefixText(throwable3)+": "+rb.getMessage());
return rb.build().asResponse(Status.INTERNAL_SERVER_ERROR, MediaType.APPLICATION_JSON_TYPE);
}
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java
index da72c6f..6939108 100644
--- a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java
+++ b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java
@@ -27,14 +27,15 @@ import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
import org.apache.brooklyn.rest.domain.ApiError;
import org.apache.brooklyn.rest.util.json.BrooklynJacksonJsonProvider;
import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.PropagatedRuntimeException;
import org.apache.brooklyn.util.net.Urls;
import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableMap;
@@ -46,14 +47,37 @@ public class WebResourceUtils {
/** @throws WebApplicationException with an ApiError as its body and the given status as its response code. */
public static WebApplicationException throwWebApplicationException(Response.Status status, String format, Object... args) {
- String msg = String.format(format, args);
+ return throwWebApplicationException(status, null, format, args);
+ }
+
+ /** @throws WebApplicationException with an ApiError as its body and the given status as its response code. */
+ public static WebApplicationException throwWebApplicationException(Response.Status status, Throwable exception) {
+ return throwWebApplicationException(status, exception, null);
+ }
+
+ /** @throws WebApplicationException with an ApiError as its body and the given status as its response code.
+ * Exception and/or format can be null, and will be filled in / prefixed as appropriate. */
+ public static WebApplicationException throwWebApplicationException(Response.Status status, Throwable exception, String format, Object... args) {
+ String suppliedMsg = format==null ? null : String.format(format, args);
+ String fullMsg = suppliedMsg;
+ if (exception!=null) {
+ if (fullMsg==null) fullMsg = Exceptions.collapseText(exception);
+ else fullMsg = suppliedMsg + ": "+Exceptions.collapseText(exception);
+ }
if (log.isDebugEnabled()) {
log.debug("responding {} {} ({})",
- new Object[]{status.getStatusCode(), status.getReasonPhrase(), msg});
+ new Object[]{status.getStatusCode(), status.getReasonPhrase(), fullMsg});
}
- ApiError apiError = ApiError.builder().message(msg).errorCode(status).build();
+ ApiError apiError =
+ (exception != null ? ApiError.builderFromThrowable(exception).prefixMessage(suppliedMsg)
+ : ApiError.builder().message(fullMsg==null ? "" : fullMsg))
+ .errorCode(status).build();
// including a Throwable is the only way to include a message with the WebApplicationException - ugly!
- throw new WebApplicationException(new Throwable(apiError.toString()), apiError.asJsonResponse());
+ throw new WebApplicationException(
+ exception==null ? new Throwable(apiError.toString()) :
+ suppliedMsg==null ? exception :
+ new PropagatedRuntimeException(suppliedMsg, exception),
+ apiError.asJsonResponse());
}
/** @throws WebApplicationException With code 500 internal server error */
@@ -66,6 +90,16 @@ public class WebResourceUtils {
return throwWebApplicationException(Response.Status.BAD_REQUEST, format, args);
}
+ /** @throws WebApplicationException With code 400 bad request */
+ public static WebApplicationException badRequest(Throwable t) {
+ return throwWebApplicationException(Response.Status.BAD_REQUEST, t);
+ }
+
+ /** @throws WebApplicationException With code 400 bad request */
+ public static WebApplicationException badRequest(Throwable t, String prefix, Object... prefixArgs) {
+ return throwWebApplicationException(Response.Status.BAD_REQUEST, t, prefix, prefixArgs);
+ }
+
/** @throws WebApplicationException With code 401 unauthorized */
public static WebApplicationException unauthorized(String format, Object... args) {
return throwWebApplicationException(Response.Status.UNAUTHORIZED, format, args);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
index 314961a..ef6fd47 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java
@@ -36,6 +36,7 @@ import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.javalang.JavaClassNames;
import org.apache.brooklyn.util.text.Strings;
+import com.google.common.annotations.Beta;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.base.Throwables;
@@ -46,12 +47,19 @@ import com.google.common.collect.Lists;
public class Exceptions {
- private static final List<Class<? extends Throwable>> BORING_THROWABLE_SUPERTYPES = ImmutableList.<Class<? extends Throwable>>of(
- ExecutionException.class, InvocationTargetException.class, PropagatedRuntimeException.class, UndeclaredThrowableException.class);
+ private static final List<Class<? extends Throwable>> ALWAYS_BORING_THROWABLE_SUPERTYPES = ImmutableList.<Class<? extends Throwable>>of(
+ ExecutionException.class, InvocationTargetException.class, UndeclaredThrowableException.class);
+
+ private static final List<Class<? extends Throwable>> BORING_IF_NO_MESSAGE_THROWABLE_SUPERTYPES = ImmutableList.<Class<? extends Throwable>>of(
+ PropagatedRuntimeException.class);
private static boolean isBoring(Throwable t) {
- for (Class<? extends Throwable> type: BORING_THROWABLE_SUPERTYPES)
+ for (Class<? extends Throwable> type: ALWAYS_BORING_THROWABLE_SUPERTYPES)
if (type.isInstance(t)) return true;
+ if (Strings.isBlank(t.getMessage())) {
+ for (Class<? extends Throwable> type: BORING_IF_NO_MESSAGE_THROWABLE_SUPERTYPES)
+ if (type.isInstance(t)) return true;
+ }
return false;
}
@@ -63,7 +71,11 @@ public class Exceptions {
};
private static List<Class<? extends Throwable>> BORING_PREFIX_THROWABLE_EXACT_TYPES = ImmutableList.<Class<? extends Throwable>>of(
- IllegalStateException.class, RuntimeException.class, CompoundRuntimeException.class);
+ RuntimeException.class, Exception.class, Throwable.class,
+ IllegalStateException.class, IllegalArgumentException.class);
+
+ private static List<Class<? extends Throwable>> BORING_PREFIX_THROWABLE_SUPERTYPES = ImmutableList.<Class<? extends Throwable>>of(
+ ClassCastException.class, CompoundRuntimeException.class, PropagatedRuntimeException.class);
/** Returns whether this is throwable either known to be boring or to have an unhelpful type name (prefix)
* which should be suppressed. null is accepted but treated as not boring. */
@@ -74,6 +86,8 @@ public class Exceptions {
if (t instanceof UserFacingException) return true;
for (Class<? extends Throwable> type: BORING_PREFIX_THROWABLE_EXACT_TYPES)
if (t.getClass().equals(type)) return true;
+ for (Class<? extends Throwable> type: BORING_PREFIX_THROWABLE_SUPERTYPES)
+ if (type.isInstance(t)) return true;
return false;
}
@@ -109,17 +123,38 @@ public class Exceptions {
}
/**
- * See {@link #propagate(Throwable)}. If wrapping the exception, then include the given message;
- * otherwise the message is not used.
+ * See {@link #propagate(Throwable)}.
+ * <p>
+ * The given message is included <b>only</b> if the given {@link Throwable}
+ * needs to be wrapped; otherwise the message is not used.
+ * To always include the message, use {@link #propagateAnnotated(String, Throwable)}.
*/
public static RuntimeException propagate(String msg, Throwable throwable) {
+ return propagate(msg, throwable, false);
+ }
+
+ /** As {@link #propagate(String, Throwable)} but always re-wraps including the given message. */
+ public static RuntimeException propagateAnnotated(String msg, Throwable throwable) {
+ return propagate(msg, throwable, true);
+ }
+
+ private static RuntimeException propagate(String msg, Throwable throwable, boolean alwaysAnnotate) {
if (throwable instanceof InterruptedException) {
throw new RuntimeInterruptedException(msg, (InterruptedException) throwable);
} else if (throwable instanceof RuntimeInterruptedException) {
Thread.currentThread().interrupt();
- throw (RuntimeInterruptedException) throwable;
+ if (alwaysAnnotate) {
+ throw new RuntimeInterruptedException(msg, (RuntimeInterruptedException) throwable);
+ } else {
+ throw (RuntimeInterruptedException) throwable;
+ }
+ }
+ if (throwable==null) {
+ throw new PropagatedRuntimeException(msg, new NullPointerException("No throwable supplied."));
+ }
+ if (!alwaysAnnotate) {
+ Throwables.propagateIfPossible(checkNotNull(throwable));
}
- Throwables.propagateIfPossible(checkNotNull(throwable));
throw new PropagatedRuntimeException(msg, throwable);
}
@@ -201,7 +236,8 @@ public class Exceptions {
message = collapsed.getMessage();
messageIsFinal = true;
} else if (Strings.isNonBlank(collapsedS)) {
- collapsedS = Strings.removeAllFromEnd(collapsedS, cause.toString(), stripBoringPrefixes(cause.toString()), cause.getMessage());
+ String causeToString = getMessageWithAppropriatePrefix(cause);
+ collapsedS = Strings.removeAllFromEnd(collapsedS, cause.toString(), causeToString, stripBoringPrefixes(causeToString), cause.getMessage());
collapsedS = stripBoringPrefixes(collapsedS);
if (Strings.isNonBlank(collapsedS))
message = appendSeparator(message, collapsedS);
@@ -224,7 +260,7 @@ public class Exceptions {
return source;
if (collapseCount==0 && messagesCause!=null) {
- message = messagesCause.toString();
+ message = getMessageWithAppropriatePrefix(messagesCause);
messagesCause = messagesCause.getCause();
}
@@ -233,7 +269,7 @@ public class Exceptions {
message = appendSeparator(message, extraMessage);
}
if (message==null) message = "";
- return new PropagatedRuntimeException(message, collapseCausalChain ? collapsed : source, true);
+ return new PropagatedRuntimeException(message, collapseCausalChain ? collapsed : source, Strings.isNonBlank(message));
}
static String appendSeparator(String message, String next) {
@@ -272,9 +308,12 @@ public class Exceptions {
private static String collapseText(Throwable t, boolean includeAllCausalMessages, Set<Throwable> visited) {
if (t == null) return null;
if (visited.contains(t)) {
- // IllegalStateException sometimes refers to itself; guard against stack overflows
+ // If a boring-prefix class has no message it will render as multiply-visited.
+ // Additionally IllegalStateException sometimes refers to itself as its cause.
+ // In both cases, don't stack overflow!
if (Strings.isNonBlank(t.getMessage())) return t.getMessage();
- else return "("+t.getClass().getName()+", recursive cause)";
+ if (t.getCause()!=null) return t.getCause().getClass().getName();
+ return t.getClass().getName();
}
Throwable t2 = collapse(t, true, includeAllCausalMessages, visited);
visited = MutableSet.copyOf(visited);
@@ -288,7 +327,7 @@ public class Exceptions {
return collapseText(t2.getCause(), includeAllCausalMessages, ImmutableSet.copyOf(visited));
return ""+t2.getClass();
}
- String result = t2.toString();
+ String result = getMessageWithAppropriatePrefix(t2);
if (!includeAllCausalMessages) {
return result;
}
@@ -331,17 +370,25 @@ public class Exceptions {
/** Some throwables require a prefix for the message to make sense,
* for instance NoClassDefFoundError's message is often just the type.
*/
- public static boolean isPrefixImportant(Throwable t) {
+ @Beta
+ public static boolean isPrefixRequiredForMessageToMakeSense(Throwable t) {
if (t instanceof NoClassDefFoundError) return true;
return false;
}
- /** For {@link Throwable} instances where know {@link #isPrefixImportant(Throwable)},
- * this returns a nice message for use as a prefix; otherwise this returns the class name.
- * Callers should typically suppress the prefix if {@link #isPrefixBoring(Throwable)} is true. */
- public static String getPrefixText(Throwable t) {
- if (t instanceof NoClassDefFoundError) return "Type not found";
+ /** For {@link Throwable} instances where know {@link #isPrefixRequiredForMessageToMakeSense(Throwable)},
+ * this returns a nice message for use as a prefix;
+ * returns empty string if {@link #isPrefixBoring(Throwable)} is true;
+ * otherwise this returns the simplified class name. */
+ private static String getPrefixText(Throwable t) {
+ if (t instanceof NoClassDefFoundError) return "Invalid java type";
+ if (isPrefixBoring(t)) return "";
return JavaClassNames.cleanSimpleClassName(t);
}
+ /** Like {@link Throwable#toString()} except suppresses boring prefixes and replaces prefixes with sensible messages where required */
+ public static String getMessageWithAppropriatePrefix(Throwable t) {
+ return appendSeparator(getPrefixText(t), t.getMessage());
+ }
+
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java
index 0859ce8..db823a3 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java
@@ -18,6 +18,7 @@
*/
package org.apache.brooklyn.util.exceptions;
+import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -33,13 +34,19 @@ public class PropagatedRuntimeException extends RuntimeException {
public PropagatedRuntimeException(String message, Throwable cause) {
super(message, cause);
warnIfWrapping(cause);
- causeEmbeddedInMessage = message.endsWith(Exceptions.collapseText(getCause()));
+ causeEmbeddedInMessage = checkCauseEmbedded();
+ }
+
+ private boolean checkCauseEmbedded() {
+ String causalText = Exceptions.collapseText(getCause());
+ if (Strings.isBlank(causalText)) return false;
+ return getMessage().endsWith(causalText);
}
public PropagatedRuntimeException(String messagePart1, String messagePart2PossiblyIncludingPart1, Throwable cause) {
super(messagePart2PossiblyIncludingPart1!=null && messagePart2PossiblyIncludingPart1.startsWith(messagePart1) ? messagePart2PossiblyIncludingPart1 : messagePart1+messagePart2PossiblyIncludingPart1, cause);
warnIfWrapping(cause);
- causeEmbeddedInMessage = getMessage().endsWith(Exceptions.collapseText(getCause()));
+ causeEmbeddedInMessage = checkCauseEmbedded();
}
public PropagatedRuntimeException(String message, Throwable cause, boolean causeEmbeddedInMessage) {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/RuntimeInterruptedException.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/RuntimeInterruptedException.java b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/RuntimeInterruptedException.java
index 7ebb581..8a4482a 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/RuntimeInterruptedException.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/RuntimeInterruptedException.java
@@ -33,18 +33,14 @@ public class RuntimeInterruptedException extends RuntimeException {
private static final long serialVersionUID = 915050245927866175L;
- public RuntimeInterruptedException(InterruptedException cause) {
+ public RuntimeInterruptedException(Exception cause) {
super(cause);
Thread.currentThread().interrupt();
}
- public RuntimeInterruptedException(String msg, InterruptedException cause) {
+ public RuntimeInterruptedException(String msg, Exception cause) {
super(msg, cause);
Thread.currentThread().interrupt();
}
- @Override
- public InterruptedException getCause() {
- return (InterruptedException) super.getCause();
- }
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/main/java/org/apache/brooklyn/util/javalang/JavaClassNames.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/JavaClassNames.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/JavaClassNames.java
index fb8e4be..e9918df 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/JavaClassNames.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/JavaClassNames.java
@@ -18,6 +18,9 @@
*/
package org.apache.brooklyn.util.javalang;
+import java.util.Iterator;
+import java.util.Map;
+
import org.apache.brooklyn.util.net.Urls;
import org.apache.brooklyn.util.text.Strings;
@@ -47,8 +50,26 @@ public class JavaClassNames {
return c;
}
+ /** as {@link #simpleClassName(Class)} but returning json types if appropriate,
+ * e.g. `map`, `string`, etc; else falling back to detailed type */
+ public static String superSimpleClassName(Class<?> t) {
+ if (Map.class.isAssignableFrom(t)) return "map";
+ if (Iterable.class.isAssignableFrom(t) || Iterator.class.isAssignableFrom(t) ||
+ t.isArray()) return "list";
+ if (CharSequence.class.isAssignableFrom(t)) return "string";
+ if (Number.class.isAssignableFrom(t)) return "number";
+ if (Boolean.class.isAssignableFrom(t)) return "boolean";
+ return simpleClassName(t);
+ }
+
+ /** as {@link #simpleClassName(Object)} but looking up the type if needed */
+ public static String superSimpleClassName(Object o) {
+ return superSimpleClassName(type(o));
+ }
+
/** returns a simplified name of the class, just the simple name if it seems useful, else the full name */
public static String simpleClassName(Class<?> t) {
+ if (t==null) return null;
int arrayCount = 0;
while (t.isArray()) {
arrayCount++;
@@ -71,7 +92,6 @@ public class JavaClassNames {
* or a type-token; callers should usually do the getClass themselves, unless they aren't sure whether
* it is already a Class-type object */
public static String simpleClassName(Object x) {
- if (x==null) return null;
return simpleClassName(type(x));
}
@@ -158,5 +178,5 @@ public class JavaClassNames {
public static String niceClassAndMethod() {
return callerNiceClassAndMethod(0);
}
-
+
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/main/java/org/apache/brooklyn/util/net/Urls.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/net/Urls.java b/utils/common/src/main/java/org/apache/brooklyn/util/net/Urls.java
index 0d4af35..84850bf 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/net/Urls.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/net/Urls.java
@@ -26,7 +26,6 @@ import java.net.URL;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
import javax.annotation.Nullable;
@@ -110,14 +109,39 @@ public class Urls {
}
/** returns true if the string begins with a non-empty string of letters followed by a colon,
- * i.e. "protocol:" returns true, but "/" returns false */
+ * i.e. "protocol:" returns true, but "/" returns false,
+ * followed by a non-space character. thereafter it is not strict, it can have spaces, but not newlines;
+ * see {@link #isUrlWithProtocol(String, boolean, boolean)} */
public static boolean isUrlWithProtocol(String x) {
+ return isUrlWithProtocol(x, true, false);
+ }
+
+ /** as {@link #isUrlWithProtocol(String)} but configurable to be strict (false, false) or allow newline chars (if e.g. in an unescaped argument) */
+ public static boolean isUrlWithProtocol(String x, boolean allowSpacesAfterCharAfterColon, boolean allowMultiline) {
if (x==null) return false;
for (int i=0; i<x.length(); i++) {
char c = x.charAt(i);
- if (c==':') return i>0;
- if (!Character.isLetter(c)) return false;
+ if (c==':') {
+ if (i==0 || i+1>=x.length()) return false;
+ char c2 = x.charAt(i+1);
+ // never allow a whitespace or quote mark right after the ':', that is too similar to json/yaml!
+ if (Character.isWhitespace(c2) || c2=='\'' || c2=='\"') return false;
+ if (!allowMultiline) {
+ if (x.indexOf('\n')>=0) return false;
+ if (x.indexOf('\r')>=0) return false;
+ }
+ if (!allowSpacesAfterCharAfterColon) {
+ if (x.indexOf(' ')>=0) return false;
+ if (x.indexOf('\t')>=0) return false;
+ }
+ return true;
+ }
+ if (!Character.isAlphabetic(c) && c!='+') {
+ // only letters, numbers and + allowed before colon
+ return false;
+ }
}
+ // no colon found
return false;
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
index dee7651..29a189e 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
@@ -174,6 +174,48 @@ public class ExceptionsTest {
Assert.assertNotNull(text);
}
+ @Test
+ public void testPropagateNoMessageGivesType() throws Exception {
+ Throwable t = new Throwable();
+ Assert.assertEquals(Exceptions.collapseText(t), Throwable.class.getName());
+ try { Exceptions.propagate(t); } catch (Throwable t2) { t = t2; }
+ Assert.assertEquals(Exceptions.collapseText(t), Throwable.class.getName());
+ }
+
+ @Test
+ public void testPropagateWithoutAnnotationSuppressed() throws Exception {
+ Throwable t = new Throwable("test");
+ try { Exceptions.propagate(t); } catch (Throwable t2) { t = t2; }
+ Assert.assertEquals(Exceptions.collapseText(t), "test");
+ }
+
+ @Test
+ public void testPropagateWithAnnotationNotExplicitIncludedWhenWrapped() throws Exception {
+ Throwable t = new Throwable("test");
+ try { Exceptions.propagate("important", t); } catch (Throwable t2) { t = t2; }
+ Assert.assertEquals(Exceptions.collapseText(t), "important: test");
+ }
+
+ @Test
+ public void testPropagateWithAnnotationNotExplicitIgnoredWhenNotWrapped() throws Exception {
+ Throwable t = new RuntimeException("test");
+ try { Exceptions.propagate("ignore", t); } catch (Throwable t2) { t = t2; }
+ Assert.assertEquals(Exceptions.collapseText(t), "test");
+ }
+
+ @Test
+ public void testPropagateWithAnnotationExplicitNotSuppressed() throws Exception {
+ Throwable t = new RuntimeException("test");
+ try { Exceptions.propagateAnnotated("important", t); } catch (Throwable t2) { t = t2; }
+ Assert.assertEquals(Exceptions.collapseText(t), "important: test");
+ }
+
+ @Test
+ public void testPrefixModificationRequired() throws Exception {
+ Throwable t = new NoClassDefFoundError("sample");
+ Assert.assertEquals(Exceptions.collapseText(t), "Invalid java type: sample");
+ }
+
private void assert12StandardChecks(RuntimeException e, boolean isPropagated) {
String collapseText = Exceptions.collapseText(e);
log.info("Exception collapsing got: "+collapseText+" ("+e+")");
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c2fe5e91/utils/common/src/test/java/org/apache/brooklyn/util/javalang/JavaClassNamesTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/JavaClassNamesTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/JavaClassNamesTest.java
index 71174b3..73f925c 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/JavaClassNamesTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/JavaClassNamesTest.java
@@ -18,7 +18,8 @@
*/
package org.apache.brooklyn.util.javalang;
-import org.apache.brooklyn.util.javalang.JavaClassNames;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -73,4 +74,18 @@ public class JavaClassNamesTest {
String anon1 = JavaClassNames.simpleClassName(new Object() {});
Assert.assertTrue(anon1.startsWith(JavaClassNamesTest.class.getName()+"$"), "anon class is: "+anon1);
}
+
+ @Test
+ public void testSuperSimpleClassNames() {
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(this), "JavaClassNamesTest");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(1), "number");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(true), "boolean");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName("hello world"), "string");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(new String[][] { }), "list");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(new int[] { 1, 2, 3 }), "list");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(MutableMap.of("key", "val")), "map");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(MutableMap.of()), "map");
+ Assert.assertEquals(JavaClassNames.superSimpleClassName(MutableList.of()), "list");
+ }
+
}