Author: chetanm
Date: Thu Oct 26 11:26:12 2017
New Revision: 1813387
URL: http://svn.apache.org/viewvc?rev=1813387&view=rev
Log:
OAK-6857 - Lucene unique index should check path validity for uniqueness
constraint
LuceneIndexEditor does not traverses a deleted subtree so the property
index created by it may not reflect the deletes. So apply the check for
path validity for paths returned by both async and sync index
Modified:
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidator.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/PropertyIndexCleanerTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/SynchronousPropertyIndexTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidatorTest.java
Modified:
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidator.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidator.java?rev=1813387&r1=1813386&r2=1813387&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidator.java
(original)
+++
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidator.java
Thu Oct 26 11:26:12 2017
@@ -19,6 +19,7 @@
package org.apache.jackrabbit.oak.plugins.index.lucene.property;
+import java.util.HashSet;
import java.util.Map;
import java.util.Set;
@@ -35,7 +36,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.collect.Iterables.filter;
import static org.apache.jackrabbit.oak.api.CommitFailedException.CONSTRAINT;
/**
@@ -66,11 +66,18 @@ public class UniquenessConstraintValidat
public void validate() throws CommitFailedException {
for (Map.Entry<String, String> e : uniqueKeys.entries()) {
String propertyRelativePath = e.getKey();
- Iterable<String> indexedPaths =
getIndexedPaths(propertyRelativePath, e.getValue());
+ String value = e.getValue();
+ Iterable<String> indexedPaths =
getIndexedPaths(propertyRelativePath, value);
Set<String> allPaths = ImmutableSet.copyOf(indexedPaths);
+
+ //If more than one match found then filter out stale paths
if (allPaths.size() > 1) {
- String msg = String.format("Uniqueness constraint violated for
property [%s] for " +
- "index [%s]. IndexedPaths %s", propertyRelativePath,
indexPath, allPaths);
+ allPaths = getValidPaths(allPaths, propertyRelativePath,
value);
+ }
+
+ if (allPaths.size() > 1) {
+ String msg = String.format("Uniqueness constraint violated for
property [%s] with value [%s] for " +
+ "index [%s]. Indexed paths %s", propertyRelativePath,
value, indexPath, allPaths);
throw new CommitFailedException(CONSTRAINT, 30, msg);
}
}
@@ -83,23 +90,36 @@ public class UniquenessConstraintValidat
private Iterable<String> getIndexedPaths(String propertyRelativePath,
String value) {
return Iterables.concat(
firstStore.getIndexedPaths(propertyRelativePath, value),
- getValidPathsFromSecondStore(propertyRelativePath, value)
+ secondStore.getIndexedPaths(propertyRelativePath, value)
);
}
- private Iterable<String> getValidPathsFromSecondStore(String
propertyRelativePath, String value) {
- return filter(secondStore.getIndexedPaths(propertyRelativePath, value),
- path -> {
- NodeState node = NodeStateUtils.getNode(rootState, path);
- if (!node.exists()) {
- return false;
- }
- PropertyState uniqueProp = getValue(node,
propertyRelativePath);
- if (uniqueProp == null) {
- return false;
- }
- return uniqueProp.getValue(Type.STRING).equals(value);
- });
+ /**
+ * Paths reported by indexes may be based on stale data. So revalidate by
checking reported paths are
+ * valid and refers to indexed value or not
+ */
+ private Set<String> getValidPaths(Set<String> allPaths, String
propertyRelativePath, String value) {
+ Set<String> validPaths = new HashSet<>();
+ for (String path : allPaths) {
+ NodeState node = NodeStateUtils.getNode(rootState, path);
+ if (!node.exists()) {
+ continue;
+ }
+
+ PropertyState uniqueProp = getValue(node, propertyRelativePath);
+ if (uniqueProp == null) {
+ continue;
+ }
+
+ //Property can be MVP. So check if any of them matches
+ for (String v : uniqueProp.getValue(Type.STRINGS)) {
+ if (v.equals(value)) {
+ validPaths.add(path);
+ break;
+ }
+ }
+ }
+ return validPaths;
}
private static PropertyState getValue(NodeState node, String
propertyRelativePath) {
Modified:
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/PropertyIndexCleanerTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/PropertyIndexCleanerTest.java?rev=1813387&r1=1813386&r2=1813387&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/PropertyIndexCleanerTest.java
(original)
+++
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/PropertyIndexCleanerTest.java
Thu Oct 26 11:26:12 2017
@@ -159,6 +159,7 @@ public class PropertyIndexCleanerTest {
clock.waitUntil(1000);
NodeBuilder builder = nodeStore.getRoot().builder();
+ builder.child("a").setProperty("foo", "bar");
PropertyIndexUpdateCallback cb = newCallback(builder, indexPath);
propertyUpdated(cb, indexPath, "/a", "foo", "bar");
cb.done();
@@ -167,6 +168,7 @@ public class PropertyIndexCleanerTest {
clock.waitUntil(1150);
builder = nodeStore.getRoot().builder();
+ builder.child("b").setProperty("foo", "bar2");
cb = newCallback(builder, indexPath);
propertyUpdated(cb, indexPath, "/b", "foo", "bar2");
cb.done();
@@ -191,6 +193,7 @@ public class PropertyIndexCleanerTest {
assertThat(query(indexPath, "foo", "bar2"), containsInAnyOrder("/b"));
builder = nodeStore.getRoot().builder();
+ builder.child("c").setProperty("foo", "bar2");
cb = newCallback(builder, indexPath);
propertyUpdated(cb, indexPath, "/c", "foo", "bar2");
@@ -312,7 +315,7 @@ public class PropertyIndexCleanerTest {
}
private PropertyIndexUpdateCallback newCallback(NodeBuilder builder,
String indexPath) {
- return new PropertyIndexUpdateCallback(indexPath, child(builder,
indexPath), nodeStore.getRoot(), clock);
+ return new PropertyIndexUpdateCallback(indexPath, child(builder,
indexPath), builder.getNodeState(), clock);
}
private PropertyDefinition pd(String indexPath, String propName){
Modified:
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/SynchronousPropertyIndexTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/SynchronousPropertyIndexTest.java?rev=1813387&r1=1813386&r2=1813387&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/SynchronousPropertyIndexTest.java
(original)
+++
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/SynchronousPropertyIndexTest.java
Thu Oct 26 11:26:12 2017
@@ -242,6 +242,23 @@ public class SynchronousPropertyIndexTes
}
@Test
+ public void uniqueProperty_RemoveAndAddInSameCommit() throws Exception{
+ defnb.async("async", "nrt");
+ defnb.indexRule("nt:base").property("foo").propertyIndex().unique();
+
+ addIndex(indexPath, defnb);
+ root.commit();
+
+ createPath("/a").setProperty("foo", "bar");
+ root.commit();
+ runAsyncIndex();
+
+ createPath("/a").remove();
+ createPath("/b").setProperty("foo", "bar");
+ root.commit();
+ }
+
+ @Test
public void nonUniqueIndex() throws Exception{
defnb.async("async", "nrt");
defnb.indexRule("nt:base").property("foo").propertyIndex().sync();
Modified:
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidatorTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidatorTest.java?rev=1813387&r1=1813386&r2=1813387&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidatorTest.java
(original)
+++
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidatorTest.java
Thu Oct 26 11:26:12 2017
@@ -31,6 +31,7 @@ import org.junit.Test;
import static java.util.Collections.singletonList;
import static org.apache.jackrabbit.oak.InitialContent.INITIAL_CONTENT;
import static org.apache.jackrabbit.oak.api.CommitFailedException.CONSTRAINT;
+import static org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.child;
import static
org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
import static
org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty;
import static org.hamcrest.Matchers.containsString;
@@ -48,13 +49,15 @@ public class UniquenessConstraintValidat
public void singleUniqueProperty() throws Exception{
defnb.indexRule("nt:base").property("foo").unique();
+ updateRootState("/a", "foo", "bar");
+ updateRootState("/b", "foo", "bar");
+
PropertyUpdateCallback callback = newCallback();
callback.propertyUpdated("/a", "foo", pd("foo"),
null, createProperty("foo", "bar"));
callback.propertyUpdated("/b", "foo", pd("foo"),
null, createProperty("foo", "bar"));
-
try {
callback.done();
fail();
@@ -87,16 +90,20 @@ public class UniquenessConstraintValidat
public void firstStore_PreExist() throws Exception{
defnb.indexRule("nt:base").property("foo").unique();
+ updateRootState("/a", "foo", "bar");
+
PropertyUpdateCallback callback = newCallback();
propertyUpdated(callback, "/a", "foo", "bar");
- builder = builder.getNodeState().builder();
+ refreshBuilder();
+ updateRootState("/b", "foo", "bar");
callback = newCallback();
propertyUpdated(callback, "/b", "foo", "bar");
callback.done();
}
-
+
+
@Test
public void secondStore_SamePath() throws Exception{
defnb.indexRule("nt:base").property("foo").unique();
@@ -115,9 +122,8 @@ public class UniquenessConstraintValidat
public void secondStore_DiffPath() throws Exception{
defnb.indexRule("nt:base").property("foo").unique();
- NodeBuilder rootBuilder = root.builder();
- rootBuilder.child("b").setProperty("foo", "bar");
- root = rootBuilder.getNodeState();
+ updateRootState("/b", "foo", "bar");
+ updateRootState("/a", "foo", "bar");
PropertyIndexUpdateCallback callback = newCallback();
propertyUpdated(callback, "/a", "foo", "bar");
@@ -179,9 +185,8 @@ public class UniquenessConstraintValidat
public void secondStore_RelativeProperty() throws Exception{
defnb.indexRule("nt:base").property("jcr:content/foo").unique();
- NodeBuilder rootBuilder = root.builder();
- rootBuilder.child("b").child("jcr:content").setProperty("foo", "bar");
- root = rootBuilder.getNodeState();
+ updateRootState("/b/jcr:content", "foo", "bar");
+ updateRootState("/a/jcr:content", "foo", "bar");
PropertyIndexUpdateCallback callback = newCallback();
propertyUpdated(callback, "/a", "jcr:content/foo", "bar");
@@ -201,6 +206,16 @@ public class UniquenessConstraintValidat
return new PropertyIndexUpdateCallback(indexPath, builder, root);
}
+ private void updateRootState(String nodePath, String propertyName, String
value) {
+ NodeBuilder rootBuilder = root.builder();
+ child(rootBuilder, nodePath).setProperty(propertyName, value);
+ root = rootBuilder.getNodeState();
+ }
+
+ private void refreshBuilder() {
+ builder = builder.getNodeState().builder();
+ }
+
private PropertyDefinition pd(String propName){
IndexDefinition defn = new IndexDefinition(root, defnb.build(),
indexPath);
return defn.getApplicableIndexingRule("nt:base").getConfig(propName);
