From 6cc27d7ab1482178e125162f21c37daef6a25233 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Sun, 22 Dec 2024 01:24:23 -0500 Subject: [PATCH 01/10] Inital core File to Path commit --- .../solr/cloud/ZkSolrResourceLoader.java | 3 +- .../solr/core/CachingDirectoryFactory.java | 6 +- .../org/apache/solr/core/CoreDescriptor.java | 9 +- .../apache/solr/core/DirectoryFactory.java | 102 ++++++++++-------- .../java/org/apache/solr/core/SolrCore.java | 28 +++-- .../java/org/apache/solr/core/SolrPaths.java | 5 +- .../apache/solr/core/SolrResourceLoader.java | 21 ++-- .../solr/core/backup/BackupManager.java | 4 +- .../solr/filestore/DistribFileStore.java | 79 +++++++------- .../org/apache/solr/filestore/FileStore.java | 2 +- .../apache/solr/filestore/NodeFileStore.java | 2 +- .../org/apache/solr/handler/IndexFetcher.java | 61 ++++++----- .../solr/handler/admin/CoreAdminHandler.java | 8 -- .../handler/admin/CoreAdminOperation.java | 3 +- .../handler/admin/ShowFileRequestHandler.java | 78 ++++++++------ .../solr/rest/ManagedResourceStorage.java | 6 +- .../schema/ManagedIndexSchemaFactory.java | 12 +-- .../spelling/AbstractLuceneSpellChecker.java | 3 +- .../fst/AnalyzingInfixLookupFactory.java | 3 +- .../fst/BlendedInfixLookupFactory.java | 3 +- .../org/apache/solr/util/RegexFileFilter.java | 10 +- .../apache/solr/util/SystemIdResolver.java | 6 +- .../org/apache/solr/util/VersionedFile.java | 53 +++++---- 23 files changed, 258 insertions(+), 249 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java index 68654851d5b..5e0ac07484c 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java @@ -17,7 +17,6 @@ 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; @@ -121,7 +120,7 @@ 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(Path.of(resource).toString()); } 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 index dfe9487809e..a84663c3b51 100644 --- 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 index 6109b2f860b..436380af7a5 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java +++ b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java @@ -16,13 +16,13 @@ */ 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.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -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"; + Paths.get("conf", "solrcore.properties").toString(); /** * 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, Paths.get("data/").toString(), CORE_TRANSIENT, "false", CORE_LOADONSTARTUP, "true"); @@ -240,7 +240,8 @@ public CoreDescriptor( *

File paths are taken as read from the core's instance directory if they are not absolute. */ protected void loadExtraProperties() { - String filename = coreProperties.getProperty(CORE_PROPERTIES, DEFAULT_EXTERNAL_PROPERTIES_FILE); + String filename = + coreProperties.getProperty(CORE_PROPERTIES, DEFAULT_EXTERNAL_PROPERTIES_FILE.toString()); Path propertiesFile = instanceDir.resolve(filename); if (Files.exists(propertiesFile)) { try (Reader r = Files.newBufferedReader(propertiesFile, StandardCharsets.UTF_8)) { diff --git a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java index aebbd64b7a1..c93c7a366d6 100644 --- 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,16 @@ 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.DirectoryStream; 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.StreamSupport; import org.apache.commons.io.file.PathUtils; import org.apache.lucene.store.Directory; import org.apache.lucene.store.FilterDirectory; @@ -338,59 +337,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)); - } - }); - - if (oldIndexDirs == null || oldIndexDirs.length == 0) - return; // nothing to do (no log message needed) - - List 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]); - 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); + final Path currentIndexDir = Path.of(currentIndexDirPath); + try (DirectoryStream oldIndexDirs = + Files.newDirectoryStream( + dataDirFile, + file -> { + String fileName = file.getFileName().toString(); + return Files.isDirectory(file) + && !file.equals(currentIndexDir) + && (fileName.equals("index") || fileName.matches(INDEX_W_TIMESTAMP_REGEX)); + })) { + + List dirsList = + StreamSupport.stream(oldIndexDirs.spliterator(), false) + .sorted(Comparator.comparing(Path::toString)) + .toList(); + + if (dirsList.isEmpty()) { + return; // nothing to do (no log message needed) } + + int i = 0; + if (afterCoreReload) { + log.info("Will not remove most recent old directory after reload {}", dirsList.get(0)); + i = 1; + } + + 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 index 3611b943feb..ec3e291e636 100644 --- 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,6 +32,7 @@ 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; @@ -1354,16 +1354,26 @@ 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(); - parentContext.gauge( - () -> dataDirFile.getTotalSpace(), true, "totalSpace", Category.CORE.toString(), "fs"); - parentContext.gauge( - () -> dataDirFile.getUsableSpace(), true, "usableSpace", Category.CORE.toString(), "fs"); + Path dataDirFile = Paths.get(dataDir); + long totalSpace; + long useableSpace; + + try { + totalSpace = Files.getFileStore(dataDirFile).getTotalSpace(); + useableSpace = Files.getFileStore(dataDirFile).getUsableSpace(); + } catch (IOException e) { + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, + "Metrics initialization failed to retrieve files total/usable space", + e); + } + + parentContext.gauge(() -> totalSpace, true, "totalSpace", Category.CORE.toString(), "fs"); + parentContext.gauge(() -> useableSpace, true, "usableSpace", Category.CORE.toString(), "fs"); + parentContext.gauge( - () -> dataDirPath.toAbsolutePath().toString(), + () -> dataDirFile.toAbsolutePath().toString(), true, "path", Category.CORE.toString(), diff --git a/solr/core/src/java/org/apache/solr/core/SolrPaths.java b/solr/core/src/java/org/apache/solr/core/SolrPaths.java index 7906c1e219f..e938c0d83cb 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrPaths.java +++ b/solr/core/src/java/org/apache/solr/core/SolrPaths.java @@ -17,7 +17,6 @@ package org.apache.solr.core; -import java.io.File; import java.lang.invoke.MethodHandles; import java.nio.file.Path; import java.nio.file.Paths; @@ -43,9 +42,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; + return (path != null && (!(path.endsWith("/") || path.endsWith("\\")))) ? path + "/" : 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 index 11f61a12ae2..f777f7a4fd2 100644 --- 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; @@ -34,6 +33,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.PathMatcher; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -353,12 +353,14 @@ public ClassLoader getClassLoader() { */ @Override public InputStream openResource(String resource) throws IOException { - if (resource.trim().startsWith("\\\\")) { // Always disallow UNC paths - throw new SolrResourceNotFoundException("Resource '" + resource + "' could not be loaded."); + String pathResource = Paths.get(resource).normalize().toString(); + if (pathResource.trim().startsWith("\\\\")) { // Always disallow UNC paths + throw new SolrResourceNotFoundException( + "Resource '" + pathResource + "' could not be loaded."); } Path instanceDir = getInstancePath().normalize(); - Path inInstanceDir = getInstancePath().resolve(resource).normalize(); - Path inConfigDir = instanceDir.resolve("conf").resolve(resource).normalize(); + Path inInstanceDir = getInstancePath().resolve(pathResource).normalize(); + Path inConfigDir = instanceDir.resolve("conf").resolve(pathResource).normalize(); if (allowUnsafeResourceloading || inInstanceDir.startsWith(instanceDir)) { // The resource is either inside instance dir or we allow unsafe loading, so allow testing if // file exists @@ -373,17 +375,17 @@ 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(pathResource); // 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(Path.of("conf").resolve(pathResource).toString()); } if (is == null) { throw new SolrResourceNotFoundException( - "Can't find resource '" + resource + "' in classpath or '" + instanceDir + "'"); + "Can't find resource '" + pathResource + "' in classpath or '" + instanceDir + "'"); } return is; } @@ -404,8 +406,7 @@ public String resourceLocation(String resource) { return inInstanceDir.normalize().toString(); } - try (InputStream is = - classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'))) { + try (InputStream is = classLoader.getResourceAsStream(Path.of(resource).toString())) { 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 index df3353a7b31..687fa989182 100644 --- 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,7 +16,6 @@ */ package org.apache.solr.core.backup; -import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; @@ -24,6 +23,7 @@ import java.lang.invoke.MethodHandles; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.time.Instant; import java.util.Collections; import java.util.List; @@ -343,7 +343,7 @@ 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, Path.of(filePath).toString()); // checking for '/' is correct for a directory since ConfigSetService#getAllConfigFiles // always separates file paths with '/' if (!filePath.endsWith("/")) { diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index 1a462dee26e..41d93dd1196 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -22,7 +22,6 @@ import static org.apache.solr.common.SolrException.ErrorCode.SERVER_ERROR; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; @@ -43,6 +42,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import java.util.function.Predicate; +import java.util.stream.Stream; import net.jcip.annotations.NotThreadSafe; import org.apache.commons.codec.digest.DigestUtils; import org.apache.http.client.HttpClient; @@ -85,13 +85,8 @@ public Path getRealpath(String path) { } private static Path _getRealPath(String path, Path solrHome) { - if (File.separatorChar == '\\') { - path = path.replace('/', File.separatorChar); - } - SolrPaths.assertNotUnc(Path.of(path)); - while (path.startsWith(File.separator)) { // Trim all leading slashes - path = path.substring(1); - } + Path dir = Path.of(path); + SolrPaths.assertNotUnc(dir); var finalPath = getFileStoreDirPath(solrHome).resolve(path); // Guard against path traversal by asserting final path is sub path of filestore if (!finalPath.normalize().startsWith(getFileStoreDirPath(solrHome).normalize())) { @@ -250,9 +245,9 @@ public Path realPath() { @SuppressWarnings("unchecked") MetaData readMetaData() throws IOException { - File file = getRealpath(getMetaPath()).toFile(); - if (file.exists()) { - try (InputStream fis = new FileInputStream(file)) { + Path file = getRealpath(getMetaPath()); + if (Files.exists(file)) { + try (InputStream fis = new FileInputStream(file.toString())) { return new MetaData((Map) Utils.fromJSON(fis)); } } @@ -428,10 +423,10 @@ public boolean fetch(String path, String from) { @Override public void get(String path, Consumer consumer, boolean fetchmissing) throws IOException { - File file = getRealpath(path).toFile(); - String simpleName = file.getName(); + Path file = getRealpath(path); + String simpleName = file.getFileName().toString(); if (isMetaDataFile(simpleName)) { - try (InputStream is = new FileInputStream(file)) { + try (InputStream is = new FileInputStream(file.toString())) { consumer.accept( new FileEntry(null, null, path) { // no metadata for metadata file @@ -458,25 +453,25 @@ public void syncToAllNodes(String path) throws IOException { } @Override - public List list(String path, Predicate predicate) { - File file = getRealpath(path).toFile(); + public List list(String path, Predicate predicate) throws IOException { + Path file = getRealpath(path); List fileDetails = new ArrayList<>(); FileType type = getType(path, false); if (type == FileType.DIRECTORY) { - file.list( - (dir, name) -> { - if (predicate == null || predicate.test(name)) { - if (!isMetaDataFile(name)) { - fileDetails.add(new FileInfo(path + "/" + name).getDetails()); + try (Stream fileStream = Files.list(file)) { + fileStream.forEach( + (f) -> { + String fileName = f.getFileName().toString(); + if (predicate == null || predicate.test(fileName)) { + if (!isMetaDataFile(fileName)) { + fileDetails.add(new FileInfo(path + "/" + fileName).getDetails()); + } } - } - return false; - }); - + }); + } } else if (type == FileType.FILE) { fileDetails.add(new FileInfo(path).getDetails()); } - return fileDetails; } @@ -550,19 +545,19 @@ public void refresh(String path) { @Override public FileType getType(String path, boolean fetchMissing) { - File file = getRealpath(path).toFile(); - if (!file.exists() && fetchMissing) { + Path file = getRealpath(path); + if (!Files.exists(file) && fetchMissing) { if (fetch(path, null)) { - file = getRealpath(path).toFile(); + file = getRealpath(path); } } return _getFileType(file); } - public static FileType _getFileType(File file) { - if (!file.exists()) return FileType.NOFILE; - if (file.isDirectory()) return FileType.DIRECTORY; - return isMetaDataFile(file.getName()) ? FileType.METADATA : FileType.FILE; + public static FileType _getFileType(Path file) { + if (!Files.exists(file)) return FileType.NOFILE; + if (Files.isDirectory(file)) return FileType.DIRECTORY; + return isMetaDataFile(file.getFileName().toString()) ? FileType.METADATA : FileType.FILE; } public static boolean isMetaDataFile(String file) { @@ -620,13 +615,17 @@ public Map getKeys() throws IOException { private static Map _getKeys(Path solrHome) throws IOException { Map result = new HashMap<>(); Path keysDir = _getRealPath(ClusterFileStore.KEYS_DIR, solrHome); - - File[] keyFiles = keysDir.toFile().listFiles(); - if (keyFiles == null) return result; - for (File keyFile : keyFiles) { - if (keyFile.isFile() && !isMetaDataFile(keyFile.getName())) { - result.put(keyFile.getName(), Files.readAllBytes(keyFile.toPath())); - } + try (Stream fileStream = Files.list(keysDir)) { + fileStream.forEach( + (keyFile) -> { + if (Files.isRegularFile(keyFile) && !isMetaDataFile(keyFile.getFileName().toString())) { + try { + result.put(keyFile.getFileName().toString(), Files.readAllBytes(keyFile)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }); } return result; } diff --git a/solr/core/src/java/org/apache/solr/filestore/FileStore.java b/solr/core/src/java/org/apache/solr/filestore/FileStore.java index 916b8b33b3b..cc5d1d59365 100644 --- a/solr/core/src/java/org/apache/solr/filestore/FileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/FileStore.java @@ -44,7 +44,7 @@ public interface FileStore { /** Fetch a resource from another node internal API */ boolean fetch(String path, String from); - List list(String path, Predicate predicate); + List list(String path, Predicate predicate) throws IOException; /** Sync a local file to all nodes. All the nodes are asked to pull the file from this node */ void syncToAllNodes(String path) throws IOException; diff --git a/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java b/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java index e6c61419437..f1b2a5843d6 100644 --- a/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java @@ -72,7 +72,7 @@ public NodeFileStore( // it up into multiple distinct APIs @Override @PermissionName(FILESTORE_READ_PERM) - public SolrJerseyResponse getFile(String path, Boolean sync, String getFrom, Boolean meta) { + public SolrJerseyResponse getFile(String path, Boolean sync, String getFrom, Boolean meta) throws IOException { final var response = instantiateJerseyResponse(SolrJerseyResponse.class); if (Boolean.TRUE.equals(sync)) { diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index a5a467386db..19010584575 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -37,7 +37,6 @@ import static org.apache.solr.handler.admin.api.ReplicationAPIBase.GENERATION; import static org.apache.solr.handler.admin.api.ReplicationAPIBase.OFFSET; -import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; @@ -1061,25 +1060,23 @@ private void reloadCore() { } private void downloadConfFiles( - List> confFilesToDownload, long latestGeneration) throws Exception { + List> confFilesToDownload, long latestGeneration) { log.info("Starting download of configuration files from leader: {}", confFilesToDownload); confFilesDownloaded = Collections.synchronizedList(new ArrayList<>()); - Path tmpConfPath = + Path tmpconfDir = solrCore.getResourceLoader().getConfigPath().resolve("conf." + getDateAsStr(new Date())); - // TODO SOLR-8282 move to PATH - File tmpconfDir = tmpConfPath.toFile(); try { - boolean status = tmpconfDir.mkdirs(); - if (!status) { + try { + Files.createDirectories(tmpconfDir); + } catch (Exception e) { throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, - "Failed to create temporary config folder: " + tmpconfDir.getName()); + "Failed to create temporary config folder: " + tmpconfDir.getFileName()); } for (Map file : confFilesToDownload) { String saveAs = (String) (file.get(ALIAS) == null ? file.get(NAME) : file.get(ALIAS)); localFileFetcher = - new LocalFsFileFetcher( - tmpconfDir.toPath(), file, saveAs, CONF_FILE_SHORT, latestGeneration); + new LocalFsFileFetcher(tmpconfDir, file, saveAs, CONF_FILE_SHORT, latestGeneration); currentFile = file; localFileFetcher.fetchFile(); confFilesDownloaded.add(new HashMap<>(file)); @@ -1087,9 +1084,13 @@ private void downloadConfFiles( // this is called before copying the files to the original conf dir // so that if there is an exception avoid corrupting the original files. terminateAndWaitFsyncService(); - copyTmpConfFiles2Conf(tmpConfPath); + copyTmpConfFiles2Conf(tmpconfDir); + + } catch (Exception e) { + throw new SolrException( + ErrorCode.SERVER_ERROR, "Failed to download configuration files from leader", e); } finally { - delTree(tmpconfDir.toPath()); + delTree(tmpconfDir); } } @@ -1156,23 +1157,22 @@ private long downloadIndexFiles( alwaysDownload); } if (!compareResult.equal || downloadCompleteIndex || alwaysDownload) { - // TODO SOLR-8282 move to PATH - File localFile = new File(indexDirPath, filename); + Path localFile = Path.of(indexDirPath, filename); if (downloadCompleteIndex && doDifferentialCopy && compareResult.equal && compareResult.checkSummed - && localFile.exists()) { + && Files.exists(localFile)) { if (log.isInfoEnabled()) { log.info( "Don't need to download this file. Local file's path is: {}, checksum is: {}", - localFile.getAbsolutePath(), + localFile.toAbsolutePath(), file.get(CHECKSUM)); } // A hard link here should survive the eventual directory move, and should be more space // efficient as compared to a file copy. TODO: Maybe we could do a move safely here? - Files.createLink(Path.of(tmpIndexDirPath, filename), localFile.toPath()); - bytesSkippedCopying += localFile.length(); + Files.createLink(Path.of(tmpIndexDirPath, filename), localFile); + bytesSkippedCopying += Files.size(localFile); } else { dirFileFetcher = new DirectoryFileFetcher( @@ -1484,6 +1484,7 @@ private void copyTmpConfFiles2Conf(Path tmpconfDir) { int numTempPathElements = tmpconfDir.getNameCount(); for (Path path : makeTmpConfDirFileList(tmpconfDir)) { Path oldPath = confPath.resolve(path.subpath(numTempPathElements, path.getNameCount())); + try { Files.createDirectories(oldPath.getParent()); } catch (IOException e) { @@ -1491,21 +1492,19 @@ private void copyTmpConfFiles2Conf(Path tmpconfDir) { ErrorCode.SERVER_ERROR, "Unable to mkdirs: " + oldPath.getParent(), e); } if (Files.exists(oldPath)) { - File oldFile = oldPath.toFile(); // TODO SOLR-8282 move to PATH - File backupFile = - new File(oldFile.getPath() + "." + getDateAsStr(new Date(oldFile.lastModified()))); - if (!backupFile.getParentFile().exists()) { - status = backupFile.getParentFile().mkdirs(); - if (!status) { - throw new SolrException( - ErrorCode.SERVER_ERROR, "Unable to mkdirs: " + backupFile.getParentFile()); + try { + Path backupFile = + Path.of( + oldPath + + "." + + getDateAsStr(new Date(Files.getLastModifiedTime(oldPath).toMillis()))); + if (!Files.exists(backupFile.getParent())) { + Files.createDirectories(backupFile.getParent()); } - } - status = oldFile.renameTo(backupFile); - if (!status) { + Files.move(oldPath, backupFile); + } catch (Exception e) { throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Unable to rename: " + oldFile + " to: " + backupFile); + SolrException.ErrorCode.SERVER_ERROR, "Unable to backup old file: " + oldPath, e); } } try { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java index c35117fb088..85ad5cdc4a9 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java @@ -25,7 +25,6 @@ import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.Expiry; import com.github.benmanes.caffeine.cache.Ticker; -import java.io.File; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collection; @@ -319,13 +318,6 @@ protected static Map buildCoreParams(SolrParams params) { return coreParams; } - protected static String normalizePath(String path) { - if (path == null) return null; - path = path.replace('/', File.separatorChar); - path = path.replace('\\', File.separatorChar); - return path; - } - public static ModifiableSolrParams params(String... params) { ModifiableSolrParams msp = new ModifiableSolrParams(); for (int i = 0; i < params.length; i += 2) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java index 0aafba7c70b..7cfcb25db1b 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java @@ -45,7 +45,6 @@ import static org.apache.solr.common.params.CoreAdminParams.SHARD; import static org.apache.solr.handler.admin.CoreAdminHandler.CallInfo; import static org.apache.solr.handler.admin.CoreAdminHandler.buildCoreParams; -import static org.apache.solr.handler.admin.CoreAdminHandler.normalizePath; import java.io.IOException; import java.lang.invoke.MethodHandles; @@ -357,7 +356,7 @@ public static NamedList getCoreStatus( if (core != null) { info.add(NAME, core.getName()); info.add("instanceDir", core.getInstancePath().toString()); - info.add("dataDir", normalizePath(core.getDataDir())); + info.add("dataDir", Path.of(core.getDataDir()).toString()); info.add("config", core.getConfigResource()); info.add("schema", core.getSchemaResource()); info.add("startTime", core.getStartTimeStamp()); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java index 1ec536f3c22..6b99f2ff924 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java @@ -16,7 +16,6 @@ */ package org.apache.solr.handler.admin; -import java.io.File; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.lang.invoke.MethodHandles; @@ -28,6 +27,7 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.stream.Stream; import org.apache.solr.cloud.ZkSolrResourceLoader; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; @@ -195,54 +195,72 @@ private void showFromZooKeeper( } // Return the file indicated (or the directory listing) from the local file system. - private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) { + private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException { Path admin = getAdminFileFromFileSystem(req, rsp, hiddenFiles); if (admin == null) { // exception already recorded return; } - // TODO SOLR-8282 move to PATH - File adminFile = admin.toFile(); + Path adminFile = admin; // Make sure the file exists, is readable and is not a hidden file - if (!adminFile.exists()) { - log.error("Can not find: {} [{}]", adminFile.getName(), adminFile.getAbsolutePath()); + if (!Files.exists(adminFile)) { + log.error("Can not find: {} [{}]", adminFile.getFileName(), adminFile.toAbsolutePath()); rsp.setException( new SolrException( ErrorCode.NOT_FOUND, - "Can not find: " + adminFile.getName() + " [" + adminFile.getAbsolutePath() + "]")); + "Can not find: " + + adminFile.getFileName() + + " [" + + adminFile.toAbsolutePath() + + "]")); return; } - if (!adminFile.canRead() || adminFile.isHidden()) { - log.error("Can not show: {} [{}]", adminFile.getName(), adminFile.getAbsolutePath()); + if (!Files.isReadable(adminFile) || Files.isHidden(adminFile)) { + log.error("Can not show: {} [{}]", adminFile.getFileName(), adminFile.toAbsolutePath()); rsp.setException( new SolrException( ErrorCode.NOT_FOUND, - "Can not show: " + adminFile.getName() + " [" + adminFile.getAbsolutePath() + "]")); + "Can not show: " + + adminFile.getFileName() + + " [" + + adminFile.toAbsolutePath() + + "]")); return; } // Show a directory listing - if (adminFile.isDirectory()) { + if (Files.isDirectory(adminFile)) { // it's really a directory, just go for it. - int basePath = adminFile.getAbsolutePath().length() + 1; + int basePath = (int) (Files.size(adminFile.toAbsolutePath()) + 1); NamedList> files = new SimpleOrderedMap<>(); - for (File f : adminFile.listFiles()) { - String path = f.getAbsolutePath().substring(basePath); - path = path.replace('\\', '/'); // normalize slashes - - if (isHiddenFile(req, rsp, f.getName().replace('\\', '/'), false, hiddenFiles)) { - continue; - } - - SimpleOrderedMap fileInfo = new SimpleOrderedMap<>(); - files.add(path, fileInfo); - if (f.isDirectory()) { - fileInfo.add("directory", true); - } else { - // TODO? content type - fileInfo.add("size", f.length()); - } - fileInfo.add("modified", new Date(f.lastModified())); + try (Stream directoryFiles = Files.list(adminFile)) { + directoryFiles.forEach( + (f) -> { + String path = f.toAbsolutePath().toString().substring(basePath); + path = path.replace('\\', '/'); // normalize slashes + + if (isHiddenFile( + req, rsp, f.getFileName().toString().replace('\\', '/'), false, hiddenFiles)) { + return; + } + + SimpleOrderedMap fileInfo = new SimpleOrderedMap<>(); + files.add(path, fileInfo); + if (Files.isDirectory(f)) { + fileInfo.add("directory", true); + } else { + // TODO? content type + try { + fileInfo.add("size", Files.size(f)); + fileInfo.add("modified", new Date(Files.getLastModifiedTime(f).toMillis())); + } catch (Exception e) { + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, + "Unable to retrieve file metadata: " + f, + e); + } + } + }); } rsp.add("files", files); } else { @@ -252,7 +270,7 @@ private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) { params.set(CommonParams.WT, "raw"); req.setParams(params); - ContentStreamBase content = new ContentStreamBase.FileStream(adminFile.toPath()); + ContentStreamBase content = new ContentStreamBase.FileStream(adminFile); content.setContentType(getSafeContentType(req.getParams().get(USE_CONTENT_TYPE))); rsp.add(RawResponseWriter.CONTENT, content); diff --git a/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java b/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java index 038b9c5f421..b2f449aeb86 100644 --- a/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java +++ b/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java @@ -20,7 +20,6 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; @@ -203,9 +202,8 @@ public OutputStream openOutputStream(String storedResourceId) throws IOException @Override public boolean delete(String storedResourceId) throws IOException { - // TODO SOLR-8282 move to PATH - File storedFile = new File(storageDir, storedResourceId); - return deleteIfFile(storedFile.toPath()); + Path storedFile = Path.of(storageDir, storedResourceId); + return deleteIfFile(storedFile); } // TODO: this interface should probably be changed, this simulates the old behavior, diff --git a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java index df44cda12e2..4d2f3e1739b 100644 --- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java @@ -17,7 +17,6 @@ package org.apache.solr.schema; import java.io.ByteArrayInputStream; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.lang.invoke.MethodHandles; @@ -401,8 +400,7 @@ private void upgradeToManagedSchema() { "On upgrading to managed schema, did not rename non-managed schema '{}' because it's the same as the managed schema's name.", resourceName); } else { - // TODO SOLR-8282 move to PATH - final File nonManagedSchemaFile = locateConfigFile(resourceName).toFile(); + final Path nonManagedSchemaFile = locateConfigFile(resourceName); if (null == nonManagedSchemaFile) { // Don't throw an exception for failure to rename the non-managed schema log.warn( @@ -412,17 +410,17 @@ private void upgradeToManagedSchema() { "nor under SolrConfig.getConfigDir() or the current directory. ", "PLEASE REMOVE THIS FILE."); } else { - File upgradedSchemaFile = new File(nonManagedSchemaFile + UPGRADED_SCHEMA_EXTENSION); - if (nonManagedSchemaFile.renameTo(upgradedSchemaFile)) { + Path upgradedSchemaFile = Path.of(nonManagedSchemaFile + UPGRADED_SCHEMA_EXTENSION); + try { + Files.move(nonManagedSchemaFile, upgradedSchemaFile); // Set the resource name to the managed schema so that the CoreAdminHandler returns a // findable filename schema.setResourceName(managedSchemaResourceName); - log.info( "After upgrading to managed schema, renamed the non-managed schema {} to {}", nonManagedSchemaFile, upgradedSchemaFile); - } else { + } catch (Exception e) { // Don't throw an exception for failure to rename the non-managed schema log.warn( "Can't rename {} to {} - PLEASE REMOVE THIS FILE.", diff --git a/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java b/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java index 1c0e7006185..3ea62098abc 100644 --- a/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java +++ b/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java @@ -16,7 +16,6 @@ */ package org.apache.solr.spelling; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.Arrays; @@ -84,7 +83,7 @@ public String init(NamedList config, SolrCore core) { // If indexDir is relative then create index inside core.getDataDir() if (indexDir != null) { if (!Path.of(indexDir).isAbsolute()) { - indexDir = core.getDataDir() + File.separator + indexDir; + indexDir = Path.of(core.getDataDir(), indexDir).toString(); } } sourceLocation = (String) config.get(LOCATION); diff --git a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java index 682b9a81a29..5b8115054b9 100644 --- a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java +++ b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java @@ -16,7 +16,6 @@ */ package org.apache.solr.spelling.suggest.fst; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; @@ -89,7 +88,7 @@ public Lookup create(NamedList params, SolrCore core) { String indexPath = params.get(INDEX_PATH) != null ? params.get(INDEX_PATH).toString() : DEFAULT_INDEX_PATH; if (!Path.of(indexPath).isAbsolute()) { - indexPath = core.getDataDir() + File.separator + indexPath; + indexPath = Path.of(core.getDataDir(), indexPath).toString(); } int minPrefixChars = diff --git a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java index 09a85917c94..c07164fb801 100644 --- a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java +++ b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java @@ -16,7 +16,6 @@ */ package org.apache.solr.spelling.suggest.fst; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; @@ -80,7 +79,7 @@ public Lookup create(NamedList params, SolrCore core) { String indexPath = params.get(INDEX_PATH) != null ? params.get(INDEX_PATH).toString() : DEFAULT_INDEX_PATH; if (!Path.of(indexPath).isAbsolute()) { - indexPath = core.getDataDir() + File.separator + indexPath; + indexPath = Path.of(core.getDataDir(), indexPath).toString(); } int minPrefixChars = diff --git a/solr/core/src/java/org/apache/solr/util/RegexFileFilter.java b/solr/core/src/java/org/apache/solr/util/RegexFileFilter.java index 3ed84d93c20..78b938acf3b 100644 --- a/solr/core/src/java/org/apache/solr/util/RegexFileFilter.java +++ b/solr/core/src/java/org/apache/solr/util/RegexFileFilter.java @@ -16,12 +16,11 @@ */ package org.apache.solr.util; -import java.io.File; -import java.io.FileFilter; +import java.nio.file.Path; import java.util.regex.Pattern; /** Accepts any file whose name matches the pattern */ -public final class RegexFileFilter implements FileFilter { +public final class RegexFileFilter { final Pattern pattern; @@ -33,9 +32,8 @@ public RegexFileFilter(Pattern regex) { pattern = regex; } - @Override - public boolean accept(File f) { - return pattern.matcher(f.getName()).matches(); + public boolean accept(Path f) { + return pattern.matcher(f.getFileName().toString()).matches(); } @Override diff --git a/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java b/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java index 6d1267d76f4..f311fa3df18 100644 --- a/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java +++ b/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java @@ -16,10 +16,10 @@ */ package org.apache.solr.util; -import java.io.File; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Path; import javax.xml.stream.XMLResolver; import javax.xml.stream.XMLStreamException; import javax.xml.transform.Source; @@ -167,9 +167,9 @@ public InputSource resolveEntity(String publicId, String systemId) throws IOExce } public static String createSystemIdFromResourceName(String name) { - name = name.replace(File.separatorChar, '/'); + Path pathName = Path.of(name); final String authority; - if (name.startsWith("/")) { + if (pathName.startsWith("/")) { // a hack to preserve absolute filenames and keep them absolute after resolving, we set the // URI's authority to "@" on absolute filenames: authority = RESOURCE_LOADER_AUTHORITY_ABSOLUTE; diff --git a/solr/core/src/java/org/apache/solr/util/VersionedFile.java b/solr/core/src/java/org/apache/solr/util/VersionedFile.java index 12ffdb63b53..972a5647043 100644 --- a/solr/core/src/java/org/apache/solr/util/VersionedFile.java +++ b/solr/core/src/java/org/apache/solr/util/VersionedFile.java @@ -16,19 +16,18 @@ */ package org.apache.solr.util; -import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FilenameFilter; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Stream; /** * @since solr 1.3 @@ -42,9 +41,9 @@ public class VersionedFile { public static InputStream getLatestFile(String dirName, String fileName) throws FileNotFoundException { // TODO SOLR-8282 move to PATH - Collection oldFiles = null; + Collection oldFiles = null; final String prefix = fileName + '.'; - File f = new File(dirName, fileName); + Path f = Path.of(dirName, fileName); InputStream is = null; // there can be a race between checking for a file and opening it... @@ -52,25 +51,25 @@ public static InputStream getLatestFile(String dirName, String fileName) // try multiple times in a row. for (int retry = 0; retry < 10 && is == null; retry++) { try { - if (!f.exists()) { - File dir = new File(dirName); - String[] names = - dir.list( - new FilenameFilter() { - @Override - public boolean accept(File dir, String name) { - return name.startsWith(prefix); - } - }); - Arrays.sort(names); - f = new File(dir, names[names.length - 1]); + if (!Files.exists(f)) { + Path dir = Path.of(dirName); + List fileList; + + try (Stream files = Files.list(dir)) { + fileList = + files.filter((file) -> file.getFileName().startsWith(prefix)).sorted().toList(); + } catch (IOException e) { + throw new RuntimeException(e); + } + + f = Path.of(dir.toString(), fileList.getLast().toString()); oldFiles = new ArrayList<>(); - for (int i = 0; i < names.length - 1; i++) { - oldFiles.add(new File(dir, names[i])); + for (int i = 0; i < fileList.size() - 1; i++) { + oldFiles.add(Path.of(dir.toString(), fileList.get(i).toString())); } } - is = new FileInputStream(f); + is = new FileInputStream(f.toString()); } catch (Exception e) { // swallow exception for now } @@ -78,7 +77,7 @@ public boolean accept(File dir, String name) { // allow exception to be thrown from the final try. if (is == null) { - is = new FileInputStream(f); + is = new FileInputStream(f.toString()); } // delete old files only after we have successfully opened the newest @@ -89,16 +88,16 @@ public boolean accept(File dir, String name) { return is; } - private static final Set deleteList = new HashSet<>(); + private static final Set deleteList = new HashSet<>(); - private static synchronized void delete(Collection files) { + private static synchronized void delete(Collection files) { synchronized (deleteList) { deleteList.addAll(files); - List deleted = new ArrayList<>(); - for (File df : deleteList) { + List deleted = new ArrayList<>(); + for (Path df : deleteList) { try { try { - Files.deleteIfExists(df.toPath()); + Files.deleteIfExists(df); } catch (IOException cause) { // TODO: should this class care if a file couldn't be deleted? // this just emulates previous behavior, where only SecurityException would be handled. @@ -106,7 +105,7 @@ private static synchronized void delete(Collection files) { // deleteList.remove(df); deleted.add(df); } catch (SecurityException e) { - if (!df.exists()) { + if (!Files.exists(df)) { deleted.add(df); } } From bc2db006adc7222acffabb5e72e5080ae7a0a394 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Sun, 22 Dec 2024 21:16:00 -0500 Subject: [PATCH 02/10] Move initialize metrics until after data dir was created --- .../apache/solr/client/api/endpoint/NodeFileStoreApis.java | 4 +++- solr/core/src/java/org/apache/solr/core/CoreDescriptor.java | 5 ++--- solr/core/src/java/org/apache/solr/core/SolrCore.java | 6 +++--- .../src/java/org/apache/solr/filestore/NodeFileStore.java | 3 ++- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java b/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java index 15f4a73cfb1..ff3388cf087 100644 --- a/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java +++ b/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java @@ -26,6 +26,7 @@ import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.QueryParam; +import java.io.IOException; import org.apache.solr.client.api.model.SolrJerseyResponse; /** @@ -63,5 +64,6 @@ SolrJerseyResponse getFile( String getFrom, @Parameter(description = "Indicates that (only) file metadata should be fetched") @QueryParam("meta") - Boolean meta); + Boolean meta) + throws IOException; } diff --git a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java index 436380af7a5..20f7d4d5c58 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java +++ b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java @@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -64,7 +63,7 @@ public class CoreDescriptor { public static final String SOLR_CORE_PROP_PREFIX = "solr.core."; public static final String DEFAULT_EXTERNAL_PROPERTIES_FILE = - Paths.get("conf", "solrcore.properties").toString(); + Path.of("conf/solrcore.properties").toString(); /** * Whether this core was configured using a configSet that was trusted. This helps in avoiding the @@ -96,7 +95,7 @@ public Properties getPersistableUserProperties() { CORE_CONFIG, "solrconfig.xml", CORE_SCHEMA, "schema.xml", CORE_CONFIGSET_PROPERTIES, ConfigSetProperties.DEFAULT_FILENAME, - CORE_DATADIR, Paths.get("data/").toString(), + CORE_DATADIR, Path.of("data/").toString(), CORE_TRANSIENT, "false", CORE_LOADONSTARTUP, "true"); diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index ec3e291e636..51c296a7ccf 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1104,9 +1104,6 @@ protected SolrCore( checkVersionFieldExistsInSchema(schema, coreDescriptor); setLatestSchema(schema); - // initialize core metrics - initializeMetrics(solrMetricsContext, null); - // init pluggable circuit breakers, after metrics because some circuit breakers use metrics initPlugins(null, CircuitBreaker.class); @@ -1124,6 +1121,9 @@ protected SolrCore( this.snapshotMgr = initSnapshotMetaDataManager(); this.solrDelPolicy = initDeletionPolicy(delPolicy); + // initialize core metrics + initializeMetrics(solrMetricsContext, null); + this.codec = initCodec(solrConfig, this.schema); initIndex(prev != null, reload); diff --git a/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java b/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java index f1b2a5843d6..a38ecbc6b0b 100644 --- a/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java @@ -72,7 +72,8 @@ public NodeFileStore( // it up into multiple distinct APIs @Override @PermissionName(FILESTORE_READ_PERM) - public SolrJerseyResponse getFile(String path, Boolean sync, String getFrom, Boolean meta) throws IOException { + public SolrJerseyResponse getFile(String path, Boolean sync, String getFrom, Boolean meta) + throws IOException { final var response = instantiateJerseyResponse(SolrJerseyResponse.class); if (Boolean.TRUE.equals(sync)) { From 4c01109ce4a0286ad03fb2e3ae5ea111dfbd7321 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Sun, 22 Dec 2024 21:20:12 -0500 Subject: [PATCH 03/10] Compile failure --- .../apache/solr/util/RegexFileFilterTest.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java b/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java index 8e69018c941..f69b7e5faa0 100644 --- a/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java +++ b/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java @@ -17,6 +17,8 @@ package org.apache.solr.util; import java.io.File; +import java.nio.file.Path; + import org.apache.solr.SolrTestCase; import org.junit.Test; @@ -27,23 +29,23 @@ public class RegexFileFilterTest extends SolrTestCase { @Test public void testAcceptTrue() { - assertTrue(endsWithDotTxt.accept(new File("/foo/bar/baz.txt"))); - assertTrue(endsWithDotTxt.accept(new File("/baz.txt"))); - assertTrue(endsWithDotTxt.accept(new File("~/baz.txt"))); - assertTrue(endsWithDotTxt.accept(new File("~/1234-abc_.txt"))); - assertTrue(endsWithDotTxt.accept(new File(".txt"))); - assertTrue(alphaWithTxtOrPdfExt.accept(new File("/foo/bar.txt"))); - assertTrue(alphaWithTxtOrPdfExt.accept(new File("/foo/baz.pdf"))); + assertTrue(endsWithDotTxt.accept(Path.of("/foo/bar/baz.txt"))); + assertTrue(endsWithDotTxt.accept(Path.of("/baz.txt"))); + assertTrue(endsWithDotTxt.accept(Path.of("~/baz.txt"))); + assertTrue(endsWithDotTxt.accept(Path.of("~/1234-abc_.txt"))); + assertTrue(endsWithDotTxt.accept(Path.of(".txt"))); + assertTrue(alphaWithTxtOrPdfExt.accept(Path.of("/foo/bar.txt"))); + assertTrue(alphaWithTxtOrPdfExt.accept(Path.of("/foo/baz.pdf"))); } @Test public void testAcceptFalse() { - assertFalse(endsWithDotTxt.accept(new File("/foo/bar.tx"))); - assertFalse(endsWithDotTxt.accept(new File("/foo/bar.txts"))); - assertFalse(endsWithDotTxt.accept(new File("/foo/bar.exe"))); - assertFalse(alphaWithTxtOrPdfExt.accept(new File("/foo/bar/b4z.txt"))); - assertFalse(alphaWithTxtOrPdfExt.accept(new File("/foo/bar/baz.jpg"))); - assertFalse(alphaWithTxtOrPdfExt.accept(new File("~/foo-bar.txt"))); - assertFalse(alphaWithTxtOrPdfExt.accept(new File("~/foobar123.txt"))); + assertFalse(endsWithDotTxt.accept(Path.of("/foo/bar.tx"))); + assertFalse(endsWithDotTxt.accept(Path.of("/foo/bar.txts"))); + assertFalse(endsWithDotTxt.accept(Path.of("/foo/bar.exe"))); + assertFalse(alphaWithTxtOrPdfExt.accept(Path.of("/foo/bar/b4z.txt"))); + assertFalse(alphaWithTxtOrPdfExt.accept(Path.of("/foo/bar/baz.jpg"))); + assertFalse(alphaWithTxtOrPdfExt.accept(Path.of("~/foo-bar.txt"))); + assertFalse(alphaWithTxtOrPdfExt.accept(Path.of("./~/foobar123.txt"))); } } From f17b8096be11e0da03fc98ba3ec29ec93d26e4a8 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Sun, 22 Dec 2024 23:44:02 -0500 Subject: [PATCH 04/10] Fix to bad conversions --- .../src/java/org/apache/solr/core/SolrCore.java | 17 ++++++++++------- .../apache/solr/filestore/DistribFileStore.java | 5 +++++ .../handler/admin/ShowFileRequestHandler.java | 4 +--- .../org/apache/solr/util/VersionedFile.java | 5 +++-- .../apache/solr/util/RegexFileFilterTest.java | 2 -- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 51c296a7ccf..cc662bb46fd 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1362,15 +1362,18 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) { try { totalSpace = Files.getFileStore(dataDirFile).getTotalSpace(); useableSpace = Files.getFileStore(dataDirFile).getUsableSpace(); - } catch (IOException e) { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Metrics initialization failed to retrieve files total/usable space", - e); + } catch (Exception e) { + // TODO Metrics used to default to 0 with java.io.File even if directory didn't exist + // Should throw an exception and initialize data directory before metrics + totalSpace = 0L; + useableSpace = 0L; } - parentContext.gauge(() -> totalSpace, true, "totalSpace", Category.CORE.toString(), "fs"); - parentContext.gauge(() -> useableSpace, true, "usableSpace", Category.CORE.toString(), "fs"); + final long finalTotalSpace = totalSpace; + final long finalUsableSpace = useableSpace; + parentContext.gauge(() -> finalTotalSpace, true, "totalSpace", Category.CORE.toString(), "fs"); + parentContext.gauge( + () -> finalUsableSpace, true, "usableSpace", Category.CORE.toString(), "fs"); parentContext.gauge( () -> dataDirFile.toAbsolutePath().toString(), diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index 41d93dd1196..a3dfc5dd7ce 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -87,7 +87,12 @@ public Path getRealpath(String path) { private static Path _getRealPath(String path, Path solrHome) { Path dir = Path.of(path); SolrPaths.assertNotUnc(dir); + + while (path.startsWith("/")) { + path = path.substring(1); + } var finalPath = getFileStoreDirPath(solrHome).resolve(path); + // Guard against path traversal by asserting final path is sub path of filestore if (!finalPath.normalize().startsWith(getFileStoreDirPath(solrHome).normalize())) { throw new SolrException(BAD_REQUEST, "Illegal path " + path); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java index 6b99f2ff924..a0b8f020818 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java @@ -231,13 +231,11 @@ private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) thr // Show a directory listing if (Files.isDirectory(adminFile)) { // it's really a directory, just go for it. - int basePath = (int) (Files.size(adminFile.toAbsolutePath()) + 1); NamedList> files = new SimpleOrderedMap<>(); try (Stream directoryFiles = Files.list(adminFile)) { directoryFiles.forEach( (f) -> { - String path = f.toAbsolutePath().toString().substring(basePath); - path = path.replace('\\', '/'); // normalize slashes + String path = f.getFileName().toString(); if (isHiddenFile( req, rsp, f.getFileName().toString().replace('\\', '/'), false, hiddenFiles)) { diff --git a/solr/core/src/java/org/apache/solr/util/VersionedFile.java b/solr/core/src/java/org/apache/solr/util/VersionedFile.java index 972a5647043..ede2753afe7 100644 --- a/solr/core/src/java/org/apache/solr/util/VersionedFile.java +++ b/solr/core/src/java/org/apache/solr/util/VersionedFile.java @@ -27,6 +27,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; /** @@ -57,12 +58,12 @@ public static InputStream getLatestFile(String dirName, String fileName) try (Stream files = Files.list(dir)) { fileList = - files.filter((file) -> file.getFileName().startsWith(prefix)).sorted().toList(); + files.filter((file) -> file.getFileName().toString().startsWith(prefix)).toList(); } catch (IOException e) { throw new RuntimeException(e); } - f = Path.of(dir.toString(), fileList.getLast().toString()); + f = Path.of(dir.toString(), fileList.getLast().getFileName().toString()); oldFiles = new ArrayList<>(); for (int i = 0; i < fileList.size() - 1; i++) { oldFiles.add(Path.of(dir.toString(), fileList.get(i).toString())); diff --git a/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java b/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java index f69b7e5faa0..ae90deb90ed 100644 --- a/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java +++ b/solr/core/src/test/org/apache/solr/util/RegexFileFilterTest.java @@ -16,9 +16,7 @@ */ package org.apache.solr.util; -import java.io.File; import java.nio.file.Path; - import org.apache.solr.SolrTestCase; import org.junit.Test; From 36d0b928baa292d4d06473378017ab1addb07c9d Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 23 Dec 2024 00:06:48 -0500 Subject: [PATCH 05/10] Sort list in getLatestFile --- solr/core/src/java/org/apache/solr/util/VersionedFile.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/util/VersionedFile.java b/solr/core/src/java/org/apache/solr/util/VersionedFile.java index ede2753afe7..62b04e69fff 100644 --- a/solr/core/src/java/org/apache/solr/util/VersionedFile.java +++ b/solr/core/src/java/org/apache/solr/util/VersionedFile.java @@ -27,7 +27,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; /** @@ -58,7 +57,10 @@ public static InputStream getLatestFile(String dirName, String fileName) try (Stream files = Files.list(dir)) { fileList = - files.filter((file) -> file.getFileName().toString().startsWith(prefix)).toList(); + files + .filter((file) -> file.getFileName().toString().startsWith(prefix)) + .sorted() + .toList(); } catch (IOException e) { throw new RuntimeException(e); } From 374a4a10a3aee18682bf03792a4f7b2bd2c3a975 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 23 Dec 2024 23:11:53 -0500 Subject: [PATCH 06/10] Refactor --- .../apache/solr/core/DirectoryFactory.java | 92 +++++++++---------- .../solr/filestore/DistribFileStore.java | 5 +- .../handler/admin/ShowFileRequestHandler.java | 3 +- .../org/apache/solr/util/VersionedFile.java | 5 +- 4 files changed, 52 insertions(+), 53 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java index c93c7a366d6..5147c8cbfc8 100644 --- a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java @@ -20,13 +20,11 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.util.Comparator; import java.util.List; -import java.util.stream.StreamSupport; +import java.util.stream.Stream; import org.apache.commons.io.file.PathUtils; import org.apache.lucene.store.Directory; import org.apache.lucene.store.FilterDirectory; @@ -340,7 +338,6 @@ public void cleanupOldIndexDirectories( final String dataDirPath, final String currentIndexDirPath, boolean afterCoreReload) throws IOException { - // TODO SOLR-8282 move to PATH Path dataDirFile = Path.of(dataDirPath); if (!Files.isDirectory(dataDirFile)) { log.debug( @@ -350,55 +347,54 @@ public void cleanupOldIndexDirectories( } final Path currentIndexDir = Path.of(currentIndexDirPath); - try (DirectoryStream oldIndexDirs = - Files.newDirectoryStream( - dataDirFile, - file -> { - String fileName = file.getFileName().toString(); - return Files.isDirectory(file) - && !file.equals(currentIndexDir) - && (fileName.equals("index") || fileName.matches(INDEX_W_TIMESTAMP_REGEX)); - })) { - - List dirsList = - StreamSupport.stream(oldIndexDirs.spliterator(), false) - .sorted(Comparator.comparing(Path::toString)) + List dirsList; + try (Stream 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() .toList(); + } + ; - if (dirsList.isEmpty()) { - return; // nothing to do (no log message needed) - } + if (dirsList.isEmpty()) { + return; // nothing to do (no log message needed) + } - int i = 0; - if (afterCoreReload) { - log.info("Will not remove most recent old directory after reload {}", dirsList.get(0)); - i = 1; - } + int i = 0; + if (afterCoreReload) { + 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={}", - 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); + 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/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index a3dfc5dd7ce..d92c1977b13 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -627,7 +627,10 @@ private static Map _getKeys(Path solrHome) throws IOException { try { result.put(keyFile.getFileName().toString(), Files.readAllBytes(keyFile)); } catch (IOException e) { - throw new RuntimeException(e); + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, + "Unable to read all bytes file: " + keyFile.getFileName(), + e); } } }); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java index a0b8f020818..baa30790a37 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java @@ -237,8 +237,7 @@ private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) thr (f) -> { String path = f.getFileName().toString(); - if (isHiddenFile( - req, rsp, f.getFileName().toString().replace('\\', '/'), false, hiddenFiles)) { + if (isHiddenFile(req, rsp, f.getFileName().toString(), false, hiddenFiles)) { return; } diff --git a/solr/core/src/java/org/apache/solr/util/VersionedFile.java b/solr/core/src/java/org/apache/solr/util/VersionedFile.java index 62b04e69fff..f3c1bb27ddf 100644 --- a/solr/core/src/java/org/apache/solr/util/VersionedFile.java +++ b/solr/core/src/java/org/apache/solr/util/VersionedFile.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Set; import java.util.stream.Stream; +import org.apache.solr.common.SolrException; /** * @since solr 1.3 @@ -40,7 +41,6 @@ public class VersionedFile { */ public static InputStream getLatestFile(String dirName, String fileName) throws FileNotFoundException { - // TODO SOLR-8282 move to PATH Collection oldFiles = null; final String prefix = fileName + '.'; Path f = Path.of(dirName, fileName); @@ -62,7 +62,8 @@ public static InputStream getLatestFile(String dirName, String fileName) .sorted() .toList(); } catch (IOException e) { - throw new RuntimeException(e); + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, "Unable to list files in " + dir, e); } f = Path.of(dir.toString(), fileList.getLast().getFileName().toString()); From 63fb99df349fbd0c9ae627024352dff3ea00a1bc Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 16 Jan 2025 12:51:07 -0500 Subject: [PATCH 07/10] First round addressing PR comments --- .../api/endpoint/NodeFileStoreApis.java | 4 +-- .../solr/cloud/ZkSolrResourceLoader.java | 1 + .../org/apache/solr/core/CoreDescriptor.java | 5 ++-- .../java/org/apache/solr/core/SolrCore.java | 30 ++++++------------- .../java/org/apache/solr/core/SolrPaths.java | 7 +++-- .../apache/solr/core/SolrResourceLoader.java | 23 +++++++------- .../solr/filestore/DistribFileStore.java | 26 +++++++++------- .../org/apache/solr/filestore/FileStore.java | 4 +-- .../apache/solr/filestore/NodeFileStore.java | 3 +- .../org/apache/solr/handler/IndexFetcher.java | 16 +++++----- .../handler/admin/CoreAdminOperation.java | 2 ++ .../handler/admin/ShowFileRequestHandler.java | 14 ++++----- .../apache/solr/pkg/SolrPackageLoader.java | 2 +- .../spelling/AbstractLuceneSpellChecker.java | 15 ++++++---- .../fst/AnalyzingInfixLookupFactory.java | 12 ++++---- .../org/apache/solr/util/VersionedFile.java | 1 + .../apache/solr/core/ResourceLoaderTest.java | 10 +++---- 17 files changed, 85 insertions(+), 90 deletions(-) diff --git a/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java b/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java index ff3388cf087..15f4a73cfb1 100644 --- a/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java +++ b/solr/api/src/java/org/apache/solr/client/api/endpoint/NodeFileStoreApis.java @@ -26,7 +26,6 @@ import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.QueryParam; -import java.io.IOException; import org.apache.solr.client.api.model.SolrJerseyResponse; /** @@ -64,6 +63,5 @@ SolrJerseyResponse getFile( String getFrom, @Parameter(description = "Indicates that (only) file metadata should be fetched") @QueryParam("meta") - Boolean meta) - throws IOException; + Boolean meta); } diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java index 5e0ac07484c..dcec5bd4ab3 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java @@ -120,6 +120,7 @@ public InputStream openResource(String resource) throws IOException { try { // delegate to the class loader (looking into $INSTANCE_DIR/lib jars) + // Use Path to normalize path separator is = classLoader.getResourceAsStream(Path.of(resource).toString()); } catch (Exception e) { throw new IOException("Error opening " + resource, e); diff --git a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java index 20f7d4d5c58..3fd83e60ff8 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java +++ b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java @@ -62,8 +62,7 @@ public class CoreDescriptor { public static final String CORE_CONFIGSET_PROPERTIES = "configSetProperties"; public static final String SOLR_CORE_PROP_PREFIX = "solr.core."; - public static final String DEFAULT_EXTERNAL_PROPERTIES_FILE = - Path.of("conf/solrcore.properties").toString(); + public static final Path DEFAULT_EXTERNAL_PROPERTIES_FILE = Path.of("conf/solrcore.properties"); /** * Whether this core was configured using a configSet that was trusted. This helps in avoiding the @@ -95,7 +94,7 @@ public Properties getPersistableUserProperties() { CORE_CONFIG, "solrconfig.xml", CORE_SCHEMA, "schema.xml", CORE_CONFIGSET_PROPERTIES, ConfigSetProperties.DEFAULT_FILENAME, - CORE_DATADIR, Path.of("data/").toString(), + CORE_DATADIR, Path.of("data").toString(), CORE_TRANSIENT, "false", CORE_LOADONSTARTUP, "true"); diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index cc662bb46fd..973ce0a30e8 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -32,7 +32,6 @@ 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; @@ -1104,6 +1103,9 @@ protected SolrCore( checkVersionFieldExistsInSchema(schema, coreDescriptor); setLatestSchema(schema); + // initialize core metrics + initializeMetrics(solrMetricsContext, null); + // init pluggable circuit breakers, after metrics because some circuit breakers use metrics initPlugins(null, CircuitBreaker.class); @@ -1121,9 +1123,6 @@ protected SolrCore( this.snapshotMgr = initSnapshotMetaDataManager(); this.solrDelPolicy = initDeletionPolicy(delPolicy); - // initialize core metrics - initializeMetrics(solrMetricsContext, null); - this.codec = initCodec(solrConfig, this.schema); initIndex(prev != null, reload); @@ -1356,24 +1355,13 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) { // initialize disk total / free metrics Path dataDirFile = Paths.get(dataDir); - long totalSpace; - long useableSpace; - try { - totalSpace = Files.getFileStore(dataDirFile).getTotalSpace(); - useableSpace = Files.getFileStore(dataDirFile).getUsableSpace(); - } catch (Exception e) { - // TODO Metrics used to default to 0 with java.io.File even if directory didn't exist - // Should throw an exception and initialize data directory before metrics - totalSpace = 0L; - useableSpace = 0L; - } - - final long finalTotalSpace = totalSpace; - final long finalUsableSpace = useableSpace; - parentContext.gauge(() -> finalTotalSpace, true, "totalSpace", Category.CORE.toString(), "fs"); - parentContext.gauge( - () -> finalUsableSpace, true, "usableSpace", Category.CORE.toString(), "fs"); + // Do not pre-compute the data directory total/usable space on initialization. Calculated when + // metric is fetched + // TODO Move metrics initialization calls to after the data directory is created to fetch true + // directory space on initialization + parentContext.gauge(() -> 0L, true, "totalSpace", Category.CORE.toString(), "fs"); + parentContext.gauge(() -> 0L, true, "usableSpace", Category.CORE.toString(), "fs"); parentContext.gauge( () -> dataDirFile.toAbsolutePath().toString(), diff --git a/solr/core/src/java/org/apache/solr/core/SolrPaths.java b/solr/core/src/java/org/apache/solr/core/SolrPaths.java index e938c0d83cb..dad7a374872 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrPaths.java +++ b/solr/core/src/java/org/apache/solr/core/SolrPaths.java @@ -18,6 +18,7 @@ package org.apache.solr.core; import java.lang.invoke.MethodHandles; +import java.nio.file.FileSystems; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; @@ -42,7 +43,9 @@ 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 + "/" : path; + return (path != null && (!(path.endsWith("/") || path.endsWith("\\")))) + ? path + FileSystems.getDefault().getSeparator() + : path; } /** @@ -91,7 +94,7 @@ public static void assertNoPathTraversal(Path pathToAssert) { /** Asserts that a path is not a Windows UNC path */ public static void assertNotUnc(Path pathToAssert) { - if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) { + if (OS.isFamilyWindows() || pathToAssert.toString().startsWith("\\\\")) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Path " + pathToAssert + " disallowed. UNC paths not supported."); diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java index f777f7a4fd2..7e956ee9231 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java @@ -30,10 +30,10 @@ 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; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -353,14 +353,11 @@ public ClassLoader getClassLoader() { */ @Override public InputStream openResource(String resource) throws IOException { - String pathResource = Paths.get(resource).normalize().toString(); - if (pathResource.trim().startsWith("\\\\")) { // Always disallow UNC paths - throw new SolrResourceNotFoundException( - "Resource '" + pathResource + "' could not be loaded."); - } + SolrPaths.assertNotUnc(Path.of(resource)); // Always disallow UNC paths + Path instanceDir = getInstancePath().normalize(); - Path inInstanceDir = getInstancePath().resolve(pathResource).normalize(); - Path inConfigDir = instanceDir.resolve("conf").resolve(pathResource).normalize(); + Path inInstanceDir = getInstancePath().resolve(resource).normalize(); + Path inConfigDir = instanceDir.resolve("conf").resolve(resource).normalize(); if (allowUnsafeResourceloading || inInstanceDir.startsWith(instanceDir)) { // The resource is either inside instance dir or we allow unsafe loading, so allow testing if // file exists @@ -375,17 +372,17 @@ 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(pathResource); + InputStream is = classLoader.getResourceAsStream(resource); // 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(Path.of("conf").resolve(pathResource).toString()); + is = classLoader.getResourceAsStream(Path.of("conf").resolve(resource).toString()); } if (is == null) { throw new SolrResourceNotFoundException( - "Can't find resource '" + pathResource + "' in classpath or '" + instanceDir + "'"); + "Can't find resource '" + resource + "' in classpath or '" + instanceDir + "'"); } return is; } @@ -406,7 +403,9 @@ public String resourceLocation(String resource) { return inInstanceDir.normalize().toString(); } - try (InputStream is = classLoader.getResourceAsStream(Path.of(resource).toString())) { + try (InputStream is = + 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/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index d92c1977b13..bac0da2a47b 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -80,7 +80,7 @@ public DistribFileStore(CoreContainer coreContainer) { } @Override - public Path getRealpath(String path) { + public Path getRealPath(String path) { return _getRealPath(path, solrHome); } @@ -88,6 +88,7 @@ private static Path _getRealPath(String path, Path solrHome) { Path dir = Path.of(path); SolrPaths.assertNotUnc(dir); + // Strip the path of from being absolute to become relative to resolve with SolrHome while (path.startsWith("/")) { path = path.substring(1); } @@ -111,7 +112,7 @@ class FileInfo { ByteBuffer getFileData(boolean validate) throws IOException { if (fileData == null) { - fileData = ByteBuffer.wrap(Files.readAllBytes(getRealpath(path))); + fileData = ByteBuffer.wrap(Files.readAllBytes(getRealPath(path))); } return fileData; } @@ -136,7 +137,7 @@ private void persistToFile(ByteBuffer data, ByteBuffer meta) throws IOException } public boolean exists(boolean validateContent, boolean fetchMissing) throws IOException { - Path file = getRealpath(path); + Path file = getRealPath(path); if (!Files.exists(file)) { if (fetchMissing) { return fetchFromAnyNode(); @@ -166,7 +167,7 @@ public boolean exists(boolean validateContent, boolean fetchMissing) throws IOEx private void deleteFile() { try { - IOUtils.deleteFilesIfExist(getRealpath(path), getRealpath(getMetaPath())); + IOUtils.deleteFilesIfExist(getRealPath(path), getRealPath(getMetaPath())); } catch (IOException e) { log.error("Unable to delete files: {}", path); } @@ -245,12 +246,12 @@ boolean fetchFromAnyNode() { } public Path realPath() { - return getRealpath(path); + return getRealPath(path); } @SuppressWarnings("unchecked") MetaData readMetaData() throws IOException { - Path file = getRealpath(getMetaPath()); + Path file = getRealPath(getMetaPath()); if (Files.exists(file)) { try (InputStream fis = new FileInputStream(file.toString())) { return new MetaData((Map) Utils.fromJSON(fis)); @@ -428,9 +429,10 @@ public boolean fetch(String path, String from) { @Override public void get(String path, Consumer consumer, boolean fetchmissing) throws IOException { - Path file = getRealpath(path); + Path file = getRealPath(path); String simpleName = file.getFileName().toString(); if (isMetaDataFile(simpleName)) { + // TODO SOLR-8282 Move to Files.newInputStream try (InputStream is = new FileInputStream(file.toString())) { consumer.accept( new FileEntry(null, null, path) { @@ -458,8 +460,8 @@ public void syncToAllNodes(String path) throws IOException { } @Override - public List list(String path, Predicate predicate) throws IOException { - Path file = getRealpath(path); + public List list(String path, Predicate predicate) { + Path file = getRealPath(path); List fileDetails = new ArrayList<>(); FileType type = getType(path, false); if (type == FileType.DIRECTORY) { @@ -473,6 +475,8 @@ public List list(String path, Predicate predicate) throws I } } }); + } catch (IOException e) { + throw new SolrException(SERVER_ERROR, "Error listing files from provided path " + path, e); } } else if (type == FileType.FILE) { fileDetails.add(new FileInfo(path).getDetails()); @@ -550,10 +554,10 @@ public void refresh(String path) { @Override public FileType getType(String path, boolean fetchMissing) { - Path file = getRealpath(path); + Path file = getRealPath(path); if (!Files.exists(file) && fetchMissing) { if (fetch(path, null)) { - file = getRealpath(path); + file = getRealPath(path); } } return _getFileType(file); diff --git a/solr/core/src/java/org/apache/solr/filestore/FileStore.java b/solr/core/src/java/org/apache/solr/filestore/FileStore.java index cc5d1d59365..acdf979496d 100644 --- a/solr/core/src/java/org/apache/solr/filestore/FileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/FileStore.java @@ -44,13 +44,13 @@ public interface FileStore { /** Fetch a resource from another node internal API */ boolean fetch(String path, String from); - List list(String path, Predicate predicate) throws IOException; + List list(String path, Predicate predicate); /** Sync a local file to all nodes. All the nodes are asked to pull the file from this node */ void syncToAllNodes(String path) throws IOException; /** get the real path on filesystem */ - Path getRealpath(String path); + Path getRealPath(String path); /** The type of the resource */ FileType getType(String path, boolean fetchMissing); diff --git a/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java b/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java index a38ecbc6b0b..e6c61419437 100644 --- a/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java @@ -72,8 +72,7 @@ public NodeFileStore( // it up into multiple distinct APIs @Override @PermissionName(FILESTORE_READ_PERM) - public SolrJerseyResponse getFile(String path, Boolean sync, String getFrom, Boolean meta) - throws IOException { + public SolrJerseyResponse getFile(String path, Boolean sync, String getFrom, Boolean meta) { final var response = instantiateJerseyResponse(SolrJerseyResponse.class); if (Boolean.TRUE.equals(sync)) { diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 19010584575..5c5e5017c0c 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1063,20 +1063,20 @@ private void downloadConfFiles( List> confFilesToDownload, long latestGeneration) { log.info("Starting download of configuration files from leader: {}", confFilesToDownload); confFilesDownloaded = Collections.synchronizedList(new ArrayList<>()); - Path tmpconfDir = + Path tmpConfDir = solrCore.getResourceLoader().getConfigPath().resolve("conf." + getDateAsStr(new Date())); try { try { - Files.createDirectories(tmpconfDir); + Files.createDirectories(tmpConfDir); } catch (Exception e) { throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, - "Failed to create temporary config folder: " + tmpconfDir.getFileName()); + "Failed to create temporary config folder: " + tmpConfDir.getFileName()); } for (Map file : confFilesToDownload) { String saveAs = (String) (file.get(ALIAS) == null ? file.get(NAME) : file.get(ALIAS)); localFileFetcher = - new LocalFsFileFetcher(tmpconfDir, file, saveAs, CONF_FILE_SHORT, latestGeneration); + new LocalFsFileFetcher(tmpConfDir, file, saveAs, CONF_FILE_SHORT, latestGeneration); currentFile = file; localFileFetcher.fetchFile(); confFilesDownloaded.add(new HashMap<>(file)); @@ -1084,13 +1084,13 @@ private void downloadConfFiles( // this is called before copying the files to the original conf dir // so that if there is an exception avoid corrupting the original files. terminateAndWaitFsyncService(); - copyTmpConfFiles2Conf(tmpconfDir); + copyTmpConfFiles2Conf(tmpConfDir); } catch (Exception e) { throw new SolrException( ErrorCode.SERVER_ERROR, "Failed to download configuration files from leader", e); } finally { - delTree(tmpconfDir); + delTree(tmpConfDir); } } @@ -1498,9 +1498,7 @@ private void copyTmpConfFiles2Conf(Path tmpconfDir) { oldPath + "." + getDateAsStr(new Date(Files.getLastModifiedTime(oldPath).toMillis()))); - if (!Files.exists(backupFile.getParent())) { - Files.createDirectories(backupFile.getParent()); - } + Files.createDirectories(backupFile.getParent()); Files.move(oldPath, backupFile); } catch (Exception e) { throw new SolrException( diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java index 7cfcb25db1b..47876c6f5e4 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java @@ -356,7 +356,9 @@ public static NamedList getCoreStatus( if (core != null) { info.add(NAME, core.getName()); info.add("instanceDir", core.getInstancePath().toString()); + info.add("dataDir", Path.of(core.getDataDir()).toString()); + info.add("config", core.getConfigResource()); info.add("schema", core.getSchemaResource()); info.add("startTime", core.getStartTimeStamp()); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java index baa30790a37..340107e762f 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java @@ -196,12 +196,11 @@ private void showFromZooKeeper( // Return the file indicated (or the directory listing) from the local file system. private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException { - Path admin = getAdminFileFromFileSystem(req, rsp, hiddenFiles); + Path adminFile = getAdminFileFromFileSystem(req, rsp, hiddenFiles); - if (admin == null) { // exception already recorded + if (adminFile == null) { // exception already recorded return; } - Path adminFile = admin; // Make sure the file exists, is readable and is not a hidden file if (!Files.exists(adminFile)) { log.error("Can not find: {} [{}]", adminFile.getFileName(), adminFile.toAbsolutePath()); @@ -235,18 +234,15 @@ private void showFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) thr try (Stream directoryFiles = Files.list(adminFile)) { directoryFiles.forEach( (f) -> { - String path = f.getFileName().toString(); - if (isHiddenFile(req, rsp, f.getFileName().toString(), false, hiddenFiles)) { return; } - + String path = f.getFileName().toString(); SimpleOrderedMap fileInfo = new SimpleOrderedMap<>(); files.add(path, fileInfo); if (Files.isDirectory(f)) { fileInfo.add("directory", true); } else { - // TODO? content type try { fileInfo.add("size", Files.size(f)); fileInfo.add("modified", new Date(Files.getLastModifiedTime(f).toMillis())); @@ -414,6 +410,10 @@ public final Set getHiddenFiles() { return hiddenFiles; } + public static String toForwardSlashPathString(String path) { + return path.replace('\\', '/'); + } + //////////////////////// SolrInfoMBeans methods ////////////////////// @Override diff --git a/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java b/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java index 0d5294f1ae6..5e25d5f6fce 100644 --- a/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java +++ b/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java @@ -279,7 +279,7 @@ public void writeMap(EntryWriter ew) throws IOException { throw new RuntimeException("Cannot load package: " + errs); } for (String file : version.files) { - paths.add(coreContainer.getFileStore().getRealpath(file)); + paths.add(coreContainer.getFileStore().getRealPath(file)); } loader = diff --git a/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java b/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java index 3ea62098abc..30eec4e61f8 100644 --- a/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java +++ b/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java @@ -70,7 +70,7 @@ public abstract class AbstractLuceneSpellChecker extends SolrSpellChecker { protected Dictionary dictionary; public static final int DEFAULT_SUGGESTION_COUNT = 5; - protected String indexDir; + protected Path indexDir; protected float accuracy = 0.5f; public static final String FIELD = "field"; @@ -79,13 +79,16 @@ public abstract class AbstractLuceneSpellChecker extends SolrSpellChecker { @Override public String init(NamedList config, SolrCore core) { super.init(config, core); - indexDir = (String) config.get(INDEX_DIR); + String indexPath = (String) config.get(INDEX_DIR); + indexDir = (indexPath != null) ? Path.of(indexPath) : null; + // If indexDir is relative then create index inside core.getDataDir() if (indexDir != null) { - if (!Path.of(indexDir).isAbsolute()) { - indexDir = Path.of(core.getDataDir(), indexDir).toString(); + if (!indexDir.isAbsolute()) { + indexDir = Path.of(core.getDataDir(), indexDir.toString()); } } + sourceLocation = (String) config.get(LOCATION); String compClass = (String) config.get(COMPARATOR_CLASS); Comparator comp = null; @@ -234,7 +237,7 @@ protected void initIndex() throws IOException { // files can't be opened. It would be better for SpellChecker to hold a single IW instance, // and close it on close, but Solr never seems to close its spell checkers. Wrapping as // FilterDirectory prevents IndexWriter from catching the pending deletions: - index = new FilterDirectory(FSDirectory.open(Path.of(indexDir))) {}; + index = new FilterDirectory(FSDirectory.open(indexDir)) {}; } else { index = new ByteBuffersDirectory(); } @@ -267,7 +270,7 @@ public String getFieldTypeName() { /* * @return the Index directory * */ - public String getIndexDir() { + public Path getIndexDir() { return indexDir; } diff --git a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java index 5b8115054b9..865f818ac80 100644 --- a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java +++ b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java @@ -85,10 +85,12 @@ public Lookup create(NamedList params, SolrCore core) { // optional parameters - String indexPath = - params.get(INDEX_PATH) != null ? params.get(INDEX_PATH).toString() : DEFAULT_INDEX_PATH; - if (!Path.of(indexPath).isAbsolute()) { - indexPath = Path.of(core.getDataDir(), indexPath).toString(); + Path indexPath = + params.get(INDEX_PATH) != null + ? Path.of(params.get(INDEX_PATH).toString()) + : Path.of(DEFAULT_INDEX_PATH); + if (!indexPath.isAbsolute()) { + indexPath = Path.of(core.getDataDir(), indexPath.toString()); } int minPrefixChars = @@ -108,7 +110,7 @@ public Lookup create(NamedList params, SolrCore core) { try { return new AnalyzingInfixSuggester( - FSDirectory.open(Path.of(indexPath)), + FSDirectory.open(indexPath), indexAnalyzer, queryAnalyzer, minPrefixChars, diff --git a/solr/core/src/java/org/apache/solr/util/VersionedFile.java b/solr/core/src/java/org/apache/solr/util/VersionedFile.java index f3c1bb27ddf..b0d5665af48 100644 --- a/solr/core/src/java/org/apache/solr/util/VersionedFile.java +++ b/solr/core/src/java/org/apache/solr/util/VersionedFile.java @@ -73,6 +73,7 @@ public static InputStream getLatestFile(String dirName, String fileName) } } + // TODO SOLR-8282 Move to Files.newInputStream is = new FileInputStream(f.toString()); } catch (Exception e) { // swallow exception for now diff --git a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java index 39a992eb3ea..d4984f98a5f 100644 --- a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java +++ b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java @@ -81,10 +81,9 @@ public void testEscapeInstanceDir() throws Exception { // UNC paths assertTrue( assertThrows( - SolrResourceNotFoundException.class, - () -> loader.openResource("\\\\192.168.10.10\\foo").close()) + SolrException.class, () -> loader.openResource("\\\\192.168.10.10\\foo").close()) .getMessage() - .contains("Resource '\\\\192.168.10.10\\foo' could not be loaded.")); + .contains("Path \\\\192.168.10.10\\foo disallowed. UNC paths not supported.")); assertNull(loader.resourceLocation("\\\\192.168.10.10\\foo")); } @@ -97,10 +96,9 @@ public void testEscapeInstanceDir() throws Exception { // UNC paths never allowed assertTrue( assertThrows( - SolrResourceNotFoundException.class, - () -> loader.openResource("\\\\192.168.10.10\\foo").close()) + SolrException.class, () -> loader.openResource("\\\\192.168.10.10\\foo").close()) .getMessage() - .contains("Resource '\\\\192.168.10.10\\foo' could not be loaded.")); + .contains("Path \\\\192.168.10.10\\foo disallowed. UNC paths not supported.")); assertNull(loader.resourceLocation("\\\\192.168.10.10\\foo")); } } From fb4daf3196331d7b02f847b64362a2b2872f4931 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 27 Jan 2025 15:33:38 -0500 Subject: [PATCH 08/10] Second round of PR comments --- .../apache/solr/cloud/ZkSolrResourceLoader.java | 6 ++++-- .../org/apache/solr/core/CoreDescriptor.java | 9 +++++---- .../org/apache/solr/core/SolrResourceLoader.java | 8 ++++++-- .../apache/solr/core/backup/BackupManager.java | 5 +++-- .../apache/solr/filestore/DistribFileStore.java | 16 ++++++++-------- .../org/apache/solr/handler/IndexFetcher.java | 5 ++--- .../solr/schema/ManagedIndexSchemaFactory.java | 4 +++- .../spelling/AbstractLuceneSpellChecker.java | 4 +--- .../suggest/fst/AnalyzingInfixLookupFactory.java | 5 ++--- .../suggest/fst/BlendedInfixLookupFactory.java | 13 +++++++------ .../org/apache/solr/util/SystemIdResolver.java | 6 +++--- .../java/org/apache/solr/util/VersionedFile.java | 15 ++++++--------- 12 files changed, 50 insertions(+), 46 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java index dcec5bd4ab3..4a1b6ca2af9 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java @@ -20,6 +20,7 @@ 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; @@ -120,8 +121,9 @@ public InputStream openResource(String resource) throws IOException { try { // delegate to the class loader (looking into $INSTANCE_DIR/lib jars) - // Use Path to normalize path separator - is = classLoader.getResourceAsStream(Path.of(resource).toString()); + 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/CoreDescriptor.java b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java index 3fd83e60ff8..27d91b4d90e 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java +++ b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java @@ -20,6 +20,7 @@ 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; @@ -62,7 +63,8 @@ public class CoreDescriptor { public static final String CORE_CONFIGSET_PROPERTIES = "configSetProperties"; public static final String SOLR_CORE_PROP_PREFIX = "solr.core."; - public static final Path DEFAULT_EXTERNAL_PROPERTIES_FILE = Path.of("conf/solrcore.properties"); + public static final String DEFAULT_EXTERNAL_PROPERTIES_FILE = + "conf" + FileSystems.getDefault().getSeparator() + "solrcore.properties"; /** * Whether this core was configured using a configSet that was trusted. This helps in avoiding the @@ -94,7 +96,7 @@ public Properties getPersistableUserProperties() { CORE_CONFIG, "solrconfig.xml", CORE_SCHEMA, "schema.xml", CORE_CONFIGSET_PROPERTIES, ConfigSetProperties.DEFAULT_FILENAME, - CORE_DATADIR, Path.of("data").toString(), + CORE_DATADIR, "data" + FileSystems.getDefault().getSeparator(), CORE_TRANSIENT, "false", CORE_LOADONSTARTUP, "true"); @@ -238,8 +240,7 @@ public CoreDescriptor( *

File paths are taken as read from the core's instance directory if they are not absolute. */ protected void loadExtraProperties() { - String filename = - coreProperties.getProperty(CORE_PROPERTIES, DEFAULT_EXTERNAL_PROPERTIES_FILE.toString()); + String filename = coreProperties.getProperty(CORE_PROPERTIES, DEFAULT_EXTERNAL_PROPERTIES_FILE); Path propertiesFile = instanceDir.resolve(filename); if (Files.exists(propertiesFile)) { try (Reader r = Files.newBufferedReader(propertiesFile, StandardCharsets.UTF_8)) { diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java index 7e956ee9231..f0e4fcc1772 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java @@ -372,12 +372,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); + 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(Path.of("conf").resolve(resource).toString()); + is = + classLoader.getResourceAsStream( + ("conf/" + resource).replace(FileSystems.getDefault().getSeparator(), "/")); } if (is == null) { 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 index 687fa989182..37a18e08243 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java +++ b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java @@ -23,7 +23,7 @@ import java.lang.invoke.MethodHandles; import java.net.URI; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; +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, Path.of(filePath).toString()); + 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("/")) { diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index bac0da2a47b..0ccde6e5098 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -84,14 +84,15 @@ public Path getRealPath(String path) { return _getRealPath(path, solrHome); } - private static Path _getRealPath(String path, Path solrHome) { - Path dir = Path.of(path); - SolrPaths.assertNotUnc(dir); + private static Path _getRealPath(String dir, Path solrHome) { + Path path = Path.of(dir); + SolrPaths.assertNotUnc(path); - // Strip the path of from being absolute to become relative to resolve with SolrHome - while (path.startsWith("/")) { - path = path.substring(1); + if (path.isAbsolute()) { + // Strip the path of from being absolute to become relative to resolve with SolrHome + path = path.subpath(0, path.getNameCount()); } + var finalPath = getFileStoreDirPath(solrHome).resolve(path); // Guard against path traversal by asserting final path is sub path of filestore @@ -432,8 +433,7 @@ public void get(String path, Consumer consumer, boolean fetchmissing) Path file = getRealPath(path); String simpleName = file.getFileName().toString(); if (isMetaDataFile(simpleName)) { - // TODO SOLR-8282 Move to Files.newInputStream - try (InputStream is = new FileInputStream(file.toString())) { + try (InputStream is = Files.newInputStream(file)) { consumer.accept( new FileEntry(null, null, path) { // no metadata for metadata file diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java index 5c5e5017c0c..80c4ba6f1f9 100644 --- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java +++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java @@ -1494,11 +1494,10 @@ private void copyTmpConfFiles2Conf(Path tmpconfDir) { if (Files.exists(oldPath)) { try { Path backupFile = - Path.of( - oldPath + oldPath.resolveSibling( + oldPath.getFileName() + "." + getDateAsStr(new Date(Files.getLastModifiedTime(oldPath).toMillis()))); - Files.createDirectories(backupFile.getParent()); Files.move(oldPath, backupFile); } catch (Exception e) { throw new SolrException( diff --git a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java index 4d2f3e1739b..dce62c012e5 100644 --- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java +++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java @@ -410,7 +410,9 @@ private void upgradeToManagedSchema() { "nor under SolrConfig.getConfigDir() or the current directory. ", "PLEASE REMOVE THIS FILE."); } else { - Path upgradedSchemaFile = Path.of(nonManagedSchemaFile + UPGRADED_SCHEMA_EXTENSION); + Path upgradedSchemaFile = + nonManagedSchemaFile.resolveSibling( + nonManagedSchemaFile.getFileName() + UPGRADED_SCHEMA_EXTENSION); try { Files.move(nonManagedSchemaFile, upgradedSchemaFile); // Set the resource name to the managed schema so that the CoreAdminHandler returns a diff --git a/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java b/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java index 30eec4e61f8..61f99a84a61 100644 --- a/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java +++ b/solr/core/src/java/org/apache/solr/spelling/AbstractLuceneSpellChecker.java @@ -84,9 +84,7 @@ public String init(NamedList config, SolrCore core) { // If indexDir is relative then create index inside core.getDataDir() if (indexDir != null) { - if (!indexDir.isAbsolute()) { - indexDir = Path.of(core.getDataDir(), indexDir.toString()); - } + indexDir = Path.of(core.getDataDir()).resolve(indexDir); } sourceLocation = (String) config.get(LOCATION); diff --git a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java index 865f818ac80..a6ba007ed13 100644 --- a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java +++ b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java @@ -89,9 +89,8 @@ public Lookup create(NamedList params, SolrCore core) { params.get(INDEX_PATH) != null ? Path.of(params.get(INDEX_PATH).toString()) : Path.of(DEFAULT_INDEX_PATH); - if (!indexPath.isAbsolute()) { - indexPath = Path.of(core.getDataDir(), indexPath.toString()); - } + + indexPath = Path.of(core.getDataDir()).resolve(indexPath); int minPrefixChars = params.get(MIN_PREFIX_CHARS) != null diff --git a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java index c07164fb801..f4c4131d561 100644 --- a/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java +++ b/solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java @@ -76,11 +76,12 @@ public Lookup create(NamedList params, SolrCore core) { // optional parameters - String indexPath = - params.get(INDEX_PATH) != null ? params.get(INDEX_PATH).toString() : DEFAULT_INDEX_PATH; - if (!Path.of(indexPath).isAbsolute()) { - indexPath = Path.of(core.getDataDir(), indexPath).toString(); - } + Path indexPath = + params.get(INDEX_PATH) != null + ? Path.of(params.get(INDEX_PATH).toString()) + : Path.of(DEFAULT_INDEX_PATH); + + indexPath = Path.of(core.getDataDir()).resolve(indexPath); int minPrefixChars = params.get(MIN_PREFIX_CHARS) != null @@ -109,7 +110,7 @@ public Lookup create(NamedList params, SolrCore core) { try { return new BlendedInfixSuggester( - FSDirectory.open(Path.of(indexPath)), + FSDirectory.open(indexPath), indexAnalyzer, queryAnalyzer, minPrefixChars, diff --git a/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java b/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java index f311fa3df18..f1b87325878 100644 --- a/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java +++ b/solr/core/src/java/org/apache/solr/util/SystemIdResolver.java @@ -19,7 +19,7 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.Path; +import java.nio.file.FileSystems; import javax.xml.stream.XMLResolver; import javax.xml.stream.XMLStreamException; import javax.xml.transform.Source; @@ -167,9 +167,9 @@ public InputSource resolveEntity(String publicId, String systemId) throws IOExce } public static String createSystemIdFromResourceName(String name) { - Path pathName = Path.of(name); + name = name.replace(FileSystems.getDefault().getSeparator(), "/"); final String authority; - if (pathName.startsWith("/")) { + if (name.startsWith("/")) { // a hack to preserve absolute filenames and keep them absolute after resolving, we set the // URI's authority to "@" on absolute filenames: authority = RESOURCE_LOADER_AUTHORITY_ABSOLUTE; diff --git a/solr/core/src/java/org/apache/solr/util/VersionedFile.java b/solr/core/src/java/org/apache/solr/util/VersionedFile.java index b0d5665af48..dbc9dc8430a 100644 --- a/solr/core/src/java/org/apache/solr/util/VersionedFile.java +++ b/solr/core/src/java/org/apache/solr/util/VersionedFile.java @@ -16,8 +16,6 @@ */ package org.apache.solr.util; -import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; @@ -38,9 +36,9 @@ public class VersionedFile { * the last fileName.* after being sorted lexicographically. * Older versions of the file are deleted (and queued for deletion if * that fails). + * TODO SOLR-8282 dirName should be a Path instead of string */ - public static InputStream getLatestFile(String dirName, String fileName) - throws FileNotFoundException { + public static InputStream getLatestFile(String dirName, String fileName) throws IOException { Collection oldFiles = null; final String prefix = fileName + '.'; Path f = Path.of(dirName, fileName); @@ -66,15 +64,14 @@ public static InputStream getLatestFile(String dirName, String fileName) SolrException.ErrorCode.SERVER_ERROR, "Unable to list files in " + dir, e); } - f = Path.of(dir.toString(), fileList.getLast().getFileName().toString()); + f = dir.resolve(fileList.getLast()); oldFiles = new ArrayList<>(); for (int i = 0; i < fileList.size() - 1; i++) { - oldFiles.add(Path.of(dir.toString(), fileList.get(i).toString())); + oldFiles.add(dir.resolve(fileList.get(i))); } } - // TODO SOLR-8282 Move to Files.newInputStream - is = new FileInputStream(f.toString()); + is = Files.newInputStream(f); } catch (Exception e) { // swallow exception for now } @@ -82,7 +79,7 @@ public static InputStream getLatestFile(String dirName, String fileName) // allow exception to be thrown from the final try. if (is == null) { - is = new FileInputStream(f.toString()); + is = Files.newInputStream(f); } // delete old files only after we have successfully opened the newest From 5f8abaed75574052e210f486f308a41be489ed86 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 31 Jan 2025 14:18:40 -0500 Subject: [PATCH 09/10] Small refactoring and fix with metrics --- .../apache/solr/core/DirectoryFactory.java | 17 +++++---- .../java/org/apache/solr/core/SolrCore.java | 38 +++++++++++++------ .../java/org/apache/solr/core/SolrPaths.java | 2 +- .../apache/solr/core/SolrResourceLoader.java | 7 ++-- .../solr/filestore/DistribFileStore.java | 2 +- .../handler/admin/CoreAdminOperation.java | 5 +-- .../java/org/apache/solr/util/FileUtils.java | 14 +++++++ .../apache/solr/core/ResourceLoaderTest.java | 10 +++-- 8 files changed, 65 insertions(+), 30 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java index 5147c8cbfc8..33274fcd969 100644 --- a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java @@ -23,6 +23,7 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; +import java.util.Comparator; import java.util.List; import java.util.stream.Stream; import org.apache.commons.io.file.PathUtils; @@ -358,7 +359,7 @@ public void cleanupOldIndexDirectories( && !file.equals(currentIndexDir) && (fileName.equals("index") || fileName.matches(INDEX_W_TIMESTAMP_REGEX)); }) - .sorted() + .sorted(Comparator.reverseOrder()) .toList(); } ; @@ -369,15 +370,17 @@ public void cleanupOldIndexDirectories( int i = 0; if (afterCoreReload) { - log.info("Will not remove most recent old directory after reload {}", dirsList.getFirst()); + 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={}", - dirsList.size() - i, - dataDirPath, - afterCoreReload); + if (log.isInfoEnabled()) + log.info( + "Found {} old index directories to clean-up under {} afterReload={}", + dirsList.size() - i, + dataDirPath, + afterCoreReload); dirsList.stream() .skip(i) diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 973ce0a30e8..720a751f07a 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -32,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,17 +1354,33 @@ public void initializeMetrics(SolrMetricsContext parentContext, String scope) { } // initialize disk total / free metrics - Path dataDirFile = Paths.get(dataDir); - - // Do not pre-compute the data directory total/usable space on initialization. Calculated when - // metric is fetched - // TODO Move metrics initialization calls to after the data directory is created to fetch true - // directory space on initialization - parentContext.gauge(() -> 0L, true, "totalSpace", Category.CORE.toString(), "fs"); - parentContext.gauge(() -> 0L, true, "usableSpace", Category.CORE.toString(), "fs"); - + Path dataDirPath = Path.of(dataDir); + parentContext.gauge( + () -> { + try { + return Files.getFileStore(dataDirPath).getTotalSpace(); + } catch (IOException e) { + return 0L; + } + }, + true, + "totalSpace", + Category.CORE.toString(), + "fs"); + parentContext.gauge( + () -> { + try { + return Files.getFileStore(dataDirPath).getUsableSpace(); + } catch (IOException e) { + return 0L; + } + }, + true, + "usableSpace", + Category.CORE.toString(), + "fs"); parentContext.gauge( - () -> dataDirFile.toAbsolutePath().toString(), + () -> dataDirPath.toAbsolutePath().toString(), true, "path", Category.CORE.toString(), diff --git a/solr/core/src/java/org/apache/solr/core/SolrPaths.java b/solr/core/src/java/org/apache/solr/core/SolrPaths.java index dad7a374872..59aa0076437 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrPaths.java +++ b/solr/core/src/java/org/apache/solr/core/SolrPaths.java @@ -94,7 +94,7 @@ public static void assertNoPathTraversal(Path pathToAssert) { /** Asserts that a path is not a Windows UNC path */ public static void assertNotUnc(Path pathToAssert) { - if (OS.isFamilyWindows() || pathToAssert.toString().startsWith("\\\\")) { + if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Path " + pathToAssert + " disallowed. UNC paths not supported."); diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java index f0e4fcc1772..6d1065a0e54 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java @@ -353,8 +353,9 @@ public ClassLoader getClassLoader() { */ @Override public InputStream openResource(String resource) throws IOException { - SolrPaths.assertNotUnc(Path.of(resource)); // Always disallow UNC paths - + if (resource.trim().startsWith("\\\\")) { // Always disallow UNC paths + throw new SolrResourceNotFoundException("Resource '" + resource + "' could not be loaded."); + } Path instanceDir = getInstancePath().normalize(); Path inInstanceDir = getInstancePath().resolve(resource).normalize(); Path inConfigDir = instanceDir.resolve("conf").resolve(resource).normalize(); @@ -381,7 +382,7 @@ public InputStream openResource(String resource) throws IOException { if (is == null && System.getProperty("jetty.testMode") != null) { is = classLoader.getResourceAsStream( - ("conf/" + resource).replace(FileSystems.getDefault().getSeparator(), "/")); + ("conf/" + resource.replace(FileSystems.getDefault().getSeparator(), "/"))); } if (is == null) { diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index 0ccde6e5098..1847b05f950 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -254,7 +254,7 @@ public Path realPath() { MetaData readMetaData() throws IOException { Path file = getRealPath(getMetaPath()); if (Files.exists(file)) { - try (InputStream fis = new FileInputStream(file.toString())) { + try (InputStream fis = Files.newInputStream(file)) { return new MetaData((Map) Utils.fromJSON(fis)); } } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java index 47876c6f5e4..67bebb00efd 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java @@ -45,6 +45,7 @@ import static org.apache.solr.common.params.CoreAdminParams.SHARD; import static org.apache.solr.handler.admin.CoreAdminHandler.CallInfo; import static org.apache.solr.handler.admin.CoreAdminHandler.buildCoreParams; +import static org.apache.solr.util.FileUtils.normalizeToOsPathSeparator; import java.io.IOException; import java.lang.invoke.MethodHandles; @@ -356,9 +357,7 @@ public static NamedList getCoreStatus( if (core != null) { info.add(NAME, core.getName()); info.add("instanceDir", core.getInstancePath().toString()); - - info.add("dataDir", Path.of(core.getDataDir()).toString()); - + info.add("dataDir", normalizeToOsPathSeparator(core.getDataDir())); info.add("config", core.getConfigResource()); info.add("schema", core.getSchemaResource()); info.add("startTime", core.getStartTimeStamp()); diff --git a/solr/core/src/java/org/apache/solr/util/FileUtils.java b/solr/core/src/java/org/apache/solr/util/FileUtils.java index 0b221aae025..0f106a11f41 100644 --- a/solr/core/src/java/org/apache/solr/util/FileUtils.java +++ b/solr/core/src/java/org/apache/solr/util/FileUtils.java @@ -17,6 +17,7 @@ package org.apache.solr.util; import java.io.IOException; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import org.apache.commons.io.FileExistsException; @@ -69,4 +70,17 @@ public static boolean isPathAChildOfParent(Path parent, Path potentialChild) { return normalizedChild.startsWith(normalizedParent) && !normalizedChild.equals(normalizedParent); } + + /** + * Takes a path and normalizes it with the OS default filesystems separator + * + * @param path the path to normalize + * @return path normalized with the filesystems default separator + */ + public static String normalizeToOsPathSeparator(String path) { + if (path == null) return null; + path = path.replace("/", FileSystems.getDefault().getSeparator()); + path = path.replace("\\", FileSystems.getDefault().getSeparator()); + return path; + } } diff --git a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java index d4984f98a5f..39a992eb3ea 100644 --- a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java +++ b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java @@ -81,9 +81,10 @@ public void testEscapeInstanceDir() throws Exception { // UNC paths assertTrue( assertThrows( - SolrException.class, () -> loader.openResource("\\\\192.168.10.10\\foo").close()) + SolrResourceNotFoundException.class, + () -> loader.openResource("\\\\192.168.10.10\\foo").close()) .getMessage() - .contains("Path \\\\192.168.10.10\\foo disallowed. UNC paths not supported.")); + .contains("Resource '\\\\192.168.10.10\\foo' could not be loaded.")); assertNull(loader.resourceLocation("\\\\192.168.10.10\\foo")); } @@ -96,9 +97,10 @@ public void testEscapeInstanceDir() throws Exception { // UNC paths never allowed assertTrue( assertThrows( - SolrException.class, () -> loader.openResource("\\\\192.168.10.10\\foo").close()) + SolrResourceNotFoundException.class, + () -> loader.openResource("\\\\192.168.10.10\\foo").close()) .getMessage() - .contains("Path \\\\192.168.10.10\\foo disallowed. UNC paths not supported.")); + .contains("Resource '\\\\192.168.10.10\\foo' could not be loaded.")); assertNull(loader.resourceLocation("\\\\192.168.10.10\\foo")); } } From 679fb29c9605b4bfa73d9a5a0dab6611610a8ef4 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 31 Jan 2025 17:21:13 -0500 Subject: [PATCH 10/10] Update CHANGES.txt --- solr/CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index a844220f2fc..803fb89a3ef 100644 --- 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)