From e827bab8404fa181bd3ef348794538b5854ac6b9 Mon Sep 17 00:00:00 2001 From: Pere Miquel Brull Date: Fri, 14 Feb 2025 14:33:01 +0100 Subject: [PATCH] FIX #19768 - Delete pipelines from logical suites at deletion (#19792) * FIX #19768 - Delete pipelines from logical suites at deletion * FIX #19768 - Delete pipelines from logical suites at deletion * FIX #19768 - Delete pipelines from logical suites at deletion * FIX #19768 - Delete pipelines from logical suites at deletion (cherry picked from commit ea1d2c700dc432137fc2d17d2fef0686e23e967a) --- .../service/jdbi3/EntityRepository.java | 2 +- .../service/jdbi3/TestSuiteRepository.java | 19 +++++ .../service/resources/EntityResourceTest.java | 3 +- .../dqtests/TestSuiteResourceTest.java | 79 +++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java index 9b831318d09..5ff760745c2 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java @@ -1309,7 +1309,7 @@ public abstract class EntityRepository { List.of(Relationship.CONTAINS.ordinal(), Relationship.PARENT_OF.ordinal())); if (childrenRecords.isEmpty()) { - LOG.info("No children to delete"); + LOG.debug("No children to delete"); return; } // Entity being deleted contains children entities diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java index 233a9acf869..55a5d3b6797 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java @@ -432,6 +432,7 @@ public class TestSuiteRepository extends EntityRepository { String updatedBy = securityContext.getUserPrincipal().getName(); preDelete(original, updatedBy); setFieldsInternal(original, putFields); + deleteChildIngestionPipelines(original.getId(), hardDelete, updatedBy); EventType changeType; TestSuite updated = JsonUtils.readValue(JsonUtils.pojoToJson(original), TestSuite.class); @@ -452,6 +453,24 @@ public class TestSuiteRepository extends EntityRepository { return new RestUtil.DeleteResponse<>(updated, changeType); } + /** + * Always delete as if it was marked recursive. Deleting a Logical Suite should + * just go ahead and clean the Ingestion Pipelines + */ + private void deleteChildIngestionPipelines(UUID id, boolean hardDelete, String updatedBy) { + List childrenRecords = + daoCollection + .relationshipDAO() + .findTo(id, entityType, Relationship.CONTAINS.ordinal(), Entity.INGESTION_PIPELINE); + + if (childrenRecords.isEmpty()) { + LOG.debug("No children to delete"); + return; + } + // Delete all the contained entities + deleteChildren(childrenRecords, hardDelete, updatedBy); + } + private void updateTestSummaryFromBucket(JsonObject bucket, TestSummary testSummary) { String key = bucket.getString("key"); Integer count = bucket.getJsonNumber("doc_count").intValue(); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java index c00e20069de..202abac41da 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java @@ -2801,8 +2801,7 @@ public abstract class EntityResourceTest authHeaders) - throws HttpResponseException { + public final T getEntity(UUID id, Map authHeaders) throws HttpResponseException { WebTarget target = getResource(id); target = target.queryParam("fields", allFields); return TestUtils.get(target, entityClass, authHeaders); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestSuiteResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestSuiteResourceTest.java index 9e82ec55f73..ec167504406 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestSuiteResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestSuiteResourceTest.java @@ -35,6 +35,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.openmetadata.schema.api.data.CreateTable; +import org.openmetadata.schema.api.services.ingestionPipelines.CreateIngestionPipeline; import org.openmetadata.schema.api.teams.CreateTeam; import org.openmetadata.schema.api.teams.CreateUser; import org.openmetadata.schema.api.tests.CreateLogicalTestCases; @@ -42,8 +43,11 @@ import org.openmetadata.schema.api.tests.CreateTestCase; import org.openmetadata.schema.api.tests.CreateTestCaseResult; import org.openmetadata.schema.api.tests.CreateTestSuite; import org.openmetadata.schema.entity.data.Table; +import org.openmetadata.schema.entity.services.ingestionPipelines.IngestionPipeline; import org.openmetadata.schema.entity.teams.Team; import org.openmetadata.schema.entity.teams.User; +import org.openmetadata.schema.metadataIngestion.SourceConfig; +import org.openmetadata.schema.metadataIngestion.TestSuitePipeline; import org.openmetadata.schema.tests.TestCase; import org.openmetadata.schema.tests.TestSuite; import org.openmetadata.schema.tests.type.TestCaseStatus; @@ -55,6 +59,7 @@ import org.openmetadata.schema.type.Include; import org.openmetadata.service.Entity; import org.openmetadata.service.resources.EntityResourceTest; import org.openmetadata.service.resources.databases.TableResourceTest; +import org.openmetadata.service.resources.services.ingestionpipelines.IngestionPipelineResourceTest; import org.openmetadata.service.resources.teams.TeamResourceTest; import org.openmetadata.service.resources.teams.UserResourceTest; import org.openmetadata.service.search.models.IndexMapping; @@ -729,6 +734,80 @@ public class TestSuiteResourceTest extends EntityResourceTest testCases1 = new ArrayList<>(); + + // We'll create tests cases for testSuite1 + for (int i = 0; i < 5; i++) { + CreateTestCase createTestCase = + testCaseResourceTest + .createRequest(String.format("test_testSuite_2_%s_", test.getDisplayName()) + i) + .withTestSuite(executableTestSuite.getFullyQualifiedName()); + TestCase testCase = + testCaseResourceTest.createAndCheckEntity(createTestCase, ADMIN_AUTH_HEADERS); + testCases1.add(testCase.getEntityReference()); + } + + // We'll create a logical test suite and associate the test cases to it + CreateTestSuite createTestSuite = createRequest(test); + TestSuite testSuite = createEntity(createTestSuite, ADMIN_AUTH_HEADERS); + addTestCasesToLogicalTestSuite( + testSuite, testCases1.stream().map(EntityReference::getId).collect(Collectors.toList())); + TestSuite logicalTestSuite = getEntity(testSuite.getId(), "*", ADMIN_AUTH_HEADERS); + + // Add ingestion pipeline to the database service + IngestionPipelineResourceTest ingestionPipelineResourceTest = + new IngestionPipelineResourceTest(); + CreateIngestionPipeline createIngestionPipeline = + ingestionPipelineResourceTest + .createRequest(test) + .withService(logicalTestSuite.getEntityReference()); + + TestSuitePipeline testSuitePipeline = new TestSuitePipeline(); + + SourceConfig sourceConfig = new SourceConfig().withConfig(testSuitePipeline); + createIngestionPipeline.withSourceConfig(sourceConfig); + IngestionPipeline ingestionPipeline = + ingestionPipelineResourceTest.createEntity(createIngestionPipeline, ADMIN_AUTH_HEADERS); + + // We can GET the Ingestion Pipeline now + IngestionPipeline actualIngestionPipeline = + ingestionPipelineResourceTest.getEntity(ingestionPipeline.getId(), ADMIN_AUTH_HEADERS); + assertNotNull(actualIngestionPipeline); + + // After deleting the test suite, we can't GET the Ingestion Pipeline + deleteEntity(logicalTestSuite.getId(), true, true, ADMIN_AUTH_HEADERS); + + assertResponse( + () -> + ingestionPipelineResourceTest.getEntity(ingestionPipeline.getId(), ADMIN_AUTH_HEADERS), + NOT_FOUND, + String.format( + "ingestionPipeline instance for %s not found", actualIngestionPipeline.getId())); + + // Test Cases are still there + TestCase testCaseInLogical = + testCaseResourceTest.getEntity(testCases1.get(0).getId(), "*", ADMIN_AUTH_HEADERS); + assertNotNull(testCaseInLogical); + } + @Test void get_listTestSuiteFromSearchWithPagination(TestInfo testInfo) throws IOException { if (supportsSearchIndex) {