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 th...@apache.org on 2026/06/11 13:20:34 UTC
(jackrabbit-oak) branch OAK-12244 updated: OAK-12244: index nodes that gain a mixin rule, delete stale docs when mixin rule is lost (#2938)
This is an automated email from the ASF dual-hosted git repository.
thomasmueller pushed a commit to branch OAK-12244
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/OAK-12244 by this push:
new 97b046601b OAK-12244: index nodes that gain a mixin rule, delete stale docs when mixin rule is lost (#2938)
97b046601b is described below
commit 97b046601b2e38ba46ec64c0518636e376e9ab66
Author: Benjamin Habegger <bh...@adobe.com>
AuthorDate: Thu Jun 11 15:20:24 2026 +0200
OAK-12244: index nodes that gain a mixin rule, delete stale docs when mixin rule is lost (#2938)
When an existing node's applicable indexing rule changes at runtime (e.g. jcr:mixinTypes
added or removed), FulltextIndexEditor did not update the index because propertiesChanged
was never set — jcr:mixinTypes is not normally listed in a rule's property definitions.
Track wasIndexable (rule matched before) alongside isIndexable() (rule matches after).
In leave(), act on transitions:
- !wasIndexable && isIndexable(): node gained a rule → addOrUpdate
- wasIndexable && !isIndexable(): node lost a rule → deleteDocuments
Tests added:
- PropertyIndexCommonTest: two end-to-end integration tests (all backends)
- LuceneIndexEditor2Test: two unit tests verifying writer.docs / writer.deletedPaths
---
.../index/lucene/LuceneIndexProviderService.java | 3 +
.../index/lucene/LuceneIndexEditor2Test.java | 127 +++++++++++++++++++++
.../search/spi/editor/FulltextIndexEditor.java | 60 +++++++++-
.../oak/plugins/index/PropertyIndexCommonTest.java | 48 ++++++++
4 files changed, 233 insertions(+), 5 deletions(-)
diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java
index ec2c5990d4..12144f4342 100644
--- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java
+++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java
@@ -384,6 +384,9 @@ public class LuceneIndexProviderService {
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);
diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor2Test.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor2Test.java
index a3cd595a2e..75db1a8839 100644
--- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor2Test.java
+++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor2Test.java
@@ -21,10 +21,13 @@ package org.apache.jackrabbit.oak.plugins.index.lucene;
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.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstant
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 class LuceneIndexEditor2Test {
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 deleteDocuments 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 deleteDocuments", writer.deletedPaths.contains("/a"));
+ }
+
private void updateBefore(LuceneIndexDefinitionBuilder defnb) {
NodeBuilder builder = before.builder();
NodeBuilder cb = TestUtil.child(builder, PathUtils.getParentPath(indexPath));
diff --git a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
index da7a5d29a7..6bb8996b4a 100644
--- a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
+++ b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
@@ -69,6 +69,23 @@ public class FulltextIndexEditor<D> implements IndexEditor, Aggregate.AggregateR
*/
public static final AtomicBoolean FT_OAK_12193_DISABLE = new AtomicBoolean(false);
+ /**
+ * Feature toggle name for OAK-12244.
+ * When active (default), nodes that gain or lose a mixin type at runtime are
+ * correctly added to or removed from the fulltext index, even when
+ * {@code jcr:mixinTypes} is not listed in the rule's property definitions.
+ */
+ public static final String FT_OAK_12244 = "FT_OAK-12244";
+
+ /**
+ * Kill switch for the OAK-12244 mixin-transition tracking. Set to {@code true} to
+ * revert to the pre-OAK-12244 behavior where mixin additions and removals do not
+ * trigger index updates unless an indexed property also changed.
+ * Default is {@code false} (mixin-transition tracking active). Wired to the
+ * {@link #FT_OAK_12244} feature toggle at runtime.
+ */
+ public static final AtomicBoolean FT_OAK_12244_DISABLE = new AtomicBoolean(false);
+
private static final List<Aggregate.Matcher> EMPTY_AGGREGATE_MATCHER_LIST = List.of();
private final FulltextIndexEditorContext<D> context;
@@ -81,6 +98,8 @@ public class FulltextIndexEditor<D> implements IndexEditor, Aggregate.AggregateR
private boolean propertiesChanged = false;
+ private boolean wasIndexable = false;
+
private final List<PropertyState> propertiesModified = new ArrayList<>();
/*
@@ -143,17 +162,48 @@ public class FulltextIndexEditor<D> implements IndexEditor, Aggregate.AggregateR
if (indexingRule != null) {
currentMatchers = indexingRule.getAggregate().createMatchers(this);
}
+
+ if (!FT_OAK_12244_DISABLE.get() && before.exists()) {
+ wasIndexable = getDefinition().getApplicableIndexingRule(before) != null;
+ }
}
}
@Override
public void leave(NodeState before, NodeState after)
throws CommitFailedException {
- if (propertiesChanged || !before.exists()) {
- if (addOrUpdate(path, after, before.exists())) {
- long indexed = context.incIndexedNodes();
- if (indexed % 1000 == 0) {
- log.debug("[{}] => Indexed {} nodes...", getIndexName(), indexed);
+ if (FT_OAK_12244_DISABLE.get()) {
+ // Legacy: no mixin-transition tracking (pre-OAK-12244 behavior)
+ if (propertiesChanged || !before.exists()) {
+ if (addOrUpdate(path, after, before.exists())) {
+ long indexed = context.incIndexedNodes();
+ if (indexed % 1000 == 0) {
+ log.debug("[{}] => Indexed {} nodes...", getIndexName(), indexed);
+ }
+ }
+ }
+ } else {
+ boolean toBeDeleted = wasIndexable && !isIndexable();
+ boolean toBeAdded = !wasIndexable && isIndexable();
+ boolean toBeUpdated = wasIndexable && isIndexable() && propertiesChanged;
+
+ if (toBeDeleted) {
+ try {
+ context.getWriter().deleteDocuments(path);
+ context.indexUpdate();
+ } catch (IOException e) {
+ CommitFailedException ce = new CommitFailedException("Fulltext", 6,
+ "Failed to delete stale index document for " + path
+ + " in index " + context.getIndexingContext().getIndexPath(), e);
+ context.getIndexingContext().indexUpdateFailed(ce);
+ throw ce;
+ }
+ } else if (toBeAdded || toBeUpdated) {
+ if (addOrUpdate(path, after, before.exists())) {
+ long indexed = context.incIndexedNodes();
+ if (indexed % 1000 == 0) {
+ log.debug("[{}] => Indexed {} nodes...", getIndexName(), indexed);
+ }
}
}
}
diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
index 1d8d24e478..f481e3539e 100644
--- a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
+++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
@@ -520,6 +520,54 @@ public abstract class PropertyIndexCommonTest extends AbstractQueryTest {
});
}
+ @Test
+ public void nodeGainsMixinAppearsInMixinBasedIndex() throws Exception {
+ indexOptions.setIndex(
+ root,
+ "test1",
+ indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), "mix:title", false, "jcr:title")
+ );
+ root.commit();
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a = test.addChild("a");
+ a.setProperty("jcr:title", "hello");
+ root.commit();
+
+ String query = "select [jcr:path] from [mix:title] where [jcr:title] = 'hello'";
+ assertEventually(() -> assertQuery(query, List.of()));
+
+ a = root.getTree("/test/a");
+ a.setProperty(JcrConstants.JCR_MIXINTYPES, List.of("mix:title"), Type.NAMES);
+ root.commit();
+
+ assertEventually(() -> assertQuery(query, List.of("/test/a")));
+ }
+
+ @Test
+ public void nodeLosesMixinDisappearsFromMixinBasedIndex() throws Exception {
+ indexOptions.setIndex(
+ root,
+ "test1",
+ indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), "mix:title", false, "jcr:title")
+ );
+ root.commit();
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a = createNodeWithMixinType(test, "a", "mix:title");
+ a.setProperty("jcr:title", "hello");
+ root.commit();
+
+ String query = "select [jcr:path] from [mix:title] where [jcr:title] = 'hello'";
+ assertEventually(() -> assertQuery(query, List.of("/test/a")));
+
+ a = root.getTree("/test/a");
+ a.removeProperty(JcrConstants.JCR_MIXINTYPES);
+ root.commit();
+
+ assertEventually(() -> assertQuery(query, List.of()));
+ }
+
@Test
public void indexingBasedOnMixinAndRelativeProps() throws Exception {
indexOptions.setIndex(