From a06f57c7e16b305c26e94c2a6e76e6b6528b8ce2 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 2 Jun 2020 22:36:52 -0400 Subject: [PATCH] SOLR: Use absolute paths for server paths. (#1546) CoreContainer's paths and SolrCore instanceDir are now absolute; mandated. This avoids bugs when the current working directory of the server is abnormal (perhaps running in tests or some IDE configs). --- .../solr/core/CachingDirectoryFactory.java | 2 +- .../org/apache/solr/core/CoreContainer.java | 2 ++ .../org/apache/solr/core/CoreDescriptor.java | 6 ++---- .../solr/core/CorePropertiesLocator.java | 6 +++--- .../apache/solr/core/DirectoryFactory.java | 19 +++++++++--------- .../java/org/apache/solr/core/NodeConfig.java | 20 ++++++++++--------- .../java/org/apache/solr/core/SolrCore.java | 6 +++--- .../handler/admin/CoreAdminOperation.java | 4 +++- .../apache/solr/core/TestCoreDiscovery.java | 2 +- 9 files changed, 35 insertions(+), 32 deletions(-) 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 732b82c1f56..0b4e193b8d7 100644 --- a/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java @@ -405,7 +405,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory { // override global config if (args.get(SolrXmlConfig.SOLR_DATA_HOME) != null) { - dataHomePath = Paths.get((String) args.get(SolrXmlConfig.SOLR_DATA_HOME)); + dataHomePath = Paths.get((String) args.get(SolrXmlConfig.SOLR_DATA_HOME)).toAbsolutePath().normalize(); } if (dataHomePath != null) { log.info("{} = {}", SolrXmlConfig.SOLR_DATA_HOME, dataHomePath); diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index f968800a7c4..c011a644d66 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1769,6 +1769,7 @@ public class CoreContainer { return solrCores.getCoreDescriptor(coreName); } + /** Where cores are created (absolute). */ public Path getCoreRootDirectory() { return cfg.getCoreRootDirectory(); } @@ -1927,6 +1928,7 @@ public class CoreContainer { return solrCores.getUnloadedCoreDescriptor(cname); } + /** The primary path of a Solr server's config, cores, and misc things. Absolute. */ //TODO return Path public String getSolrHome() { return solrHome.toString(); 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 10729295bce..d622734391d 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java +++ b/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java @@ -182,7 +182,7 @@ public class CoreDescriptor { */ public CoreDescriptor(String name, Path instanceDir, Map coreProps, Properties containerProperties, ZkController zkController) { - this.instanceDir = instanceDir; + this.instanceDir = instanceDir.toAbsolutePath(); originalCoreProperties.setProperty(CORE_NAME, name); @@ -290,9 +290,7 @@ public class CoreDescriptor { return defaultProperties.get(CORE_DATADIR).equals(coreProperties.getProperty(CORE_DATADIR)); } - /** - * @return the core instance directory - */ + /** The core instance directory (absolute). */ public Path getInstanceDir() { return instanceDir; } diff --git a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java index b32a85e88ce..2d2712f1a18 100644 --- a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java +++ b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java @@ -62,7 +62,7 @@ public class CorePropertiesLocator implements CoresLocator { @Override public void create(CoreContainer cc, CoreDescriptor... coreDescriptors) { for (CoreDescriptor cd : coreDescriptors) { - Path propertiesFile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME); + Path propertiesFile = cd.getInstanceDir().resolve(PROPERTIES_FILENAME); if (Files.exists(propertiesFile)) throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Could not create a new core in " + cd.getInstanceDir() @@ -78,7 +78,7 @@ public class CorePropertiesLocator implements CoresLocator { @Override public void persist(CoreContainer cc, CoreDescriptor... coreDescriptors) { for (CoreDescriptor cd : coreDescriptors) { - Path propFile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME); + Path propFile = cd.getInstanceDir().resolve(PROPERTIES_FILENAME); writePropertiesFile(cd, propFile); } } @@ -105,7 +105,7 @@ public class CorePropertiesLocator implements CoresLocator { } for (CoreDescriptor cd : coreDescriptors) { if (cd == null) continue; - Path propfile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME); + Path propfile = cd.getInstanceDir().resolve(PROPERTIES_FILENAME); try { Files.deleteIfExists(propfile); } catch (IOException e) { 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 b1f34086776..57366921c05 100644 --- a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java @@ -23,9 +23,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.nio.file.NoSuchFileException; -import java.util.Arrays; import java.nio.file.Path; -import java.nio.file.Paths; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -55,7 +54,7 @@ public abstract class DirectoryFactory implements NamedListInitializedPlugin, protected static final String INDEX_W_TIMESTAMP_REGEX = "index\\.[0-9]{17}"; // see SnapShooter.DATE_FMT - // May be set by sub classes as data root, in which case getDataHome will use it as base + // May be set by sub classes as data root, in which case getDataHome will use it as base. Absolute. protected Path dataHomePath; // hint about what the directory contains - default is index directory @@ -331,16 +330,16 @@ public abstract class DirectoryFactory implements NamedListInitializedPlugin, * @return a String with absolute path to data direcotry */ public String getDataHome(CoreDescriptor cd) throws IOException { - String dataDir; + Path dataDir; if (dataHomePath != null) { - String instanceDirLastPath = cd.getInstanceDir().getName(cd.getInstanceDir().getNameCount()-1).toString(); - dataDir = Paths.get(coreContainer.getSolrHome()).resolve(dataHomePath) - .resolve(instanceDirLastPath).resolve(cd.getDataDir()).toAbsolutePath().toString(); + Path instanceDirLastPath = cd.getInstanceDir().getName(cd.getInstanceDir().getNameCount()-1); + dataDir = dataHomePath.resolve(instanceDirLastPath).resolve(cd.getDataDir()); } else { // by default, we go off the instance directory - dataDir = cd.getInstanceDir().resolve(cd.getDataDir()).toAbsolutePath().toString(); + dataDir = cd.getInstanceDir().resolve(cd.getDataDir()); } - return dataDir; + assert dataDir.isAbsolute(); + return dataDir.toString(); } public void cleanupOldIndexDirectories(final String dataDirPath, final String currentIndexDirPath, boolean afterCoreReload) { @@ -398,7 +397,7 @@ public abstract class DirectoryFactory implements NamedListInitializedPlugin, public void initCoreContainer(CoreContainer cc) { this.coreContainer = cc; if (cc != null && cc.getConfig() != null) { - this.dataHomePath = cc.getConfig().getSolrDataHome(); + this.dataHomePath = cc.getConfig().getSolrDataHome(); // absolute } } diff --git a/solr/core/src/java/org/apache/solr/core/NodeConfig.java b/solr/core/src/java/org/apache/solr/core/NodeConfig.java index 01aaf3512ab..353d83e82a9 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java +++ b/solr/core/src/java/org/apache/solr/core/NodeConfig.java @@ -29,6 +29,7 @@ import org.apache.solr.update.UpdateShardHandlerConfig; public class NodeConfig { + // all Path fields here are absolute and normalized. private final String nodeName; @@ -95,6 +96,7 @@ public class NodeConfig { Properties solrProperties, PluginInfo[] backupRepositoryPlugins, MetricsConfig metricsConfig, PluginInfo transientCacheConfig, PluginInfo tracerConfig, boolean fromZookeeper) { + // all Path params here are absolute and normalized. this.nodeName = nodeName; this.coreRootDirectory = coreRootDirectory; this.solrDataHome = solrDataHome; @@ -134,10 +136,12 @@ public class NodeConfig { return nodeName; } + /** Absolute. */ public Path getCoreRootDirectory() { return coreRootDirectory; } + /** Absolute. */ public Path getSolrDataHome() { return solrDataHome; } @@ -208,6 +212,7 @@ public class NodeConfig { return managementPath; } + /** Absolute. */ public Path getConfigSetBaseDirectory() { return configSetBaseDirectory; } @@ -259,7 +264,7 @@ public class NodeConfig { } public static class NodeConfigBuilder { - + // all Path fields here are absolute and normalized. private SolrResourceLoader loader; private Path coreRootDirectory; private Path solrDataHome; @@ -314,26 +319,23 @@ public class NodeConfig { public NodeConfigBuilder(String nodeName, Path solrHome) { this.nodeName = nodeName; - this.solrHome = solrHome; + this.solrHome = solrHome.toAbsolutePath(); this.coreRootDirectory = solrHome; // always init from sysprop because config element may be missing - String dataHomeProperty = System.getProperty(SolrXmlConfig.SOLR_DATA_HOME); - if (dataHomeProperty != null && !dataHomeProperty.isEmpty()) { - solrDataHome = solrHome.resolve(dataHomeProperty); - } - this.configSetBaseDirectory = solrHome.resolve("configsets"); + setSolrDataHome(System.getProperty(SolrXmlConfig.SOLR_DATA_HOME)); + setConfigSetBaseDirectory("configsets"); this.metricsConfig = new MetricsConfig.MetricsConfigBuilder().build(); } public NodeConfigBuilder setCoreRootDirectory(String coreRootDirectory) { - this.coreRootDirectory = solrHome.resolve(coreRootDirectory); + this.coreRootDirectory = solrHome.resolve(coreRootDirectory).normalize(); return this; } public NodeConfigBuilder setSolrDataHome(String solrDataHomeString) { // keep it null unless explicitly set to non-empty value if (solrDataHomeString != null && !solrDataHomeString.isEmpty()) { - this.solrDataHome = solrHome.resolve(solrDataHomeString); + this.solrDataHome = solrHome.resolve(solrDataHomeString).normalize(); } return this; } 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 d7bd4c08a2b..9b635e49402 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -327,7 +327,7 @@ public final class SolrCore implements SolrInfoBean, Closeable { return schema; } - /** The core's instance directory. */ + /** The core's instance directory (absolute). */ public Path getInstancePath() { return getCoreDescriptor().getInstanceDir(); } @@ -1358,7 +1358,7 @@ public final class SolrCore implements SolrInfoBean, Closeable { private String initUpdateLogDir(CoreDescriptor coreDescriptor) { String updateLogDir = coreDescriptor.getUlogDir(); if (updateLogDir == null) { - updateLogDir = coreDescriptor.getInstanceDir().resolve(dataDir).normalize().toAbsolutePath().toString(); + updateLogDir = coreDescriptor.getInstanceDir().resolve(dataDir).toString(); } return updateLogDir; } @@ -3001,7 +3001,7 @@ public final class SolrCore implements SolrInfoBean, Closeable { public static void deleteUnloadedCore(CoreDescriptor cd, boolean deleteDataDir, boolean deleteInstanceDir) { if (deleteDataDir) { - File dataDir = new File(cd.getInstanceDir().resolve(cd.getDataDir()).toAbsolutePath().toString()); + File dataDir = cd.getInstanceDir().resolve(cd.getDataDir()).toFile(); try { FileUtils.deleteDirectory(dataDir); } catch (IOException e) { 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 f8c7e8c772a..6351326dea7 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 @@ -77,7 +77,7 @@ enum CoreAdminOperation implements CoreAdminOp { String coreName = params.required().get(CoreAdminParams.NAME); Map coreParams = buildCoreParams(params); CoreContainer coreContainer = it.handler.coreContainer; - Path instancePath = coreContainer.getCoreRootDirectory().resolve(coreName); + Path instancePath; // TODO: Should we nuke setting odd instance paths? They break core discovery, generally String instanceDir = it.req.getParams().get(CoreAdminParams.INSTANCE_DIR); @@ -86,6 +86,8 @@ enum CoreAdminOperation implements CoreAdminOp { if (instanceDir != null) { instanceDir = PropertiesUtil.substituteProperty(instanceDir, coreContainer.getContainerProperties()); instancePath = coreContainer.getCoreRootDirectory().resolve(instanceDir).normalize(); + } else { + instancePath = coreContainer.getCoreRootDirectory().resolve(coreName); } boolean newCollection = params.getBool(CoreAdminParams.NEW_COLLECTION, false); diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java index a9bd969144a..c97f8167e99 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java +++ b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java @@ -397,7 +397,7 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 { assertNull(cc.getCore("core0")); SolrCore core3 = cc.create("core3", ImmutableMap.of("configSet", "minimal")); - assertThat(core3.getCoreDescriptor().getInstanceDir().toAbsolutePath().toString(), containsString("relative")); + assertThat(core3.getCoreDescriptor().getInstanceDir().toString(), containsString("relative")); } finally { cc.shutdown();