You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2020/06/03 02:40:49 UTC

[lucene-solr] branch branch_8x updated: SOLR: Use absolute paths for server paths. (#1546)

This is an automated email from the ASF dual-hosted git repository.

dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new d8a40ef  SOLR: Use absolute paths for server paths. (#1546)
d8a40ef is described below

commit d8a40efae528bfbf65c774331384cd545039cfb8
Author: David Smiley <ds...@apache.org>
AuthorDate: Tue Jun 2 22:36:52 2020 -0400

    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).
    
    (cherry picked from commit a06f57c7e16b305c26e94c2a6e76e6b6528b8ce2)
---
 .../apache/solr/core/CachingDirectoryFactory.java    |  2 +-
 .../src/java/org/apache/solr/core/CoreContainer.java |  2 ++
 .../java/org/apache/solr/core/CoreDescriptor.java    |  6 ++----
 .../org/apache/solr/core/CorePropertiesLocator.java  |  6 +++---
 .../java/org/apache/solr/core/DirectoryFactory.java  | 19 +++++++++----------
 .../src/java/org/apache/solr/core/NodeConfig.java    | 20 +++++++++++---------
 .../core/src/java/org/apache/solr/core/SolrCore.java |  6 +++---
 .../solr/handler/admin/CoreAdminOperation.java       |  4 +++-
 .../test/org/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 732b82c..0b4e193 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 6625f09..76b8945 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1777,6 +1777,7 @@ public class CoreContainer {
     return solrCores.getCoreDescriptor(coreName);
   }
 
+  /** Where cores are created (absolute). */
   public Path getCoreRootDirectory() {
     return cfg.getCoreRootDirectory();
   }
@@ -1938,6 +1939,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 1072929..d622734 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<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));
   }
 
-  /**
-   * @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 b32a85e..2d2712f 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 b1f34086..5736692 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 aab1b47..f1af975 100644
--- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/NodeConfig.java
@@ -28,6 +28,7 @@ import org.apache.solr.update.UpdateShardHandlerConfig;
 
 
 public class NodeConfig {
+  // all Path fields here are absolute and normalized.
 
   private final String nodeName;
 
@@ -89,6 +90,7 @@ public class NodeConfig {
                      Path solrHome, SolrResourceLoader loader,
                      Properties solrProperties, PluginInfo[] backupRepositoryPlugins,
                      MetricsConfig metricsConfig, PluginInfo transientCacheConfig, PluginInfo tracerConfig) {
+    // all Path params here are absolute and normalized.
     this.nodeName = nodeName;
     this.coreRootDirectory = coreRootDirectory;
     this.solrDataHome = solrDataHome;
@@ -127,10 +129,12 @@ public class NodeConfig {
     return nodeName;
   }
 
+  /** Absolute. */
   public Path getCoreRootDirectory() {
     return coreRootDirectory;
   }
 
+  /** Absolute. */
   public Path getSolrDataHome() {
     return solrDataHome;
   }
@@ -201,6 +205,7 @@ public class NodeConfig {
     return managementPath;
   }
 
+  /** Absolute. */
   public Path getConfigSetBaseDirectory() {
     return configSetBaseDirectory;
   }
@@ -248,7 +253,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;
@@ -302,26 +307,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);
-      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 8646a9f..39fe489 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -328,7 +328,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
     return schema;
   }
 
-  /** The core's instance directory. */
+  /** The core's instance directory (absolute). */
   public Path getInstancePath() {
     return getCoreDescriptor().getInstanceDir();
   }
@@ -1361,7 +1361,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
   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;
   }
@@ -3020,7 +3020,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
 
   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 f8c7e8c..6351326 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<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);
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 a9bd969..c97f816 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();