Author: epbernard Date: 2008-09-09 13:15:44 -0400 (Tue, 09 Sep 2008) New Revision: 15160 Added: search/trunk/src/java/org/hibernate/search/annotations/FilterCacheModeType.java search/trunk/src/java/org/hibernate/search/util/FilterCacheModeTypeHelper.java search/trunk/src/test/org/hibernate/search/test/filter/InstanceBasedExcludeAllFilter.java Modified: search/trunk/src/java/org/hibernate/search/annotations/FullTextFilterDef.java search/trunk/src/java/org/hibernate/search/engine/FilterDef.java search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java search/trunk/src/test/org/hibernate/search/test/filter/Driver.java search/trunk/src/test/org/hibernate/search/test/filter/ExcludeAllFilter.java search/trunk/src/test/org/hibernate/search/test/filter/FilterTest.java search/trunk/src/test/org/hibernate/search/test/filter/SecurityFilterFactory.java Log: HSEARCH-260 @FulltextFilterDef.cache is now one single paramenter enum based HSEARCH-259 fitlers are now isolated by name in the cache Added: search/trunk/src/java/org/hibernate/search/annotations/FilterCacheModeType.java =================================================================== --- search/trunk/src/java/org/hibernate/search/annotations/FilterCacheModeType.java (rev 0) +++ search/trunk/src/java/org/hibernate/search/annotations/FilterCacheModeType.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -0,0 +1,30 @@ +package org.hibernate.search.annotations; + +/** + * Cache mode strategy for Full Text filters + * + * @author Emmanuel Bernard + */ +public enum FilterCacheModeType { + /** + * NO filter instance and no result is cached by Hibernate Search + * For every filter call, a new filter instance is created + */ + NO, + + /** + * The filter instance is cached by Hibernate Search and reused across + * concurrent filter.bits() calls + * Results are not cache by Hibernate Search + */ + INSTANCE_ONLY, + + /** + * Both the filter instance and the BitSet results are cached. + * The filter instance is cached by Hibernate Search and reused across + * concurrent filter.bits() calls + * BitSet Results are cached per IndexReader + */ + INSTANCE_AND_RESULTS + +} Modified: search/trunk/src/java/org/hibernate/search/annotations/FullTextFilterDef.java =================================================================== --- search/trunk/src/java/org/hibernate/search/annotations/FullTextFilterDef.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/java/org/hibernate/search/annotations/FullTextFilterDef.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -32,16 +32,10 @@ * */ @SuppressWarnings("unchecked") - Class impl(); + Class<?> impl(); /** - * Enable caching for this filter (default true). + * Cache mode for the filter. Default to instance and results caching */ - boolean cache() default true; - - /** - * Determines whether the <code>BitSet</code> returned from the filter should be - * cached or not. Default is <code>CacheBitResults.AUTOMATIC</code>. - */ - CacheBitResults cacheBitResult() default CacheBitResults.AUTOMATIC; + FilterCacheModeType cache() default FilterCacheModeType.INSTANCE_AND_RESULTS; } Modified: search/trunk/src/java/org/hibernate/search/engine/FilterDef.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/FilterDef.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/java/org/hibernate/search/engine/FilterDef.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -8,6 +8,8 @@ import org.hibernate.search.SearchException; import org.hibernate.search.annotations.CacheBitResults; +import org.hibernate.search.annotations.FilterCacheModeType; +import org.hibernate.search.annotations.FullTextFilterDef; /** * A wrapper class which encapsualtes all required information to create a defined filter. @@ -17,28 +19,29 @@ //TODO serialization @SuppressWarnings("unchecked") public class FilterDef { - private Class impl; private Method factoryMethod; private Method keyMethod; private Map<String, Method> setters = new HashMap<String, Method>(); - private boolean cache; - private CacheBitResults useCachingWrapperFilter; + private final FilterCacheModeType cacheMode; + private final Class<?> impl; + private final String name; - public CacheBitResults getUseCachingWrapperFilter() { - return useCachingWrapperFilter; + public FilterDef(FullTextFilterDef def) { + this.name = def.name(); + this.impl = def.impl(); + this.cacheMode = def.cache(); } - public void setUseCachingWrapperFilter( - CacheBitResults useCachingWrapperFilter) { - this.useCachingWrapperFilter = useCachingWrapperFilter; + public String getName() { + return name; } - public Class getImpl() { - return impl; + public FilterCacheModeType getCacheMode() { + return cacheMode; } - public void setImpl(Class impl) { - this.impl = impl; + public Class<?> getImpl() { + return impl; } public Method getFactoryMethod() { @@ -75,12 +78,4 @@ throw new SearchException( "Unable to set Filter parameter: " + parameterName + " on filter class: " + this.impl, e ); } } - - public void setCache(boolean cache) { - this.cache = cache; - } - - public boolean isCache() { - return cache; - } } Modified: search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -18,6 +18,9 @@ import java.util.concurrent.locks.ReentrantLock; import org.apache.lucene.analysis.Analyzer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.hibernate.annotations.common.reflection.ReflectionManager; import org.hibernate.annotations.common.reflection.XClass; import org.hibernate.annotations.common.reflection.java.JavaReflectionManager; @@ -49,9 +52,6 @@ import org.hibernate.search.store.DirectoryProvider; import org.hibernate.search.store.DirectoryProviderFactory; import org.hibernate.search.store.optimization.OptimizerStrategy; -import org.hibernate.search.util.ScopedAnalyzer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * @author Emmanuel Bernard @@ -200,10 +200,8 @@ throw new SearchException("Multiple definition of @FullTextFilterDef.name=" + defAnn.name() + ": " + mappedXClass.getName() ); } - FilterDef filterDef = new FilterDef(); - filterDef.setImpl( defAnn.impl() ); - filterDef.setCache( defAnn.cache() ); - filterDef.setUseCachingWrapperFilter( defAnn.cacheBitResult() ); + + FilterDef filterDef = new FilterDef(defAnn); try { filterDef.getImpl().newInstance(); } Modified: search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -41,7 +41,6 @@ import org.hibernate.search.FullTextFilter; import org.hibernate.search.FullTextQuery; import org.hibernate.search.SearchException; -import org.hibernate.search.annotations.CacheBitResults; import org.hibernate.search.engine.DocumentBuilder; import org.hibernate.search.engine.DocumentExtractor; import org.hibernate.search.engine.EntityInfo; @@ -53,6 +52,7 @@ import org.hibernate.search.engine.SearchFactoryImplementor; import org.hibernate.search.filter.ChainedFilter; import org.hibernate.search.filter.FilterKey; +import org.hibernate.search.filter.StandardFilterKey; import org.hibernate.search.reader.ReaderProvider; import org.hibernate.search.store.DirectoryProvider; import org.hibernate.search.util.ContextHelper; @@ -60,6 +60,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.hibernate.search.util.FilterCacheModeTypeHelper.*; + /** * Implementation of {@link org.hibernate.search.FullTextQuery} * @@ -352,7 +354,7 @@ FilterKey key = createFilterKey(def, instance); // try to get the filter out of the cache - Filter filter = def.isCache() ? + Filter filter = cacheInstance( def.getCacheMode() ) ? searchFactoryImplementor.getFilterCachingStrategy().getCachedFilter( key ) : null; @@ -360,7 +362,7 @@ filter = createFilter(def, instance); // add filter to cache if we have to - if ( def.isCache() ) { + if ( cacheInstance( def.getCacheMode() ) ) { searchFactoryImplementor.getFilterCachingStrategy().addCachedFilter( key, filter ); } } @@ -391,8 +393,9 @@ filter = (Filter) instance; } catch (ClassCastException e) { - throw new SearchException( "@Key method does not return a org.apache.lucene.search.Filter class: " - + def.getImpl().getName() + "." + def.getFactoryMethod().getName() ); + throw new SearchException( "Filter implementation does not implement the Filter interface: " + + def.getImpl().getName() + ". " + + (def.getFactoryMethod() != null ? def.getFactoryMethod().getName() : ""), e ); } } @@ -409,7 +412,7 @@ * <code>def</code>. */ private Filter addCachingWrapperFilter(Filter filter, FilterDef def) { - if (def.getUseCachingWrapperFilter() == CacheBitResults.AUTOMATIC && def.isCache() ) { + if ( cacheResults( def.getCacheMode() ) ) { int cachingWrapperFilterSize = getSearchFactoryImplementor().getFilterCacheBitResultsSize(); filter = new org.hibernate.search.filter.CachingWrapperFilter(filter, cachingWrapperFilterSize); } @@ -419,7 +422,7 @@ private FilterKey createFilterKey(FilterDef def, Object instance) { FilterKey key = null; - if ( !def.isCache() ) { + if ( !cacheInstance( def.getCacheMode() ) ) { return key; // if the filter is not cached there is no key! } @@ -454,7 +457,12 @@ } } key.setImpl( def.getImpl() ); - return key; + + //Make sure Filters are isolated by filter def name + StandardFilterKey wrapperKey = new StandardFilterKey(); + wrapperKey.addParameter( def.getName() ); + wrapperKey.addParameter( key ); + return wrapperKey; } private Object createFilterInstance(FullTextFilterImpl fullTextFilter, @@ -472,7 +480,7 @@ for (Map.Entry<String, Object> entry : fullTextFilter.getParameters().entrySet()) { def.invoke( entry.getKey(), instance, entry.getValue() ); } - if ( def.isCache() && def.getKeyMethod() == null && fullTextFilter.getParameters().size() > 0 ) { + if ( cacheInstance( def.getCacheMode() ) && def.getKeyMethod() == null && fullTextFilter.getParameters().size() > 0 ) { throw new SearchException( "Filter with parameters and no @Key method: " + fullTextFilter.getName() ); } return instance; Added: search/trunk/src/java/org/hibernate/search/util/FilterCacheModeTypeHelper.java =================================================================== --- search/trunk/src/java/org/hibernate/search/util/FilterCacheModeTypeHelper.java (rev 0) +++ search/trunk/src/java/org/hibernate/search/util/FilterCacheModeTypeHelper.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -0,0 +1,37 @@ +package org.hibernate.search.util; + +import org.hibernate.search.annotations.FilterCacheModeType; +import org.hibernate.annotations.common.AssertionFailure; + +/** + * @author Emmanuel Bernard + */ +public class FilterCacheModeTypeHelper { + private FilterCacheModeTypeHelper() {} + + public static boolean cacheInstance(FilterCacheModeType type) { + switch ( type ) { + case NO: + return false; + case INSTANCE_AND_RESULTS: + return true; + case INSTANCE_ONLY: + return true; + default: + throw new AssertionFailure("Unknwn FilterCacheModeType:" + type); + } + } + + public static boolean cacheResults(FilterCacheModeType type) { + switch ( type ) { + case NO: + return false; + case INSTANCE_AND_RESULTS: + return true; + case INSTANCE_ONLY: + return false; + default: + throw new AssertionFailure("Unknwn FilterCacheModeType:" + type); + } + } +} Modified: search/trunk/src/test/org/hibernate/search/test/filter/Driver.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/filter/Driver.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/test/org/hibernate/search/test/filter/Driver.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -13,6 +13,7 @@ import org.hibernate.search.annotations.Resolution; import org.hibernate.search.annotations.FullTextFilterDef; import org.hibernate.search.annotations.FullTextFilterDefs; +import org.hibernate.search.annotations.FilterCacheModeType; /** * @author Emmanuel Bernard @@ -20,9 +21,10 @@ @Entity @Indexed @FullTextFilterDefs( { - @FullTextFilterDef(name = "bestDriver", impl = BestDriversFilter.class), //actual Filter implementation - @FullTextFilterDef(name = "security", impl = SecurityFilterFactory.class), //Filter factory with parameters - @FullTextFilterDef(name = "cachetest", impl = ExcludeAllFilterFactory.class, cache = true) //Filter factory with parameters + @FullTextFilterDef(name = "bestDriver", impl = BestDriversFilter.class, cache = FilterCacheModeType.NO), //actual Filter implementation + @FullTextFilterDef(name = "security", impl = SecurityFilterFactory.class, cache = FilterCacheModeType.INSTANCE_AND_RESULTS), //Filter factory with parameters + @FullTextFilterDef(name = "cacheresultstest", impl = ExcludeAllFilterFactory.class, cache = FilterCacheModeType.INSTANCE_AND_RESULTS), + @FullTextFilterDef(name = "cacheinstancetest", impl = InstanceBasedExcludeAllFilter.class, cache = FilterCacheModeType.INSTANCE_ONLY) }) public class Driver { @Id Modified: search/trunk/src/test/org/hibernate/search/test/filter/ExcludeAllFilter.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/filter/ExcludeAllFilter.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/test/org/hibernate/search/test/filter/ExcludeAllFilter.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -1,4 +1,4 @@ -// $Id:$ +// $Id$ package org.hibernate.search.test.filter; import java.util.BitSet; @@ -12,7 +12,7 @@ */ @SuppressWarnings("serial") public class ExcludeAllFilter extends Filter { - private static boolean done = false; + private static volatile boolean done = false; public BitSet bits(IndexReader reader) throws IOException { if (done) throw new IllegalStateException("Called twice"); Modified: search/trunk/src/test/org/hibernate/search/test/filter/FilterTest.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/filter/FilterTest.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/test/org/hibernate/search/test/filter/FilterTest.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -65,7 +65,7 @@ assertEquals("No filter should happen", 3, ftQuery.getResultSize() ); ftQuery = s.createFullTextQuery( query, Driver.class ); - ftQuery.enableFullTextFilter( "cachetest"); + ftQuery.enableFullTextFilter( "cacheresultstest"); assertEquals("Should filter out all", 0, ftQuery.getResultSize() ); // HSEARCH-174 - we call System.gc() to force a garbage collection. @@ -74,15 +74,33 @@ System.gc(); ftQuery = s.createFullTextQuery( query, Driver.class ); - ftQuery.enableFullTextFilter( "cachetest"); + ftQuery.enableFullTextFilter( "cacheresultstest"); try { ftQuery.getResultSize(); } catch (IllegalStateException e) { - fail("Cache does not work"); + fail("Cache results does not work"); } + ftQuery = s.createFullTextQuery( query, Driver.class ); + ftQuery.enableFullTextFilter( "cacheinstancetest"); + assertEquals("Should filter out all", 0, ftQuery.getResultSize() ); + + ftQuery = s.createFullTextQuery( query, Driver.class ); + ftQuery.enableFullTextFilter( "cacheinstancetest"); + try { + ftQuery.getResultSize(); + fail("Cache instance does not work"); + } + catch (IllegalStateException e) { + //success + } + + s.getTransaction().commit(); + + + s.close(); deleteData(); } Added: search/trunk/src/test/org/hibernate/search/test/filter/InstanceBasedExcludeAllFilter.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/filter/InstanceBasedExcludeAllFilter.java (rev 0) +++ search/trunk/src/test/org/hibernate/search/test/filter/InstanceBasedExcludeAllFilter.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -0,0 +1,21 @@ +package org.hibernate.search.test.filter; + +import java.util.BitSet; +import java.io.IOException; + +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.search.Filter; + +/** + * @author Emmanuel Bernard + */ +public class InstanceBasedExcludeAllFilter extends Filter { + private volatile boolean done = false; + + public BitSet bits(IndexReader reader) throws IOException { + if (done) throw new IllegalStateException("Called twice"); + BitSet bitSet = new BitSet( reader.maxDoc() ); + done = true; + return bitSet; + } +} Modified: search/trunk/src/test/org/hibernate/search/test/filter/SecurityFilterFactory.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/filter/SecurityFilterFactory.java 2008-09-08 19:50:44 UTC (rev 15159) +++ search/trunk/src/test/org/hibernate/search/test/filter/SecurityFilterFactory.java 2008-09-09 17:15:44 UTC (rev 15160) @@ -1,18 +1,17 @@ //$Id$ package org.hibernate.search.test.filter; -import org.hibernate.search.filter.FilterKey; -import org.hibernate.search.filter.StandardFilterKey; -import org.hibernate.search.annotations.Key; -import org.hibernate.search.annotations.Factory; +import org.apache.lucene.index.Term; import org.apache.lucene.search.Filter; -import org.apache.lucene.search.QueryFilter; import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.QueryWrapperFilter; -import org.apache.lucene.search.CachingWrapperFilter; -import org.apache.lucene.index.Term; +import org.apache.lucene.search.TermQuery; +import org.hibernate.search.annotations.Factory; +import org.hibernate.search.annotations.Key; +import org.hibernate.search.filter.FilterKey; +import org.hibernate.search.filter.StandardFilterKey; + /** * Apply a security filter to the results * @@ -38,6 +37,6 @@ @Factory public Filter getFilter() { Query query = new TermQuery( new Term("teacher", login) ); - return new CachingWrapperFilter( new QueryWrapperFilter(query) ); + return new QueryWrapperFilter(query); } }