You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sv...@apache.org on 2016/04/11 11:02:01 UTC

[2/4] brooklyn-server git commit: Code review comments.

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 <ge...@cloudsoftcorp.com>
Authored: Tue Apr 5 15:06:13 2016 +0100
Committer: Geoff Macartney <ge...@cloudsoftcorp.com>
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>