OSDN Git Service

Remove AdtPlugin dependency from AttrsXmlParser.
authorRaphael Moll <ralf@android.com>
Thu, 18 Nov 2010 00:03:09 +0000 (16:03 -0800)
committerRaphael Moll <ralf@android.com>
Thu, 18 Nov 2010 00:53:14 +0000 (16:53 -0800)
The AdtPluin was used just for logging. Instead the
AttrsXmlParser takes an ILogger (AdtPlug implements
ILogger and can be used directly in unit tests too).

For unit tests there is a new StdSdkLog convenience
class that prints to stdout/stderr (formerly MockStdLogger
from the Sdk Manager was doing that.)

Change-Id: I658af61d04efb19ad6e3bf9c0bf471452372885a

eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/resources/platform/AttrsXmlParser.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/resources/platform/DeclareStyleableInfo.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/resources/platform/ViewClassInfo.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/sdk/AndroidTargetParser.java
eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/common/resources/platform/AttrsXmlParserManifestTest.java
eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/common/resources/platform/AttrsXmlParserTest.java
eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/sdk/LayoutParamsParserTest.java
sdkmanager/app/tests/com/android/sdkmanager/CommandLineProcessorTest.java
sdkmanager/app/tests/com/android/sdkmanager/SdkCommandLineTest.java
sdkmanager/libs/sdklib/src/com/android/sdklib/ISdkLog.java
sdkmanager/libs/sdklib/src/com/android/sdklib/StdSdkLog.java [moved from sdkmanager/app/tests/com/android/sdkmanager/MockStdLogger.java with 78% similarity]

index 85b3c76..deacd7a 100644 (file)
 package com.android.ide.common.resources.platform;
 
 import com.android.ide.common.api.IAttributeInfo.Format;
+import com.android.ide.common.log.ILogger;
 import com.android.ide.common.resources.platform.ViewClassInfo.LayoutParamsInfo;
-import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.adt.internal.editors.descriptors.DescriptorsUtils;
 import com.android.ide.eclipse.adt.internal.editors.manifest.descriptors.AndroidManifestDescriptors;
 
-import org.eclipse.core.runtime.IStatus;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 import org.xml.sax.SAXException;
@@ -47,6 +46,7 @@ public final class AttrsXmlParser {
 
     private Document mDocument;
     private String mOsAttrsXmlPath;
+
     // all attributes that have the same name are supposed to have the same
     // parameters so we'll keep a cache of them to avoid processing them twice.
     private HashMap<String, AttributeInfo> mAttributeMap;
@@ -55,30 +55,56 @@ public final class AttrsXmlParser {
     private final Map<String, DeclareStyleableInfo> mStyleMap =
         new HashMap<String, DeclareStyleableInfo>();
 
-    /** Map of all (constant, value) pairs for attributes of format enum or flag.
+    /**
+     * Map of all (constant, value) pairs for attributes of format enum or flag.
      * E.g. for attribute name=gravity, this tells us there's an enum/flag called "center"
      * with value 0x11.
      */
     private Map<String, Map<String, Integer>> mEnumFlagValues;
 
+    /**
+     * A logger object. Must not be null.
+     */
+    private final ILogger mLog;
+
 
     /**
      * Creates a new {@link AttrsXmlParser}, set to load things from the given
      * XML file. Nothing has been parsed yet. Callers should call {@link #preload()}
      * next.
+     *
+     * @param osAttrsXmlPath The path of the <code>attrs.xml</code> file to parse.
+     *              Must not be null. Should point to an existing valid XML document.
+     * @param log A logger object. Must not be null.
      */
-    public AttrsXmlParser(String osAttrsXmlPath) {
-        this(osAttrsXmlPath, null /* inheritableAttributes */);
+    public AttrsXmlParser(String osAttrsXmlPath, ILogger log) {
+        this(osAttrsXmlPath, null /* inheritableAttributes */, log);
     }
 
     /**
      * Creates a new {@link AttrsXmlParser} set to load things from the given
-     * XML file. If inheritableAttributes is non-null, it must point to a preloaded
+     * XML file.
+     * <p/>
+     * If inheritableAttributes is non-null, it must point to a preloaded
      * {@link AttrsXmlParser} which attributes will be used for this one. Since
      * already defined attributes are not modifiable, they are thus "inherited".
+     *
+     * @param osAttrsXmlPath The path of the <code>attrs.xml</code> file to parse.
+     *              Must not be null. Should point to an existing valid XML document.
+     * @param inheritableAttributes An optional parser with attributes to inherit. Can be null.
+     *              If not null, the parser must have had its {@link #preload()} method
+     *              invoked prior to being used here.
+     * @param log A logger object. Must not be null.
      */
-    public AttrsXmlParser(String osAttrsXmlPath, AttrsXmlParser inheritableAttributes) {
+    public AttrsXmlParser(
+            String osAttrsXmlPath,
+            AttrsXmlParser inheritableAttributes,
+            ILogger log) {
         mOsAttrsXmlPath = osAttrsXmlPath;
+        mLog = log;
+
+        assert osAttrsXmlPath != null;
+        assert log != null;
 
         if (inheritableAttributes == null) {
             mAttributeMap = new HashMap<String, AttributeInfo>();
@@ -91,7 +117,7 @@ public final class AttrsXmlParser {
     }
 
     /**
-     * @return The OS path of the attrs.xml file parsed
+     * Returns the OS path of the attrs.xml file parsed.
      */
     public String getOsAttrsXmlPath() {
         return mOsAttrsXmlPath;
@@ -106,7 +132,7 @@ public final class AttrsXmlParser {
         Document doc = getDocument();
 
         if (doc == null) {
-            AdtPlugin.log(IStatus.WARNING, "Failed to find %1$s", //$NON-NLS-1$
+            mLog.warning("Failed to find %1$s", //$NON-NLS-1$
                     mOsAttrsXmlPath);
             return this;
         }
@@ -119,7 +145,7 @@ public final class AttrsXmlParser {
         }
 
         if (res == null) {
-            AdtPlugin.log(IStatus.WARNING, "Failed to find a <resources> node in %1$s", //$NON-NLS-1$
+            mLog.warning("Failed to find a <resources> node in %1$s", //$NON-NLS-1$
                     mOsAttrsXmlPath);
             return this;
         }
@@ -161,7 +187,7 @@ public final class AttrsXmlParser {
     }
 
     /**
-     * Returns a list of all decleare-styleable found in the xml file.
+     * Returns a list of all <code>decleare-styleable</code> found in the XML file.
      */
     public Map<String, DeclareStyleableInfo> getDeclareStyleableList() {
         return Collections.unmodifiableMap(mStyleMap);
@@ -189,13 +215,13 @@ public final class AttrsXmlParser {
                 DocumentBuilder builder = factory.newDocumentBuilder();
                 mDocument = builder.parse(new File(mOsAttrsXmlPath));
             } catch (ParserConfigurationException e) {
-                AdtPlugin.log(e, "Failed to create XML document builder for %1$s", //$NON-NLS-1$
+                mLog.error(e, "Failed to create XML document builder for %1$s", //$NON-NLS-1$
                         mOsAttrsXmlPath);
             } catch (SAXException e) {
-                AdtPlugin.log(e, "Failed to parse XML document %1$s", //$NON-NLS-1$
+                mLog.error(e, "Failed to parse XML document %1$s", //$NON-NLS-1$
                         mOsAttrsXmlPath);
             } catch (IOException e) {
-                AdtPlugin.log(e, "Failed to read XML document %1$s", //$NON-NLS-1$
+                mLog.error(e, "Failed to read XML document %1$s", //$NON-NLS-1$
                         mOsAttrsXmlPath);
             }
         }
@@ -203,7 +229,8 @@ public final class AttrsXmlParser {
     }
 
     /**
-     * Finds all the <declare-styleable> and <attr> nodes in the top <resources> node.
+     * Finds all the &lt;declare-styleable&gt; and &lt;attr&gt; nodes
+     * in the top &lt;resources&gt; node.
      */
     private void parseResources(Node res) {
 
@@ -438,8 +465,9 @@ public final class AttrsXmlParser {
                         formats.add(format);
                     }
                 } catch (IllegalArgumentException e) {
-                    AdtPlugin.log(e, "Unknown format name '%s' in <attr name=\"%s\">, file '%s'.", //$NON-NLS-1$
-                            f, name, getOsAttrsXmlPath());
+                    mLog.error(e,
+                        "Unknown format name '%s' in <attr name=\"%s\">, file '%s'.", //$NON-NLS-1$
+                        f, name, getOsAttrsXmlPath());
                 }
             }
         }
@@ -487,7 +515,7 @@ public final class AttrsXmlParser {
             if (child.getNodeType() == Node.ELEMENT_NODE && child.getNodeName().equals(filter)) {
                 Node nameNode = child.getAttributes().getNamedItem("name");  //$NON-NLS-1$
                 if (nameNode == null) {
-                    AdtPlugin.log(IStatus.WARNING,
+                    mLog.warning(
                             "Missing name attribute in <attr name=\"%s\"><%s></attr>", //$NON-NLS-1$
                             attrName, filter);
                 } else {
@@ -499,9 +527,9 @@ public final class AttrsXmlParser {
 
                     Node valueNode = child.getAttributes().getNamedItem("value");  //$NON-NLS-1$
                     if (valueNode == null) {
-                        AdtPlugin.log(IStatus.WARNING,
-                                "Missing value attribute in <attr name=\"%s\"><%s name=\"%s\"></attr>", //$NON-NLS-1$
-                                attrName, filter, name);
+                        mLog.warning(
+                            "Missing value attribute in <attr name=\"%s\"><%s name=\"%s\"></attr>", //$NON-NLS-1$
+                            attrName, filter, name);
                     } else {
                         String value = valueNode.getNodeValue();
                         try {
@@ -517,7 +545,7 @@ public final class AttrsXmlParser {
                             map.put(name, Integer.valueOf(i));
 
                         } catch(NumberFormatException e) {
-                            AdtPlugin.log(e,
+                            mLog.error(e,
                                     "Value in <attr name=\"%s\"><%s name=\"%s\" value=\"%s\"></attr> is not a valid decimal or hexadecimal", //$NON-NLS-1$
                                     attrName, filter, name, value);
                         }
index 56c047f..0d5ad23 100644 (file)
@@ -136,14 +136,16 @@ public final class AndroidTargetParser {
             // gather the attribute definition
             progress.subTask("Attributes definitions");
             AttrsXmlParser attrsXmlParser = new AttrsXmlParser(
-                    mAndroidTarget.getPath(IAndroidTarget.ATTRIBUTES));
+                    mAndroidTarget.getPath(IAndroidTarget.ATTRIBUTES),
+                    AdtPlugin.getDefault());
             attrsXmlParser.preload();
             progress.worked(1);
 
             progress.subTask("Manifest definitions");
             AttrsXmlParser attrsManifestXmlParser = new AttrsXmlParser(
                     mAndroidTarget.getPath(IAndroidTarget.MANIFEST_ATTRIBUTES),
-                    attrsXmlParser);
+                    attrsXmlParser,
+                    AdtPlugin.getDefault());
             attrsManifestXmlParser.preload();
             progress.worked(1);
 
index 3033275..b10f68d 100755 (executable)
@@ -16,6 +16,7 @@
 
 package com.android.ide.common.resources.platform;
 
+import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.tests.AdtTestData;
 
 import java.util.Arrays;
@@ -35,7 +36,7 @@ public class AttrsXmlParserManifestTest extends TestCase {
     @Override
     public void setUp() throws Exception {
         mFilePath = AdtTestData.getInstance().getTestFilePath(MOCK_DATA_PATH); //$NON-NLS-1$
-        mParser = new AttrsXmlParser(mFilePath);
+        mParser = new AttrsXmlParser(mFilePath, AdtPlugin.getDefault());
     }
 
     @Override
index 39ed330..3e47c35 100644 (file)
@@ -18,6 +18,7 @@ package com.android.ide.common.resources.platform;
 
 
 import com.android.ide.common.api.IAttributeInfo.Format;
+import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.tests.AdtTestData;
 
 import java.util.Map;
@@ -35,7 +36,7 @@ public class AttrsXmlParserTest extends TestCase {
     @Override
     public void setUp() throws Exception {
         mFilePath = AdtTestData.getInstance().getTestFilePath(MOCK_DATA_PATH); //$NON-NLS-1$
-        mParser = new AttrsXmlParser(mFilePath);
+        mParser = new AttrsXmlParser(mFilePath, AdtPlugin.getDefault());
     }
 
     @Override
index d3ebc57..42f2455 100644 (file)
@@ -19,6 +19,7 @@ package com.android.ide.eclipse.adt.internal.sdk;
 import com.android.ide.common.resources.platform.AttrsXmlParser;
 import com.android.ide.common.resources.platform.ViewClassInfo;
 import com.android.ide.common.resources.platform.ViewClassInfo.LayoutParamsInfo;
+import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.adt.internal.sdk.AndroidJarLoader.ClassWrapper;
 import com.android.ide.eclipse.adt.internal.sdk.IAndroidClassLoader.IClassDescriptor;
 import com.android.ide.eclipse.tests.AdtTestData;
@@ -60,7 +61,8 @@ public class LayoutParamsParserTest extends TestCase {
         public MockLayoutParamsParser() {
             super(new MockFrameworkClassLoader(),
                   new AttrsXmlParser(
-                          AdtTestData.getInstance().getTestFilePath(MOCK_DATA_PATH)).preload());
+                          AdtTestData.getInstance().getTestFilePath(MOCK_DATA_PATH),
+                          AdtPlugin.getDefault()).preload());
 
             mTopViewClass = new ClassWrapper(mock_android.view.View.class);
             mTopGroupClass = new ClassWrapper(mock_android.view.ViewGroup.class);
index a213652..688ce52 100644 (file)
 package com.android.sdkmanager;
 
 import com.android.sdklib.ISdkLog;
+import com.android.sdklib.StdSdkLog;
 
 import junit.framework.TestCase;
 
 
 public class CommandLineProcessorTest extends TestCase {
 
-    private MockStdLogger mLog;
+    private StdSdkLog mLog;
     
     /**
      * A mock version of the {@link CommandLineProcessor} class that does not
@@ -92,7 +93,7 @@ public class CommandLineProcessorTest extends TestCase {
 
     @Override
     protected void setUp() throws Exception {
-        mLog = new MockStdLogger();
+        mLog = new StdSdkLog();
         super.setUp();
     }
 
index 07a32e0..8206e2a 100644 (file)
 package com.android.sdkmanager;
 
 import com.android.sdklib.ISdkLog;
+import com.android.sdklib.StdSdkLog;
 
 import junit.framework.TestCase;
 
 public class SdkCommandLineTest extends TestCase {
 
-    private MockStdLogger mLog;
+    private StdSdkLog mLog;
     
     /**
      * A mock version of the {@link SdkCommandLine} class that does not
@@ -69,7 +70,7 @@ public class SdkCommandLineTest extends TestCase {
 
     @Override
     protected void setUp() throws Exception {
-        mLog = new MockStdLogger();
+        mLog = new StdSdkLog();
         super.setUp();
     }
 
index 163f7a9..ec22177 100644 (file)
@@ -20,9 +20,26 @@ import java.util.Formatter;
 
 /**
  * Interface used to display warnings/errors while parsing the SDK content.
+ * <p/>
+ * There are a few default implementations available:
+ * <ul>
+ * <li> {@link NullSdkLog} is an implementation that does <em>nothing</em> with the log.
+ *  Useful for limited cases where you need to call a class that requires a non-null logging
+ *  yet the calling code does not have any mean of reporting logs itself. It can be
+ *  acceptable for use a temporary implementation but most of the time that means the caller
+ *  code needs to be reworked to take a logger object from its own caller.
+ * </li>
+ * <li> {@link StdSdkLog} is an implementation that dumps the log to {@link System#out} or
+ *  {@link System#err}. This is useful for unit tests or code that does not have any GUI.
+ *  Apps based on Eclipse or SWT should not use it and should provide a better way to report
+ *  to the user.
+ * </li>
+ * <li> ADT has a <code>AdtPlugin</code> which implements a similar interface called
+ *      <code>ILogger</code>, useful in case we don't want to pull the whole SdkLib.
+ * </ul>
  */
 public interface ISdkLog {
-    
+
     /**
      * Prints a warning message on stdout.
      * <p/>
@@ -30,13 +47,13 @@ public interface ISdkLog {
      * need to put such a prefix in the format string.
      * <p/>
      * Implementations should only display warnings in verbose mode.
-     * 
+     *
      * @param warningFormat is an optional error format. If non-null, it will be printed
      *          using a {@link Formatter} with the provided arguments.
      * @param args provides the arguments for warningFormat.
      */
     void warning(String warningFormat, Object... args);
-    
+
     /**
      * Prints an error message on stderr.
      * <p/>
@@ -44,7 +61,7 @@ public interface ISdkLog {
      * need to put such a prefix in the format string.
      * <p/>
      * Implementation should always display errors, independent of verbose mode.
-     * 
+     *
      * @param t is an optional {@link Throwable} or {@link Exception}. If non-null, it's
      *          message will be printed out.
      * @param errorFormat is an optional error format. If non-null, it will be printed
@@ -52,13 +69,13 @@ public interface ISdkLog {
      * @param args provides the arguments for errorFormat.
      */
     void error(Throwable t, String errorFormat, Object... args);
-    
+
     /**
      * Prints a message as-is on stdout.
      * <p/>
-     * Implementation should always display errors, independent of verbose mode.
+     * Implementation can omit printing such messages when not in verbose mode.
      * No prefix is used, the message is printed as-is after formatting.
-     * 
+     *
      * @param msgFormat is an optional error format. If non-null, it will be printed
      *          using a {@link Formatter} with the provided arguments.
      * @param args provides the arguments for msgFormat.
@@ -4,7 +4,7 @@
  * Licensed under the Eclipse Public License, Version 1.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.eclipse.org/org/documents/epl-v10.php
  *
  * Unless required by applicable law or agreed to in writing, software
  * limitations under the License.
  */
 
-package com.android.sdkmanager;
+package com.android.sdklib;
 
-import com.android.sdklib.ISdkLog;
 
 /**
- * 
+ * An implementation of {@link ISdkLog} that prints to {@link System#out} and {@link System#err}.
+ * <p/>
+ * This is mostly useful for unit tests. It should not be used by GUI-based tools (e.g.
+ * Eclipse plugin or SWT-based apps) which should have a better way to expose their logging
+ * error and warnings.
  */
-public class MockStdLogger implements ISdkLog {
+public class StdSdkLog implements ISdkLog {
 
     public void error(Throwable t, String errorFormat, Object... args) {
         if (errorFormat != null) {