Record and propagate namespaces from the <resources> element correctly.

* Reduces the size of merged values.xml
* Improves correctness of merged xml

Sadly, this is also backwards compatible by allowing multiple definitions of a prefix with different namespaces.
Will be cleaned up after transition.

--
MOS_MIGRATED_REVID=127481147

1 files changed, 56 insertions, 25 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
index a72d0633c7..f8b9982a0f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
@@ -16,22 +16,23 @@ package com.google.devtools.build.android;
 import static com.android.resources.ResourceType.DECLARE_STYLEABLE;
 import static com.android.resources.ResourceType.ID;
 
+import com.android.resources.ResourceType;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.android.FullyQualifiedName.Factory;
 import com.google.devtools.build.android.ParsedAndroidData.KeyValueConsumer;
 import com.google.devtools.build.android.proto.SerializeFormat;
+import com.google.devtools.build.android.proto.SerializeFormat.DataValueXml;
 import com.google.devtools.build.android.xml.ArrayXmlResourceValue;
 import com.google.devtools.build.android.xml.AttrXmlResourceValue;
 import com.google.devtools.build.android.xml.IdXmlResourceValue;
+import com.google.devtools.build.android.xml.Namespaces;
 import com.google.devtools.build.android.xml.PluralXmlResourceValue;
 import com.google.devtools.build.android.xml.SimpleXmlResourceValue;
 import com.google.devtools.build.android.xml.StyleXmlResourceValue;
 import com.google.devtools.build.android.xml.StyleableXmlResourceValue;
 import com.google.protobuf.InvalidProtocolBufferException;
-
-import com.android.resources.ResourceType;
-
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -40,7 +41,6 @@ import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Objects;
-
 import javax.xml.stream.FactoryConfigurationError;
 import javax.xml.stream.XMLEventReader;
 import javax.xml.stream.XMLInputFactory;
@@ -59,8 +59,8 @@ public class DataResourceXml implements DataResource {
   /**
    * Parses xml resources from a Path to the provided overwritable and combining collections.
    *
-   * This method is a bit tricky in the service of performance -- creating several collections and
-   * merging them was more expensive than writing to mutable collections directly.
+   * <p>This method is a bit tricky in the service of performance -- creating several collections
+   * and merging them was more expensive than writing to mutable collections directly.
    *
    * @param xmlInputFactory Used to create an XMLEventReader from the supplied resource path.
    * @param path The path to the xml resource to be parsed.
@@ -88,6 +88,7 @@ public class DataResourceXml implements DataResource {
         for (StartElement start = XmlResourceValues.findNextStart(eventReader);
             start != null;
             start = XmlResourceValues.findNextStart(eventReader)) {
+          Namespaces.Collector namespacesCollector = Namespaces.collector();
           if (XmlResourceValues.isEatComment(start) || XmlResourceValues.isSkip(start)) {
             continue;
           }
@@ -112,8 +113,12 @@ public class DataResourceXml implements DataResource {
                   start.getLocation());
             }
             FullyQualifiedName key = fqnFactory.create(resourceType, elementName);
+            XmlResourceValue xmlResourceValue =
+                parseXmlElements(resourceType, eventReader, start, namespacesCollector);
             consumer.consume(
-                key, DataResourceXml.of(path, parseXmlElements(resourceType, eventReader, start)));
+                key,
+                DataResourceXml.createWithNamespaces(
+                    path, xmlResourceValue, namespacesCollector.toNamespaces()));
           }
         }
       }
@@ -124,11 +129,14 @@ public class DataResourceXml implements DataResource {
     }
   }
 
+  @SuppressWarnings("deprecation")
   public static DataValue from(SerializeFormat.DataValue protoValue, FileSystem currentFileSystem)
       throws InvalidProtocolBufferException {
-    return of(
+    DataValueXml xmlValue = protoValue.getXmlValue();
+    return createWithNamespaces(
         currentFileSystem.getPath(protoValue.getSource().getFilename()),
-        valueFromProto(protoValue.getXmlValue()));
+        valueFromProto(xmlValue),
+        Namespaces.from(xmlValue.getNamespace()));
   }
 
   private static XmlResourceValue valueFromProto(SerializeFormat.DataValueXml proto)
@@ -155,23 +163,26 @@ public class DataResourceXml implements DataResource {
   }
 
   private static XmlResourceValue parseXmlElements(
-      ResourceType resourceType, XMLEventReader eventReader, StartElement start)
+      ResourceType resourceType,
+      XMLEventReader eventReader,
+      StartElement start,
+      Namespaces.Collector namespacesCollector)
       throws XMLStreamException {
     // Handle ids first, as they are a special kind of item.
     if (resourceType == ID) {
-      return XmlResourceValues.parseId(eventReader, start);
+      return XmlResourceValues.parseId(eventReader, start, namespacesCollector);
     }
     // Handle item stubs.
     if (XmlResourceValues.isItem(start)) {
-      return XmlResourceValues.parseSimple(eventReader, resourceType, start); 
+      return XmlResourceValues.parseSimple(eventReader, resourceType, start, namespacesCollector);
     }
     switch (resourceType) {
       case STYLE:
         return XmlResourceValues.parseStyle(eventReader, start);
       case ARRAY:
-        return ArrayXmlResourceValue.parseArray(eventReader, start);
+        return ArrayXmlResourceValue.parseArray(eventReader, start, namespacesCollector);
       case PLURALS:
-        return XmlResourceValues.parsePlurals(eventReader, start);
+        return XmlResourceValues.parsePlurals(eventReader, start, namespacesCollector);
       case ATTR:
         return XmlResourceValues.parseAttr(eventReader, start);
       case LAYOUT:
@@ -193,7 +204,7 @@ public class DataResourceXml implements DataResource {
       case STYLEABLE:
       case TRANSITION:
       case XML:
-        return XmlResourceValues.parseSimple(eventReader, resourceType, start);
+        return XmlResourceValues.parseSimple(eventReader, resourceType, start, namespacesCollector);
       default:
         throw new XMLStreamException(
             String.format("Unhandled resourceType %s", resourceType), start.getLocation());
@@ -207,16 +218,28 @@ public class DataResourceXml implements DataResource {
     return ResourceType.getEnum(start.getName().getLocalPart());
   }
 
-  private Path source;
-  private XmlResourceValue xml;
+  private final Path source;
+  private final XmlResourceValue xml;
+  private final Namespaces namespaces;
 
-  private DataResourceXml(Path source, XmlResourceValue xmlValue) {
+  private DataResourceXml(Path source, XmlResourceValue xmlValue, Namespaces namespaces) {
     this.source = source;
     this.xml = xmlValue;
+    this.namespaces = namespaces;
+  }
+
+  public static DataResourceXml createWithNoNamespace(Path source, XmlResourceValue xml) {
+    return createWithNamespaces(source, xml, ImmutableMap.<String, String>of());
+  }
+
+  public static DataResourceXml createWithNamespaces(
+      Path source, XmlResourceValue xml, ImmutableMap<String, String> prefixToUri) {
+    return createWithNamespaces(source, xml, Namespaces.from(prefixToUri));
   }
 
-  public static DataResourceXml of(Path source, XmlResourceValue xml) {
-    return new DataResourceXml(source, xml);
+  public static DataResourceXml createWithNamespaces(
+      Path source, XmlResourceValue xml, Namespaces namespaces) {
+    return new DataResourceXml(source, xml, namespaces);
   }
 
   @Override
@@ -226,7 +249,7 @@ public class DataResourceXml implements DataResource {
 
   @Override
   public int hashCode() {
-    return Objects.hash(source, xml);
+    return Objects.hash(source, xml, namespaces);
   }
 
   @Override
@@ -235,22 +258,29 @@ public class DataResourceXml implements DataResource {
       return false;
     }
     DataResourceXml other = (DataResourceXml) obj;
-    return Objects.equals(source, other.source) && Objects.equals(xml, other.xml);
+    return Objects.equals(source, other.source)
+        && Objects.equals(xml, other.xml)
+        && Objects.equals(namespaces, other.namespaces);
   }
 
   @Override
   public String toString() {
-    return MoreObjects.toStringHelper(getClass()).add("source", source).add("xml", xml).toString();
+    return MoreObjects.toStringHelper(getClass())
+        .add("source", source)
+        .add("xml", xml)
+        .add("namespaces", namespaces)
+        .toString();
   }
 
   @Override
   public void writeResource(FullyQualifiedName key, AndroidDataWritingVisitor mergedDataWriter) {
+    mergedDataWriter.defineNamespacesFor(key, namespaces);
     xml.write(key, source, mergedDataWriter);
   }
 
   @Override
   public int serializeTo(DataKey key, OutputStream outStream) throws IOException {
-    return xml.serializeTo(source, outStream);
+    return xml.serializeTo(source, namespaces, outStream);
   }
 
   // TODO(corysmith): Clean up all the casting. The type structure is unclean.
@@ -262,6 +292,7 @@ public class DataResourceXml implements DataResource {
     DataResourceXml xmlResource = (DataResourceXml) resource;
     // TODO(corysmith): Combine the sources so that we know both of the originating files.
     // For right now, use the current source.
-    return of(source, xml.combineWith(xmlResource.xml));
+    return createWithNamespaces(
+        source, xml.combineWith(xmlResource.xml), namespaces.union(xmlResource.namespaces));
   }
 }