SOLR-16903: Migrate from java.io.File to java.nio.file.Path in solr-c…

…ore (#2924) Co-authored-by: Matthew Biscocho <[email protected]>
apache · Feb 1, 2025 · b32b675 · b32b675
1 parent dac5fe1
commit b32b675
Show file tree

Hide file tree

Showing 26 changed files with 330 additions and 277 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -137,7 +137,7 @@ Other Changes
 
 * SOLR-17321: Minimum Java version for Apache Solr is now 21, and for SolrJ, it is 17. (Sanjay Dutt, David Smiley)
 
-* SOLR-16903: Update CLI tools to use java.nio.file.Path instead of java.io.File (Andrey Bozhko)
+* SOLR-16903: Update CLI tools and Solr Core to use java.nio.file.Path instead of java.io.File (Andrey Bozhko, Matthew Biscocho)
 
 * SOLR-17568: SolrCloud no longer reroutes/proxies a core request to another node if not found locally. (David Smiley)
 

diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
@@ -17,10 +17,10 @@
 package org.apache.solr.cloud;
 
 import java.io.ByteArrayInputStream;
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.invoke.MethodHandles;
+import java.nio.file.FileSystems;
 import java.nio.file.Path;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.ZooKeeperException;
@@ -121,7 +121,9 @@ public InputStream openResource(String resource) throws IOException {
 
     try {
       // delegate to the class loader (looking into $INSTANCE_DIR/lib jars)
-      is = classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'));
+      is =
+          classLoader.getResourceAsStream(
+              resource.replace(FileSystems.getDefault().getSeparator(), "/"));
     } catch (Exception e) {
       throw new IOException("Error opening " + resource, e);
     }

diff --git a/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
@@ -16,7 +16,6 @@
  */
 package org.apache.solr.core;
 
-import java.io.File;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.DirectoryStream;
@@ -367,10 +366,7 @@ private void close(CacheValue val) {
   }
 
   private static boolean isSubPath(CacheValue cacheValue, CacheValue otherCacheValue) {
-    int one = cacheValue.path.lastIndexOf(File.separatorChar);
-    int two = otherCacheValue.path.lastIndexOf(File.separatorChar);
-
-    return otherCacheValue.path.startsWith(cacheValue.path + File.separatorChar) && two > one;
+    return Path.of(otherCacheValue.path).startsWith(Path.of(cacheValue.path));
   }
 
   @Override

diff --git a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
@@ -16,11 +16,11 @@
  */
 package org.apache.solr.core;
 
-import java.io.File;
 import java.io.IOException;
 import java.io.Reader;
 import java.lang.invoke.MethodHandles;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.HashMap;
@@ -64,7 +64,7 @@ public class CoreDescriptor {
   public static final String SOLR_CORE_PROP_PREFIX = "solr.core.";
 
   public static final String DEFAULT_EXTERNAL_PROPERTIES_FILE =
-      "conf" + File.separator + "solrcore.properties";
+      "conf" + FileSystems.getDefault().getSeparator() + "solrcore.properties";
 
   /**
    * Whether this core was configured using a configSet that was trusted. This helps in avoiding the
@@ -96,7 +96,7 @@ public Properties getPersistableUserProperties() {
           CORE_CONFIG, "solrconfig.xml",
           CORE_SCHEMA, "schema.xml",
           CORE_CONFIGSET_PROPERTIES, ConfigSetProperties.DEFAULT_FILENAME,
-          CORE_DATADIR, "data" + File.separator,
+          CORE_DATADIR, "data" + FileSystems.getDefault().getSeparator(),
           CORE_TRANSIENT, "false",
           CORE_LOADONSTARTUP, "true");
 

diff --git a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java
@@ -17,17 +17,15 @@
 package org.apache.solr.core;
 
 import java.io.Closeable;
-import java.io.File;
-import java.io.FileFilter;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
+import java.util.stream.Stream;
 import org.apache.commons.io.file.PathUtils;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FilterDirectory;
@@ -338,59 +336,68 @@ public String getDataHome(CoreDescriptor cd) throws IOException {
   }
 
   public void cleanupOldIndexDirectories(
-      final String dataDirPath, final String currentIndexDirPath, boolean afterCoreReload) {
+      final String dataDirPath, final String currentIndexDirPath, boolean afterCoreReload)
+      throws IOException {
 
-    // TODO SOLR-8282 move to PATH
-    File dataDir = new File(dataDirPath);
-    if (!dataDir.isDirectory()) {
+    Path dataDirFile = Path.of(dataDirPath);
+    if (!Files.isDirectory(dataDirFile)) {
       log.debug(
           "{} does not point to a valid data directory; skipping clean-up of old index directories.",
           dataDirPath);
       return;
     }
 
-    final File currentIndexDir = new File(currentIndexDirPath);
-    File[] oldIndexDirs =
-        dataDir.listFiles(
-            new FileFilter() {
-              @Override
-              public boolean accept(File file) {
-                String fileName = file.getName();
-                return file.isDirectory()
-                    && !file.equals(currentIndexDir)
-                    && (fileName.equals("index") || fileName.matches(INDEX_W_TIMESTAMP_REGEX));
-              }
-            });
+    final Path currentIndexDir = Path.of(currentIndexDirPath);
+    List<Path> dirsList;
+    try (Stream<Path> oldIndexDirs = Files.list(dataDirFile)) {
+      dirsList =
+          oldIndexDirs
+              .filter(
+                  (file) -> {
+                    String fileName = file.getFileName().toString();
+                    return Files.isDirectory(file)
+                        && !file.equals(currentIndexDir)
+                        && (fileName.equals("index") || fileName.matches(INDEX_W_TIMESTAMP_REGEX));
+                  })
+              .sorted(Comparator.reverseOrder())
+              .toList();
+    }
+    ;
 
-    if (oldIndexDirs == null || oldIndexDirs.length == 0)
+    if (dirsList.isEmpty()) {
       return; // nothing to do (no log message needed)
-
-    List<File> dirsList = Arrays.asList(oldIndexDirs);
-    dirsList.sort(Collections.reverseOrder());
+    }
 
     int i = 0;
     if (afterCoreReload) {
-      log.info("Will not remove most recent old directory after reload {}", oldIndexDirs[0]);
+      if (log.isInfoEnabled())
+        log.info("Will not remove most recent old directory after reload {}", dirsList.getFirst());
       i = 1;
     }
-    log.info(
-        "Found {} old index directories to clean-up under {} afterReload={}",
-        oldIndexDirs.length - i,
-        dataDirPath,
-        afterCoreReload);
-    for (; i < dirsList.size(); i++) {
-      File dir = dirsList.get(i);
-      String dirToRmPath = dir.getAbsolutePath();
-      try {
-        if (deleteOldIndexDirectory(dirToRmPath)) {
-          log.info("Deleted old index directory: {}", dirToRmPath);
-        } else {
-          log.warn("Delete old index directory {} failed.", dirToRmPath);
-        }
-      } catch (IOException ioExc) {
-        log.error("Failed to delete old directory {} due to: ", dir.getAbsolutePath(), ioExc);
-      }
-    }
+
+    if (log.isInfoEnabled())
+      log.info(
+          "Found {} old index directories to clean-up under {} afterReload={}",
+          dirsList.size() - i,
+          dataDirPath,
+          afterCoreReload);
+
+    dirsList.stream()
+        .skip(i)
+        .forEach(
+            (entry) -> {
+              String dirToRmPath = entry.toAbsolutePath().toString();
+              try {
+                if (deleteOldIndexDirectory(dirToRmPath)) {
+                  log.info("Deleted old index directory: {}", dirToRmPath);
+                } else {
+                  log.warn("Delete old index directory {} failed.", dirToRmPath);
+                }
+              } catch (IOException ioExc) {
+                log.error(
+                    "Failed to delete old directory {} due to: ", entry.toAbsolutePath(), ioExc);
+              }
+            });
   }
 
   // Extension point to allow subclasses to infuse additional code when deleting old index

diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -22,7 +22,6 @@
 import com.codahale.metrics.Counter;
 import com.codahale.metrics.Timer;
 import java.io.Closeable;
-import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
@@ -33,9 +32,9 @@
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Constructor;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -1354,14 +1353,32 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
           Category.CORE.toString());
     }
 
-    // TODO SOLR-8282 move to PATH
     // initialize disk total / free metrics
-    Path dataDirPath = Paths.get(dataDir);
-    File dataDirFile = dataDirPath.toFile();
+    Path dataDirPath = Path.of(dataDir);
     parentContext.gauge(
-        () -> dataDirFile.getTotalSpace(), true, "totalSpace", Category.CORE.toString(), "fs");
+        () -> {
+          try {
+            return Files.getFileStore(dataDirPath).getTotalSpace();
+          } catch (IOException e) {
+            return 0L;
+          }
+        },
+        true,
+        "totalSpace",
+        Category.CORE.toString(),
+        "fs");
     parentContext.gauge(
-        () -> dataDirFile.getUsableSpace(), true, "usableSpace", Category.CORE.toString(), "fs");
+        () -> {
+          try {
+            return Files.getFileStore(dataDirPath).getUsableSpace();
+          } catch (IOException e) {
+            return 0L;
+          }
+        },
+        true,
+        "usableSpace",
+        Category.CORE.toString(),
+        "fs");
     parentContext.gauge(
         () -> dataDirPath.toAbsolutePath().toString(),
         true,

diff --git a/solr/core/src/java/org/apache/solr/core/SolrPaths.java b/solr/core/src/java/org/apache/solr/core/SolrPaths.java
@@ -17,8 +17,8 @@
 
 package org.apache.solr.core;
 
-import java.io.File;
 import java.lang.invoke.MethodHandles;
+import java.nio.file.FileSystems;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Collections;
@@ -44,7 +44,7 @@ private SolrPaths() {} // don't create this
   /** Ensures a directory name always ends with a '/'. */
   public static String normalizeDir(String path) {
     return (path != null && (!(path.endsWith("/") || path.endsWith("\\"))))
-        ? path + File.separator
+        ? path + FileSystems.getDefault().getSeparator()
         : path;
   }
 

diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -18,7 +18,6 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import java.io.Closeable;
-import java.io.File;
 import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -31,6 +30,7 @@
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.DirectoryStream;
+import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.PathMatcher;
@@ -373,12 +373,16 @@ public InputStream openResource(String resource) throws IOException {
 
     // Delegate to the class loader (looking into $INSTANCE_DIR/lib jars).
     // We need a ClassLoader-compatible (forward-slashes) path here!
-    InputStream is = classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'));
+    InputStream is =
+        classLoader.getResourceAsStream(
+            resource.replace(FileSystems.getDefault().getSeparator(), "/"));
 
     // This is a hack just for tests (it is not done in ZKResourceLoader)!
     // TODO can we nuke this?
     if (is == null && System.getProperty("jetty.testMode") != null) {
-      is = classLoader.getResourceAsStream(("conf/" + resource).replace(File.separatorChar, '/'));
+      is =
+          classLoader.getResourceAsStream(
+              ("conf/" + resource.replace(FileSystems.getDefault().getSeparator(), "/")));
     }
 
     if (is == null) {
@@ -405,7 +409,8 @@ public String resourceLocation(String resource) {
     }
 
     try (InputStream is =
-        classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'))) {
+        classLoader.getResourceAsStream(
+            resource.replace(FileSystems.getDefault().getSeparator(), "/"))) {
       if (is != null) return "classpath:" + resource;
     } catch (IOException e) {
       // ignore

diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
@@ -16,14 +16,14 @@
  */
 package org.apache.solr.core.backup;
 
-import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.lang.invoke.MethodHandles;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystems;
 import java.time.Instant;
 import java.util.Collections;
 import java.util.List;
@@ -343,7 +343,8 @@ private void downloadConfigToRepo(ConfigSetService configSetService, String conf
     // getAllConfigFiles always separates file paths with '/'
     for (String filePath : filePaths) {
       // Replace '/' to ensure that propre file is resolved for writing.
-      URI uri = repository.resolve(dir, filePath.replace('/', File.separatorChar));
+      URI uri =
+          repository.resolve(dir, filePath.replace("/", FileSystems.getDefault().getSeparator()));
       // checking for '/' is correct for a directory since ConfigSetService#getAllConfigFiles
       // always separates file paths with '/'
       if (!filePath.endsWith("/")) {