Store non-english string files as assets.
diff --git a/lib/BUCK b/lib/BUCK index 47aa52b..62c3557 100644 --- a/lib/BUCK +++ b/lib/BUCK
@@ -109,6 +109,8 @@ name = 'jackson-databind', binary_jar = 'jackson-databind-2.0.5.jar', visibility = [ + '//src/com/facebook/buck/android:rules', + '//src/com/facebook/buck/android:steps', '//src/com/facebook/buck/cli:cli', '//src/com/facebook/buck/command:command', '//src/com/facebook/buck/event/listener:listener',
diff --git a/src/com/facebook/buck/android/AndroidBinaryRule.java b/src/com/facebook/buck/android/AndroidBinaryRule.java index 9291d75..ed58ae6 100644 --- a/src/com/facebook/buck/android/AndroidBinaryRule.java +++ b/src/com/facebook/buck/android/AndroidBinaryRule.java
@@ -50,12 +50,12 @@ import com.facebook.buck.shell.SymlinkFilesIntoDirectoryStep; import com.facebook.buck.step.ExecutionContext; import com.facebook.buck.step.Step; +import com.facebook.buck.step.fs.CopyStep; import com.facebook.buck.step.fs.MakeCleanDirectoryStep; import com.facebook.buck.step.fs.MkdirAndSymlinkFileStep; import com.facebook.buck.step.fs.MkdirStep; import com.facebook.buck.util.BuckConstant; import com.facebook.buck.util.DefaultDirectoryTraverser; -import com.facebook.buck.util.DefaultFilteredDirectoryCopier; import com.facebook.buck.util.DirectoryTraversal; import com.facebook.buck.util.DirectoryTraverser; import com.facebook.buck.util.HumanReadableException; @@ -67,6 +67,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -274,6 +275,16 @@ return this.compressResources; } + public boolean isStoreStringsAsAssets() { + // TODO(natthu): introduce another parameter to AndroidBinaryRule and replace this with + // isCompressResources() && this.isStoreStringsAsAssets; + return false; + } + + public boolean requiresResourceFilter() { + return resourceFilter.isEnabled() || isStoreStringsAsAssets(); + } + public FilterResourcesStep.ResourceFilter getResourceFilter() { return this.resourceFilter; } @@ -331,10 +342,6 @@ } } - public DexSplitMode getDexSplitMode() { - return dexSplitMode; - } - /** The APK at this path is the final one that points to an APK that a user should install. */ @Override public String getApkPath() { @@ -361,6 +368,51 @@ return inputs.build(); } + /** + * Sets up filtering of resources, images/drawables and strings in particular, based on build + * rule parameters {@code resourceFilter} and {@code isStoreStringsAsAssets}. + * + * {@link com.facebook.buck.android.FilterResourcesStep.ResourceFilter} {@code resourceFilter} + * determines which drawables end up in the APK (based on density - mdpi, hdpi etc), and also + * whether higher density drawables get scaled down to the specified density (if not present). + * + * {@code isStoreStringsAsAssets} determines whether non-english string resources are packaged + * separately as assets (and not bundled together into the {@code resources.arsc} file). + * + * @return The set of resource directories that will eventually contain filtered resources. + */ + private Set<String> getFilteredResourceDirectories( + ImmutableList.Builder<Step> commands, + Set<String> resourceDirectories) { + ImmutableBiMap.Builder<String, String> filteredResourcesDirMapBuilder = ImmutableBiMap.builder(); + String resDestinationBasePath = getBinPath("__filtered__%s__"); + int count = 0; + for (String resDir : resourceDirectories) { + filteredResourcesDirMapBuilder.put(resDir, + java.nio.file.Paths.get(resDestinationBasePath, String.valueOf(count++)).toString()); + } + + ImmutableBiMap<String, String> resSourceToDestDirMap = filteredResourcesDirMapBuilder.build(); + FilterResourcesStep.Builder filterResourcesStepBuilder = FilterResourcesStep.builder() + .setInResToOutResDirMap(resSourceToDestDirMap) + .setResourceFilter(resourceFilter); + + if (isStoreStringsAsAssets()) { + filterResourcesStepBuilder.enableStringsFilter(); + } + + FilterResourcesStep filterResourcesStep = filterResourcesStepBuilder.build(); + commands.add(filterResourcesStep); + + if (isStoreStringsAsAssets()) { + Path tmpStringsDirPath = getPathForTmpStringAssetsDirectory(); + commands.add(new MakeCleanDirectoryStep(tmpStringsDirPath)); + commands.add(new CompileStringsStep(filterResourcesStep, tmpStringsDirPath)); + } + + return resSourceToDestDirMap.values(); + } + @Override public List<Step> getBuildSteps(BuildContext context, BuildableContext buildableContext) { ImmutableList.Builder<Step> commands = ImmutableList.builder(); @@ -380,20 +432,8 @@ Set<String> resDirectories = transitiveDependencies.resDirectories; Set<String> rDotJavaPackages = transitiveDependencies.rDotJavaPackages; - - FilterResourcesStep.ResourceFilter resourceFilter = getResourceFilter(); - // If resource filtering was requested (currently only by dpi). - if (resourceFilter.isEnabled()) { - FilterResourcesStep filterResourcesCommand = new FilterResourcesStep( - resDirectories, - new File(getBinPath("__filtered__%s__")), - resourceFilter.getDensity(), - DefaultFilteredDirectoryCopier.getInstance(), - FilterResourcesStep.DefaultDrawableFinder.getInstance(), - resourceFilter.shouldDownscale() ? FilterResourcesStep.ImageMagickScaler.getInstance() : null - ); - commands.add(filterResourcesCommand); - resDirectories = filterResourcesCommand.getFilteredResourceDirectories(); + if (requiresResourceFilter()) { + resDirectories = getFilteredResourceDirectories(commands, resDirectories); } // Extract the resources from third-party jars. @@ -547,7 +587,8 @@ Optional<String> assetsDirectory; if (transitiveDependencies.assetsDirectories.isEmpty() && extraAssets.isEmpty() - && transitiveDependencies.nativeLibAssetsDirectories.isEmpty()) { + && transitiveDependencies.nativeLibAssetsDirectories.isEmpty() + && !isStoreStringsAsAssets()) { assetsDirectory = Optional.absent(); } else { assetsDirectory = Optional.of(getPathToAllAssetsDirectory()); @@ -561,6 +602,15 @@ } } + if (isStoreStringsAsAssets()) { + Path stringAssetsDir = java.nio.file.Paths.get(assetsDirectory.get()).resolve("strings"); + commands.add(new MakeCleanDirectoryStep(stringAssetsDir)); + commands.add(new CopyStep( + getPathForTmpStringAssetsDirectory(), + stringAssetsDir, + /* shouldRecurse */ true)); + } + commands.add(new MkdirStep(outputGenDirectory)); if (!canSkipAaptResourcePackaging()) { @@ -727,8 +777,11 @@ @VisibleForTesting String getPathToAllAssetsDirectory() { - String format = "__assets_%s__"; - return getBinPath(format); + return getBinPath("__assets_%s__"); + } + + private Path getPathForTmpStringAssetsDirectory() { + return java.nio.file.Paths.get(getBinPath("__strings_%s__")); } /**
diff --git a/src/com/facebook/buck/android/BUCK b/src/com/facebook/buck/android/BUCK index 8f45d28..e8461d7 100644 --- a/src/com/facebook/buck/android/BUCK +++ b/src/com/facebook/buck/android/BUCK
@@ -74,6 +74,8 @@ ':split_dex', ':steps', '//lib:guava', + '//lib:jackson-core', + '//lib:jackson-databind', '//lib:jsr305', '//src/com/facebook/buck/dalvik:dalvik', '//src/com/facebook/buck/graph:graph', @@ -123,6 +125,8 @@ ':exceptions', ':split_dex', '//lib:guava', + '//lib:jackson-core', + '//lib:jackson-databind', '//lib:jsr305', '//lib:sdklib', '//src/com/facebook/buck/dalvik:dalvik',
diff --git a/src/com/facebook/buck/android/CompileStringsStep.java b/src/com/facebook/buck/android/CompileStringsStep.java new file mode 100644 index 0000000..50a638c --- /dev/null +++ b/src/com/facebook/buck/android/CompileStringsStep.java
@@ -0,0 +1,286 @@ +/* + * Copyright 2013-present Facebook, Inc. + * + * 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.facebook.buck.android; + +import com.facebook.buck.event.ThrowableLogEvent; +import com.facebook.buck.step.ExecutionContext; +import com.facebook.buck.step.Step; +import com.facebook.buck.util.ProjectFilesystem; +import com.facebook.buck.util.XmlDomParser; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; + +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * This {@link Step} takes in a {@link FilterResourcesStep} that provides a list of string resource + * files (strings.xml), groups them by locales, and for each locale generates a json file with all + * the string resources for that locale. + * + * <p>A typical strings.xml file looks like: + * <pre> + * {@code + * <?xml version="1.0" encoding="utf-8"?> + * <resources> + * <string name="resource_name1">I am a string.</string> + * <string name="resource_name2">I am another string.</string> + * <plurals name="time_hours_ago"> + * <item quantity="one">1 minute ago</item> + * <item quantity="other">%d minutes ago</item> + * </plurals> + * <string-array name="logging_levels"> + * <item>Default</item> + * <item>Verbose</item> + * <item>Debug</item> + * </string-array> + * </resources> + * } + * </pre></p> + * + * <p>For more information on the xml file format, refer to: + * <a href="http://developer.android.com/guide/topics/resources/string-resource.html"> + * String Resources - Android Developers + * </a></p> + * + * <p>So for each supported locale in a project, this step goes through all such xml files for that + * locale, and builds a map of resource name to resource value, where resource value is either: + * <ol> + * <li> a string </li> + * <li> a map of plurals </li> + * <li> a list of strings </li> + * </ol> + * and dumps this map into the json file.</p> + */ +public class CompileStringsStep implements Step { + + @VisibleForTesting + static final Pattern STRING_FILE_PATTERN = Pattern.compile( + ".*res/values-([a-z]{2})(?:-r([A-Z]{2}))*/strings.xml"); + + private final FilterResourcesStep filterResourcesStep; + private final Path destinationDir; + private final ObjectMapper objectMapper; + + @VisibleForTesting + CompileStringsStep( + FilterResourcesStep filterResourcesStep, + Path destinationDir, + ObjectMapper mapper) { + this.filterResourcesStep = Preconditions.checkNotNull(filterResourcesStep); + this.destinationDir = Preconditions.checkNotNull(destinationDir); + this.objectMapper = Preconditions.checkNotNull(mapper); + } + + /** + * Note: The ordering of files in the input list determines which resource value ends up in the + * output json file, in the event of multiple xml files of a locale sharing the same string + * resource name - file that appears first in the list wins. + * + * @param filterResourcesStep {@link FilterResourcesStep} that filters non english string files. + * @param destinationDir Output directory for the generated json files. + */ + public CompileStringsStep(FilterResourcesStep filterResourcesStep, Path destinationDir) { + this(filterResourcesStep, destinationDir, new ObjectMapper(new JsonFactory())); + } + + @Override + public int execute(ExecutionContext context) { + ImmutableSet<String> filteredStringFiles = filterResourcesStep.getNonEnglishStringFiles(); + ImmutableMultimap<String, String> filesByLocale = groupFilesByLocale(filteredStringFiles); + ProjectFilesystem filesystem = context.getProjectFilesystem(); + + for (String locale : filesByLocale.keySet()) { + // TODO: Merge base locale resource with the country specific ones + // eg. es_ES = es_ES + es; + try { + ImmutableMap<String, Object> resources = compileStringFiles(filesByLocale.get(locale)); + File jsonFile = filesystem.getFileForRelativePath(destinationDir.resolve(locale + ".json")); + objectMapper.writeValue(jsonFile, resources); + } catch (IOException e) { + context.getBuckEventBus().post(ThrowableLogEvent.create(e, + "Error creating json string file for locale: %s", + locale)); + return 1; + } + } + + return 0; + } + + /** + * Groups a list of file paths matching STRING_FILE_PATTERN by the locale. + * eg. given the following list: + * + * ImmutableSet.of( + * '/one/res/values-es/strings.xml', + * '/two/res/values-es/strings.xml', + * '/three/res/values-pt-rBR/strings.xml', + * '/four/res/values/-pt-rPT/strings.xml'); + * + * returns: + * + * ImmutableMap.of( + * 'es', ImmutableSet.of('/one/res/values-es/strings.xml', '/two/res/values-es/strings.xml'), + * 'pt_BR', ImmutableSet.of('/three/res/values-pt-rBR/strings.xml'), + * 'pt_PT', ImmutableSet.of('/four/res/values/-pt-rPT/strings.xml')); + */ + @VisibleForTesting + ImmutableMultimap<String, String> groupFilesByLocale(ImmutableSet<String> files) { + ImmutableMultimap.Builder<String, String> localeToFiles = ImmutableMultimap.builder(); + + for (String filepath : files) { + Matcher matcher = STRING_FILE_PATTERN.matcher(filepath); + if (!matcher.matches()) { + continue; + } + String baseLocale = matcher.group(1); + String country = matcher.group(2); + String locale = country == null ? baseLocale : baseLocale + "_" + country; + + localeToFiles.put(locale, filepath); + } + + return localeToFiles.build(); + } + + private ImmutableMap<String, Object> compileStringFiles(Collection<String> filepaths) + throws IOException { + + Map<String, String> stringsMap = Maps.newHashMap(); + Map<String, Map<String, String>> pluralsMap = Maps.newHashMap(); + Map<String, List<String>> arraysMap = Maps.newHashMap(); + + for (String stringFilePath : filepaths) { + File stringFile = (Paths.get(stringFilePath)).toFile(); + Document dom = XmlDomParser.parse(stringFile); + + NodeList stringNodes = dom.getElementsByTagName("string"); + scrapeStringNodes(stringNodes, stringsMap); + + NodeList pluralNodes = dom.getElementsByTagName("plurals"); + scrapePluralsNodes(pluralNodes, pluralsMap); + + NodeList arrayNodes = dom.getElementsByTagName("string-array"); + scrapeStringArrayNodes(arrayNodes, arraysMap); + } + + ImmutableMap.Builder<String, Object> resourcesBuilder = ImmutableMap.builder(); + resourcesBuilder.putAll(stringsMap); + resourcesBuilder.putAll(pluralsMap); + resourcesBuilder.putAll(arraysMap); + return resourcesBuilder.build(); + } + + + /** + * Scrapes string resource names and values from the list of xml nodes passed and populates + * {@code stringsMap}, ignoring resource names that are already present in the map. + * + * @param stringNodes A list of {@code <string></string>} nodes. + * @param stringsMap Map from string resource name to its value. + */ + @VisibleForTesting + void scrapeStringNodes(NodeList stringNodes, Map<String, String> stringsMap) { + for (int i = 0; i < stringNodes.getLength(); ++i) { + Node node = stringNodes.item(i); + String resourceName = node.getAttributes().getNamedItem("name").getNodeValue(); + // Ignore a resource if it has already been found. + if (!stringsMap.containsKey(resourceName)) { + stringsMap.put(resourceName, node.getTextContent()); + } + } + } + + /** + * Similar to {@code scrapeStringNodes}, but for plurals nodes. + */ + @VisibleForTesting + void scrapePluralsNodes( + NodeList pluralNodes, + Map<String, Map<String, String>> pluralsMap) { + + for (int i = 0; i < pluralNodes.getLength(); ++i) { + Node node = pluralNodes.item(i); + String resourceName = node.getAttributes().getNamedItem("name").getNodeValue(); + // Ignore a resource if it has already been found. + if (pluralsMap.containsKey(resourceName)) { + continue; + } + ImmutableMap.Builder<String, String> quantityToStringBuilder = ImmutableMap.builder(); + + NodeList itemNodes = ((Element) node).getElementsByTagName("item"); + for (int j = 0; j < itemNodes.getLength(); ++j) { + Node itemNode = itemNodes.item(j); + String quantity = itemNode.getAttributes().getNamedItem("quantity").getNodeValue(); + quantityToStringBuilder.put(quantity, itemNode.getTextContent()); + } + pluralsMap.put(resourceName, quantityToStringBuilder.build()); + } + } + + /** + * Similar to {@code scrapeStringNodes}, but for string array nodes. + */ + @VisibleForTesting + void scrapeStringArrayNodes(NodeList arrayNodes, Map<String, List<String>> arraysMap) { + for (int i = 0; i < arrayNodes.getLength(); ++i) { + Node node = arrayNodes.item(i); + String resourceName = node.getAttributes().getNamedItem("name").getNodeValue(); + // Ignore a resource if it has already been found. + if (arraysMap.containsKey(resourceName)) { + continue; + } + ImmutableList.Builder<String> arrayItemsBuilder = ImmutableList.builder(); + + NodeList itemNodes = ((Element)node).getElementsByTagName("item"); + for (int j = 0; j < itemNodes.getLength(); ++j) { + arrayItemsBuilder.add(itemNodes.item(j).getTextContent()); + } + arraysMap.put(resourceName, arrayItemsBuilder.build()); + } + } + + @Override + public String getShortName() { + return "compile_strings"; + } + + @Override + public String getDescription(ExecutionContext context) { + return "Combine, parse string resource xml files into one json file per locale."; + } +}
diff --git a/src/com/facebook/buck/android/FilterResourcesStep.java b/src/com/facebook/buck/android/FilterResourcesStep.java index 3568a85..da7894f 100644 --- a/src/com/facebook/buck/android/FilterResourcesStep.java +++ b/src/com/facebook/buck/android/FilterResourcesStep.java
@@ -16,9 +16,11 @@ package com.facebook.buck.android; +import com.facebook.buck.event.ThrowableLogEvent; import com.facebook.buck.shell.BashStep; import com.facebook.buck.step.ExecutionContext; import com.facebook.buck.step.Step; +import com.facebook.buck.util.DefaultFilteredDirectoryCopier; import com.facebook.buck.util.DirectoryTraversal; import com.facebook.buck.util.Escaper; import com.facebook.buck.util.FilteredDirectoryCopier; @@ -26,12 +28,15 @@ import com.facebook.buck.util.HumanReadableException; import com.facebook.buck.util.MorePaths; import com.facebook.buck.util.ProjectFilesystem; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; import java.io.File; import java.io.IOException; @@ -56,86 +61,108 @@ "hdpi", 240.0, "xhdpi", 320.0); - private static final Pattern DRAWABLE_PATH_PATTERN = Pattern.compile( + @VisibleForTesting + static final Pattern DRAWABLE_PATH_PATTERN = Pattern.compile( ".*drawable.*/.*(png|jpg|jpeg|gif)", Pattern.CASE_INSENSITIVE); - private final File baseDestination; - private final String resourceFilter; + @VisibleForTesting + static final Pattern NON_ENGLISH_STRING_PATH = Pattern.compile( + "(\\b|.*/)res/values-.+/strings.xml", Pattern.CASE_INSENSITIVE); + + private final ImmutableBiMap<String, String> inResDirToOutResDirMap; + private final boolean filterDrawables; + private final boolean filterStrings; private final FilteredDirectoryCopier filteredDirectoryCopier; + @Nullable + private final String resourceFilter; + @Nullable private final DrawableFinder drawableFinder; - private final ImmutableBiMap<String, String> originalToFiltered; @Nullable private final ImageScaler imageScaler; + private final ImmutableSet.Builder<String> nonEnglishStringFilesBuilder; /** * Creates a command that filters a specified set of directories. - * @param resDirectories set of {@code res} directories to filter - * @param baseDestination destination directory, where copies of these directories will be placed - * (will be created if necessary) + * @param inResDirToOutResDirMap set of {@code res} directories to filter + * @param filterDrawables whether to filter drawables (images) + * @param filterStrings whether to filter non-english strings + * @param filteredDirectoryCopier refer {@link FilteredDirectoryCopier} + * * @param resourceFilter filtering to perform (currently based on density; allowed values are - * {@code "mdpi"}, {@code "hdpi"} and {@code "xhdpi"}) + * {@code "mdpi"}, {@code "hdpi"} and {@code "xhdpi"}). Only applicable if filterDrawables + * is true + * @param drawableFinder refer {@link DrawableFinder}. Only applicable if filterDrawables is true. * @param imageScaler if not null, use the {@link ImageScaler} to downscale higher-density * drawables for which we weren't able to find an image file of the proper density (as opposed - * to allowing Android to do it at runtime) + * to allowing Android to do it at runtime). Only applicable if filterDrawables. is true. */ - public FilterResourcesStep( - Set<String> resDirectories, - File baseDestination, - String resourceFilter, + @VisibleForTesting + FilterResourcesStep( + ImmutableBiMap<String, String> inResDirToOutResDirMap, + boolean filterDrawables, + boolean filterStrings, FilteredDirectoryCopier filteredDirectoryCopier, - DrawableFinder drawableFinder, + @Nullable String resourceFilter, + @Nullable DrawableFinder drawableFinder, @Nullable ImageScaler imageScaler) { - this.baseDestination = Preconditions.checkNotNull(baseDestination); - this.resourceFilter = Preconditions.checkNotNull(resourceFilter); + + Preconditions.checkArgument(filterDrawables || filterStrings); + Preconditions.checkArgument(!filterDrawables || + (resourceFilter != null && drawableFinder != null)); + this.inResDirToOutResDirMap = Preconditions.checkNotNull(inResDirToOutResDirMap); + this.filterDrawables = filterDrawables; + this.filterStrings = filterStrings; this.filteredDirectoryCopier = Preconditions.checkNotNull(filteredDirectoryCopier); - this.drawableFinder = Preconditions.checkNotNull(drawableFinder); - this.originalToFiltered = assignDestinations( - Preconditions.checkNotNull(resDirectories), - Preconditions.checkNotNull(baseDestination)); + + this.resourceFilter = resourceFilter; + this.drawableFinder = drawableFinder; this.imageScaler = imageScaler; - } - - private static ImmutableBiMap<String, String> assignDestinations(Set<String> sources, File base) { - ImmutableBiMap.Builder<String, String> builder = ImmutableBiMap.builder(); - int count = 0; - for (String source : sources) { - builder.put(source, new File(base, String.valueOf(count++)).getPath()); - } - return builder.build(); - } - - /** - * Returns drop-in set of resource directories for the rest of the build (e.g. {@code aapt}). - * @return set of directories which, after running {@link #execute(ExecutionContext)}, - * will contain filtered copies of the resource directories passed on construction. - */ - public ImmutableSet<String> getFilteredResourceDirectories() { - // getFilteredResourceDirectories() yields matching destination directories in the same order - // as the sources, which is relevant to aapt. Adding - // return new TreeSet<String>(destinations); - // here would yield a broken app - return originalToFiltered.values(); + this.nonEnglishStringFilesBuilder = ImmutableSet.builder(); } @Override public int execute(ExecutionContext context) { try { return doExecute(context); - } catch (IOException e) { - e.printStackTrace(context.getStdErr()); + } catch (Exception e) { + context.getBuckEventBus().post(ThrowableLogEvent.create(e, + "There was an error filtering resources.")); return 1; } } - private int doExecute(ExecutionContext context) throws IOException { - // Get list of candidate drawables. - Set<String> drawables = drawableFinder.findDrawables(originalToFiltered.keySet()); + /** + * @return If {@code filterStrings} is true, set containing absolute file paths to non-english + * string files, matching NON_ENGLISH_STRING_PATH regex; else empty set. + */ + public ImmutableSet<String> getNonEnglishStringFiles() { + return nonEnglishStringFilesBuilder.build(); + } - // Create a filter that removes drawables not of desired density. - Predicate<File> densityFilter = Filters.createImageDensityFilter(drawables, resourceFilter); + private int doExecute(ExecutionContext context) throws IOException { + List<Predicate<File>> filePredicates = Lists.newArrayList(); + if (filterDrawables) { + Set<String> drawables = drawableFinder.findDrawables(inResDirToOutResDirMap.keySet()); + filePredicates.add(Filters.createImageDensityFilter(drawables, resourceFilter)); + } + + if (filterStrings) { + filePredicates.add(new Predicate<File>() { + @Override + public boolean apply(File input) { + String inputPath = input.getAbsolutePath(); + if (NON_ENGLISH_STRING_PATH.matcher(inputPath).matches()) { + nonEnglishStringFilesBuilder.add(inputPath); + return false; + } + return true; + } + }); + } + + // Create filtered copies of all resource directories. These will be passed to aapt instead. - filteredDirectoryCopier.copyDirs(originalToFiltered, - densityFilter); + filteredDirectoryCopier.copyDirs(inResDirToOutResDirMap, Predicates.and(filePredicates)); // If an ImageScaler was specified, but only if it is available, try to apply it. if ((imageScaler != null) && imageScaler.isAvailable(context)) { @@ -152,10 +179,7 @@ @Override public String getDescription(ExecutionContext context) { - return String.format( - "Filtering %d resource directories into: %s", - originalToFiltered.size(), - baseDestination); + return "Filtering drawable and string resources."; } /** @@ -169,7 +193,7 @@ ProjectFilesystem filesystem = context.getProjectFilesystem(); // Go over all the images that remain after filtering. - for (String drawable : drawableFinder.findDrawables(getFilteredResourceDirectories())) { + for (String drawable : drawableFinder.findDrawables(inResDirToOutResDirMap.values())) { File drawableFile = filesystem.getFileForRelativePath(drawable); if (drawable.endsWith(".9.png")) { @@ -317,4 +341,43 @@ } } + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + + private ImmutableBiMap<String, String> inResDirToOutResDirMap; + private ResourceFilter resourceFilter; + private boolean filterStrings = false; + + private Builder() { + } + + public Builder setInResToOutResDirMap(ImmutableBiMap<String, String> inResDirToOutResDirMap) { + this.inResDirToOutResDirMap = inResDirToOutResDirMap; + return this; + } + + public Builder setResourceFilter(ResourceFilter resourceFilter) { + this.resourceFilter = resourceFilter; + return this; + } + + public Builder enableStringsFilter() { + this.filterStrings = true; + return this; + } + + public FilterResourcesStep build() { + return new FilterResourcesStep( + inResDirToOutResDirMap, + resourceFilter.isEnabled(), + filterStrings, + DefaultFilteredDirectoryCopier.getInstance(), + resourceFilter.getDensity(), + DefaultDrawableFinder.getInstance(), + resourceFilter.shouldDownscale() ? ImageMagickScaler.getInstance() : null); + } + } }
diff --git a/src/com/facebook/buck/util/XmlDomParser.java b/src/com/facebook/buck/util/XmlDomParser.java index 2d9a1c1..1c07aaa 100644 --- a/src/com/facebook/buck/util/XmlDomParser.java +++ b/src/com/facebook/buck/util/XmlDomParser.java
@@ -23,8 +23,10 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; +import java.io.InputStream; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -33,7 +35,15 @@ public class XmlDomParser { public static Document parse(File xml) throws IOException { - return parse(new InputSource(Files.newInputStreamSupplier(xml).getInput()), false); + return parse(Files.newInputStreamSupplier(xml).getInput()); + } + + public static Document parse(String xmlContents) throws IOException { + return parse(new ByteArrayInputStream(xmlContents.getBytes())); + } + + public static Document parse(InputStream stream) throws IOException { + return parse(new InputSource(stream), /* namespaceAware */ false); } public static Document parse(InputSource xml, boolean namespaceAware) throws IOException {
diff --git a/test/com/facebook/buck/android/BUCK b/test/com/facebook/buck/android/BUCK index 06f0945..8d86667 100644 --- a/test/com/facebook/buck/android/BUCK +++ b/test/com/facebook/buck/android/BUCK
@@ -5,6 +5,8 @@ '//lib:easymock', '//lib:guava', '//lib:junit', + '//lib:jackson-core', + '//lib:jackson-databind', '//src/com/facebook/buck/android:exceptions', '//src/com/facebook/buck/android:r', '//src/com/facebook/buck/android:rules',
diff --git a/test/com/facebook/buck/android/CompileStringsStepTest.java b/test/com/facebook/buck/android/CompileStringsStepTest.java new file mode 100644 index 0000000..83ab510 --- /dev/null +++ b/test/com/facebook/buck/android/CompileStringsStepTest.java
@@ -0,0 +1,273 @@ +/* + * Copyright 2013-present Facebook, Inc. + * + * 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.facebook.buck.android; + +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.getCurrentArguments; +import static org.junit.Assert.assertEquals; + +import com.facebook.buck.step.ExecutionContext; +import com.facebook.buck.util.ProjectFilesystem; +import com.facebook.buck.util.XmlDomParser; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.ImmutableSet; + +import org.easymock.EasyMockSupport; +import org.easymock.IAnswer; +import org.junit.Test; +import org.w3c.dom.NodeList; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; + +public class CompileStringsStepTest extends EasyMockSupport { + + private static final String XML_HEADER = "<?xml version='1.0' encoding='utf-8'?>"; + + private static final String TESTDATA_DIR = "testdata/com/facebook/buck/android/"; + private static final String FIRST_FILE = TESTDATA_DIR + "first/res/values-es/strings.xml"; + private static final String SECOND_FILE = TESTDATA_DIR + "second/res/values-es/strings.xml"; + private static final String THIRD_FILE = TESTDATA_DIR + "third/res/values-pt/strings.xml"; + + @Test + public void testStringFilePattern() { + testStringPathRegex("res/values-es/strings.xml", true, "es", null); + testStringPathRegex("/one/res/values-es/strings.xml", true, "es", null); + testStringPathRegex("/two/res/values-es-rUS/strings.xml", true, "es", "US"); + // Not matching strings. + testStringPathRegex("/one/res/values/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-e/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-esc/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-es-rU/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-es-rUSA/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-es-RUS/strings.xml", false, null, null); + testStringPathRegex("/one/res/values-rUS/strings.xml", false, null, null); + } + + private void testStringPathRegex(String input, boolean matches, String locale, String country) { + Matcher matcher = CompileStringsStep.STRING_FILE_PATTERN.matcher(input); + assertEquals(matches, matcher.matches()); + if (!matches) { + return; + } + assertEquals(locale, matcher.group(1)); + assertEquals(country, matcher.group(2)); + } + + @Test + public void testGroupFilesByLocale() { + ImmutableSet<String> files = ImmutableSet.of( + "/project/dir/res/values-da/strings.xml", + "/project/dir/res/values-da-rAB/strings.xml", + "/project/dir/dontmatch/res/values/strings.xml", + "/project/groupme/res/values-da/strings.xml", + "/project/groupmetoo/res/values-da-rAB/strings.xml", + "/project/foreveralone/res/values-es/strings.xml" + ); + + ImmutableMultimap<String, String> groupedByLocale = + createNonExecutingStep().groupFilesByLocale(files); + + ImmutableMultimap<String, String> expectedMap = + ImmutableMultimap.<String, String>builder() + .putAll("da", ImmutableSet.<String>of( + "/project/dir/res/values-da/strings.xml", + "/project/groupme/res/values-da/strings.xml")) + .putAll("da_AB", ImmutableSet.<String>of( + "/project/dir/res/values-da-rAB/strings.xml", + "/project/groupmetoo/res/values-da-rAB/strings.xml")) + .putAll("es", ImmutableSet.<String>of("/project/foreveralone/res/values-es/strings.xml")) + .build(); + + assertEquals(expectedMap, groupedByLocale); + } + + @Test + public void testScrapeStringNodes() throws IOException { + String xmlInput = + "<string name='name1'>Value1</string>" + + "<string name='name2'>Value with space</string>" + + "<string name='name3'>Value with \"quotes\"</string>" + + "<string name='name4'></string>" + + "<string name='name3'>IGNORE</string>" + // ignored because "name3" already found + "<string name='name5'>Value with %1$s</string>"; + NodeList stringNodes = XmlDomParser.parse(createResourcesXml(xmlInput)) + .getElementsByTagName("string"); + + Map<String, String> stringsMap = new HashMap<>(); + createNonExecutingStep().scrapeStringNodes(stringNodes, stringsMap); + + assertEquals( + ImmutableMap.of( + "name1", "Value1", + "name2", "Value with space", + "name3", "Value with \"quotes\"", + "name4", "", + "name5", "Value with %1$s"), + stringsMap + ); + } + + @Test + public void testScrapePluralsNodes() throws IOException { + String xmlInput = + "<plurals name='name1'>" + + "<item quantity='zero'>%d people saw this</item>" + + "<item quantity='one'>%d person saw this</item>" + + "<item quantity='many'>%d people saw this</item>" + + "</plurals>" + + "<plurals name='name2'>" + + "<item quantity='zero'>%d people ate this</item>" + + "<item quantity='many'>%d people ate this</item>" + + "</plurals>" + + "<plurals name='name3'></plurals>" + // Test empty array. + "<plurals name='name2'></plurals>"; // Ignored since "name2" already found. + NodeList pluralsNodes = XmlDomParser.parse(createResourcesXml(xmlInput)) + .getElementsByTagName("plurals"); + + Map<String, Map<String, String>> pluralsMap = new HashMap<>(); + createNonExecutingStep().scrapePluralsNodes(pluralsNodes, pluralsMap); + + assertEquals( + ImmutableMap.of( + "name1", ImmutableMap.of( + "zero", "%d people saw this", + "one", "%d person saw this", + "many", "%d people saw this"), + "name2", ImmutableMap.of( + "zero", "%d people ate this", + "many", "%d people ate this"), + "name3", ImmutableMap.of() + ), + pluralsMap + ); + } + + @Test + public void testScrapeStringArrayNodes() throws IOException { + String xmlInput = + "<string-array name='name1'>" + + "<item>Value11</item>" + + "<item>Value12</item>" + + "</string-array>" + + "<string-array name='name2'>" + + "<item>Value21</item>" + + "</string-array>" + + "<string-array name='name3'></string-array>" + + "<string-array name='name2'>" + + "<item>ignored</item>" + // Ignored because "name2" already found above. + "</string-array>"; + + NodeList arrayNodes = XmlDomParser.parse(createResourcesXml(xmlInput)) + .getElementsByTagName("string-array"); + + Map<String, List<String>> arraysMap = new HashMap<>(); + createNonExecutingStep().scrapeStringArrayNodes(arrayNodes, arraysMap); + + assertEquals( + ImmutableMap.of( + "name1", ImmutableList.of("Value11", "Value12"), + "name2", ImmutableList.of("Value21"), + "name3", ImmutableList.of() + ), + arraysMap + ); + } + + private CompileStringsStep createNonExecutingStep() { + return new CompileStringsStep( + createMock(FilterResourcesStep.class), + createMock(Path.class), + createMock(ObjectMapper.class)); + } + + private String createResourcesXml(String contents) { + return XML_HEADER + "<resources>" + contents + "</resources>"; + } + + @Test + public void testSuccessfulStepExecution() throws IOException { + Path destinationDir = Paths.get(""); + + ExecutionContext context = createMock(ExecutionContext.class); + ProjectFilesystem filesystem = createMock(ProjectFilesystem.class); + expect(filesystem.getFileForRelativePath(anyObject(Path.class))).andAnswer(new IAnswer<File>() { + @Override + public File answer() throws Throwable { + return ((Path)getCurrentArguments()[0]).toFile(); + } + }).times(2); + expect(context.getProjectFilesystem()).andReturn(filesystem); + + ObjectMapper mapper = createMock(ObjectMapper.class); + mapper.writeValue(anyObject(File.class), anyObject()); + + final ImmutableMap.Builder<String, ImmutableMap<String, Object>> capturedResourcesBuilder = + ImmutableMap.builder(); + expectLastCall().andAnswer(new IAnswer<Object>() { + @Override + public Object answer() throws Throwable { + File file = (File)getCurrentArguments()[0]; + capturedResourcesBuilder.put(file.getName(), getMapFromArgs()); + return null; + } + + @SuppressWarnings("unchecked") + private ImmutableMap<String, Object> getMapFromArgs() { + return (ImmutableMap<String, Object>) getCurrentArguments()[1]; + } + }).times(2); + + FilterResourcesStep filterResourcesStep = createMock(FilterResourcesStep.class); + expect(filterResourcesStep.getNonEnglishStringFiles()).andReturn(ImmutableSet.of( + FIRST_FILE, + SECOND_FILE, + THIRD_FILE)); + + replayAll(); + CompileStringsStep step = new CompileStringsStep(filterResourcesStep, destinationDir, mapper); + assertEquals(0, step.execute(context)); + assertEquals( + ImmutableMap.of( + "es.json", ImmutableMap.of( + "name1_1", "Value11", + "name1_2", "Value12", + "name1_3", "Value13", + "name2_1", "Value21", + "name2_2", "Value22"), + "pt.json", ImmutableMap.of( + "name3_1", "Value31", + "name3_2", "Value32", + "name3_3", "Value33") + ), + capturedResourcesBuilder.build()); + + verifyAll(); + } +}
diff --git a/test/com/facebook/buck/android/FilterResourcesStepTest.java b/test/com/facebook/buck/android/FilterResourcesStepTest.java index a9448a2..d8c7732 100644 --- a/test/com/facebook/buck/android/FilterResourcesStepTest.java +++ b/test/com/facebook/buck/android/FilterResourcesStepTest.java
@@ -17,6 +17,8 @@ package com.facebook.buck.android; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import com.facebook.buck.android.FilterResourcesStep.ImageScaler; import com.facebook.buck.step.ExecutionContext; @@ -27,6 +29,7 @@ import com.facebook.buck.util.ProjectFilesystem; import com.facebook.buck.util.Verbosity; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -47,15 +50,19 @@ private final static String second = "/second-path/res"; private final static String third = "/third-path/res"; - private final static ImmutableSet<String> resDirectories = ImmutableSet.of(first, second, third); + private final static ImmutableBiMap<String, String> inResDirToOutResDirMap = + ImmutableBiMap.of( + first, "/dest/1", + second, "/dest/2", + third, "/dest/3"); private static Set<String> qualifiers = ImmutableSet.of("mdpi", "hdpi", "xhdpi"); private final String targetDensity = "mdpi"; private final File baseDestination = new File("/dest"); - private final String scaleSource = getFile(first, "xhdpi", "other.png"); - private final String scaleDest = getFile(first, "mdpi", "other.png"); + private final String scaleSource = getDrawableFile(first, "xhdpi", "other.png"); + private final String scaleDest = getDrawableFile(first, "mdpi", "other.png"); - private String getFile(String dir, String qualifier, String filename) { + private String getDrawableFile(String dir, String qualifier, String filename) { return MorePaths.newPathInstance( new File(dir, String.format("drawable-%s/%s", qualifier, filename))).toString(); } @@ -95,8 +102,8 @@ // Create mock FilteredDirectoryCopier to find what we're calling on it. FilteredDirectoryCopier copier = EasyMock.createMock(FilteredDirectoryCopier.class); // We'll want to see what the filtering command passes to the copier. - Capture<Map<String, String>> dirMapCapture = new Capture<Map<String, String>>(); - Capture<Predicate<File>> predCapture = new Capture<Predicate<File>>(); + Capture<Map<String, String>> dirMapCapture = new Capture<>(); + Capture<Predicate<File>> predCapture = new Capture<>(); copier.copyDirs(EasyMock.capture(dirMapCapture), EasyMock.capture(predCapture)); EasyMock.replay(copier); @@ -113,15 +120,16 @@ EasyMock.replay(scaler); FilterResourcesStep command = new FilterResourcesStep( - resDirectories, - baseDestination, - targetDensity, + inResDirToOutResDirMap, + /* filterDrawables */ true, + /* filterStrings */ true, copier, + targetDensity, finder, scaler); EasyMock - .expect(finder.findDrawables(resDirectories)) + .expect(finder.findDrawables(inResDirToOutResDirMap.keySet())) .andAnswer(new IAnswer<Set<String>>() { @SuppressWarnings("unchecked") @Override @@ -129,7 +137,7 @@ ImmutableSet.Builder<String> builder = ImmutableSet.builder(); for (String dir : (Iterable<String>) EasyMock.getCurrentArguments()[0]) { for (String qualifier : qualifiers) { - builder.add(getFile(dir, qualifier, "some.png")); + builder.add(getDrawableFile(dir, qualifier, "some.png")); } } @@ -142,14 +150,14 @@ // Called by the downscaling step. EasyMock - .expect(finder.findDrawables(command.getFilteredResourceDirectories())) + .expect(finder.findDrawables(inResDirToOutResDirMap.values())) .andAnswer(new IAnswer<Set<String>>() { @SuppressWarnings("unchecked") @Override public Set<String> answer() throws Throwable { ImmutableSet.Builder<String> builder = ImmutableSet.builder(); for (String dir : (Iterable<String>) EasyMock.getCurrentArguments()[0]) { - builder.add(getFile(dir, targetDensity, "some.png")); + builder.add(getDrawableFile(dir, targetDensity, "some.png")); } builder.add(scaleSource); @@ -162,8 +170,8 @@ // We'll use this to verify the source->destination mappings created by the command. ImmutableMap.Builder<String, String> dirMapBuilder = ImmutableMap.builder(); - Iterator<String> destIterator = command.getFilteredResourceDirectories().iterator(); - for(String dir : resDirectories) { + Iterator<String> destIterator = inResDirToOutResDirMap.values().iterator(); + for (String dir : inResDirToOutResDirMap.keySet()) { String nextDestination = destIterator.next(); dirMapBuilder.put(dir, nextDestination); @@ -178,7 +186,7 @@ assertEquals(dirMapBuilder.build(), dirMapCapture.getValue()); // Ensure the right filter is created. - Set<String> drawables = finder.findDrawables(resDirectories); + Set<String> drawables = finder.findDrawables(inResDirToOutResDirMap.keySet()); Predicate<File> expectedPred = Filters.createImageDensityFilter(drawables, targetDensity); Predicate<File> capturedPred = predCapture.getValue(); for (String drawablePath : drawables) { @@ -190,4 +198,17 @@ // and we're calling finder.findDrawables twice. EasyMock.verify(copier, context, finder, filesystem, scaler); } + + @Test + public void testNonEnglishStringsPathRegex() { + assertTrue(matchesRegex("res/values-es/strings.xml")); + assertFalse(matchesRegex("res/values-/strings.xml")); + assertTrue(matchesRegex("/res/values-es/strings.xml")); + assertFalse(matchesRegex("rootres/values-es/strings.xml")); + assertTrue(matchesRegex("root/res/values-es-rUS/strings.xml")); + } + + private static boolean matchesRegex(String input) { + return FilterResourcesStep.NON_ENGLISH_STRING_PATH.matcher(input).matches(); + } }
diff --git a/testdata/com/facebook/buck/android/first/res/values-es/strings.xml b/testdata/com/facebook/buck/android/first/res/values-es/strings.xml new file mode 100644 index 0000000..96b492b --- /dev/null +++ b/testdata/com/facebook/buck/android/first/res/values-es/strings.xml
@@ -0,0 +1,6 @@ +<?xml version='1.0' encoding='utf-8'?> +<resources> + <string name='name1_1'>Value11</string> + <string name='name1_2'>Value12</string> + <string name='name1_3'>Value13</string> +</resources>
diff --git a/testdata/com/facebook/buck/android/second/res/values-es/strings.xml b/testdata/com/facebook/buck/android/second/res/values-es/strings.xml new file mode 100644 index 0000000..f2e139c --- /dev/null +++ b/testdata/com/facebook/buck/android/second/res/values-es/strings.xml
@@ -0,0 +1,6 @@ +<?xml version='1.0' encoding='utf-8'?> +<resources> + <string name='name2_1'>Value21</string> + <string name='name2_2'>Value22</string> + <string name='name1_3'>Value23</string> +</resources>
diff --git a/testdata/com/facebook/buck/android/third/res/values-pt/strings.xml b/testdata/com/facebook/buck/android/third/res/values-pt/strings.xml new file mode 100644 index 0000000..c6e3135 --- /dev/null +++ b/testdata/com/facebook/buck/android/third/res/values-pt/strings.xml
@@ -0,0 +1,6 @@ +<?xml version='1.0' encoding='utf-8'?> +<resources> + <string name='name3_1'>Value31</string> + <string name='name3_2'>Value32</string> + <string name='name3_3'>Value33</string> +</resources>