[
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)
