java,runfiles: merge classes

This commit prepares simplifying the Java runfiles
library, which itself prepares mergint the
manifest-based and directory-based logic so that
the Runfiles class could handle both at the same
time.

This commit only moves classes around:
- moves the ManifestBased and DirectoryBased
  classes into Runfiles, as nested classes
- adds a createManifestBasedForTesting and
  createDirectoryBasedForTesting method to create
  the classes for testing
- moves the ManifestBasedTest and
  DirectoryBasedTest into RunfilesTest

Otherwise this commit has no functional change.

Change-Id: I56b582c1a95793b0a871748697673b53a780f0fb
PiperOrigin-RevId: 194227675

8 files changed, 171 insertions, 274 deletions

diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD b/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD
index e6d3d0b7c9..00f44a07d6 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD
@@ -12,8 +12,6 @@ filegroup(
 filegroup(
     name = "java-srcs",
     srcs = [
-        "DirectoryBased.java",
-        "ManifestBased.java",
         "Runfiles.java",
         "Util.java",
     ],
@@ -41,18 +39,6 @@ java_test(
 )
 
 java_test(
-    name = "DirectoryBasedTest",
-    srcs = ["DirectoryBasedTest.java"],
-    deps = [":test_deps"],
-)
-
-java_test(
-    name = "ManifestBasedTest",
-    srcs = ["ManifestBasedTest.java"],
-    deps = [":test_deps"],
-)
-
-java_test(
     name = "UtilTest",
     srcs = ["UtilTest.java"],
     deps = [":test_deps"],
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD.tools b/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD.tools
index 6a0bf082f0..5337b2b0d5 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD.tools
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/BUILD.tools
@@ -3,8 +3,6 @@ package(default_visibility = ["//visibility:private"])
 filegroup(
     name = "java-srcs",
     srcs = [
-        "DirectoryBased.java",
-        "ManifestBased.java",
         "Runfiles.java",
         "Util.java",
     ],
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java
deleted file mode 100644
index 0215a7c26b..0000000000
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java
+++ /dev/null
@@ -1,45 +0,0 @@
-// Copyright 2018 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.runfiles;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-
-/** {@link Runfiles} implementation that appends runfiles paths to the runfiles root. */
-final class DirectoryBased extends Runfiles {
-  private final String runfilesRoot;
-
-  DirectoryBased(String runfilesDir) throws IOException {
-    Util.checkArgument(!Util.isNullOrEmpty(runfilesDir));
-    Util.checkArgument(new File(runfilesDir).isDirectory());
-    this.runfilesRoot = runfilesDir;
-  }
-
-  @Override
-  String rlocationChecked(String path) {
-    return runfilesRoot + "/" + path;
-  }
-
-  @Override
-  public Map<String, String> getEnvVars() {
-    HashMap<String, String> result = new HashMap<>(2);
-    result.put("RUNFILES_DIR", runfilesRoot);
-    // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR.
-    result.put("JAVA_RUNFILES", runfilesRoot);
-    return result;
-  }
-}
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java
deleted file mode 100644
index abd9073179..0000000000
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright 2018 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.runfiles;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.fail;
-
-import java.io.File;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Unit tests for {@link DirectoryBased} implementation of {@link Runfiles}. */
-@RunWith(JUnit4.class)
-public final class DirectoryBasedTest {
-
-  @Test
-  public void testRlocation() throws Exception {
-    // The DirectoryBased implementation simply joins the runfiles directory and the runfile's path
-    // on a "/". DirectoryBased does not perform any normalization, nor does it check that the path
-    // exists.
-    File dir = new File(System.getenv("TEST_TMPDIR"), "mock/runfiles");
-    assertThat(dir.mkdirs()).isTrue();
-    DirectoryBased r = new DirectoryBased(dir.toString());
-    // Escaping for "\": once for string and once for regex.
-    assertThat(r.rlocation("arg")).matches(".*[/\\\\]mock[/\\\\]runfiles[/\\\\]arg");
-  }
-
-  @Test
-  public void testCtorArgumentValidation() throws Exception {
-    try {
-      new DirectoryBased(null);
-      fail();
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
-
-    try {
-      new DirectoryBased("");
-      fail();
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
-
-    try {
-      new DirectoryBased("non-existent directory is bad");
-      fail();
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
-
-    new DirectoryBased(System.getenv("TEST_TMPDIR"));
-  }
-}
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java
deleted file mode 100644
index 30b3bd2496..0000000000
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright 2018 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.runfiles;
-
-import java.io.BufferedReader;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.nio.charset.StandardCharsets;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
-/** {@link Runfiles} implementation that parses a runfiles-manifest file to look up runfiles. */
-final class ManifestBased extends Runfiles {
-  private final Map<String, String> runfiles;
-  private final String manifestPath;
-
-  ManifestBased(String manifestPath) throws IOException {
-    Util.checkArgument(manifestPath != null);
-    Util.checkArgument(!manifestPath.isEmpty());
-    this.manifestPath = manifestPath;
-    this.runfiles = loadRunfiles(manifestPath);
-  }
-
-  private static Map<String, String> loadRunfiles(String path) throws IOException {
-    HashMap<String, String> result = new HashMap<>();
-    try (BufferedReader r =
-        new BufferedReader(
-            new InputStreamReader(new FileInputStream(path), StandardCharsets.UTF_8))) {
-      String line = null;
-      while ((line = r.readLine()) != null) {
-        int index = line.indexOf(' ');
-        String runfile = (index == -1) ? line : line.substring(0, index);
-        String realPath = (index == -1) ? line : line.substring(index + 1);
-        result.put(runfile, realPath);
-      }
-    }
-    return Collections.unmodifiableMap(result);
-  }
-
-  private static String findRunfilesDir(String manifest) {
-    if (manifest.endsWith("/MANIFEST")
-        || manifest.endsWith("\\MANIFEST")
-        || manifest.endsWith(".runfiles_manifest")) {
-      String path = manifest.substring(0, manifest.length() - 9);
-      if (new File(path).isDirectory()) {
-        return path;
-      }
-    }
-    return "";
-  }
-
-  @Override
-  public String rlocationChecked(String path) {
-    return runfiles.get(path);
-  }
-
-  @Override
-  public Map<String, String> getEnvVars() {
-    HashMap<String, String> result = new HashMap<>(4);
-    result.put("RUNFILES_MANIFEST_ONLY", "1");
-    result.put("RUNFILES_MANIFEST_FILE", manifestPath);
-    String runfilesDir = findRunfilesDir(manifestPath);
-    result.put("RUNFILES_DIR", runfilesDir);
-    // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR.
-    result.put("JAVA_RUNFILES", runfilesDir);
-    return result;
-  }
-}
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBasedTest.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBasedTest.java
deleted file mode 100644
index 30c8d497e8..0000000000
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBasedTest.java
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright 2018 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.runfiles;
-
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.fail;
-
-import com.google.common.collect.ImmutableList;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Unit tests for {@link ManifestBased} implementation of {@link Runfiles}. */
-@RunWith(JUnit4.class)
-public final class ManifestBasedTest {
-
-  @Test
-  public void testCtorArgumentValidation() throws Exception {
-    try {
-      new ManifestBased(null);
-      fail();
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
-
-    try {
-      new ManifestBased("");
-      fail();
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
-
-    try (MockFile mf = new MockFile(ImmutableList.of("a b"))) {
-      new ManifestBased(mf.path.toString());
-    }
-  }
-
-  @Test
-  public void testRlocation() throws Exception {
-    try (MockFile mf =
-        new MockFile(
-            ImmutableList.of(
-                "Foo/runfile1 C:/Actual Path\\runfile1",
-                "Foo/Bar/runfile2 D:\\the path\\run file 2.txt"))) {
-      ManifestBased r = new ManifestBased(mf.path.toString());
-      assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1");
-      assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt");
-      assertThat(r.rlocation("unknown")).isNull();
-    }
-  }
-}
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java
index f72be41872..8b6277311c 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java
@@ -14,8 +14,14 @@
 
 package com.google.devtools.build.runfiles;
 
+import java.io.BufferedReader;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 
 /**
@@ -42,7 +48,7 @@ import java.util.Map;
 public abstract class Runfiles {
 
   // Package-private constructor, so only package-private classes may extend it.
-  Runfiles() {}
+  private Runfiles() {}
 
   /**
    * Returns a new {@link Runfiles} instance.
@@ -158,4 +164,95 @@ public abstract class Runfiles {
   }
 
   abstract String rlocationChecked(String path);
+
+  /** {@link Runfiles} implementation that parses a runfiles-manifest file to look up runfiles. */
+  private static final class ManifestBased extends Runfiles {
+    private final Map<String, String> runfiles;
+    private final String manifestPath;
+
+    ManifestBased(String manifestPath) throws IOException {
+      Util.checkArgument(manifestPath != null);
+      Util.checkArgument(!manifestPath.isEmpty());
+      this.manifestPath = manifestPath;
+      this.runfiles = loadRunfiles(manifestPath);
+    }
+
+    private static Map<String, String> loadRunfiles(String path) throws IOException {
+      HashMap<String, String> result = new HashMap<>();
+      try (BufferedReader r =
+          new BufferedReader(
+              new InputStreamReader(new FileInputStream(path), StandardCharsets.UTF_8))) {
+        String line = null;
+        while ((line = r.readLine()) != null) {
+          int index = line.indexOf(' ');
+          String runfile = (index == -1) ? line : line.substring(0, index);
+          String realPath = (index == -1) ? line : line.substring(index + 1);
+          result.put(runfile, realPath);
+        }
+      }
+      return Collections.unmodifiableMap(result);
+    }
+
+    private static String findRunfilesDir(String manifest) {
+      if (manifest.endsWith("/MANIFEST")
+          || manifest.endsWith("\\MANIFEST")
+          || manifest.endsWith(".runfiles_manifest")) {
+        String path = manifest.substring(0, manifest.length() - 9);
+        if (new File(path).isDirectory()) {
+          return path;
+        }
+      }
+      return "";
+    }
+
+    @Override
+    public String rlocationChecked(String path) {
+      return runfiles.get(path);
+    }
+
+    @Override
+    public Map<String, String> getEnvVars() {
+      HashMap<String, String> result = new HashMap<>(4);
+      result.put("RUNFILES_MANIFEST_ONLY", "1");
+      result.put("RUNFILES_MANIFEST_FILE", manifestPath);
+      String runfilesDir = findRunfilesDir(manifestPath);
+      result.put("RUNFILES_DIR", runfilesDir);
+      // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR.
+      result.put("JAVA_RUNFILES", runfilesDir);
+      return result;
+    }
+  }
+
+  /** {@link Runfiles} implementation that appends runfiles paths to the runfiles root. */
+  private static final class DirectoryBased extends Runfiles {
+    private final String runfilesRoot;
+
+    DirectoryBased(String runfilesDir) throws IOException {
+      Util.checkArgument(!Util.isNullOrEmpty(runfilesDir));
+      Util.checkArgument(new File(runfilesDir).isDirectory());
+      this.runfilesRoot = runfilesDir;
+    }
+
+    @Override
+    String rlocationChecked(String path) {
+      return runfilesRoot + "/" + path;
+    }
+
+    @Override
+    public Map<String, String> getEnvVars() {
+      HashMap<String, String> result = new HashMap<>(2);
+      result.put("RUNFILES_DIR", runfilesRoot);
+      // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR.
+      result.put("JAVA_RUNFILES", runfilesRoot);
+      return result;
+    }
+  }
+
+  static Runfiles createManifestBasedForTesting(String manifestPath) throws IOException {
+    return new ManifestBased(manifestPath);
+  }
+
+  static Runfiles createDirectoryBasedForTesting(String runfilesDir) throws IOException {
+    return new DirectoryBased(runfilesDir);
+  }
 }
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java
index 5c2eead374..157c62e482 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java
@@ -236,4 +236,77 @@ public final class RunfilesTest {
     assertThat(envvars.get("RUNFILES_DIR")).isEqualTo(dir.toString());
     assertThat(envvars.get("JAVA_RUNFILES")).isEqualTo(dir.toString());
   }
+
+  @Test
+  public void testDirectoryBasedRlocation() throws Exception {
+    // The DirectoryBased implementation simply joins the runfiles directory and the runfile's path
+    // on a "/". DirectoryBased does not perform any normalization, nor does it check that the path
+    // exists.
+    File dir = new File(System.getenv("TEST_TMPDIR"), "mock/runfiles");
+    assertThat(dir.mkdirs()).isTrue();
+    Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString());
+    // Escaping for "\": once for string and once for regex.
+    assertThat(r.rlocation("arg")).matches(".*[/\\\\]mock[/\\\\]runfiles[/\\\\]arg");
+  }
+
+  @Test
+  public void testManifestBasedRlocation() throws Exception {
+    try (MockFile mf =
+        new MockFile(
+            ImmutableList.of(
+                "Foo/runfile1 C:/Actual Path\\runfile1",
+                "Foo/Bar/runfile2 D:\\the path\\run file 2.txt"))) {
+      Runfiles r = Runfiles.createManifestBasedForTesting(mf.path.toString());
+      assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1");
+      assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt");
+      assertThat(r.rlocation("unknown")).isNull();
+    }
+  }
+
+  @Test
+  public void testDirectoryBasedCtorArgumentValidation() throws Exception {
+    try {
+      Runfiles.createDirectoryBasedForTesting(null);
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+
+    try {
+      Runfiles.createDirectoryBasedForTesting("");
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+
+    try {
+      Runfiles.createDirectoryBasedForTesting("non-existent directory is bad");
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+
+    Runfiles.createDirectoryBasedForTesting(System.getenv("TEST_TMPDIR"));
+  }
+
+  @Test
+  public void testManifestBasedCtorArgumentValidation() throws Exception {
+    try {
+      Runfiles.createManifestBasedForTesting(null);
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+
+    try {
+      Runfiles.createManifestBasedForTesting("");
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+
+    try (MockFile mf = new MockFile(ImmutableList.of("a b"))) {
+      Runfiles.createManifestBasedForTesting(mf.path.toString());
+    }
+  }
 }