Skip to content

Commit

Permalink
Extract Qualifiers class for reuse.
Browse files Browse the repository at this point in the history 
Also, introduce caching for duplicated qualifiers.

Rolling Forward:

Fixed attempting to parse non-resource directories with dashes as resource directories.
Added test for this case.

RELNOTES: None
PiperOrigin-RevId: 195658978
  • Loading branch information
@corbinrsmith-work
corbinrsmith-work authored and Copybara-Service committed May 7, 2018
1 parent 2aa8130 commit 6846d71
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 73 deletions.
188 changes: 126 additions & 62 deletions src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Strings.emptyToNull;

import com.android.SdkConstants;
import com.android.annotations.VisibleForTesting;
import com.android.ide.common.resources.configuration.FolderConfiguration;
import com.android.ide.common.resources.configuration.ResourceQualifier;
import com.android.resources.ResourceFolderType;
import com.android.resources.ResourceType;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
Expand All @@ -33,6 +36,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -58,6 +62,7 @@ public class FullyQualifiedName implements DataKey {
// Using a HashMap to deduplicate the instances -- the key retrieves a single instance.
private static final ConcurrentMap<FullyQualifiedName, FullyQualifiedName> instanceCache =
new ConcurrentHashMap<>();

private static final AtomicInteger cacheHit = new AtomicInteger(0);
private final String pkg;
private final ImmutableList<String> qualifiers;
Expand Down Expand Up @@ -185,7 +190,7 @@ public String toPrettyString() {
// TODO(corysmith): Add package when we start tracking it.
return String.format(
"%s%s/%s",
pkg != DEFAULT_PACKAGE ? pkg + ':' : "",
DEFAULT_PACKAGE.equals(pkg) ? "" : pkg + ':',
DASH_JOINER.join(
ImmutableList.<String>builder().add(type.getName()).addAll(qualifiers).build()),
name);
Expand Down Expand Up @@ -398,29 +403,29 @@ public String toString() {

/** Represents the type of a {@link FullyQualifiedName}. */
public interface Type {
public String getName();
String getName();

public ConcreteType getType();
ConcreteType getType();

public boolean isOverwritable(FullyQualifiedName fqn);
boolean isOverwritable(FullyQualifiedName fqn);

public int compareTo(Type other);
int compareTo(Type other);

@Override
public boolean equals(Object obj);
boolean equals(Object obj);

@Override
public int hashCode();
int hashCode();

@Override
public String toString();
String toString();

/**
* The category of type that a {@link Type} can be.
*
* <p><em>Note:</em> used for strict ordering of {@link FullyQualifiedName}s.
*/
public enum ConcreteType {
enum ConcreteType {
RESOURCE_TYPE,
VIRTUAL_TYPE;
}
Expand Down Expand Up @@ -478,53 +483,44 @@ public String toString() {
}
}

/** A factory for parsing an generating FullyQualified names with qualifiers and package. */
public static class Factory {
/** Represents the configuration qualifiers in a resource directory. */
public static class Qualifiers {

private static final Qualifiers EMPTY_QUALIFIERS = new Qualifiers(null, ImmutableList.of());

// Qualifiers are reasonably expensive to create, so cache them on directory names.
private static final ConcurrentMap<String, Qualifiers> qualifierCache =
new ConcurrentHashMap<>();

public static final String INVALID_QUALIFIED_NAME_MESSAGE_NO_MATCH =
String.format(
"%%s is not a valid qualified name. "
+ "It should be in the pattern [package:]{%s}/name",
Joiner.on(",")
.join(
ImmutableList.<String>builder()
.add(ResourceType.getNames())
.add(VirtualType.getNames())
.build()));
public static final String INVALID_QUALIFIED_NAME_MESSAGE_NO_TYPE_OR_NAME =
String.format(
"Could not find either resource type (%%s) or name (%%s) in %%s. "
+ "It should be in the pattern [package:]{%s}/name",
Joiner.on(",")
.join(
ImmutableList.<String>builder()
.add(ResourceType.getNames())
.add(VirtualType.getNames())
.build()));
public static final String INVALID_QUALIFIERS = "%s contains invalid qualifiers.";
private static final Pattern PARSING_REGEX =
Pattern.compile(
"(?:(?<package>[^:]+):){0,1}(?<type>[^-/]+)(?:[^/]*)/(?:(?:(?<namespace>\\{[^}]+\\}))"
+ "|(?:(?<misplacedPackage>[^:]+):)){0,1}(?<name>.+)");
private final ResourceFolderType folderType;
private final ImmutableList<String> qualifiers;
private final String pkg;

private Factory(ImmutableList<String> qualifiers, String pkg) {
private Qualifiers(ResourceFolderType folderType, ImmutableList<String> qualifiers) {
this.folderType = folderType;
this.qualifiers = qualifiers;
this.pkg = pkg;
}

/** Creates a factory with default package from a directory name split on '-'. */
public static Factory fromDirectoryName(String[] dirNameAndQualifiers) {
return from(getQualifiers(dirNameAndQualifiers));
public static Qualifiers parseFrom(String directoryName) {
return qualifierCache.computeIfAbsent(
directoryName, d -> getQualifiers(Splitter.on(SdkConstants.RES_QUALIFIER_SEP).split(d)));
}

private static Qualifiers getQualifiers(String... dirNameAndQualifiers) {
return getQualifiers(Arrays.asList(dirNameAndQualifiers));
}

private static List<String> getQualifiers(String[] dirNameAndQualifiers) {
private static Qualifiers getQualifiers(Iterable<String> dirNameAndQualifiers) {
PeekingIterator<String> rawQualifiers =
Iterators.peekingIterator(Iterators.forArray(dirNameAndQualifiers));
Iterators.peekingIterator(dirNameAndQualifiers.iterator());
// Remove directory name
rawQualifiers.next();
List<String> transformedLocaleQualifiers = new ArrayList<>();
final ResourceFolderType folderType = ResourceFolderType.getTypeByName(rawQualifiers.next());

// If there is no folder type, there are no qualifiers to parse.
if (folderType == null) {
return EMPTY_QUALIFIERS;
}

List<String> handledQualifiers = new ArrayList<>();
// Do some substitution of language/region qualifiers.
while (rawQualifiers.hasNext()) {
Expand All @@ -533,14 +529,14 @@ private static List<String> getQualifiers(String[] dirNameAndQualifiers) {
&& rawQualifiers.hasNext()
&& "419".equalsIgnoreCase(rawQualifiers.peek())) {
// Replace the es-419.
transformedLocaleQualifiers.add("b+es+419");
handledQualifiers.add("b+es+419");
// Consume the next value, as it's been replaced.
rawQualifiers.next();
} else if ("sr".equalsIgnoreCase(qualifier)
&& rawQualifiers.hasNext()
&& "rlatn".equalsIgnoreCase(rawQualifiers.peek())) {
// Replace the sr-rLatn.
transformedLocaleQualifiers.add("b+sr+Latn");
handledQualifiers.add("b+sr+Latn");
// Consume the next value, as it's been replaced.
rawQualifiers.next();
} else {
Expand All @@ -558,21 +554,12 @@ private static List<String> getQualifiers(String[] dirNameAndQualifiers) {
}
config.normalize();

// This is fragile but better than the Gradle scheme of just dropping
// entire subtrees.
ImmutableList.Builder<String> builder = ImmutableList.<String>builder();
addIfNotNull(config.getCountryCodeQualifier(), builder);
addIfNotNull(config.getNetworkCodeQualifier(), builder);
if (transformedLocaleQualifiers.isEmpty()) {
addIfNotNull(config.getLocaleQualifier(), builder);
} else {
builder.addAll(transformedLocaleQualifiers);
}
// index 3 is past the country code, network code, and locale indices.
for (int i = 3; i < FolderConfiguration.getQualifierCount(); ++i) {
for (int i = 0; i < FolderConfiguration.getQualifierCount(); ++i) {
addIfNotNull(config.getQualifier(i), builder);
}
return builder.build();
return new Qualifiers(folderType, builder.build());
}

private static void addIfNotNull(
Expand All @@ -582,12 +569,89 @@ private static void addIfNotNull(
}
}

/** Returns the qualifiers as a list of strings. */
public List<String> asList() {
return qualifiers;
}

public ResourceFolderType asFolderType() {
return folderType;
}

@VisibleForTesting
public static Qualifiers fromList(List<String> qualifiers) {
return new Qualifiers(null, ImmutableList.copyOf(qualifiers));
}
}

/** A factory for parsing an generating FullyQualified names with qualifiers and package. */
public static class Factory {

public static final String INVALID_QUALIFIED_NAME_MESSAGE_NO_MATCH =
String.format(
"%%s is not a valid qualified name. "
+ "It should be in the pattern [package:]{%s}/name",
Joiner.on(",")
.join(
ImmutableList.<String>builder()
.add(ResourceType.getNames())
.add(VirtualType.getNames())
.build()));
public static final String INVALID_QUALIFIED_NAME_MESSAGE_NO_TYPE_OR_NAME =
String.format(
"Could not find either resource type (%%s) or name (%%s) in %%s. "
+ "It should be in the pattern [package:]{%s}/name",
Joiner.on(",")
.join(
ImmutableList.<String>builder()
.add(ResourceType.getNames())
.add(VirtualType.getNames())
.build()));
private static final Pattern PARSING_REGEX =
Pattern.compile(
"(?:(?<package>[^:]+):){0,1}(?<type>[^-/]+)(?:[^/]*)/(?:(?:(?<namespace>\\{[^}]+\\}))"
+ "|(?:(?<misplacedPackage>[^:]+):)){0,1}(?<name>.+)");
// private final ImmutableList<String> qualifiers;

private final String pkg;

private final Qualifiers qs;

private Factory(Qualifiers qualifiers, String pkg) {
// this.qualifiers = qualifiers;
this.pkg = pkg;
this.qs = qualifiers;
}

/** Creates a factory with default package from a directory name split on '-'. */
@VisibleForTesting
public static Factory fromDirectoryName(String... dirNameAndQualifiers) {
return using(Qualifiers.getQualifiers(dirNameAndQualifiers), DEFAULT_PACKAGE);
}

/** Creates a factory with default package from a directory with '-' separating qualifiers. */
public static Factory fromDirectoryName(String dirNameAndQualifiers) {
return using(Qualifiers.parseFrom(dirNameAndQualifiers), DEFAULT_PACKAGE);
}

@VisibleForTesting
public static Factory from(List<String> qualifiers, String pkg) {
return new Factory(ImmutableList.copyOf(qualifiers), pkg.isEmpty() ? DEFAULT_PACKAGE : pkg);
return using(Qualifiers.fromList(qualifiers), pkg);
}

@VisibleForTesting
public static Factory from(List<String> qualifiers) {
return from(ImmutableList.copyOf(qualifiers), DEFAULT_PACKAGE);
return from(qualifiers, DEFAULT_PACKAGE);
}

/** Creates a factory with the qualifiers and package. */
public static Factory using(Qualifiers qualifiers) {
return using(qualifiers, DEFAULT_PACKAGE);
}

/** Creates a factory with the qualifiers and package. */
public static Factory using(Qualifiers qualifiers, String pkg) {
return new Factory(qualifiers, pkg.isEmpty() ? DEFAULT_PACKAGE : pkg);
}

private static String deriveRawFullyQualifiedName(Path source) {
Expand Down Expand Up @@ -620,7 +684,7 @@ private static String getSourceExtension(Path source) {
}

public FullyQualifiedName create(Type type, String name, String pkg) {
return FullyQualifiedName.of(pkg, qualifiers, type, name);
return FullyQualifiedName.of(pkg, qs.asList(), type, name);
}

public FullyQualifiedName create(ResourceType type, String name) {
Expand Down Expand Up @@ -662,7 +726,7 @@ public FullyQualifiedName parse(String raw) {
String.format(INVALID_QUALIFIED_NAME_MESSAGE_NO_TYPE_OR_NAME, type, name, raw));
}

return FullyQualifiedName.of(parsedPackage, qualifiers, type, name);
return FullyQualifiedName.of(parsedPackage, qs.asList(), type, name);
}

private String firstNonNull(String... values) {
Expand Down
16 changes: 8 additions & 8 deletions src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
import com.google.devtools.build.android.FullyQualifiedName.Qualifiers;
import com.google.devtools.build.android.xml.StyleableXmlResourceValue;
import java.io.IOException;
import java.nio.file.FileVisitOption;
Expand Down Expand Up @@ -298,17 +299,16 @@ private static class ResourceFileVisitor extends SimpleFileVisitor<Path> {
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
throws IOException {
final String[] dirNameAndQualifiers =
dir.getFileName().toString().split(SdkConstants.RES_QUALIFIER_SEP);
folderType = ResourceFolderType.getTypeByName(dirNameAndQualifiers[0]);
if (folderType == null) {
return FileVisitResult.CONTINUE;
}
try {
fqnFactory = FullyQualifiedName.Factory.fromDirectoryName(dirNameAndQualifiers);
final Qualifiers qualifiers = Qualifiers.parseFrom(dir.getFileName().toString());
folderType = qualifiers.asFolderType();
if (folderType == null) {
return FileVisitResult.CONTINUE;
}
fqnFactory = FullyQualifiedName.Factory.using(qualifiers);
return FileVisitResult.CONTINUE;
} catch (IllegalArgumentException e) {
logger.warning(
logger.severe(
String.format("%s is an invalid resource directory due to %s", dir, e.getMessage()));
return FileVisitResult.SKIP_SUBTREE;
}
Expand Down
4 changes: 1 addition & 3 deletions src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.build.android.aapt2;

import com.android.SdkConstants;
import com.android.builder.core.VariantType;
import com.android.repository.Revision;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -195,8 +194,7 @@ private Path createAttributesProto(
Namespaces namespaces = Namespaces.from(qName);
String attributeName = namespaceUri.isEmpty() ? localPart : prefix + ":" + localPart;

final String[] dirNameAndQualifiers = type.split(SdkConstants.RES_QUALIFIER_SEP);
Factory fqnFactory = Factory.fromDirectoryName(dirNameAndQualifiers);
Factory fqnFactory = Factory.fromDirectoryName(type);
FullyQualifiedName fqn =
fqnFactory.create(VirtualType.RESOURCES_ATTRIBUTE, qName.toString());
ResourcesAttribute resourceAttribute =
Expand Down

0 comments on commit 6846d71

Please sign in to comment.