Author: alexparvulescu Date: Fri Oct 26 12:05:48 2012 New Revision: 1402479
URL: http://svn.apache.org/viewvc?rev=1402479&view=rev Log: OAK-394 IndexManagerHook to manage existing indexes - simplified the indexmanager diff mechanism to rely on a single updates list and the reindex flag Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManager.java jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManager.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManager.java?rev=1402479&r1=1402478&r2=1402479&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManager.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManager.java Fri Oct 26 12:05:48 2012 @@ -63,18 +63,11 @@ public class IndexHookManager implements NodeBuilder builder = after.builder(); // <path>, <builder> - Map<String, NodeBuilder> changedDefs = new HashMap<String, NodeBuilder>(); - // <path>, <builder> - Map<String, NodeBuilder> existingDefs = new HashMap<String, NodeBuilder>(); + Map<String, NodeBuilder> allDefs = new HashMap<String, NodeBuilder>(); - IndexDefDiff diff = new IndexDefDiff(builder, changedDefs, existingDefs); + IndexDefDiff diff = new IndexDefDiff(builder, allDefs); after.compareAgainstBaseState(before, diff); - // <path>, <builder> - Map<String, NodeBuilder> allDefs = new HashMap<String, NodeBuilder>( - changedDefs); - allDefs.putAll(existingDefs); - // <type, <<path>, <builder>> Map<String, Map<String, NodeBuilder>> updates = new HashMap<String, Map<String, NodeBuilder>>(); for (String def : allDefs.keySet()) { @@ -94,23 +87,35 @@ public class IndexHookManager implements // commit for (String type : updates.keySet()) { - Map<String, NodeBuilder> indexDefs = updates.get(type); - for (String p : indexDefs.keySet()) { + Map<String, NodeBuilder> update = updates.get(type); + for (String path : update.keySet()) { + NodeBuilder updateBuiler = update.get(path); + boolean reindex = getAndResetReindex(updateBuiler); List<? extends IndexHook> hooks = provider.getIndexHooks(type, - indexDefs.get(p)); + updateBuiler); for (IndexHook hook : hooks) { - boolean reindex = changedDefs.keySet().contains(p); if (reindex) { hook.processCommit(MemoryNodeState.EMPTY_NODE, after); } else { hook.processCommit(before, after); } } + } } return builder.getNodeState(); } + protected static boolean getAndResetReindex(NodeBuilder builder) { + boolean isReindex = false; + PropertyState ps = builder.getProperty(REINDEX_PROPERTY_NAME); + if (ps != null) { + isReindex = ps.getValue(Type.BOOLEAN); + builder.setProperty(REINDEX_PROPERTY_NAME, false); + } + return isReindex; + } + protected static class IndexDefDiff implements NodeStateDiff { private final IndexDefDiff parent; @@ -122,23 +127,19 @@ public class IndexHookManager implements private String path; private final Map<String, NodeBuilder> updates; - private final Map<String, NodeBuilder> existing; public IndexDefDiff(IndexDefDiff parent, NodeBuilder node, String name, - String path, Map<String, NodeBuilder> updates, - Map<String, NodeBuilder> existing) { + String path, Map<String, NodeBuilder> updates) { this.parent = parent; this.node = node; this.name = name; this.path = path; this.updates = updates; - this.existing = existing; if (node != null && isIndexNodeType(node.getProperty(JCR_PRIMARYTYPE))) { - getAndResetReindex(node); + node.setProperty(REINDEX_PROPERTY_NAME, true); this.updates.put(getPath(), node); - this.existing.remove(getPath()); } if (node != null && node.hasChildNode(INDEX_DEFINITIONS_NAME)) { @@ -148,25 +149,19 @@ public class IndexHookManager implements INDEX_DEFINITIONS_NAME, indexName); NodeBuilder indexChild = index.child(indexName); if (isIndexNodeType(indexChild.getProperty(JCR_PRIMARYTYPE))) { - boolean reindex = getAndResetReindex(indexChild); - if (reindex) { - this.updates.put(indexPath, indexChild); - } else { - this.existing.put(indexPath, indexChild); - } + this.updates.put(indexPath, indexChild); } } } } - public IndexDefDiff(NodeBuilder root, Map<String, NodeBuilder> updates, - Map<String, NodeBuilder> existing) { - this(null, root, null, "/", updates, existing); + public IndexDefDiff(NodeBuilder root, Map<String, NodeBuilder> updates) { + this(null, root, null, "/", updates); } public IndexDefDiff(IndexDefDiff parent, String name) { this(parent, getChildNode(parent.node, name), name, null, - parent.updates, parent.existing); + parent.updates); } private static NodeBuilder getChildNode(NodeBuilder node, String name) { @@ -221,14 +216,5 @@ public class IndexHookManager implements INDEX_DEFINITIONS_NODE_TYPE); } - private boolean getAndResetReindex(NodeBuilder builder) { - boolean isReindex = false; - PropertyState ps = builder.getProperty(REINDEX_PROPERTY_NAME); - if (ps != null) { - isReindex = ps.getValue(Type.BOOLEAN); - builder.setProperty(REINDEX_PROPERTY_NAME, false); - } - return isReindex; - } } } Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java?rev=1402479&r1=1402478&r2=1402479&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java (original) +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java Fri Oct 26 12:05:48 2012 @@ -21,8 +21,9 @@ import static org.apache.jackrabbit.oak. import static org.junit.Assert.assertTrue; import java.util.HashMap; +import java.util.Iterator; +import java.util.List; import java.util.Map; -import java.util.Set; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.plugins.index.IndexHookManager.IndexDefDiff; @@ -31,6 +32,8 @@ import org.apache.jackrabbit.oak.spi.sta import org.apache.jackrabbit.oak.spi.state.NodeState; import org.junit.Test; +import com.google.common.collect.Lists; + public class IndexHookManagerTest { @Test @@ -68,31 +71,33 @@ public class IndexHookManagerTest { NodeState after = builder.getNodeState(); // <path>, <state> - Map<String, NodeBuilder> changedDefs = new HashMap<String, NodeBuilder>(); - // <path>, <state> - Map<String, NodeBuilder> existingDefs = new HashMap<String, NodeBuilder>(); - - IndexDefDiff diff = new IndexDefDiff(builder, changedDefs, existingDefs); + Map<String, NodeBuilder> defs = new HashMap<String, NodeBuilder>(); + IndexDefDiff diff = new IndexDefDiff(builder, defs); after.compareAgainstBaseState(before, diff); - Set<String> updates = changedDefs.keySet(); - String[] expected = new String[] { "/oak:index/foo", - "/test/other/oak:index/index2" }; - for (String def : expected) { - assertTrue("Expected to find " + def, updates.remove(def)); + List<String> reindex = Lists.newArrayList("/oak:index/foo", + "/test/other/oak:index/index2"); + List<String> updates = Lists.newArrayList("/oak:index/existing"); + + Iterator<String> iterator = defs.keySet().iterator(); + while (iterator.hasNext()) { + String path = iterator.next(); + if (IndexHookManager.getAndResetReindex(defs.get(path))) { + assertTrue("Missing " + path + " from reindex list", + reindex.remove(path)); + } else { + assertTrue("Missing " + path + " from updates list", + updates.remove(path)); + } + iterator.remove(); } + assertTrue(reindex.isEmpty()); assertTrue(updates.isEmpty()); - - Set<String> existing = existingDefs.keySet(); - String[] expectedE = new String[] { "/oak:index/existing", }; - for (String def : expectedE) { - assertTrue("Expected to find " + def, existing.remove(def)); - } - assertTrue(existing.isEmpty()); + assertTrue(defs.isEmpty()); } @Test - public void testReindex() throws Exception { + public void testReindexFlag() throws Exception { NodeState root = MemoryNodeState.EMPTY_NODE; NodeBuilder builder = root.builder(); @@ -101,23 +106,22 @@ public class IndexHookManagerTest { .setProperty(JCR_PRIMARYTYPE, INDEX_DEFINITIONS_NODE_TYPE, Type.NAME) .setProperty(IndexConstants.REINDEX_PROPERTY_NAME, true); - NodeState state = builder.getNodeState(); // <path>, <state> - Map<String, NodeBuilder> changedDefs = new HashMap<String, NodeBuilder>(); - // <path>, <state> - Map<String, NodeBuilder> existingDefs = new HashMap<String, NodeBuilder>(); - - IndexDefDiff diff = new IndexDefDiff(builder, changedDefs, existingDefs); + Map<String, NodeBuilder> defs = new HashMap<String, NodeBuilder>(); + IndexDefDiff diff = new IndexDefDiff(builder, defs); state.compareAgainstBaseState(state, diff); - Set<String> updates = changedDefs.keySet(); - String[] expected = new String[] { "/oak:index/reindexed" }; - for (String def : expected) { - assertTrue("Expected to find " + def, updates.remove(def)); + List<String> reindex = Lists.newArrayList("/oak:index/reindexed"); + Iterator<String> iterator = defs.keySet().iterator(); + while (iterator.hasNext()) { + String path = iterator.next(); + assertTrue("Missing " + path + " from reindex list", + reindex.remove(path)); + iterator.remove(); } - assertTrue(updates.isEmpty()); - assertTrue(existingDefs.isEmpty()); + assertTrue(reindex.isEmpty()); + assertTrue(defs.isEmpty()); } }
