From 9facf48e54e4c22d93b1bb6b275ce1ef7ccc6968 Mon Sep 17 00:00:00 2001 From: Benjamin Habegger Date: Fri, 12 Jun 2026 11:13:55 +0200 Subject: [PATCH] OAK-12244: fix mixin type changes not reflected in fulltext index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: when a node gains or loses a mixin type at runtime, FulltextIndexEditor did not update the index because propertiesChanged was never set — jcr:mixinTypes is not normally listed in a rule's property definitions. Fix: track wasIndexable (rule matched before) alongside isIndexable() (rule matches after). In leave(), act on the indexing-rule transition: - !wasIndexable && isIndexable(): node gained a rule → addOrUpdate - wasIndexable && !isIndexable(): node lost a rule → deleteDocument Split FulltextIndexWriter into two explicit operations: - deleteDocumentTree(path): node physically removed; cascade is correct - deleteDocument(path): node lost indexability at runtime; exact only The original deleteDocuments used a PrefixQuery that cascaded to all descendants; in the mixin-loss branch this was a bug — children carrying their own mixin types were incorrectly evicted from the index. Additional changes: - Snapshot FT_OAK_12244_DISABLE once per commit cycle in FulltextIndexEditorContext as typeChangeTrackingEnabled so enter() and leave() always agree - Skip getApplicableIndexingRule(before) on the hot path via hasNodeTypeChange guard when neither jcr:primaryType nor jcr:mixinTypes changed - Register FT_OAK_12244 toggle in ElasticIndexProviderService - Reuse CommitFailedException code 5 for the deleteDocument error path Tests: - PropertyIndexCommonTest: end-to-end integration tests (all backends) - LuceneIndexEditor2Test: unit tests verifying writer.docs / writer.deletedPaths - Verified: 1245 tests, 0 failures in oak-lucene --- .../index/lucene/hybrid/DocumentQueue.java | 2 +- .../hybrid/LocalIndexWriterFactory.java | 8 ++- .../plugins/index/lucene/hybrid/NRTIndex.java | 7 ++- .../lucene/writer/DefaultIndexWriter.java | 7 ++- .../index/lucene/writer/IndexWriterPool.java | 30 +++++++-- .../writer/MultiplexingIndexWriter.java | 11 +++- .../writer/PooledLuceneIndexWriter.java | 10 ++- .../index/lucene/LuceneIndexEditor2Test.java | 11 +++- .../lucene/writer/IndexWriterPoolTest.java | 12 +++- .../writer/MultiplexingIndexWriterTest.java | 6 +- .../elastic/ElasticIndexProviderService.java | 5 ++ .../elastic/index/ElasticIndexWriter.java | 8 ++- .../elastic/index/ElasticIndexWriterTest.java | 8 +-- .../spi/editor/FulltextIndexEditor.java | 61 ++++++++++++------- .../editor/FulltextIndexEditorContext.java | 7 +++ .../spi/editor/FulltextIndexWriter.java | 19 +++++- .../index/PropertyIndexCommonTest.java | 28 +++++++++ 17 files changed, 188 insertions(+), 52 deletions(-) diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/DocumentQueue.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/DocumentQueue.java index 60e63d4e0a4..2bb7f6e5f1e 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/DocumentQueue.java +++ b/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 docs, boolean doc doc.markProcessed(); } if (doc.delete) { - writer.deleteDocuments(doc.docPath); + writer.deleteDocumentTree(doc.docPath); } else { writer.updateDocument(doc.docPath, doc.doc); } diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java index e98e78ef8c5..5f0a7ccc6e9 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java +++ b/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 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); diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java index c70755d4b67..5c9ae9f145a 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/NRTIndex.java +++ b/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 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 } diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java index 79a7790cbf0..96a62098103 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java +++ b/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 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; diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPool.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPool.java index 56ab1c6f05a..d2cefdcb839 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPool.java +++ b/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 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. diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/PooledLuceneIndexWriter.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/PooledLuceneIndexWriter.java index 73e27e5ff62..208a7e2b112 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/PooledLuceneIndexWriter.java +++ b/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 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++; } 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 75db1a8839b..8ccffdc9a10 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 @@ -249,7 +249,7 @@ public void nodeLosesMixinTriggersDocumentDeletion() throws Exception { 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")); + assertTrue("Removing mixin should trigger deleteDocument for the node", writer.deletedPaths.contains("/a")); } @Test @@ -305,7 +305,7 @@ public void nodeLosesMixinDoesNotTriggerDocumentDeletionWhenToggleDisabled() thr 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")); + assertFalse("Mixin tracking disabled: removing mixin should not trigger deleteDocument", writer.deletedPaths.contains("/a")); } private void updateBefore(LuceneIndexDefinitionBuilder defnb) { @@ -417,7 +417,12 @@ public void updateDocument(String path, Iterable doc) } @Override - public void deleteDocuments(String path) { + public void deleteDocumentTree(String path) { + deletedPaths.add(path); + } + + @Override + public void deleteDocument(String path) { deletedPaths.add(path); } diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPoolTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPoolTest.java index 2d94694ae79..472d8d06cb7 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/IndexWriterPoolTest.java +++ b/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 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); diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/MultiplexingIndexWriterTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/MultiplexingIndexWriterTest.java index b3438faa8c0..436a382e684 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/MultiplexingIndexWriterTest.java +++ b/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)); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java index fbc3fc794f0..cc2088bf8be 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java @@ -62,6 +62,8 @@ import java.util.Hashtable; import java.util.List; +import org.apache.jackrabbit.oak.plugins.index.search.spi.editor.FulltextIndexEditor; + import static java.util.Collections.emptyMap; import static org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.registerMBean; @@ -227,6 +229,9 @@ private void activate(BundleContext bundleContext, Config config) { oakRegs.add(whiteboard.register(FeatureToggle.class, new FeatureToggle(ElasticConnection.FT_OAK_12234, ElasticConnection.FT_OAK_12234_DISABLE), emptyMap())); + oakRegs.add(whiteboard.register(FeatureToggle.class, + new FeatureToggle(FulltextIndexEditor.FT_OAK_12244, FulltextIndexEditor.FT_OAK_12244_DISABLE), + emptyMap())); if (System.getProperty(QueryEngineSettings.OAK_INFERENCE_ENABLED) != null) { this.isInferenceEnabled = Boolean.parseBoolean(System.getProperty(QueryEngineSettings.OAK_INFERENCE_ENABLED)); } else { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriter.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriter.java index 893dd792f7a..9e71388e428 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriter.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriter.java @@ -166,7 +166,7 @@ public void updateDocument(String path, ElasticDocument doc) throws IOException } @Override - public void deleteDocuments(String path) throws IOException { + public void deleteDocumentTree(String path) throws IOException { retryPolicy.withRetries(() -> bulkProcessorHandler.delete(indexName, ElasticIndexUtils.idFromPath(path))); if (!ElasticIndexEditorProvider.FT_OAK_12206_DISABLE.get()) { // Delete all descendants: mirrors Lucene's PrefixQuery on the path term. @@ -187,6 +187,12 @@ public void deleteDocuments(String path) throws IOException { } } + @Override + public void deleteDocument(String path) throws IOException { + // Exact-document delete: no descendant sweep + retryPolicy.withRetries(() -> bulkProcessorHandler.delete(indexName, ElasticIndexUtils.idFromPath(path))); + } + @Override public boolean close(long timestamp) throws IOException { boolean updateStatus = bulkProcessorHandler.flushIndex(indexName); diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriterTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriterTest.java index e644f2a456f..ee6c9651fb9 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriterTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriterTest.java @@ -111,7 +111,7 @@ public void singleUpdateDocument() throws IOException { @Test public void singleDeleteDocument() throws IOException { - indexWriter.deleteDocuments("/bar"); + indexWriter.deleteDocumentTree("/bar"); ArgumentCaptor idCaptor = ArgumentCaptor.forClass(String.class); verify(bulkProcessorHandlerMock).delete(eq(indexAlias), idCaptor.capture()); @@ -127,8 +127,8 @@ public void singleDeleteDocument() throws IOException { public void multiRequests() throws IOException { indexWriter.updateDocument("/foo", new ElasticDocument("/foo")); indexWriter.updateDocument("/bar", new ElasticDocument("/bar")); - indexWriter.deleteDocuments("/foo"); - indexWriter.deleteDocuments("/bar"); + indexWriter.deleteDocumentTree("/foo"); + indexWriter.deleteDocumentTree("/bar"); verify(bulkProcessorHandlerMock, times(2)).index(eq(indexAlias), anyString(), any(ElasticDocument.class)); verify(bulkProcessorHandlerMock, times(2)).delete(eq(indexAlias), anyString()); @@ -182,7 +182,7 @@ public void splitLargeString() { @Test public void ft_oak_12206_toggleShouldBeRemoved() { // Time-bombed: if this test fails, the feature toggle FT_OAK-12206 and its guard in - // ElasticIndexWriter#deleteDocuments should be removed — the fix has been in production long enough. + // ElasticIndexWriter#deleteDocumentTree should be removed — the fix has been in production long enough. assertTrue("Feature toggle " + ElasticIndexEditorProvider.FT_OAK_12206 + " is overdue for removal", LocalDate.now().isBefore(LocalDate.of(2027, 5, 6))); } 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 6bb8996b4a5..13985c2f076 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 @@ -35,10 +35,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.jackrabbit.JcrConstants; + import java.io.IOException; import java.util.ArrayList; import java.util.BitSet; import java.util.List; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -52,7 +55,7 @@ public class FulltextIndexEditor implements IndexEditor, Aggregate.AggregateR /** * Feature toggle name for OAK-12193. - * When this toggle is active (default), deleteDocuments calls are skipped for + * When this toggle is active (default), deleteDocumentTree calls are skipped for * removed subtrees that do not contain any node matching the index definition's * indexing rules. This avoids accumulating large numbers of buffered deletes in * the Lucene writer's DocumentsWriterDeleteQueue during delete-heavy async @@ -62,7 +65,7 @@ public class FulltextIndexEditor implements IndexEditor, Aggregate.AggregateR /** * Kill switch for the OAK-12193 filtered delete behavior. Set to {@code true} to - * revert to the legacy behavior of always issuing a deleteDocuments call for every + * revert to the legacy behavior of always issuing a deleteDocumentTree call for every * removed subtree regardless of whether the index could have indexed its nodes. * Default is {@code false} (filtered behavior active). Wired to the * {@link #FT_OAK_12193} feature toggle at runtime. @@ -163,8 +166,13 @@ public void enter(NodeState before, NodeState after) { currentMatchers = indexingRule.getAggregate().createMatchers(this); } - if (!FT_OAK_12244_DISABLE.get() && before.exists()) { - wasIndexable = getDefinition().getApplicableIndexingRule(before) != null; + if (context.isTypeChangeTrackingEnabled() && before.exists()) { + if (hasNodeTypeChange(before, after)) { + wasIndexable = getDefinition().getApplicableIndexingRule(before) != null; + } else { + // Types unchanged: before and after match the same rule + wasIndexable = indexingRule != null; + } } } } @@ -172,27 +180,19 @@ public void enter(NodeState before, NodeState after) { @Override public void leave(NodeState before, NodeState after) throws CommitFailedException { - 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 { + if (context.isTypeChangeTrackingEnabled()) { + // OAK-12244: act on rule-gained / rule-lost transitions boolean toBeDeleted = wasIndexable && !isIndexable(); - boolean toBeAdded = !wasIndexable && isIndexable(); + boolean toBeAdded = !wasIndexable && isIndexable(); boolean toBeUpdated = wasIndexable && isIndexable() && propertiesChanged; if (toBeDeleted) { try { - context.getWriter().deleteDocuments(path); + // Exact-document delete: children that still carry the mixin must not be evicted + context.getWriter().deleteDocument(path); context.indexUpdate(); } catch (IOException e) { - CommitFailedException ce = new CommitFailedException("Fulltext", 6, + CommitFailedException ce = new CommitFailedException("Fulltext", 5, "Failed to delete stale index document for " + path + " in index " + context.getIndexingContext().getIndexPath(), e); context.getIndexingContext().indexUpdateFailed(ce); @@ -206,6 +206,16 @@ public void leave(NodeState before, NodeState after) } } } + } else { + // pre-OAK-12244: only property changes and new nodes trigger indexing + if (propertiesChanged || !before.exists()) { + if (addOrUpdate(path, after, before.exists())) { + long indexed = context.incIndexedNodes(); + if (indexed % 1000 == 0) { + log.debug("[{}] => Indexed {} nodes...", getIndexName(), indexed); + } + } + } } BitSet bitSet = matcherState.affectedMatchers; @@ -293,14 +303,14 @@ public Editor childNodeDeleted(String name, NodeState before) } if (!FT_OAK_12193_DISABLE.get()) { - // OAK-12193: skip the deleteDocuments call when no node in the removed subtree - // could have been indexed. Legacy behavior would route a deleteDocuments call + // OAK-12193: skip the deleteDocumentTree call when no node in the removed subtree + // could have been indexed. Legacy behavior would route a deleteDocumentTree call // for every removed subtree regardless, accumulating buffered deletes in the // Lucene writer during delete-heavy async cycles. if (!isDeleted && subtreeHasIndexableNode(context.getDefinition(), before)) { try { FulltextIndexWriter writer = context.getWriter(); - writer.deleteDocuments(childPath); + writer.deleteDocumentTree(childPath); this.context.indexUpdate(); } catch (IOException e) { CommitFailedException ce = new CommitFailedException("Fulltext", 5, "Failed to remove the index entries of" @@ -315,7 +325,7 @@ public Editor childNodeDeleted(String name, NodeState before) try { FulltextIndexWriter writer = context.getWriter(); // Remove all index entries in the removed subtree - writer.deleteDocuments(childPath); + writer.deleteDocumentTree(childPath); this.context.indexUpdate(); } catch (IOException e) { CommitFailedException ce = new CommitFailedException("Fulltext", 5, "Failed to remove the index entries of" @@ -511,6 +521,13 @@ private boolean isIndexable() { return indexingRule != null; } + private static boolean hasNodeTypeChange(NodeState before, NodeState after) { + return !Objects.equals(before.getProperty(JcrConstants.JCR_MIXINTYPES), + after.getProperty(JcrConstants.JCR_MIXINTYPES)) + || !Objects.equals(before.getProperty(JcrConstants.JCR_PRIMARYTYPE), + after.getProperty(JcrConstants.JCR_PRIMARYTYPE)); + } + private String getIndexName() { return context.getDefinition().getIndexName(); } diff --git a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditorContext.java b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditorContext.java index b8771332286..38427b6bf83 100644 --- a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditorContext.java +++ b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditorContext.java @@ -89,6 +89,8 @@ public abstract class FulltextIndexEditorContext { private final boolean indexDefnRewritten; + private final boolean typeChangeTrackingEnabled; + private FulltextBinaryTextExtractor textExtractor; private PropertyUpdateCallback propertyUpdateCallback; @@ -109,6 +111,7 @@ protected FulltextIndexEditorContext(NodeState root, NodeBuilder definition, this.updateCallback = updateCallback; this.extractedTextCache = extractedTextCache; this.asyncIndexing = asyncIndexing; + this.typeChangeTrackingEnabled = !FulltextIndexEditor.FT_OAK_12244_DISABLE.get(); if (this.definition.isOfOldFormat()) { indexDefnRewritten = true; IndexDefinition.updateDefinition(definition, indexingContext.getIndexPath()); @@ -121,6 +124,10 @@ protected FulltextIndexEditorContext(NodeState root, NodeBuilder definition, public abstract DocumentMaker newDocumentMaker(IndexDefinition.IndexingRule rule, String path); + public boolean isTypeChangeTrackingEnabled() { + return typeChangeTrackingEnabled; + } + protected FulltextBinaryTextExtractor createBinaryTextExtractor(ExtractedTextCache extractedTextCache, IndexDefinition definition, boolean reindex) { return new FulltextBinaryTextExtractor(extractedTextCache, definition, reindex); diff --git a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexWriter.java b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexWriter.java index bc19febc66a..b936b7f48b6 100644 --- a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexWriter.java +++ b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexWriter.java @@ -36,11 +36,24 @@ public interface FulltextIndexWriter { void updateDocument(String path, D doc) throws IOException; /** - * Deletes documents which are same or child of given path + * Deletes the document at the given path and all descendant documents. + * Use this when a node is physically removed from the repository. * - * @param path path whose children need to be deleted + * @param path path of the node whose document and all descendants need to be deleted */ - void deleteDocuments(String path) throws IOException; + void deleteDocumentTree(String path) throws IOException; + + /** + * Deletes only the document at the given path, leaving descendant documents untouched. + * Use this when a node loses indexability at runtime (e.g. mixin removed) while its + * children may still be indexable. + * + *

Default implementation falls back to {@link #deleteDocumentTree} for implementations + * that do not differentiate; override in backends that support exact-document deletion. + * + * @param path path of the node whose document needs to be deleted + */ + void deleteDocument(String path) throws IOException; /** * Closes the underlying resources. 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 f481e3539ec..bb96e14f16c 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 @@ -568,6 +568,34 @@ public void nodeLosesMixinDisappearsFromMixinBasedIndex() throws Exception { assertEventually(() -> assertQuery(query, List.of())); } + @Test + public void parentLosesMixinDoesNotCascadeDeleteChildWithSameMixin() 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", "parent"); + Tree child = createNodeWithMixinType(a, "child", "mix:title"); + child.setProperty("jcr:title", "childTitle"); + root.commit(); + + String childQuery = "select [jcr:path] from [mix:title] where [jcr:title] = 'childTitle'"; + assertEventually(() -> assertQuery(childQuery, List.of("/test/a/child"))); + + // Remove mixin from parent only — child still has mix:title + a = root.getTree("/test/a"); + a.removeProperty(JcrConstants.JCR_MIXINTYPES); + root.commit(); + + // Child must survive — deleteDocumentTree on the parent must not cascade to it + assertEventually(() -> assertQuery(childQuery, List.of("/test/a/child"))); + } + @Test public void indexingBasedOnMixinAndRelativeProps() throws Exception { indexOptions.setIndex(