fix(domains): deleted domain's assets and activity reappear on same-name recreate (1.13) (#28923) - #31879
fix(domains): deleted domain's assets and activity reappear on same-name recreate (1.13) (#28923)#31879sonika-shah wants to merge 2 commits into
Conversation
) Domain hard-delete search cleanup matched the singular `domain.id` field and ran `ctx._source.remove('domain')`, but assets store their domain in the plural `domains` array. The cleanup was therefore a no-op for regular assets: their search documents kept the deleted domain's entry (id + fullyQualifiedName). Because the domain-assets listing filters by `domains.fullyQualifiedName` (InheritedFieldEntitySearch.forDomain, includeDeleted=true), recreating a domain with the same name (same FQN, new UUID) matched those stale docs and the old assets reappeared under the new domain — including assets in subdomains cleared per-subdomain by the recursive delete cascade. Match the plural `domains.id` and remove the stale entry via `domains.removeIf(domain -> domain.id == params.id)`, mirroring the DATA_PRODUCT branch. Adds DomainRecreateSameNameIT (domain + subdomain cases) and updates the existing SearchRepositoryBehaviorTest assertion.
…me-name entity does not inherit them (#28923) The legacy activity feed (`thread_entity`) is listed for an entity by FQN hash (`field_relationship.toFQNHash = MD5(fqn)`), not by entity id. The "Permanently Deleted <entity>" thread is written by the async ActivityFeedPublisher AFTER the delete transaction commits, so the in-transaction cleanup() purge cannot catch it. As a result, hard-deleting a domain and recreating one with the same name (same FQN) resurfaced the dead domain's activity under the new domain. In ActivityFeedPublisher, on ENTITY_DELETED (hard delete; soft delete emits ENTITY_SOFT_DELETED) purge the entity's threads via feedRepository.deleteByAbout(entityId) — which also removes the FQN-keyed field_relationship rows — instead of writing the deletion thread. This runs in the post-commit consumer, so it removes the history and supersedes the trailing delete event; keyed by entity id so it can never remove a same-named successor's threads. Adds ActivityFeedPublisherTest cases for hard vs soft delete.
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
backend
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // delete transaction commits, so it also supersedes this trailing delete event itself. | ||
| if (changeEvent.getEventType() == EventType.ENTITY_DELETED) { | ||
| feedRepository.deleteByAbout(changeEvent.getEntityId()); | ||
| return; |
There was a problem hiding this comment.
The new hard-delete branch introduces an early return, while the accompanying integration tests also contain oversized multi-responsibility methods and raw JSON literals passed to contains. Restructure sendMessage to use a single trailing return, extract focused test helpers, and replace the repeated response fragments with named constants to comply with the repository’s Java standards.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Code Review ✅ ApprovedBackports a fix for domain recreation to 1.13 by correcting search document asset cleanup and legacy feed thread purging on hard delete. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
1 participant
Backport of #31798 to 1.13. Fixes #28923.
The asset-side fix is the same as main; the activity-side fix is rewritten for 1.13's architecture (this is not a straight cherry-pick — 1.13 has no
activity_stream/ActivityStreamPublisher; it uses the legacythread_entityfeed).Problem
After a domain is hard deleted and a new domain is created with the same name (same FQN, new UUID), the deleted domain's assets and activity history reappear under the new domain. Both read paths are FQN-keyed, so a reused FQN inherits the dead entity's data.
1. Assets — stale search documents (same as main)
getDomainAssetslists assets bydomains.fullyQualifiedName(InheritedFieldEntitySearch.forDomain,includeDeleted=true). On hard delete, the DOMAIN branch ofSearchRepository.deleteOrUpdateChildrenmatched the singulardomain.idand ranctx._source.remove('domain'), but assets store the pluraldomainsarray — so nothing was stripped and the recreated same-FQN domain matched the stale docs.Fix: match
domains.idand strip viactx._source.domains.removeIf(domain -> domain.id == params.id), mirroring theDATA_PRODUCTbranch.2. Activity history — rewritten for the legacy feed
1.13 stores activity in
thread_entity, listed for an entity by FQN hash (field_relationship.toFQNHash = MD5(fqn)), not by entity id. The "Permanently Deleted " thread is written by the asyncActivityFeedPublisherafter the delete transaction commits, so the in-transactioncleanup()purge cannot catch it; the recreated same-FQN domain then inherits it.Fix: in
ActivityFeedPublisher.sendMessage, onENTITY_DELETED(hard delete — soft delete emitsENTITY_SOFT_DELETED) purge the entity's threads viafeedRepository.deleteByAbout(entityId)(which also removes the FQN-keyedfield_relationshiprows) and skip writing the deletion thread. Runs in the post-commit consumer, so it removes the history and supersedes the trailing delete event; keyed by entity id so it can never remove a same-named successor's threads. (Same principle as main'sActivityStreamPublisherchange, adapted to the legacy DAO.)Testing (all green on a fresh 1.13 instance)
DomainRecreateSameNameIT— domain and subdomain (recursive parent delete) asset cases; the recursive delete uses the per-entity path so the strip runs per subdomain.Tests run: 2, Failures: 0.ActivityFeedPublisherTest— hard delete purges by id / soft delete does not.Tests run: 4, Failures: 0.SearchRepositoryBehaviorTest— updated the domain hard-delete assertion to the correcteddomains.idmatch.Tests run: 2, Failures: 0.Files
SearchRepository.java,SearchClient.java— asset search cleanup.ActivityFeedPublisher.java— legacy-feed purge on hard delete.DomainRecreateSameNameIT.java,ActivityFeedPublisherTest.java,SearchRepositoryBehaviorTest.java— tests.Greptile Summary
This backport prevents a recreated domain from inheriting the hard-deleted predecessor’s assets and legacy activity feed.
domainsarrays in search documents.Confidence Score: 4/5
The PR appears safe to merge functionally, with a non-blocking repository-style cleanup required for the new Java control flow and integration tests.
The UUID-keyed feed purge and plural-domain search cleanup address the stale-association paths without an identified behavioral regression; the remaining accepted concern is maintainability and standards compliance.
Files Needing Attention: openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisher.java; openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainRecreateSameNameIT.java
Important Files Changed
Sequence Diagram
Reviews (1): Last reviewed commit: "fix(feed): purge entity feed threads on ..." | Re-trigger Greptile
Context used (3)