Bug 2150: Feature wrappers now log parse errors better.

[controller.git] / opendaylight / config / config-persister-feature-adapter / src / main / java / org / opendaylight / controller / configpusherfeature / internal / AbstractFeatureWrapper.java

diff --git a/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/AbstractFeatureWrapper.java b/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/AbstractFeatureWrapper.java
index 1bf2025c46ee3af3085f77ab6fdf166a54b71846..4f598aae80e2ac2678902e8367f3bce9fb7950ba 100644 (file)
--- a/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/AbstractFeatureWrapper.java
+++ b/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/AbstractFeatureWrapper.java
@@ -7,22 +7,22 @@
  */
 package org.opendaylight.controller.configpusherfeature.internal;
 
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import com.google.common.io.Files;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
-
 import javax.xml.bind.JAXBException;
-
+import javax.xml.stream.XMLStreamException;
 import org.apache.karaf.features.BundleInfo;
 import org.apache.karaf.features.Conditional;
 import org.apache.karaf.features.ConfigFileInfo;
+import org.apache.karaf.features.ConfigInfo;
 import org.apache.karaf.features.Dependency;
 import org.apache.karaf.features.Feature;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Preconditions;
-
 /*
  * Wrap a Feature for the purposes of extracting the FeatureConfigSnapshotHolders from
  * its underlying ConfigFileInfo's
@@ -30,7 +30,10 @@ import com.google.common.base.Preconditions;
  * Delegates the the contained feature and provides additional methods.
  */
 public class AbstractFeatureWrapper implements Feature {
-    private static final Logger logger = LoggerFactory.getLogger(AbstractFeatureWrapper.class);
+    private static final Logger LOG = LoggerFactory.getLogger(AbstractFeatureWrapper.class);
+
+    protected static final String CONFIG_FILE_SUFFIX = "xml";
+
     protected Feature feature = null;
 
     protected AbstractFeatureWrapper() {
@@ -40,7 +43,7 @@ public class AbstractFeatureWrapper implements Feature {
     /*
      * @param f Feature to wrap
      */
-    public AbstractFeatureWrapper(Feature f) {
+    public AbstractFeatureWrapper(final Feature f) {
         Preconditions.checkNotNull(f,"FeatureWrapper requires non-null Feature in constructor");
         this.feature = f;
     }
@@ -50,17 +53,35 @@ public class AbstractFeatureWrapper implements Feature {
      * from the underlying Feature Config files
      */
     public LinkedHashSet<FeatureConfigSnapshotHolder> getFeatureConfigSnapshotHolders() throws Exception {
-        LinkedHashSet <FeatureConfigSnapshotHolder> snapShotHolders = new LinkedHashSet<FeatureConfigSnapshotHolder>();
-        for(ConfigFileInfo c: getConfigurationFiles()) {
-            try {
-                snapShotHolders.add(new FeatureConfigSnapshotHolder(c,this));
-            } catch (JAXBException e) {
-                logger.debug("{} is not a config subsystem config file",c.getFinalname());
+        final LinkedHashSet <FeatureConfigSnapshotHolder> snapShotHolders = new LinkedHashSet<>();
+        for(final ConfigFileInfo c: getConfigurationFiles()) {
+            // Skip non xml files
+            if(Files.getFileExtension(c.getFinalname()).equals(CONFIG_FILE_SUFFIX)) {
+                final Optional<FeatureConfigSnapshotHolder> featureConfigSnapshotHolder = getFeatureConfigSnapshotHolder(c);
+                if(featureConfigSnapshotHolder.isPresent()) {
+                    snapShotHolders.add(featureConfigSnapshotHolder.get());
+                }
             }
         }
         return snapShotHolders;
     }
 
+    protected Optional<FeatureConfigSnapshotHolder> getFeatureConfigSnapshotHolder(final ConfigFileInfo c) {
+        try {
+            return Optional.of(new FeatureConfigSnapshotHolder(c, this));
+        } catch (final JAXBException e) {
+            LOG.warn("Unable to parse configuration snapshot. Config from '{}' will be IGNORED. " +
+                            "Note that subsequent config files may fail due to this problem. " +
+                            "Xml markup in this file needs to be fixed, for detailed information see enclosed exception.",
+                    c.getFinalname(), e);
+        } catch (final XMLStreamException e) {
+            // Files that cannot be loaded are ignored as non config subsystem files e.g. jetty.xml
+            LOG.debug("Unable to read configuration file '{}'. Not a configuration snapshot",
+                    c.getFinalname(), e);
+        }
+        return Optional.absent();
+    }
+
     @Override
     public int hashCode() {
         final int prime = 31;
@@ -70,19 +91,24 @@ public class AbstractFeatureWrapper implements Feature {
     }
 
     @Override
-    public boolean equals(Object obj) {
-        if (this == obj)
+    public boolean equals(final Object obj) {
+        if (this == obj) {
             return true;
-        if (obj == null)
+        }
+        if (obj == null) {
             return false;
-        if (getClass() != obj.getClass())
+        }
+        if (getClass() != obj.getClass()) {
             return false;
-        AbstractFeatureWrapper other = (AbstractFeatureWrapper) obj;
+        }
+        final AbstractFeatureWrapper other = (AbstractFeatureWrapper) obj;
         if (feature == null) {
-            if (other.feature != null)
+            if (other.feature != null) {
                 return false;
-        } else if (!feature.equals(other.feature))
+            }
+        } else if (!feature.equals(other.feature)) {
             return false;
+        }
         return true;
     }
 
@@ -95,6 +121,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getId()
      */
+    @Override
     public String getId() {
         return feature.getId();
     }
@@ -103,6 +130,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getName()
      */
+    @Override
     public String getName() {
         return feature.getName();
     }
@@ -111,6 +139,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getDescription()
      */
+    @Override
     public String getDescription() {
         return feature.getDescription();
     }
@@ -119,6 +148,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getDetails()
      */
+    @Override
     public String getDetails() {
         return feature.getDetails();
     }
@@ -127,6 +157,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getVersion()
      */
+    @Override
     public String getVersion() {
         return feature.getVersion();
     }
@@ -135,6 +166,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#hasVersion()
      */
+    @Override
     public boolean hasVersion() {
         return feature.hasVersion();
     }
@@ -143,6 +175,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getResolver()
      */
+    @Override
     public String getResolver() {
         return feature.getResolver();
     }
@@ -151,6 +184,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getInstall()
      */
+    @Override
     public String getInstall() {
         return feature.getInstall();
     }
@@ -159,6 +193,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getDependencies()
      */
+    @Override
     public List<Dependency> getDependencies() {
         return feature.getDependencies();
     }
@@ -167,6 +202,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getBundles()
      */
+    @Override
     public List<BundleInfo> getBundles() {
         return feature.getBundles();
     }
@@ -175,7 +211,8 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getConfigurations()
      */
-    public Map<String, Map<String, String>> getConfigurations() {
+    @Override
+    public List<ConfigInfo> getConfigurations() {
         return feature.getConfigurations();
     }
 
@@ -183,6 +220,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getConfigurationFiles()
      */
+    @Override
     public List<ConfigFileInfo> getConfigurationFiles() {
         return feature.getConfigurationFiles();
     }
@@ -191,6 +229,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getConditional()
      */
+    @Override
     public List<? extends Conditional> getConditional() {
         return feature.getConditional();
     }
@@ -199,6 +238,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getStartLevel()
      */
+    @Override
     public int getStartLevel() {
         return feature.getStartLevel();
     }
@@ -207,6 +247,7 @@ public class AbstractFeatureWrapper implements Feature {
      * @return
      * @see org.apache.karaf.features.Feature#getRegion()
      */
+    @Override
     public String getRegion() {
         return feature.getRegion();
     }