Skip to content

OAK-12365: Delete stale index document when a node's last aggregated property is removed #3086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bhabegger wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

OAK-12365: Delete stale index document when a node's last aggregated property is removed #3086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .../main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProviderService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,9 @@ private void activate(BundleContext bundleContext, Configuration config) throws
oakRegs.add(whiteboard.register(FeatureToggle.class,
new FeatureToggle(FulltextIndexEditor.FT_OAK_12244, FulltextIndexEditor.FT_OAK_12244_DISABLE),
emptyMap()));
oakRegs.add(whiteboard.register(FeatureToggle.class,
new FeatureToggle(FulltextIndexEditor.FT_OAK_12365, FulltextIndexEditor.FT_OAK_12365_DISABLE),
emptyMap()));
initializeIndexDir(bundleContext, config);
initializeExtractedTextCache(bundleContext, config, statisticsProvider);
tracker = createTracker(bundleContext, config);
Expand Down
116 changes: 116 additions & 0 deletions .../src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,13 @@ public class LuceneIndexEditor2Test {
@Before
public void resetToggles() {
FulltextIndexEditor.FT_OAK_12244_DISABLE.set(false);
FulltextIndexEditor.FT_OAK_12365_DISABLE.set(false);
}

@After
public void restoreToggles() {
FulltextIndexEditor.FT_OAK_12244_DISABLE.set(false);
FulltextIndexEditor.FT_OAK_12365_DISABLE.set(false);
}

private final NodeState root = INITIAL_CONTENT;
Expand Down Expand Up @@ -308,6 +310,120 @@ public void nodeLosesMixinDoesNotTriggerDocumentDeletionWhenToggleDisabled() thr
assertFalse("Mixin tracking disabled: removing mixin should not trigger deleteDocument", writer.deletedPaths.contains("/a"));
}

@Test
public void nodeLosesLastAggregatedPropertyTriggersDocumentDeletion() throws Exception {
// OAK-12365: root's rule/type is unchanged; only the aggregated child content disappears.
LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
defnb.indexRule("nt:base").property("jcr:content/status").propertyIndex();
defnb.aggregateRule("nt:base").include("*");

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: aggregated child property present — root must be indexed
NodeBuilder builder = before.builder();
builder.child("a").child("jcr:content").setProperty("status", "published");
before = hook.processCommit(root, builder.getNodeState(), CommitInfo.EMPTY);
assertTrue("Root with aggregated property should be indexed", writer.docs.containsKey("/a"));

// Commit 2: last aggregated property removed, root's own rule/type unchanged —
// stale root document must be deleted
builder = before.builder();
builder.child("a").child("jcr:content").removeProperty("status");
hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
assertTrue("Removing the last aggregated property should trigger deleteDocument for the root",
writer.deletedPaths.contains("/a"));
}

@Test
public void nodeLosesLastAggregatedPropertyTriggersDocumentDeletionWhenMixinToggleDisabled() throws Exception {
// Fix lives in addOrUpdate(), so it also covers the legacy (FT_OAK_12244 disabled) path.
FulltextIndexEditor.FT_OAK_12244_DISABLE.set(true);

LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
defnb.indexRule("nt:base").property("jcr:content/status").propertyIndex();
defnb.aggregateRule("nt:base").include("*");

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: aggregated child property present — root must be indexed
NodeBuilder builder = before.builder();
builder.child("a").child("jcr:content").setProperty("status", "published");
before = hook.processCommit(root, builder.getNodeState(), CommitInfo.EMPTY);
assertTrue("Root with aggregated property should be indexed", writer.docs.containsKey("/a"));

// Commit 2: last aggregated property removed — stale root document must still be
// deleted even with mixin-transition tracking disabled
builder = before.builder();
builder.child("a").child("jcr:content").removeProperty("status");
hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
assertTrue("Removing the last aggregated property should trigger deleteDocument regardless of FT_OAK_12244",
writer.deletedPaths.contains("/a"));
}

@Test
public void nodeLosesLastAggregatedPropertyDoesNotTriggerDeletionWhenOak12365ToggleDisabled() throws Exception {
FulltextIndexEditor.FT_OAK_12365_DISABLE.set(true);

LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
defnb.indexRule("nt:base").property("jcr:content/status").propertyIndex();
defnb.aggregateRule("nt:base").include("*");

NodeState defnState = defnb.build();
IndexDefinition defn = new IndexDefinition(root, defnState, indexPath);
LuceneIndexEditorContext ctx = newContext(defnState.builder(), defn, true);
EditorHook hook = createHook(ctx);

updateBefore(defnb);

NodeBuilder builder = before.builder();
builder.child("a").child("jcr:content").setProperty("status", "published");
before = hook.processCommit(root, builder.getNodeState(), CommitInfo.EMPTY);
assertTrue("Root with aggregated property should be indexed", writer.docs.containsKey("/a"));

builder = before.builder();
builder.child("a").child("jcr:content").removeProperty("status");
hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
assertFalse("OAK-12365 disabled: removing the last aggregated property should not trigger deleteDocument",
writer.deletedPaths.contains("/a"));
}

@Test
public void nodeKeepsOneOfMultipleAggregatedPropertiesDoesNotTriggerDeletion() throws Exception {
LuceneIndexDefinitionBuilder defnb = new LuceneIndexDefinitionBuilder();
defnb.indexRule("nt:base").property("jcr:content/status").propertyIndex();
defnb.indexRule("nt:base").property("jcr:content/type").propertyIndex();
defnb.aggregateRule("nt:base").include("*");

NodeState defnState = defnb.build();
IndexDefinition defn = new IndexDefinition(root, defnState, indexPath);
LuceneIndexEditorContext ctx = newContext(defnState.builder(), defn, true);
EditorHook hook = createHook(ctx);

updateBefore(defnb);

NodeBuilder builder = before.builder();
builder.child("a").child("jcr:content").setProperty("status", "published");
builder.child("a").child("jcr:content").setProperty("type", "page");
before = hook.processCommit(root, builder.getNodeState(), CommitInfo.EMPTY);
assertTrue("Root with aggregated properties should be indexed", writer.docs.containsKey("/a"));

builder = before.builder();
builder.child("a").child("jcr:content").removeProperty("status");
hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY);
assertFalse("Removing one of several aggregated properties should not delete the root document",
writer.deletedPaths.contains("/a"));
}

private void updateBefore(LuceneIndexDefinitionBuilder defnb) {
NodeBuilder builder = before.builder();
NodeBuilder cb = TestUtil.child(builder, PathUtils.getParentPath(indexPath));
Expand Down
3 changes: 3 additions & 0 deletions ...ain/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ private void activate(BundleContext bundleContext, Config config) {
oakRegs.add(whiteboard.register(FeatureToggle.class,
new FeatureToggle(FulltextIndexEditor.FT_OAK_12244, FulltextIndexEditor.FT_OAK_12244_DISABLE),
emptyMap()));
oakRegs.add(whiteboard.register(FeatureToggle.class,
new FeatureToggle(FulltextIndexEditor.FT_OAK_12365, FulltextIndexEditor.FT_OAK_12365_DISABLE),
emptyMap()));
oakRegs.add(whiteboard.register(FeatureToggle.class,
new FeatureToggle(ElasticIndexStatistics.FT_OAK_12248, ElasticIndexStatistics.FT_OAK_12248_ENABLE),
emptyMap()));
Expand Down
21 changes: 21 additions & 0 deletions ...n/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,22 @@ public class FulltextIndexEditor<D> implements IndexEditor, Aggregate.AggregateR
*/
public static final AtomicBoolean FT_OAK_12244_DISABLE = new AtomicBoolean(false);

/**
* Feature toggle name for OAK-12365.
* When active (default), a node whose last indexed property is removed (rule/type
* unchanged) has its stale index document deleted, instead of being silently left
* behind because {@code makeDocument()} now returns {@code null}.
*/
public static final String FT_OAK_12365 = "FT_OAK-12365";

/**
* Kill switch for the OAK-12365 stale-document deletion. Set to {@code true} to
* revert to the pre-OAK-12365 behavior where a node losing its last indexed property
* leaves its previous document in the index. Default is {@code false} (deletion
* active). Wired to the {@link #FT_OAK_12365} feature toggle at runtime.
*/
public static final AtomicBoolean FT_OAK_12365_DISABLE = new AtomicBoolean(false);

private static final List<Aggregate.Matcher> EMPTY_AGGREGATE_MATCHER_LIST = List.of();

private final FulltextIndexEditorContext<D> context;
Expand Down Expand Up @@ -376,6 +392,11 @@ private boolean addOrUpdate(String path, NodeState state, boolean isUpdate)
context.indexUpdate();
context.getWriter().updateDocument(path, d);
return true;
} else if (isUpdate && !FT_OAK_12365_DISABLE.get()) {
// OAK-12365: node still matches the rule but has no content left to index.
log.debug("[{}] Deleting stale document for {}", getIndexName(), path);
context.indexUpdate();
context.getWriter().deleteDocument(path);
}
} catch (IOException e) {
log.warn("Failed to index the node [{}] due to {}", path, e.toString());
Expand Down
27 changes: 27 additions & 0 deletions ...search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,33 @@ public void nodeLosesMixinDisappearsFromMixinBasedIndex() throws Exception {
assertEventually(() -> assertQuery(query, List.of()));
}

@Test
public void nodeLosesLastAggregatedPropertyDisappearsFromFulltextIndex() throws Exception {
// OAK-12365: root's rule/type is unchanged; only the aggregated child content
// disappears. CONTAINS() is required here since equality queries are revalidated
// against the live tree and wouldn't surface a stale document.
IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder();
builder.noAsync(); // wildcard aggregateRule never converges on the async lane in time
builder.indexRule("nt:base").property("jcr:content/status").propertyIndex();
builder.aggregateRule("nt:base").include("*");
indexOptions.setIndex(root, "test1", builder);
root.commit();

Tree test = root.getTree("/").addChild("test");
test.addChild("a").addChild("jcr:content").setProperty("status", "published");
root.commit();

// ISCHILDNODE('/test'): the wildcard aggregate also propagates the fulltext
// content up to "/test" itself, so scope the query to exclude that ancestor.
String query = "select [jcr:path] from [nt:base] where ISCHILDNODE('/test') and CONTAINS(*, 'published')";
assertEventually(() -> assertQuery(query, List.of("/test/a")));

root.getTree("/test/a/jcr:content").removeProperty("status");
root.commit();

assertEventually(() -> assertQuery(query, List.of()));
}

@Test
public void parentLosesMixinDoesNotCascadeDeleteChildWithSameMixin() throws Exception {
indexOptions.setIndex(
Expand Down