Bug 2150: Feature wrappers now log parse errors better. 52/11752/8
authorMaros Marsalek <mmarsale@cisco.com>
Mon, 25 May 2015 14:32:13 +0000 (16:32 +0200)
committerGerrit Code Review <gerrit@opendaylight.org>
Tue, 26 May 2015 12:16:23 +0000 (12:16 +0000)
getFeatureConfigSnapshotHolders() logging is now similar
to what XmlDirectoryPersister.fromXmlSnapshot() does.

Also, files not ending with .xml (and not available files)
are filtered out as they do not hold configuration snapshots.

Warn level for config load failure.

Change-Id: I7c9d472bbe0ad031aaf7cc8dead1fd1e41d93306
Signed-off-by: Vratko Polak <vrpolak@cisco.com>
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/AbstractFeatureWrapper.java
opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/ChildAwareFeatureWrapper.java
opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigSnapshotHolder.java

index d18928d06c1f09ac9dd3af62846a8bcf9b08a07d..4f598aae80e2ac2678902e8367f3bce9fb7950ba 100644 (file)
@@ -7,10 +7,13 @@
  */
 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 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;
@@ -28,6 +31,9 @@ import org.slf4j.LoggerFactory;
  */
 public class AbstractFeatureWrapper implements Feature {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractFeatureWrapper.class);
+
+    protected static final String CONFIG_FILE_SUFFIX = "xml";
+
     protected Feature feature = null;
 
     protected AbstractFeatureWrapper() {
@@ -47,17 +53,35 @@ public class AbstractFeatureWrapper implements Feature {
      * from the underlying Feature Config files
      */
     public LinkedHashSet<FeatureConfigSnapshotHolder> getFeatureConfigSnapshotHolders() throws Exception {
-        final LinkedHashSet <FeatureConfigSnapshotHolder> snapShotHolders = new LinkedHashSet<FeatureConfigSnapshotHolder>();
+        final LinkedHashSet <FeatureConfigSnapshotHolder> snapShotHolders = new LinkedHashSet<>();
         for(final ConfigFileInfo c: getConfigurationFiles()) {
-            try {
-                snapShotHolders.add(new FeatureConfigSnapshotHolder(c,this));
-            } catch (final JAXBException e) {
-                LOG.debug("{} is not a config subsystem config file",c.getFinalname());
+            // 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;
index 4ea3c5ff752968265d9f9513ba45dbb927a4e652..2cb53ca560ec541cb325fb57058b57e2185539de 100644 (file)
@@ -7,10 +7,10 @@
  */
 package org.opendaylight.controller.configpusherfeature.internal;
 
+import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import java.util.LinkedHashSet;
 import java.util.List;
-import javax.xml.bind.JAXBException;
 import org.apache.felix.utils.version.VersionRange;
 import org.apache.felix.utils.version.VersionTable;
 import org.apache.karaf.features.Dependency;
@@ -71,12 +71,9 @@ public class ChildAwareFeatureWrapper extends AbstractFeatureWrapper implements
         LinkedHashSet <FeatureConfigSnapshotHolder> snapShotHolders = new LinkedHashSet<FeatureConfigSnapshotHolder>();
         for(ChildAwareFeatureWrapper c: getChildFeatures()) {
             for(FeatureConfigSnapshotHolder h: c.getFeatureConfigSnapshotHolders()) {
-                FeatureConfigSnapshotHolder f;
-                try {
-                    f = new FeatureConfigSnapshotHolder(h,this);
-                    snapShotHolders.add(f);
-                } catch (JAXBException e) {
-                    LOG.debug("{} is not a config subsystem config file",h.getFileInfo().getFinalname());
+                final Optional<FeatureConfigSnapshotHolder> featureConfigSnapshotHolder = getFeatureConfigSnapshotHolder(h.getFileInfo());
+                if(featureConfigSnapshotHolder.isPresent()) {
+                    snapShotHolders.add(featureConfigSnapshotHolder.get());
                 }
             }
         }
index 518716cfa75b2043b189970719fd80fc8442701a..15c14daea576c29bc78cb01c076f97d6cd368fa9 100644 (file)
@@ -46,7 +46,7 @@ public class FeatureConfigSnapshotHolder implements ConfigSnapshotHolder {
      * @param holder - FeatureConfigSnapshotHolder that we
      * @param feature - new
      */
-    public FeatureConfigSnapshotHolder(final FeatureConfigSnapshotHolder holder, final Feature feature) throws JAXBException {
+    public FeatureConfigSnapshotHolder(final FeatureConfigSnapshotHolder holder, final Feature feature) throws JAXBException, XMLStreamException {
         this(holder.fileInfo,holder.getFeature());
         this.featureChain.add(feature);
     }
@@ -57,7 +57,7 @@ public class FeatureConfigSnapshotHolder implements ConfigSnapshotHolder {
      * @param fileInfo - ConfigFileInfo to read into the ConfigSnapshot
      * @param feature - Feature the ConfigFileInfo was attached to
      */
-    public FeatureConfigSnapshotHolder(final ConfigFileInfo fileInfo, final Feature feature) throws JAXBException {
+    public FeatureConfigSnapshotHolder(final ConfigFileInfo fileInfo, final Feature feature) throws JAXBException, XMLStreamException {
         Preconditions.checkNotNull(fileInfo);
         Preconditions.checkNotNull(fileInfo.getFinalname());
         Preconditions.checkNotNull(feature);
@@ -69,12 +69,9 @@ public class FeatureConfigSnapshotHolder implements ConfigSnapshotHolder {
         XMLInputFactory xif = XMLInputFactory.newFactory();
         xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
         xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
-        try {
-            XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(new File(fileInfo.getFinalname())));
-            unmarshalled = ((ConfigSnapshot) um.unmarshal(xsr));
-        } catch (final XMLStreamException e) {
-            throw new JAXBException(e);
-        }
+
+        XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(new File(fileInfo.getFinalname())));
+        unmarshalled = ((ConfigSnapshot) um.unmarshal(xsr));
     }
     /*
      * (non-Javadoc)