ace-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r1538937 - in /ace/trunk/org.apache.ace.client.repository: ./ src/org/apache/ace/client/repository/helper/configuration/impl/ test/ test/org/apache/ace/client/repository/helper/configuration/impl/
Date Tue, 05 Nov 2013 10:52:39 GMT
Author: jawi
Date: Tue Nov  5 10:52:38 2013
New Revision: 1538937

URL: http://svn.apache.org/r1538937
Log:
ACE-113 - use proper error handler for configuration helper:

- instead of letting the default error handler pollute the standard
  out/error, log all parsing exceptions to the log service.


Added:
    ace/trunk/org.apache.ace.client.repository/test/invalid.txt   (with props)
    ace/trunk/org.apache.ace.client.repository/test/invalid.xml   (with props)
Modified:
    ace/trunk/org.apache.ace.client.repository/   (props changed)
    ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/Activator.java
    ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImpl.java
    ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImplTest.java

Propchange: ace/trunk/org.apache.ace.client.repository/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Tue Nov  5 10:52:38 2013
@@ -4,3 +4,4 @@ generated
 store
 bundle-cache
 felix-cache
+test-output

Modified: ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/Activator.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/Activator.java?rev=1538937&r1=1538936&r2=1538937&view=diff
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/Activator.java
(original)
+++ ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/Activator.java
Tue Nov  5 10:52:38 2013
@@ -29,6 +29,7 @@ import org.apache.ace.connectionfactory.
 import org.apache.felix.dm.DependencyActivatorBase;
 import org.apache.felix.dm.DependencyManager;
 import org.osgi.framework.BundleContext;
+import org.osgi.service.log.LogService;
 
 /**
  * Activator class for the Configuration ArtifactHelper.
@@ -47,6 +48,9 @@ public class Activator extends Dependenc
             .add(createServiceDependency()
                 .setService(ConnectionFactory.class)
                 .setRequired(true))
+            .add(createServiceDependency()
+                .setService(LogService.class)
+                .setRequired(false))
             );
     }
 
@@ -54,4 +58,4 @@ public class Activator extends Dependenc
     public synchronized void destroy(BundleContext context, DependencyManager manager) throws
Exception {
         // Nothing to do
     }
-}
\ No newline at end of file
+}

Modified: ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImpl.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImpl.java?rev=1538937&r1=1538936&r2=1538937&view=diff
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImpl.java
(original)
+++ ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImpl.java
Tue Nov  5 10:52:38 2013
@@ -21,6 +21,8 @@ package org.apache.ace.client.repository
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.net.URL;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Map;
@@ -35,21 +37,97 @@ import org.apache.ace.client.repository.
 import org.apache.ace.client.repository.helper.configuration.ConfigurationHelper;
 import org.apache.ace.client.repository.object.ArtifactObject;
 import org.apache.ace.connectionfactory.ConnectionFactory;
+import org.osgi.service.log.LogService;
 import org.xml.sax.Attributes;
+import org.xml.sax.ErrorHandler;
+import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.XMLReader;
 import org.xml.sax.helpers.DefaultHandler;
 
 public class ConfigurationHelperImpl implements ArtifactRecognizer, ConfigurationHelper {
 
+    static class MetaDataNamespaceCollector extends DefaultHandler {
+        private String m_metaDataNameSpace = "";
+
+        public String getMetaDataNamespace() {
+            return m_metaDataNameSpace;
+        }
+
+        @Override
+        public void startElement(String uri, String localName, String qName, Attributes attributes)
throws SAXException {
+            if (qName.equals("MetaData") || qName.endsWith(":MetaData")) {
+                String nsAttributeQName = "xmlns";
+                if (qName.endsWith(":MetaData")) {
+                    nsAttributeQName = "xmlns" + ":" + qName.split(":")[0];
+                }
+                for (int i = 0; i < attributes.getLength(); i++) {
+                    if (attributes.getQName(i).equals(nsAttributeQName)) {
+                        m_metaDataNameSpace = attributes.getValue(i);
+                    }
+                }
+            }
+            // first element is expected to have been the MetaData
+            // root so we can now terminate processing.
+            throw new SAXException("Done");
+        }
+    }
+
+    static class LoggingErrorHandler implements ErrorHandler {
+        private final ArtifactResource m_resource;
+        private final LogService m_log;
+
+        public LoggingErrorHandler(ArtifactResource resource, LogService log) {
+            m_resource = resource;
+            m_log = log;
+        }
+
+        @Override
+        public void warning(SAXParseException exception) throws SAXException {
+            log(LogService.LOG_WARNING, "Artifact '" + getName() + "' contains a warning!",
exception);
+        }
+
+        @Override
+        public void error(SAXParseException exception) throws SAXException {
+            log(LogService.LOG_ERROR, "Artifact '" + getName() + "' contains an error!",
exception);
+        }
+
+        @Override
+        public void fatalError(SAXParseException exception) throws SAXException {
+            log(LogService.LOG_ERROR, "Artifact '" + getName() + "' contains a fatal error!",
exception);
+        }
+
+        private String getName() {
+            URL url = m_resource.getURL();
+            try {
+                if ("file".equals(url.getProtocol())) {
+                    return new File(url.toURI()).getName();
+                }
+            }
+            catch (URISyntaxException exception) {
+                // Ignore; fall through to return complete name...
+            }
+            return url.getFile();
+        }
+
+        private void log(int level, String msg, Exception exception) {
+            if (m_log != null) {
+                m_log.log(level, msg.concat(" ").concat(exception.getMessage()), exception);
+            }
+        }
+    }
+
     // known valid metatype namespaces
     private static final String NAMESPACE_1_0 = "http://www.osgi.org/xmlns/metatype/v1.0.0";
     private static final String NAMESPACE_1_1 = "http://www.osgi.org/xmlns/metatype/v1.1.0";
     private static final String NAMESPACE_1_2 = "http://www.osgi.org/xmlns/metatype/v1.2.0";
 
     private final SAXParserFactory m_saxParserFactory;
-
     // Injected by Dependency Manager
     private volatile ConnectionFactory m_connectionFactory;
+    private volatile LogService m_log;
+
     // Created in #start()
     private volatile VelocityArtifactPreprocessor m_artifactPreprocessor;
 
@@ -58,11 +136,20 @@ public class ConfigurationHelperImpl imp
         m_saxParserFactory.setNamespaceAware(false);
         m_saxParserFactory.setValidating(false);
     }
-    
+
     public boolean canHandle(String mimetype) {
         return MIMETYPE.equals(mimetype);
     }
 
+    public boolean canUse(ArtifactObject object) {
+        return MIMETYPE.equals(object.getMimetype());
+    }
+
+    public Map<String, String> checkAttributes(Map<String, String> attributes)
{
+        // All necessary checks will be done by the constructor using getMandatoryAttributes.
+        return attributes;
+    }
+
     public Map<String, String> extractMetaData(ArtifactResource artifact) throws IllegalArgumentException
{
         Map<String, String> result = new HashMap<String, String>();
         result.put(ArtifactObject.KEY_PROCESSOR_PID, PROCESSOR);
@@ -79,43 +166,6 @@ public class ConfigurationHelperImpl imp
         return result;
     }
 
-    public String recognize(ArtifactResource artifact) {
-        MetaDataNamespaceCollector handler = new MetaDataNamespaceCollector();
-        InputStream input = null;
-        try {
-            input = artifact.openStream();
-            SAXParser parser = m_saxParserFactory.newSAXParser();
-            parser.parse(input, handler);
-        }
-        catch (Exception e) {
-            String namespace = handler.getMetaDataNamespace();
-            if (namespace != null
-                && (namespace.equals(NAMESPACE_1_0)
-                    || namespace.equals(NAMESPACE_1_1)
-                    || namespace.equals(NAMESPACE_1_2))) {
-                return MIMETYPE;
-            }
-        }
-        finally {
-            if (input != null) {
-                try {
-                    input.close();
-                }
-                catch (IOException e) {}
-            }
-        }
-        return null;
-    }
-
-    public boolean canUse(ArtifactObject object) {
-        return MIMETYPE.equals(object.getMimetype());
-    }
-
-    public Map<String, String> checkAttributes(Map<String, String> attributes)
{
-        // All necessary checks will be done by the constructor using getMandatoryAttributes.
-        return attributes;
-    }
-
     public <TYPE extends ArtifactObject> String getAssociationFilter(TYPE obj, Map<String,
String> properties) {
         return "(" + KEY_FILENAME + "=" + obj.getAttribute(KEY_FILENAME) + ")";
     }
@@ -129,19 +179,49 @@ public class ConfigurationHelperImpl imp
     }
 
     public String[] getDefiningKeys() {
-        return new String[] {KEY_FILENAME};
+        return new String[] { KEY_FILENAME };
+    }
+
+    public String getExtension(ArtifactResource artifact) {
+        return ".xml";
     }
 
     public String[] getMandatoryAttributes() {
-        return new String[] {KEY_FILENAME};
+        return new String[] { KEY_FILENAME };
     }
 
     public ArtifactPreprocessor getPreprocessor() {
         return m_artifactPreprocessor;
     }
-    
-    public String getExtension(ArtifactResource artifact) {
-        return ".xml";
+
+    public String recognize(ArtifactResource artifact) {
+        MetaDataNamespaceCollector handler = new MetaDataNamespaceCollector();
+        InputStream input = null;
+        try {
+            input = artifact.openStream();
+            SAXParser parser = m_saxParserFactory.newSAXParser();
+
+            XMLReader reader = parser.getXMLReader();
+            reader.setErrorHandler(new LoggingErrorHandler(artifact, m_log));
+            reader.setContentHandler(handler);
+            reader.parse(new InputSource(input));
+        }
+        catch (Exception e) {
+            String namespace = handler.getMetaDataNamespace();
+            if (NAMESPACE_1_0.equals(namespace) || NAMESPACE_1_1.equals(namespace) || NAMESPACE_1_2.equals(namespace))
{
+                return MIMETYPE;
+            }
+        }
+        finally {
+            if (input != null) {
+                try {
+                    input.close();
+                }
+                catch (IOException e) {
+                }
+            }
+        }
+        return null;
     }
 
     /**
@@ -156,34 +236,13 @@ public class ConfigurationHelperImpl imp
      */
     protected void stop() {
         m_artifactPreprocessor = null;
-        
     }
 
-    static class MetaDataNamespaceCollector extends DefaultHandler {
-
-        private String m_metaDataNameSpace = "";
-
-        public String getMetaDataNamespace() {
-            return m_metaDataNameSpace;
-        }
-
-        @Override
-        public void startElement(String uri, String localName, String qName, Attributes attributes)
-            throws SAXException {
-            if (qName.equals("MetaData") || qName.endsWith(":MetaData")) {
-                String nsAttributeQName = "xmlns";
-                if (qName.endsWith(":MetaData")) {
-                    nsAttributeQName = "xmlns" + ":" + qName.split(":")[0];
-                }
-                for (int i = 0; i < attributes.getLength(); i++) {
-                    if (attributes.getQName(i).equals(nsAttributeQName)) {
-                        m_metaDataNameSpace = attributes.getValue(i);
-                    }
-                }
-            }
-            // first element is expected to have been the MetaData
-            // root so we can now terminate processing.
-            throw new SAXException("Done");
-        }
+    /**
+     * @param log
+     *            the log service to set, can be <code>null</code>.
+     */
+    final void setLog(LogService log) {
+        m_log = log;
     }
-}
\ No newline at end of file
+}

Added: ace/trunk/org.apache.ace.client.repository/test/invalid.txt
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/test/invalid.txt?rev=1538937&view=auto
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/test/invalid.txt (added)
+++ ace/trunk/org.apache.ace.client.repository/test/invalid.txt Tue Nov  5 10:52:38 2013
@@ -0,0 +1 @@
+hello world
\ No newline at end of file

Propchange: ace/trunk/org.apache.ace.client.repository/test/invalid.txt
------------------------------------------------------------------------------
    svn:eol-style = native

Added: ace/trunk/org.apache.ace.client.repository/test/invalid.xml
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/test/invalid.xml?rev=1538937&view=auto
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/test/invalid.xml (added)
+++ ace/trunk/org.apache.ace.client.repository/test/invalid.xml Tue Nov  5 10:52:38 2013
@@ -0,0 +1,12 @@
+<my MetaData>
+  <OCD name="Apache Felix Http Config" id="org.osgi.service.http">
+    <AD id="org.osgi.service.http.port" type="STRING" cardinality="0" />
+  </OCD>
+  <Designate pid="org.apache.felix.http" bundle="*">
+    <Object ocdref="org.osgi.service.http">
+      <Attribute adref="org.osgi.service.http.port">
+        <Value>8080</Value>
+      </Attribute>
+    </Object>
+  </Designate>
+</my MetaData>

Propchange: ace/trunk/org.apache.ace.client.repository/test/invalid.xml
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImplTest.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImplTest.java?rev=1538937&r1=1538936&r2=1538937&view=diff
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImplTest.java
(original)
+++ ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/helper/configuration/impl/ConfigurationHelperImplTest.java
Tue Nov  5 10:52:38 2013
@@ -19,78 +19,124 @@
 package org.apache.ace.client.repository.helper.configuration.impl;
 
 import static org.apache.ace.test.utils.TestUtils.UNIT;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 
 import org.apache.ace.client.repository.helper.ArtifactResource;
+import org.osgi.service.log.LogService;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
 public class ConfigurationHelperImplTest {
+    private LogService m_mockLogService;
 
     // ACE-259 Basic recognizer tests
 
     @Test(groups = { UNIT })
+    public void testCanHandleCommentBeforeRoot() throws Exception {
+        ConfigurationHelperImpl c = createConfigurationHelper();
+        String mime = c.recognize(convertToArtifactResource("validWithComment.xml"));
+        Assert.assertNotNull(mime);
+
+        verifyZeroInteractions(m_mockLogService);
+    }
+
+    @Test(groups = { UNIT })
+    public void testInvalidXmlContentNotRecognized() throws Exception {
+        ConfigurationHelperImpl c = createConfigurationHelper();
+        String mime = c.recognize(convertToArtifactResource("invalid.xml"));
+        Assert.assertNull(mime);
+
+        verify(m_mockLogService, times(1)).log(anyInt(), anyString(), any(Throwable.class));
+        verifyNoMoreInteractions(m_mockLogService);
+    }
+
+    @Test(groups = { UNIT })
     public void testNamespace10Recognized() throws Exception {
-        ConfigurationHelperImpl c = new ConfigurationHelperImpl();
+        ConfigurationHelperImpl c = createConfigurationHelper();
         String mime = c.recognize(convertToArtifactResource("valid10.xml"));
         Assert.assertNotNull(mime);
+
+        verifyZeroInteractions(m_mockLogService);
     }
 
     @Test(groups = { UNIT })
     public void testNamespace11Recognized() throws Exception {
-        ConfigurationHelperImpl c = new ConfigurationHelperImpl();
+        ConfigurationHelperImpl c = createConfigurationHelper();
         String mime = c.recognize(convertToArtifactResource("valid11.xml"));
         Assert.assertNotNull(mime);
+
+        verifyZeroInteractions(m_mockLogService);
     }
 
     @Test(groups = { UNIT })
     public void testNamespace12Recognized() throws Exception {
-        ConfigurationHelperImpl c = new ConfigurationHelperImpl();
+        ConfigurationHelperImpl c = createConfigurationHelper();
         String mime = c.recognize(convertToArtifactResource("valid12.xml"));
         Assert.assertNotNull(mime);
+
+        verifyZeroInteractions(m_mockLogService);
     }
 
     @Test(groups = { UNIT })
     public void testNamespace13NotRecognized() throws Exception {
-        ConfigurationHelperImpl c = new ConfigurationHelperImpl();
+        ConfigurationHelperImpl c = createConfigurationHelper();
         String mime = c.recognize(convertToArtifactResource("invalid13.xml"));
         Assert.assertNull(mime);
+
+        verifyZeroInteractions(m_mockLogService);
     }
 
     @Test(groups = { UNIT })
-    public void testCanHandleCommentBeforeRoot() throws Exception {
-        ConfigurationHelperImpl c = new ConfigurationHelperImpl();
-        String mime = c.recognize(convertToArtifactResource("validWithComment.xml"));
-        Assert.assertNotNull(mime);
+    public void testNoXmlContentNotRecognized() throws Exception {
+        ConfigurationHelperImpl c = createConfigurationHelper();
+        String mime = c.recognize(convertToArtifactResource("invalid.txt"));
+        Assert.assertNull(mime);
+
+        verify(m_mockLogService, times(1)).log(anyInt(), anyString(), any(Throwable.class));
+        verifyNoMoreInteractions(m_mockLogService);
     }
 
-    /**
-     * @param url
-     * @return
-     */
     private ArtifactResource convertToArtifactResource(final String res) {
         if (res == null) {
             return null;
         }
-        
+
         final URL url = getClass().getClassLoader().getResource("./" + res);
-        
         return new ArtifactResource() {
-            public URL getURL() {
-                return url;
-            }
-            
             @Override
             public long getSize() throws IOException {
                 return -1L;
             }
-            
+
+            public URL getURL() {
+                return url;
+            }
+
             public InputStream openStream() throws IOException {
                 return getURL().openStream();
             }
         };
     }
-}
\ No newline at end of file
+
+    /**
+     * @return a new {@link ConfigurationHelperImpl} instance, never <code>null</code>.
+     */
+    private ConfigurationHelperImpl createConfigurationHelper() {
+        m_mockLogService = mock(LogService.class);
+
+        ConfigurationHelperImpl c = new ConfigurationHelperImpl();
+        c.setLog(m_mockLogService);
+        return c;
+    }
+}



Mime
View raw message