SOLR-16903: Migrate all tests using File to NIO Path (#3263)

* Remove unnecessary toAbsolutePath() calls
* Remove some callers of FileSystems.getDefault().getSeparator()
* Remove Path.toString in where it could easily be avoided.
* Use Path.toFile().setReadable in TestCoreDiscovery
* Throw AssumptionViolatedException for file permissions on windows


Co-authored-by: David Smiley <[email protected]>

Author: mlbiscoc

Diff for: gradle/testing/randomization/policies/solr-tests.policy

@@ -198,6 +198,9 @@ grant {
   permission "java.net.URLPermission" "http://[::1]:*/solr/-", "HEAD,GET,PUT,POST:*";
   permission "java.net.URLPermission" "https://[::1]:*/solr/-", "HEAD,GET,PUT,POST:*";
   permission "java.net.URLPermission" "socket://[::1]:*", "CONNECT:*";
+
+  // Needed by TestCoreDiscovery
+  permission java.lang.RuntimePermission "accessUserInformation";
 };
 
 // additional permissions based on system properties set by /bin/solr

Diff for: solr/benchmark/src/java/org/apache/solr/bench/MiniClusterState.java

@@ -20,13 +20,13 @@
 import static org.apache.solr.bench.BaseBenchState.log;
 
 import com.codahale.metrics.Meter;
-import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.lang.management.ManagementFactory;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -130,8 +130,7 @@ public void tearDown(BenchmarkParams benchmarkParams) throws Exception {
               benchmarkParams.getBenchmark() + ".txt");
       Files.createDirectories(metricsResults.getParent());
 
-      cluster.dumpMetrics(
-          metricsResults.getParent().toFile(), metricsResults.getFileName().toString());
+      cluster.dumpMetrics(metricsResults.getParent(), metricsResults.getFileName().toString());
     }
 
     /**
@@ -170,8 +169,14 @@ private void logClusterDirectorySize() throws IOException {
                   long clusterSize =
                       Files.walk(node)
                           .filter(Files::isRegularFile)
-                          .map(Path::toFile)
-                          .mapToLong(File::length)
+                          .mapToLong(
+                              file -> {
+                                try {
+                                  return Files.size(file);
+                                } catch (IOException e) {
+                                  throw new RuntimeException(e);
+                                }
+                              })
                           .sum();
                   log("mini cluster node size (bytes) " + node + " " + clusterSize);
                 } catch (IOException e) {
@@ -553,7 +558,9 @@ public static ModifiableSolrParams params(ModifiableSolrParams params, String...
    */
   public static Path getFile(String name) {
     final URL url =
-        MiniClusterState.class.getClassLoader().getResource(name.replace(File.separatorChar, '/'));
+        MiniClusterState.class
+            .getClassLoader()
+            .getResource(name.replace(FileSystems.getDefault().getSeparator(), "/"));
     if (url != null) {
       try {
         return Path.of(url.toURI());

Diff for: solr/core/src/java/org/apache/solr/blockcache/BlockDirectory.java

@@ -16,10 +16,11 @@
  */
 package org.apache.solr.blockcache;
 
-import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Set;
 import org.apache.lucene.index.IndexFileNames;
 import org.apache.lucene.store.Directory;
@@ -289,12 +290,12 @@ String getFileCacheName(String name) throws IOException {
 
   private long getFileModified(String name) throws IOException {
     if (in instanceof FSDirectory) {
-      File directory = ((FSDirectory) in).getDirectory().toFile();
-      File file = new File(directory, name);
-      if (!file.exists()) {
+      Path directory = ((FSDirectory) in).getDirectory();
+      Path file = directory.resolve(name);
+      if (!Files.exists(file)) {
         throw new FileNotFoundException("File [" + name + "] not found");
       }
-      return file.lastModified();
+      return Files.getLastModifiedTime(file).toMillis();
     } else {
       throw new UnsupportedOperationException();
     }

Diff for: solr/core/src/java/org/apache/solr/cli/CreateTool.java

@@ -20,13 +20,13 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.Options;
-import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.file.PathUtils;
 import org.apache.solr.cli.CommonCLIOptions.DefaultValues;
 import org.apache.solr.client.solrj.SolrClient;
@@ -185,7 +185,7 @@ protected void createCore(CommandLine cli, SolrClient solrClient) throws Excepti
             "Failed to create new core instance directory: " + coreInstanceDir.toAbsolutePath());
       }
 
-      FileUtils.copyDirectoryToDirectory(confDir.toFile(), coreInstanceDir.toFile());
+      PathUtils.copyDirectory(confDir, coreInstanceDir, StandardCopyOption.COPY_ATTRIBUTES);
 
       echoIfVerbose(
           "\nCopying configuration to new core instance directory:\n"

Diff for: solr/core/src/test/org/apache/solr/AnalysisAfterCoreReloadTest.java

@@ -18,12 +18,12 @@
 
 import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 
-import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import org.apache.commons.io.FileUtils;
+import java.nio.file.StandardCopyOption;
+import org.apache.commons.io.file.PathUtils;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
@@ -41,9 +41,9 @@ public class AnalysisAfterCoreReloadTest extends SolrTestCaseJ4 {
 
   @BeforeClass
   public static void beforeClass() throws Exception {
-    String tmpSolrHome = createTempDir().toFile().getAbsolutePath();
-    FileUtils.copyDirectory(new File(TEST_HOME()), new File(tmpSolrHome).getAbsoluteFile());
-    initCore("solrconfig.xml", "schema.xml", new File(tmpSolrHome).getAbsolutePath());
+    Path tmpSolrHome = createTempDir();
+    PathUtils.copyDirectory(TEST_HOME(), tmpSolrHome, StandardCopyOption.COPY_ATTRIBUTES);
+    initCore("solrconfig.xml", "schema.xml", tmpSolrHome);
   }
 
   @AfterClass

Diff for: solr/core/src/test/org/apache/solr/SolrInfoBeanTest.java

@@ -16,12 +16,14 @@
  */
 package org.apache.solr;
 
-import java.io.File;
 import java.net.URI;
 import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.stream.Stream;
 import org.apache.lucene.tests.util.TestUtil;
 import org.apache.solr.core.SolrInfoBean;
 import org.apache.solr.handler.admin.LukeRequestHandler;
@@ -88,30 +90,37 @@ public void testCallMBeanInfo() throws Exception {
   }
 
   private static List<Class<?>> getClassesForPackage(String pckgname) throws Exception {
-    ArrayList<File> directories = new ArrayList<>();
+    ArrayList<Path> directories = new ArrayList<>();
     ClassLoader cld = h.getCore().getResourceLoader().getClassLoader();
     String path = pckgname.replace('.', '/');
     Enumeration<URL> resources = cld.getResources(path);
     while (resources.hasMoreElements()) {
       final URI uri = resources.nextElement().toURI();
       if (!"file".equalsIgnoreCase(uri.getScheme())) continue;
-      final File f = new File(uri);
+      final Path f = Path.of(uri);
       directories.add(f);
     }
 
     ArrayList<Class<?>> classes = new ArrayList<>();
-    for (File directory : directories) {
-      if (directory.exists()) {
-        String[] files = directory.list();
-        for (String file : files) {
-          if (file.endsWith(".class")) {
-            String clazzName = file.substring(0, file.length() - 6);
-            // exclude Test classes that happen to be in these packages.
-            // class.ForName'ing some of them can cause trouble.
-            if (!clazzName.endsWith("Test") && !clazzName.startsWith("Test")) {
-              classes.add(Class.forName(pckgname + '.' + clazzName));
-            }
-          }
+    for (Path directory : directories) {
+      if (Files.exists(directory)) {
+        try (Stream<Path> files = Files.list(directory)) {
+          files.forEach(
+              (file) -> {
+                String fileName = file.getFileName().toString();
+                if (fileName.endsWith(".class")) {
+                  String clazzName = fileName.substring(0, fileName.length() - 6);
+                  // exclude Test classes that happen to be in these packages.
+                  // class.ForName'ing some of them can cause trouble.
+                  if (!clazzName.endsWith("Test") && !clazzName.startsWith("Test")) {
+                    try {
+                      classes.add(Class.forName(pckgname + '.' + clazzName));
+                    } catch (ClassNotFoundException e) {
+                      throw new RuntimeException(e);
+                    }
+                  }
+                }
+              });
         }
       }
     }

Diff for: solr/core/src/test/org/apache/solr/SolrTestCaseJ4Test.java

@@ -16,10 +16,9 @@
  */
 package org.apache.solr;
 
-import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.file.PathUtils;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -31,24 +30,24 @@ public class SolrTestCaseJ4Test extends SolrTestCaseJ4 {
   public static void beforeClass() throws Exception {
     // Create a temporary directory that holds a core NOT named "collection1". Use the smallest
     // configuration sets we can, so we don't copy that much junk around.
-    String tmpSolrHome = createTempDir().toFile().getAbsolutePath();
+    Path tmpSolrHome = createTempDir();
 
-    Path subHome = Path.of(tmpSolrHome, "core0", "conf");
+    Path subHome = tmpSolrHome.resolve("core0/conf");
     Files.createDirectories(subHome);
-    String top = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
-    Files.copy(Path.of(top, "schema-tiny.xml"), subHome.resolve("schema-tiny.xml"));
-    Files.copy(Path.of(top, "solrconfig-minimal.xml"), subHome.resolve("solrconfig-minimal.xml"));
+    Path top = SolrTestCaseJ4.TEST_HOME().resolve("collection1/conf");
+    Files.copy(top.resolve("schema-tiny.xml"), subHome.resolve("schema-tiny.xml"));
+    Files.copy(top.resolve("solrconfig-minimal.xml"), subHome.resolve("solrconfig-minimal.xml"));
     Files.copy(
-        Path.of(top, "solrconfig.snippet.randomindexconfig.xml"),
+        top.resolve("solrconfig.snippet.randomindexconfig.xml"),
         subHome.resolve("solrconfig.snippet.randomindexconfig.xml"));
 
-    FileUtils.copyDirectory(new File(tmpSolrHome, "core0"), new File(tmpSolrHome, "core1"));
+    PathUtils.copyDirectory(tmpSolrHome.resolve("core0"), tmpSolrHome.resolve("core1"));
     // Core discovery will default to the name of the dir the core.properties file is in. So if
     // everything else is OK as defaults, just the _presence_ of this file is sufficient.
-    FileUtils.touch(new File(tmpSolrHome, "core0/core.properties"));
-    FileUtils.touch(new File(tmpSolrHome, "core1/core.properties"));
+    PathUtils.touch(tmpSolrHome.resolve("core0/core.properties"));
+    PathUtils.touch(tmpSolrHome.resolve("core1/core.properties"));
 
-    Files.copy(getFile("solr/solr.xml"), Path.of(tmpSolrHome, "solr.xml"));
+    Files.copy(getFile("solr/solr.xml"), tmpSolrHome.resolve("solr.xml"));
 
     initCore("solrconfig-minimal.xml", "schema-tiny.xml", tmpSolrHome, "core1");
   }

Diff for: solr/core/src/test/org/apache/solr/TestCpuTimeSearch.java

@@ -48,7 +48,7 @@ public static void setupSolr() throws Exception {
     System.setProperty(AllowListUrlChecker.DISABLE_URL_ALLOW_LIST, "true");
 
     Path configSet = createTempDir("configSet");
-    copyMinConf(configSet.toFile());
+    copyMinConf(configSet);
     solrRule.startSolr(LuceneTestCase.createTempDir());
 
     solrRule.newCollection("core1").withConfigSet(configSet.toString()).create();

Diff for: solr/core/src/test/org/apache/solr/TestSolrCoreProperties.java

@@ -49,7 +49,7 @@ public static void beforeTest() throws Exception {
     Files.createDirectories(dataDir);
     Files.createDirectories(confDir);
 
-    Files.copy(Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), homeDir.resolve("solr.xml"));
+    Files.copy(SolrTestCaseJ4.TEST_HOME().resolve("solr.xml"), homeDir.resolve("solr.xml"));
     String src_dir = TEST_HOME() + "/collection1/conf";
     Files.copy(Path.of(src_dir, "schema-tiny.xml"), confDir.resolve("schema.xml"));
     Files.copy(
@@ -71,7 +71,7 @@ public static void beforeTest() throws Exception {
     Properties nodeProperties = new Properties();
     // this sets the property for jetty starting SolrDispatchFilter
     if (System.getProperty("solr.data.dir") == null) {
-      nodeProperties.setProperty("solr.data.dir", createTempDir().toFile().getCanonicalPath());
+      nodeProperties.setProperty("solr.data.dir", createTempDir().toRealPath().toString());
     }
 
     solrClientTestRule.startSolr(homeDir, nodeProperties, JettyConfig.builder().build());

Diff for: solr/core/src/test/org/apache/solr/TestTolerantSearch.java

@@ -16,7 +16,6 @@
  */
 package org.apache.solr;
 
-import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.file.Files;
@@ -45,23 +44,23 @@ public class TestTolerantSearch extends SolrJettyTestBase {
   private static String shard1;
   private static String shard2;
 
-  private static File createSolrHome() throws Exception {
+  private static Path createSolrHome() throws Exception {
     Path workDir = createTempDir();
-    setupJettyTestHome(workDir.toFile(), "collection1");
+    setupJettyTestHome(workDir, "collection1");
     Files.copy(
         Path.of(SolrTestCaseJ4.TEST_HOME() + "/collection1/conf/solrconfig-tolerant-search.xml"),
         workDir.resolve("collection1").resolve("conf").resolve("solrconfig.xml"),
         StandardCopyOption.REPLACE_EXISTING);
     FileUtils.copyDirectory(
         workDir.resolve("collection1").toFile(), workDir.resolve("collection2").toFile());
-    return workDir.toFile();
+    return workDir;
   }
 
   @BeforeClass
   public static void createThings() throws Exception {
     systemSetPropertySolrDisableUrlAllowList("true");
-    File solrHome = createSolrHome();
-    createAndStartJetty(solrHome.getAbsolutePath());
+    Path solrHome = createSolrHome();
+    createAndStartJetty(solrHome);
     String url = getBaseUrl();
     collection1 = getHttpSolrClient(url, "collection1");
     collection2 = getHttpSolrClient(url, "collection2");

Diff for: solr/core/src/test/org/apache/solr/blockcache/BlockDirectoryTest.java

@@ -17,9 +17,9 @@
 package org.apache.solr.blockcache;
 
 import com.github.benmanes.caffeine.cache.Caffeine;
-import java.io.File;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.nio.file.Path;
 import java.util.Map;
 import java.util.Random;
 import org.apache.lucene.store.Directory;
@@ -99,16 +99,16 @@ public void releaseResources() {}
   private static final int MAX_BUFFER_SIZE = 12000;
   private static final int MAX_NUMBER_OF_READS = 20000;
   private BlockDirectory directory;
-  private File file;
+  private Path file;
   private Random random;
   private MapperCache mapperCache;
 
   @Override
   @Before
   public void setUp() throws Exception {
     super.setUp();
-    file = createTempDir().toFile();
-    FSDirectory dir = FSDirectory.open(new File(file, "base").toPath());
+    file = createTempDir();
+    FSDirectory dir = FSDirectory.open(file.resolve("base"));
     mapperCache = new MapperCache();
 
     if (random().nextBoolean()) {
@@ -138,7 +138,7 @@ public void tearDown() throws Exception {
 
   @Test
   public void testEOF() throws IOException {
-    Directory fsDir = FSDirectory.open(new File(file, "normal").toPath());
+    Directory fsDir = FSDirectory.open(file.resolve("normal"));
     String name = "test.eof";
     createFile(name, fsDir, directory);
     long fsLength = fsDir.fileLength(name);
@@ -170,7 +170,7 @@ public void testRandomAccessWrites() throws IOException {
     int i = 0;
     try {
       for (; i < 10; i++) {
-        Directory fsDir = FSDirectory.open(new File(file, "normal").toPath());
+        Directory fsDir = FSDirectory.open(file.resolve("normal"));
         String name = getName();
         createFile(name, fsDir, directory);
         assertInputsEquals(name, fsDir, directory);
@@ -253,9 +253,9 @@ private String getName() {
     return Long.toUnsignedString(random.nextLong());
   }
 
-  public static void rm(File file) {
+  public static void rm(Path file) {
     try {
-      IOUtils.rm(file.toPath());
+      IOUtils.rm(file);
     } catch (Throwable ignored) {
       // TODO: should this class care if a file couldnt be deleted?
       // this just emulates previous behavior, where only SecurityException would be handled.