diff options
author | Googler <noreply@google.com> | 2016-05-04 19:53:41 +0000 |
---|---|---|
committer | Kristina Chodorow <kchodorow@google.com> | 2016-05-05 19:10:02 +0000 |
commit | 799a82509438572e43a9b62e19890156d721c12f (patch) | |
tree | da1dce6eac8fc72bceb9277f698a9a14c15b8b9e /src/tools/android/java/com/google/devtools | |
parent | 12f54455e945b16df70bf1981e4eabc1d3570f82 (diff) |
4.95 of 5: Merger changes
* Merge conflicts are now bright red warnings, as the previous merger was simultaneously stricter and looser than expected.
* Legacy resource rules are now detected and cause bright red warnings.
* Merge conflicts will test for equality either of file contents or parsed xml
--
MOS_MIGRATED_REVID=121510152
Diffstat (limited to 'src/tools/android/java/com/google/devtools')
4 files changed, 394 insertions, 202 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 965579ef30..3b39205ad8 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 @@ -14,39 +14,189 @@ package com.google.devtools.build.android; import com.google.common.base.Joiner; +import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.devtools.build.android.ParsedAndroidData.ParsedAndroidDataBuildingPathWalker; import com.android.ide.common.res2.MergingException; +import java.io.BufferedInputStream; import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributeView; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; /** * Handles the Merging of ParsedAndroidData. */ public class AndroidDataMerger { + private static final Logger logger = Logger.getLogger(AndroidDataMerger.class.getCanonicalName()); + + /** Interface for comparing paths. */ + interface SourceChecker { + boolean checkEquality(Path one, Path two) throws IOException; + } + + /** Compares two paths by the contents of the files. */ + static class ContentComparingChecker implements SourceChecker { + + static SourceChecker create() { + return new ContentComparingChecker(); + } + + @Override + public boolean checkEquality(Path one, Path two) throws IOException { + // TODO(corysmith): Is there a filesystem hash we can use? + if (getFileSize(one) != getFileSize(two)) { + return false; + } + try (final InputStream oneStream = new BufferedInputStream(Files.newInputStream(one)); + final InputStream twoStream = new BufferedInputStream(Files.newInputStream(two))) { + int bytesRead = 0; + while (true) { + int oneByte = oneStream.read(); + int twoByte = twoStream.read(); + bytesRead++; + if (oneByte == -1 || twoByte == -1) { + if (oneByte == twoByte) { + return true; + } else { + // getFileSize did not return correct size. + logger.severe( + String.format( + "Filesystem size of %s (%s) or %s (%s) is inconsistant with bytes read %s.", + one, + getFileSize(one), + two, + getFileSize(two), + bytesRead)); + return false; + } + } + if (oneByte != twoByte) { + return false; + } + } + } + } + + private long getFileSize(Path path) throws IOException { + return Files.getFileAttributeView(path, BasicFileAttributeView.class).readAttributes().size(); + } + } + + static class NoopSourceChecker implements SourceChecker { + static SourceChecker create() { + return new NoopSourceChecker(); + } + + @Override + public boolean checkEquality(Path one, Path two) { + return false; + } + } + + private final SourceChecker deDuplicator; + + /** Creates a merger with no path deduplication. */ + public static AndroidDataMerger create() { + return new AndroidDataMerger(NoopSourceChecker.create()); + } + + /** Creates a merger with a custom deduplicator. */ + public static AndroidDataMerger create(SourceChecker deDuplicator) { + return new AndroidDataMerger(deDuplicator); + } + + /** Creates a merger with a file contents hashing deduplicator. */ + public static AndroidDataMerger createWithPathDeduplictor() { + return create(ContentComparingChecker.create()); + } + + private AndroidDataMerger(SourceChecker deDuplicator) { + this.deDuplicator = deDuplicator; + } + /** - * Merges DataResources into an UnwrittenMergedAndroidData. + * Merges a list of {@link DependencyAndroidData} with a {@link UnvalidatedAndroidData}. * + * @see AndroidDataMerger#merge(ParsedAndroidData, ParsedAndroidData, UnvalidatedAndroidData, + * boolean) for details. + */ + UnwrittenMergedAndroidData merge( + List<DependencyAndroidData> transitive, + List<DependencyAndroidData> direct, + UnvalidatedAndroidData primary, + boolean allowPrimaryOverrideAll) + throws IOException, MergingException { + Stopwatch timer = Stopwatch.createStarted(); + try { + final ParsedAndroidData.Builder directBuilder = ParsedAndroidData.Builder.newBuilder(); + final ParsedAndroidData.Builder transitiveBuilder = ParsedAndroidData.Builder.newBuilder(); + final AndroidDataSerializer serializer = AndroidDataSerializer.create(); + for (final DependencyAndroidData dependency : direct) { + parseDependencyData(directBuilder, serializer, dependency); + } + for (final DependencyAndroidData dependency : transitive) { + parseDependencyData(transitiveBuilder, serializer, dependency); + } + logger.fine( + String.format("Merged dependencies read in %sms", timer.elapsed(TimeUnit.MILLISECONDS))); + timer.reset().start(); + return merge( + transitiveBuilder.build(), directBuilder.build(), primary, allowPrimaryOverrideAll); + } finally { + logger.fine(String.format("Resources merged in %sms", timer.elapsed(TimeUnit.MILLISECONDS))); + } + } + + private void parseDependencyData( + final ParsedAndroidData.Builder parsedDataBuilder, + final AndroidDataSerializer serializer, + final DependencyAndroidData dependency) + throws IOException, MergingException { + try { + dependency.deserialize(serializer, parsedDataBuilder.consumers()); + } catch (DeserializationException e) { + if (!e.isLegacy()) { + throw new MergingException(e); + } + //TODO(corysmith): List the offending a target here. + logger.warning( + String.format( + "\u001B[31mDEPRECATION:\u001B[0m Legacy resources used for %s", + dependency.getManifest())); + // Legacy android resources -- treat them as direct dependencies. + dependency.walk(ParsedAndroidDataBuildingPathWalker.create(parsedDataBuilder)); + } + } + + /** + * Merges DataResources into an UnwrittenMergedAndroidData. + * <p> * This method has two basic states, library and binary. These are distinguished by - * allowPrimaryOverrideAll, which allows the primary data to overwrite any value in the closure, - * a trait associated with binaries, as a binary is a leaf node. The other semantics are - * slightly more complicated: a given resource can be overwritten only if it resides in the - * direct dependencies of primary data. This forces an explicit simple priority for each resource, + * allowPrimaryOverrideAll, which allows the primary data to overwrite any value in the closure, a + * trait associated with binaries, as a binary is a leaf node. The other semantics are slightly + * more complicated: a given resource can be overwritten only if it resides in the direct + * dependencies of primary data. This forces an explicit simple priority for each resource, * instead of the more subtle semantics of multiple layers of libraries with potential overwrites. + * <p> + * The UnwrittenMergedAndroidData contains only one of each DataKey in both the direct and + * transitive closure. * - * The UnwrittenMergedAndroidData contains only one of each DataKey in both the - * direct and transitive closure. - * - * The merge semantics are as follows: + * The merge semantics are as follows: <pre> * Key: * A(): package A * A(foo): package A with resource symbol foo @@ -76,115 +226,124 @@ public class AndroidDataMerger { * A() -> B(foo),C(foo) -> D() == Conflict * A(foo),B(foo) -> C() -> D(foo) == Valid * A() -> B(foo),C(foo) -> D(foo) == Valid + * </pre> * * @param transitive The transitive dependencies to merge. * @param direct The direct dependencies to merge. * @param primaryData The primary data to merge against. * @param allowPrimaryOverrideAll Boolean that indicates if the primary data will be considered - * 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. + * 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 merge conflicts or issues with parsing resources from - * Primary. - * @throws IOException if there are issues with reading resources. + * Primary. */ UnwrittenMergedAndroidData merge( ParsedAndroidData transitive, ParsedAndroidData direct, UnvalidatedAndroidData primaryData, boolean allowPrimaryOverrideAll) - throws MergingException, IOException { - - // Extract the primary resources. - ParsedAndroidData primary = ParsedAndroidData.from(primaryData); + throws MergingException { - Map<DataKey, DataResource> overwritableDeps = new HashMap<>(); - Map<DataKey, DataAsset> assets = new HashMap<>(); + try { + // Extract the primary resources. + ParsedAndroidData primary = ParsedAndroidData.from(primaryData); - Set<MergeConflict> conflicts = new HashSet<>(); - conflicts.addAll(primary.conflicts()); + Map<DataKey, DataResource> overwritableDeps = new HashMap<>(); + Map<DataKey, DataAsset> assets = new HashMap<>(); - for (MergeConflict conflict : Iterables.concat(direct.conflicts(), transitive.conflicts())) { - if (allowPrimaryOverrideAll - && (primary.containsOverwritable(conflict.dataKey()) - || primary.containsAsset(conflict.dataKey()))) { - continue; + Set<MergeConflict> conflicts = new HashSet<>(); + conflicts.addAll(primary.conflicts()); + for (MergeConflict conflict : Iterables.concat(direct.conflicts(), transitive.conflicts())) { + if (allowPrimaryOverrideAll + && (primary.containsOverwritable(conflict.dataKey()) + || primary.containsAsset(conflict.dataKey()))) { + continue; + } + conflicts.add(conflict); } - conflicts.add(conflict); - } - // resources - for (Map.Entry<DataKey, DataResource> entry : direct.iterateOverwritableEntries()) { - // Direct dependencies are simply overwritten, no conflict. - if (!primary.containsOverwritable(entry.getKey())) { - overwritableDeps.put(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 (primary.containsOverwritable(entry.getKey()) && allowPrimaryOverrideAll) { - continue; + // resources + for (Map.Entry<DataKey, DataResource> entry : direct.iterateOverwritableEntries()) { + // Direct dependencies are simply overwritten, no conflict. + if (!primary.containsOverwritable(entry.getKey())) { + overwritableDeps.put(entry.getKey(), entry.getValue()); + } } - // 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 (primary.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(primary.foundResourceConflict(entry.getKey(), entry.getValue())); - } else { - // If it's in none of the of sources, add it. - overwritableDeps.put(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 (primary.containsOverwritable(entry.getKey()) && allowPrimaryOverrideAll) { + 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 (primary.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(primary.foundResourceConflict(entry.getKey(), entry.getValue())); + } else { + // If it's in none of the of sources, add it. + overwritableDeps.put(entry.getKey(), entry.getValue()); + } } - } - // assets - for (Map.Entry<DataKey, DataAsset> entry : direct.iterateAssetEntries()) { - // Direct dependencies are simply overwritten, no conflict. - if (!primary.containsAsset(entry.getKey())) { - assets.put(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 (primary.containsAsset(entry.getKey()) && allowPrimaryOverrideAll) { - continue; + // assets + for (Map.Entry<DataKey, DataAsset> entry : direct.iterateAssetEntries()) { + // Direct dependencies are simply overwritten, no conflict. + if (!primary.containsAsset(entry.getKey())) { + assets.put(entry.getKey(), entry.getValue()); + } } - // 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 (primary.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(primary.foundAssetConflict(entry.getKey(), entry.getValue())); - } else { - // If it's in none of the of sources, add it. - assets.put(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 (primary.containsAsset(entry.getKey()) && allowPrimaryOverrideAll) { + 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 (primary.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(primary.foundAssetConflict(entry.getKey(), entry.getValue())); + } else { + // If it's in none of the of sources, add it. + assets.put(entry.getKey(), entry.getValue()); + } } - } - if (!conflicts.isEmpty()) { - List<String> messages = new ArrayList<>(); - for (MergeConflict conflict : conflicts) { - messages.add(conflict.toConflictMessage()); + if (!conflicts.isEmpty()) { + List<String> messages = new ArrayList<>(); + for (MergeConflict conflict : conflicts) { + if (!conflict.first().equals(conflict.second()) + && !deDuplicator.checkEquality( + conflict.first().source(), conflict.second().source())) { + messages.add(conflict.toConflictMessage()); + } + } + if (!messages.isEmpty()) { + // TODO(corysmith): Turn these into errors. + logger.warning(Joiner.on("").join(messages)); + } } - throw new MergingException(Joiner.on("\n").join(messages)); - } - return UnwrittenMergedAndroidData.of( - primaryData.getManifest(), - primary, - ParsedAndroidData.of( - ImmutableSet.<MergeConflict>of(), - ImmutableMap.copyOf(overwritableDeps), - direct.mergeNonOverwritable(transitive), - ImmutableMap.copyOf(assets))); + return UnwrittenMergedAndroidData.of( + primaryData.getManifest(), + primary, + ParsedAndroidData.of( + ImmutableSet.<MergeConflict>of(), + ImmutableMap.copyOf(overwritableDeps), + direct.mergeNonOverwritable(transitive), + ImmutableMap.copyOf(assets))); + } catch (IOException e) { + throw new MergingException(e); + } } } diff --git a/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java b/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java index d5d5837258..a892ec817d 100644 --- a/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java +++ b/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java @@ -139,8 +139,9 @@ class DependencyAndroidData { } /** - * Adds all the resource directories as ResourceSets. This acts a loose merge - * strategy as it does not test for overrides. + * Adds all the resource directories as ResourceSets. This acts a loose merge strategy as it does + * not test for overrides. + * * @param resourceSets A list of resource sets to append to. */ void addAsResourceSets(List<ResourceSet> resourceSets) { @@ -152,8 +153,9 @@ class DependencyAndroidData { } /** - * Adds all the asset directories as AssetSets. This acts a loose merge - * strategy as it does not test for overrides. + * Adds all the asset directories as AssetSets. This acts a loose merge strategy as it does not + * test for overrides. + * * @param assetSets A list of asset sets to append to. */ void addAsAssetSets(List<AssetSet> assetSets) { @@ -212,4 +214,15 @@ class DependencyAndroidData { pathWalker.walkAssets(path); } } + + public void deserialize( + AndroidDataSerializer serializer, + KeyValueConsumers consumers) + throws DeserializationException { + // Missing symbolsTxt means the resources where provided via android_resources rules. + if (symbolsTxt == null) { + throw new DeserializationException(true); + } + serializer.read(symbolsTxt, consumers); + } } 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 8ce67fc559..a45993b497 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 @@ -19,7 +19,6 @@ import com.google.common.base.Preconditions; import com.android.annotations.VisibleForTesting; import com.android.annotations.concurrency.Immutable; -import java.nio.file.Path; import java.util.Objects; /** @@ -29,23 +28,23 @@ import java.util.Objects; */ @Immutable public class MergeConflict { - private static final String CONFLICT_MESSAGE = "%s is provided from %s and %s"; + private static final String CONFLICT_MESSAGE = "\n\u001B[31mCONFLICT:\u001B[0m" + + " %s is provided with ambigious priority from: \n\t%s\n\t%s"; private final DataKey dataKey; - private final Path first; - private final Path second; + private final DataValue first; + private final DataValue second; - private MergeConflict(DataKey dataKey, Path first, Path second) { + private MergeConflict(DataKey dataKey, DataValue sortedFirst, DataValue sortedSecond) { this.dataKey = dataKey; - this.first = first; - this.second = second; + this.first = sortedFirst; + this.second = sortedSecond; } /** * Creates a MergeConflict between two DataResources. * - * The {@link DataKey} must match the first.dataKey() and second - * .dataKey(). + * 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. @@ -54,25 +53,34 @@ public class MergeConflict { */ public static MergeConflict between(DataKey dataKey, DataValue first, DataValue second) { Preconditions.checkNotNull(dataKey); - return of(dataKey, first.source(), second.source()); + return of(dataKey, first, second); } @VisibleForTesting - static MergeConflict of(DataKey key, Path first, Path second) { + static MergeConflict of(DataKey key, DataValue first, DataValue second) { // Make sure the paths are always ordered. - Path sortedFirst = first.compareTo(second) > 0 ? first : second; - Path sortedSecond = sortedFirst != first ? first : second; + DataValue sortedFirst = first.source().compareTo(second.source()) > 0 ? first : second; + DataValue sortedSecond = sortedFirst != first ? first : second; return new MergeConflict(key, sortedFirst, sortedSecond); } public String toConflictMessage() { - return String.format(CONFLICT_MESSAGE, dataKey, first, second); + return String.format( + CONFLICT_MESSAGE, dataKey.toPrettyString(), first.source(), second.source()); } public DataKey dataKey() { return dataKey; } + DataValue first() { + return first; + } + + DataValue second() { + return second; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) 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 50c62e9001..891dbe04c1 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 @@ -15,7 +15,6 @@ package com.google.devtools.build.android; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -30,9 +29,9 @@ import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -52,6 +51,72 @@ import javax.xml.stream.XMLStreamException; */ @Immutable public class ParsedAndroidData { + + static class Builder { + private final Map<DataKey, DataResource> overwritingResources; + private final Map<DataKey, DataResource> nonOverwritingResources; + private final Map<DataKey, DataAsset> assets; + private final Set<MergeConflict> conflicts; + private final List<Exception> errors = new ArrayList<>(); + + public Builder( + Map<DataKey, DataResource> overwritingResources, + Map<DataKey, DataResource> nonOverwritingResources, + Map<DataKey, DataAsset> assets, + Set<MergeConflict> conflicts) { + this.overwritingResources = overwritingResources; + this.nonOverwritingResources = nonOverwritingResources; + this.assets = assets; + this.conflicts = conflicts; + } + + static Builder newBuilder() { + final Map<DataKey, DataResource> overwritingResources = new LinkedHashMap<>(); + final Map<DataKey, DataResource> nonOverwritingResources = new LinkedHashMap<>(); + final Map<DataKey, DataAsset> assets = new LinkedHashMap<>(); + final Set<MergeConflict> conflicts = new LinkedHashSet<>(); + return new Builder(overwritingResources, nonOverwritingResources, assets, conflicts); + } + + private void checkForErrors() throws MergingException { + if (!errors.isEmpty()) { + StringBuilder messageBuilder = new StringBuilder(); + for (Exception e : errors) { + messageBuilder.append("\n").append(e.getMessage()); + } + throw new MergingException(messageBuilder.toString()); + } + } + + ParsedAndroidData build() throws MergingException { + checkForErrors(); + return ParsedAndroidData.of( + ImmutableSet.copyOf(conflicts), + ImmutableMap.copyOf(overwritingResources), + ImmutableMap.copyOf(nonOverwritingResources), + ImmutableMap.copyOf(assets)); + } + + ResourceFileVisitor resourceVisitor() { + return new ResourceFileVisitor( + new OverwritableConsumer<>(overwritingResources, conflicts), + new NonOverwritableConsumer(nonOverwritingResources), + errors); + } + + AssetFileVisitor assetVisitorFor(Path path) { + return new AssetFileVisitor( + RelativeAssetPath.Factory.of(path), new OverwritableConsumer<>(assets, conflicts)); + } + + public KeyValueConsumers consumers() { + return KeyValueConsumers.of( + new OverwritableConsumer<>(overwritingResources, conflicts), + new NonOverwritableConsumer(nonOverwritingResources), + new OverwritableConsumer<>(assets, conflicts)); + } + } + /** A Consumer style interface that will appendTo a DataKey and DataValue. */ interface KeyValueConsumer<K extends DataKey, V extends DataValue> { void consume(K key, V value); @@ -62,7 +127,7 @@ public class ParsedAndroidData { private Map<DataKey, DataResource> target; - public NonOverwritableConsumer(Map<DataKey, DataResource> target) { + NonOverwritableConsumer(Map<DataKey, DataResource> target) { this.target = target; } @@ -98,67 +163,31 @@ public class ParsedAndroidData { /** * An AndroidDataPathWalker that collects DataAsset and DataResources for an ParsedAndroidData. */ - private static final class ParsedAndroidDataBuildingPathWalker implements AndroidDataPathWalker { - private final Set<MergeConflict> conflicts; - private final Map<DataKey, DataAsset> assets; - private final ResourceFileVisitor resourceVisitor; + static final class ParsedAndroidDataBuildingPathWalker implements AndroidDataPathWalker { private static final ImmutableSet<FileVisitOption> FOLLOW_LINKS = ImmutableSet.of(FileVisitOption.FOLLOW_LINKS); - private Map<DataKey, DataResource> overwritingResources; - private Map<DataKey, DataResource> nonOverwritingResources; - - private static ParsedAndroidDataBuildingPathWalker create() { - final Map<DataKey, DataResource> overwritingResources = new HashMap<>(); - final Map<DataKey, DataResource> nonOverwritingResources = new HashMap<>(); - final Map<DataKey, DataAsset> assets = new HashMap<>(); - final Set<MergeConflict> conflicts = new HashSet<>(); - - final ResourceFileVisitor resourceVisitor = - new ResourceFileVisitor( - new OverwritableConsumer<>(overwritingResources, conflicts), - new NonOverwritableConsumer(nonOverwritingResources)); - return new ParsedAndroidDataBuildingPathWalker( - conflicts, assets, overwritingResources, nonOverwritingResources, resourceVisitor); + private final Builder builder; + + private ParsedAndroidDataBuildingPathWalker(Builder builder) { + this.builder = builder; } - private ParsedAndroidDataBuildingPathWalker( - Set<MergeConflict> conflicts, - Map<DataKey, DataAsset> assets, - Map<DataKey, DataResource> overwritingResources, - Map<DataKey, DataResource> nonOverwritingResources, - ResourceFileVisitor resourceVisitor) { - this.conflicts = conflicts; - this.assets = assets; - this.overwritingResources = overwritingResources; - this.nonOverwritingResources = nonOverwritingResources; - this.resourceVisitor = resourceVisitor; + static ParsedAndroidDataBuildingPathWalker create(Builder builder) { + return new ParsedAndroidDataBuildingPathWalker(builder); } @Override public void walkResources(Path path) throws IOException { - Files.walkFileTree(path, FOLLOW_LINKS, Integer.MAX_VALUE, resourceVisitor); + Files.walkFileTree(path, FOLLOW_LINKS, Integer.MAX_VALUE, builder.resourceVisitor()); } @Override public void walkAssets(Path path) throws IOException { - Files.walkFileTree( - path, - FOLLOW_LINKS, - Integer.MAX_VALUE, - new AssetFileVisitor( - RelativeAssetPath.Factory.of(path), new OverwritableConsumer<>(assets, conflicts))); + Files.walkFileTree(path, FOLLOW_LINKS, Integer.MAX_VALUE, builder.assetVisitorFor(path)); } - /** - * Creates an {@link ParsedAndroidData} from {@link DataAsset} and {@link DataResource}. - */ - public ParsedAndroidData createParsedAndroidData() throws MergingException { - resourceVisitor.checkForErrors(); - return ParsedAndroidData.of( - ImmutableSet.copyOf(conflicts), - ImmutableMap.copyOf(overwritingResources), - ImmutableMap.copyOf(nonOverwritingResources), - ImmutableMap.copyOf(assets)); + ParsedAndroidData createParsedAndroidData() throws MergingException { + return builder.build(); } } @@ -169,7 +198,7 @@ public class ParsedAndroidData { private final RelativeAssetPath.Factory dataKeyFactory; private KeyValueConsumer<DataKey, DataAsset> assetConsumer; - public AssetFileVisitor( + AssetFileVisitor( RelativeAssetPath.Factory dataKeyFactory, KeyValueConsumer<DataKey, DataAsset> assetConsumer) { this.dataKeyFactory = dataKeyFactory; @@ -191,18 +220,20 @@ public class ParsedAndroidData { * A FileVisitor that walks a resource tree and extract FullyQualifiedName and resource values. */ private static class ResourceFileVisitor extends SimpleFileVisitor<Path> { - private final List<Exception> errors = new ArrayList<>(); - private final OverwritableConsumer<DataKey, DataResource> overwritingConsumer; - private final NonOverwritableConsumer nonOverwritingConsumer; + private final KeyValueConsumer<DataKey, DataResource> overwritingConsumer; + private final KeyValueConsumer<DataKey, DataResource> nonOverwritingConsumer; + private final List<Exception> errors; private boolean inValuesSubtree; private FullyQualifiedName.Factory fqnFactory; private final XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory(); - public ResourceFileVisitor( - OverwritableConsumer<DataKey, DataResource> overwritingConsumer, - NonOverwritableConsumer nonOverwritingConsumer) { + ResourceFileVisitor( + KeyValueConsumer<DataKey, DataResource> overwritingConsumer, + KeyValueConsumer<DataKey, DataResource> nonOverwritingConsumer, + List<Exception> errors) { this.overwritingConsumer = overwritingConsumer; this.nonOverwritingConsumer = nonOverwritingConsumer; + this.errors = errors; } private static String deriveRawFullyQualifiedName(Path path) { @@ -222,37 +253,19 @@ public class ParsedAndroidData { return pathWithExtension; } - private void checkForErrors() throws MergingException { - if (!getErrors().isEmpty()) { - StringBuilder errors = new StringBuilder(); - for (Exception e : getErrors()) { - errors.append("\n").append(e.getMessage()); - } - throw new MergingException(errors.toString()); - } - } - @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { final String[] dirNameAndQualifiers = dir.getFileName().toString().split("-"); inValuesSubtree = "values".equals(dirNameAndQualifiers[0]); - fqnFactory = FullyQualifiedName.Factory.from(getQualifiers(dirNameAndQualifiers)); + fqnFactory = FullyQualifiedName.Factory.fromDirectoryName(dirNameAndQualifiers); return FileVisitResult.CONTINUE; } - private List<String> getQualifiers(String[] dirNameAndQualifiers) { - if (dirNameAndQualifiers.length == 1) { - return ImmutableList.of(); - } - return Arrays.asList( - Arrays.copyOfRange(dirNameAndQualifiers, 1, dirNameAndQualifiers.length)); - } - @Override public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException { try { - if (!Files.isDirectory(path)) { + if (!Files.isDirectory(path) && !path.getFileName().toString().startsWith(".")) { if (inValuesSubtree) { DataResourceXml.parse( xmlInputFactory, path, fqnFactory, overwritingConsumer, nonOverwritingConsumer); @@ -267,10 +280,6 @@ public class ParsedAndroidData { } return super.visitFile(path, attrs); } - - public List<Exception> getErrors() { - return errors; - } } /** Creates ParsedAndroidData of conflicts, assets overwriting and nonOverwriting resources. */ @@ -295,7 +304,7 @@ public class ParsedAndroidData { public static ParsedAndroidData from(UnvalidatedAndroidData primary) throws IOException, MergingException { final ParsedAndroidDataBuildingPathWalker pathWalker = - ParsedAndroidDataBuildingPathWalker.create(); + ParsedAndroidDataBuildingPathWalker.create(Builder.newBuilder()); primary.walk(pathWalker); return pathWalker.createParsedAndroidData(); } @@ -314,7 +323,7 @@ public class ParsedAndroidData { public static ParsedAndroidData from(List<DependencyAndroidData> dependencyAndroidDataList) throws IOException, MergingException { final ParsedAndroidDataBuildingPathWalker pathWalker = - ParsedAndroidDataBuildingPathWalker.create(); + ParsedAndroidDataBuildingPathWalker.create(Builder.newBuilder()); for (DependencyAndroidData data : dependencyAndroidDataList) { data.walk(pathWalker); } @@ -369,28 +378,30 @@ public class ParsedAndroidData { /** * Returns a list of resources that would overwrite other values when defined. * - * <p>Example: + * <p> + * Example: * * A string resource (string.Foo=bar) could be redefined at string.Foo=baz. * * @return A map of key -> overwriting resources. */ @VisibleForTesting - public Map<DataKey, DataResource> getOverwritingResources() { + Map<DataKey, DataResource> getOverwritingResources() { return overwritingResources; } /** * Returns a list of resources that would not overwrite other values when defined. * - * <p>Example: + * <p> + * Example: * * A id resource (id.Foo) could be redefined at id.Foo with no adverse effects. * * @return A map of key -> non-overwriting resources. */ @VisibleForTesting - public Map<DataKey, DataResource> getNonOverwritingResources() { + Map<DataKey, DataResource> getNonOverwritingResources() { return nonOverwritingResources; } @@ -398,7 +409,8 @@ public class ParsedAndroidData { * Returns a list of assets. * * Assets always overwrite during merging, just like overwriting resources. - * <p>Example: + * <p> + * Example: * * A text asset (foo/bar.txt, containing fooza) could be replaced with (foo/bar.txt, containing * ouza!) depending on the merging process. @@ -417,7 +429,7 @@ public class ParsedAndroidData { return overwritingResources.entrySet(); } - public Iterable<Entry<DataKey, DataResource>> iterateDataResourceEntries() { + Iterable<Entry<DataKey, DataResource>> iterateDataResourceEntries() { return Iterables.concat(overwritingResources.entrySet(), nonOverwritingResources.entrySet()); } |