3.73 of 5: Convert xml parsing to use the map based storage

Introduces two new interfaces:
  DataValue, which compliments DataKey and allows a cleaner interface MergeConflict
  AndroidDataSet.KeyValueConsumer, abstraction that allows a common interface for consuming parsed resources, which is a lot cleanr for DataValue as well.

--
MOS_MIGRATED_REVID=118466739

9 files changed, 169 insertions, 198 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java
index 613884ded9..df6d9ab8f8 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java
@@ -32,6 +32,7 @@ import java.util.Set;
  * Handles the Merging of AndroidDataSet.
  */
 public class AndroidDataMerger {
+
   /**
    * Merges DataResources into an UnwrittenMergedAndroidData.
    *
@@ -176,6 +177,7 @@ public class AndroidDataMerger {
       }
       throw new MergingException(Joiner.on("\n").join(messages));
     }
+
     return UnwrittenMergedAndroidData.of(
         primaryData.getManifest(),
         primary,
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataSet.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataSet.java
index 8f5a9382a5..5df7cc01da 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataSet.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataSet.java
@@ -50,6 +50,49 @@ import javax.xml.stream.XMLStreamException;
  */
 @Immutable
 public class AndroidDataSet {
+
+  /** A Consumer style interface that will accept a DataKey and DataValue. */
+  interface KeyValueConsumer<K extends DataKey, V extends DataValue> {
+    void consume(K key, V value);
+  }
+
+  private static class NonOverwritableConsumer implements KeyValueConsumer<DataKey, DataResource> {
+
+    private Map<DataKey, DataResource> target;
+
+    public NonOverwritableConsumer(Map<DataKey, DataResource> target) {
+      this.target = target;
+    }
+
+    @Override
+    public void consume(DataKey key, DataResource value) {
+      if (!target.containsKey(key)) {
+        target.put(key, value);
+      }
+    }
+  }
+
+  // TODO(corysmith): Determine a better name.
+  private static class OverwritableConsumer<K extends DataKey, V extends DataValue>
+      implements KeyValueConsumer<K, V> {
+    private Map<K, V> target;
+    private Set<MergeConflict> conflicts;
+
+    OverwritableConsumer(Map<K, V> target, Set<MergeConflict> conflicts) {
+      this.target = target;
+      this.conflicts = conflicts;
+    }
+
+    @Override
+    public void consume(K key, V value) {
+      if (target.containsKey(key)) {
+        conflicts.add(MergeConflict.between(key, value, target.get(key)));
+      } else {
+        target.put(key, value);
+      }
+    }
+  }
+
   /**
    * An AndroidDataPathWalker that collects DataAsset and DataResources for an AndroidDataSet.
    */
@@ -59,16 +102,19 @@ public class AndroidDataSet {
     private final ResourceFileVisitor resourceVisitor;
     private static final ImmutableSet<FileVisitOption> FOLLOW_LINKS =
         ImmutableSet.of(FileVisitOption.FOLLOW_LINKS);
-    private final Map<DataKey, DataResource> overwritingResources;
-    private final Map<DataKey, DataResource> nonOverwritingResources;
+    private Map<DataKey, DataResource> overwritingResources;
+    private Map<DataKey, DataResource> nonOverwritingResources;
 
     private static AndroidDataSetBuildingPathWalker create() {
-      Map<DataKey, DataResource> overwritingResources = new HashMap<>();
-      Map<DataKey, DataResource> nonOverwritingResources = new HashMap<>();
+      final Map<DataKey, DataResource> overwritingResources = new HashMap<>();
+      final Map<DataKey, DataResource> nonOverwritingResources = new HashMap<>();
       final Map<DataKey, DataAsset> assets = new HashMap<>();
-      Set<MergeConflict> conflicts = new HashSet<>();
+      final Set<MergeConflict> conflicts = new HashSet<>();
+
       final ResourceFileVisitor resourceVisitor =
-          new ResourceFileVisitor(conflicts, overwritingResources, nonOverwritingResources);
+          new ResourceFileVisitor(
+              new OverwritableConsumer<>(overwritingResources, conflicts),
+              new NonOverwritableConsumer(nonOverwritingResources));
       return new AndroidDataSetBuildingPathWalker(
           conflicts, assets, overwritingResources, nonOverwritingResources, resourceVisitor);
     }
@@ -88,7 +134,6 @@ public class AndroidDataSet {
 
     @Override
     public void walkResources(Path path) throws IOException {
-
       Files.walkFileTree(path, FOLLOW_LINKS, Integer.MAX_VALUE, resourceVisitor);
     }
 
@@ -98,11 +143,12 @@ public class AndroidDataSet {
           path,
           FOLLOW_LINKS,
           Integer.MAX_VALUE,
-          new AssetFileVisitor(RelativeAssetPath.Factory.of(path), assets, conflicts));
+          new AssetFileVisitor(
+              RelativeAssetPath.Factory.of(path), new OverwritableConsumer<>(assets, conflicts)));
     }
 
     /**
-     * Creates an AndroidDataSet from the collected DataAsset and DataResource instances.
+     * Creates an {@link AndroidDataSet} from {@link DataAsset} and {@link DataResource} instances.
      */
     public AndroidDataSet createAndroidDataSet() throws MergingException {
       resourceVisitor.checkForErrors();
@@ -115,20 +161,17 @@ public class AndroidDataSet {
   }
 
   /**
-   * A FileVisitor that walks the asset tree and extracts FileDataResources.
+   * A {@link java.nio.file.FileVisitor} that walks the asset tree and extracts {@link DataAsset}s.
    */
   private static class AssetFileVisitor extends SimpleFileVisitor<Path> {
     private final RelativeAssetPath.Factory dataKeyFactory;
-    private final Map<DataKey, DataAsset> assets;
-    private final Set<MergeConflict> conflicts;
+    private KeyValueConsumer<DataKey, DataAsset> assetConsumer;
 
     public AssetFileVisitor(
         RelativeAssetPath.Factory dataKeyFactory,
-        Map<DataKey, DataAsset> assets,
-        Set<MergeConflict> conflicts) {
+        KeyValueConsumer<DataKey, DataAsset> assetConsumer) {
       this.dataKeyFactory = dataKeyFactory;
-      this.assets = assets;
-      this.conflicts = conflicts;
+      this.assetConsumer = assetConsumer;
     }
 
     @Override
@@ -136,11 +179,7 @@ public class AndroidDataSet {
       if (!Files.isDirectory(path)) {
         RelativeAssetPath key = dataKeyFactory.create(path);
         FileDataResource asset = FileDataResource.of(key, path);
-        if (assets.containsKey(key)) {
-          conflicts.add(MergeConflict.between(key, assets.get(key), asset));
-        } else {
-          assets.put(key, asset);
-        }
+        assetConsumer.consume(key, asset);
       }
       return super.visitFile(path, attrs);
     }
@@ -150,21 +189,35 @@ public class AndroidDataSet {
    * A FileVisitor that walks a resource tree and extract FullyQualifiedName and resource values.
    */
   private static class ResourceFileVisitor extends SimpleFileVisitor<Path> {
-    private final Set<MergeConflict> conflicts;
-    private final Map<DataKey, DataResource> overwritingResources;
-    private final Map<DataKey, DataResource> nonOverwritingResources;
     private final List<Exception> errors = new ArrayList<>();
+    private final OverwritableConsumer<DataKey, DataResource> overwritingConsumer;
+    private final NonOverwritableConsumer nonOverwritingConsumer;
     private boolean inValuesSubtree;
     private FullyQualifiedName.Factory fqnFactory;
     private final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
 
     public ResourceFileVisitor(
-        Set<MergeConflict> conflicts,
-        Map<DataKey, DataResource> overwritingResources,
-        Map<DataKey, DataResource> nonOverwritingResources) {
-      this.conflicts = conflicts;
-      this.overwritingResources = overwritingResources;
-      this.nonOverwritingResources = nonOverwritingResources;
+        OverwritableConsumer<DataKey, DataResource> overwritingConsumer,
+        NonOverwritableConsumer nonOverwritingConsumer) {
+      this.overwritingConsumer = overwritingConsumer;
+      this.nonOverwritingConsumer = nonOverwritingConsumer;
+    }
+
+    private static String deriveRawFullyQualifiedName(Path path) {
+      if (path.getNameCount() < 2) {
+        throw new IllegalArgumentException(
+            String.format(
+                "The resource path %s is too short. "
+                    + "The path is expected to be <resource type>/<file name>.",
+                path));
+      }
+      String pathWithExtension =
+          path.subpath(path.getNameCount() - 2, path.getNameCount()).toString();
+      int extensionStart = pathWithExtension.lastIndexOf('.');
+      if (extensionStart > 0) {
+        return pathWithExtension.substring(0, extensionStart);
+      }
+      return pathWithExtension;
     }
 
     private void checkForErrors() throws MergingException {
@@ -199,25 +252,12 @@ public class AndroidDataSet {
       try {
         if (!Files.isDirectory(path)) {
           if (inValuesSubtree) {
-            List<DataResource> targetOverwritableResources = new ArrayList<>();
-            List<DataResource> targetNonOverwritableResources = new ArrayList<>();
             XmlDataResource.fromPath(
-                xmlInputFactory,
-                path,
-                fqnFactory,
-                targetOverwritableResources,
-                targetNonOverwritableResources);
-            for (DataResource dataResource : targetOverwritableResources) {
-              putOverwritingResource(dataResource);
-            }
-            for (DataResource dataResource : targetNonOverwritableResources) {
-              if (!nonOverwritingResources.containsKey(dataResource.dataKey())) {
-                nonOverwritingResources.put(dataResource.dataKey(), dataResource);
-              }
-            }
+                xmlInputFactory, path, fqnFactory, overwritingConsumer, nonOverwritingConsumer);
           } else {
-            DataResource dataResource = FileDataResource.fromPath(path, fqnFactory);
-            putOverwritingResource(dataResource);
+            String rawFqn = deriveRawFullyQualifiedName(path);
+            FullyQualifiedName key = fqnFactory.parse(rawFqn);
+            overwritingConsumer.consume(key, FileDataResource.of(key, path));
           }
         }
       } catch (IllegalArgumentException | XMLStreamException e) {
@@ -226,18 +266,6 @@ public class AndroidDataSet {
       return super.visitFile(path, attrs);
     }
 
-    private void putOverwritingResource(DataResource dataResource) {
-      if (overwritingResources.containsKey(dataResource.dataKey())) {
-        conflicts.add(
-            MergeConflict.between(
-                dataResource.dataKey(),
-                dataResource,
-                overwritingResources.get(dataResource.dataKey())));
-      } else {
-        overwritingResources.put(dataResource.dataKey(), dataResource);
-      }
-    }
-
     public List<Exception> getErrors() {
       return errors;
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataAsset.java b/src/tools/android/java/com/google/devtools/build/android/DataAsset.java
index 5379e88960..051b41c5f9 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataAsset.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataAsset.java
@@ -19,16 +19,7 @@ import java.nio.file.Path;
 /**
  * Represents an Asset created from a binary file.
  */
-public interface DataAsset {
-  /**
-   * Provides the RelativeAssetPath of the DataAsset.
-   */
-  DataKey dataKey();
-
-  /**
-   * Provides the Path to the file from which the DataResource was derived.
-   */
-  Path source();
+public interface DataAsset extends DataValue {
 
   /**
    * Writes the asset to the given asset directory.
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataResource.java b/src/tools/android/java/com/google/devtools/build/android/DataResource.java
index 877a701258..e369a08bb0 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataResource.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataResource.java
@@ -19,18 +19,7 @@ import java.nio.file.Path;
 /**
  * Represents an Android Resource parsed from an xml or binary file.
  */
-public interface DataResource {
-
-  /**
-   * Provides the FullyQualifiedName of the DataResource.
-   */
-  DataKey dataKey();
-
-  /**
-   * Provides the Path to the file from which the DataResource was derived.
-   */
-  Path source();
-
+public interface DataResource extends DataValue {
   /**
    * Writes the resource to the given resource directory.
    * @param newResourceDirectory The new directory for this resource.
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataValue.java b/src/tools/android/java/com/google/devtools/build/android/DataValue.java
new file mode 100644
index 0000000000..7598ab9390
--- /dev/null
+++ b/src/tools/android/java/com/google/devtools/build/android/DataValue.java
@@ -0,0 +1,35 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.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.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.android;
+
+import java.nio.file.Path;
+
+/**
+ * Represents the value associated with DataKey interface for resource and asset values.
+ *
+ * A DataValue is either an android resource or asset derived from a Path.
+ */
+public interface DataValue {
+
+  /**
+   * Provides the DataKey of this value.
+   */
+  DataKey dataKey();
+
+  /**
+   * Provides the Path to the file from which the DataValue was derived.
+   */
+  Path source();
+
+}
diff --git a/src/tools/android/java/com/google/devtools/build/android/FileDataResource.java b/src/tools/android/java/com/google/devtools/build/android/FileDataResource.java
index 9114e3f8cd..f6c09a3399 100644
--- a/src/tools/android/java/com/google/devtools/build/android/FileDataResource.java
+++ b/src/tools/android/java/com/google/devtools/build/android/FileDataResource.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.android;
 
 import com.google.common.base.MoreObjects;
-import com.google.devtools.build.android.FullyQualifiedName.Factory;
 
 import java.io.IOException;
 import java.nio.file.Path;
@@ -61,27 +60,6 @@ public class FileDataResource implements DataResource, DataAsset {
         .toString();
   }
 
-  static DataResource fromPath(Path path, Factory fqnFactory) {
-    if (path.getNameCount() < 2) {
-      throw new IllegalArgumentException(
-          String.format(
-              "The resource path %s is too short. "
-                  + "The path is expected to be <resource type>/<file name>.",
-              path));
-    }
-    String rawFqn =
-        removeFileExtension(path.subpath(path.getNameCount() - 2, path.getNameCount()).toString());
-    return of(fqnFactory.parse(rawFqn), path);
-  }
-
-  private static String removeFileExtension(String pathWithExtension) {
-    int extensionStart = pathWithExtension.lastIndexOf('.');
-    if (extensionStart > 0) {
-      return pathWithExtension.substring(0, extensionStart);
-    }
-    return pathWithExtension;
-  }
-
   @Override
   public DataKey dataKey() {
     return dataKey;
diff --git a/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java b/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java
index 948a357c1a..d52fa8535c 100644
--- a/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java
+++ b/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java
@@ -30,6 +30,7 @@ import java.util.Objects;
 @Immutable
 public class MergeConflict {
   private static final String CONFLICT_MESSAGE = "%s is provided from %s and %s";
+
   private final DataKey dataKey;
   private final Path first;
   private final Path second;
@@ -51,25 +52,7 @@ public class MergeConflict {
    * @param second The second DataResource.
    * @return A new MergeConflict.
    */
-  public static MergeConflict between(DataKey dataKey, DataResource first, DataResource second) {
-    Preconditions.checkNotNull(dataKey);
-    Preconditions.checkArgument(dataKey.equals(first.dataKey()));
-    Preconditions.checkArgument(dataKey.equals(second.dataKey()));
-    return of(dataKey, first.source(), second.source());
-  }
-
-  /**
-   * Creates a MergeConflict between two DataResources.
-   *
-   * The {@link DataKey} must match the first.dataKey() and second
-   * .dataKey().
-   *
-   * @param dataKey The dataKey name that both DataResources share.
-   * @param first The first DataResource.
-   * @param second The second DataResource.
-   * @return A new MergeConflict.
-   */
-  public static MergeConflict between(DataKey dataKey, DataAsset first, DataAsset second) {
+  public static MergeConflict between(DataKey dataKey, DataValue first, DataValue second) {
     Preconditions.checkNotNull(dataKey);
     Preconditions.checkArgument(dataKey.equals(first.dataKey()));
     Preconditions.checkArgument(dataKey.equals(second.dataKey()));
diff --git a/src/tools/android/java/com/google/devtools/build/android/XmlDataResource.java b/src/tools/android/java/com/google/devtools/build/android/XmlDataResource.java
index 4a11082ad4..545159a181 100644
--- a/src/tools/android/java/com/google/devtools/build/android/XmlDataResource.java
+++ b/src/tools/android/java/com/google/devtools/build/android/XmlDataResource.java
@@ -13,7 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
+import static com.android.resources.ResourceType.DECLARE_STYLEABLE;
+import static com.android.resources.ResourceType.ID;
+
 import com.google.common.base.MoreObjects;
+import com.google.devtools.build.android.AndroidDataSet.KeyValueConsumer;
 import com.google.devtools.build.android.FullyQualifiedName.Factory;
 import com.google.devtools.build.android.xml.ArrayXmlResourceValue;
 
@@ -23,7 +27,6 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.List;
 import java.util.Objects;
 
 import javax.xml.stream.FactoryConfigurationError;
@@ -49,10 +52,8 @@ public class XmlDataResource implements DataResource {
    * @param xmlInputFactory Used to create an XMLEventReader from the supplied resource path.
    * @param path The path to the xml resource to be parsed.
    * @param fqnFactory Used to create {@link FullyQualifiedName}s from the resource names.
-   * @param targetOverwritableResources A collection that will have overwritable
-   *        {@link XmlDataResource}s added to it.
-   * @param targetNonOverwritableResources A collection that will have nonoverwritable
-   *        {@link XmlDataResource}s added to it.
+   * @param overwritingConsumer A consumer for overwritable {@link XmlDataResource}s.
+   * @param nonOverwritingConsumer  A consumer for nonoverwritable {@link XmlDataResource}s.
    * @throws XMLStreamException Thrown with the resource format is invalid.
    * @throws FactoryConfigurationError Thrown with the {@link XMLInputFactory} is misconfigured.
    * @throws IOException Thrown when there is an error reading a file.
@@ -61,8 +62,8 @@ public class XmlDataResource implements DataResource {
       XMLInputFactory xmlInputFactory,
       Path path,
       Factory fqnFactory,
-      List<DataResource> targetOverwritableResources,
-      List<DataResource> targetNonOverwritableResources)
+      KeyValueConsumer<DataKey, DataResource> overwritingConsumer,
+      KeyValueConsumer<DataKey, DataResource> nonOverwritingConsumer)
       throws XMLStreamException, FactoryConfigurationError, IOException {
     XMLEventReader eventReader =
         xmlInputFactory.createXMLEventReader(Files.newBufferedReader(path, StandardCharsets.UTF_8));
@@ -71,78 +72,45 @@ public class XmlDataResource implements DataResource {
       for (StartElement start = XmlResourceValues.findNextStart(eventReader);
           start != null;
           start = XmlResourceValues.findNextStart(eventReader)) {
-        parseXmlElements(
-            path,
-            fqnFactory,
-            targetOverwritableResources,
-            targetNonOverwritableResources,
-            eventReader,
-            start);
+        ResourceType resourceType = getResourceType(start);
+        if (resourceType == DECLARE_STYLEABLE) {
+          // Styleables are special, as they produce multiple overwrite and non-overwrite values,
+          // so we let the value handle the assignments.
+          XmlResourceValues.parseDeclareStyleable(
+              fqnFactory, path, overwritingConsumer, nonOverwritingConsumer, eventReader, start);
+        } else {
+          // Of simple resources, only IDs are nonOverwriting.
+          KeyValueConsumer<DataKey, DataResource> consumer =
+              resourceType == ID ? nonOverwritingConsumer : overwritingConsumer;
+          FullyQualifiedName key =
+              fqnFactory.create(resourceType, XmlResourceValues.getElementName(start));
+          consumer.consume(
+              key,
+              XmlDataResource.of(key, path, parseXmlElements(resourceType, eventReader, start)));
+        }
       }
     }
   }
 
-  private static void parseXmlElements(
-      Path path,
-      Factory fqnFactory,
-      List<DataResource> overwritable,
-      List<DataResource> nonOverwritable,
-      XMLEventReader eventReader,
-      StartElement start)
+  private static XmlResourceValue parseXmlElements(
+      ResourceType resourceType, XMLEventReader eventReader, StartElement start)
       throws XMLStreamException {
-    ResourceType resourceType = getResourceType(start);
     switch (resourceType) {
       case STYLE:
-        overwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(ResourceType.STYLE, XmlResourceValues.getElementName(start)),
-                path,
-                XmlResourceValues.parseStyle(eventReader, start)));
-        break;
-      case DECLARE_STYLEABLE:
-        // Styleables are special, as they produce multiple overwrite and non-overwrite values,
-        // so we let the value handle the assignments.
-        XmlResourceValues.parseDeclareStyleable(
-            fqnFactory, path, nonOverwritable, overwritable, eventReader, start);
-        break;
+        return XmlResourceValues.parseStyle(eventReader, start);
       case ARRAY:
-        overwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(ResourceType.ARRAY, XmlResourceValues.getElementName(start)),
-                path,
-                ArrayXmlResourceValue.parseArray(eventReader, start)));
-        break;
+        return ArrayXmlResourceValue.parseArray(eventReader, start);
       case PLURALS:
-        overwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(ResourceType.PLURALS, XmlResourceValues.getElementName(start)),
-                path,
-                XmlResourceValues.parsePlurals(eventReader)));
-        break;
+        return XmlResourceValues.parsePlurals(eventReader);
       case ATTR:
-        overwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(ResourceType.ATTR, XmlResourceValues.getElementName(start)),
-                path,
-                XmlResourceValues.parseAttr(eventReader, start)));
-        break;
+        return XmlResourceValues.parseAttr(eventReader, start);
       case ID:
-        nonOverwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(ResourceType.ID, XmlResourceValues.getElementName(start)),
-                path,
-                XmlResourceValues.parseId()));
-        break;
+        return XmlResourceValues.parseId();
       case STRING:
       case BOOL:
       case COLOR:
       case DIMEN:
-        overwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(resourceType, XmlResourceValues.getElementName(start)),
-                path,
-                XmlResourceValues.parseSimple(eventReader, resourceType)));
-        break;
+        return XmlResourceValues.parseSimple(eventReader, resourceType);
       default:
         throw new XMLStreamException(
             String.format("Unhandled resourceType %s", resourceType), start.getLocation());
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 1ed6631d1c..3a3e5e3d9e 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,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
+import com.google.devtools.build.android.AndroidDataSet.KeyValueConsumer;
 import com.google.devtools.build.android.xml.AttrXmlResourceValue;
 import com.google.devtools.build.android.xml.IdXmlResourceValue;
 import com.google.devtools.build.android.xml.PluralXmlResourceValue;
@@ -86,8 +87,8 @@ public class XmlResourceValues {
   static void parseDeclareStyleable(
       FullyQualifiedName.Factory fqnFactory,
       Path path,
-      List<DataResource> nonOverwritable,
-      List<DataResource> overwritable,
+      KeyValueConsumer<DataKey, DataResource> overwritingConsumer,
+      KeyValueConsumer<DataKey, DataResource> nonOverwritingConsumer,
       XMLEventReader eventReader,
       StartElement start)
       throws XMLStreamException {
@@ -100,18 +101,14 @@ public class XmlResourceValues {
         StartElement attr = element.asStartElement();
         members.add(getElementName(attr));
         String attrName = getElementName(attr);
-        overwritable.add(
-            XmlDataResource.of(
-                fqnFactory.create(ResourceType.ATTR, attrName),
-                path,
-                parseAttr(eventReader, start)));
+        FullyQualifiedName fqn = fqnFactory.create(ResourceType.ATTR, attrName);
+        overwritingConsumer.consume(
+            fqn, XmlDataResource.of(fqn, path, parseAttr(eventReader, start)));
       }
     }
-    nonOverwritable.add(
-        XmlDataResource.of(
-            fqnFactory.create(ResourceType.STYLEABLE, styleableName),
-            path,
-            StyleableXmlResourceValue.of(members)));
+    FullyQualifiedName fqn = fqnFactory.create(ResourceType.STYLEABLE, styleableName);
+    nonOverwritingConsumer.consume(
+        fqn, XmlDataResource.of(fqn, path, StyleableXmlResourceValue.of(members)));
   }
 
   static XmlResourceValue parseAttr(XMLEventReader eventReader, StartElement start)