kpi resource test task done (#9006)

* kpi rsource test task done

* addressing comments

* addressing comments

Co-authored-by: Himank Mehta <himankmehta@Himanks-MacBook-Air.local>

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

@@ -225,14 +225,15 @@ public class KpiRepository extends EntityRepository<Kpi> {
 
     @Override
     public void entitySpecificUpdate() throws IOException {
-      updateFromRelationships(
+      updateToRelationships(
           "dataInsightChart",
           KPI,
-          new ArrayList<>(List.of(original.getDataInsightChart())),
+          original.getId(),
-          new ArrayList<>(List.of(updated.getDataInsightChart())),
           Relationship.USES,
           DATA_INSIGHT_CHART,
-          updated.getId());
+          new ArrayList<>(List.of(original.getDataInsightChart())),
+          new ArrayList<>(List.of(updated.getDataInsightChart())),
+          false);
       recordChange("targetDefinition", original.getTargetDefinition(), updated.getTargetDefinition());
       recordChange("startDate", original.getStartDate(), updated.getStartDate());
       recordChange("endDate", original.getEndDate(), updated.getEndDate());

openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java

@@ -344,7 +344,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
     new TestDefinitionResourceTest().setupTestDefinitions(test);
     new TestCaseResourceTest().setupTestCase(test);
     new TypeResourceTest().setupTypes();
-    new KpiResourceTest().setupKpi(test);
+    new KpiResourceTest().setupKpi();
 
     runWebhookTests = new Random().nextBoolean();
     if (runWebhookTests) {
@@ -374,6 +374,13 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
     return createRequest(getEntityName(test)).withDescription("").withDisplayName(null).withOwner(null);
   }
 
+  public final K createRequest() {
+    return createRequest(String.format("test%s", UUID.randomUUID()))
+        .withDescription("")
+        .withDisplayName(null)
+        .withOwner(null);
+  }
+
   public final K createPutRequest(TestInfo test) {
     return createPutRequest(getEntityName(test)).withDescription("").withDisplayName(null).withOwner(null);
   }
@@ -520,7 +527,6 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
       createdUUIDs.add(
           createEntity(createRequest(getEntityName(test, i + 1), "", null, null), ADMIN_AUTH_HEADERS).getId());
     }
-
     T entity = createEntity(createRequest(getEntityName(test, 0), "", null, null), ADMIN_AUTH_HEADERS);
     deleteAndCheckEntity(entity, ADMIN_AUTH_HEADERS);
 
@@ -862,7 +868,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
   }
 
   @Test
-  void post_entityAlreadyExists_409_conflict(TestInfo test) throws HttpResponseException {
+  protected void post_entityAlreadyExists_409_conflict(TestInfo test) throws HttpResponseException {
     K create = createRequest(getEntityName(test), "", "", null);
     // Create first time using POST
     createEntity(create, ADMIN_AUTH_HEADERS);
@@ -1071,7 +1077,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
   }
 
   @Test
-  void put_entityEmptyDescriptionUpdate_200(TestInfo test) throws IOException {
+  protected void put_entityEmptyDescriptionUpdate_200(TestInfo test) throws IOException {
     // Create entity with empty description
     K request = createRequest(getEntityName(test), "", "displayName", null);
     T entity = createEntity(request, ADMIN_AUTH_HEADERS);
@@ -1084,11 +1090,10 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
   }
 
   @Test
-  void put_entityNonEmptyDescriptionUpdate_200(TestInfo test) throws IOException {
+  protected void put_entityNonEmptyDescriptionUpdate_200(TestInfo test) throws IOException {
     // Create entity with non-empty description
     K request = createRequest(getEntityName(test), supportsEmptyDescription ? null : "description", null, null);
     T entity = createAndCheckEntity(request, ADMIN_AUTH_HEADERS);
-
     // BOT user can update empty description and empty displayName
     ChangeDescription change = getChangeDescription(entity.getVersion());
     request = createPutRequest(getEntityName(test), "description", "displayName", null);

openmetadata-service/src/test/java/org/openmetadata/service/resources/events/WebhookResourceTest.java

@@ -26,7 +26,13 @@ import com.fasterxml.jackson.core.type.TypeReference;
 import java.io.IOException;
 import java.net.URI;
 import java.time.Duration;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.ws.rs.core.Response;
@@ -292,7 +298,7 @@ public class WebhookResourceTest extends EntityResourceTest<Webhook, CreateWebho
   }
 
   @Test
-  void put_entityNonEmptyDescriptionUpdate_200(TestInfo test) {
+  protected void put_entityNonEmptyDescriptionUpdate_200(TestInfo test) {
     // TODO fix this test as currently bot can't update webhook
   }
 

openmetadata-service/src/test/java/org/openmetadata/service/resources/kpi/KpiResourceTest.java

@@ -1,13 +1,20 @@
 package org.openmetadata.service.resources.kpi;
 
 import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static javax.ws.rs.core.Response.Status.CONFLICT;
 import static javax.ws.rs.core.Response.Status.CREATED;
 import static javax.ws.rs.core.Response.Status.NOT_FOUND;
 import static javax.ws.rs.core.Response.Status.OK;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.openmetadata.service.exception.CatalogExceptionMessage.ENTITY_ALREADY_EXISTS;
 import static org.openmetadata.service.security.SecurityUtil.getPrincipalName;
+import static org.openmetadata.service.util.EntityUtil.fieldAdded;
+import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
 import static org.openmetadata.service.util.EntityUtil.fieldUpdated;
 import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS;
+import static org.openmetadata.service.util.TestUtils.INGESTION_BOT_AUTH_HEADERS;
+import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE;
+import static org.openmetadata.service.util.TestUtils.UpdateType.NO_CHANGE;
 import static org.openmetadata.service.util.TestUtils.assertResponse;
 import static org.openmetadata.service.util.TestUtils.assertResponseContains;
 
@@ -21,7 +28,6 @@ import java.util.UUID;
 import javax.ws.rs.client.WebTarget;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.http.client.HttpResponseException;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
@@ -45,7 +51,6 @@ import org.openmetadata.service.util.TestUtils;
 
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 @Slf4j
-@Disabled
 public class KpiResourceTest extends EntityResourceTest<Kpi, CreateKpiRequest> {
   public KpiResourceTest() {
     super(Entity.KPI, Kpi.class, KpiResource.KpiList.class, "kpi", KpiResource.FIELDS);
@@ -56,12 +61,12 @@ public class KpiResourceTest extends EntityResourceTest<Kpi, CreateKpiRequest> {
     supportsPatch = false;
   }
 
-  public void setupKpi(TestInfo test) throws IOException {
+  public void setupKpi() throws IOException {
     DataInsightResourceTest dataInsightResourceTest = new DataInsightResourceTest();
     CreateDataInsightChart chartRequest =
         dataInsightResourceTest
-            .createRequest(test)
+            .createRequest()
-            .withName("TestChart")
+            .withName(String.format("TestChart" + "%s", UUID.randomUUID()))
             .withOwner(USER1_REF)
             .withDataIndexType(DataReportIndex.ENTITY_REPORT_DATA_INDEX)
             .withMetrics(
@@ -84,15 +89,15 @@ public class KpiResourceTest extends EntityResourceTest<Kpi, CreateKpiRequest> {
   }
 
   @Test
-  void post_testWithInvalidValues_4xx(TestInfo test) {
+  void post_testWithInvalidValues_4xx(TestInfo test) throws HttpResponseException {
-    CreateKpiRequest create1 = createRequest("Test2" + UUID.randomUUID());
+    String uuid = "Test2" + UUID.randomUUID();
+    CreateKpiRequest create1 = createRequest(uuid);
     create1.withDataInsightChart(USER1_REF);
 
     assertResponseContains(
         () -> createAndCheckEntity(create1, ADMIN_AUTH_HEADERS),
         NOT_FOUND,
         "dataInsightChart instance for " + USER1_REF.getId() + " not found");
-
     CreateKpiRequest create2 = createRequest(String.format("Test%s", UUID.randomUUID()));
     KpiTarget target = new KpiTarget().withName("Test").withValue("Test");
     create2.withTargetDefinition(List.of(target));
@@ -227,6 +232,11 @@ public class KpiResourceTest extends EntityResourceTest<Kpi, CreateKpiRequest> {
 
   @Override
   public CreateKpiRequest createRequest(String name) {
+    try {
+      this.setupKpi();
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
     return new CreateKpiRequest()
         .withName(name)
         .withDescription(name)
@@ -250,6 +260,70 @@ public class KpiResourceTest extends EntityResourceTest<Kpi, CreateKpiRequest> {
     assertEquals(request.getTargetDefinition(), createdEntity.getTargetDefinition());
   }
 
+  @Override
+  protected void post_entityAlreadyExists_409_conflict(TestInfo test) throws HttpResponseException {
+    CreateKpiRequest create = createRequest(getEntityName(test), "", "", null);
+    // Create first time using POST
+    createEntity(create, ADMIN_AUTH_HEADERS);
+    CreateKpiRequest create1 = createRequest(getEntityName(test), "", "", null);
+    // Second time creating the same entity using POST should fail
+    assertResponse(() -> createEntity(create1, ADMIN_AUTH_HEADERS), CONFLICT, ENTITY_ALREADY_EXISTS);
+  }
+
+  @Override
+  @Test
+  protected void put_entityNonEmptyDescriptionUpdate_200(TestInfo test) throws IOException {
+    // Create entity with non-empty description
+    CreateKpiRequest request =
+        createRequest(getEntityName(test), supportsEmptyDescription ? null : "description", null, null);
+    Kpi entity = createAndCheckEntity(request, ADMIN_AUTH_HEADERS);
+    UUID oldUUID = request.getDataInsightChart().getId();
+    // BOT user can update empty description and empty displayName
+    ChangeDescription change = getChangeDescription(entity.getVersion());
+    request = createPutRequest(getEntityName(test), "description", "displayName", null);
+    UUID newUUID = request.getDataInsightChart().getId();
+    if (supportsEmptyDescription) {
+      fieldAdded(change, "description", "description");
+    }
+    fieldAdded(change, "displayName", "displayName");
+    fieldAdded(change, "dataInsightChart", newUUID);
+    fieldDeleted(change, "dataInsightChart", oldUUID);
+    entity = updateAndCheckEntity(request, OK, INGESTION_BOT_AUTH_HEADERS, MINOR_UPDATE, change);
+
+    // Updating non-empty description and non-empty displayName is allowed for users other than bots
+    oldUUID = request.getDataInsightChart().getId();
+    request = createPutRequest(getEntityName(test), "updatedDescription", "updatedDisplayName", null);
+    newUUID = request.getDataInsightChart().getId();
+    change = getChangeDescription(entity.getVersion());
+    fieldUpdated(change, "description", "description", "updatedDescription");
+    fieldUpdated(change, "displayName", "displayName", "updatedDisplayName");
+    fieldAdded(change, "dataInsightChart", newUUID);
+    fieldDeleted(change, "dataInsightChart", oldUUID);
+    updateAndCheckEntity(request, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
+
+    // Updating non-empty description and non-empty displayName is ignored for bot users
+    request = createPutRequest(getEntityName(test), "updatedDescription2", "updatedDisplayName2", null);
+    updateAndCheckEntity(request, OK, INGESTION_BOT_AUTH_HEADERS, NO_CHANGE, null);
+  }
+
+  @Override
+  @Test
+  protected void put_entityEmptyDescriptionUpdate_200(TestInfo test) throws IOException {
+    // Create entity with empty description
+    CreateKpiRequest request = createRequest(getEntityName(test), "", "displayName", null);
+    Kpi entity = createEntity(request, ADMIN_AUTH_HEADERS);
+    UUID oldUUID = request.getDataInsightChart().getId();
+
+    // Update empty description with a new description
+    request = createPutRequest(getEntityName(test), "updatedDescription", "displayName", null);
+    UUID newUUID = request.getDataInsightChart().getId();
+    ChangeDescription change = getChangeDescription(entity.getVersion());
+    fieldUpdated(change, "description", "", "updatedDescription");
+    fieldAdded(change, "dataInsightChart", newUUID);
+    fieldDeleted(change, "dataInsightChart", oldUUID);
+    updateAndCheckEntity(request, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
+  }
+
   @Override
   public void compareEntities(Kpi expected, Kpi updated, Map<String, String> authHeaders) throws HttpResponseException {
     validateCommonEntityFields(expected, updated, getPrincipalName(authHeaders));