diff --git a/ingestion/examples/sample_data/tests/testSuites.json b/ingestion/examples/sample_data/tests/testSuites.json index 0e9ecdccd87..cf241c8e698 100644 --- a/ingestion/examples/sample_data/tests/testSuites.json +++ b/ingestion/examples/sample_data/tests/testSuites.json @@ -19,10 +19,6 @@ ], "resolutions": { "sequenceOne": [ - { - "testCaseResolutionStatusType": "New", - "severity": "Severity1" - }, { "testCaseResolutionStatusType": "Ack", "severity": "Severity1" @@ -108,10 +104,6 @@ ], "resolutions": { "sequenceOne": [ - { - "testCaseResolutionStatusType": "New", - "severity": "Severity1" - }, { "testCaseResolutionStatusType": "Ack", "severity": "Severity1" @@ -186,10 +178,6 @@ ], "resolutions": { "sequenceOne": [ - { - "testCaseResolutionStatusType": "New", - "severity": "Severity1" - }, { "testCaseResolutionStatusType": "Ack", "severity": "Severity1" @@ -264,10 +252,6 @@ ], "resolutions": { "sequenceOne": [ - { - "testCaseResolutionStatusType": "New", - "severity": "Severity1" - }, { "testCaseResolutionStatusType": "Ack", "severity": "Severity1" diff --git a/ingestion/src/metadata/ingestion/models/tests_data.py b/ingestion/src/metadata/ingestion/models/tests_data.py index 57a73442c4d..d30f05d3181 100644 --- a/ingestion/src/metadata/ingestion/models/tests_data.py +++ b/ingestion/src/metadata/ingestion/models/tests_data.py @@ -17,6 +17,9 @@ from typing import List from pydantic import BaseModel from metadata.generated.schema.api.tests.createTestCase import CreateTestCaseRequest +from metadata.generated.schema.api.tests.createTestCaseResolutionStatus import ( + CreateTestCaseResolutionStatus, +) from metadata.generated.schema.api.tests.createTestSuite import CreateTestSuiteRequest from metadata.generated.schema.tests.basic import TestCaseResult from metadata.generated.schema.tests.testCase import TestCase @@ -38,3 +41,9 @@ class OMetaTestCaseSample(BaseModel): class OMetaTestCaseResultsSample(BaseModel): test_case_results: TestCaseResult test_case_name: str + + +class OMetaTestCaseResolutionStatus(BaseModel): + """For sample data""" + + test_case_resolution: CreateTestCaseResolutionStatus diff --git a/ingestion/src/metadata/ingestion/sink/metadata_rest.py b/ingestion/src/metadata/ingestion/sink/metadata_rest.py index 4b9270acc84..bccd1775f14 100644 --- a/ingestion/src/metadata/ingestion/sink/metadata_rest.py +++ b/ingestion/src/metadata/ingestion/sink/metadata_rest.py @@ -47,6 +47,9 @@ from metadata.generated.schema.entity.teams.team import Team from metadata.generated.schema.entity.teams.user import User from metadata.generated.schema.tests.basic import TestCaseResult from metadata.generated.schema.tests.testCase import TestCase +from metadata.generated.schema.tests.testCaseResolutionStatus import ( + TestCaseResolutionStatus, +) from metadata.generated.schema.tests.testSuite import TestSuite from metadata.generated.schema.type.schema import Topic from metadata.ingestion.api.models import Either, Entity, StackTraceError @@ -67,6 +70,7 @@ from metadata.ingestion.models.profile_data import OMetaTableProfileSampleData from metadata.ingestion.models.search_index_data import OMetaIndexSampleData from metadata.ingestion.models.tests_data import ( OMetaLogicalTestSuiteSample, + OMetaTestCaseResolutionStatus, OMetaTestCaseResultsSample, OMetaTestCaseSample, OMetaTestSuiteSample, @@ -411,6 +415,15 @@ class MetadataRestSink(Sink): # pylint: disable=too-many-public-methods ) return Either(right=res) + @_run_dispatch.register + def write_test_case_resolution_status( + self, record: OMetaTestCaseResolutionStatus + ) -> TestCaseResolutionStatus: + """For sample data""" + res = self.metadata.create_test_case_resolution(record.test_case_resolution) + + return Either(right=res) + @_run_dispatch.register def write_data_insight_sample( self, record: OMetaDataInsightSample diff --git a/ingestion/src/metadata/ingestion/source/database/sample_data.py b/ingestion/src/metadata/ingestion/source/database/sample_data.py index 1a1fe42d101..582080fe7b9 100644 --- a/ingestion/src/metadata/ingestion/source/database/sample_data.py +++ b/ingestion/src/metadata/ingestion/source/database/sample_data.py @@ -118,6 +118,7 @@ from metadata.ingestion.models.pipeline_status import OMetaPipelineStatus from metadata.ingestion.models.profile_data import OMetaTableProfileSampleData from metadata.ingestion.models.tests_data import ( OMetaLogicalTestSuiteSample, + OMetaTestCaseResolutionStatus, OMetaTestCaseResultsSample, OMetaTestCaseSample, OMetaTestSuiteSample, @@ -566,6 +567,7 @@ class SampleDataSource( yield from self.ingest_test_suite() yield from self.ingest_test_case() yield from self.ingest_test_case_results() + yield from self.ingest_incidents() yield from self.ingest_logical_test_suite() yield from self.ingest_data_insights() yield from self.ingest_life_cycle() @@ -1408,6 +1410,15 @@ class SampleDataSource( ) yield Either(right=test_case_req) + def ingest_incidents(self) -> Iterable[Either[OMetaTestCaseResolutionStatus]]: + """ + Ingest incidents after the first test failures have been added. + + The test failure already creates the incident with NEW, so we + start always from ACK in the sample flows. + """ + for test_suite in self.tests_suites["tests"]: + for test_case in test_suite["testCases"]: test_case_fqn = f"{entity_link.get_table_or_column_fqn(test_case['entityLink'])}.{test_case['name']}" for _, resolutions in test_case["resolutions"].items(): @@ -1449,8 +1460,10 @@ class SampleDataSource( testCaseFailureComment="Resolution comment", ) - self.metadata.create_test_case_resolution( - create_test_case_resolution + yield Either( + right=OMetaTestCaseResolutionStatus( + test_case_resolution=create_test_case_resolution + ) ) def ingest_test_case_results(self) -> Iterable[Either[OMetaTestCaseResultsSample]]: diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/exception/IncidentManagerException.java b/openmetadata-service/src/main/java/org/openmetadata/service/exception/IncidentManagerException.java new file mode 100644 index 00000000000..500d7cf795f --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/exception/IncidentManagerException.java @@ -0,0 +1,23 @@ +package org.openmetadata.service.exception; + +import javax.ws.rs.core.Response; +import org.openmetadata.schema.tests.type.TestCaseResolutionStatusTypes; +import org.openmetadata.sdk.exception.WebServiceException; + +public class IncidentManagerException extends WebServiceException { + + protected IncidentManagerException(Response.Status status, String message) { + super(status.getStatusCode(), message); + } + + public IncidentManagerException(String message) { + super(Response.Status.INTERNAL_SERVER_ERROR, message); + } + + public static IncidentManagerException invalidStatus( + TestCaseResolutionStatusTypes lastStatus, TestCaseResolutionStatusTypes newStatus) { + return new IncidentManagerException( + Response.Status.BAD_REQUEST, + String.format("Incident with status [%s] cannot be moved to [%s]", lastStatus, newStatus)); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index 6202253ff84..0446bffc469 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -3592,6 +3592,32 @@ public interface CollectionDAO { default String getTimeSeriesTableName() { return "data_quality_data_time_series"; } + + @ConnectionAwareSqlQuery( + value = + "SELECT json FROM data_quality_data_time_series where entityFQNHash = :testCaseFQNHash " + + "AND JSON_EXTRACT(json, '$.incidentId') IS NOT NULL", + connectionType = MYSQL) + @ConnectionAwareSqlQuery( + value = + "SELECT json FROM data_quality_data_time_series where entityFQNHash = :testCaseFQNHash " + + "AND json ->> 'incidentId' IS NOT NULL", + connectionType = POSTGRES) + List getResultsWithIncidents(@Bind("testCaseFQNHash") String testCaseFQNHash); + + @ConnectionAwareSqlUpdate( + value = + "SELECT json FROM data_quality_data_time_series where entityFQNHash = :entityFQNHash " + + "AND JSON_EXTRACT(json, '$.incidentId') IS NOT NULL", + connectionType = MYSQL) + @ConnectionAwareSqlUpdate( + value = + "SELECT json FROM data_quality_data_time_series where entityFQNHash = :entityFQNHash " + + "AND json ->> 'incidentId' IS NOT NULL", + connectionType = POSTGRES) + // TODO: need to find the right way to get this cleaned + void cleanTestCaseIncidents( + @Bind("entityFQNHash") String entityFQNHash, @Bind("stateId") String stateId); } interface TestCaseResolutionStatusTimeSeriesDAO extends EntityTimeSeriesDAO { @@ -3605,6 +3631,10 @@ public interface CollectionDAO { "SELECT json FROM test_case_resolution_status_time_series " + "WHERE stateId = :stateId ORDER BY timestamp DESC") List listTestCaseResolutionStatusesForStateId(@Bind("stateId") String stateId); + + @SqlUpdate( + "DELETE FROM test_case_resolution_status_time_series WHERE entityFQNHash = :entityFQNHash") + void delete(@BindFQN("entityFQNHash") String entityFQNHash); } class EntitiesCountRowMapper implements RowMapper { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java index 12f1ceeea58..82a8688dbb4 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java @@ -19,6 +19,7 @@ public abstract class EntityTimeSeriesRepository entityClass; + @Getter protected final CollectionDAO daoCollection; protected EntityTimeSeriesRepository( String collectionPath, @@ -30,6 +31,7 @@ public abstract class EntityTimeSeriesRepository { private static final String TEST_SUITE_FIELD = "testSuite"; private static final String TEST_CASE_RESULT_FIELD = "testCaseResult"; + private static final String INCIDENTS_FIELD = "incidents"; public static final String COLLECTION_PATH = "/v1/dataQuality/testCases"; private static final String UPDATE_FIELDS = "owner,entityLink,testSuite,testDefinition"; private static final String PATCH_FIELDS = "owner,entityLink,testSuite,testDefinition"; @@ -79,10 +81,12 @@ public class TestCaseRepository extends EntityRepository { test.setTestSuite(fields.contains(TEST_SUITE_FIELD) ? getTestSuite(test) : test.getTestSuite()); test.setTestDefinition( fields.contains(TEST_DEFINITION) ? getTestDefinition(test) : test.getTestDefinition()); - test.withTestCaseResult( + test.setTestCaseResult( fields.contains(TEST_CASE_RESULT_FIELD) ? getTestCaseResult(test) : test.getTestCaseResult()); + test.setIncidents( + fields.contains(INCIDENTS_FIELD) ? getIncidentIds(test) : test.getIncidents()); } @Override @@ -90,7 +94,7 @@ public class TestCaseRepository extends EntityRepository { test.setTestSuites(fields.contains("testSuites") ? test.getTestSuites() : null); test.setTestSuite(fields.contains(TEST_SUITE) ? test.getTestSuite() : null); test.setTestDefinition(fields.contains(TEST_DEFINITION) ? test.getTestDefinition() : null); - test.withTestCaseResult( + test.setTestCaseResult( fields.contains(TEST_CASE_RESULT_FIELD) ? test.getTestCaseResult() : null); } @@ -228,16 +232,22 @@ public class TestCaseRepository extends EntityRepository { Relationship.APPLIED_TO); } + @Override + protected void postDelete(TestCase test) { + // If we delete the test case, we need to clean up the resolution ts + daoCollection.testCaseResolutionStatusTimeSeriesDao().delete(test.getFullyQualifiedName()); + } + public RestUtil.PutResponse addTestCaseResult( String updatedBy, UriInfo uriInfo, String fqn, TestCaseResult testCaseResult) { // Validate the request content TestCase testCase = findByName(fqn, Include.NON_DELETED); // set the test case resolution status reference if test failed - testCaseResult.setTestCaseResolutionStatusReference( - testCaseResult.getTestCaseStatus() != TestCaseStatus.Failed - ? null - : setTestCaseResolutionStatus(testCase, updatedBy)); + testCaseResult.setIncidentId( + testCaseResult.getTestCaseStatus() == TestCaseStatus.Failed + ? createIncidentOnFailure(testCase, updatedBy) + : null); daoCollection .dataQualityDataTimeSeriesDao() @@ -260,8 +270,12 @@ public class TestCaseRepository extends EntityRepository { Response.Status.CREATED, changeEvent, RestUtil.ENTITY_FIELDS_CHANGED); } - private TestCaseResolutionStatus setTestCaseResolutionStatus( - TestCase testCase, String updatedBy) { + private UUID createIncidentOnFailure(TestCase testCase, String updatedBy) { + + TestCaseResolutionStatusRepository testCaseResolutionStatusRepository = + (TestCaseResolutionStatusRepository) + Entity.getEntityTimeSeriesRepository(Entity.TEST_CASE_RESOLUTION_STATUS); + String json = daoCollection .testCaseResolutionStatusTimeSeriesDao() @@ -270,21 +284,27 @@ public class TestCaseRepository extends EntityRepository { TestCaseResolutionStatus storedTestCaseResolutionStatus = json != null ? JsonUtils.readValue(json, TestCaseResolutionStatus.class) : null; - if ((storedTestCaseResolutionStatus != null) - && (storedTestCaseResolutionStatus.getTestCaseResolutionStatusType() - != TestCaseResolutionStatusTypes.Resolved)) { - // if we already have a non resolve status then we'll simply return it - return storedTestCaseResolutionStatus; + // if we already have a non resolve status then we'll simply return it + if (Boolean.TRUE.equals( + testCaseResolutionStatusRepository.unresolvedIncident(storedTestCaseResolutionStatus))) { + return storedTestCaseResolutionStatus.getStateId(); } // if the test case resolution is null or resolved then we'll create a new one - return new TestCaseResolutionStatus() - .withStateId(UUID.randomUUID()) - .withTimestamp(System.currentTimeMillis()) - .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.New) - .withUpdatedBy(getEntityReferenceByName(Entity.USER, updatedBy, Include.ALL)) - .withUpdatedAt(System.currentTimeMillis()) - .withTestCaseReference(testCase.getEntityReference()); + TestCaseResolutionStatus status = + new TestCaseResolutionStatus() + .withStateId(UUID.randomUUID()) + .withTimestamp(System.currentTimeMillis()) + .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.New) + .withUpdatedBy(getEntityReferenceByName(Entity.USER, updatedBy, Include.ALL)) + .withUpdatedAt(System.currentTimeMillis()) + .withTestCaseReference(testCase.getEntityReference()); + + TestCaseResolutionStatus incident = + testCaseResolutionStatusRepository.createNewRecord( + status, null, testCase.getFullyQualifiedName()); + + return incident.getStateId(); } public RestUtil.PutResponse deleteTestCaseResult( @@ -487,6 +507,26 @@ public class TestCaseRepository extends EntityRepository { testCaseResults, String.valueOf(startTs), String.valueOf(endTs), testCaseResults.size()); } + /** + * Check all the test case results that have an ongoing incident and get the stateId of the incident + */ + private List getIncidentIds(TestCase test) { + List testCaseResults; + testCaseResults = + JsonUtils.readObjects( + daoCollection + .dataQualityDataTimeSeriesDao() + .getResultsWithIncidents( + FullyQualifiedName.buildHash(test.getFullyQualifiedName())), + TestCaseResult.class); + + return testCaseResults.stream() + .map(TestCaseResult::getIncidentId) + .collect(Collectors.toSet()) + .stream() + .toList(); + } + public int getTestCaseCount(List testCaseIds) { return daoCollection.testCaseDAO().countOfTestCases(testCaseIds); } @@ -683,6 +723,9 @@ public class TestCaseRepository extends EntityRepository { Entity.getEntityTimeSeriesRepository(Entity.TEST_CASE_RESOLUTION_STATUS); } + /** + * If the task is resolved, we'll resolve the Incident with the given reason + */ @Override @Transaction public TestCase performTask(String userName, ResolveTask resolveTask) { @@ -719,14 +762,20 @@ public class TestCaseRepository extends EntityRepository { Entity.TEST_CASE_RESOLUTION_STATUS, JsonUtils.pojoToJson(testCaseResolutionStatus)); testCaseResolutionStatusRepository.postCreate(testCaseResolutionStatus); + + // TODO: remove incident ID from test case result + return Entity.getEntity(testCaseResolutionStatus.getTestCaseReference(), "", Include.ALL); } + /** + * If we close the task, we'll flag the incident as Resolved as a False Positive, if + * it is not resolved yet. + * Closing the task means that the incident is not applicable. + */ @Override @Transaction public void closeTask(String userName, CloseTask closeTask) { - // closing task in the context of test case resolution status means that the resolution task - // has been reassigned to someone else TestCaseResolutionStatus latestTestCaseResolutionStatus = testCaseResolutionStatusRepository.getLatestRecord(closeTask.getTestCaseFQN()); if (latestTestCaseResolutionStatus == null) { @@ -740,19 +789,23 @@ public class TestCaseRepository extends EntityRepository { return; } - User user = Entity.getEntityByName(Entity.USER, userName, "", Include.ALL); - User assignee = Entity.getEntityByName(Entity.USER, closeTask.getComment(), "", Include.ALL); + User user = getEntityByName(Entity.USER, userName, "", Include.ALL); TestCaseResolutionStatus testCaseResolutionStatus = new TestCaseResolutionStatus() .withId(UUID.randomUUID()) .withStateId(latestTestCaseResolutionStatus.getStateId()) .withTimestamp(System.currentTimeMillis()) - .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.Assigned) + .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.Resolved) .withTestCaseResolutionStatusDetails( - new Assigned().withAssignee(assignee.getEntityReference())) + new Resolved() + .withTestCaseFailureComment(closeTask.getComment()) + // If we close the task directly we won't know the reason + .withTestCaseFailureReason(TestCaseFailureReasonType.FalsePositive) + .withResolvedBy(user.getEntityReference())) .withUpdatedAt(System.currentTimeMillis()) .withTestCaseReference(latestTestCaseResolutionStatus.getTestCaseReference()) .withUpdatedBy(user.getEntityReference()); + Entity.getCollectionDAO() .testCaseResolutionStatusTimeSeriesDao() .insert( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java index 5cf814930d8..6a2ae22da4c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java @@ -11,7 +11,6 @@ import java.util.UUID; import javax.json.JsonPatch; import javax.ws.rs.core.Response; import org.jdbi.v3.sqlobject.transaction.Transaction; -import org.openmetadata.schema.api.feed.CloseTask; import org.openmetadata.schema.api.feed.ResolveTask; import org.openmetadata.schema.entity.feed.Thread; import org.openmetadata.schema.entity.teams.User; @@ -22,12 +21,14 @@ import org.openmetadata.schema.tests.type.TestCaseResolutionStatus; import org.openmetadata.schema.tests.type.TestCaseResolutionStatusTypes; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.Include; +import org.openmetadata.schema.type.Relationship; import org.openmetadata.schema.type.TaskDetails; import org.openmetadata.schema.type.TaskStatus; import org.openmetadata.schema.type.TaskType; import org.openmetadata.schema.type.ThreadType; import org.openmetadata.service.Entity; import org.openmetadata.service.exception.EntityNotFoundException; +import org.openmetadata.service.exception.IncidentManagerException; import org.openmetadata.service.resources.feeds.MessageParser; import org.openmetadata.service.util.EntityUtil; import org.openmetadata.service.util.JsonUtils; @@ -96,11 +97,47 @@ public class TestCaseResolutionStatusRepository } } - @Override - protected void postCreate(TestCaseResolutionStatus entity) { - super.postCreate(entity); - if (entity.getTestCaseResolutionStatusType() == TestCaseResolutionStatusTypes.Assigned) { - createAssignedTask(entity); + public Boolean unresolvedIncident(TestCaseResolutionStatus incident) { + return incident != null + && !incident + .getTestCaseResolutionStatusType() + .equals(TestCaseResolutionStatusTypes.Resolved); + } + + private Thread getIncidentTask(TestCaseResolutionStatus incident) { + // Fetch the latest task (which comes from the NEW state) and close it + String jsonThread = + Entity.getCollectionDAO() + .feedDAO() + .fetchThreadByTestCaseResolutionStatusId(incident.getStateId()); + return JsonUtils.readValue(jsonThread, Thread.class); + } + + /** + * Ensure we are following the correct status flow + */ + private void validateStatus( + TestCaseResolutionStatusTypes lastStatus, TestCaseResolutionStatusTypes newStatus) { + switch (lastStatus) { + case New -> { + /* New can go to any status */ + } + case Ack -> { + if (newStatus.equals(TestCaseResolutionStatusTypes.New)) { + throw IncidentManagerException.invalidStatus(lastStatus, newStatus); + } + } + case Assigned -> { + if (List.of(TestCaseResolutionStatusTypes.New, TestCaseResolutionStatusTypes.Ack) + .contains(newStatus)) { + throw IncidentManagerException.invalidStatus(lastStatus, newStatus); + } + } + case Resolved -> { + if (!newStatus.equals(TestCaseResolutionStatusTypes.Resolved)) { + throw IncidentManagerException.invalidStatus(lastStatus, newStatus); + } + } } } @@ -108,109 +145,156 @@ public class TestCaseResolutionStatusRepository @Transaction public TestCaseResolutionStatus createNewRecord( TestCaseResolutionStatus recordEntity, String extension, String recordFQN) { - TestCaseResolutionStatus latestTestCaseFailure = + + TestCaseResolutionStatus lastIncident = getLatestRecord(recordEntity.getTestCaseReference().getFullyQualifiedName()); - recordEntity.setStateId( - ((latestTestCaseFailure != null) - && (latestTestCaseFailure.getTestCaseResolutionStatusType() - != TestCaseResolutionStatusTypes.Resolved)) - ? latestTestCaseFailure.getStateId() - : UUID.randomUUID()); + if (recordEntity.getStateId() == null) { + recordEntity.setStateId(UUID.randomUUID()); + } - if (latestTestCaseFailure != null - && latestTestCaseFailure - .getTestCaseResolutionStatusType() - .equals(TestCaseResolutionStatusTypes.Assigned)) { - String jsonThread = - Entity.getCollectionDAO() - .feedDAO() - .fetchThreadByTestCaseResolutionStatusId(latestTestCaseFailure.getId()); - Thread thread = JsonUtils.readValue(jsonThread, Thread.class); - if (recordEntity - .getTestCaseResolutionStatusType() - .equals(TestCaseResolutionStatusTypes.Assigned)) { - // We have an open task and we are passing an assigned status type - // (i.e. we are re-assigning). This scenario is when the test case resolution status is - // being sent through the API (and not resolving an open task) - // we'll get the associated thread with the latest test case failure + // if we have an ongoing incident, set the stateId if the new record to be created + // and validate the flow + if (Boolean.TRUE.equals(unresolvedIncident(lastIncident))) { + recordEntity.setStateId(lastIncident.getStateId()); + validateStatus( + lastIncident.getTestCaseResolutionStatusType(), + recordEntity.getTestCaseResolutionStatusType()); + } - // we'll close the task (the flow will also create a new assigned test case resolution - // status and open a new task) - Assigned assigned = - JsonUtils.convertValue( - recordEntity.getTestCaseResolutionStatusDetails(), Assigned.class); - User assignee = - Entity.getEntity(Entity.USER, assigned.getAssignee().getId(), "", Include.ALL); - User updatedBy = - Entity.getEntity(Entity.USER, recordEntity.getUpdatedBy().getId(), "", Include.ALL); - CloseTask closeTask = - new CloseTask() - .withComment(assignee.getFullyQualifiedName()) - .withTestCaseFQN(recordEntity.getTestCaseReference().getFullyQualifiedName()); - Entity.getFeedRepository().closeTask(thread, updatedBy.getFullyQualifiedName(), closeTask); - return getLatestRecord(recordEntity.getTestCaseReference().getFullyQualifiedName()); - } else if (recordEntity - .getTestCaseResolutionStatusType() - .equals(TestCaseResolutionStatusTypes.Resolved)) { - // We have an open task and we are passing a resolved status type (i.e. we are marking it as - // resolved). This scenario is when the test case resolution status is being sent through - // the API (and not resolving an open task) - Resolved resolved = - JsonUtils.convertValue( - recordEntity.getTestCaseResolutionStatusDetails(), Resolved.class); - TestCase testCase = - Entity.getEntity( - Entity.TEST_CASE, recordEntity.getTestCaseReference().getId(), "", Include.ALL); - User updatedBy = - Entity.getEntity(Entity.USER, recordEntity.getUpdatedBy().getId(), "", Include.ALL); - ResolveTask resolveTask = - new ResolveTask() - .withTestCaseFQN(testCase.getFullyQualifiedName()) - .withTestCaseFailureReason(resolved.getTestCaseFailureReason()) - .withNewValue(resolved.getTestCaseFailureComment()); - Entity.getFeedRepository() - .resolveTask( - new FeedRepository.ThreadContext(thread), - updatedBy.getFullyQualifiedName(), - resolveTask); - return getLatestRecord(testCase.getFullyQualifiedName()); + switch (recordEntity.getTestCaseResolutionStatusType()) { + // When we create a NEW incident, we need to open a task with the test case owner as the + // assignee. We don't need to check any past history + case New -> { + // If there is already an unresolved incident, return it without doing any + // further logic. + if (Boolean.TRUE.equals(unresolvedIncident(lastIncident))) { + return getLatestRecord(lastIncident.getTestCaseReference().getFullyQualifiedName()); + } + openNewTask(recordEntity); } - - throw new IllegalArgumentException( - String.format( - "Test Case Resolution status %s with type `Assigned` cannot be moved to `New` or `Ack`. You can `Assign` or `Resolve` the test case failure. ", - latestTestCaseFailure.getId().toString())); + case Ack -> { + /* nothing to do for ACK. The Owner already has the task open. It will close it when reassigning it */ + } + case Assigned -> assignTask(recordEntity, lastIncident); + // When the incident is Resolved, we will close the Assigned task. + case Resolved -> { + resolveTask(recordEntity, lastIncident); + // We don't create a new record. The new status will be added via the + // TestCaseFailureResolutionTaskWorkflow + // implemented in the TestCaseRepository. + return getLatestRecord(recordEntity.getTestCaseReference().getFullyQualifiedName()); + } + default -> throw new IllegalArgumentException( + String.format("Invalid status %s", recordEntity.getTestCaseResolutionStatusType())); } return super.createNewRecord(recordEntity, extension, recordFQN); } - private void createAssignedTask(TestCaseResolutionStatus entity) { + private void openNewTask(TestCaseResolutionStatus incidentStatus) { + + List owners = + EntityUtil.getEntityReferences( + daoCollection + .relationshipDAO() + .findFrom( + incidentStatus.getTestCaseReference().getId(), + Entity.TEST_CASE, + Relationship.OWNS.ordinal(), + Entity.USER)); + createTask(incidentStatus, owners, "New Incident"); + } + + private void assignTask( + TestCaseResolutionStatus newIncidentStatus, TestCaseResolutionStatus lastIncidentStatus) { + + if (lastIncidentStatus == null) { + throw new IncidentManagerException( + String.format( + "Cannot find the last incident status for stateId %s", + newIncidentStatus.getStateId())); + } + + Thread thread = getIncidentTask(lastIncidentStatus); + Assigned assigned = - JsonUtils.convertValue(entity.getTestCaseResolutionStatusDetails(), Assigned.class); - List assignees = Collections.singletonList(assigned.getAssignee()); + JsonUtils.convertValue( + newIncidentStatus.getTestCaseResolutionStatusDetails(), Assigned.class); + User updatedBy = + Entity.getEntity(Entity.USER, newIncidentStatus.getUpdatedBy().getId(), "", Include.ALL); + + patchTaskAssignee(thread, assigned.getAssignee(), updatedBy.getName()); + } + + private void resolveTask( + TestCaseResolutionStatus newIncidentStatus, TestCaseResolutionStatus lastIncidentStatus) { + + if (lastIncidentStatus == null) { + throw new IncidentManagerException( + String.format( + "Cannot find the last incident status for stateId %s", + newIncidentStatus.getStateId())); + } + + Thread thread = getIncidentTask(lastIncidentStatus); + + Resolved resolved = + JsonUtils.convertValue( + newIncidentStatus.getTestCaseResolutionStatusDetails(), Resolved.class); + TestCase testCase = + Entity.getEntity( + Entity.TEST_CASE, newIncidentStatus.getTestCaseReference().getId(), "", Include.ALL); + User updatedBy = + Entity.getEntity(Entity.USER, newIncidentStatus.getUpdatedBy().getId(), "", Include.ALL); + ResolveTask resolveTask = + new ResolveTask() + .withTestCaseFQN(testCase.getFullyQualifiedName()) + .withTestCaseFailureReason(resolved.getTestCaseFailureReason()) + .withNewValue(resolved.getTestCaseFailureComment()); + Entity.getFeedRepository() + .resolveTask( + new FeedRepository.ThreadContext(thread), + updatedBy.getFullyQualifiedName(), + resolveTask); + } + + private void createTask( + TestCaseResolutionStatus incidentStatus, List assignees, String message) { + TaskDetails taskDetails = new TaskDetails() .withAssignees(assignees) .withType(TaskType.RequestTestCaseFailureResolution) .withStatus(TaskStatus.Open) - .withTestCaseResolutionStatusId(entity.getId()); + // Each incident flow - flagged by its State ID - will have a single unique Task + .withTestCaseResolutionStatusId(incidentStatus.getStateId()); MessageParser.EntityLink entityLink = new MessageParser.EntityLink( - Entity.TEST_CASE, entity.getTestCaseReference().getFullyQualifiedName()); + Entity.TEST_CASE, incidentStatus.getTestCaseReference().getFullyQualifiedName()); Thread thread = new Thread() .withId(UUID.randomUUID()) .withThreadTs(System.currentTimeMillis()) - .withMessage("Test Case Failure Resolution requested for ") - .withCreatedBy(entity.getUpdatedBy().getName()) + .withMessage(message) + .withCreatedBy(incidentStatus.getUpdatedBy().getName()) .withAbout(entityLink.getLinkString()) .withType(ThreadType.Task) .withTask(taskDetails) - .withUpdatedBy(entity.getUpdatedBy().getName()) + .withUpdatedBy(incidentStatus.getUpdatedBy().getName()) .withUpdatedAt(System.currentTimeMillis()); FeedRepository feedRepository = Entity.getFeedRepository(); feedRepository.create(thread); } + + private void patchTaskAssignee(Thread originalTask, EntityReference newAssignee, String user) { + Thread updatedTask = JsonUtils.deepCopy(originalTask, Thread.class); + updatedTask.setTask( + updatedTask.getTask().withAssignees(Collections.singletonList(newAssignee))); + + JsonPatch patch = JsonUtils.getJsonPatch(originalTask, updatedTask); + + FeedRepository feedRepository = Entity.getFeedRepository(); + feedRepository.patchThread(null, originalTask.getId(), user, patch); + } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java index 2b916d0fe82..91a55862053 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java @@ -73,7 +73,7 @@ import org.openmetadata.service.util.ResultList; public class TestCaseResource extends EntityResource { public static final String COLLECTION_PATH = "/v1/dataQuality/testCases"; - static final String FIELDS = "owner,testSuite,testDefinition,testSuites"; + static final String FIELDS = "owner,testSuite,testDefinition,testSuites,incidents"; @Override public TestCase addHref(UriInfo uriInfo, TestCase test) { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java index a7b45e2c941..78921f47108 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java @@ -1089,54 +1089,41 @@ public class TestCaseResourceTest extends EntityResourceTest testCaseFailureStatuses = new ArrayList<>(); - List resolvedTestCaseFailureStatus = new ArrayList<>(); - for (TestCaseResolutionStatusTypes statusType : TestCaseResolutionStatusTypes.values()) { - CreateTestCaseResolutionStatus createTestCaseFailureStatus = - new CreateTestCaseResolutionStatus() - .withTestCaseReference(testCaseEntity.getFullyQualifiedName()) - .withTestCaseResolutionStatusType(statusType) - .withTestCaseResolutionStatusDetails(null); - if (statusType.equals(TestCaseResolutionStatusTypes.Assigned)) { - createTestCaseFailureStatus.setTestCaseResolutionStatusDetails( - new Assigned().withAssignee(USER1_REF)); - } - if (statusType.equals(TestCaseResolutionStatusTypes.Resolved)) { - createTestCaseFailureStatus.setTestCaseResolutionStatusDetails( - new Resolved() - .withTestCaseFailureComment("resolved") - .withTestCaseFailureReason(TestCaseFailureReasonType.MissingData) - .withResolvedBy(USER1_REF)); - resolvedTestCaseFailureStatus.add(createTestCaseFailureStatus); - continue; - } - testCaseFailureStatuses.add(createTestCaseFailureStatus); - } - // Create 2 the test case failure statuses with all stages - // this should generate 2 sequence IDs + void post_createTestCaseResultFailure(TestInfo test) + throws HttpResponseException, ParseException { + // We're going to check how each test only has a single open stateID + // and 2 tests have their own flow Long startTs = System.currentTimeMillis(); - createTestCaseResolutionStatus(testCaseFailureStatuses); - // create resolved test case failure status last - createTestCaseResolutionStatus(resolvedTestCaseFailureStatus); + TestCase testCaseEntity1 = createEntity(createRequest(getEntityName(test)), ADMIN_AUTH_HEADERS); + TestCase testCaseEntity2 = + createEntity(createRequest(getEntityName(test) + "2"), ADMIN_AUTH_HEADERS); - // Start a new sequence ID - createTestCaseResolutionStatus(testCaseFailureStatuses); - // create resolved test case failure status last - createTestCaseResolutionStatus(resolvedTestCaseFailureStatus); + // Add a failed result, which will create a NEW incident and add a new status + for (TestCase testCase : List.of(testCaseEntity1, testCaseEntity2)) { + putTestCaseResult( + testCase.getFullyQualifiedName(), + new TestCaseResult() + .withResult("result") + .withTestCaseStatus(TestCaseStatus.Failed) + .withTimestamp(TestUtils.dateToTimestamp("2024-01-01")), + ADMIN_AUTH_HEADERS); + + CreateTestCaseResolutionStatus createAckIncident = + new CreateTestCaseResolutionStatus() + .withTestCaseReference(testCase.getFullyQualifiedName()) + .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.Ack) + .withTestCaseResolutionStatusDetails(null); + createTestCaseFailureStatus(createAckIncident); + } Long endTs = System.currentTimeMillis(); // Get the test case failure statuses ResultList testCaseFailureStatusResultList = getTestCaseFailureStatus(startTs, endTs, null, null); - assertEquals(8, testCaseFailureStatusResultList.getData().size()); + assertEquals(4, testCaseFailureStatusResultList.getData().size()); - // check we have only 2 distinct sequence IDs + // check we have only 2 distinct sequence IDs, one for each test case List stateIds = testCaseFailureStatusResultList.getData().stream() .map(TestCaseResolutionStatus::getStateId) @@ -1156,58 +1143,38 @@ public class TestCaseResourceTest extends EntityResourceTest storedTestCaseResolutions = getTestCaseFailureStatusByStateId(stateId); - assertEquals(4, storedTestCaseResolutions.getData().size()); + assertEquals(2, storedTestCaseResolutions.getData().size()); assertEquals(stateId, storedTestCaseResolutions.getData().get(0).getStateId()); // Get the test case resolution statuses by status type storedTestCaseResolutions = - getTestCaseFailureStatus(startTs, endTs, null, TestCaseResolutionStatusTypes.Assigned); + getTestCaseFailureStatus(startTs, endTs, null, TestCaseResolutionStatusTypes.Ack); assertEquals(2, storedTestCaseResolutions.getData().size()); assertEquals( - TestCaseResolutionStatusTypes.Assigned, + TestCaseResolutionStatusTypes.Ack, storedTestCaseResolutions.getData().get(0).getTestCaseResolutionStatusType()); - - // Get test case resolution statuses by assignee name - storedTestCaseResolutions = getTestCaseFailureStatus(startTs, endTs, USER1.getName(), null); - assertEquals(2, storedTestCaseResolutions.getData().size()); } @Test - void test_listTestCaseFailureStatusPagination(TestInfo test) throws IOException { + void test_listTestCaseFailureStatusPagination(TestInfo test) throws IOException, ParseException { // Create a number of entities between 5 and 20 inclusive Random rand = new Random(); int maxEntities = rand.nextInt(16) + 5; - TestCase testCaseEntity = createEntity(createRequest(getEntityName(test)), ADMIN_AUTH_HEADERS); - TestCaseResolutionStatusTypes[] testCaseFailureStatusTypes = - TestCaseResolutionStatusTypes.values(); - List testCaseFailureStatuses = new ArrayList<>(); - - for (int i = 0; i < maxEntities; i++) { - // randomly pick a status type - TestCaseResolutionStatusTypes testCaseFailureStatusType = - testCaseFailureStatusTypes[i % testCaseFailureStatusTypes.length]; - - CreateTestCaseResolutionStatus createTestCaseFailureStatus = - new CreateTestCaseResolutionStatus() - .withTestCaseReference(testCaseEntity.getFullyQualifiedName()) - .withTestCaseResolutionStatusType(testCaseFailureStatusType) - .withTestCaseResolutionStatusDetails(null); - if (testCaseFailureStatusType.equals(TestCaseResolutionStatusTypes.Assigned)) { - createTestCaseFailureStatus.setTestCaseResolutionStatusDetails( - new Assigned().withAssignee(USER1_REF)); - } - if (testCaseFailureStatusType.equals(TestCaseResolutionStatusTypes.Resolved)) { - createTestCaseFailureStatus.setTestCaseResolutionStatusDetails( - new Resolved() - .withTestCaseFailureComment("resolved") - .withTestCaseFailureReason(TestCaseFailureReasonType.MissingData) - .withResolvedBy(USER1_REF)); - } - testCaseFailureStatuses.add(createTestCaseFailureStatus); - } Long startTs = System.currentTimeMillis() - 1000; - createTestCaseResolutionStatus(testCaseFailureStatuses); + for (int i = 0; i < maxEntities; i++) { + // We'll create random test cases + TestCase testCaseEntity = + createEntity(createRequest(getEntityName(test) + i), ADMIN_AUTH_HEADERS); + // Adding failed test case, which will create a NEW incident + putTestCaseResult( + testCaseEntity.getFullyQualifiedName(), + new TestCaseResult() + .withResult("result") + .withTestCaseStatus(TestCaseStatus.Failed) + .withTimestamp(TestUtils.dateToTimestamp("2024-01-01")), + ADMIN_AUTH_HEADERS); + } Long endTs = System.currentTimeMillis() + 1000; // List all entities and use it for checking pagination @@ -1217,57 +1184,6 @@ public class TestCaseResourceTest extends EntityResourceTest testCaseFailureStatuses = new ArrayList<>(); - - for (int i = 0; i < maxEntities; i++) { - // create `maxEntities` number of test cases - testCaseEntity = createEntity(createRequest(getEntityName(test) + i), ADMIN_AUTH_HEADERS); - - for (int j = 0; j < 5; j++) { - // create 5 test case failure statuses for each test case - // randomly pick a status type - TestCaseResolutionStatusTypes testCaseFailureStatusType = - testCaseFailureStatusTypes[j % TestCaseResolutionStatusTypes.values().length]; - - CreateTestCaseResolutionStatus createTestCaseFailureStatus = - new CreateTestCaseResolutionStatus() - .withTestCaseReference(testCaseEntity.getFullyQualifiedName()) - .withTestCaseResolutionStatusType(testCaseFailureStatusType) - .withTestCaseResolutionStatusDetails(null); - if (testCaseFailureStatusType.equals(TestCaseResolutionStatusTypes.Assigned)) { - createTestCaseFailureStatus.setTestCaseResolutionStatusDetails( - new Assigned().withAssignee(USER1_REF)); - } - if (testCaseFailureStatusType.equals(TestCaseResolutionStatusTypes.Resolved)) { - createTestCaseFailureStatus.setTestCaseResolutionStatusDetails( - new Resolved() - .withTestCaseFailureComment("resolved") - .withTestCaseFailureReason(TestCaseFailureReasonType.MissingData) - .withResolvedBy(USER1_REF)); - } - testCaseFailureStatuses.add(createTestCaseFailureStatus); - } - } - Long startTs = System.currentTimeMillis() - 1000; - createTestCaseResolutionStatus(testCaseFailureStatuses); - Long endTs = System.currentTimeMillis() + 1000; - - // List all entities and use it for checking pagination - ResultList allEntities = - getTestCaseFailureStatus(1000000, null, true, startTs, endTs, null); - - paginateTestCaseFailureStatus(maxEntities, allEntities, true, startTs, endTs); - } - @Test void patch_TestCaseResultFailure(TestInfo test) throws HttpResponseException { TestCase testCaseEntity = createEntity(createRequest(getEntityName(test)), ADMIN_AUTH_HEADERS); @@ -1326,24 +1242,34 @@ public class TestCaseResourceTest extends EntityResourceTest createTestCaseFailureStatus( - createTestCaseFailureStatus.withTestCaseResolutionStatusType( + createAssignedIncident.withTestCaseResolutionStatusType( TestCaseResolutionStatusTypes.Ack)), BAD_REQUEST, - "with type `Assigned` cannot be moved to `New` or `Ack`. You can `Assign` or `Resolve` the test case failure."); + "Incident with status [Assigned] cannot be moved to [Ack]"); } public void deleteTestCaseResult(String fqn, Long timestamp, Map authHeaders) @@ -1626,7 +1571,7 @@ public class TestCaseResourceTest extends EntityResourceTest testCaseResultMap = new HashMap<>(); for (TestCaseResult result : actualTestCaseResults.getData()) { - result.setTestCaseResolutionStatusReference(null); + result.setIncidentId(null); testCaseResultMap.put(result.getTimestamp(), result); } for (TestCaseResult result : expectedTestCaseResults) { diff --git a/openmetadata-spec/src/main/resources/json/schema/tests/basic.json b/openmetadata-spec/src/main/resources/json/schema/tests/basic.json index 2112e9d3e8a..77aa37d55a8 100644 --- a/openmetadata-spec/src/main/resources/json/schema/tests/basic.json +++ b/openmetadata-spec/src/main/resources/json/schema/tests/basic.json @@ -86,9 +86,9 @@ "$ref": "#/definitions/testResultValue" } }, - "testCaseResolutionStatusReference": { - "description": "Reference to the failure status object for the test case result.", - "$ref": "./testCaseResolutionStatus.json" + "incidentId": { + "description": "Reference to an ongoing Incident ID (stateId) for this result.", + "$ref": "../type/basic.json#/definitions/uuid" }, "passedRows": { "description": "Number of rows that passed.", diff --git a/openmetadata-spec/src/main/resources/json/schema/tests/testCase.json b/openmetadata-spec/src/main/resources/json/schema/tests/testCase.json index 60bd78c6247..d80f6cd9eb4 100644 --- a/openmetadata-spec/src/main/resources/json/schema/tests/testCase.json +++ b/openmetadata-spec/src/main/resources/json/schema/tests/testCase.json @@ -69,6 +69,7 @@ } }, "testCaseResult": { + "description": "Latest test case result obtained for this test case.", "$ref": "./basic.json#/definitions/testCaseResult" }, "version": { @@ -105,6 +106,13 @@ "description": "Compute the passed and failed row count for the test case.", "type": "boolean", "default": false + }, + "incidents": { + "description": "List of incident IDs (stateId) for any testCaseResult of a given test case.", + "type": "array", + "items": { + "$ref": "../type/basic.json#/definitions/uuid" + } } }, "required": ["name", "testDefinition", "entityLink", "testSuite"],