OAK-12244: index nodes that gain a mixin rule, delete stale docs when…

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java

@@ -384,6 +384,9 @@ private void activate(BundleContext bundleContext, Configuration config) throws
         oakRegs.add(whiteboard.register(FeatureToggle.class,
                 new FeatureToggle(FulltextIndexEditor.FT_OAK_12193, FulltextIndexEditor.FT_OAK_12193_DISABLE),
                 emptyMap()));
+        oakRegs.add(whiteboard.register(FeatureToggle.class,
+                new FeatureToggle(FulltextIndexEditor.FT_OAK_12244, FulltextIndexEditor.FT_OAK_12244_DISABLE),
+                emptyMap()));
         initializeIndexDir(bundleContext, config);
         initializeExtractedTextCache(bundleContext, config, statisticsProvider);
         tracker = createTracker(bundleContext, config);

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/DocumentQueue.java

@@ -266,7 +266,7 @@ private void processDocs(String indexPath, Iterable<LuceneDoc> docs, boolean doc
                     doc.markProcessed();
                 }
                 if (doc.delete) {
-                    writer.deleteDocuments(doc.docPath);
+                    writer.deleteDocumentTree(doc.docPath);
                 } else {
                     writer.updateDocument(doc.docPath, doc.doc);
                 }

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java

@@ -56,12 +56,18 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
         }
 
         @Override
-        public void deleteDocuments(String path) throws IOException {
+        public void deleteDocumentTree(String path) throws IOException {
             //Hybrid index logic drops the deletes. So no use to
             //add them to the list
             //addLuceneDoc(LuceneDoc.forDelete(definition.getIndexPathFromConfig(), path));
         }
 
+        @Override
+        public void deleteDocument(String path) throws IOException {
+            //Hybrid index logic drops the deletes. So no use to
+            //add them to the list
+        }
+
         @Override
         public boolean close(long timestamp) throws IOException {
             documentHolder.done(indexPath);

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java

@@ -411,7 +411,12 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
         }
 
         @Override
-        public void deleteDocuments(String path) throws IOException {
+        public void deleteDocumentTree(String path) throws IOException {
+            //Do not delete documents. Query side would handle it
+        }
+
+        @Override
+        public void deleteDocument(String path) throws IOException {
             //Do not delete documents. Query side would handle it
         }
 

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java

@@ -104,11 +104,16 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
     }
 
     @Override
-    public void deleteDocuments(String path) throws IOException {
+    public void deleteDocumentTree(String path) throws IOException {
         getWriter().deleteDocuments(newPathTerm(path));
         getWriter().deleteDocuments(new PrefixQuery(newPathTerm(path + "/")));
     }
 
+    @Override
+    public void deleteDocument(String path) throws IOException {
+        getWriter().deleteDocuments(newPathTerm(path));
+    }
+
     void deleteAll() throws IOException {
         getWriter().deleteAll();
         indexUpdated = true;

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPool.java

@@ -123,17 +123,31 @@ public void execute() throws IOException {
         }
     }
 
-    private static class DeleteOperation extends Operation {
+    private static class DeleteTreeOperation extends Operation {
         private final String path;
 
-        DeleteOperation(LuceneIndexWriter delegate, String path) {
+        DeleteTreeOperation(LuceneIndexWriter delegate, String path) {
             super(delegate);
             this.path = path;
         }
 
         @Override
         public void execute() throws IOException {
-            delegate.deleteDocuments(path);
+            delegate.deleteDocumentTree(path);
+        }
+    }
+
+    private static class DeleteDocumentOperation extends Operation {
+        private final String path;
+
+        DeleteDocumentOperation(LuceneIndexWriter delegate, String path) {
+            super(delegate);
+            this.path = path;
+        }
+
+        @Override
+        public void execute() throws IOException {
+            delegate.deleteDocument(path);
         }
     }
 
@@ -280,10 +294,16 @@ public void updateDocument(LuceneIndexWriter writer, String path, Iterable<? ext
         enqueueOperation(new UpdateOperation(writer, path, doc));
     }
 
-    public void deleteDocuments(LuceneIndexWriter writer, String path) throws IOException {
+    public void deleteDocumentTree(LuceneIndexWriter writer, String path) throws IOException {
+        checkOpen();
+        this.deleteCount++;
+        enqueueOperation(new DeleteTreeOperation(writer, path));
+    }
+
+    public void deleteDocument(LuceneIndexWriter writer, String path) throws IOException {
         checkOpen();
         this.deleteCount++;
-        enqueueOperation(new DeleteOperation(writer, path));
+        enqueueOperation(new DeleteDocumentOperation(writer, path));
     }
 
     public boolean closeWriter(LuceneIndexWriter writer, long timestamp) throws IOException {

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/MultiplexingIndexWriter.java

@@ -62,9 +62,9 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
     }
 
     @Override
-    public void deleteDocuments(String path) throws IOException {
+    public void deleteDocumentTree(String path) throws IOException {
         Mount mount = mountInfoProvider.getMountByPath(path);
-        getWriter(mount).deleteDocuments(path);
+        getWriter(mount).deleteDocumentTree(path);
 
         //In case of default mount look for other mounts with roots under this path
         //Note that one mount cannot be part of another mount
@@ -76,6 +76,13 @@ public void deleteDocuments(String path) throws IOException {
         }
     }
 
+    @Override
+    public void deleteDocument(String path) throws IOException {
+        // Single-document delete: no mount-under-path cleanup needed
+        Mount mount = mountInfoProvider.getMountByPath(path);
+        getWriter(mount).deleteDocument(path);
+    }
+
     @Override
     public boolean close(long timestamp) throws IOException {
         // explicitly get writers for mounts which haven't got writers even at close.

oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/PooledLuceneIndexWriter.java

@@ -51,8 +51,14 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
     }
 
     @Override
-    public void deleteDocuments(String path) throws IOException {
-        writerPool.deleteDocuments(delegateWriter, path);
+    public void deleteDocumentTree(String path) throws IOException {
+        writerPool.deleteDocumentTree(delegateWriter, path);
+        deleteCount++;
+    }
+
+    @Override
+    public void deleteDocument(String path) throws IOException {
+        writerPool.deleteDocument(delegateWriter, path);
         deleteCount++;
     }
 

oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor2Test.java

@@ -21,10 +21,13 @@
 
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.index.IndexCommitCallback;
 import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider;
@@ -50,13 +53,29 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
+import org.apache.jackrabbit.oak.plugins.index.search.spi.editor.FulltextIndexEditor;
+import org.junit.After;
+import org.junit.Before;
+
 public class LuceneIndexEditor2Test {
 
+    @Before
+    public void resetToggles() {
+        FulltextIndexEditor.FT_OAK_12244_DISABLE.set(false);
+    }
+
+    @After
+    public void restoreToggles() {
+        FulltextIndexEditor.FT_OAK_12244_DISABLE.set(false);
+    }
+
     private final NodeState root = INITIAL_CONTENT;
     private NodeState before = root;
     private final IndexUpdateCallback updateCallback = mock(IndexUpdateCallback.class);
@@ -181,6 +200,114 @@ public void relativeProperties() throws Exception{
         propCallback.reset();
     }
 
+    @Test
+    public void nodeGainsMixinTriggersIndexUpdate() throws Exception {
+        LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
+        defnb.indexRule("mix:title").property("jcr:title").propertyIndex();
+
+        NodeState defnState = defnb.build();
+        IndexDefinition defn = new IndexDefinition(root, defnState, indexPath);
+        LuceneIndexEditorContext ctx = newContext(defnState.builder(), defn, true);
+        EditorHook hook = createHook(ctx);
+
+        updateBefore(defnb);
+
+        // Commit 1: node exists without the mixin — must not be indexed
+        NodeBuilder builder = before.builder();
+        builder.child("a").setProperty("jcr:title", "hello");
+        before = hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertFalse("Node without mixin should not be indexed", writer.docs.containsKey("/a"));
+
+        // Commit 2: mixin added to existing node — must be indexed
+        builder = before.builder();
+        builder.child("a").setProperty(JcrConstants.JCR_MIXINTYPES, List.of("mix:title"), Type.NAMES);
+        hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertTrue("Node after gaining mixin should be added to index", writer.docs.containsKey("/a"));
+    }
+
+    @Test
+    public void nodeLosesMixinTriggersDocumentDeletion() throws Exception {
+        LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
+        defnb.indexRule("mix:title").property("jcr:title").propertyIndex();
+
+        NodeState defnState = defnb.build();
+        IndexDefinition defn = new IndexDefinition(root, defnState, indexPath);
+        LuceneIndexEditorContext ctx = newContext(defnState.builder(), defn, true);
+        EditorHook hook = createHook(ctx);
+
+        updateBefore(defnb);
+
+        // Commit 1: node with mixin — must be indexed
+        NodeBuilder builder = before.builder();
+        builder.child("a")
+                .setProperty(JcrConstants.JCR_MIXINTYPES, List.of("mix:title"), Type.NAMES)
+                .setProperty("jcr:title", "hello");
+        before = hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertTrue("Node with mixin should be indexed", writer.docs.containsKey("/a"));
+
+        // Commit 2: mixin removed — existing index document must be deleted
+        builder = before.builder();
+        builder.child("a").removeProperty(JcrConstants.JCR_MIXINTYPES);
+        hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertTrue("Removing mixin should trigger deleteDocument for the node", writer.deletedPaths.contains("/a"));
+    }
+
+    @Test
+    public void nodeGainsMixinDoesNotTriggerIndexUpdateWhenToggleDisabled() throws Exception {
+        FulltextIndexEditor.FT_OAK_12244_DISABLE.set(true);
+
+        LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
+        defnb.indexRule("mix:title").property("jcr:title").propertyIndex();
+
+        NodeState defnState = defnb.build();
+        IndexDefinition defn = new IndexDefinition(root, defnState, indexPath);
+        LuceneIndexEditorContext ctx = newContext(defnState.builder(), defn, true);
+        EditorHook hook = createHook(ctx);
+
+        updateBefore(defnb);
+
+        // Commit 1: node exists without the mixin
+        NodeBuilder builder = before.builder();
+        builder.child("a").setProperty("jcr:title", "hello");
+        before = hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertFalse("Node without mixin should not be indexed", writer.docs.containsKey("/a"));
+
+        // Commit 2: mixin added — with toggle disabled, node must not be indexed
+        builder = before.builder();
+        builder.child("a").setProperty(JcrConstants.JCR_MIXINTYPES, List.of("mix:title"), Type.NAMES);
+        hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertFalse("Mixin tracking disabled: node gaining mixin should not be indexed", writer.docs.containsKey("/a"));
+    }
+
+    @Test
+    public void nodeLosesMixinDoesNotTriggerDocumentDeletionWhenToggleDisabled() throws Exception {
+        FulltextIndexEditor.FT_OAK_12244_DISABLE.set(true);
+
+        LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
+        defnb.indexRule("mix:title").property("jcr:title").propertyIndex();
+
+        NodeState defnState = defnb.build();
+        IndexDefinition defn = new IndexDefinition(root, defnState, indexPath);
+        LuceneIndexEditorContext ctx = newContext(defnState.builder(), defn, true);
+        EditorHook hook = createHook(ctx);
+
+        updateBefore(defnb);
+
+        // Commit 1: node with mixin — indexed because it's a new node
+        NodeBuilder builder = before.builder();
+        builder.child("a")
+                .setProperty(JcrConstants.JCR_MIXINTYPES, List.of("mix:title"), Type.NAMES)
+                .setProperty("jcr:title", "hello");
+        before = hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertTrue("Node with mixin should be indexed", writer.docs.containsKey("/a"));
+
+        // Commit 2: mixin removed — with toggle disabled, stale document must not be deleted
+        builder = before.builder();
+        builder.child("a").removeProperty(JcrConstants.JCR_MIXINTYPES);
+        hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
+        assertFalse("Mixin tracking disabled: removing mixin should not trigger deleteDocument", writer.deletedPaths.contains("/a"));
+    }
+
     private void updateBefore(LuceneIndexDefinitionBuilder defnb) {
         NodeBuilder builder = before.builder();
         NodeBuilder cb = TestUtil.child(builder, PathUtils.getParentPath(indexPath));
@@ -290,7 +417,12 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
         }
 
         @Override
-        public void deleteDocuments(String path) {
+        public void deleteDocumentTree(String path) {
+            deletedPaths.add(path);
+        }
+
+        @Override
+        public void deleteDocument(String path) {
             deletedPaths.add(path);
         }
 

oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPoolTest.java

@@ -78,7 +78,13 @@ public void updateDocument(String path, Iterable<? extends IndexableField> doc)
         }
 
         @Override
-        public void deleteDocuments(String path) {
+        public void deleteDocumentTree(String path) {
+            delay();
+            deletedPaths.add(path);
+        }
+
+        @Override
+        public void deleteDocument(String path) {
             delay();
             deletedPaths.add(path);
         }
@@ -106,7 +112,7 @@ public void testSingleWriter() throws IOException {
         TestWriter writer = new TestWriter();
         Document doc = TestUtil.newDoc("value");
         indexWriterPool.updateDocument(writer, "test", doc);
-        indexWriterPool.deleteDocuments(writer, "test");
+        indexWriterPool.deleteDocumentTree(writer, "test");
         boolean closeResult = indexWriterPool.closeWriter(writer, 30);
         indexWriterPool.close();
 
@@ -163,7 +169,7 @@ public void testCloseWriterPoolWithoutClosingWriters() throws IOException {
         TestWriter writer = new TestWriter(100);
         Document doc = TestUtil.newDoc("value");
         indexWriterPool.updateDocument(writer, "test", doc);
-        indexWriterPool.deleteDocuments(writer, "test-deletion");
+        indexWriterPool.deleteDocumentTree(writer, "test-deletion");
         indexWriterPool.close();
 
         assertEquals(Map.of("test", doc), writer.docs);

oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/MultiplexingIndexWriterTest.java

@@ -209,14 +209,14 @@ public void deletes() throws Exception{
         assertEquals(2, numDocs(defaultMount));
 
         writer = factory.newInstance(defn, builder, null, true);
-        writer.deleteDocuments("/libs/config");
+        writer.deleteDocumentTree("/libs/config");
         writer.close(0);
 
         assertEquals(1, numDocs(fooMount));
         assertEquals(2, numDocs(defaultMount));
 
         writer = factory.newInstance(defn, builder, null, true);
-        writer.deleteDocuments("/content");
+        writer.deleteDocumentTree("/content");
         writer.close(0);
 
         assertEquals(1, numDocs(fooMount));
@@ -240,7 +240,7 @@ public void deleteIncludingMount() throws Exception{
         assertEquals(2, numDocs(defaultMount));
 
         writer = factory.newInstance(defn, builder, null, true);
-        writer.deleteDocuments("/content");
+        writer.deleteDocumentTree("/content");
         writer.close(0);
 
         assertEquals(0, numDocs(fooMount));