SOLR-4475: Fix various places that still assume File based paths even when not using a file based DirectoryFactory.

SOLR-4551: CachingDirectoryFactory needs to create CacheEntry's with the fullpath not path.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1454913 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -62,6 +62,12 @@ New Features
 Bug Fixes
 ----------------------
 
+* SOLR-4475: Fix various places that still assume File based paths even when 
+  not using a file based DirectoryFactory. (Mark Miller)
+  
+* SOLR-4551: CachingDirectoryFactory needs to create CacheEntry's with the 
+  fullpath not path. (Mark Miller)
+
 Other Changes
 ----------------------
 

solr/build.xml

@@ -70,7 +70,7 @@
     <java jar="${example}/start.jar" fork="true" dir="${example}" maxmemory="${example.heap.size}">
       <jvmarg line="${example.jvm.line}"/>
       <sysproperty key="solr.solr.home" file="${example.solr.home}"/>
-      <sysproperty key="solr.data.dir" file="${example.data.dir}"/>
+      <sysproperty key="solr.data.dir" value="${example.data.dir}"/>
       <sysproperty key="jetty.port" value="${example.jetty.port}"/>
     </java>
   </target>

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

@@ -204,7 +204,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
   protected abstract Directory create(String path, DirContext dirContext) throws IOException;
   
   @Override
-  public boolean exists(String path) {
+  public boolean exists(String path) throws IOException {
     // back compat behavior
     File dirFile = new File(path);
     return dirFile.canRead() && dirFile.list().length > 0;
@@ -231,7 +231,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
   @Override
   public final Directory get(String path,  DirContext dirContext, String rawLockType, boolean forceNew)
       throws IOException {
-    String fullPath = new File(path).getAbsolutePath();
+    String fullPath = normalize(path);
     synchronized (this) {
       if (closed) {
         throw new RuntimeException("Already closed");
@@ -271,7 +271,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
         newCacheValue.directory = directory;
         newCacheValue.path = fullPath;
         
-        injectLockFactory(directory, path, rawLockType);
+        injectLockFactory(directory, fullPath, rawLockType);
         
         byDirectoryCache.put(directory, newCacheValue);
         byPathCache.put(fullPath, newCacheValue);
@@ -372,4 +372,11 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
     }
     return dir;
   }
+  
+  protected String stripTrailingSlash(String path) {
+    if (path.endsWith("/")) {
+      path = path.substring(0, path.length() - 1);
+    }
+    return path;
+  }
 }

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

@@ -166,19 +166,8 @@ public class CoreDescriptor {
 
   public String getDataDir() {
     String dataDir = coreProperties.getProperty(CORE_DATADIR);
-    if (dataDir == null) {
+    if (dataDir == null) dataDir = getDefaultDataDir();
-      dataDir = getDefaultDataDir();
-    }
-    if (new File(dataDir).isAbsolute()) {
     return dataDir;
-    } else {
-      if (new File(getInstanceDir()).isAbsolute()) {
-        return SolrResourceLoader.normalizeDir(SolrResourceLoader.normalizeDir(getInstanceDir()) + dataDir);
-      } else  {
-        return SolrResourceLoader.normalizeDir(coreContainer.getSolrHome() +
-                SolrResourceLoader.normalizeDir(getRawInstanceDir()) + dataDir);
-      }
-    }
   }
 
   public void setDataDir(String s) {

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

@@ -18,6 +18,7 @@ package org.apache.solr.core;
  */
 
 import java.io.Closeable;
+import java.io.File;
 import java.io.IOException;
 
 import org.apache.lucene.store.Directory;
@@ -77,9 +78,10 @@ public abstract class DirectoryFactory implements NamedListInitializedPlugin,
   
   /**
    * Returns true if a Directory exists for a given path.
+   * @throws IOException If there is a low-level I/O error.
    * 
    */
-  public abstract boolean exists(String path);
+  public abstract boolean exists(String path) throws IOException;
   
   /**
    * Removes the Directory's persistent storage.
@@ -172,6 +174,15 @@ public abstract class DirectoryFactory implements NamedListInitializedPlugin,
     return path;
   }
   
+  /**
+   * @param path the path to check
+   * @return true if absolute, as in not relative
+   */
+  public boolean isAbsolute(String path) {
+    // back compat
+    return new File(path).isAbsolute();
+  }
+  
   public static long sizeOfDirectory(Directory directory) throws IOException {
     final String[] files = directory.listAll();
     long size = 0;

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

@@ -16,7 +16,6 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import java.io.File;
 import java.io.IOException;
 
 import org.apache.lucene.store.Directory;
@@ -28,8 +27,8 @@ import org.apache.lucene.store.Directory;
 public abstract class EphemeralDirectoryFactory extends CachingDirectoryFactory {
   
   @Override
-  public boolean exists(String path) {
+  public boolean exists(String path) throws IOException {
-    String fullPath = new File(path).getAbsolutePath();
+    String fullPath = normalize(path);
     synchronized (this) {
       CacheValue cacheValue = byPathCache.get(fullPath);
       Directory directory = null;
@@ -48,6 +47,12 @@ public abstract class EphemeralDirectoryFactory extends CachingDirectoryFactory
     return false;
   }
   
+  @Override
+  public boolean isAbsolute(String path) {
+    return true;
+  }
+  
+  
   @Override
   public void remove(Directory dir) throws IOException {
     // ram dir does not persist its dir anywhere
@@ -60,6 +65,7 @@ public abstract class EphemeralDirectoryFactory extends CachingDirectoryFactory
   
   @Override
   public String normalize(String path) throws IOException {
+    path = stripTrailingSlash(path);
     return path;
   }
 }

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

@@ -66,4 +66,9 @@ public class MMapDirectoryFactory extends StandardDirectoryFactory {
     }
     return mapDirectory;
   }
+  
+  @Override
+  public boolean isAbsolute(String path) {
+    return new File(path).isAbsolute();
+  }
 }

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

@@ -16,13 +16,12 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.NIOFSDirectory;
-import org.apache.solr.core.DirectoryFactory.DirContext;
-
 import java.io.File;
 import java.io.IOException;
 
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.NIOFSDirectory;
+
 
 /**
  * Factory to instantiate {@link org.apache.lucene.store.NIOFSDirectory}
@@ -35,4 +34,9 @@ public class NIOFSDirectoryFactory extends StandardDirectoryFactory {
     return new NIOFSDirectory(new File(path));
   }
   
+  @Override
+  public boolean isAbsolute(String path) {
+    return new File(path).isAbsolute();
+  }
+  
 }

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

@@ -55,4 +55,9 @@ public class NRTCachingDirectoryFactory extends StandardDirectoryFactory {
     return new NRTCachingDirectory(FSDirectory.open(new File(path)), maxMergeSizeMB, maxCachedMB);
   }
   
+  @Override
+  public boolean isAbsolute(String path) {
+    return new File(path).isAbsolute();
+  }
+
 }

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

@@ -16,13 +16,12 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.SimpleFSDirectory;
-import org.apache.solr.core.DirectoryFactory.DirContext;
-
 import java.io.File;
 import java.io.IOException;
 
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.SimpleFSDirectory;
+
 
 /**
  * Factory to instantiate {@link org.apache.lucene.store.SimpleFSDirectory}
@@ -35,4 +34,8 @@ public class SimpleFSDirectoryFactory extends StandardDirectoryFactory {
     return new SimpleFSDirectory(new File(path));
   }
 
+  @Override
+  public boolean isAbsolute(String path) {
+    return new File(path).isAbsolute();
+  }
 }

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

@@ -459,7 +459,7 @@ public final class SolrCore implements SolrInfoMBean {
       boolean indexExists = getDirectoryFactory().exists(indexDir);
       boolean firstTime;
       synchronized (SolrCore.class) {
-        firstTime = dirs.add(new File(indexDir).getCanonicalPath());
+        firstTime = dirs.add(getDirectoryFactory().normalize(indexDir));
       }
       boolean removeLocks = solrConfig.unlockOnStartup;
 
@@ -656,12 +656,24 @@ public final class SolrCore implements SolrInfoMBean {
     coreDescriptor = cd;
     this.setName( name );
     resourceLoader = config.getResourceLoader();
-    if (dataDir == null){
+    this.solrConfig = config;
-      if(cd.usingDefaultDataDir()) {
+    
-        dataDir = config.getDataDir();
+    if (updateHandler == null) {
+      initDirectoryFactory();
     }
-      if(dataDir == null) {
+    
+    if (dataDir == null) {
+      if (cd.usingDefaultDataDir()) dataDir = config.getDataDir();
+      if (dataDir == null) {
         dataDir = cd.getDataDir();
+        try {
+          if (!directoryFactory.isAbsolute(dataDir)) {
+            dataDir = directoryFactory.normalize(SolrResourceLoader
+                .normalizeDir(cd.getInstanceDir()) + dataDir);
+          }
+        } catch (IOException e) {
+          throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, null, e);
+        }
       }
     }
 
@@ -703,7 +715,6 @@ public final class SolrCore implements SolrInfoMBean {
 
     this.schema = schema;
     this.dataDir = dataDir;
-    this.solrConfig = config;
     this.startTime = System.currentTimeMillis();
     this.maxWarmingSearchers = config.maxWarmingSearchers;
 

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

@@ -43,13 +43,21 @@ public class StandardDirectoryFactory extends CachingDirectoryFactory {
   
   @Override
   public String normalize(String path) throws IOException {
-    return new File(path).getCanonicalPath();
+    String cpath = new File(path).getCanonicalPath();
+    
+    return stripTrailingSlash(cpath);
   }
   
   public boolean isPersistent() {
     return true;
   }
   
+  @Override
+  public boolean isAbsolute(String path) {
+    // back compat
+    return new File(path).isAbsolute();
+  }
+  
   @Override
   public void remove(Directory dir) throws IOException {
     CacheValue val = byDirectoryCache.get(dir);
@@ -60,10 +68,9 @@ public class StandardDirectoryFactory extends CachingDirectoryFactory {
     FileUtils.deleteDirectory(dirFile);
   }
 
-
   @Override
   public void remove(String path) throws IOException {
-    String fullPath = new File(path).getAbsolutePath();
+    String fullPath = normalize(path);
     File dirFile = new File(fullPath);
     FileUtils.deleteDirectory(dirFile);
   }

solr/core/src/java/org/apache/solr/handler/SnapPuller.java

@@ -439,9 +439,13 @@ public class SnapPuller {
             @Override
             public void preClose() {
               LOG.info("removing old index files " + freezeIndexDir);
+              try {
                 if (core.getDirectoryFactory().exists(freezeIndexDirPath)) {
                   DirectoryFactory.empty(freezeIndexDir);
                 }
+              } catch (IOException e) {
+                SolrException.log(LOG, null, e);
+              }
             }
             
             @Override
@@ -674,13 +678,14 @@ public class SnapPuller {
     
   }
 
-
   /**
    * All the files are copied to a temp dir first
    */
   private String createTempindexDir(SolrCore core, String tmpIdxDirName) {
-    File tmpIdxDir = new File(core.getDataDir(), tmpIdxDirName);
+    // TODO: there should probably be a DirectoryFactory#concatPath(parent, name)
-    return tmpIdxDir.toString();
+    // or something
+    String tmpIdxDir = core.getDataDir() + tmpIdxDirName;
+    return tmpIdxDir;
   }
 
   private void reloadCore() {

solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java

@@ -22,7 +22,6 @@ import java.io.IOException;
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.store.Directory;
 import org.apache.solr.SolrTestCaseJ4;
-import org.apache.solr.core.DirectoryFactory.DirContext;
 import org.junit.BeforeClass;
 import org.junit.Test;
 

solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java

@@ -71,7 +71,7 @@ import org.junit.Test;
 public class TestReplicationHandler extends SolrTestCaseJ4 {
 
 
-  private static final String CONF_DIR = "." + File.separator + "solr"
+  private static final String CONF_DIR = "solr"
       + File.separator + "collection1" + File.separator + "conf"
       + File.separator;
 

solr/test-framework/src/java/org/apache/solr/core/MockDirectoryFactory.java

@@ -17,6 +17,7 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
+import java.io.File;
 import java.io.IOException;
 
 import org.apache.lucene.store.Directory;
@@ -55,4 +56,11 @@ public class MockDirectoryFactory extends EphemeralDirectoryFactory {
     return dir;
   }
   
+  @Override
+  public boolean isAbsolute(String path) {
+    // TODO: kind of a hack - we don't know what the delegate is, so
+    // we treat it as file based since this works on most ephem impls
+    return new File(path).isAbsolute();
+  }
+
 }

solr/test-framework/src/java/org/apache/solr/core/MockFSDirectoryFactory.java

@@ -43,4 +43,11 @@ public class MockFSDirectoryFactory extends StandardDirectoryFactory {
     }
     return dir;
   }
+  
+  @Override
+  public boolean isAbsolute(String path) {
+    // TODO: kind of a hack - we don't know what the delegate is, so
+    // we treat it as file based since this works on most ephem impls
+    return new File(path).isAbsolute();
+  }
 }