[ https://issues.apache.org/jira/browse/OAK-8589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16921460#comment-16921460 ]
Vikas Saurabh edited comment on OAK-8589 at 9/3/19 2:35 PM: ------------------------------------------------------------ [~tmueller], can you please review: {noformat} diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java index 037407e26e..72147939cc 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java @@ -312,7 +312,13 @@ public final class IndexDefinitionBuilder { private Tree findExisting(String name) { for (Tree tree : getPropsTree().getChildren()){ - if (name.equals(tree.getProperty(FulltextIndexConstants.PROP_NAME).getValue(Type.STRING))){ + String treeName = tree.getName(); + PropertyState ps = tree.getProperty(FulltextIndexConstants.PROP_NAME); + if(ps != null) { + treeName = ps.getValue(Type.STRING); + } + + if (name.equals(treeName)) { return tree; } } diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java index 2402779cbb..1fecbe6c0a 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java @@ -973,4 +973,20 @@ public class IndexDefinitionBuilderTest { assertThat(state.getProperty(INDEX_TAGS).getValue(Type.STRINGS), Matchers.containsInAnyOrder("foo5")); } + + @Test + public void unnamedPropertyRuleInExistingIndex() { + // create an initial index with property rule for "foo" + builder + .indexRule("nt:base") + .property("foo") + // remove "name" property explicitly + .getBuilderTree().removeProperty("name"); + NodeState initialIndexState = builder.build(); + + // Use initial index def to add some other property rule - this should work + new IndexDefinitionBuilder(initialIndexState.builder()) + .indexRule("nt:base") + .property("bar"); + } } {noformat} The idea is basically a copy of what {{PropertyDefinition#getName}} is already doing except that one works one NodeState while other on Tree. was (Author: catholicon): [~tmueller], can you please review: {noformat} diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java index 037407e26e..d6e8162d50 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilder.java @@ -312,13 +312,18 @@ public final class IndexDefinitionBuilder { private Tree findExisting(String name) { for (Tree tree : getPropsTree().getChildren()){ - if (name.equals(tree.getProperty(FulltextIndexConstants.PROP_NAME).getValue(Type.STRING))){ + if (name.equals(getName(tree))) { return tree; } } return null; } + private static String getName(Tree definition){ + PropertyState ps = definition.getProperty(FulltextIndexConstants.PROP_NAME); + return ps == null ? definition.getName() : ps.getValue(Type.STRING); + } + private String createPropNodeName(String name, boolean regex) { name = regex ? "prop" : getSafePropName(name); if (name.isEmpty()){ diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java index 2402779cbb..1fecbe6c0a 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/IndexDefinitionBuilderTest.java @@ -973,4 +973,20 @@ public class IndexDefinitionBuilderTest { assertThat(state.getProperty(INDEX_TAGS).getValue(Type.STRINGS), Matchers.containsInAnyOrder("foo5")); } + + @Test + public void unnamedPropertyRuleInExistingIndex() { + // create an initial index with property rule for "foo" + builder + .indexRule("nt:base") + .property("foo") + // remove "name" property explicitly + .getBuilderTree().removeProperty("name"); + NodeState initialIndexState = builder.build(); + + // Use initial index def to add some other property rule - this should work + new IndexDefinitionBuilder(initialIndexState.builder()) + .indexRule("nt:base") + .property("bar"); + } } {noformat} The idea is basically a copy of what {{PropertyDefinition#getName}} is already doing except that one works one NodeState while other on Tree. > NPE in IndexDefintionBuilder with existing property rule without "name" > property > -------------------------------------------------------------------------------- > > Key: OAK-8589 > URL: https://issues.apache.org/jira/browse/OAK-8589 > Project: Jackrabbit Oak > Issue Type: Improvement > Environment: Inde > Reporter: Vikas Saurabh > Assignee: Vikas Saurabh > Priority: Major > > {{IndexDefinitionBuilder#findExisting}} throws NPE when > {{IndexDefinitionBuilder}} is initialized with an existing index that has a > property rule without {{name}} property defined. -- This message was sent by Atlassian Jira (v8.3.2#803003)