From 572e8b38e198f85154a211d33008c91d4dfe95ec Mon Sep 17 00:00:00 2001 From: Vikas Saurabh Date: Fri, 8 Jan 2016 05:37:17 +0530 Subject: [PATCH] OAK-3826: Lucene index augmentation doesn't work in Osgi environment OAK-3815 was marked won't fix, so we're back to using bind/unbind in IndexAugmentFactory. Removed tracker usage altogether. Adjusted IndexAugmentorFactoryTest to use MockOsgi. Unfortunately, I couldn't find a way to unbind references - so, that's not covered. --- .../index/lucene/IndexAugmentorFactory.java | 71 ++++++++++++++++++---- .../index/lucene/LuceneIndexProviderService.java | 27 +------- .../index/lucene/IndexAugmentorFactoryTest.java | 68 ++++++++++++++++----- .../index/lucene/LuceneIndexAugmentTest.java | 32 +++++----- 4 files changed, 132 insertions(+), 66 deletions(-) diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactory.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactory.java index 96373d2..4fe2a73 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactory.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactory.java @@ -20,11 +20,17 @@ import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Deactivate; +import org.apache.felix.scr.annotations.Reference; +import org.apache.felix.scr.annotations.ReferenceCardinality; +import org.apache.felix.scr.annotations.ReferencePolicy; +import org.apache.felix.scr.annotations.References; +import org.apache.felix.scr.annotations.Service; import org.apache.jackrabbit.oak.plugins.index.lucene.spi.FulltextQueryTermsProvider; import org.apache.jackrabbit.oak.plugins.index.lucene.spi.IndexFieldProvider; import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.apache.jackrabbit.oak.spi.whiteboard.Tracker; -import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; import org.apache.jackrabbit.oak.util.PerfLogger; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.document.Field; @@ -39,25 +45,51 @@ import java.util.List; import java.util.Map; import java.util.Set; +@SuppressWarnings("UnusedDeclaration") +@Component(metatype = true, label = "Apache Jackrabbit Oak IndexAugmentorFactory") +@Service(value = IndexAugmentorFactory.class) +@References({ + @Reference(name = "IndexFieldProvider", + policy = ReferencePolicy.DYNAMIC, + cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, + referenceInterface = IndexFieldProvider.class, + bind = "bindIndexFieldProviderService", + unbind = "unbindIndexFieldProviderService"), + @Reference(name = "FulltextQueryTermsProvider", + policy = ReferencePolicy.DYNAMIC, + cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, + referenceInterface = FulltextQueryTermsProvider.class, + bind = "bindFulltextQueryTermsProviderService", + unbind = "unbindFulltextQueryTermsProviderService") +}) public class IndexAugmentorFactory { private static final PerfLogger PERFLOG = new PerfLogger( LoggerFactory.getLogger(IndexAugmentorFactory.class.getName() + ".perf")); - private final Tracker indexFieldProviderTracker; - private final Tracker fulltextQueryTermsProviderTracker; + final Set indexFieldProviderSet; + final Set fulltextQueryTermsProviderSet; - private volatile Map indexFieldProviderMap; - private volatile Map fulltextQueryTermsProviderMap; + volatile Map indexFieldProviderMap; + volatile Map fulltextQueryTermsProviderMap; - public IndexAugmentorFactory(Whiteboard whiteboard) { - indexFieldProviderTracker = whiteboard.track(IndexFieldProvider.class); - fulltextQueryTermsProviderTracker = whiteboard.track(FulltextQueryTermsProvider.class); + public IndexAugmentorFactory() { + indexFieldProviderSet = Sets.newHashSet(); + fulltextQueryTermsProviderSet = Sets.newHashSet(); indexFieldProviderMap = Maps.newHashMap(); fulltextQueryTermsProviderMap = Maps.newHashMap(); } + @Deactivate + private synchronized void deactivate() { + fulltextQueryTermsProviderMap.clear(); + indexFieldProviderMap.clear(); + + fulltextQueryTermsProviderSet.clear(); + indexFieldProviderSet.clear(); + } + @Nonnull public IndexFieldProvider getIndexFieldProvider(String nodeType) { IndexFieldProvider provider = indexFieldProviderMap.get(nodeType); @@ -70,15 +102,30 @@ public class IndexAugmentorFactory { return (provider != null) ? provider : FulltextQueryTermsProvider.DEFAULT; } - public void refreshServices() { + synchronized void bindIndexFieldProviderService(IndexFieldProvider indexFieldProvider) { + indexFieldProviderSet.add(indexFieldProvider); + refreshIndexFieldProviderServices(); + } + + synchronized void unbindIndexFieldProviderService(IndexFieldProvider indexFieldProvider) { + indexFieldProviderSet.remove(indexFieldProvider); refreshIndexFieldProviderServices(); + } + + synchronized void bindFulltextQueryTermsProviderService(FulltextQueryTermsProvider fulltextQueryTermsProvider) { + fulltextQueryTermsProviderSet.add(fulltextQueryTermsProvider); + refreshFulltextQueryTermsProviderServices(); + } + + synchronized void unbindFulltextQueryTermsProviderService(FulltextQueryTermsProvider fulltextQueryTermsProvider) { + fulltextQueryTermsProviderSet.remove(fulltextQueryTermsProvider); refreshFulltextQueryTermsProviderServices(); } private void refreshIndexFieldProviderServices() { ListMultimap indexFieldProviderListMultimap = LinkedListMultimap.create(); - for (IndexFieldProvider provider : indexFieldProviderTracker.getServices()) { + for (IndexFieldProvider provider : indexFieldProviderSet) { Set supportedNodeTypes = provider.getSupportedTypes(); for (String nodeType : supportedNodeTypes) { indexFieldProviderListMultimap.put(nodeType, provider); @@ -99,7 +146,7 @@ public class IndexAugmentorFactory { private void refreshFulltextQueryTermsProviderServices() { ListMultimap fulltextQueryTermsProviderLinkedListMultimap = LinkedListMultimap.create(); - for (FulltextQueryTermsProvider provider : fulltextQueryTermsProviderTracker.getServices()) { + for (FulltextQueryTermsProvider provider : fulltextQueryTermsProviderSet) { Set supportedNodeTypes = provider.getSupportedTypes(); for (String nodeType : supportedNodeTypes) { fulltextQueryTermsProviderLinkedListMultimap.put(nodeType, provider); diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java index c29981c..bebc993 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java @@ -45,7 +45,6 @@ import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; import org.apache.felix.scr.annotations.ReferencePolicy; import org.apache.felix.scr.annotations.ReferencePolicyOption; -import org.apache.felix.scr.annotations.References; import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean; import org.apache.jackrabbit.oak.cache.CacheStats; import org.apache.jackrabbit.oak.commons.PropertiesUtil; @@ -53,8 +52,6 @@ import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard; import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider; import org.apache.jackrabbit.oak.plugins.index.aggregate.NodeAggregator; import org.apache.jackrabbit.oak.plugins.index.fulltext.PreExtractedTextProvider; -import org.apache.jackrabbit.oak.plugins.index.lucene.spi.FulltextQueryTermsProvider; -import org.apache.jackrabbit.oak.plugins.index.lucene.spi.IndexFieldProvider; import org.apache.jackrabbit.oak.spi.commit.BackgroundObserver; import org.apache.jackrabbit.oak.plugins.index.lucene.score.ScorerProviderFactory; import org.apache.jackrabbit.oak.spi.commit.BackgroundObserverMBean; @@ -77,20 +74,6 @@ import static org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.registerM @SuppressWarnings("UnusedDeclaration") @Component(metatype = true, label = "Apache Jackrabbit Oak LuceneIndexProvider") -@References({ - @Reference(name = "IndexFieldProvider", - policy = ReferencePolicy.DYNAMIC, - cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, - referenceInterface = IndexFieldProvider.class, - bind = "indexFieldProviderServiceUpdated", - unbind = "indexFieldProviderServiceUpdated"), - @Reference(name = "FulltextQueryTermsProvider", - policy = ReferencePolicy.DYNAMIC, - cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, - referenceInterface = FulltextQueryTermsProvider.class, - bind = "indexFulltextQueryTermsProviderServiceUpdated", - unbind = "indexFulltextQueryTermsProviderServiceUpdated") -}) public class LuceneIndexProviderService { public static final String REPOSITORY_HOME = "repository.home"; @@ -194,6 +177,7 @@ public class LuceneIndexProviderService { @Reference ScorerProviderFactory scorerFactory; + @Reference private IndexAugmentorFactory augmentorFactory; @Reference(policy = ReferencePolicy.DYNAMIC, @@ -226,7 +210,6 @@ public class LuceneIndexProviderService { whiteboard = new OsgiWhiteboard(bundleContext); threadPoolSize = PropertiesUtil.toInteger(config.get(PROP_THREAD_POOL_SIZE), PROP_THREAD_POOL_SIZE_DEFAULT); initializeExtractedTextCache(bundleContext, config); - augmentorFactory = new IndexAugmentorFactory(whiteboard); indexProvider = new LuceneIndexProvider(createTracker(bundleContext, config), scorerFactory, augmentorFactory); initializeLogging(config); initialize(); @@ -490,12 +473,4 @@ public class LuceneIndexProviderService { this.extractedTextProvider = null; registerExtractedTextProvider(null); } - - private void indexFieldProviderServiceUpdated(IndexFieldProvider indexFieldProvider) { - augmentorFactory.refreshServices(); - } - - private void indexFulltextQueryTermsProviderServiceUpdated(FulltextQueryTermsProvider fulltextQueryTermsProvider) { - augmentorFactory.refreshServices(); - } } diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactoryTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactoryTest.java index 7e20e9a..1b26bed 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactoryTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexAugmentorFactoryTest.java @@ -22,11 +22,12 @@ package org.apache.jackrabbit.oak.plugins.index.lucene; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Service; +import org.apache.jackrabbit.oak.plugins.index.lucene.score.impl.ScorerProviderFactoryImpl; import org.apache.jackrabbit.oak.plugins.index.lucene.spi.FulltextQueryTermsProvider; import org.apache.jackrabbit.oak.plugins.index.lucene.spi.IndexFieldProvider; import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.apache.jackrabbit.oak.spi.whiteboard.DefaultWhiteboard; -import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.document.Field; import org.apache.lucene.document.StringField; @@ -35,26 +36,28 @@ import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; +import org.apache.sling.testing.mock.osgi.MockOsgi; +import org.apache.sling.testing.mock.osgi.junit.OsgiContext; import org.hamcrest.CoreMatchers; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import javax.annotation.Nonnull; +import java.util.Collections; import java.util.List; import java.util.Set; import static org.apache.lucene.search.BooleanClause.Occur.SHOULD; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; public class IndexAugmentorFactoryTest { - private IndexAugmentorFactory indexAugmentorFactory; - private Whiteboard whiteboard = new DefaultWhiteboard(); + private IndexAugmentorFactory indexAugmentorFactory = new IndexAugmentorFactory(); - @Before - public void initializeFactory() { - indexAugmentorFactory = new IndexAugmentorFactory(whiteboard); - } + @Rule + public final OsgiContext context = new OsgiContext(); @Test public void compositeIndexProvider() @@ -64,15 +67,23 @@ public class IndexAugmentorFactoryTest { final String typeC = "type:C"; final String typeD = "type:D"; + context.registerInjectActivateService(indexAugmentorFactory); + new IdentifiableIndexFiledProvider("1", Sets.newHashSet(typeA, typeB)); new IdentifiableIndexFiledProvider("2", Sets.newHashSet(typeC)); new IdentifiableIndexFiledProvider("3", Sets.newHashSet(typeA, typeB)); - indexAugmentorFactory.refreshServices(); + //register an instance which would be unregistered before validation + IndexFieldProvider unreg = new IdentifiableIndexFiledProvider("4", Sets.newHashSet(typeD)); + indexAugmentorFactory.unbindIndexFieldProviderService(unreg); validateComposedFields(typeA, "1", "3"); validateComposedFields(typeC, "2"); validateComposedFields(typeD); + + MockOsgi.deactivate(indexAugmentorFactory, context.bundleContext(), Collections.EMPTY_MAP); + + validateDeactivatedService(); } @Test @@ -84,21 +95,35 @@ public class IndexAugmentorFactoryTest { final String typeD = "type:D"; final String typeE = "type:E"; + context.registerInjectActivateService(indexAugmentorFactory); + new IdentifiableQueryTermsProvider("1", Sets.newHashSet(typeA, typeB)); new IdentifiableQueryTermsProvider("2", Sets.newHashSet(typeC)); new IdentifiableQueryTermsProvider("3", Sets.newHashSet(typeA, typeB)); new IdentifiableQueryTermsProvider(null, Sets.newHashSet(typeE)); - indexAugmentorFactory.refreshServices(); + //register an instance which would be unregistered before validation + FulltextQueryTermsProvider unreg = new IdentifiableQueryTermsProvider("4", Sets.newHashSet(typeD)); + indexAugmentorFactory.unbindFulltextQueryTermsProviderService(unreg); validateComposedQueryTerms(typeA, "1", "3"); validateComposedQueryTerms(typeC, "2"); validateComposedQueryTerms(typeD); validateComposedQueryTerms(typeE); + + MockOsgi.deactivate(indexAugmentorFactory, context.bundleContext(), Collections.EMPTY_MAP); + + validateDeactivatedService(); } void validateComposedFields(String type, String ... expected) { IndexFieldProvider compositeIndexProvider = indexAugmentorFactory.getIndexFieldProvider(type); + + if (expected.length > 0) { + assertTrue("Composed index field provider doesn't declare correct supported type", + compositeIndexProvider.getSupportedTypes().contains(type)); + } + Iterable fields = compositeIndexProvider.getAugmentedFields(null, null, null); Set ids = Sets.newHashSet(); for (Field f : fields) { @@ -111,6 +136,12 @@ public class IndexAugmentorFactoryTest { void validateComposedQueryTerms(String type, String ... expected) { FulltextQueryTermsProvider compositeQueryTermsProvider = indexAugmentorFactory.getFulltextQueryTermsProvider(type); + + if (expected.length > 0) { + assertTrue("Composed query terms provider doesn't declare correct supported type", + compositeQueryTermsProvider.getSupportedTypes().contains(type)); + } + Query q = compositeQueryTermsProvider.getQueryTerm(null, null, null); if (q == null) { assertEquals("No query terms generated for " + type + ".", 0, expected.length); @@ -136,6 +167,17 @@ public class IndexAugmentorFactoryTest { } } + private void validateDeactivatedService() { + assertEquals("indexFieldProviderSet should be empty after deactivation", + 0, indexAugmentorFactory.indexFieldProviderSet.size()); + assertEquals("fulltextQueryTermsProviderSet should be empty after deactivation", + 0, indexAugmentorFactory.fulltextQueryTermsProviderSet.size()); + assertEquals("indexFieldProviderMap should be empty after deactivation", + 0, indexAugmentorFactory.indexFieldProviderMap.size()); + assertEquals("fulltextQueryTermsProviderMap should be empty after deactivation", + 0, indexAugmentorFactory.fulltextQueryTermsProviderMap.size()); + } + class IdentifiableIndexFiledProvider implements IndexFieldProvider { private final Field id; private final Set nodeTypes; @@ -143,8 +185,7 @@ public class IndexAugmentorFactoryTest { IdentifiableIndexFiledProvider(String id, Set nodeTypes) { this.id = new StringField("id", id, Field.Store.NO); this.nodeTypes = nodeTypes; - - whiteboard.register(IndexFieldProvider.class, this, null); + context.registerService(IndexFieldProvider.class, this); } @Nonnull @@ -167,8 +208,7 @@ public class IndexAugmentorFactoryTest { IdentifiableQueryTermsProvider(String id, Set nodeTypes) { this.id = (id == null)?null:new TermQuery(new Term(id, "1")); this.nodeTypes = nodeTypes; - - whiteboard.register(FulltextQueryTermsProvider.class, this, null); + context.registerService(FulltextQueryTermsProvider.class, this); } @Override diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAugmentTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAugmentTest.java index 741126a..fe0da74 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAugmentTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexAugmentTest.java @@ -319,6 +319,22 @@ public class LuceneIndexAugmentTest extends AbstractQueryTest { } }; checkSimpleBehavior(rootTree, testIndex++); + + //setup composite query term provider with one returning null query + factory.registerQueryTermsProvider(new FulltextQueryTermsProvider() { + @Override + public Query getQueryTerm(String text, Analyzer analyzer, NodeState indexDefinition) { + return null; + } + + @Nonnull + @Override + public Set getSupportedTypes() { + return Collections.singleton(TestUtil.NT_TEST); + } + }); + factory.useSuperBehavior = true; + checkSimpleBehavior(rootTree, testIndex++); } //OAK-3576 @@ -642,25 +658,13 @@ public class LuceneIndexAugmentTest extends AbstractQueryTest { IndexFieldProvider indexFieldProvider = null; FulltextQueryTermsProvider fulltextQueryTermsProvider = null; private boolean useSuperBehavior = false; - private final Whiteboard whiteboard; - - SimpleIndexAugmentorFactory() { - this(new DefaultWhiteboard()); - } - - SimpleIndexAugmentorFactory(Whiteboard whiteboard) { - super(whiteboard); - this.whiteboard = whiteboard; - } void registerIndexFieldProvider(IndexFieldProvider provider) { - whiteboard.register(IndexFieldProvider.class, provider, null); - refreshServices(); + bindIndexFieldProviderService(provider); } void registerQueryTermsProvider(FulltextQueryTermsProvider provider) { - whiteboard.register(FulltextQueryTermsProvider.class, provider, null); - refreshServices(); + bindFulltextQueryTermsProviderService(provider); } @Nonnull -- 2.6.2