Automated rollback of commit 3181c2f1362622985aca24747ed9512573d25dc0.

RELNOTES: None
PiperOrigin-RevId: 174502289

6 files changed, 194 insertions, 126 deletions

diff --git a/src/test/java/com/google/devtools/build/android/AndroidDataMergerTest.java b/src/test/java/com/google/devtools/build/android/AndroidDataMergerTest.java
index 46177e7eac..e28d65ceb3 100644
--- a/src/test/java/com/google/devtools/build/android/AndroidDataMergerTest.java
+++ b/src/test/java/com/google/devtools/build/android/AndroidDataMergerTest.java
@@ -22,7 +22,8 @@ import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.jimfs.Jimfs;
-import com.google.common.truth.Subject;
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.SubjectFactory;
 import com.google.devtools.build.android.AndroidDataBuilder.ResourceType;
 import com.google.devtools.build.android.AndroidDataMerger.MergeConflictException;
 import com.google.devtools.build.android.AndroidDataMerger.SourceChecker;
@@ -179,7 +180,7 @@ public class AndroidDataMergerTest {
                     file("layout/exit").root(directRoot).source("res/layout/exit.xml"))
                 .combining(
                     xml("id/exit")
-                        .root(transitiveRoot)
+                        .root(directRoot)
                         .source("values/ids.xml")
                         .value(IdXmlResourceValue.of()))
                 .build());
@@ -243,7 +244,7 @@ public class AndroidDataMergerTest {
                     file("layout/exit").root(directRoot).source("res/layout/exit.xml"))
                 .combining(
                     xml("id/exit")
-                        .root(transitiveRoot)
+                        .root(directRoot)
                         .source("values/ids.xml")
                         .value(IdXmlResourceValue.of()))
                 .build());
@@ -462,11 +463,6 @@ public class AndroidDataMergerTest {
     Path primaryRoot = fileSystem.getPath("primary");
     Path directRoot = fileSystem.getPath("direct");
     Path transitiveRoot = fileSystem.getPath("transitive");
-    DataSource primaryStrings = DataSource.of(primaryRoot.resolve("res/values/strings.xml"));
-    DataSource transitive1String =
-        DataSource.of(transitiveRoot.resolve("1/res/values/strings.xml"));
-    DataSource transitive2String =
-        DataSource.of(transitiveRoot.resolve("2/res/values/strings.xml"));
 
     ParsedAndroidData transitiveDependency =
         ParsedAndroidDataBuilder.buildOn(transitiveRoot, fqnFactory)
@@ -502,7 +498,7 @@ public class AndroidDataMergerTest {
                 .overwritable(
                     xml("string/exit")
                         .root(primaryRoot)
-                        .source(primaryStrings.overwrite(transitive1String, transitive2String))
+                        .source("values/strings.xml")
                         .value(SimpleXmlResourceValue.createWithValue(Type.STRING, "way out")))
                 .build(),
             ParsedAndroidDataBuilder.empty());
@@ -589,13 +585,13 @@ public class AndroidDataMergerTest {
     assertThat(loggingHandler.warnings)
         .containsExactly(
             MergeConflict.of(
-                    fullyQualifiedName,
-                    DataResourceXml.createWithNoNamespace(
-                        primaryRoot.resolve("res/values/strings.xml"),
-                        SimpleXmlResourceValue.createWithValue(Type.STRING, "no way out")),
-                    DataResourceXml.createWithNoNamespace(
-                        transitiveRoot.resolve("res/values/strings.xml"),
-                        SimpleXmlResourceValue.createWithValue(Type.STRING, "wrong way out")))
+                fullyQualifiedName,
+                DataResourceXml.createWithNoNamespace(
+                    directRoot.resolve("res/values/strings.xml"),
+                    SimpleXmlResourceValue.createWithValue(Type.STRING, "no way out")),
+                DataResourceXml.createWithNoNamespace(
+                    transitiveRoot.resolve("res/values/strings.xml"),
+                    SimpleXmlResourceValue.createWithValue(Type.STRING, "wrong way out")))
                 .toConflictMessage());
   }
 
@@ -681,7 +677,7 @@ public class AndroidDataMergerTest {
             ParsedAndroidDataBuilder.buildOn(fqnFactory)
                 .overwritable(
                     xml("string/exit")
-                        .source(primaryStrings.overwrite(directStrings, transitiveStrings))
+                        .source(primaryStrings.overwrite(directStrings))
                         .value(SimpleXmlResourceValue.createWithValue(Type.STRING, "way out")))
                 .build(),
             ParsedAndroidDataBuilder.empty());
@@ -949,7 +945,7 @@ public class AndroidDataMergerTest {
                     file("layout/zzDirect").source(directLayout))
                 .combining(
                     xml("id/back_door").source(transitiveLayout).value(IdXmlResourceValue.of()),
-                    xml("id/slide").source(transitiveLayout).value(IdXmlResourceValue.of()))
+                    xml("id/slide").source(directLayout).value(IdXmlResourceValue.of()))
                 .build());
     assertAbout(unwrittenMergedAndroidData).that(data).isEqualTo(expected);
   }
@@ -1168,14 +1164,10 @@ public class AndroidDataMergerTest {
   public void mergeAssetsTransitiveConflictWithPrimaryOverride() throws Exception {
     Path primaryRoot = fileSystem.getPath("primary");
     Path transitiveRoot = fileSystem.getPath("transitive");
-    DataSource transitiveSource =
-        DataSource.of(transitiveRoot.resolve("assets/hunting/of/the/snark.txt"));
-    DataSource primarySource =
-        DataSource.of(primaryRoot.resolve("assets/hunting/of/the/snark.txt"));
 
     ParsedAndroidData transitiveDependency =
         ParsedAndroidDataBuilder.buildOn(transitiveRoot)
-            .assets(file().source(transitiveSource))
+            .assets(file().source("hunting/of/the/snark.txt"))
             .build();
 
     ParsedAndroidData directDependency = ParsedAndroidDataBuilder.empty();
@@ -1193,7 +1185,7 @@ public class AndroidDataMergerTest {
         UnwrittenMergedAndroidData.of(
             primary.getManifest(),
             ParsedAndroidDataBuilder.buildOn(primaryRoot)
-                .assets(file().source(primarySource.overwrite(transitiveSource)))
+                .assets(file().source("hunting/of/the/snark.txt"))
                 .build(),
             ParsedAndroidDataBuilder.empty());
     assertAbout(unwrittenMergedAndroidData).that(data).isEqualTo(expected);
@@ -1263,13 +1255,13 @@ public class AndroidDataMergerTest {
             MergeConflict.of(
                     RelativeAssetPath.Factory.of(directRoot.resolve("assets"))
                         .create(directRoot.resolve("assets/hunting/of/the/snark.txt")),
-                    DataValueFile.of(primaryRoot.resolve("assets/hunting/of/the/snark.txt")),
+                    DataValueFile.of(directRoot.resolve("assets/hunting/of/the/snark.txt")),
                     DataValueFile.of(transitiveRoot.resolve("assets/hunting/of/the/snark.txt")))
                 .toConflictMessage());
   }
 
   @Test
-  public void imergeAssetsDirectTransitivePrimaryConflictWithPrimaryOverride() throws Exception {
+  public void mergeAssetsDirectTransitivePrimaryConflictWithPrimaryOverride() throws Exception {
     Path primaryRoot = fileSystem.getPath("primary");
     Path directRoot = fileSystem.getPath("direct");
     Path transitiveRoot = fileSystem.getPath("transitive");
@@ -1277,8 +1269,6 @@ public class AndroidDataMergerTest {
     DataSource primarySource =
         DataSource.of(primaryRoot.resolve("assets/hunting/of/the/snark.txt"));
     DataSource directSource = DataSource.of(directRoot.resolve("assets/hunting/of/the/snark.txt"));
-    DataSource transitiveSource =
-        DataSource.of(transitiveRoot.resolve("assets/hunting/of/the/snark.txt"));
 
     ParsedAndroidData transitiveDependency =
         ParsedAndroidDataBuilder.buildOn(transitiveRoot)
@@ -1299,19 +1289,26 @@ public class AndroidDataMergerTest {
     UnwrittenMergedAndroidData data =
         AndroidDataMerger.createWithDefaults()
             .merge(transitiveDependency, directDependency, primary, true, true);
-
+    
     UnwrittenMergedAndroidData expected =
         UnwrittenMergedAndroidData.of(
             primary.getManifest(),
             ParsedAndroidDataBuilder.buildOn(primaryRoot)
-                .assets(file().source(primarySource.overwrite(directSource, transitiveSource)))
+                .assets(file().source(primarySource.overwrite(directSource)))
                 .build(),
             ParsedAndroidDataBuilder.empty());
     assertAbout(unwrittenMergedAndroidData).that(data).isEqualTo(expected);
   }
 
-  final Subject.Factory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>
-      unwrittenMergedAndroidData = UnwrittenMergedAndroidDataSubject.FACTORY;
+  final SubjectFactory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>
+      unwrittenMergedAndroidData =
+          new SubjectFactory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>() {
+            @Override
+            public UnwrittenMergedAndroidDataSubject getSubject(
+                FailureStrategy fs, UnwrittenMergedAndroidData that) {
+              return new UnwrittenMergedAndroidDataSubject(fs, that);
+            }
+          };
 
   private static final class TestLoggingHandler extends Handler {
     public final List<String> warnings = new ArrayList<String>();
diff --git a/src/test/java/com/google/devtools/build/android/AndroidDataWriterTest.java b/src/test/java/com/google/devtools/build/android/AndroidDataWriterTest.java
index a9e6dae9c6..a8cbd2161e 100644
--- a/src/test/java/com/google/devtools/build/android/AndroidDataWriterTest.java
+++ b/src/test/java/com/google/devtools/build/android/AndroidDataWriterTest.java
@@ -251,8 +251,6 @@ public class AndroidDataWriterTest {
                 source.resolve("AndroidManifest.xml"), direct, ParsedAndroidDataBuilder.empty())
             .write(mergedDataWriter);
 
-
-
     assertAbout(paths).that(actual.getManifest()).exists();
     assertAbout(paths).that(actual.getResourceDir().resolve("values/values.xml")).exists();
     assertAbout(paths)
@@ -264,9 +262,9 @@ public class AndroidDataWriterTest {
             "<integer name='zoo'>54321</integer>",
             "<!-- " + fs.getPath("source/res/values/strings.xml") + " --><eat-comment/>",
             "<string name='foo'>meow</string>",
+            "<!-- " + fs.getPath("source/res/values/integers.xml") + " --><eat-comment/>",
             "<public name='foo' type='integer' id='0x7f040000'/>",
             "<public name='foo' type='string' id='0x7f050000'/>",
-            "<!-- " + fs.getPath("source/res/values/integers.xml") + " --><eat-comment/>",
             "<public name='zoo' type='integer' />",
             END_RESOURCES);
   }
diff --git a/src/test/java/com/google/devtools/build/android/ParsedAndroidDataTest.java b/src/test/java/com/google/devtools/build/android/ParsedAndroidDataTest.java
index 6b2fe1baf4..78d9789af4 100644
--- a/src/test/java/com/google/devtools/build/android/ParsedAndroidDataTest.java
+++ b/src/test/java/com/google/devtools/build/android/ParsedAndroidDataTest.java
@@ -337,6 +337,8 @@ public class ParsedAndroidDataTest {
     DataSource otherRootValuesPath = DataSource.of(otherRoot.resolve("res/values/attr.xml"));
     FullyQualifiedName idSomeId = fqnFactory.parse("id/some_id");
 
+
+
     Truth.assertAbout(parsedAndroidData)
         .that(dataSet)
         .isEqualTo(
@@ -380,9 +382,10 @@ public class ParsedAndroidDataTest {
                 ImmutableMap.<DataKey, DataResource>of(
                     idSomeId, // key
                     DataResourceXml.createWithNoNamespace(
-                        otherRootValuesPath, IdXmlResourceValue.of()) // value
+                        rootValuesPath, IdXmlResourceValue.of()) // value
                     ),
                 ImmutableMap.<DataKey, DataAsset>of()));
+
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/android/UnwrittenMergedAndroidDataSubject.java b/src/test/java/com/google/devtools/build/android/UnwrittenMergedAndroidDataSubject.java
index 3a72ddae99..5d774a1155 100644
--- a/src/test/java/com/google/devtools/build/android/UnwrittenMergedAndroidDataSubject.java
+++ b/src/test/java/com/google/devtools/build/android/UnwrittenMergedAndroidDataSubject.java
@@ -16,25 +16,26 @@ package com.google.devtools.build.android;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.base.Objects;
-import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.FailureStrategy;
 import com.google.common.truth.Subject;
+import com.google.common.truth.SubjectFactory;
 import javax.annotation.Nullable;
 
 class UnwrittenMergedAndroidDataSubject
     extends Subject<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData> {
 
-  static final Subject.Factory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>
+  static final SubjectFactory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>
       FACTORY =
-          new Subject.Factory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>() {
+          new SubjectFactory<UnwrittenMergedAndroidDataSubject, UnwrittenMergedAndroidData>() {
             @Override
-            public UnwrittenMergedAndroidDataSubject createSubject(
-                FailureMetadata fs, UnwrittenMergedAndroidData that) {
+            public UnwrittenMergedAndroidDataSubject getSubject(
+                FailureStrategy fs, UnwrittenMergedAndroidData that) {
               return new UnwrittenMergedAndroidDataSubject(fs, that);
             }
           };
 
   public UnwrittenMergedAndroidDataSubject(
-      FailureMetadata failureStrategy, @Nullable UnwrittenMergedAndroidData subject) {
+      FailureStrategy failureStrategy, @Nullable UnwrittenMergedAndroidData subject) {
     super(failureStrategy, subject);
   }
 
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 61d415787c..8e550a2d60 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
@@ -16,7 +16,7 @@ package com.google.devtools.build.android;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Stopwatch;
-import com.google.common.collect.Sets;
+import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
@@ -24,7 +24,10 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -227,7 +230,8 @@ class AndroidDataMerger {
    *     the ultimate source of truth, provided it doesn't conflict with itself.
    * @return An UnwrittenMergedAndroidData, containing DataResource objects that can be written to
    *     disk for aapt processing or serialized for future merge passes.
-   * @throws MergingException if there are issues with parsing resources from primaryData.
+   * @throws MergingException if there are issues with parsing resources from
+   *     primaryData.
    * @throws MergeConflictException if there are merge conflicts
    */
   UnwrittenMergedAndroidData merge(
@@ -254,25 +258,123 @@ class AndroidDataMerger {
   private UnwrittenMergedAndroidData doMerge(
       ParsedAndroidData transitive,
       ParsedAndroidData direct,
-      ParsedAndroidData primary,
+      ParsedAndroidData parsedPrimary,
       Path primaryManifest,
       boolean allowPrimaryOverrideAll,
       boolean throwOnResourceConflict) {
     try {
       // Create the builders for the final parsed data.
-      ParsedAndroidData mergedPrimary =
-          primary
-              .overwrite(direct, false)
-              .combine(direct)
-              .overwrite(transitive, !allowPrimaryOverrideAll)
-              .combine(transitive);
-      // Filter out all the resources that are in the primary, as they only need to be written once.
-      // This also removes conflicts that have keys in the primary -- those conflicts are recorded
-      // in the overwrite steps above.
-      ParsedAndroidData mergedTransitive = direct.union(transitive).filterBy(mergedPrimary);
+      final ParsedAndroidData.Builder primaryBuilder = ParsedAndroidData.Builder.newBuilder();
+      final ParsedAndroidData.Builder transitiveBuilder = ParsedAndroidData.Builder.newBuilder();
+      final KeyValueConsumers transitiveConsumers = transitiveBuilder.consumers();
+      final KeyValueConsumers primaryConsumers = primaryBuilder.consumers();
+      final Set<MergeConflict> conflicts = new HashSet<>();
+
+      // Find all internal conflicts.
+      conflicts.addAll(parsedPrimary.conflicts());
+      for (MergeConflict conflict : Iterables.concat(direct.conflicts(), transitive.conflicts())) {
+        if (allowPrimaryOverrideAll
+            && (parsedPrimary.containsOverwritable(conflict.dataKey())
+                || parsedPrimary.containsAsset(conflict.dataKey()))) {
+          continue;
+        }
+        conflicts.add(conflict);
+      }
+
+      // overwriting resources
+      for (Entry<DataKey, DataResource> entry : parsedPrimary.iterateOverwritableEntries()) {
+        if (direct.containsOverwritable(entry.getKey())) {
+          primaryConsumers.overwritingConsumer.accept(
+              entry.getKey(), entry.getValue().overwrite(direct.getOverwritable(entry.getKey())));
+        } else {
+          primaryConsumers.overwritingConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
 
-      Set<MergeConflict> conflicts =
-          Sets.union(mergedPrimary.conflicts(), mergedTransitive.conflicts());
+      for (Map.Entry<DataKey, DataResource> entry : direct.iterateOverwritableEntries()) {
+        // Direct dependencies are simply overwritten, no conflict.
+        if (!parsedPrimary.containsOverwritable(entry.getKey())) {
+          transitiveConsumers.overwritingConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
+      for (Map.Entry<DataKey, DataResource> entry : transitive.iterateOverwritableEntries()) {
+        // If the primary is considered to be intentional (usually at the binary level),
+        // skip.
+        if (allowPrimaryOverrideAll && parsedPrimary.containsOverwritable(entry.getKey())) {
+          continue;
+        }
+        // If a transitive value is in the direct map, report a conflict, as it is commonly
+        // unintentional.
+        if (direct.containsOverwritable(entry.getKey())) {
+          conflicts.add(direct.foundResourceConflict(entry.getKey(), entry.getValue()));
+        } else if (parsedPrimary.containsOverwritable(entry.getKey())) {
+          // If overwriting a transitive value with a primary map, assume it's an unintentional
+          // override, unless allowPrimaryOverrideAll is set. At which point, this code path
+          // should not be reached.
+          conflicts.add(parsedPrimary.foundResourceConflict(entry.getKey(), entry.getValue()));
+        } else {
+          // If it's in none of the of sources, add it.
+          transitiveConsumers.overwritingConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
+
+      // combining resources
+      for (Entry<DataKey, DataResource> entry : parsedPrimary.iterateCombiningEntries()) {
+        primaryConsumers.combiningConsumer.accept(entry.getKey(), entry.getValue());
+      }
+      for (Map.Entry<DataKey, DataResource> entry : direct.iterateCombiningEntries()) {
+        if (parsedPrimary.containsCombineable(entry.getKey())) {
+          // If it is in the primary, add it to the primary to be combined.
+          primaryConsumers.combiningConsumer.accept(entry.getKey(), entry.getValue());
+        } else {
+          // If the combining asset is not in the primary, put it into the transitive.
+          transitiveConsumers.combiningConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
+      for (Map.Entry<DataKey, DataResource> entry : transitive.iterateCombiningEntries()) {
+        if (parsedPrimary.containsCombineable(entry.getKey())) {
+          primaryConsumers.combiningConsumer.accept(entry.getKey(), entry.getValue());
+        } else {
+          transitiveConsumers.combiningConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
+
+      // assets
+      for (Entry<DataKey, DataAsset> entry : parsedPrimary.iterateAssetEntries()) {
+        if (direct.containsAsset(entry.getKey())) {
+          primaryConsumers.assetConsumer.accept(
+              entry.getKey(), entry.getValue().overwrite(direct.getAsset(entry.getKey())));
+        } else {
+          primaryConsumers.assetConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
+
+      for (Map.Entry<DataKey, DataAsset> entry : direct.iterateAssetEntries()) {
+        // Direct dependencies are simply overwritten, no conflict.
+        if (!parsedPrimary.containsAsset(entry.getKey())) {
+          transitiveConsumers.assetConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
+      for (Map.Entry<DataKey, DataAsset> entry : transitive.iterateAssetEntries()) {
+        // If the primary is considered to be intentional (usually at the binary level),
+        // skip.
+        if (allowPrimaryOverrideAll && parsedPrimary.containsAsset(entry.getKey())) {
+          continue;
+        }
+        // If a transitive value is in the direct map report a conflict, as it is commonly
+        // unintentional.
+        if (direct.containsAsset(entry.getKey())) {
+          conflicts.add(direct.foundAssetConflict(entry.getKey(), entry.getValue()));
+        } else if (parsedPrimary.containsAsset(entry.getKey())) {
+          // If overwriting a transitive value with a primary map, assume it's an unintentional
+          // override, unless allowPrimaryOverrideAll is set. At which point, this code path
+          // should not be reached.
+          conflicts.add(parsedPrimary.foundAssetConflict(entry.getKey(), entry.getValue()));
+        } else {
+          // If it's in none of the of sources, add it.
+          transitiveConsumers.assetConsumer.accept(entry.getKey(), entry.getValue());
+        }
+      }
 
       if (!conflicts.isEmpty()) {
         List<String> messages = new ArrayList<>();
@@ -289,7 +391,8 @@ class AndroidDataMerger {
           logger.warning(conflictMessage);
         }
       }
-      return UnwrittenMergedAndroidData.of(primaryManifest, mergedPrimary, mergedTransitive);
+      return UnwrittenMergedAndroidData.of(
+          primaryManifest, primaryBuilder.build(), transitiveBuilder.build());
     } catch (IOException e) {
       throw MergingException.wrapException(e);
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java b/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
index 1eea7a3904..fcc674d7f6 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
@@ -13,10 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
-import static com.google.common.base.Predicates.in;
-import static com.google.common.base.Predicates.not;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
-
 import com.android.SdkConstants;
 import com.android.resources.FolderTypeRelationship;
 import com.android.resources.ResourceFolderType;
@@ -27,6 +23,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import com.google.common.collect.Sets.SetView;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
@@ -40,8 +37,6 @@ import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.EnumSet;
-import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -164,7 +159,7 @@ public class ParsedAndroidData {
     @Override
     public void accept(DataKey key, DataResource value) {
       if (target.containsKey(key)) {
-        target.put(key, value.combineWith(target.get(key)));
+        target.put(key, target.get(key).combineWith(value));
       } else {
         target.put(key, value);
       }
@@ -603,21 +598,17 @@ public class ParsedAndroidData {
     return overwritingResources.entrySet();
   }
 
-  /** Overwrites the resources. */
   ParsedAndroidData overwrite(ParsedAndroidData overwritableData, boolean createConflicts) {
     Map<DataKey, DataResource> newEntries = new LinkedHashMap<>();
-    Set<MergeConflict> newConflicts = new LinkedHashSet<>();
-    newConflicts.addAll(conflicts);
+    Set<MergeConflict> newConflicts =
+        createConflicts ? new LinkedHashSet<MergeConflict>() : conflicts;
     overwrite(
         overwritableData.overwritingResources,
         overwritingResources,
-        new OverwritableConsumer<>(newEntries, newConflicts, createConflicts));
+        new OverwritableConsumer<>(newEntries, newConflicts));
 
     Map<DataKey, DataAsset> newAssets = new LinkedHashMap<>();
-    overwrite(
-        overwritableData.assets,
-        assets,
-        new OverwritableConsumer<>(newAssets, newConflicts, createConflicts));
+    overwrite(overwritableData.assets, assets, new OverwritableConsumer<>(newAssets, newConflicts));
 
     return ParsedAndroidData.of(
         ImmutableSet.copyOf(newConflicts),
@@ -627,76 +618,51 @@ public class ParsedAndroidData {
   }
 
   private static <K extends DataKey, V extends DataValue> void overwrite(
-      Map<K, V> overwritee, Map<K, V> overwriter, BiConsumer<K, V> consumer) {
+      Map<K, V> overwritee, Map<K, V> overwriter, OverwritableConsumer<K, V> consumer) {
+    SetView<K> overwritten = Sets.intersection(overwritee.keySet(), overwriter.keySet());
 
     // Feed the consumer keys and values that will be overwritten, followed by the overwritting
-    // value. This ensures the proper bookkeeping is done inside the consumer.
-    Maps.filterKeys(overwritee, in(overwriter.keySet())).forEach(consumer);
-    overwriter.forEach(consumer);
+    // value. This ensures the proper book keeping is done inside the consumer.
+    for (K key : overwritten) {
+      consumer.accept(key, overwritee.get(key));
+    }
+    for (K key : overwriter.keySet()) {
+      consumer.accept(key, overwriter.get(key));
+    }
   }
 
-  /** Combines combinable resources from other. */
+  /** Combines all combinable resources. */
   ParsedAndroidData combine(ParsedAndroidData other) {
     Map<DataKey, DataResource> combinedResources = new LinkedHashMap<>();
     CombiningConsumer consumer = new CombiningConsumer(combinedResources);
-
-    Maps.filterKeys(other.combiningResources, in(combiningResources.keySet())).forEach(consumer);
-    combiningResources.forEach(consumer);
-
-    return of(conflicts, overwritingResources, ImmutableMap.copyOf(combinedResources), assets);
-  }
-
-  Map<DataKey, DataResource> combineResources(Map<DataKey, DataResource>... combinableResources) {
-    Map<DataKey, DataResource> combinedResources = new LinkedHashMap<>();
-    CombiningConsumer consumer = new CombiningConsumer(combinedResources);
-    for (Map<DataKey, DataResource> resources : combinableResources) {
-      resources.forEach(consumer);
+    for (Entry<DataKey, DataResource> entry :
+        Iterables.concat(combiningResources.entrySet(), other.combiningResources.entrySet())) {
+      consumer.accept(entry.getKey(), entry.getValue());
     }
-    return combinedResources;
+    return of(conflicts, overwritingResources, ImmutableMap.copyOf(combinedResources), assets);
   }
 
-  /** Removes all resources, assets, and conflicts that have keys in the filter. */
-  ParsedAndroidData filterBy(ParsedAndroidData filter) {
-    Set<DataKey> conflictKeys =
-        Sets.union(filter.overwritingResources.keySet(), filter.assets.keySet());
+  /** Removes conflicts, resources, and assets that are in the other. */
+  ParsedAndroidData difference(ParsedAndroidData other) {
     return of(
-        conflicts
-            .stream()
-            .filter(m -> !conflictKeys.contains(m.dataKey()))
-            .collect(toImmutableSet()),
+        ImmutableSet.copyOf(Sets.difference(conflicts, other.conflicts)),
         ImmutableMap.copyOf(
-            Maps.filterKeys(overwritingResources, not(in(filter.overwritingResources.keySet())))),
+            Maps.difference(overwritingResources, other.overwritingResources).entriesOnlyOnLeft()),
         ImmutableMap.copyOf(
-            Maps.filterKeys(combiningResources, not(in(filter.combiningResources.keySet())))),
-        ImmutableMap.copyOf(Maps.filterKeys(assets, not(in(filter.assets.keySet())))));
+            Maps.difference(combiningResources, other.combiningResources).entriesOnlyOnLeft()),
+        ImmutableMap.copyOf(Maps.difference(assets, other.assets).entriesOnlyOnLeft()));
   }
 
-  /**
-   * Creates a union of the android data, with any {@linkplain MergeConflict}s added to the result.
-   */
+  /** Creates a union of both sets. Duplicates are ignored. */
   ParsedAndroidData union(ParsedAndroidData other) {
-    Map<DataKey, DataResource> newEntries = new HashMap<>();
-    Set<MergeConflict> newConflicts = new HashSet<>();
-    final OverwritableConsumer<DataKey, DataResource> resourceConsumer =
-        new OverwritableConsumer<>(newEntries, newConflicts, true);
-    other.overwritingResources.forEach(resourceConsumer);
-    overwritingResources.forEach(resourceConsumer);
-
-    Map<DataKey, DataAsset> newAssets = new HashMap<>();
-    final OverwritableConsumer<DataKey, DataAsset> assetConsumer =
-        new OverwritableConsumer<>(newAssets, newConflicts, true);
-    other.assets.forEach(assetConsumer);
-    assets.forEach(assetConsumer);
-
     return of(
-        ImmutableSet.<MergeConflict>builder()
-            .addAll(conflicts)
-            .addAll(other.conflicts)
-            .addAll(newConflicts)
-            .build(),
-        ImmutableMap.copyOf(newEntries),
-        ImmutableMap.copyOf(combineResources(combiningResources, other.combiningResources)),
-        ImmutableMap.copyOf(newAssets));
+        ImmutableSet.copyOf(Sets.union(conflicts, other.conflicts)),
+        ImmutableMap.copyOf(
+            Iterables.concat(
+                overwritingResources.entrySet(), other.overwritingResources.entrySet())),
+        ImmutableMap.copyOf(
+            Iterables.concat(combiningResources.entrySet(), other.combiningResources.entrySet())),
+        ImmutableMap.copyOf(Iterables.concat(assets.entrySet(), other.assets.entrySet())));
   }
 
   private Iterable<Entry<DataKey, DataResource>> iterateDataResourceEntries() {