From add22b5d285a8a2d00928b734db4d6a848cc0e27 Mon Sep 17 00:00:00 2001 From: Teddy Date: Fri, 11 Oct 2024 10:26:13 +0200 Subject: [PATCH] GEN 1654 - Fix Alerts for Test Suites (#18222) * fix: test suite alerts * fix: return testSuites field for test suite alerts * style: ran java linting --- .../subscription/AlertsRuleEvaluator.java | 98 +++++++++++++++---- .../service/formatter/util/FormatterUtil.java | 5 +- .../service/jdbi3/TestCaseRepository.java | 3 +- ...a => AlertsRuleEvaluatorResourceTest.java} | 52 +++++++++- 4 files changed, 133 insertions(+), 25 deletions(-) rename openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/{AlertsRuleEvaluatorTest.java => AlertsRuleEvaluatorResourceTest.java} (79%) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluator.java b/openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluator.java index 13836d950d8..8db0110d157 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluator.java @@ -15,6 +15,7 @@ import static org.openmetadata.service.Entity.THREAD; import static org.openmetadata.service.Entity.USER; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.regex.Matcher; @@ -28,6 +29,7 @@ import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatus import org.openmetadata.schema.entity.teams.Team; import org.openmetadata.schema.entity.teams.User; import org.openmetadata.schema.tests.TestCase; +import org.openmetadata.schema.tests.TestSuite; import org.openmetadata.schema.tests.type.TestCaseResult; import org.openmetadata.schema.tests.type.TestCaseStatus; import org.openmetadata.schema.type.ChangeEvent; @@ -97,27 +99,24 @@ public class AlertsRuleEvaluator { if (nullOrEmpty(ownerReferences)) { entity = Entity.getEntity( - changeEvent.getEntityType(), entity.getId(), "owner", Include.NON_DELETED); + changeEvent.getEntityType(), entity.getId(), "owners", Include.NON_DELETED); ownerReferences = entity.getOwners(); } if (!nullOrEmpty(ownerReferences)) { - for (EntityReference owner : ownerReferences) { - if (USER.equals(owner.getType())) { - User user = Entity.getEntity(Entity.USER, owner.getId(), "", Include.NON_DELETED); - for (String name : ownerNameList) { - if (user.getName().equals(name)) { - return true; - } - } - } else if (TEAM.equals(owner.getType())) { - Team team = Entity.getEntity(Entity.TEAM, owner.getId(), "", Include.NON_DELETED); - for (String name : ownerNameList) { - if (team.getName().equals(name)) { - return true; - } - } - } - } + return matchOwners(ownerReferences, ownerNameList); + } + + if (changeEvent.getEntityType().equals(TEST_CASE)) { + // If we did not match on the owner name and are dealing with a test case, + // check if the match happens on the test suite owner name + TestCase testCase = + Entity.getEntity( + changeEvent.getEntityType(), + entity.getId(), + "testSuites,owners", + Include.NON_DELETED); + Optional> testSuites = Optional.ofNullable(testCase.getTestSuites()); + return testSuites.filter(suites -> testSuiteOwnerMatcher(suites, ownerNameList)).isPresent(); } return false; } @@ -147,6 +146,15 @@ public class AlertsRuleEvaluator { return true; } } + + if (changeEvent.getEntityType().equals(TEST_CASE)) { + // If we did not match on the entity FQN and are dealing with a test case, + // check if the match happens on the test suite FQN + TestCase testCase = ((TestCase) entity); + Optional> testSuites = Optional.ofNullable(testCase.getTestSuites()); + return testSuites.filter(suites -> testSuiteMatcher(suites, entityNames)).isPresent(); + } + return false; } @@ -455,6 +463,14 @@ public class AlertsRuleEvaluator { } } } + + if (changeEvent.getEntityType().equals(TEST_CASE)) { + // If we did not match on the domain and are dealing with a test case, + // check if the match happens on the test suite domain + TestCase testCase = ((TestCase) entity); + Optional> testSuites = Optional.ofNullable(testCase.getTestSuites()); + return testSuites.filter(suites -> testSuiteMatcher(suites, fieldChangeUpdate)).isPresent(); + } return false; } @@ -543,4 +559,50 @@ public class AlertsRuleEvaluator { JsonUtils.pojoToJson(event.getEntity()))); } } + + private boolean testSuiteMatcher(List testSuites, List entityNames) { + for (TestSuite testSuite : testSuites) { + for (String name : entityNames) { + Pattern pattern = Pattern.compile(name); + Matcher matcherTestSuiteFQN = pattern.matcher(testSuite.getFullyQualifiedName()); + if (matcherTestSuiteFQN.find()) return true; + if (testSuite.getDomain() != null) { + Matcher matcherDomainFQN = pattern.matcher(testSuite.getDomain().getFullyQualifiedName()); + if (matcherDomainFQN.find()) return true; + } + } + } + return false; + } + + private boolean testSuiteOwnerMatcher(List testSuites, List ownerNameList) { + boolean match; + for (TestSuite testSuite : testSuites) { + List owners = testSuite.getOwners(); + match = matchOwners(owners, ownerNameList); + if (match) return true; + } + return false; + } + + private boolean matchOwners(List ownerReferences, List ownerNameList) { + for (EntityReference owner : ownerReferences) { + if (USER.equals(owner.getType())) { + User user = Entity.getEntity(Entity.USER, owner.getId(), "", Include.NON_DELETED); + for (String name : ownerNameList) { + if (user.getName().equals(name)) { + return true; + } + } + } else if (TEAM.equals(owner.getType())) { + Team team = Entity.getEntity(Entity.TEAM, owner.getId(), "", Include.NON_DELETED); + for (String name : ownerNameList) { + if (team.getName().equals(name)) { + return true; + } + } + } + } + return false; + } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/formatter/util/FormatterUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/formatter/util/FormatterUtil.java index c48926d365c..805ad8fa904 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/formatter/util/FormatterUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/formatter/util/FormatterUtil.java @@ -316,7 +316,10 @@ public class FormatterUtil { JsonUtils.readOrConvertValue(entityTimeSeries, TestCaseResult.class); TestCase testCase = Entity.getEntityByName( - TEST_CASE, testCaseResult.getTestCaseFQN(), TEST_CASE_RESULT, Include.ALL); + TEST_CASE, + testCaseResult.getTestCaseFQN(), + TEST_CASE_RESULT + ",testSuites", + Include.ALL); ChangeEvent changeEvent = getChangeEvent(updateBy, eventType, testCase.getEntityReference().getType(), testCase); return changeEvent diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java index bef5cd30e74..64e0228cdf1 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java @@ -206,7 +206,8 @@ public class TestCaseRepository extends EntityRepository { return records.stream() .map( testSuiteId -> - Entity.getEntity(TEST_SUITE, testSuiteId.getId(), "", Include.ALL, false) + Entity.getEntity( + TEST_SUITE, testSuiteId.getId(), "owners,domain", Include.ALL, false) .withInherited(true) .withChangeDescription(null)) .toList(); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluatorResourceTest.java similarity index 79% rename from openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluatorTest.java rename to openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluatorResourceTest.java index 3b9a10d7cb7..b149a70c985 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluatorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluatorResourceTest.java @@ -14,7 +14,9 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.openmetadata.schema.api.data.CreateTable; +import org.openmetadata.schema.api.tests.CreateTestCase; import org.openmetadata.schema.entity.data.Table; +import org.openmetadata.schema.tests.TestCase; import org.openmetadata.schema.tests.type.TestCaseResult; import org.openmetadata.schema.tests.type.TestCaseStatus; import org.openmetadata.schema.type.ChangeDescription; @@ -23,20 +25,24 @@ import org.openmetadata.schema.type.Column; import org.openmetadata.schema.type.ColumnDataType; import org.openmetadata.schema.type.EventType; import org.openmetadata.schema.type.FieldChange; +import org.openmetadata.schema.type.Include; import org.openmetadata.service.Entity; import org.openmetadata.service.OpenMetadataApplicationTest; import org.openmetadata.service.resources.EntityResourceTest; import org.openmetadata.service.resources.databases.TableResourceTest; +import org.openmetadata.service.resources.dqtests.TestCaseResourceTest; import org.springframework.expression.EvaluationContext; import org.springframework.expression.spel.support.SimpleEvaluationContext; class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { private static TableResourceTest tableResourceTest; + private static TestCaseResourceTest testCaseResourceTest; @BeforeAll public static void setup(TestInfo test) throws URISyntaxException, IOException { tableResourceTest = new TableResourceTest(); tableResourceTest.setup(test); + testCaseResourceTest = new TestCaseResourceTest(); } @Test @@ -47,6 +53,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); assertTrue(evaluateExpression("matchAnySource('alert')", evaluationContext)); @@ -73,6 +80,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); assertTrue( @@ -81,12 +89,25 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { assertFalse(evaluateExpression("matchAnyOwnerName('tempName')", evaluationContext)); } - // issue: https://github.com/open-metadata/OpenMetadata/issues/10376 - void test_matchAnyEntityFqn(TestInfo test) throws IOException { + @Test + void test_matchAnyEntityFqn() throws IOException { // Create Table Entity + // SpEl parsing fails for non-basic UTF-8 + // https://github.com/open-metadata/OpenMetadata/issues/10376 List columns = List.of(TableResourceTest.getColumn(C1, ColumnDataType.INT, null)); - CreateTable create = tableResourceTest.createRequest(test).withColumns(columns); + CreateTable create = tableResourceTest.createRequest("table").withColumns(columns); Table createdTable = tableResourceTest.createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + createdTable.setFullyQualifiedName( + "ServiceName.DatabaseName.SchemaName." + createdTable.getName()); + CreateTestCase createTestCase = testCaseResourceTest.createRequest("testCase"); + TestCase testCase = + testCaseResourceTest.createAndCheckEntity(createTestCase, ADMIN_AUTH_HEADERS); + testCase = Entity.getEntity(testCase.getEntityReference(), "testSuites", Include.ALL); + testCase.setFullyQualifiedName( + "ServiceName.DatabaseName.SchemaName.table." + testCase.getName()); + String testSuiteFqn = + "ServiceName.DatabaseName.SchemaName.table." + testCase.getName() + ".testSuite"; + testCase.getTestSuites().get(0).setFullyQualifiedName(testSuiteFqn); // Create a change Event with the Entity Table ChangeEvent changeEvent = new ChangeEvent(); @@ -97,11 +118,27 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); String fqn = createdTable.getFullyQualifiedName(); - assertTrue(evaluateExpression("matchAnyEntityFqn('" + fqn + "')", evaluationContext)); - assertFalse(evaluateExpression("matchAnyEntityFqn('testFQN1')", evaluationContext)); + assertTrue(evaluateExpression("matchAnyEntityFqn({'" + fqn + "'})", evaluationContext)); + assertFalse(evaluateExpression("(matchAnyEntityFqn({'FOO'}))", evaluationContext)); + + // Create a change Event with the Entity Test Case + changeEvent = new ChangeEvent(); + changeEvent.setEntityType(Entity.TEST_CASE); + changeEvent.setEntity(testCase); + // Test Entity FQN match for test case + alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); + evaluationContext = + SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() + .withRootObject(alertsRuleEvaluator) + .build(); + assertTrue( + evaluateExpression("matchAnyEntityFqn({'" + testSuiteFqn + "'})", evaluationContext)); + assertFalse(evaluateExpression("(matchAnyEntityFqn({'FOO'}))", evaluationContext)); } @Test @@ -120,6 +157,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); String id = createdTable.getId().toString(); @@ -138,6 +176,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); assertTrue(evaluateExpression("matchAnyEventType('entityCreated')", evaluationContext)); @@ -164,6 +203,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); assertTrue(evaluateExpression("matchTestResult('Success')", evaluationContext)); @@ -180,6 +220,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); assertTrue(evaluateExpression("matchUpdatedBy('test')", evaluationContext)); @@ -200,6 +241,7 @@ class AlertsRuleEvaluatorResourceTest extends OpenMetadataApplicationTest { AlertsRuleEvaluator alertsRuleEvaluator = new AlertsRuleEvaluator(changeEvent); EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() .withRootObject(alertsRuleEvaluator) .build(); assertTrue(evaluateExpression("matchAnyFieldChange('test')", evaluationContext));