address review comments
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/97f25243 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/97f25243 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/97f25243 Branch: refs/heads/master Commit: 97f252439a1b247e6820dd4b2d1ebc9cd1323eca Parents: c7034e6 Author: Alex Heneveld <[email protected]> Authored: Wed Jul 19 14:54:15 2017 +0100 Committer: Alex Heneveld <[email protected]> Committed: Wed Jul 19 14:54:15 2017 +0100 ---------------------------------------------------------------------- .../catalog/internal/BasicBrooklynCatalog.java | 14 ++++++----- .../core/typereg/BasicManagedBundle.java | 13 +++++----- .../core/typereg/BasicRegisteredType.java | 1 + .../brooklyn/util/core/LoaderDispatcher.java | 1 - .../rest/resources/CatalogResource.java | 3 +-- .../rest/transform/CatalogTransformer.java | 26 +++++--------------- .../brooklyn/util/exceptions/Exceptions.java | 6 +++-- .../brooklyn/util/osgi/VersionedName.java | 4 +-- 8 files changed, 29 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/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 cb1a138..30c2f0b 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 @@ -1055,13 +1055,14 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { File fJar = Os.newTempFile(containingBundle.getVersionedName().toOsgiString(), ".jar"); try { Streams.copy(ResourceUtils.create().getResourceFromUrl(url), new FileOutputStream(fJar)); + subCatalog.addToClasspath(new String[] { "file:"+fJar.getAbsolutePath() }); + Collection<CatalogItemDtoAbstract<?, ?>> result = scanAnnotationsInternal(mgmt, subCatalog, MutableMap.of("version", containingBundle.getSuppliedVersionString()), containingBundle); + return result; } catch (FileNotFoundException e) { - throw Exceptions.propagate("Error extracting "+url+" to scan "+containingBundle.getVersionedName(), e); + throw Exceptions.propagateAnnotated("Error extracting "+url+" to scan "+containingBundle.getVersionedName(), e); + } finally { + fJar.delete(); } - subCatalog.addToClasspath(new String[] { "file:"+fJar.getAbsolutePath() }); - Collection<CatalogItemDtoAbstract<?, ?>> result = scanAnnotationsInternal(mgmt, subCatalog, MutableMap.of("version", containingBundle.getSuppliedVersionString()), containingBundle); - fJar.delete(); - return result; } private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsInternal(ManagementContext mgmt, CatalogDo subCatalog, Map<?, ?> catalogMetadata, ManagedBundle containingBundle) { @@ -1376,8 +1377,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { result = osgiManager.get().install(null, new FileInputStream(bf), true, true, forceUpdate).get(); } catch (FileNotFoundException e) { throw Exceptions.propagate(e); + } finally { + bf.delete(); } - bf.delete(); uninstallEmptyWrapperBundles(); if (result.getCode().isError()) { throw new IllegalStateException(result.getMessage()); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java index 1cf4e84..19c1699 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java @@ -81,7 +81,7 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage @Override public String getOsgiVersionString() { - return version==null ? version : BrooklynVersionSyntax.toValidOsgiVersion(version); + return version==null ? null : BrooklynVersionSyntax.toValidOsgiVersion(version); } public void setVersion(String version) { @@ -130,7 +130,7 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage @Override public int hashCode() { // checksum deliberately omitted here to match with OsgiBundleWithUrl - return Objects.hashCode(symbolicName, version, url); + return Objects.hashCode(symbolicName, getOsgiVersionString(), url); } @Override @@ -142,10 +142,11 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage if (!Objects.equal(symbolicName, other.getSymbolicName())) return false; if (!Objects.equal(getOsgiVersionString(), other.getOsgiVersionString())) return false; if (!Objects.equal(url, other.getUrl())) return false; - if (other instanceof BasicManagedBundle) { - // make equality symetricm, OsgiBunde equals this iff this equals OsgiBundle; - // checksum compared if available, but not required - if (!Objects.equal(checksum, ((BasicManagedBundle)other).getChecksum())) return false; + if (other instanceof ManagedBundle) { + // checksum compared if available, but not required; + // this makes equality with other OsgiBundleWithUrl items symmetric, + // but for two MB's we look additionally at checksum + if (!Objects.equal(checksum, ((ManagedBundle)other).getChecksum())) return false; } return true; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java index 2edd49f..f76a9f4 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java @@ -192,6 +192,7 @@ public class BasicRegisteredType implements RegisteredType { if (!Objects.equal(bundles, other.bundles)) return false; if (!Objects.equal(containingBundle, other.containingBundle)) return false; if (!Objects.equal(deprecated, other.deprecated)) return false; + if (!Objects.equal(description, other.description)) return false; if (!Objects.equal(disabled, other.disabled)) return false; if (!Objects.equal(iconUrl, other.iconUrl)) return false; if (!Objects.equal(implementationPlan, other.implementationPlan)) return false; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/core/src/main/java/org/apache/brooklyn/util/core/LoaderDispatcher.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/LoaderDispatcher.java b/core/src/main/java/org/apache/brooklyn/util/core/LoaderDispatcher.java index 4d13aa8..54af77b 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/LoaderDispatcher.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/LoaderDispatcher.java @@ -54,7 +54,6 @@ public interface LoaderDispatcher<T> { @Override public Maybe<Class<?>> tryLoadFrom(BrooklynClassLoadingContext loader, String className) { try { - // return Maybe.<Class<?>>of(loader.loadClass(className)); return loader.tryLoadClass(className); } catch (IllegalStateException e) { propagateIfCauseNotClassNotFound(e); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index 0133962..fcb26f9 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -140,8 +140,7 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat List<RegisteredType> itemsRT = MutableList.of(); for (CatalogItem<?, ?> ci: items) { RegisteredType rt = brooklyn().getTypeRegistry().get(ci.getId()); - if (rt!=null) itemsRT.add(rt); - else itemsRT.add(RegisteredTypes.of(ci)); + itemsRT.add(rt!=null ? rt : RegisteredTypes.of(ci)); } return buildCreateResponse(itemsRT); } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java index b1ec6e5..9fe024a 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java @@ -223,8 +223,13 @@ public class CatalogTransformer { } private static Set<Object> makeTags(EntitySpec<?> spec, RegisteredType item) { + return makeTags(spec, MutableSet.copyOf(item.getTags())); + } + private static Set<Object> makeTags(EntitySpec<?> spec, CatalogItem<?, ?> item) { + return makeTags(spec, MutableSet.copyOf(item.tags().getTags())); + } + private static Set<Object> makeTags(EntitySpec<?> spec, Set<Object> tags) { // Combine tags on item with an InterfacesTag. - Set<Object> tags = MutableSet.copyOf(item.getTags()); if (spec != null) { Class<?> type; if (spec.getImplementation() != null) { @@ -238,8 +243,6 @@ public class CatalogTransformer { } return tags; } - - /** @deprecated since 0.12.0 use {@link RegisteredType} methods instead */ @Deprecated public static <T extends Entity> CatalogEntitySummary catalogEntitySummary(BrooklynRestResourceUtils b, CatalogItem<T,EntitySpec<? extends T>> item, UriBuilder ub) { @@ -381,21 +384,4 @@ public class CatalogTransformer { return iconUrl; } - private static Set<Object> makeTags(EntitySpec<?> spec, CatalogItem<?, ?> item) { - // Combine tags on item with an InterfacesTag. - Set<Object> tags = MutableSet.copyOf(item.tags().getTags()); - if (spec != null) { - Class<?> type; - if (spec.getImplementation() != null) { - type = spec.getImplementation(); - } else { - type = spec.getType(); - } - if (type != null) { - tags.add(new BrooklynTags.TraitsTag(Reflections.getAllInterfaces(type))); - } - } - return tags; - } - } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/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 82fcce7..55bf8a2 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 @@ -152,8 +152,10 @@ public class Exceptions { return propagate(msg, throwable, false); } - /** As {@link #propagate(String)} but always re-wraps including the given message. - * See {@link #propagateAnnotateIfWrapping(String, Throwable)} if the message is optional. */ + /** As {@link #propagate(String, Throwable)} but unlike earlier deprecated version + * this always re-wraps including the given message, until semantics of that method change to match this. + * See {@link #propagateAnnotateIfWrapping(String, Throwable)} if the message + * should be omitted and the given throwable preserved if it can already be propagated. */ public static RuntimeException propagateAnnotated(String msg, Throwable throwable) { return propagate(msg, throwable, true); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97f25243/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java b/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java index dc35334..9fec03c 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java @@ -127,8 +127,8 @@ public class VersionedName { } /** As {@link #equals(Object)} but accepting the argument as equal - * if versions are identical when injected to OSGi-valid versions, - * and accepting strings as the other */ + * if versions are identical under the {@link #getOsgiVersion()} conversion; + * also accepts strings as the other, converting as per {@link #fromString(String)} */ public boolean equalsOsgi(Object other) { if (other instanceof String) { other = VersionedName.fromString((String)other);
