[ https://issues.apache.org/jira/browse/OAK-2999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14612882#comment-14612882 ]
Chetan Mehrotra commented on OAK-2999: -------------------------------------- {noformat} - boolean dirty = false; + boolean dirty = propertiesChanged; + for (PropertyState property : state.getProperties()) { String pname = property.getName(); @@ -327,6 +328,8 @@ } dirty |= indexProperty(path, fields, state, property, pname, pd); + // If the only property was MVP and indexed earlier and has now been emptied + dirty |= !before.hasProperty(pname) || (property.count() == 0); {noformat} If we set dirty to {{propertiesChanged}} then the other change might not be required as if any indexed property is changed then {{propertiesChanged}} would be true and dirty would remain true. In fact we can then do away with dirty flag at all. However there is one issue with how {{propertiesChanged}} is currently set {code} private void markPropertyChanged(String name) { if (isIndexable() && !propertiesChanged && indexingRule.isIndexed(name)) { propertiesChanged = true; } } {code} {{indexingRule.isIndexed(name)}} just checks for presence of property definition. But that has one bug where a property might have property definition but with {{index}} set to false i.e. its a negative entry to prevent some properties from getting indexed. This is typically used when you need to exclude certain properties from fulltext index. There are other cases also where propertyDefinition might exist for given name but the type does not match. Due to this {{propertiesChanged}} so far was like hint with actual check done in various indexing logic {noformat} + nt:base + properties - jcr:primaryType = "nt:unstructured" +loginTime - name = "loginTime" - index = false + allProps - name = ".*" - isRegexp = true - nodeScopeIndex = true {noformat} So we would need to be carefull there. Some minor comments * Change {{state}} in {{makeDocument}} to {{after}} * There are some unnecessary changes in the patch mostly around formatting. Can you avoid them > Index updation fails on updating multivalued property > ----------------------------------------------------- > > Key: OAK-2999 > URL: https://issues.apache.org/jira/browse/OAK-2999 > Project: Jackrabbit Oak > Issue Type: Bug > Components: lucene > Affects Versions: 1.2, 1.0.15 > Reporter: Rishabh Maurya > Assignee: Amit Jain > Fix For: 1.2.3, 1.3.3, 1.0.17 > > Attachments: OAK-2999.patch > > > On emptying a multivalued property, fulltext index updation fails and one can > search on old values. Following test demonstrates the issue. > Added below test in > [LuceneIndexQueryTest.java|https://github.com/apache/jackrabbit-oak/blob/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java] > which should pass - > {code} > @Test > public void testMultiValuedPropUpdate() throws Exception { > Tree test = root.getTree("/").addChild("test"); > String child = "child"; > String mulValuedProp = "prop"; > test.addChild(child).setProperty(mulValuedProp, of("foo","bar"), > Type.STRINGS); > root.commit(); > assertQuery( > "/jcr:root//*[jcr:contains(@" + mulValuedProp + ", 'foo')]", > "xpath", ImmutableList.of("/test/" + child)); > test.getChild(child).setProperty(mulValuedProp, new > ArrayList<String>(), Type.STRINGS); > root.commit(); > assertQuery( > "/jcr:root//*[jcr:contains(@" + mulValuedProp + ", 'foo')]", > "xpath", new ArrayList<String>()); > test.getChild(child).setProperty(mulValuedProp, of("bar"), > Type.STRINGS); > root.commit(); > assertQuery( > "/jcr:root//*[jcr:contains(@" + mulValuedProp + ", 'foo')]", > "xpath", new ArrayList<String>()); > } > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)