diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 0dcdb051cc7..56caef4a5ba 100644 --- a/solr/CHANGES.txt +++ b/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 ---------------------- diff --git a/solr/build.xml b/solr/build.xml index 6084dee4952..8ef54fea7d3 100644 --- a/solr/build.xml +++ b/solr/build.xml @@ -70,7 +70,7 @@ - + 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 82110530f25..d427a6197c9 100644 --- a/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java +++ b/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; + } } 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 7cec3075628..8fddec520f6 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java +++ b/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) { - 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); - } - } + if (dataDir == null) dataDir = getDefaultDataDir(); + return dataDir; } public void setDataDir(String s) { 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 3180ed225e6..dd899e4272a 100644 --- a/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java +++ b/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; diff --git a/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java index eb8610e7762..1f317260730 100644 --- a/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java +++ b/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) { - String fullPath = new File(path).getAbsolutePath(); + public boolean exists(String path) throws IOException { + 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; } } diff --git a/solr/core/src/java/org/apache/solr/core/MMapDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/MMapDirectoryFactory.java index 1ddffe128c5..4bc5b599c43 100644 --- a/solr/core/src/java/org/apache/solr/core/MMapDirectoryFactory.java +++ b/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(); + } } diff --git a/solr/core/src/java/org/apache/solr/core/NIOFSDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/NIOFSDirectoryFactory.java index 3a6534afbed..303f001552d 100644 --- a/solr/core/src/java/org/apache/solr/core/NIOFSDirectoryFactory.java +++ b/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(); + } + } diff --git a/solr/core/src/java/org/apache/solr/core/NRTCachingDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/NRTCachingDirectoryFactory.java index 578555c0315..f641f9f70ec 100644 --- a/solr/core/src/java/org/apache/solr/core/NRTCachingDirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/NRTCachingDirectoryFactory.java @@ -54,5 +54,10 @@ public class NRTCachingDirectoryFactory extends StandardDirectoryFactory { protected Directory create(String path, DirContext dirContext) throws IOException { return new NRTCachingDirectory(FSDirectory.open(new File(path)), maxMergeSizeMB, maxCachedMB); } + + @Override + public boolean isAbsolute(String path) { + return new File(path).isAbsolute(); + } } diff --git a/solr/core/src/java/org/apache/solr/core/SimpleFSDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/SimpleFSDirectoryFactory.java index a125d095b8e..7489d7512ff 100644 --- a/solr/core/src/java/org/apache/solr/core/SimpleFSDirectoryFactory.java +++ b/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(); + } } 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 6827e769392..c82c10035ca 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/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){ - if(cd.usingDefaultDataDir()) { - dataDir = config.getDataDir(); - } - if(dataDir == null) { + this.solrConfig = config; + + if (updateHandler == null) { + initDirectoryFactory(); + } + + 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; diff --git a/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java index b92f539b2e7..300dc354a31 100644 --- a/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java +++ b/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); @@ -59,11 +67,10 @@ public class StandardDirectoryFactory extends CachingDirectoryFactory { File dirFile = new File(val.path); 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); } diff --git a/solr/core/src/java/org/apache/solr/handler/SnapPuller.java b/solr/core/src/java/org/apache/solr/handler/SnapPuller.java index edbef1761e4..7142e02d72c 100644 --- a/solr/core/src/java/org/apache/solr/handler/SnapPuller.java +++ b/solr/core/src/java/org/apache/solr/handler/SnapPuller.java @@ -439,8 +439,12 @@ public class SnapPuller { @Override public void preClose() { LOG.info("removing old index files " + freezeIndexDir); - if (core.getDirectoryFactory().exists(freezeIndexDirPath)) { - DirectoryFactory.empty(freezeIndexDir); + try { + if (core.getDirectoryFactory().exists(freezeIndexDirPath)) { + DirectoryFactory.empty(freezeIndexDir); + } + } catch (IOException e) { + SolrException.log(LOG, null, e); } } @@ -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); - return tmpIdxDir.toString(); + // TODO: there should probably be a DirectoryFactory#concatPath(parent, name) + // or something + String tmpIdxDir = core.getDataDir() + tmpIdxDirName; + return tmpIdxDir; } private void reloadCore() { diff --git a/solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java b/solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java index 4ec7df041bd..fec51e254ff 100755 --- a/solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java +++ b/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; diff --git a/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java b/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java index 070955b6df0..3222ea74412 100644 --- a/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java +++ b/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; diff --git a/solr/test-framework/src/java/org/apache/solr/core/MockDirectoryFactory.java b/solr/test-framework/src/java/org/apache/solr/core/MockDirectoryFactory.java index 6deaa61b38b..90e9fafb135 100644 --- a/solr/test-framework/src/java/org/apache/solr/core/MockDirectoryFactory.java +++ b/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; @@ -54,5 +55,12 @@ 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(); + } } diff --git a/solr/test-framework/src/java/org/apache/solr/core/MockFSDirectoryFactory.java b/solr/test-framework/src/java/org/apache/solr/core/MockFSDirectoryFactory.java index a7aeb0f9ff3..4a71cb212c3 100644 --- a/solr/test-framework/src/java/org/apache/solr/core/MockFSDirectoryFactory.java +++ b/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(); + } }