MINOR - Move test case incident to the top of the data_quality_data_time_series (#14600)

* Add field and index

* MINOR - Move test case incident to the top of the data_quality_data_time_series table

* Fix test

* Fix compile

* Format

* Update incidentId

* Rename field

* Fix patch

bootstrap/sql/migrations/native/1.3.0/mysql/schemaChanges.sql

@@ -132,3 +132,7 @@ WHERE de.serviceType = 'Databricks'
 UPDATE dbservice_entity de 
 SET de.json = JSON_REMOVE(de.json, '$.connection.config.useUnityCatalog')
 WHERE de.serviceType IN ('Databricks','UnityCatalog');
+
+-- Add Incident ID for test case results
+ALTER TABLE data_quality_data_time_series ADD COLUMN incidentId varchar(36);
+ALTER TABLE data_quality_data_time_series ADD INDEX data_quality_data_time_series_incidentId(incidentId);

bootstrap/sql/migrations/native/1.3.0/postgres/schemaChanges.sql

@@ -144,3 +144,7 @@ WHERE de.serviceType = 'Databricks'
 UPDATE dbservice_entity de 
 SET json = json #- '{connection,config,useUnityCatalog}'
 WHERE de.serviceType IN ('Databricks','UnityCatalog');
+
+-- Add Incident ID for test case results
+ALTER TABLE data_quality_data_time_series ADD COLUMN incidentId varchar(36);
+CREATE INDEX IF NOT EXISTS data_quality_data_time_series_incidentId ON data_quality_data_time_series(incidentId);

ingestion/src/metadata/utils/logger.py

@@ -254,9 +254,11 @@ def _(record: TableAndTests) -> str:
 
 
 @get_log_name.register
-def _(_: TestCaseResults) -> Optional[str]:
+def _(record: TestCaseResults) -> str:
     """We don't want to log this in the status"""
-    return None
+    return ",".join(
+        set(result.testCase.name.__root__ for result in record.test_results)
+    )
 
 
 @get_log_name.register

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java

@@ -3593,31 +3593,46 @@ public interface CollectionDAO {
       return "data_quality_data_time_series";
     }
 
-    @ConnectionAwareSqlQuery(
+    @SqlQuery(
         value =
-            "SELECT json FROM data_quality_data_time_series where entityFQNHash = :testCaseFQNHash "
+            "SELECT DISTINCT incidentId FROM data_quality_data_time_series "
-                + "AND JSON_EXTRACT(json, '$.incidentId') IS NOT NULL",
+                + "WHERE entityFQNHash = :testCaseFQNHash AND incidentId IS NOT NULL")
-        connectionType = MYSQL)
+    List<String> getResultsWithIncidents(@BindFQN("testCaseFQNHash") String testCaseFQNHash);
-    @ConnectionAwareSqlQuery(
+
+    @SqlUpdate(
         value =
-            "SELECT json FROM data_quality_data_time_series where entityFQNHash = :testCaseFQNHash "
+            "UPDATE data_quality_data_time_series SET incidentId = NULL "
-                + "AND json ->> 'incidentId' IS NOT NULL",
+                + "WHERE entityFQNHash = :testCaseFQNHash and incidentId = :incidentStateId")
-        connectionType = POSTGRES)
+    void cleanTestCaseIncident(
-    List<String> getResultsWithIncidents(@Bind("testCaseFQNHash") String testCaseFQNHash);
+        @BindFQN("testCaseFQNHash") String testCaseFQNHash,
+        @Bind("incidentStateId") String incidentStateId);
 
     @ConnectionAwareSqlUpdate(
         value =
-            "SELECT json FROM data_quality_data_time_series where entityFQNHash = :entityFQNHash "
+            "INSERT INTO data_quality_data_time_series(entityFQNHash, extension, jsonSchema, json, incidentId) "
-                + "AND JSON_EXTRACT(json, '$.incidentId') IS NOT NULL",
+                + "VALUES (:testCaseFQNHash, :extension, :jsonSchema, :json, :incidentStateId)",
         connectionType = MYSQL)
     @ConnectionAwareSqlUpdate(
         value =
-            "SELECT json FROM data_quality_data_time_series where entityFQNHash = :entityFQNHash "
+            "INSERT INTO data_quality_data_time_series(entityFQNHash, extension, jsonSchema, json, incidentId) "
-                + "AND json ->> 'incidentId' IS NOT NULL",
+                + "VALUES (:testCaseFQNHash, :extension, :jsonSchema, (:json :: jsonb), :incidentStateId)",
         connectionType = POSTGRES)
-    // TODO: need to find the right way to get this cleaned
+    void insert(
-    void cleanTestCaseIncidents(
+        @Define("table") String table,
-        @Bind("entityFQNHash") String entityFQNHash, @Bind("stateId") String stateId);
+        @BindFQN("testCaseFQNHash") String testCaseFQNHash,
+        @Bind("extension") String extension,
+        @Bind("jsonSchema") String jsonSchema,
+        @Bind("json") String json,
+        @Bind("incidentStateId") String incidentStateId);
+
+    default void insert(
+        String entityFQNHash,
+        String extension,
+        String jsonSchema,
+        String json,
+        String incidentStateId) {
+      insert(getTimeSeriesTableName(), entityFQNHash, extension, jsonSchema, json, incidentStateId);
+    }
   }
 
   interface TestCaseResolutionStatusTimeSeriesDAO extends EntityTimeSeriesDAO {

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java

@@ -1,6 +1,7 @@
 package org.openmetadata.service.jdbi3;
 
 import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
+import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
 import static org.openmetadata.service.Entity.TEST_CASE;
 import static org.openmetadata.service.Entity.TEST_DEFINITION;
 import static org.openmetadata.service.Entity.TEST_SUITE;
@@ -15,7 +16,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.UUID;
-import java.util.stream.Collectors;
 import javax.json.JsonPatch;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
@@ -58,7 +58,7 @@ import org.openmetadata.service.util.ResultList;
 public class TestCaseRepository extends EntityRepository<TestCase> {
   private static final String TEST_SUITE_FIELD = "testSuite";
   private static final String TEST_CASE_RESULT_FIELD = "testCaseResult";
-  private static final String INCIDENTS_FIELD = "incidents";
+  private static final String INCIDENTS_FIELD = "incidentId";
   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";
@@ -85,8 +85,8 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
         fields.contains(TEST_CASE_RESULT_FIELD)
             ? getTestCaseResult(test)
             : test.getTestCaseResult());
-    test.setIncidents(
+    test.setIncidentId(
-        fields.contains(INCIDENTS_FIELD) ? getIncidentIds(test) : test.getIncidents());
+        fields.contains(INCIDENTS_FIELD) ? getIncidentId(test) : test.getIncidentId());
   }
 
   @Override
@@ -243,11 +243,13 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
     // Validate the request content
     TestCase testCase = findByName(fqn, Include.NON_DELETED);
 
-    // set the test case resolution status reference if test failed
+    // set the test case resolution status reference if test failed, by either
-    testCaseResult.setIncidentId(
+    // creating a new incident or returning the stateId of an unresolved incident
-        testCaseResult.getTestCaseStatus() == TestCaseStatus.Failed
+    // for this test case
-            ? createIncidentOnFailure(testCase, updatedBy)
+    UUID incidentStateId = null;
-            : null);
+    if (TestCaseStatus.Failed.equals(testCaseResult.getTestCaseStatus())) {
+      incidentStateId = getOrCreateIncidentOnFailure(testCase, updatedBy);
+    }
 
     daoCollection
         .dataQualityDataTimeSeriesDao()
@@ -255,7 +257,8 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
             testCase.getFullyQualifiedName(),
             TESTCASE_RESULT_EXTENSION,
             TEST_CASE_RESULT_FIELD,
-            JsonUtils.pojoToJson(testCaseResult));
+            JsonUtils.pojoToJson(testCaseResult),
+            incidentStateId != null ? incidentStateId.toString() : null);
 
     setFieldsInternal(testCase, new EntityUtil.Fields(allowedFields, TEST_SUITE_FIELD));
     setTestSuiteSummary(
@@ -270,7 +273,7 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
         Response.Status.CREATED, changeEvent, RestUtil.ENTITY_FIELDS_CHANGED);
   }
 
-  private UUID createIncidentOnFailure(TestCase testCase, String updatedBy) {
+  private UUID getOrCreateIncidentOnFailure(TestCase testCase, String updatedBy) {
 
     TestCaseResolutionStatusRepository testCaseResolutionStatusRepository =
         (TestCaseResolutionStatusRepository)
@@ -510,21 +513,22 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
   /**
    * Check all the test case results that have an ongoing incident and get the stateId of the incident
    */
-  private List<UUID> getIncidentIds(TestCase test) {
+  private UUID getIncidentId(TestCase test) {
-    List<TestCaseResult> testCaseResults;
+    UUID ongoingIncident = null;
-    testCaseResults =
-        JsonUtils.readObjects(
-            daoCollection
-                .dataQualityDataTimeSeriesDao()
-                .getResultsWithIncidents(
-                    FullyQualifiedName.buildHash(test.getFullyQualifiedName())),
-            TestCaseResult.class);
 
-    return testCaseResults.stream()
+    List<UUID> incidents =
-        .map(TestCaseResult::getIncidentId)
+        daoCollection
-        .collect(Collectors.toSet())
+            .dataQualityDataTimeSeriesDao()
-        .stream()
+            .getResultsWithIncidents(test.getFullyQualifiedName())
-        .toList();
+            .stream()
+            .map(UUID::fromString)
+            .toList();
+
+    if (!nullOrEmpty(incidents)) {
+      ongoingIncident = incidents.get(0);
+    }
+
+    return ongoingIncident;
   }
 
   public int getTestCaseCount(List<UUID> testCaseIds) {
@@ -715,12 +719,15 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
 
   public static class TestCaseFailureResolutionTaskWorkflow extends FeedRepository.TaskWorkflow {
     final TestCaseResolutionStatusRepository testCaseResolutionStatusRepository;
+    final CollectionDAO.DataQualityDataTimeSeriesDAO dataQualityDataTimeSeriesDao;
 
     TestCaseFailureResolutionTaskWorkflow(FeedRepository.ThreadContext threadContext) {
       super(threadContext);
       this.testCaseResolutionStatusRepository =
           (TestCaseResolutionStatusRepository)
               Entity.getEntityTimeSeriesRepository(Entity.TEST_CASE_RESOLUTION_STATUS);
+
+      this.dataQualityDataTimeSeriesDao = Entity.getCollectionDAO().dataQualityDataTimeSeriesDao();
     }
 
     /**
@@ -763,9 +770,20 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
               JsonUtils.pojoToJson(testCaseResolutionStatus));
       testCaseResolutionStatusRepository.postCreate(testCaseResolutionStatus);
 
-      // TODO: remove incident ID from test case result
+      // When we resolve a task, we clean up the test case results associated
+      // with the resolved stateId
+      dataQualityDataTimeSeriesDao.cleanTestCaseIncident(
+          latestTestCaseResolutionStatus.getTestCaseReference().getFullyQualifiedName(),
+          latestTestCaseResolutionStatus.getStateId().toString());
 
-      return Entity.getEntity(testCaseResolutionStatus.getTestCaseReference(), "", Include.ALL);
+      // Return the TestCase with the StateId to avoid any unnecessary PATCH when resolving the task
+      // in the feed repo,
+      // since the `threadContext.getAboutEntity()` will give us the task with the `incidentId`
+      // informed, which
+      // we'll remove here.
+      TestCase testCaseEntity =
+          Entity.getEntity(testCaseResolutionStatus.getTestCaseReference(), "", Include.ALL);
+      return testCaseEntity.withIncidentId(latestTestCaseResolutionStatus.getStateId());
     }
 
     /**

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<TestCase, TestCaseRepository> {
   public static final String COLLECTION_PATH = "/v1/dataQuality/testCases";
 
-  static final String FIELDS = "owner,testSuite,testDefinition,testSuites,incidents";
+  static final String FIELDS = "owner,testSuite,testDefinition,testSuites,incidentId";
 
   @Override
   public TestCase addHref(UriInfo uriInfo, TestCase test) {

openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java

@@ -1089,6 +1089,44 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
     paginate(maxEntities, allEntities, logicalTestSuite);
   }
 
+  @Test
+  void get_testCaseResultWithIncidentId(TestInfo test)
+      throws HttpResponseException, ParseException {
+
+    // We create a test case with a failure
+    TestCase testCaseEntity = createEntity(createRequest(getEntityName(test)), ADMIN_AUTH_HEADERS);
+    putTestCaseResult(
+        testCaseEntity.getFullyQualifiedName(),
+        new TestCaseResult()
+            .withResult("result")
+            .withTestCaseStatus(TestCaseStatus.Failed)
+            .withTimestamp(TestUtils.dateToTimestamp("2024-01-01")),
+        ADMIN_AUTH_HEADERS);
+
+    // We can get it via API with a list of ongoing incidents
+    TestCase result = getTestCase(testCaseEntity.getFullyQualifiedName(), ADMIN_AUTH_HEADERS);
+
+    assertNotNull(result.getIncidentId());
+
+    // Resolving the status triggers resolving the task, which triggers removing the ongoing
+    // incident from the test case
+    CreateTestCaseResolutionStatus createResolvedStatus =
+        new CreateTestCaseResolutionStatus()
+            .withTestCaseReference(testCaseEntity.getFullyQualifiedName())
+            .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.Resolved)
+            .withTestCaseResolutionStatusDetails(
+                new Resolved()
+                    .withTestCaseFailureComment("resolved")
+                    .withTestCaseFailureReason(TestCaseFailureReasonType.MissingData)
+                    .withResolvedBy(USER1_REF));
+    createTestCaseFailureStatus(createResolvedStatus);
+
+    // If we read again, the incident list will be empty
+    result = getTestCase(testCaseEntity.getFullyQualifiedName(), ADMIN_AUTH_HEADERS);
+
+    assertNull(result.getIncidentId());
+  }
+
   @Test
   void post_createTestCaseResultFailure(TestInfo test)
       throws HttpResponseException, ParseException {
@@ -1491,6 +1529,13 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
     return TestUtils.get(target, TestCaseResource.TestCaseResultList.class, authHeaders);
   }
 
+  public TestCase getTestCase(String fqn, Map<String, String> authHeaders)
+      throws HttpResponseException {
+    WebTarget target = getCollection().path("/name/" + fqn);
+    target = target.queryParam("fields", "incidentId");
+    return TestUtils.get(target, TestCase.class, authHeaders);
+  }
+
   private TestSummary getTestSummary(Map<String, String> authHeaders, String testSuiteId)
       throws IOException {
     TestSuiteResourceTest testSuiteResourceTest = new TestSuiteResourceTest();
@@ -1571,7 +1616,6 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
     assertEquals(expectedTestCaseResults.size(), actualTestCaseResults.getData().size());
     Map<Long, TestCaseResult> testCaseResultMap = new HashMap<>();
     for (TestCaseResult result : actualTestCaseResults.getData()) {
-      result.setIncidentId(null);
       testCaseResultMap.put(result.getTimestamp(), result);
     }
     for (TestCaseResult result : expectedTestCaseResults) {

openmetadata-spec/src/main/resources/json/schema/tests/basic.json

@@ -86,10 +86,6 @@
             "$ref": "#/definitions/testResultValue"
           }
         },
-        "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.",
           "type": "integer"

openmetadata-spec/src/main/resources/json/schema/tests/testCase.json

@@ -107,12 +107,9 @@
       "type": "boolean",
       "default": false
     },
-    "incidents": {
+    "incidentId": {
-      "description": "List of incident IDs (stateId) for any testCaseResult of a given test case.",
+      "description": "Reference to an ongoing Incident ID (stateId) for this test case.",
-      "type": "array",
+      "$ref": "../type/basic.json#/definitions/uuid"
-      "items": {
-        "$ref": "../type/basic.json#/definitions/uuid"
-      }
     }
   },
   "required": ["name", "testDefinition", "entityLink", "testSuite"],