Add methods for filtering LocalResourceContainer

Dynamically Configured Resource Filtering change 1/6

To enable dynamically configured resource filtering, we'll need to be able to
pass filtered resources into resource processing code used by android_library
targets. The resource parsing action, used only in android_library targets,
takes the LocalResourceContainer as an input, unlike android_binary processing
code which never used it directly.

Abstract the code for actually building the collection of resource roots out of the withResources method, and create an additional method for calling it from resource filtering.

Also, split some common test code out of ResourceFilterTestBase to make a ResourceTestBase class for testing changes to this (and eventually, other) resource-oriented classes, and create LocalResourceContainerTest for this class in particular.

RELNOTES: none
PiperOrigin-RevId: 160640192

9 files changed, 580 insertions, 176 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index e2e70fb513..dafdb335a6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -319,7 +319,7 @@ public class AndroidCommon {
   }
 
   static PathFragment getSourceDirectoryRelativePathFromResource(Artifact resource) {
-    PathFragment resourceDir = LocalResourceContainer.Builder.findResourceDir(resource);
+    PathFragment resourceDir = LocalResourceContainer.findResourceDir(resource);
     if (resourceDir == null) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index 81e9000583..d9ce0a1516 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidManifestMerger;
 import com.google.devtools.build.lib.rules.android.ResourceContainer.Builder.JavaPackageSource;
 import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType;
@@ -387,7 +388,7 @@ public final class ApplicationManifest {
       List<String> uncompressedExtensions,
       boolean crunchPng,
       Artifact proguardCfg)
-      throws InterruptedException {
+      throws InterruptedException, RuleErrorException {
     LocalResourceContainer data =
         new LocalResourceContainer.Builder(ruleContext)
             .withAssets(
@@ -441,7 +442,7 @@ public final class ApplicationManifest {
       @Nullable Artifact dataBindingInfoZip,
       @Nullable Artifact featureOf,
       @Nullable Artifact featureAfter)
-      throws InterruptedException {
+      throws InterruptedException, RuleErrorException {
     LocalResourceContainer data = new LocalResourceContainer.Builder(ruleContext)
         .withAssets(
             AndroidCommon.getAssetDir(ruleContext),
@@ -492,7 +493,7 @@ public final class ApplicationManifest {
       Artifact manifestOut,
       Artifact mergedResources,
       Artifact dataBindingInfoZip)
-      throws InterruptedException {
+      throws InterruptedException, RuleErrorException {
     LocalResourceContainer data =
         new LocalResourceContainer.Builder(ruleContext)
             .withAssets(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
index 56f8b37b38..13b610726a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
@@ -25,11 +25,13 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
-import java.util.Collection;
 import java.util.LinkedHashSet;
+import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * The collected resources and assets artifacts and roots.
@@ -48,6 +50,28 @@ public final class LocalResourceContainer {
       "exports_manifest"
   };
 
+  /** Set of allowable android directories prefixes. */
+  public static final ImmutableSet<String> RESOURCE_DIRECTORY_TYPES =
+      ImmutableSet.of(
+          "animator",
+          "anim",
+          "color",
+          "drawable",
+          "interpolator",
+          "layout",
+          "menu",
+          "mipmap",
+          "raw",
+          "transition",
+          "values",
+          "xml");
+
+  public static final String INCORRECT_RESOURCE_LAYOUT_MESSAGE =
+      String.format(
+          "'%%s' is not in the expected resource directory structure of "
+              + "<resource directory>/{%s}/<file>",
+          Joiner.on(',').join(RESOURCE_DIRECTORY_TYPES));
+
   /**
    * Determines if the attributes contain resource and asset attributes.
    */
@@ -116,33 +140,12 @@ public final class LocalResourceContainer {
   }
 
   public static class Builder {
-    /**
-     * Set of allowable android directories prefixes.
-     */
-    // TODO(bazel-team): Pull from AOSP constant?
-    public static final ImmutableSet<String> RESOURCE_DIRECTORY_TYPES = ImmutableSet.of(
-        "animator",
-        "anim",
-        "color",
-        "drawable",
-        "interpolator",
-        "layout",
-        "menu",
-        "mipmap",
-        "raw",
-        "transition",
-        "values",
-        "xml");
-
-    public static final String INCORRECT_RESOURCE_LAYOUT_MESSAGE = String.format(
-        "'%%s' is not in the expected resource directory structure of "
-            + "<resource directory>/{%s}/<file>", Joiner.on(',').join(RESOURCE_DIRECTORY_TYPES));
-
-    private RuleContext ruleContext;
-    private Collection<Artifact> assets = new LinkedHashSet<>();
-    private Collection<Artifact> resources = new LinkedHashSet<>();
-    private Collection<PathFragment> resourceRoots = new LinkedHashSet<>();
-    private Collection<PathFragment> assetRoots = new LinkedHashSet<>();
+
+    private final RuleContext ruleContext;
+    private final Set<Artifact> assets = new LinkedHashSet<>();
+    private final Set<Artifact> resources = new LinkedHashSet<>();
+    private final Set<PathFragment> resourceRoots = new LinkedHashSet<>();
+    private final Set<PathFragment> assetRoots = new LinkedHashSet<>();
 
     public Builder(RuleContext ruleContext) {
       this.ruleContext = ruleContext;
@@ -183,32 +186,14 @@ public final class LocalResourceContainer {
      * @param targets {@link FileProvider}s for a given set of resource.
      * @return The Builder.
      */
-    public LocalResourceContainer.Builder withResources(Iterable<FileProvider> targets) {
+    public LocalResourceContainer.Builder withResources(Iterable<FileProvider> targets)
+        throws RuleErrorException {
       PathFragment lastResourceDir = null;
       Artifact lastFile = null;
       for (FileProvider target : targets) {
         for (Artifact file : target.getFilesToBuild()) {
-          PathFragment packageFragment = file.getArtifactOwner().getLabel()
-              .getPackageIdentifier().getSourceRoot();
-          PathFragment packageRelativePath =
-              file.getRootRelativePath().relativeTo(packageFragment);
-          PathFragment resourceDir = findResourceDir(file);
-          if (resourceDir == null) {
-            ruleContext.attributeError(ResourceType.RESOURCES.getAttribute(), String.format(
-                INCORRECT_RESOURCE_LAYOUT_MESSAGE, file.getRootRelativePath()));
-            return this;
-          }
-          if (lastResourceDir == null || resourceDir.equals(lastResourceDir)) {
-            resourceRoots.add(
-                trimTail(file.getExecPath(), makeRelativeTo(resourceDir, packageRelativePath)));
-          } else {
-            ruleContext.attributeError(ResourceType.RESOURCES.getAttribute(), String.format(
-                "'%s' (generated by '%s') is not in the same directory '%s' (derived from %s)."
-                + " All resources must share a common directory.",
-                file.getRootRelativePath(), file.getArtifactOwner().getLabel(), lastResourceDir,
-                lastFile.getRootRelativePath()));
-            return this;
-          }
+          PathFragment resourceDir =
+              addResourceDir(file, lastFile, lastResourceDir, resourceRoots, ruleContext);
           resources.add(file);
           lastFile = file;
           lastResourceDir = resourceDir;
@@ -217,66 +202,138 @@ public final class LocalResourceContainer {
       return this;
     }
 
-    /**
-     * Finds and validates the resource directory PathFragment from the artifact Path.
-     *
-     * <p>If the artifact is not a Fileset, the resource directory is presumed to be the second
-     * directory from the end. Filesets are expect to have the last directory as the resource
-     * directory.
-     *
-     */
-    public static PathFragment findResourceDir(Artifact artifact) {
-      PathFragment fragment = artifact.getPath().asFragment();
-      int segmentCount = fragment.segmentCount();
-      if (segmentCount < 3) {
-        return null;
-      }
-      // TODO(bazel-team): Expand Fileset to verify, or remove Fileset as an option for resources.
-      if (artifact.isFileset() || artifact.isTreeArtifact()) {
-        return fragment.subFragment(segmentCount - 1, segmentCount);
-      }
+    public LocalResourceContainer build() {
+      return new LocalResourceContainer(
+          ImmutableList.copyOf(resources),
+          ImmutableList.copyOf(resourceRoots),
+          ImmutableList.copyOf(assets),
+          ImmutableList.copyOf(assetRoots));
+    }
+  }
 
-      // Check the resource folder type layout.
-      // get the prefix of the parent folder of the fragment.
-      String parentDirectory = fragment.getSegment(segmentCount - 2);
-      int dashIndex = parentDirectory.indexOf('-');
-      String androidFolder =
-          dashIndex == -1 ? parentDirectory : parentDirectory.substring(0, dashIndex);
-      if (!RESOURCE_DIRECTORY_TYPES.contains(androidFolder)) {
-        return null;
-      }
+  /**
+   * Gets the roots of some resources.
+   *
+   * @return a list of roots, or an empty list of the passed resources cannot all be contained in a
+   *     single {@link LocalResourceContainer}. If that's the case, it will be reported to the
+   *     {@link RuleErrorConsumer}.
+   */
+  @VisibleForTesting
+  static ImmutableList<PathFragment> getResourceRoots(
+      RuleErrorConsumer ruleErrorConsumer, Iterable<Artifact> files) throws RuleErrorException {
+    Artifact lastFile = null;
+    PathFragment lastResourceDir = null;
+    Set<PathFragment> resourceRoots = new LinkedHashSet<>();
+    for (Artifact file : files) {
+      PathFragment resourceDir =
+          addResourceDir(file, lastFile, lastResourceDir, resourceRoots, ruleErrorConsumer);
+      lastFile = file;
+      lastResourceDir = resourceDir;
+    }
 
-      return fragment.subFragment(segmentCount - 3, segmentCount - 2);
+    return ImmutableList.copyOf(resourceRoots);
+  }
+
+  /**
+   * Inner method for adding resource roots to a collection. May fail and report to the {@link
+   * RuleErrorConsumer} if the input is invalid.
+   *
+   * @param file the file to add the resource directory for
+   * @param lastFile the last file this method was called on. May be null if this is the first call
+   *     for this set of resources.
+   * @param lastResourceDir the resource directory of the last file, as returned by the most recent
+   *     call to this method, or null if this is the first call.
+   * @param resourceRoots the collection to add resources to
+   * @param ruleErrorConsumer for reporting errors
+   * @return the resource root of {@code file}.
+   * @throws RuleErrorException if the current resource has no resource directory or if it is
+   *     incompatible with {@code lastResourceDir}. An error will also be reported to the {@link
+   *     RuleErrorConsumer} in this case.
+   */
+  private static PathFragment addResourceDir(
+      Artifact file,
+      @Nullable Artifact lastFile,
+      @Nullable PathFragment lastResourceDir,
+      Set<PathFragment> resourceRoots,
+      RuleErrorConsumer ruleErrorConsumer) throws RuleErrorException {
+    PathFragment resourceDir = findResourceDir(file);
+    if (resourceDir == null) {
+      ruleErrorConsumer.attributeError(
+          ResourceType.RESOURCES.getAttribute(),
+          String.format(INCORRECT_RESOURCE_LAYOUT_MESSAGE, file.getRootRelativePath()));
+      throw new RuleErrorException();
     }
 
-    /**
-     * Returns the root-part of a given path by trimming off the end specified by
-     * a given tail. Assumes that the tail is known to match, and simply relies on
-     * the segment lengths.
-     */
-    private static PathFragment trimTail(PathFragment path, PathFragment tail) {
-      return path.subFragment(0, path.segmentCount() - tail.segmentCount());
+    if (lastResourceDir != null && !resourceDir.equals(lastResourceDir)) {
+      ruleErrorConsumer.attributeError(
+          ResourceType.RESOURCES.getAttribute(),
+          String.format(
+              "'%s' (generated by '%s') is not in the same directory '%s' (derived from %s)."
+                  + " All resources must share a common directory.",
+              file.getRootRelativePath(),
+              file.getArtifactOwner().getLabel(),
+              lastResourceDir,
+              lastFile.getRootRelativePath()));
+      throw new RuleErrorException();
     }
 
-    private static PathFragment makeRelativeTo(PathFragment ancestor, PathFragment path) {
-      String cutAtSegment = ancestor.getSegment(ancestor.segmentCount() - 1);
-      int totalPathSegments = path.segmentCount() - 1;
-      for (int i = totalPathSegments; i >= 0; i--) {
-        if (path.getSegment(i).equals(cutAtSegment)) {
-          return path.subFragment(i, totalPathSegments);
-        }
-      }
-      throw new IllegalArgumentException("PathFragment " + path
-          + " is not beneath " + ancestor);
+    PathFragment packageFragment =
+        file.getArtifactOwner().getLabel().getPackageIdentifier().getSourceRoot();
+    PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment);
+    resourceRoots.add(
+        trimTail(file.getExecPath(), makeRelativeTo(resourceDir, packageRelativePath)));
+
+    return resourceDir;
+  }
+
+  /**
+   * Finds and validates the resource directory PathFragment from the artifact Path.
+   *
+   * <p>If the artifact is not a Fileset, the resource directory is presumed to be the second
+   * directory from the end. Filesets are expect to have the last directory as the resource
+   * directory.
+   */
+  public static PathFragment findResourceDir(Artifact artifact) {
+    PathFragment fragment = artifact.getPath().asFragment();
+    int segmentCount = fragment.segmentCount();
+    if (segmentCount < 3) {
+      return null;
+    }
+    // TODO(bazel-team): Expand Fileset to verify, or remove Fileset as an option for resources.
+    if (artifact.isFileset() || artifact.isTreeArtifact()) {
+      return fragment.subFragment(segmentCount - 1, segmentCount);
     }
 
-    public LocalResourceContainer build() {
-      return new LocalResourceContainer(
-          ImmutableList.copyOf(resources),
-          ImmutableList.copyOf(resourceRoots),
-          ImmutableList.copyOf(assets),
-          ImmutableList.copyOf(assetRoots));
+    // Check the resource folder type layout.
+    // get the prefix of the parent folder of the fragment.
+    String parentDirectory = fragment.getSegment(segmentCount - 2);
+    int dashIndex = parentDirectory.indexOf('-');
+    String androidFolder =
+        dashIndex == -1 ? parentDirectory : parentDirectory.substring(0, dashIndex);
+    if (!RESOURCE_DIRECTORY_TYPES.contains(androidFolder)) {
+      return null;
+    }
+
+    return fragment.subFragment(segmentCount - 3, segmentCount - 2);
+  }
+
+  /**
+   * Returns the root-part of a given path by trimming off the end specified by a given tail.
+   * Assumes that the tail is known to match, and simply relies on the segment lengths.
+   */
+  private static PathFragment trimTail(PathFragment path, PathFragment tail) {
+    return path.subFragment(0, path.segmentCount() - tail.segmentCount());
+  }
+
+  private static PathFragment makeRelativeTo(PathFragment ancestor, PathFragment path) {
+    String cutAtSegment = ancestor.getSegment(ancestor.segmentCount() - 1);
+    int totalPathSegments = path.segmentCount() - 1;
+    for (int i = totalPathSegments; i >= 0; i--) {
+      if (path.getSegment(i).equals(cutAtSegment)) {
+        return path.subFragment(i, totalPathSegments);
+      }
     }
+    throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor);
   }
 
   private final ImmutableList<Artifact> resources;
@@ -311,4 +368,27 @@ public final class LocalResourceContainer {
   public ImmutableList<PathFragment> getResourceRoots() {
     return resourceRoots;
   }
+
+  /**
+   * Filters this object.
+   *
+   * @return a new {@link LocalResourceContainer} with resources filtered by the passed {@link
+   *     ResourceFilter}, or this object if no resources should be filtered.
+   */
+  public LocalResourceContainer filter(
+      RuleErrorConsumer ruleErrorConsumer, ResourceFilter resourceFilter)
+      throws RuleErrorException {
+    ImmutableList<Artifact> filteredResources = resourceFilter.filter(ruleErrorConsumer, resources);
+
+    if (filteredResources.size() == resources.size()) {
+      // Nothing was filtered out
+      return this;
+    }
+
+    return new LocalResourceContainer(
+        filteredResources,
+        getResourceRoots(ruleErrorConsumer, filteredResources),
+        assets,
+        assetRoots);
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index 888047d871..35f940f6b5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -2410,7 +2410,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
     checkError("java/android/resources", "r",
         "'java/android/resources/res/somefile.xml' is not in the expected resource directory "
         + "structure of <resource directory>/{"
-        + Joiner.on(',').join(LocalResourceContainer.Builder.RESOURCE_DIRECTORY_TYPES) + "}",
+        + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}",
         "android_binary(name = 'r',",
         "                manifest = 'AndroidManifest.xml',",
         "                resource_files = ['res/somefile.xml', 'r/t/f/m/raw/fold']",
@@ -2424,7 +2424,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
         "r",
         "'java/android/resources/res/other/somefile.xml' is not in the expected resource directory "
         + "structure of <resource directory>/{"
-        + Joiner.on(',').join(LocalResourceContainer.Builder.RESOURCE_DIRECTORY_TYPES) + "}",
+        + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}",
         "android_binary(name = 'r',",
         "               manifest = 'AndroidManifest.xml',",
         "               resource_files = ['res/other/somefile.xml', 'r/t/f/m/raw/fold']",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java
index ab4a1c1eac..b8298e2ea9 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java
@@ -822,7 +822,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase {
     checkError("java/android", "r",
         "'java/android/res/somefile.xml' is not in the expected resource directory structure of"
             + " <resource directory>/{"
-            + Joiner.on(',').join(LocalResourceContainer.Builder.RESOURCE_DIRECTORY_TYPES) + "}",
+            + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}",
         "android_library(name = 'r',",
         "                manifest = 'AndroidManifest.xml',",
         "                resource_files = ['res/somefile.xml', 'r/t/f/m/raw/fold']",
@@ -834,7 +834,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase {
     checkError("java/android", "r",
         "'java/android/res/other/somefile.xml' is not in the expected resource directory structure"
             + " of <resource directory>/{"
-            + Joiner.on(',').join(LocalResourceContainer.Builder.RESOURCE_DIRECTORY_TYPES) + "}",
+            + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}",
         "android_library(name = 'r',",
         "                manifest = 'AndroidManifest.xml',",
         "                resource_files = ['res/other/somefile.xml', 'r/t/f/m/raw/fold']",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
index 96e6b301fe..c011cde25e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
@@ -77,6 +77,48 @@ java_test(
 )
 
 java_test(
+    name = "ResourceFilterTest",
+    srcs = ["ResourceFilterTest.java"],
+    deps = [
+        ":ResourceTestBase",
+        "//src/main/java/com/google/devtools/build/lib:android-rules",
+        "//src/main/java/com/google/devtools/build/lib/actions",
+        "//third_party:guava",
+        "//third_party:junit4",
+        "//third_party:truth",
+    ],
+)
+
+java_test(
+    name = "LocalResourceContainerTest",
+    srcs = ["LocalResourceContainerTest.java"],
+    deps = [
+        ":ResourceTestBase",
+        "//src/main/java/com/google/devtools/build/lib:android-rules",
+        "//src/main/java/com/google/devtools/build/lib:packages-internal",
+        "//src/main/java/com/google/devtools/build/lib:vfs",
+        "//src/main/java/com/google/devtools/build/lib/actions",
+        "//third_party:guava",
+        "//third_party:junit4",
+        "//third_party:truth",
+    ],
+)
+
+java_library(
+    name = "ResourceTestBase",
+    srcs = ["ResourceTestBase.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib:inmemoryfs",
+        "//src/main/java/com/google/devtools/build/lib:packages-internal",
+        "//src/main/java/com/google/devtools/build/lib:vfs",
+        "//src/main/java/com/google/devtools/build/lib/actions",
+        "//third_party:guava",
+        "//third_party:junit4",
+        "//third_party:truth",
+    ],
+)
+
+java_test(
     name = "AndroidDeviceScriptFixtureTest",
     srcs = ["AndroidDeviceScriptFixtureTest.java"],
     deps = [
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java
new file mode 100644
index 0000000000..6f397bf5fb
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java
@@ -0,0 +1,172 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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.google.devtools.build.lib.rules.android;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests {@link LocalResourceContainer} */
+@RunWith(JUnit4.class)
+public class LocalResourceContainerTest extends ResourceTestBase {
+  private static final PathFragment DEFAULT_RESOURCE_ROOT = PathFragment.create(RESOURCE_ROOT);
+  private static final ImmutableList<PathFragment> RESOURCES_ROOTS =
+      ImmutableList.of(DEFAULT_RESOURCE_ROOT);
+
+  @Before
+  @Test
+  public void testGetResourceRootsNoResources() throws Exception {
+    assertThat(getResourceRoots()).isEmpty();
+  }
+
+  @Test
+  public void testGetResourceRootsInvalidResourceDirectory() throws Exception {
+    try {
+      getResourceRoots("is-this-drawable-or-values/foo.xml");
+      assertWithMessage("Expected exception not thrown!").fail();
+    } catch (RuleErrorException e) {
+      // expected
+    }
+
+    errorConsumer.assertAttributeError(
+        "resources", "is not in the expected resource directory structure");
+  }
+
+  @Test
+  public void testGetResourceRootsMultipleRoots() throws Exception {
+    try {
+      getResourceRoots("subdir/values/foo.xml", "otherdir/values/bar.xml");
+      assertWithMessage("Expected exception not thrown!").fail();
+    } catch (RuleErrorException e) {
+      // expected
+    }
+
+    errorConsumer.assertAttributeError(
+        "resources", "All resources must share a common directory");
+  }
+
+  @Test
+  public void testGetResourceRoots() throws Exception {
+    assertThat(getResourceRoots("values-hdpi/foo.xml", "values-mdpi/bar.xml"))
+        .isEqualTo(RESOURCES_ROOTS);
+  }
+
+  @Test
+  public void testGetResourceRootsCommonSubdirectory() throws Exception {
+    assertThat(getResourceRoots("subdir/values-hdpi/foo.xml", "subdir/values-mdpi/bar.xml"))
+        .containsExactly(DEFAULT_RESOURCE_ROOT.getRelative("subdir"));
+  }
+
+  private ImmutableList<PathFragment> getResourceRoots(String... pathResourceStrings)
+      throws Exception {
+    return getResourceRoots(getResources(pathResourceStrings));
+  }
+
+  private ImmutableList<PathFragment> getResourceRoots(ImmutableList<Artifact> artifacts)
+      throws Exception {
+    return LocalResourceContainer.getResourceRoots(errorConsumer, artifacts);
+  }
+
+  @Test
+  public void testFilterEmpty() throws Exception {
+    assertFilter(ImmutableList.<Artifact>of(), ImmutableList.<Artifact>of());
+  }
+
+  @Test
+  public void testFilterNoop() throws Exception {
+    ImmutableList<Artifact> resources = getResources("values-en/foo.xml", "values-es/bar.xml");
+    assertFilter(resources, resources);
+  }
+
+  @Test
+  public void testFilterToEmpty() throws Exception {
+    assertFilter(
+        getResources("values-en/foo.xml", "values-es/bar.xml"), ImmutableList.<Artifact>of());
+  }
+
+  @Test
+  public void testPartiallyFilter() throws Exception {
+    Artifact keptResource = getResource("values-en/foo.xml");
+    assertFilter(
+        ImmutableList.of(keptResource, getResource("values-es/bar.xml")),
+        ImmutableList.of(keptResource));
+  }
+
+  private void assertFilter(
+      ImmutableList<Artifact> unfilteredResources, ImmutableList<Artifact> filteredResources)
+      throws Exception {
+    ResourceFilter filter =
+        new FakeResourceFilter(ImmutableMap.of(unfilteredResources, filteredResources));
+    ImmutableList<PathFragment> unfilteredResourcesRoots = getResourceRoots(unfilteredResources);
+    LocalResourceContainer unfiltered =
+        new LocalResourceContainer(
+            unfilteredResources,
+            unfilteredResourcesRoots,
+            unfilteredResources,
+            unfilteredResourcesRoots);
+
+    LocalResourceContainer filtered = unfiltered.filter(errorConsumer, filter);
+
+    if (unfilteredResources.equals(filteredResources)) {
+      // The filtering was a no-op; the original object, not a copy, should be returned
+      assertThat(filtered).isSameAs(unfiltered);
+    } else {
+      // The resources and their roots should be filtered
+      assertThat(filtered.getResources()).containsExactlyElementsIn(filteredResources).inOrder();
+      assertThat(filtered.getResourceRoots())
+          .containsExactlyElementsIn(getResourceRoots(filteredResources))
+          .inOrder();
+
+      // The assets and their roots should not be filtered; the original objects, not a copy, should
+      // be returned.
+      assertThat(filtered.getAssets()).isSameAs(unfiltered.getAssets());
+      assertThat(filtered.getAssetRoots()).isSameAs(unfiltered.getAssetRoots());
+    }
+  }
+
+  private static class FakeResourceFilter extends ResourceFilter {
+    private final Map<ImmutableList<Artifact>, ImmutableList<Artifact>> filterInputToOutputMap;
+
+    FakeResourceFilter(
+        Map<ImmutableList<Artifact>, ImmutableList<Artifact>> filterInputToOutputMap) {
+      super(
+          ImmutableList.<String>of(),
+          ImmutableList.<String>of(),
+          FilterBehavior.FILTER_IN_ANALYSIS);
+      this.filterInputToOutputMap = filterInputToOutputMap;
+    }
+
+    @Override
+    public ImmutableList<Artifact> filter(
+        RuleErrorConsumer errorConsumer, ImmutableList<Artifact> artifacts) {
+      if (filterInputToOutputMap.containsKey(artifacts)) {
+        return filterInputToOutputMap.get(artifacts);
+      }
+
+      assertWithMessage("Called with unexpected input: " + artifacts).fail();
+      return artifacts;
+    }
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
index 8dbb18df72..b7aaf7d1d0 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
@@ -14,22 +14,13 @@
 package com.google.devtools.build.lib.rules.android;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 
-import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.Root;
-import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.rules.android.ResourceFilter.FilterBehavior;
-import com.google.devtools.build.lib.vfs.FileSystem;
-import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -38,53 +29,7 @@ import org.junit.runners.JUnit4;
 // TODO(asteinb): Test behavior not already covered in this test, and, when practical, move unit
 // tests currently located in {@link AndroidBinaryTest} to this class instead.
 @RunWith(JUnit4.class)
-public class ResourceFilterTest {
-  private FakeRuleErrorConsumer errorConsumer;
-
-  @Before
-  public void setup() {
-    errorConsumer = new FakeRuleErrorConsumer();
-  }
-
-  private static final class FakeRuleErrorConsumer implements RuleErrorConsumer {
-    // Use an ArrayListMultimap since it allows duplicates - we'll want to know if a warning is
-    // reported twice.
-    private final Multimap<String, String> attributeWarnings = ArrayListMultimap.create();
-
-    @Override
-    public void ruleWarning(String message) {}
-
-    @Override
-    public void ruleError(String message) {
-      assertWithMessage(message).fail();
-    }
-
-    @Override
-    public void attributeWarning(String attrName, String message) {
-      attributeWarnings.put(attrName, message);
-    }
-
-    @Override
-    public void attributeError(String attrName, String message) {
-      assertWithMessage(message + " (attribute: " + attrName + ")").fail();
-    }
-
-    public Collection<String> getAndClearAttributeWarnings(String attrName) {
-      if (!attributeWarnings.containsKey(attrName)) {
-        return ImmutableList.of();
-      }
-
-      return attributeWarnings.removeAll(attrName);
-    }
-
-    public void assertNoAttributeWarnings(String attrName) {
-      assertThat(attributeWarnings).doesNotContainKey(attrName);
-    }
-  };
-
-  private static final FileSystem FILE_SYSTEM = new InMemoryFileSystem();
-  private static final Root ROOT = Root.asSourceRoot(FILE_SYSTEM.getRootDirectory());
-
+public class ResourceFilterTest extends ResourceTestBase {
   @Test
   public void testFilterInExecution() {
     testNoopFilter(
@@ -98,10 +43,10 @@ public class ResourceFilterTest {
   @Test
   public void testFilterEmpty() {
     testNoopFilter(
-        ImmutableList.of(),
-        ImmutableList.of(),
+        ImmutableList.<String>of(),
+        ImmutableList.<String>of(),
         FilterBehavior.FILTER_IN_ANALYSIS,
-        ImmutableList.of());
+        ImmutableList.<String>of());
   }
 
   @Test
@@ -128,7 +73,7 @@ public class ResourceFilterTest {
   @Test
   public void testFilterByDensityPersistsOrdering() {
     testFilter(
-        ImmutableList.of(),
+        ImmutableList.<String>of(),
         ImmutableList.of("hdpi", "ldpi"),
         FilterBehavior.FILTER_IN_ANALYSIS,
         // If we add resources to the output list in density order, these resources will be
@@ -306,7 +251,11 @@ public class ResourceFilterTest {
       FilterBehavior filterBehavior,
       List<String> resources) {
     testFilter(
-        resourceConfigurationFilters, densities, filterBehavior, resources, ImmutableList.of());
+        resourceConfigurationFilters,
+        densities,
+        filterBehavior,
+        resources,
+        ImmutableList.<String>of());
   }
 
   private void testFilter(
@@ -317,12 +266,12 @@ public class ResourceFilterTest {
       List<String> resourcesToDiscard) {
     List<Artifact> unexpectedResources = new ArrayList<>();
     for (String resource : resourcesToDiscard) {
-      unexpectedResources.add(getArtifact(resource));
+      unexpectedResources.add(getResource(resource));
     }
 
     List<Artifact> expectedResources = new ArrayList<>();
     for (String resource : resourcesToKeep) {
-      expectedResources.add(getArtifact(resource));
+      expectedResources.add(getResource(resource));
     }
 
     ImmutableList<Artifact> allArtifacts =
@@ -333,8 +282,4 @@ public class ResourceFilterTest {
 
     assertThat(filtered).containsExactlyElementsIn(expectedResources).inOrder();
   }
-
-  private static Artifact getArtifact(String pathString) {
-    return new Artifact(FILE_SYSTEM.getPath("/java/android/res/" + pathString), ROOT);
-  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
new file mode 100644
index 0000000000..cada5518d9
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
@@ -0,0 +1,164 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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.google.devtools.build.lib.rules.android;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Multimap;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactOwner;
+import com.google.devtools.build.lib.actions.Root;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import java.util.Collection;
+import org.junit.After;
+import org.junit.Before;
+
+/** Base class for tests that work with resource artifacts. */
+public abstract class ResourceTestBase {
+  public static final String RESOURCE_ROOT = "java/android/res";
+
+  private static final ArtifactOwner OWNER = () -> {
+    try {
+      return Label.create("java", "all");
+    } catch (LabelSyntaxException e) {
+      assertWithMessage(e.getMessage()).fail();
+      return null;
+    }
+  };
+
+  /** A faked {@link RuleErrorConsumer} that validates that only expected errors were reported. */
+  public static final class FakeRuleErrorConsumer implements RuleErrorConsumer {
+    private String ruleErrorMessage = null;
+    private String attributeErrorAttribute = null;
+    private String attributeErrorMessage = null;
+
+    // Use an ArrayListMultimap since it allows duplicates - we'll want to know if a warning is
+    // reported twice.
+    private final Multimap<String, String> attributeWarnings = ArrayListMultimap.create();
+
+    @Override
+    public void ruleWarning(String message) {}
+
+    @Override
+    public void ruleError(String message) {
+      ruleErrorMessage = message;
+    }
+
+    @Override
+    public void attributeWarning(String attrName, String message) {
+      attributeWarnings.put(attrName, message);
+    }
+
+    @Override
+    public void attributeError(String attrName, String message) {
+      attributeErrorAttribute = attrName;
+      attributeErrorMessage = message;
+    }
+
+    public Collection<String> getAndClearAttributeWarnings(String attrName) {
+      if (!attributeWarnings.containsKey(attrName)) {
+        return ImmutableList.of();
+      }
+
+      return attributeWarnings.removeAll(attrName);
+    }
+
+    public void assertNoAttributeWarnings(String attrName) {
+      assertThat(attributeWarnings).doesNotContainKey(attrName);
+    }
+
+    /**
+     * Called at the end of a test to assert that that test produced a rule error
+     *
+     * @param expectedMessage a substring of the expected message
+     */
+    public void assertRuleError(String expectedMessage) {
+      // Clear the message before asserting so that if we fail here the error is not masked by the
+      // @After call to assertNoUnexpectedErrors.
+
+      String message = ruleErrorMessage;
+      ruleErrorMessage = null;
+
+      assertThat(message).contains(expectedMessage);
+    }
+
+    /**
+     * Called at the end of a test to assert that that test produced an attribute error
+     *
+     * @param expectedAttribute the attribute that caused the error
+     * @param expectedMessage a substring of the expected message
+     */
+    public void assertAttributeError(String expectedAttribute, String expectedMessage) {
+      // Clear the message before asserting so that if we fail here the error is not masked by the
+      // @After call to assertNoUnexpectedErrors.
+      String attr = attributeErrorAttribute;
+      String message = attributeErrorMessage;
+      attributeErrorAttribute = null;
+      attributeErrorMessage = null;
+
+      assertThat(message).contains(expectedMessage);
+      assertThat(attr).isEqualTo(expectedAttribute);
+    }
+
+    /**
+     * Asserts this {@link RuleErrorConsumer} encountered no unexpected errors. To consume an
+     * expected error, call {@link #assertRuleError(String)} or {@link #assertAttributeError(String,
+     * String)} in your test after the error is produced.
+     */
+    private void assertNoUnexpectedErrors() {
+      assertThat(ruleErrorMessage).isNull();
+      assertThat(attributeErrorMessage).isNull();
+      assertThat(attributeErrorAttribute).isNull();
+    }
+  };
+
+  public FakeRuleErrorConsumer errorConsumer;
+  public FileSystem fileSystem;
+  public Root root;
+
+  @Before
+  public void setup() {
+    errorConsumer = new FakeRuleErrorConsumer();
+    fileSystem = new InMemoryFileSystem();
+    root = Root.asDerivedRoot(fileSystem.getRootDirectory());
+  }
+
+  @After
+  public void assertNoErrors() {
+    errorConsumer.assertNoUnexpectedErrors();
+  }
+
+  public ImmutableList<Artifact> getResources(String... pathStrings) {
+    ImmutableList.Builder<Artifact> builder = ImmutableList.builder();
+    for (String pathString : pathStrings) {
+      builder.add(getResource(pathString));
+    }
+
+    return builder.build();
+  }
+
+  public Artifact getResource(String pathString) {
+    Path path = fileSystem.getPath("/" + RESOURCE_ROOT + "/" + pathString);
+    return new Artifact(
+        path, root, root.getExecPath().getRelative(path.relativeTo(root.getPath())), OWNER);
+  }
+}