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/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);

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();

solr/core/src/java/org/apache/solr/core/CoreDescriptor.java

@@ -182,7 +182,7 @@ public class CoreDescriptor {
    */
   public CoreDescriptor(String name, Path instanceDir, Map<String, String> 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));
   }
 
-  /**
+  /** The core instance directory (absolute). */
-   * @return the core instance directory
-   */
   public Path getInstanceDir() {
     return instanceDir;
   }

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) {

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();
+      Path instanceDirLastPath = cd.getInstanceDir().getName(cd.getInstanceDir().getNameCount()-1);
-      dataDir = Paths.get(coreContainer.getSolrHome()).resolve(dataHomePath)
+      dataDir = dataHomePath.resolve(instanceDirLastPath).resolve(cd.getDataDir());
-          .resolve(instanceDirLastPath).resolve(cd.getDataDir()).toAbsolutePath().toString();
     } 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
     }
   }
   

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 <solrDataHome> config element may be missing
-      String dataHomeProperty = System.getProperty(SolrXmlConfig.SOLR_DATA_HOME);
+      setSolrDataHome(System.getProperty(SolrXmlConfig.SOLR_DATA_HOME));
-      if (dataHomeProperty != null && !dataHomeProperty.isEmpty()) {
+      setConfigSetBaseDirectory("configsets");
-        solrDataHome = solrHome.resolve(dataHomeProperty);
-      }
-      this.configSetBaseDirectory = solrHome.resolve("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;
     }

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) {

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<String, String> 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);

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();