You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by al...@apache.org on 2012/10/26 14:05:49 UTC

svn commit: r1402479 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManager.java test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java

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());
     }
 }