[ https://issues.apache.org/jira/browse/OAK-3826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15094074#comment-15094074 ]
Chetan Mehrotra commented on OAK-3826: -------------------------------------- Approach looks fine. Some minor comments # Omit metatype - As for now this component does not have any config to be edited {code} +@Component(metatype = true, label = "Apache Jackrabbit Oak IndexAugmentorFactory") {code} # You can omit bind/unbind as the _The default value is the name created by appending the reference name to the string unbind_ {code} + @Reference(name = "IndexFieldProvider", + policy = ReferencePolicy.DYNAMIC, + cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, + referenceInterface = IndexFieldProvider.class, + bind = "bindIndexFieldProviderService", + unbind = "unbindIndexFieldProviderService") {code} # Visibility of state in {{IndexAugmentorFactory}} - Keep them provide. And provide a method to enable assert if they are all reset to empty # Use {{com.google.common.collect.Sets#newIdentityHashSet}} which would be safer and not depend on equals of service impls {code} + public IndexAugmentorFactory() { + indexFieldProviderSet = Sets.newHashSet(); + fulltextQueryTermsProviderSet = Sets.newHashSet(); {code} # Instead of directly assigning {{tempMap}} to {{indexFieldProviderMap}} use {{com.google.common.collect.ImmutableMap#copyOf}}. This provides immutable variant and fast access for single entry setup! {code} Map<String, CompositeIndexFieldProvider> tempMap = Maps.newHashMap(); for (String nodeType : indexFieldProviderListMultimap.keySet()) { List<IndexFieldProvider> providers = indexFieldProviderListMultimap.get(nodeType); CompositeIndexFieldProvider compositeIndexFieldProvider = new CompositeIndexFieldProvider(nodeType, providers); tempMap.put(nodeType, compositeIndexFieldProvider); } indexFieldProviderMap = tempMap; {code} # CompositeIndexFieldProvider - It might be safer to make use of ImmutableList to copy the list. Not sure the semantics of list from a listMultiMap (may be a view?) > Lucene index augmentation doesn't work in Osgi environment > ---------------------------------------------------------- > > Key: OAK-3826 > URL: https://issues.apache.org/jira/browse/OAK-3826 > Project: Jackrabbit Oak > Issue Type: Bug > Components: lucene > Reporter: Vikas Saurabh > Assignee: Vikas Saurabh > Priority: Minor > Fix For: 1.4 > > Attachments: OAK-3826-v2.patch, OAK-3826.patch > > > OAK-3576 introduced a way to hook SPI to provide extra fields and query terms > for a lucene index. > In Osgi world, due to OAK-3815, {{LuceneIndexProviderService}} registered > references to SPI and pinged {{IndexAugmentFactory}} to update its map. But, > it seems bind/unbind methods get called ahead of time as compared to the > information Tracker contains. This leads to wrong set of services captured by > {{IndexAugmentFactory}} -- This message was sent by Atlassian JIRA (v6.3.4#6332)