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

[brooklyn-ui] branch master updated: attempt to fix WARN and ERROR on service creation, and tidy some references and logs

This is an automated email from the ASF dual-hosted git repository.

jcabrerizo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-ui.git


The following commit(s) were added to refs/heads/master by this push:
     new 1a6a88d  attempt to fix WARN and ERROR on service creation, and tidy some references and logs
     new 01295c1  Merge pull request #277 from ahgittin/fix-warnings-about-ui-modules-java-bundles
1a6a88d is described below

commit 1a6a88d1c22585522d04fe56ca94d7d4ed559f73
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Wed Sep 1 13:09:24 2021 +0100

    attempt to fix WARN and ERROR on service creation, and tidy some references and logs
    
    metadata service appeared to have an inconsistent naming strategy; when i updated the name it worked perfectly.
---
 modularity-server/external-modules/pom.xml         |  3 +--
 .../brooklyn/ui/modularity/ExternalUiModule.java   | 12 ++++++---
 modularity-server/metadata-registry/pom.xml        |  5 ++--
 .../registry/impl/UiMetadataConfigListener.java    | 31 +++++++++++++++++-----
 .../registry/impl/UiMetadataRegistryImpl.java      |  1 -
 .../ui/modularity/module/api/UiModuleListener.java |  1 +
 .../module/registry/UiModuleRegistryImpl.java      |  4 +++
 7 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/modularity-server/external-modules/pom.xml b/modularity-server/external-modules/pom.xml
index fc23600..fa83702 100644
--- a/modularity-server/external-modules/pom.xml
+++ b/modularity-server/external-modules/pom.xml
@@ -103,9 +103,8 @@
                     <instructions>
 
       <!-- exportScr and below from https://felix.apache.org/documentation/faqs/apache-felix-bundle-plugin-faq.html -->
-      <!-- Enable processing of OSGI DS component annotations -->
+      <!-- Enable processing of OSGI DS component and metatype annotations - only needed for some type of tests it seems but no harm in having it --> 
       <_dsannotations>*</_dsannotations>
-      <!-- Enable processing of OSGI metatype annotations -->
       <_metatypeannotations>*</_metatypeannotations>
 
                         <Import-Package>
diff --git a/modularity-server/external-modules/src/main/java/org/apache/brooklyn/ui/modularity/ExternalUiModule.java b/modularity-server/external-modules/src/main/java/org/apache/brooklyn/ui/modularity/ExternalUiModule.java
index 7866efe..160ad08 100644
--- a/modularity-server/external-modules/src/main/java/org/apache/brooklyn/ui/modularity/ExternalUiModule.java
+++ b/modularity-server/external-modules/src/main/java/org/apache/brooklyn/ui/modularity/ExternalUiModule.java
@@ -41,7 +41,7 @@ import com.google.common.collect.ImmutableList;
 @Component(
         name = ExternalUiModule.PID,
         configurationPid = ExternalUiModule.PID,
-        configurationPolicy = ConfigurationPolicy.REQUIRE,
+        configurationPolicy = ConfigurationPolicy.REQUIRE,  // only trigger if there is a corresponding PID config admin, setting up an external ui module?
         immediate = true
 )
 public class ExternalUiModule implements UiModule {
@@ -71,7 +71,12 @@ public class ExternalUiModule implements UiModule {
 
     @Activate
     public void activate(final Map<String, String> properties) {
-        this.setModuleProperties(properties);
+        if (!properties.containsKey(KEY_URL) && properties.containsKey("component.id")) {
+            // activation properties aren't usually for a module; do this check to suppress warning
+            LOG.debug("Not setting module properties for activation properties "+properties);
+        } else {
+            this.setModuleProperties(properties);
+        }
     }
 
     @Modified
@@ -90,7 +95,8 @@ public class ExternalUiModule implements UiModule {
         }
 
         if (issues.size() > 0) {
-            throw new IllegalArgumentException("Invalid UI module [" + properties.get(KEY_ID) + "] ... " + issues.toString());
+            LOG.error("Invalid UI module (ignoring) [" + properties.get(KEY_ID) + "] ... " + issues.toString()+"; properties: "+properties, new Throwable("source of error"));
+            return;
         }
 
         this.id = properties.get(KEY_ID);
diff --git a/modularity-server/metadata-registry/pom.xml b/modularity-server/metadata-registry/pom.xml
index ce62d4a..2c15bb7 100644
--- a/modularity-server/metadata-registry/pom.xml
+++ b/modularity-server/metadata-registry/pom.xml
@@ -114,10 +114,9 @@
                     <exportScr>true</exportScr>
                     <instructions>
 
-      <!-- exportScr and below from https://felix.apache.org/documentation/faqs/apache-felix-bundle-plugin-faq.html -->
-      <!-- Enable processing of OSGI DS component annotations -->
+      <!-- exportScr above and below from https://felix.apache.org/documentation/faqs/apache-felix-bundle-plugin-faq.html -->
+      <!-- Enable processing of OSGI DS component and metatype annotations - only needed for some type of tests it seems but no harm in having it -->
       <_dsannotations>*</_dsannotations>
-      <!-- Enable processing of OSGI metatype annotations -->
       <_metatypeannotations>*</_metatypeannotations>
 
                         <Import-Package>
diff --git a/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataConfigListener.java b/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataConfigListener.java
index 564f6c7..ef15c21 100644
--- a/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataConfigListener.java
+++ b/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataConfigListener.java
@@ -33,11 +33,11 @@ import static com.google.common.base.Predicates.not;
 
 @Component(
         name = "Brooklyn UI Metadata",
-        configurationPid = UiMetadataConfigListener.PID, configurationPolicy = ConfigurationPolicy.REQUIRE, immediate = true,
+        configurationPid = UiMetadataConfigListener.PID, configurationPolicy = ConfigurationPolicy.OPTIONAL, immediate = true,
         property = {UiMetadataRegistry.METADATA_TYPE + ":String=" + UiMetadataRegistry.METADATA_TYPE_DEFAULT}
 )
 public class UiMetadataConfigListener {
-    static final String PID = "org.apache.brooklyn.ui.metadata";
+    static final String PID = "org.apache.brooklyn.ui.modularity.metadata";
     private static final Logger logger = LoggerFactory.getLogger(UiMetadataConfigListener.class);
     private static final List<String> EXCLUDE = Arrays.asList(
             "felix.fileinstall.filename", "service.factoryPid", "component.name", "component.id"
@@ -46,28 +46,45 @@ public class UiMetadataConfigListener {
     @Reference
     private UiMetadataRegistry metadataRegistry;
 
+    protected String getId(final Map<String, String> properties) {
+        return properties.containsKey(UiMetadataRegistry.METADATA_ID) ? 
+                        properties.get(UiMetadataRegistry.METADATA_ID) : properties.get("service.pid");
+    }
+
     @Activate
     public void activate(final Map<String, String> properties) {
-        modified(properties);
+        if (getId(properties)==null) {
+            logger.debug("Skipping recording of metadata config for irrelevant activation record: "+properties);
+        } else {
+            modified(properties);
+        }
     }
 
     @Modified
     public void modified(final Map<String, String> properties) {
+        String id = getId(properties);
+        if (id==null) {
+            logger.warn("Skipping update of UI metadata because ID is not specified: "+properties);
+            return;
+        }
         metadataRegistry.modifyMetadata(
                 properties.containsKey(UiMetadataRegistry.METADATA_TYPE) ?
                         properties.get(UiMetadataRegistry.METADATA_TYPE) : UiMetadataRegistry.METADATA_TYPE_DEFAULT,
-                properties.containsKey(UiMetadataRegistry.METADATA_ID) ?
-                        properties.get(UiMetadataRegistry.METADATA_ID) : properties.get("service.pid"),
+                id,
                 Maps.filterKeys(properties, not(in(EXCLUDE)))
         );
     }
     
     @Deactivate
     public void deactivate(final Map<String, String> properties) {
+        String id = getId(properties);
+        if (id==null) {
+            logger.debug("Skipping deactivation of UI metadata because ID is not specified: "+properties);
+            return;
+        }
         metadataRegistry.unregisterMetadata(
                 properties.containsKey(UiMetadataRegistry.METADATA_TYPE) ?
                         properties.get(UiMetadataRegistry.METADATA_TYPE) : UiMetadataRegistry.METADATA_TYPE_DEFAULT,
-                properties.containsKey(UiMetadataRegistry.METADATA_ID) ?
-                        properties.get(UiMetadataRegistry.METADATA_ID) : properties.get("service.pid"));
+                id);
     }
 }
diff --git a/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataRegistryImpl.java b/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataRegistryImpl.java
index 5aac821..a18fbee 100644
--- a/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataRegistryImpl.java
+++ b/modularity-server/metadata-registry/src/main/java/org/apache/brooklyn/ui/modularity/metadata/registry/impl/UiMetadataRegistryImpl.java
@@ -53,7 +53,6 @@ public class UiMetadataRegistryImpl implements UiMetadataRegistry {
 
     @Override
     public Map<String, Map<String, String>> getByType(final String type) {
-        logger.error(type);
         return metadataTable.row(type);
     }
 
diff --git a/modularity-server/module-api/src/main/java/org/apache/brooklyn/ui/modularity/module/api/UiModuleListener.java b/modularity-server/module-api/src/main/java/org/apache/brooklyn/ui/modularity/module/api/UiModuleListener.java
index 8125e44..690998b 100644
--- a/modularity-server/module-api/src/main/java/org/apache/brooklyn/ui/modularity/module/api/UiModuleListener.java
+++ b/modularity-server/module-api/src/main/java/org/apache/brooklyn/ui/modularity/module/api/UiModuleListener.java
@@ -259,6 +259,7 @@ public class UiModuleListener implements ServletContextListener {
         }
         @SuppressWarnings("unchecked")
         Map<String, ?> config = (Map<String, ?>) new Yaml().load(is);
+        LOG.debug("Creating Brooklyn UI module definition for "+path+"; "+config+" / "+is);
         return UiModuleImpl.createFromMap(config).path(path);
     }
 }
diff --git a/modularity-server/module-registry/src/main/java/org/apache/brooklyn/ui/modularity/module/registry/UiModuleRegistryImpl.java b/modularity-server/module-registry/src/main/java/org/apache/brooklyn/ui/modularity/module/registry/UiModuleRegistryImpl.java
index ccd369e..a34d32a 100644
--- a/modularity-server/module-registry/src/main/java/org/apache/brooklyn/ui/modularity/module/registry/UiModuleRegistryImpl.java
+++ b/modularity-server/module-registry/src/main/java/org/apache/brooklyn/ui/modularity/module/registry/UiModuleRegistryImpl.java
@@ -37,6 +37,10 @@ public class UiModuleRegistryImpl implements UiModuleRegistry {
     private final ConcurrentHashMap<String, UiModule> registry = new ConcurrentHashMap<>();
 
     public void register(final UiModule uiModule) {
+        if (uiModule.getId()==null) {
+            LOG.error("Skipping invalid Brooklyn UI module "+uiModule, new Throwable("source of error"));
+            return;
+        }
         LOG.info("Registering new Brooklyn web component [{}] [{}]", uiModule.getId(), uiModule.getName());
         registry.put(uiModule.getId(), uiModule);
     }