diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainRecreateSameNameIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainRecreateSameNameIT.java new file mode 100644 index 000000000000..319d69c7e737 --- /dev/null +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DomainRecreateSameNameIT.java @@ -0,0 +1,222 @@ +/* + * Copyright 2024 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.it.tests; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.awaitility.Awaitility; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.openmetadata.it.factories.DatabaseSchemaTestFactory; +import org.openmetadata.it.factories.DatabaseServiceTestFactory; +import org.openmetadata.it.util.SdkClients; +import org.openmetadata.it.util.TestNamespace; +import org.openmetadata.it.util.TestNamespaceExtension; +import org.openmetadata.schema.api.data.CreateTable; +import org.openmetadata.schema.api.domains.CreateDomain; +import org.openmetadata.schema.entity.data.DatabaseSchema; +import org.openmetadata.schema.entity.data.Table; +import org.openmetadata.schema.entity.domains.Domain; +import org.openmetadata.schema.entity.services.DatabaseService; +import org.openmetadata.schema.type.Column; +import org.openmetadata.schema.type.ColumnDataType; +import org.openmetadata.sdk.client.OpenMetadataClient; +import org.openmetadata.sdk.network.HttpMethod; +import org.openmetadata.sdk.network.RequestOptions; + +/** + * Regression test for GitHub Issue #28923: after a domain is HARD deleted and a new domain is + * created with the same name (same FQN, new UUID), assets that belonged to the deleted domain must + * NOT reappear under the new domain. + * + *

The domain-assets listing filters by {@code domains.fullyQualifiedName} (see {@code + * InheritedFieldEntitySearch.forDomain}). Before the fix, the domain hard-delete search cleanup in + * {@code SearchRepository.deleteOrUpdateChildren} matched the singular {@code domain.id} and ran + * {@code ctx._source.remove('domain')} — but assets store the plural {@code domains} array, so the + * stale domain entry was never stripped from their search documents and a recreated same-FQN domain + * matched those stale docs. + * + *

The activity-history half of #28923 is covered deterministically by {@code + * ActivityFeedPublisherTest} (the activity write path is an async change-event consumer, so an + * end-to-end assertion here would be timing-dependent). + */ +@Execution(ExecutionMode.CONCURRENT) +@ExtendWith(TestNamespaceExtension.class) +public class DomainRecreateSameNameIT { + + @Test + void test_recreatedDomainDoesNotInheritDeletedDomainAssets(TestNamespace ns) throws Exception { + OpenMetadataClient adminClient = SdkClients.adminClient(); + String domainName = ns.shortPrefix() + "_recreate"; + + DatabaseService dbService = DatabaseServiceTestFactory.createPostgres(ns); + DatabaseSchema schema = DatabaseSchemaTestFactory.createSimple(ns, dbService); + + Domain domainV1 = createDomain(adminClient, domainName); + String domainFqn = domainV1.getFullyQualifiedName(); + + Table table = + createTableInDomain( + adminClient, ns.shortPrefix() + "_asset", schema.getFullyQualifiedName(), domainFqn); + + Awaitility.await("asset indexed under original domain") + .atMost(Duration.ofSeconds(30)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted( + () -> + assertTrue( + domainAssetsContain(adminClient, domainV1.getId().toString(), table.getId()), + "Sanity: asset should be listed under the original domain before deletion")); + + Map hardDelete = new HashMap<>(); + hardDelete.put("hardDelete", "true"); + hardDelete.put("recursive", "true"); + adminClient.domains().delete(domainV1.getId().toString(), hardDelete); + + Domain domainV2 = createDomain(adminClient, domainName); + assertNotEquals( + domainV1.getId(), domainV2.getId(), "Recreated domain must be a new entity with a new id"); + + Awaitility.await("recreated domain must not inherit the deleted domain's assets") + .atMost(Duration.ofSeconds(30)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted( + () -> + assertFalse( + domainAssetsContain(adminClient, domainV2.getId().toString(), table.getId()), + "Issue #28923: asset from the hard-deleted domain reappeared under the " + + "recreated same-named domain")); + } + + @Test + void test_recreatedSubdomainDoesNotInheritDeletedSubdomainAssets(TestNamespace ns) + throws Exception { + OpenMetadataClient adminClient = SdkClients.adminClient(); + String parentName = ns.shortPrefix() + "_parent"; + String childName = "child"; + + DatabaseService dbService = DatabaseServiceTestFactory.createPostgres(ns); + DatabaseSchema schema = DatabaseSchemaTestFactory.createSimple(ns, dbService); + + Domain parentV1 = createDomain(adminClient, parentName); + Domain childV1 = createSubdomain(adminClient, childName, parentV1.getFullyQualifiedName()); + + Table table = + createTableInDomain( + adminClient, + ns.shortPrefix() + "_asset", + schema.getFullyQualifiedName(), + childV1.getFullyQualifiedName()); + + Awaitility.await("asset indexed under original subdomain") + .atMost(Duration.ofSeconds(30)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted( + () -> + assertTrue( + domainAssetsContain(adminClient, childV1.getId().toString(), table.getId()), + "Sanity: asset should be listed under the original subdomain before deletion")); + + Map hardDelete = new HashMap<>(); + hardDelete.put("hardDelete", "true"); + hardDelete.put("recursive", "true"); + adminClient.domains().delete(parentV1.getId().toString(), hardDelete); + + // The subdomain strip is an async update-by-query; wait until it settles before recreating so + // the assertion is deterministic rather than racing the in-flight reindex. + Awaitility.await("deleted subdomain stripped from asset search doc") + .atMost(Duration.ofSeconds(60)) + .pollInterval(Duration.ofSeconds(1)) + .until(() -> !assetSearchDocReferencesDomain(adminClient, table.getId(), childV1.getId())); + + Domain parentV2 = createDomain(adminClient, parentName); + Domain childV2 = createSubdomain(adminClient, childName, parentV2.getFullyQualifiedName()); + assertNotEquals( + childV1.getId(), childV2.getId(), "Recreated subdomain must be a new entity with a new id"); + + Awaitility.await("recreated subdomain must not inherit the deleted subdomain's assets") + .atMost(Duration.ofSeconds(30)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted( + () -> + assertFalse( + domainAssetsContain(adminClient, childV2.getId().toString(), table.getId()), + "Issue #28923: asset from the hard-deleted subdomain reappeared under the " + + "recreated same-named subdomain (parent recursive hard delete)")); + } + + private boolean assetSearchDocReferencesDomain( + OpenMetadataClient client, Object assetId, Object domainId) throws Exception { + String response = + client + .getHttpClient() + .executeForString( + HttpMethod.GET, + "/v1/search/query?q=id:" + assetId + "&index=table_search_index&from=0&size=1", + null, + RequestOptions.builder().build()); + return response.contains("\"" + domainId + "\""); + } + + private boolean domainAssetsContain(OpenMetadataClient client, String domainId, Object assetId) + throws Exception { + String response = + client + .getHttpClient() + .executeForString( + HttpMethod.GET, + "/v1/domains/" + domainId + "/assets?limit=100&offset=0", + null, + RequestOptions.builder().build()); + return response.contains("\"id\":\"" + assetId + "\""); + } + + private Domain createDomain(OpenMetadataClient client, String name) { + CreateDomain createDomain = + new CreateDomain() + .withName(name) + .withDomainType(CreateDomain.DomainType.AGGREGATE) + .withDescription("Domain for issue #28923 recreate-same-name regression test"); + return client.domains().create(createDomain); + } + + private Domain createSubdomain(OpenMetadataClient client, String name, String parentFqn) { + CreateDomain createDomain = + new CreateDomain() + .withName(name) + .withDomainType(CreateDomain.DomainType.AGGREGATE) + .withParent(parentFqn) + .withDescription("Subdomain for issue #28923 recreate-same-name regression test"); + return client.domains().create(createDomain); + } + + private Table createTableInDomain( + OpenMetadataClient client, String name, String schemaFqn, String domainFqn) { + Column column = new Column().withName("id").withDataType(ColumnDataType.INT); + CreateTable createTable = + new CreateTable() + .withName(name) + .withDatabaseSchema(schemaFqn) + .withColumns(List.of(column)) + .withDomains(List.of(domainFqn)); + return client.tables().create(createTable); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisher.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisher.java index 1a9815303acc..9a8f98034a35 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisher.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisher.java @@ -24,6 +24,7 @@ import org.openmetadata.schema.entity.events.SubscriptionDestination; import org.openmetadata.schema.entity.feed.Thread; import org.openmetadata.schema.type.ChangeEvent; +import org.openmetadata.schema.type.EventType; import org.openmetadata.schema.utils.JsonUtils; import org.openmetadata.service.Entity; import org.openmetadata.service.apps.bundles.changeEvent.Destination; @@ -58,6 +59,15 @@ public ActivityFeedPublisher( public void sendMessage(ChangeEvent changeEvent, Set recipients) throws EventPublisherException { try { + // Hard delete (soft delete emits ENTITY_SOFT_DELETED): purge the entity's feed threads by + // entity id instead of recording a "Permanently Deleted" thread. deleteByAbout also removes + // the FQN-keyed field_relationship rows, so an entity later recreated with the same fully + // qualified name does not inherit the dead entity's activity (#28923). This runs after the + // delete transaction commits, so it also supersedes this trailing delete event itself. + if (changeEvent.getEventType() == EventType.ENTITY_DELETED) { + feedRepository.deleteByAbout(changeEvent.getEntityId()); + return; + } // Thread are created in FeedRepository Directly if (!changeEvent.getEntityType().equals(Entity.THREAD)) { for (Thread thread : diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index 888752c7a45d..ba61e74f18c2 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java @@ -52,7 +52,8 @@ public interface SearchClient } } """; - String REMOVE_DOMAINS_CHILDREN_SCRIPT = "ctx._source.remove('domain')"; + String REMOVE_DOMAINS_CHILDREN_SCRIPT = + "ctx._source.domains.removeIf(domain -> domain.id == params.id)"; // Updates field if null or if inherited is true and the parent is the same (matched by previous // ID), setting inherited=true on the new object. diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java index f6d689c3ba22..2bb30953676f 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java @@ -2700,10 +2700,14 @@ public void deleteOrUpdateChildren(EntityInterface entity, IndexMapping indexMap String entityType = entity.getEntityReference().getType(); switch (entityType) { case Entity.DOMAIN -> { + // Assets store the domain in the plural "domains" array, so strip the deleted domain from + // every referencing asset by matching domains.id. Matching the singular "domain" field left + // stale references behind, which a same-named recreated domain then inherited (#28923). searchClient.updateChildren( GLOBAL_SEARCH_ALIAS, - new ImmutablePair<>(entityType + ".id", docId), - new ImmutablePair<>(REMOVE_DOMAINS_CHILDREN_SCRIPT, null)); + new ImmutablePair<>("domains.id", docId), + new ImmutablePair<>( + REMOVE_DOMAINS_CHILDREN_SCRIPT, Collections.singletonMap("id", docId))); // we are doing below because we want to delete the data products with domain when domain is // deleted searchClient.deleteEntityByFields( diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisherTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisherTest.java index e86ad1b50baf..b5881fc868db 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisherTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/changeEvent/feed/ActivityFeedPublisherTest.java @@ -16,7 +16,10 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mockConstruction; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Collections; @@ -25,6 +28,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.MockedConstruction; import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; @@ -35,6 +39,7 @@ import org.openmetadata.schema.type.EventType; import org.openmetadata.service.events.errors.EventPublisherException; import org.openmetadata.service.exception.EntityNotFoundException; +import org.openmetadata.service.jdbi3.FeedRepository; import org.openmetadata.service.util.FeedUtils; @ExtendWith(MockitoExtension.class) @@ -83,6 +88,45 @@ void testSendMessage_ThrowsOnNonEntityNotFoundException() { } } + @Test + void testSendMessage_HardDeletePurgesActivityById() throws EventPublisherException { + ChangeEvent event = createChangeEvent(EventType.ENTITY_DELETED); + + try (MockedConstruction feedRepoConstruction = + mockConstruction(FeedRepository.class); + MockedStatic mockedFeedUtils = mockStatic(FeedUtils.class)) { + ActivityFeedPublisher hardDeletePublisher = + new ActivityFeedPublisher(eventSubscription, subscriptionDestination); + + assertDoesNotThrow(() -> hardDeletePublisher.sendMessage(event, Collections.emptySet())); + + FeedRepository feedRepository = feedRepoConstruction.constructed().getFirst(); + verify(feedRepository).deleteByAbout(event.getEntityId()); + mockedFeedUtils.verify(() -> FeedUtils.getThreadWithMessage(any(), any()), never()); + } + } + + @Test + void testSendMessage_SoftDeleteDoesNotPurge() throws EventPublisherException { + ChangeEvent event = createChangeEvent(EventType.ENTITY_SOFT_DELETED); + + try (MockedConstruction feedRepoConstruction = + mockConstruction(FeedRepository.class); + MockedStatic mockedFeedUtils = mockStatic(FeedUtils.class)) { + mockedFeedUtils + .when(() -> FeedUtils.getThreadWithMessage(any(), any())) + .thenReturn(Collections.emptyList()); + ActivityFeedPublisher softDeletePublisher = + new ActivityFeedPublisher(eventSubscription, subscriptionDestination); + + assertDoesNotThrow(() -> softDeletePublisher.sendMessage(event, Collections.emptySet())); + + FeedRepository feedRepository = feedRepoConstruction.constructed().getFirst(); + verify(feedRepository, never()).deleteByAbout(any(UUID.class)); + mockedFeedUtils.verify(() -> FeedUtils.getThreadWithMessage(any(), any())); + } + } + private ChangeEvent createChangeEvent(EventType eventType) { ChangeEvent event = new ChangeEvent(); event.setEventType(eventType); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java index 9e61df69a95e..6a490db996bb 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java @@ -2370,9 +2370,10 @@ void deleteEntityIndexRemovesDomainReferencesAndChildren() throws Exception { .updateChildren( SearchClient.GLOBAL_SEARCH_ALIAS, new org.apache.commons.lang3.tuple.ImmutablePair<>( - "domain.id", domain.getId().toString()), + "domains.id", domain.getId().toString()), new org.apache.commons.lang3.tuple.ImmutablePair<>( - SearchClient.REMOVE_DOMAINS_CHILDREN_SCRIPT, null)); + SearchClient.REMOVE_DOMAINS_CHILDREN_SCRIPT, + Map.of("id", domain.getId().toString()))); verify(searchClient) .deleteEntityByFields( List.of("cluster_domain_search_index"),