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, 36 insertions, 62 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java b/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
index 411be94ba3..1caaff264f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
+++ b/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
@@ -13,35 +13,30 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
+import com.android.resources.ResourceType;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.android.ParsedAndroidData.KeyValueConsumer;
 import com.google.devtools.build.android.proto.SerializeFormat;
 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.CodedOutputStream;
-
-import com.android.resources.ResourceType;
-
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.StringWriter;
 import java.nio.file.Path;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.logging.Logger;
-
 import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
-import javax.xml.stream.XMLEventFactory;
 import javax.xml.stream.XMLEventReader;
 import javax.xml.stream.XMLEventWriter;
 import javax.xml.stream.XMLInputFactory;
@@ -80,11 +75,6 @@ public class XmlResourceValues {
   private static final QName ATTR_TYPE = QName.valueOf("type");
 
   private static final XMLOutputFactory XML_OUTPUT_FACTORY = XMLOutputFactory.newInstance();
-  private static final XMLEventFactory XML_EVENT_FACTORY = XMLEventFactory.newInstance();
-
-  static final String XLIFF_NAMESPACE = "urn:oasis:names:tc:xliff:document:1.2";
-  static final String XLIFF_PREFIX = "xliff";
-
 
   private static XMLInputFactory inputFactoryInstance = null;
   public static XMLInputFactory getXmlInputFactory() {
@@ -96,9 +86,11 @@ public class XmlResourceValues {
     return inputFactoryInstance;
   }
 
-  static XmlResourceValue parsePlurals(XMLEventReader eventReader, StartElement start)
+  static XmlResourceValue parsePlurals(
+      XMLEventReader eventReader, StartElement start, Namespaces.Collector namespacesCollector)
       throws XMLStreamException {
     ImmutableMap.Builder<String, String> values = ImmutableMap.builder();
+    namespacesCollector.collectFrom(start);
     for (XMLEvent element = nextTag(eventReader);
         !isEndTag(element, TAG_PLURALS);
         element = nextTag(eventReader)) {
@@ -107,7 +99,9 @@ public class XmlResourceValues {
           throw new XMLStreamException(
               String.format("Expected start element %s", element), element.getLocation());
         }
-        String contents = readContentsAsString(eventReader, element.asStartElement().getName());
+        String contents =
+            readContentsAsString(
+                eventReader, element.asStartElement().getName(), namespacesCollector);
         values.put(
             getElementAttributeByName(element.asStartElement(), ATTR_QUANTITY),
             contents == null ? "" : contents);
@@ -158,7 +152,7 @@ public class XmlResourceValues {
             || (XmlResourceValues.peekNextTag(eventReader) != null
                 && XmlResourceValues.peekNextTag(eventReader).isStartElement())) {
           overwritingConsumer.consume(
-              attrName, DataResourceXml.of(path, parseAttr(eventReader, attr)));
+              attrName, DataResourceXml.createWithNoNamespace(path, parseAttr(eventReader, attr)));
           members.put(attrName, Boolean.TRUE);
         } else {
           members.put(attrName, Boolean.FALSE);
@@ -167,7 +161,7 @@ public class XmlResourceValues {
     }
     combiningConsumer.consume(
         fqnFactory.create(ResourceType.STYLEABLE, getElementName(start)),
-        DataResourceXml.of(path, StyleableXmlResourceValue.of(members)));
+        DataResourceXml.createWithNoNamespace(path, StyleableXmlResourceValue.of(members)));
   }
 
   static XmlResourceValue parseAttr(XMLEventReader eventReader, StartElement start)
@@ -178,24 +172,31 @@ public class XmlResourceValues {
     return value;
   }
 
-  static XmlResourceValue parseId(XMLEventReader eventReader, StartElement start)
+  static XmlResourceValue parseId(
+      XMLEventReader eventReader, StartElement start, Namespaces.Collector namespacesCollector)
       throws XMLStreamException {
     if (XmlResourceValues.isEndTag(eventReader.peek(), start.getName())) {
       return IdXmlResourceValue.of();
     } else {
-      return IdXmlResourceValue.of(readContentsAsString(eventReader, start.getName()));
+      return IdXmlResourceValue.of(
+          readContentsAsString(
+              eventReader, start.getName(), namespacesCollector.collectFrom(start)));
     }
   }
 
   static XmlResourceValue parseSimple(
-      XMLEventReader eventReader, ResourceType resourceType, StartElement start)
+      XMLEventReader eventReader,
+      ResourceType resourceType,
+      StartElement start,
+      Namespaces.Collector namespacesCollector)
       throws XMLStreamException {
     String contents;
+    namespacesCollector.collectFrom(start);
     // Check that the element is unary. If it is, the contents is null
     if (isEndTag(eventReader.peek(), start.getName())) {
       contents = null;
     } else {
-      contents = readContentsAsString(eventReader, start.getName());
+      contents = readContentsAsString(eventReader, start.getName(), namespacesCollector);
     }
     return SimpleXmlResourceValue.of(
         start.getName().equals(TAG_ITEM)
@@ -218,18 +219,15 @@ public class XmlResourceValues {
       }
       String value = escapeXmlValues(attribute.getValue()).replace("\"", "&quot;");
       if (!name.getNamespaceURI().isEmpty()) {
-        // xliff is always provided on resources to help with file size
-        if (!(XLIFF_NAMESPACE.equals(name.getNamespaceURI())
-            && XLIFF_PREFIX.equals(name.getPrefix()))) {
-          // Declare the xmlns here, so that the written xml will be semantically correct,
-          // if a bit verbose. This allows the resource keys to be written into a <resources>
-          // tag without a global namespace.
-          attributeMap.put("xmlns:" + name.getPrefix(), name.getNamespaceURI());
-        }
         attributeMap.put(name.getPrefix() + ":" + attribute.getName().getLocalPart(), value);
       } else {
         attributeMap.put(attribute.getName().getLocalPart(), value);
       }
+      Iterator<Namespace> namespaces = iterateNamespacesFrom(start);
+      while (namespaces.hasNext()) {
+        Namespace namespace = namespaces.next();
+        attributeMap.put("xmlns:" + namespace.getPrefix(), namespace.getNamespaceURI());
+      }
     }
     return attributeMap;
   }
@@ -245,34 +243,20 @@ public class XmlResourceValues {
    *
    * @param eventReader The current xml stream.
    * @param startTag The name of the tag to close on.
+   * @param namespacesCollector A builder for collecting namespaces.
    * @return A xml escaped string representation of the xml stream
    */
   @Nullable
-  public static String readContentsAsString(XMLEventReader eventReader, QName startTag)
+  public static String readContentsAsString(
+      XMLEventReader eventReader, QName startTag, Namespaces.Collector namespacesCollector)
       throws XMLStreamException {
     StringWriter contents = new StringWriter();
     XMLEventWriter writer = XML_OUTPUT_FACTORY.createXMLEventWriter(contents);
     while (!isEndTag(eventReader.peek(), startTag)) {
       XMLEvent xmlEvent = (XMLEvent) eventReader.next();
       if (xmlEvent.isStartElement()) {
-        // TODO(corysmith): Replace this with a proper representation of the contents that can be
-        // serialized and reconstructed appropriately without modification.
-        StartElement start = xmlEvent.asStartElement();
-        QName name = start.getName();
-        // Lazy with the list for memory usage -- this code path happens a lot.
-        List<Namespace> namespaces = maybeAddNamespace(name, null);
-        Iterator<Attribute> attributes = iterateAttributesFrom(start);
-        while (attributes.hasNext()) {
-          Attribute attribute = attributes.next();
-          namespaces = maybeAddNamespace(attribute.getName(), namespaces);
-        }
-        if (namespaces != null) {
-          writer.add(
-              XML_EVENT_FACTORY.createStartElement(
-                  name, iterateAttributesFrom(start), namespaces.iterator()));
-        } else {
-          writer.add(xmlEvent);
-        }
+        namespacesCollector.collectFrom(xmlEvent.asStartElement());
+        writer.add(xmlEvent);
       } else {
         writer.add(xmlEvent);
       }
@@ -284,25 +268,15 @@ public class XmlResourceValues {
   }
 
   @SuppressWarnings({
-    "cast",
     "unchecked"
   }) // The interface returns Iterator, force casting based on documentation.
-  static Iterator<Attribute> iterateAttributesFrom(StartElement start) {
-    return (Iterator<Attribute>) start.getAttributes();
+  public static Iterator<Attribute> iterateAttributesFrom(StartElement start) {
+    return start.getAttributes();
   }
 
-  private static List<Namespace> maybeAddNamespace(QName name, List<Namespace> namespaces) {
-    // The xliff namespace is provided by default.
-    if (name.getNamespaceURI() != null
-        && !name.getNamespaceURI().isEmpty()
-        && !(XLIFF_NAMESPACE.equals(name.getNamespaceURI())
-            && XLIFF_PREFIX.equals(name.getPrefix()))) {
-      if (namespaces == null) {
-        namespaces = new ArrayList<>();
-      }
-      namespaces.add(XML_EVENT_FACTORY.createNamespace(name.getPrefix(), name.getNamespaceURI()));
-    }
-    return namespaces;
+  @SuppressWarnings("unchecked")
+  public static Iterator<Namespace> iterateNamespacesFrom(StartElement start) {
+    return start.getNamespaces();
   }
 
   /* XML helper methods follow. */