Skip to content

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

Open
bhabegger wants to merge 1 commit into
apache:trunkfrom
oak-indexing:issue/OAK-12365
Open

OAK-12365: Delete stale index document when a node's last aggregated property is removed#3086
bhabegger wants to merge 1 commit into
apache:trunkfrom
oak-indexing:issue/OAK-12365

Conversation

@bhabegger

Copy link
Copy Markdown
Contributor

Summary

FulltextIndexEditor.addOrUpdate() calls makeDocument() on every commit touching a node. When the node's indexing rule still matches but its last indexable content came from a relative/aggregated property that was just removed, makeDocument() returns null (nothing to index) and addOrUpdate() previously did nothing further — leaving the stale document in the index indefinitely.

A direct (non-relative) property removal on the same node was already handled correctly: FulltextDocumentMaker.removeProperties() forces dirty=true in that case, so a rebuilt document is written instead of null. The gap is specifically the aggregated-content case, where the root node's own propertiesModified list stays empty.

Fix: in addOrUpdate(), when makeDocument() returns null on an update to a previously-existing node, delete the stale document. Guarded by feature toggle FT_OAK-12365 (enabled by default), following this codebase's convention for bug-fix toggles.

Covers both the OAK-12244 type-tracking-enabled path and the legacy (toggle-disabled) path, since both funnel through addOrUpdate().

Related but distinct from OAK-12244 (mixin add/remove rule transitions): this is a content transition — the rule still matches throughout, but the document's content disappeared.

Test plan

  • LuceneIndexEditor2Test#nodeLosesLastAggregatedPropertyTriggersDocumentDeletion — unit test verifying deleteDocument() is called (new, RED→GREEN)
  • LuceneIndexEditor2Test#nodeLosesLastAggregatedPropertyTriggersDocumentDeletionWhenMixinToggleDisabled — same, with FT_OAK_12244 disabled to confirm the legacy path is also covered
  • PropertyIndexCommonTest#nodeLosesLastAggregatedPropertyDisappearsFromFulltextIndex — integration test using a fulltext CONTAINS() query (a plain equality query can't observe this bug, since Oak's query engine revalidates simple WHERE conditions against the live tree)
  • Full oak-search + oak-lucene suites: 1250 tests, 0 failures, 0 errors

@bhabegger

Copy link
Copy Markdown
Contributor Author

[AI] Addressed feedback from an internal automated review:

  • Registered FT_OAK-12365 as a FeatureToggle on the Whiteboard in both LuceneIndexProviderService and ElasticIndexProviderService — it was declared but never wired, so the kill switch had no effect at runtime.
  • Added a unit test verifying FT_OAK_12365_DISABLE actually suppresses the deletion.
  • Added a unit test verifying that removing one of several aggregated properties does not delete the document (only fully-empty content should).
  • Reset FT_OAK_12365_DISABLE in @Before/@After alongside the existing FT_OAK_12244_DISABLE reset.
  • Added a debug log line on the new deletion path, matching the existing trace log on the update path.

Full oak-search + oak-lucene (1252 tests) and oak-search-elastic (512 tests) suites pass.

…property is removed

FulltextIndexEditor.addOrUpdate() calls makeDocument() on every commit
touching a node. When the node's indexing rule still matches but its last
indexable content came from a relative/aggregated property that just got
removed, makeDocument() returns null (nothing to index) and addOrUpdate()
previously did nothing further, leaving the old document in the index
forever.

A direct (non-relative) property removal on the same node was already
handled correctly: FulltextDocumentMaker.removeProperties() forces
dirty=true in that case, so a rebuilt document is written instead of null.
The gap is specifically the aggregated-content case, where the root's own
propertiesModified list stays empty.

Fix: in addOrUpdate(), when makeDocument() returns null on an update to a
previously-existing node, delete the stale document. Guarded by feature
toggle FT_OAK-12365 (enabled by default), following this codebase's
convention for bug-fix toggles.

Covers both the OAK-12244 type-tracking-enabled path and the legacy
(toggle-disabled) path, since both funnel through addOrUpdate().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants