Simplify the merge code, and fix a few incorrectly declared tests.

RELNOTES: None
PiperOrigin-RevId: 174485947

1 files changed, 16 insertions, 119 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 8e550a2d60..61d415787c 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.Iterables;
+import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
@@ -24,10 +24,7 @@ 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;
@@ -230,8 +227,7 @@ 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(
@@ -258,123 +254,25 @@ class AndroidDataMerger {
   private UnwrittenMergedAndroidData doMerge(
       ParsedAndroidData transitive,
       ParsedAndroidData direct,
-      ParsedAndroidData parsedPrimary,
+      ParsedAndroidData primary,
       Path primaryManifest,
       boolean allowPrimaryOverrideAll,
       boolean throwOnResourceConflict) {
     try {
       // Create the builders for the final parsed data.
-      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());
-        }
-      }
+      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);
 
-      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());
-        }
-      }
+      Set<MergeConflict> conflicts =
+          Sets.union(mergedPrimary.conflicts(), mergedTransitive.conflicts());
 
       if (!conflicts.isEmpty()) {
         List<String> messages = new ArrayList<>();
@@ -391,8 +289,7 @@ class AndroidDataMerger {
           logger.warning(conflictMessage);
         }
       }
-      return UnwrittenMergedAndroidData.of(
-          primaryManifest, primaryBuilder.build(), transitiveBuilder.build());
+      return UnwrittenMergedAndroidData.of(primaryManifest, mergedPrimary, mergedTransitive);
     } catch (IOException e) {
       throw MergingException.wrapException(e);
     }