Fix tests

open-metadata · Jan 4, 2024 · 80a2b15 · 80a2b15
1 parent 4354263
commit 80a2b15
Show file tree

Hide file tree

Showing 5 changed files with 174 additions and 177 deletions.
diff --git a/...ta-service/src/main/java/org/openmetadata/service/exception/IncidentManagerException.java b/...ta-service/src/main/java/org/openmetadata/service/exception/IncidentManagerException.java
@@ -1,6 +1,7 @@
 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 {
@@ -12,4 +13,11 @@ protected IncidentManagerException(Response.Status status, String 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
@@ -3615,7 +3615,8 @@ default String getTimeSeriesTableName() {
             "SELECT json FROM data_quality_data_time_series where entityFQNHash = :entityFQNHash "
                 + "AND json ->> 'incidentId' IS NOT NULL",
         connectionType = POSTGRES)
-    void cleanTestCaseIncidents(@Bind("entityFQNHash") String entityFQNHash, @Bind("stateId") String stateId);
+    void cleanTestCaseIncidents(
+        @Bind("entityFQNHash") String entityFQNHash, @Bind("stateId") String stateId);
   }
 
   interface TestCaseResolutionStatusTimeSeriesDAO extends EntityTimeSeriesDAO {

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
@@ -516,10 +516,15 @@ private List<UUID> getIncidentIds(TestCase test) {
         JsonUtils.readObjects(
             daoCollection
                 .dataQualityDataTimeSeriesDao()
-                .getResultsWithIncidents(FullyQualifiedName.buildHash(test.getFullyQualifiedName())),
+                .getResultsWithIncidents(
+                    FullyQualifiedName.buildHash(test.getFullyQualifiedName())),
             TestCaseResult.class);
 
-    return testCaseResults.stream().map(TestCaseResult::getIncidentId).collect(Collectors.toSet()).stream().toList();
+    return testCaseResults.stream()
+        .map(TestCaseResult::getIncidentId)
+        .collect(Collectors.toSet())
+        .stream()
+        .toList();
   }
 
   public int getTestCaseCount(List<UUID> testCaseIds) {

diff --git a/...vice/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java b/...vice/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java
@@ -111,6 +111,34 @@ private Thread getIncidentTask(TestCaseResolutionStatus incident) {
     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);
+        }
+      }
+    }
+  }
+
   @Override
   @Transaction
   public TestCaseResolutionStatus createNewRecord(
@@ -119,18 +147,33 @@ public TestCaseResolutionStatus createNewRecord(
     TestCaseResolutionStatus lastIncident =
         getLatestRecord(recordEntity.getTestCaseReference().getFullyQualifiedName());
 
+    if (recordEntity.getStateId() == null) {
+      recordEntity.setStateId(UUID.randomUUID());
+    }
+
     if (lastIncident != null) {
       // if we have an ongoing incident, set the stateId if the new record to be created
       recordEntity.setStateId(
           Boolean.TRUE.equals(unresolvedIncident(recordEntity))
               ? lastIncident.getStateId()
               : recordEntity.getStateId());
+
+      validateStatus(
+          lastIncident.getTestCaseResolutionStatusType(),
+          recordEntity.getTestCaseResolutionStatusType());
     }
 
     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 -> openNewTask(recordEntity, lastIncident);
+      case New -> {
+        // If there is already an unresolved incident, return it without doing any
+        // further logic.
+        if (lastIncident != null) {
+          return getLatestRecord(lastIncident.getTestCaseReference().getFullyQualifiedName());
+        }
+        openNewTask(recordEntity);
+      }
       case Ack -> {
         /* nothing to do for ACK. The Owner already has the task open. It will close it when reassigning it */
       }
@@ -149,12 +192,7 @@ public TestCaseResolutionStatus createNewRecord(
     return super.createNewRecord(recordEntity, extension, recordFQN);
   }
 
-  private void openNewTask(
-      TestCaseResolutionStatus incidentStatus, TestCaseResolutionStatus lastIncidentStatus) {
-
-    if (lastIncidentStatus != null) {
-      throw new IllegalArgumentException("An existing incident cannot be moved to NEW");
-    }
+  private void openNewTask(TestCaseResolutionStatus incidentStatus) {
 
     List<EntityReference> owners =
         EntityUtil.getEntityReferences(