Code review comments. https://github.com/apache/brooklyn-server/pull/80#discussion_r57442437 https://github.com/apache/brooklyn-server/pull/80#discussion_r57442486 https://github.com/apache/brooklyn-server/pull/80#discussion_r57442585 https://github.com/apache/brooklyn-server/pull/80#discussion-diff-57442710
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/84720cfa Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/84720cfa Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/84720cfa Branch: refs/heads/master Commit: 84720cfa88bd98eed6b73918d3f250bee1291157 Parents: 7a1d51c Author: Geoff Macartney <[email protected]> Authored: Tue Apr 5 15:06:13 2016 +0100 Committer: Geoff Macartney <[email protected]> Committed: Tue Apr 5 15:06:13 2016 +0100 ---------------------------------------------------------------------- .../catalog/internal/CatalogBomScanner.java | 32 ++++++++----- .../resources/OSGI-INF/blueprint/blueprint.xml | 50 ++++++++++---------- 2 files changed, 46 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/84720cfa/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java index 5b2d52b..782055a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java @@ -40,6 +40,7 @@ import org.slf4j.LoggerFactory; import org.yaml.snakeyaml.Yaml; import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.util.List; import java.util.Map; @@ -119,6 +120,9 @@ public class CatalogBomScanner { @Override public void removedBundle(Bundle bundle, BundleEvent bundleEvent, Iterable<? extends CatalogItem<?, ?>> items) { + if (!items.iterator().hasNext()) { + return; + } LOG.debug("Unloading catalog BOM entries from {} {} {}", bundleIds(bundle)); List<Exception> exceptions = MutableList.of(); final BrooklynCatalog catalog = getManagementContext().getCatalog(); @@ -143,32 +147,38 @@ public class CatalogBomScanner { } private Iterable<? extends CatalogItem<?, ?>> scanForCatalog(Bundle bundle) { - LOG.debug("Scanning for catalog items in bundle {} {} {}", bundleIds(bundle)); - final URL bom = bundle.getResource(CATALOG_BOM_URL); + Iterable<? extends CatalogItem<?, ?>> catalogItems = ImmutableList.of(); + + final URL bom = bundle.getResource(CATALOG_BOM_URL); if (null != bom) { LOG.debug("Found catalog BOM in {} {} {}", bundleIds(bundle)); String bomText = readBom(bom); String bomWithLibraryPath = addLibraryDetails(bundle, bomText); - final Iterable<? extends CatalogItem<?, ?>> catalogItems = - getManagementContext().getCatalog().addItems(bomWithLibraryPath); + catalogItems = getManagementContext().getCatalog().addItems(bomWithLibraryPath); for (CatalogItem<?, ?> item : catalogItems) { LOG.debug("Added to catalog: {}, {}", item.getSymbolicName(), item.getVersion()); } - return catalogItems; + } else { + LOG.debug("No BOM found in {} {} {}", bundleIds(bundle)); } - return ImmutableList.of(); + + return catalogItems; } private String addLibraryDetails(Bundle bundle, String bomText) { final Map<String, Object> bom = (Map<String, Object>)Iterables.getOnlyElement(Yamls.parseAll(bomText)); final Object catalog = bom.get(BROOKLYN_CATALOG); - if (null != catalog && catalog instanceof Map<?, ?>) { - addLibraryDetails(bundle, (Map<String, Object>) catalog); + if (null != catalog) { + if (catalog instanceof Map<?, ?>) { + addLibraryDetails(bundle, (Map<String, Object>) catalog); + } else { + LOG.warn("Unexpected syntax for {} (expected Map), ignoring", BROOKLYN_CATALOG); + } } final String updatedBom = new Yaml().dump(bom); - LOG.debug("Updated catalog bom:\n{}", updatedBom); + LOG.trace("Updated catalog bom:\n{}", updatedBom); return updatedBom; } @@ -188,8 +198,8 @@ public class CatalogBomScanner { } private String readBom(URL bom) { - try { - return Streams.readFullyString(bom.openStream()); + try (final InputStream ins = bom.openStream()) { + return Streams.readFullyString(ins); } catch (IOException e) { throw Exceptions.propagate("Error loading Catalog BOM from " + bom, e); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/84720cfa/karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml ---------------------------------------------------------------------- diff --git a/karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 51dfce8..36c0fef 100644 --- a/karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -25,62 +25,62 @@ limitations under the License. <!-- Makes sure core bundle is already started --> <reference id="brooklynVersion" - interface="org.apache.brooklyn.core.BrooklynVersionService"/> + interface="org.apache.brooklyn.core.BrooklynVersionService" /> <reference id="systemService" interface="org.apache.karaf.system.SystemService" /> <cm:property-placeholder persistent-id="org.apache.brooklyn.osgilauncher" update-strategy="reload"> <cm:default-properties> - <cm:property name="ignoreCatalogErrors" value="true"/> - <cm:property name="ignorePersistenceErrors" value="true"/> - <cm:property name="highAvailabilityMode" value="DISABLED"/> - <cm:property name="persistMode" value="DISABLED"/> - <cm:property name="persistenceDir" value=""/> - <cm:property name="persistenceLocation" value=""/> - <cm:property name="persistPeriod" value="1s"/> - <cm:property name="globalBrooklynPropertiesFile" value="~/.brooklyn/brooklyn.properties"/> - <cm:property name="localBrooklynPropertiesFile" value=""/> <!-- used to be settable through cli params --> + <cm:property name="ignoreCatalogErrors" value="true" /> + <cm:property name="ignorePersistenceErrors" value="true" /> + <cm:property name="highAvailabilityMode" value="DISABLED" /> + <cm:property name="persistMode" value="DISABLED" /> + <cm:property name="persistenceDir" value="" /> + <cm:property name="persistenceLocation" value="" /> + <cm:property name="persistPeriod" value="1s" /> + <cm:property name="globalBrooklynPropertiesFile" value="~/.brooklyn/brooklyn.properties" /> + <cm:property name="localBrooklynPropertiesFile" value="" /> <!-- used to be settable through cli params --> </cm:default-properties> </cm:property-placeholder> <bean id="propertiesBuilderFactory" class="org.apache.brooklyn.launcher.common.BrooklynPropertiesFactoryHelper"> - <argument value="${globalBrooklynPropertiesFile}"/> - <argument value="${localBrooklynPropertiesFile}"/> + <argument value="${globalBrooklynPropertiesFile}" /> + <argument value="${localBrooklynPropertiesFile}" /> </bean> <bean id="propertiesBuilder" class="org.apache.brooklyn.core.internal.BrooklynProperties.Factory.Builder" factory-ref="propertiesBuilderFactory" - factory-method="createPropertiesBuilder"/> + factory-method="createPropertiesBuilder" /> <bean id="launcher" class="org.apache.brooklyn.launcher.osgi.OsgiLauncher" init-method="init" destroy-method="destroy"> - <property name="brooklynVersion" ref="brooklynVersion"/> - <property name="brooklynPropertiesBuilder" ref="propertiesBuilder"/> + <property name="brooklynVersion" ref="brooklynVersion" /> + <property name="brooklynPropertiesBuilder" ref="propertiesBuilder" /> - <property name="ignoreCatalogErrors" value="${ignoreCatalogErrors}"/> - <property name="ignorePersistenceErrors" value="${ignorePersistenceErrors}"/> - <property name="highAvailabilityMode" value="${highAvailabilityMode}"/> - <property name="persistMode" value="${persistMode}"/> - <property name="persistenceDir" value="${persistenceDir}"/> - <property name="persistenceLocation" value="${persistenceLocation}"/> - <property name="persistPeriod" value="${persistPeriod}"/> + <property name="ignoreCatalogErrors" value="${ignoreCatalogErrors}" /> + <property name="ignorePersistenceErrors" value="${ignorePersistenceErrors}" /> + <property name="highAvailabilityMode" value="${highAvailabilityMode}" /> + <property name="persistMode" value="${persistMode}" /> + <property name="persistenceDir" value="${persistenceDir}" /> + <property name="persistenceLocation" value="${persistenceLocation}" /> + <property name="persistPeriod" value="${persistPeriod}" /> </bean> <bean id="localManagementContextService" class="org.apache.brooklyn.core.mgmt.internal.LocalManagementContext" factory-ref="launcher" - factory-method="getManagementContext"/> + factory-method="getManagementContext" /> <bean id="campPlatform" class="org.apache.brooklyn.camp.CampPlatform" factory-ref="launcher" - factory-method="getCampPlatform"/> + factory-method="getCampPlatform" /> <service interface="org.apache.brooklyn.core.mgmt.ShutdownHandler"> <bean class="org.apache.brooklyn.launcher.osgi.OsgiShutdownHandler" /> @@ -98,7 +98,7 @@ limitations under the License. availability="optional"> <reference-listener bind-method="bind" unbind-method="unbind"> - <bean class="org.apache.brooklyn.core.catalog.internal.CatalogBomScanner"/> + <bean class="org.apache.brooklyn.core.catalog.internal.CatalogBomScanner" /> </reference-listener> </reference-list>
