Author: epbernard Date: 2008-04-29 20:27:12 -0400 (Tue, 29 Apr 2008) New Revision: 14612 Added: search/trunk/src/java/org/hibernate/search/store/DirectoryProviderHelper.java Removed: search/trunk/src/java/org/hibernate/search/util/DirectoryProviderHelper.java Modified: search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java search/trunk/src/java/org/hibernate/search/event/FullTextIndexEventListener.java search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java search/trunk/src/java/org/hibernate/search/store/FSDirectoryProvider.java search/trunk/src/java/org/hibernate/search/store/FSMasterDirectoryProvider.java search/trunk/src/java/org/hibernate/search/store/FSSlaveDirectoryProvider.java search/trunk/src/test/org/hibernate/search/test/FSDirectoryTest.java Log: HSEARCH-181 Fix a few directory related issues around DirectoryProvider (Sanne Grinovero) Modified: search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -40,4 +40,6 @@ public LuceneIndexingParameters getIndexingParameters(DirectoryProvider<?> provider); void addIndexingParmeters(DirectoryProvider<?> provider, LuceneIndexingParameters indexingParams); + + public String getIndexingStrategy(); } Modified: search/trunk/src/java/org/hibernate/search/event/FullTextIndexEventListener.java =================================================================== --- search/trunk/src/java/org/hibernate/search/event/FullTextIndexEventListener.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/event/FullTextIndexEventListener.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -40,16 +40,13 @@ public void initialize(Configuration cfg) { searchFactoryImplementor = SearchFactoryImpl.getSearchFactory( cfg ); - String indexingStrategy = cfg.getProperties().getProperty( Environment.INDEXING_STRATEGY, "event" ); + String indexingStrategy = searchFactoryImplementor.getIndexingStrategy(); if ( "event".equals( indexingStrategy ) ) { used = searchFactoryImplementor.getDocumentBuilders().size() != 0; } else if ( "manual".equals( indexingStrategy ) ) { used = false; } - else { - throw new SearchException(Environment.INDEXING_STRATEGY + " unknown: " + indexingStrategy); - } } public SearchFactoryImplementor getSearchFactoryImplementor() { Modified: search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -66,6 +66,7 @@ private BackendQueueProcessorFactory backendQueueProcessorFactory; private Map<String, FilterDef> filterDefinitions = new HashMap<String, FilterDef>(); private FilterCachingStrategy filterCachingStrategy; + private String indexingStrategy; /** * Each directory provider (index) can have its own performance settings. @@ -86,6 +87,7 @@ public SearchFactoryImpl(Configuration cfg) { //yuk ReflectionManager reflectionManager = getReflectionManager( cfg ); + setIndexingStrategy(cfg); //need to be done before the document builds InitContext context = new InitContext(cfg); initDocumentBuilders(cfg, reflectionManager, context ); @@ -98,6 +100,17 @@ buildFilterCachingStrategy( cfg.getProperties() ); } + private void setIndexingStrategy(Configuration cfg) { + indexingStrategy = cfg.getProperties().getProperty( Environment.INDEXING_STRATEGY, "event" ); + if ( ! ("event".equals( indexingStrategy ) || "manual".equals( indexingStrategy ) ) ) { + throw new SearchException(Environment.INDEXING_STRATEGY + " unknown: " + indexingStrategy); + } + } + + public String getIndexingStrategy() { + return indexingStrategy; + } + private void bindFilterDefs(XClass mappedXClass) { FullTextFilterDef defAnn = mappedXClass.getAnnotation( FullTextFilterDef.class ); if ( defAnn != null ) { Added: search/trunk/src/java/org/hibernate/search/store/DirectoryProviderHelper.java =================================================================== --- search/trunk/src/java/org/hibernate/search/store/DirectoryProviderHelper.java (rev 0) +++ search/trunk/src/java/org/hibernate/search/store/DirectoryProviderHelper.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -0,0 +1,143 @@ +//$Id$ +package org.hibernate.search.store; + +import java.util.Properties; +import java.io.File; +import java.io.IOException; + +import org.hibernate.search.SearchException; +import org.hibernate.annotations.common.util.StringHelper; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.store.FSDirectory; + +/** + * @author Emmanuel Bernard + * @author Sanne Grinovero + */ +public class DirectoryProviderHelper { + + private static final Log log = LogFactory.getLog( DirectoryProviderHelper.class ); + private static final String ROOTINDEX_PROP_NAME = "sourceBase"; + private static final String RELATIVEINDEX_PROP_NAME = "source"; + + /** + * Build a directory name out of a root and relative path, guessing the significant part + * and checking for the file availability + * @param directoryProviderName + * @param properties + * @param needWritePermissions when true the directory will be tested for read-write permissions. + * @return + */ + public static File getSourceDirectory( String directoryProviderName, Properties properties, boolean needWritePermissions ) { + String root = properties.getProperty( ROOTINDEX_PROP_NAME ); + String relative = properties.getProperty( RELATIVEINDEX_PROP_NAME ); + File sourceDirectory; + if ( log.isTraceEnabled() ) { + log.trace( + "Guess source directory from " + ROOTINDEX_PROP_NAME + " " + root != null ? root : "<null>" + + " and " + RELATIVEINDEX_PROP_NAME + " " + relative != null ? relative : "<null>" + ); + } + if ( relative == null ) relative = directoryProviderName; + if ( StringHelper.isEmpty( root ) ) { + log.debug( "No root directory, go with relative " + relative ); + sourceDirectory = new File( relative ); + if ( ! sourceDirectory.isDirectory() ) { // this also tests for existence + throw new SearchException( "Unable to read source directory: " + relative ); + } + //else keep source as it + } + else { + File rootDir = new File( root ); + makeSanityCheckedDirectory( rootDir, directoryProviderName, needWritePermissions ); + sourceDirectory = new File( root, relative ); + makeSanityCheckedDirectory( sourceDirectory, directoryProviderName, needWritePermissions ); + log.debug( "Get directory from root + relative" ); + } + return sourceDirectory; + } + + /** + * Creates an FSDirectory in provided directory if not already existing. + * @param indexDir The directory where to write a new index + * @return the created FSDirectory + * @throws IOException + */ + public static FSDirectory createFSIndex(File indexDir) throws IOException { + FSDirectory fsDirectory = FSDirectory.getDirectory( indexDir ); + if ( ! IndexReader.indexExists( fsDirectory ) ) { + log.debug( "Initialize index: '" + indexDir.getAbsolutePath() + "'" ); + IndexWriter iw = new IndexWriter( fsDirectory, new StandardAnalyzer(), true ); + iw.close(); + } + return fsDirectory; + } + + /** + * Verify the index directory exists and is writable, + * or creates it if not existing. + * @param annotatedIndexName The index name declared on the @Indexed annotation + * @param properties The properties may override the indexname. + * @param verifyIsWritable Verify the directory is writable + * @return the File representing the Index Directory + * @throws SearchException + */ + public static File getVerifiedIndexDir(String annotatedIndexName, Properties properties, boolean verifyIsWritable) { + String indexBase = properties.getProperty( "indexBase", "." ); + String indexName = properties.getProperty( "indexName", annotatedIndexName ); + File baseIndexDir = new File( indexBase ); + makeSanityCheckedDirectory( baseIndexDir, indexName, verifyIsWritable ); + File indexDir = new File( baseIndexDir, indexName ); + makeSanityCheckedDirectory( indexDir, indexName, verifyIsWritable ); + return indexDir; + } + + /** + * @param directory The directory to create or verify + * @param indexName To label exceptions + * @param verifyIsWritable Verify the directory is writable + * @throws SearchException + */ + private static void makeSanityCheckedDirectory(File directory, String indexName, boolean verifyIsWritable) { + if ( ! directory.exists() ) { + log.warn( "Index directory not found, creating: '" + directory.getAbsolutePath() + "'" ); + //if not existing, create the full path + if ( ! directory.mkdirs() ) { + throw new SearchException( "Unable to create index directory: " + + directory.getAbsolutePath() + " for index " + + indexName ); + } + } + else { + // else check it is not a file + if ( ! directory.isDirectory() ) { + throw new SearchException( "Unable to initialize index: " + + indexName + ": " + + directory.getAbsolutePath() + " is a file." ); + } + } + // and ensure it's writable + if ( verifyIsWritable && ( ! directory.canWrite() ) ) { + throw new SearchException( "Cannot write into index directory: " + + directory.getAbsolutePath() + " for index " + + indexName ); + } + } + + static long getRefreshPeriod(Properties properties, String directoryProviderName) { + String refreshPeriod = properties.getProperty( "refresh", "3600" ); + long period; + try { + period = Long.parseLong( refreshPeriod ); + } catch (NumberFormatException nfe) { + throw new SearchException( "Unable to initialize index: " + directoryProviderName +"; refresh period is not numeric.", nfe ); + } + log.debug( "Refresh period " + period + " seconds" ); + return period * 1000; //per second + } + +} Modified: search/trunk/src/java/org/hibernate/search/store/FSDirectoryProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/store/FSDirectoryProvider.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/store/FSDirectoryProvider.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -5,13 +5,9 @@ import java.io.IOException; import java.util.Properties; -import org.apache.lucene.analysis.standard.StandardAnalyzer; -import org.apache.lucene.index.IndexWriter; import org.apache.lucene.store.FSDirectory; -import org.apache.commons.logging.LogFactory; -import org.apache.commons.logging.Log; -import org.hibernate.HibernateException; -import org.hibernate.search.util.DirectoryProviderHelper; +import org.hibernate.search.Environment; +import org.hibernate.search.SearchException; import org.hibernate.search.engine.SearchFactoryImplementor; /** @@ -21,31 +17,24 @@ * * @author Emmanuel Bernard * @author Sylvain Vieujot + * @author Sanne Grinovero */ public class FSDirectoryProvider implements DirectoryProvider<FSDirectory> { - private static Log log = LogFactory.getLog( FSDirectoryProvider.class ); + private FSDirectory directory; private String indexName; public void initialize(String directoryProviderName, Properties properties, SearchFactoryImplementor searchFactoryImplementor) { - File indexDir = DirectoryProviderHelper.determineIndexDir( directoryProviderName, properties ); + // on "manual" indexing skip read-write check on index directory + boolean manual = searchFactoryImplementor.getIndexingStrategy().equals( "manual" ); + File indexDir = DirectoryProviderHelper.getVerifiedIndexDir( directoryProviderName, properties, ! manual ); try { - boolean create = !indexDir.exists(); - if (create) { - log.debug( "index directory not found, creating: '" + indexDir.getAbsolutePath() + "'" ); - indexDir.mkdirs(); - } indexName = indexDir.getCanonicalPath(); - directory = FSDirectory.getDirectory( indexName ); //this is cheap so it's not done in start() - if ( create ) { - log.debug( "Initialize index: '" + indexName + "'" ); - IndexWriter iw = new IndexWriter( directory, new StandardAnalyzer(), create ); - iw.close(); - } + directory = DirectoryProviderHelper.createFSIndex( indexDir ); } catch (IOException e) { - throw new HibernateException( "Unable to initialize index: " + directoryProviderName, e ); + throw new SearchException( "Unable to initialize index: " + directoryProviderName, e ); } } Modified: search/trunk/src/java/org/hibernate/search/store/FSMasterDirectoryProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/store/FSMasterDirectoryProvider.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/store/FSMasterDirectoryProvider.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -11,14 +11,11 @@ import java.io.IOException; import org.apache.lucene.store.FSDirectory; -import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.hibernate.search.util.DirectoryProviderHelper; +import org.hibernate.search.SearchException; import org.hibernate.search.util.FileHelper; import org.hibernate.search.engine.SearchFactoryImplementor; -import org.hibernate.HibernateException; /** * File based DirectoryProvider that takes care of index copy @@ -29,10 +26,13 @@ * A copy is triggered every refresh seconds * * @author Emmanuel Bernard + * @author Sanne Grinovero */ //TODO rename copy? public class FSMasterDirectoryProvider implements DirectoryProvider<FSDirectory> { + private static Log log = LogFactory.getLog( FSMasterDirectoryProvider.class ); + private FSDirectory directory; private int current; private String indexName; @@ -40,54 +40,37 @@ private SearchFactoryImplementor searchFactory; //variables needed between initialize and start - private String source; + private File sourceDir; private File indexDir; private String directoryProviderName; private Properties properties; - public void initialize(String directoryProviderName, Properties properties, SearchFactoryImplementor searchFactoryImplementor) { this.properties = properties; this.directoryProviderName = directoryProviderName; //source guessing - source = DirectoryProviderHelper.getSourceDirectory( "sourceBase", "source", directoryProviderName, properties ); - if ( source == null) - throw new IllegalStateException("FSMasterDirectoryProvider requires a viable source directory"); - log.debug( "Source directory: " + source ); - indexDir = DirectoryProviderHelper.determineIndexDir( directoryProviderName, properties ); - log.debug( "Index directory: " + indexDir ); + sourceDir = DirectoryProviderHelper.getSourceDirectory( directoryProviderName, properties, true ); + log.debug( "Source directory: " + sourceDir.getPath() ); + indexDir = DirectoryProviderHelper.getVerifiedIndexDir( directoryProviderName, properties, true ); + log.debug( "Index directory: " + indexDir.getPath() ); try { - boolean create = !indexDir.exists(); - if (create) { - log.debug( "index directory not found, creating: '" + indexDir.getAbsolutePath() + "'" ); - indexDir.mkdirs(); - } indexName = indexDir.getCanonicalPath(); - directory = FSDirectory.getDirectory( indexName); - if ( create ) { - log.debug( "Initialize index: '" + indexName + "'" ); - IndexWriter iw = new IndexWriter( directory, new StandardAnalyzer(), create ); - iw.close(); - } + directory = DirectoryProviderHelper.createFSIndex( indexDir ); } catch (IOException e) { - throw new HibernateException( "Unable to initialize index: " + directoryProviderName, e ); + throw new SearchException( "Unable to initialize index: " + directoryProviderName, e ); } this.searchFactory = searchFactoryImplementor; } public void start() { - //source guessing - String refreshPeriod = properties.getProperty( "refresh", "3600" ); - long period = Long.parseLong( refreshPeriod ); - log.debug("Refresh period " + period + " seconds"); - period *= 1000; //per second + long period = DirectoryProviderHelper.getRefreshPeriod( properties, directoryProviderName ); try { //copy to source - if ( new File( source, "current1").exists() ) { + if ( new File( sourceDir, "current1").exists() ) { current = 2; } - else if ( new File( source, "current2").exists() ) { + else if ( new File( sourceDir, "current2").exists() ) { current = 1; } else { @@ -95,19 +78,19 @@ current = 1; } String currentString = Integer.valueOf( current ).toString(); - File subDir = new File( source, currentString ); + File subDir = new File( sourceDir, currentString ); FileHelper.synchronize( indexDir, subDir, true ); - new File( source, "current1").delete(); - new File( source, "current2").delete(); + new File( sourceDir, "current1 ").delete(); + new File( sourceDir, "current2" ).delete(); //TODO small hole, no file can be found here - new File( source, "current" + currentString).createNewFile(); - log.debug( "Current directory: " + current); + new File( sourceDir, "current" + currentString ).createNewFile(); + log.debug( "Current directory: " + current ); } catch (IOException e) { - throw new HibernateException( "Unable to initialize index: " + directoryProviderName, e ); + throw new SearchException( "Unable to initialize index: " + directoryProviderName, e ); } - timer = new Timer(true); //daemon thread, the copy algorithm is robust - TimerTask task = new FSMasterDirectoryProvider.TriggerTask(indexName, source, this ); + timer = new Timer( true ); //daemon thread, the copy algorithm is robust + TimerTask task = new FSMasterDirectoryProvider.TriggerTask( indexDir, sourceDir, this ); timer.scheduleAtFixedRate( task, period, period ); } @@ -136,10 +119,10 @@ class TriggerTask extends TimerTask { - private ExecutorService executor; - private FSMasterDirectoryProvider.CopyDirectory copyTask; + private final ExecutorService executor; + private final FSMasterDirectoryProvider.CopyDirectory copyTask; - public TriggerTask(String source, String destination, DirectoryProvider directoryProvider) { + public TriggerTask(File source, File destination, DirectoryProvider directoryProvider) { executor = Executors.newSingleThreadExecutor(); copyTask = new FSMasterDirectoryProvider.CopyDirectory( source, destination, directoryProvider ); } @@ -149,19 +132,19 @@ executor.execute( copyTask ); } else { - log.info( "Skipping directory synchronization, previous work still in progress: " + indexName); + log.info( "Skipping directory synchronization, previous work still in progress: " + indexName ); } } } class CopyDirectory implements Runnable { - private String source; - private String destination; + private final File source; + private final File destination; private volatile boolean inProgress; private Lock directoryProviderLock; private DirectoryProvider directoryProvider; - public CopyDirectory(String source, String destination, DirectoryProvider directoryProvider) { + public CopyDirectory(File source, File destination, DirectoryProvider directoryProvider) { this.source = source; this.destination = destination; this.directoryProvider = directoryProvider; @@ -171,7 +154,7 @@ //TODO get rid of current and use the marker file instead? long start = System.currentTimeMillis(); inProgress = true; - if (directoryProviderLock == null) { + if ( directoryProviderLock == null ) { directoryProviderLock = searchFactory.getLockableDirectoryProviders().get( directoryProvider ); directoryProvider = null; searchFactory = null; //get rid of any useless link (help hot redeployment?) @@ -180,26 +163,25 @@ directoryProviderLock.lock(); int oldIndex = current; int index = current == 1 ? 2 : 1; - File sourceFile = new File(source); File destinationFile = new File(destination, Integer.valueOf(index).toString() ); //TODO make smart a parameter try { - log.trace("Copying " + sourceFile + " into " + destinationFile); - FileHelper.synchronize( sourceFile, destinationFile, true); + log.trace( "Copying " + source + " into " + destinationFile ); + FileHelper.synchronize( source, destinationFile, true ); current = index; } catch (IOException e) { //don't change current - log.error( "Unable to synchronize source of " + indexName, e); + log.error( "Unable to synchronize source of " + indexName, e ); inProgress = false; return; } - if ( ! new File(destination, "current" + oldIndex).delete() ) { + if ( ! new File( destination, "current" + oldIndex ).delete() ) { log.warn( "Unable to remove previous marker file from source of " + indexName ); } try { - new File(destination, "current" + index).createNewFile(); + new File( destination, "current" + index ).createNewFile(); } catch( IOException e ) { log.warn( "Unable to create current marker in source of " + indexName, e ); @@ -209,7 +191,7 @@ directoryProviderLock.unlock(); inProgress = false; } - log.trace( "Copy for " + indexName + " took " + (System.currentTimeMillis() - start) + " ms"); + log.trace( "Copy for " + indexName + " took " + (System.currentTimeMillis() - start) + " ms" ); } } Modified: search/trunk/src/java/org/hibernate/search/store/FSSlaveDirectoryProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/store/FSSlaveDirectoryProvider.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/store/FSSlaveDirectoryProvider.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -10,14 +10,11 @@ import java.io.IOException; import org.apache.lucene.store.FSDirectory; -import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.hibernate.HibernateException; import org.hibernate.AssertionFailure; +import org.hibernate.search.SearchException; import org.hibernate.search.util.FileHelper; -import org.hibernate.search.util.DirectoryProviderHelper; import org.hibernate.search.engine.SearchFactoryImplementor; /** @@ -30,9 +27,12 @@ * A copy is triggered every refresh seconds * * @author Emmanuel Bernard + * @author Sanne Grinovero */ public class FSSlaveDirectoryProvider implements DirectoryProvider<FSDirectory> { + private static Log log = LogFactory.getLog( FSSlaveDirectoryProvider.class ); + private FSDirectory directory1; private FSDirectory directory2; private int current; @@ -40,7 +40,7 @@ private Timer timer; //variables needed between initialize and start - private String source; + private File sourceIndexDir; private File indexDir; private String directoryProviderName; private Properties properties; @@ -49,58 +49,33 @@ this.properties = properties; this.directoryProviderName = directoryProviderName; //source guessing - source = DirectoryProviderHelper.getSourceDirectory( "sourceBase", "source", directoryProviderName, properties ); - if (source == null) - throw new IllegalStateException("FSSlaveDirectoryProvider requires a viable source directory"); - if ( ! new File(source, "current1").exists() && ! new File(source, "current2").exists() ) { - throw new IllegalStateException("No current marker in source directory"); + sourceIndexDir = DirectoryProviderHelper.getSourceDirectory( directoryProviderName, properties, false ); + if ( ! new File( sourceIndexDir, "current1" ).exists() && ! new File( sourceIndexDir, "current2" ).exists() ) { + throw new IllegalStateException( "No current marker in source directory" ); } - log.debug( "Source directory: " + source ); - indexDir = DirectoryProviderHelper.determineIndexDir( directoryProviderName, properties ); + log.debug( "Source directory: " + sourceIndexDir.getPath() ); + indexDir = DirectoryProviderHelper.getVerifiedIndexDir( directoryProviderName, properties, true ); log.debug( "Index directory: " + indexDir.getPath() ); try { - boolean create = !indexDir.exists(); - if (create) { - log.debug( "index directory not found, creating: '" + indexDir.getAbsolutePath() + "'" ); - indexDir.mkdirs(); - } indexName = indexDir.getCanonicalPath(); } catch (IOException e) { - throw new HibernateException( "Unable to initialize index: " + directoryProviderName, e ); + throw new SearchException( "Unable to initialize index: " + directoryProviderName, e ); } } public void start() { - //source guessing - String refreshPeriod = properties.getProperty( "refresh", "3600" ); - long period = Long.parseLong( refreshPeriod ); - log.debug("Refresh period " + period + " seconds"); - period *= 1000; //per second + long period = DirectoryProviderHelper.getRefreshPeriod( properties, directoryProviderName ); try { - boolean create; - - File subDir = new File( indexName, "1" ); - create = ! subDir.exists(); - directory1 = FSDirectory.getDirectory( subDir.getCanonicalPath()); - if ( create ) { - log.debug( "Initialize index: '" + subDir.getAbsolutePath() + "'" ); - IndexWriter iw = new IndexWriter( directory1, new StandardAnalyzer(), create ); - iw.close(); - } - - subDir = new File( indexName, "2" ); - create = ! subDir.exists(); - directory2 = FSDirectory.getDirectory( subDir.getCanonicalPath()); - if ( create ) { - log.debug( "Initialize index: '" + subDir.getAbsolutePath() + "'" ); - IndexWriter iw = new IndexWriter( directory2, new StandardAnalyzer(), create ); - iw.close(); - } - File currentMarker = new File(indexName, "current1"); - File current2Marker = new File(indexName, "current2"); + directory1 = DirectoryProviderHelper.createFSIndex( new File(indexDir, "1") ); + directory2 = DirectoryProviderHelper.createFSIndex( new File(indexDir, "2") ); + File currentMarker = new File( indexDir, "current1" ); + File current2Marker = new File( indexDir, "current2" ); if ( currentMarker.exists() ) { current = 1; + if ( current2Marker.exists() ) { + current2Marker.delete(); //TODO or throw an exception? + } } else if ( current2Marker.exists() ) { current = 2; @@ -109,38 +84,38 @@ //no default log.debug( "Setting directory 1 as current"); current = 1; - File sourceFile = new File(source); - File destinationFile = new File(indexName, Integer.valueOf(current).toString() ); + File destinationFile = new File( indexDir, Integer.valueOf( current ).toString() ); int sourceCurrent; - if ( new File(sourceFile, "current1").exists() ) { + if ( new File( sourceIndexDir, "current1").exists() ) { sourceCurrent = 1; } - else if ( new File(sourceFile, "current2").exists() ) { + else if ( new File( sourceIndexDir, "current2").exists() ) { sourceCurrent = 2; } else { - throw new AssertionFailure("No current file marker found in source directory: " + source); + throw new AssertionFailure( "No current file marker found in source directory: " + sourceIndexDir.getPath() ); } try { - FileHelper.synchronize( new File(sourceFile, String.valueOf(sourceCurrent) ), destinationFile, true); + FileHelper.synchronize( new File( sourceIndexDir, String.valueOf(sourceCurrent) ), destinationFile, true); } catch (IOException e) { - throw new HibernateException("Umable to synchonize directory: " + indexName, e); + throw new SearchException( "Unable to synchronize directory: " + indexName, e ); } if (! currentMarker.createNewFile() ) { - throw new HibernateException("Unable to create the directory marker file: " + indexName); + throw new SearchException( "Unable to create the directory marker file: " + indexName ); } } log.debug( "Current directory: " + current); } catch (IOException e) { - throw new HibernateException( "Unable to initialize index: " + directoryProviderName, e ); + throw new SearchException( "Unable to initialize index: " + directoryProviderName, e ); } timer = new Timer(true); //daemon thread, the copy algorithm is robust - TimerTask task = new TriggerTask(source, indexName); + TimerTask task = new TriggerTask( sourceIndexDir, indexDir ); timer.scheduleAtFixedRate( task, period, period ); } + //FIXME this is Thread-Unsafe! A memory barrier is missing. public FSDirectory getDirectory() { if (current == 1) { return directory1; @@ -149,7 +124,7 @@ return directory2; } else { - throw new AssertionFailure("Illegal current directory: " + current); + throw new AssertionFailure( "Illegal current directory: " + current ); } } @@ -174,12 +149,12 @@ class TriggerTask extends TimerTask { - private ExecutorService executor; - private CopyDirectory copyTask; + private final ExecutorService executor; + private final CopyDirectory copyTask; - public TriggerTask(String source, String destination) { + public TriggerTask(File sourceIndexDir, File destination) { executor = Executors.newSingleThreadExecutor(); - copyTask = new CopyDirectory( source, destination ); + copyTask = new CopyDirectory( sourceIndexDir, destination ); } public void run() { @@ -193,12 +168,12 @@ } class CopyDirectory implements Runnable { - private String source; - private String destination; + private final File source; + private final File destination; private volatile boolean inProgress; - public CopyDirectory(String source, String destination) { - this.source = source; + public CopyDirectory(File sourceIndexDir, File destination) { + this.source = sourceIndexDir; this.destination = destination; } @@ -216,16 +191,16 @@ sourceFile = new File(source, "2"); } else { - log.error("Unable to determine current in source directory"); + log.error( "Unable to determine current in source directory" ); inProgress = false; return; } - File destinationFile = new File(destination, Integer.valueOf(index).toString() ); + File destinationFile = new File( destination, Integer.valueOf( index ).toString() ); //TODO make smart a parameter try { - log.trace("Copying " + sourceFile + " into " + destinationFile); - FileHelper.synchronize( sourceFile, destinationFile, true); + log.trace( "Copying " + sourceFile + " into " + destinationFile ); + FileHelper.synchronize( sourceFile, destinationFile, true ); current = index; } catch (IOException e) { @@ -234,11 +209,11 @@ inProgress = false; return; } - if ( ! new File(indexName, "current" + oldIndex).delete() ) { + if ( ! new File( indexName, "current" + oldIndex ).delete() ) { log.warn( "Unable to remove previous marker file in " + indexName ); } try { - new File(indexName, "current" + index).createNewFile(); + new File( indexName, "current" + index ).createNewFile(); } catch( IOException e ) { log.warn( "Unable to create current marker file in " + indexName, e ); @@ -247,7 +222,7 @@ finally { inProgress = false; } - log.trace( "Copy for " + indexName + " took " + (System.currentTimeMillis() - start) + " ms"); + log.trace( "Copy for " + indexName + " took " + (System.currentTimeMillis() - start) + " ms" ); } } Deleted: search/trunk/src/java/org/hibernate/search/util/DirectoryProviderHelper.java =================================================================== --- search/trunk/src/java/org/hibernate/search/util/DirectoryProviderHelper.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/java/org/hibernate/search/util/DirectoryProviderHelper.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -1,93 +0,0 @@ -//$Id$ -package org.hibernate.search.util; - -import java.util.Properties; -import java.io.File; -import java.io.IOException; -import java.text.MessageFormat; - -import org.hibernate.HibernateException; -import org.hibernate.AssertionFailure; -import org.hibernate.search.SearchException; -import org.hibernate.annotations.common.util.StringHelper; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -/** - * @author Emmanuel Bernard - */ -public class DirectoryProviderHelper { - private static Log log = LogFactory.getLog( DirectoryProviderHelper.class ); - /** - * Build a directory name out of a root and relative path, guessing the significant part - * and checking for the file availability - * - */ - public static String getSourceDirectory(String rootPropertyName, String relativePropertyName, - String directoryProviderName, Properties properties) { - //TODO check that it's a directory - String root = properties.getProperty( rootPropertyName ); - String relative = properties.getProperty( relativePropertyName ); - if ( log.isTraceEnabled() ) { - log.trace( - "Guess source directory from " + rootPropertyName + " " + root != null ? root : "<null>" - + " and " + relativePropertyName + " " + relative != null ? relative : "<null>" - ); - } - if (relative == null) relative = directoryProviderName; - if ( StringHelper.isEmpty( root ) ) { - log.debug( "No root directory, go with relative " + relative ); - File sourceFile = new File(relative); - if ( ! sourceFile.exists() ) { - throw new SearchException("Unable to read source directory: " + relative); - } - //else keep source as it - } - else { - File rootDir = new File(root); - if ( ! rootDir.exists() ) { - rootDir.mkdirs(); - } - else if ( ! rootDir.isDirectory() ) { - throw new SearchException(rootPropertyName + " is not a directory"); - } - //test it again in case mkdir failed for wrong reasons - if ( rootDir.exists() ) { - File sourceFile = new File(root, relative); - if (! sourceFile.exists() ) sourceFile.mkdirs(); - log.debug( "Get directory from root + relative"); - try { - relative = sourceFile.getCanonicalPath(); - } - catch (IOException e) { - throw new AssertionFailure("Unable to get canonical path: " + root + " + " + relative); - } - } - else { - throw new SearchException(rootPropertyName + " does not exist and cannot be created"); - } - } - return relative; - } - - public static File determineIndexDir(String directoryProviderName, Properties properties) { - String indexBase = properties.getProperty( "indexBase", "." ); - String indexName = properties.getProperty( "indexName", directoryProviderName ); - File indexDir = new File( indexBase ); - if ( ! indexDir.exists() ) { - //if the base directory does not exist, create it - //we do not fear concurrent creation since mkdir does not raise exceptions - indexDir.mkdirs(); - } - else if ( ! indexDir.isDirectory() ) { - throw new SearchException( MessageFormat.format( "Index directory is not a directory: {0}", indexBase ) ); - } - if ( !indexDir.canWrite() ) { - throw new SearchException( "Cannot write into index directory: " - + ( indexDir.isAbsolute() ? indexBase : indexDir.getAbsolutePath() ) ); - } - - indexDir = new File( indexDir, indexName ); - return indexDir; - } -} Modified: search/trunk/src/test/org/hibernate/search/test/FSDirectoryTest.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/FSDirectoryTest.java 2008-04-29 23:46:43 UTC (rev 14611) +++ search/trunk/src/test/org/hibernate/search/test/FSDirectoryTest.java 2008-04-30 00:27:12 UTC (rev 14612) @@ -14,28 +14,29 @@ import org.apache.lucene.search.Hits; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermQuery; import org.hibernate.Session; import org.hibernate.event.PostDeleteEventListener; import org.hibernate.event.PostInsertEventListener; import org.hibernate.event.PostUpdateEventListener; import org.hibernate.search.Environment; import org.hibernate.search.event.FullTextIndexEventListener; +import org.hibernate.search.store.DirectoryProviderHelper; import org.hibernate.search.store.FSDirectoryProvider; -import org.hibernate.search.util.DirectoryProviderHelper; +import org.hibernate.search.util.FileHelper; /** * @author Gavin King */ public class FSDirectoryTest extends SearchTestCase { - protected void setUp() throws Exception { File sub = getBaseIndexDir(); sub.mkdir(); File[] files = sub.listFiles(); for (File file : files) { if ( file.isDirectory() ) { - delete( file ); + FileHelper.delete( file ); } } //super.setUp(); //we need a fresh session factory each time for index set up @@ -51,44 +52,22 @@ protected void tearDown() throws Exception { super.tearDown(); File sub = getBaseIndexDir(); - delete( sub ); + FileHelper.delete( sub ); } - private void delete(File sub) { - if ( sub.isDirectory() ) { - for (File file : sub.listFiles()) { - delete( file ); - } - sub.delete(); - } - else { - sub.delete(); - } - } - - private void recursiveDelete(File f) { - for (File file : f.listFiles()) { - if ( file.isDirectory() ) { - recursiveDelete( file ); - } - } - f.delete(); - } - public void testDirectoryProviderHelperMkdirsGetSource() throws Exception { String root = "./testDir"; String relative = "dir1/dir2/dir3"; Properties properties = new Properties(); - properties.put( "root", root ); - properties.put( "relative", relative ); + properties.put( "sourceBase", root ); + properties.put( "source", relative ); - String rel = DirectoryProviderHelper.getSourceDirectory( "root", "relative", "name", properties ); + File rel = DirectoryProviderHelper.getSourceDirectory( "name", properties, true ); - File f = new File( rel ); - assertTrue( f.exists() ); + assertTrue( rel.exists() ); - recursiveDelete( new File( root ) ); + FileHelper.delete( new File( root ) ); } public void testDirectoryProviderHelperMkdirsDetermineIndex() throws Exception { @@ -99,16 +78,15 @@ properties.put( "indexBase", root ); properties.put( "indexName", relative ); - File f = DirectoryProviderHelper.determineIndexDir( "name", properties ); + File f = DirectoryProviderHelper.getVerifiedIndexDir( "name", properties, true ); assertTrue( new File( root ).exists() ); - recursiveDelete( new File( "./testDir" ) ); + FileHelper.delete( new File( "./testDir" ) ); } public void testEventIntegration() throws Exception { - Session s = getSessions().openSession(); s.getTransaction().begin(); s.persist( @@ -205,7 +183,6 @@ searcher.close(); } - s = getSessions().openSession(); s.getTransaction().begin(); List list = s.createQuery( "from Document" ).list(); @@ -216,6 +193,24 @@ s.close(); } + public void testSearchOnDeletedIndex() throws Exception { + Session s = getSessions().openSession(); + s.getTransaction().begin(); + s.persist( new Document( "Hibernate Search in Action", "", "") ); + s.getTransaction().commit(); + s.close(); + + IndexSearcher searcher = new IndexSearcher( new File( getBaseIndexDir(), "Documents" ).getCanonicalPath() ); + // deleting before search, but after IndexSearcher creation: + // ( fails when deleting -concurrently- to IndexSearcher initialization! ) + FileHelper.delete(getBaseIndexDir()); + TermQuery query = new TermQuery( new Term("title","action") ); + Hits hits = searcher.search( query ); + assertEquals( 1, hits.length() ); + assertEquals( "Hibernate Search in Action", hits.doc( 0 ).get( "title" ) ); + searcher.close(); + } + protected Class[] getMappings() { return new Class[] { Document.class