diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java index 5e96742b03e..59264ea96a4 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java @@ -55,7 +55,7 @@ public class ChangeEventHandler implements EventHandler { Object entity = responseContext.getEntity(); String changeType = responseContext.getHeaderString(RestUtil.CHANGE_CUSTOM_HEADER); - if (responseCode == Status.CREATED.getStatusCode()) { + if (responseCode == Status.CREATED.getStatusCode() && !RestUtil.ENTITY_FIELDS_CHANGED.equals(changeType)) { EntityInterface entityInterface = Entity.getEntityInterface(entity); EntityReference entityReference = Entity.getEntityReference(entity); changeEvent = new ChangeEvent() diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java index f8fbea1fdec..94b293254d3 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java @@ -252,6 +252,7 @@ public abstract class EntityRepository { ChangeDescription change = new ChangeDescription().withPreviousVersion(entityInterface.getVersion()); change.getFieldsAdded().add(new FieldChange().withName("followers") .withNewValue(List.of(Entity.getEntityReference(user)))); + ChangeEvent changeEvent = new ChangeEvent().withChangeDescription(change).withEventType(EventType.ENTITY_UPDATED) .withEntityType(entityName).withEntityId(entityId).withUserName(updatedBy) .withDateTime(new Date()).withCurrentVersion(entityInterface.getVersion()) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java index 018510e3cdb..d35ba90e15e 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/charts/ChartResource.java @@ -315,8 +315,8 @@ public class ChartResource { @Parameter(description = "Id of the user to be added as follower", schema = @Schema(type = "string")) String userId) throws IOException, ParseException { - return dao.addFollower(securityContext.getUserPrincipal().getName(), - UUID.fromString(id), UUID.fromString(userId)).toResponse(); + return dao.addFollower(securityContext.getUserPrincipal().getName(), UUID.fromString(id), + UUID.fromString(userId)).toResponse(); } @DELETE diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java index 0f824710aab..5a5f47171fb 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java @@ -619,7 +619,8 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { validateCreatedEntity(getEntity, create, authHeaders); // Validate that change event was created - validateChangeEvents(entityInterface, EventType.ENTITY_CREATED, null, authHeaders); + validateChangeEvents(entityInterface, entityInterface.getUpdatedAt(), EventType.ENTITY_CREATED, + null, authHeaders); return entity; } @@ -642,7 +643,8 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { if (updateType != NO_CHANGE) { EventType expectedEventType = updateType == UpdateType.CREATED ? EventType.ENTITY_CREATED : EventType.ENTITY_UPDATED; - validateChangeEvents(entityInterface, expectedEventType, changeDescription, authHeaders); + validateChangeEvents(entityInterface, entityInterface.getUpdatedAt(), expectedEventType, changeDescription, + authHeaders); } return updated; } @@ -703,7 +705,8 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { if (updateType != NO_CHANGE) { EventType expectedEventType = updateType == UpdateType.CREATED ? EventType.ENTITY_CREATED : EventType.ENTITY_UPDATED; - validateChangeEvents(entityInterface, expectedEventType, expectedChange, authHeaders); + validateChangeEvents(entityInterface, entityInterface.getUpdatedAt(), expectedEventType, expectedChange, + authHeaders); } return returned; } @@ -743,9 +746,11 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { * and ensures entityCreate, entityUpdated, and entityDeleted change events are created in the system with * valid date. */ - protected final void validateChangeEvents(EntityInterface entityInterface, EventType expectedEventType, + protected final void validateChangeEvents(EntityInterface entityInterface, + Date updateTime, EventType expectedEventType, ChangeDescription expectedChangeDescription, Map authHeaders) throws IOException { + String updatedBy = TestUtils.getPrincipal(authHeaders); ResultList changeEvents; ChangeEvent changeEvent = null; @@ -753,12 +758,11 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { while (iteration < 10) { // Some times change event is not returned on quickly querying with a millisecond // Try multiple times before giving up - changeEvents = getChangeEvents(entityName, entityName, null, - entityInterface.getUpdatedAt(), authHeaders); + changeEvents = getChangeEvents(entityName, entityName, null, updateTime, authHeaders); assertTrue(changeEvents.getData().size() > 0); for (ChangeEvent event : changeEvents.getData()) { - if (event.getDateTime().getTime() == entityInterface.getUpdatedAt().getTime()) { + if (event.getDateTime().getTime() == updateTime.getTime()) { changeEvent = event; break; } @@ -774,21 +778,21 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { iteration++; } - LOG.info("Did not find change event {} {} {}", entityInterface.getUpdatedAt().getTime(), entityInterface.getId(), + LOG.info("Did not find change event {} {} {}", updateTime.getTime(), entityInterface.getId(), expectedEventType); assertNotNull(changeEvent, "Expected change event " + expectedEventType + " at " - + entityInterface.getUpdatedAt().getTime() + " was not found for entity " + entityInterface.getId()); + + updateTime.getTime() + " was not found for entity " + entityInterface.getId()); - // Top most changeEvent corresponds to the update assertEquals(expectedEventType, changeEvent.getEventType()); assertEquals(entityName, changeEvent.getEntityType()); assertEquals(entityInterface.getId(), changeEvent.getEntityId()); assertEquals(entityInterface.getVersion(), changeEvent.getCurrentVersion()); - assertEquals(entityInterface.getUpdatedBy(), changeEvent.getUserName()); + assertEquals(updatedBy, changeEvent.getUserName()); + // // previous, entity, changeDescription - + // if (expectedEventType == EventType.ENTITY_CREATED) { assertEquals(changeEvent.getEventType(), EventType.ENTITY_CREATED); assertEquals(changeEvent.getPreviousVersion(), 0.1); @@ -798,7 +802,6 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { } else if (expectedEventType == EventType.ENTITY_UPDATED) { assertNull(changeEvent.getEntity()); assertChangeDescription(expectedChangeDescription, changeEvent.getChangeDescription()); - assertEquals(entityInterface.getChangeDescription().getPreviousVersion(), changeEvent.getPreviousVersion()); } else if (expectedEventType == EventType.ENTITY_DELETED) { assertNull(changeEvent.getEntity()); assertNull(changeEvent.getChangeDescription()); @@ -900,9 +903,9 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { } public void addAndCheckFollower(UUID entityId, UUID userId, Status status, int totalFollowerCount, - Map authHeaders) throws HttpResponseException { + Map authHeaders) throws IOException { WebTarget target = getFollowersCollection(entityId); - TestUtils.put(target, userId.toString(), status, authHeaders); + ChangeEvent event = TestUtils.put(target, userId.toString(), ChangeEvent.class, status, authHeaders); // GET .../entity/{entityId} returns newly added follower T getEntity = getEntity(entityId, authHeaders); @@ -915,17 +918,26 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { // GET .../users/{userId} shows user as following the entity checkUserFollowing(userId, entityId, true, authHeaders); + + // Validate change events + validateChangeEvents(entityInterface, event.getDateTime(), EventType.ENTITY_UPDATED, event.getChangeDescription(), + authHeaders); } protected void deleteAndCheckFollower(UUID entityId, UUID userId, int totalFollowerCount, - Map authHeaders) throws HttpResponseException { + Map authHeaders) throws IOException { // Delete the follower WebTarget target = getFollowerResource(entityId, userId); - TestUtils.delete(target, authHeaders); + ChangeEvent change = TestUtils.delete(target, ChangeEvent.class, authHeaders); // Get the entity and ensure the deleted follower is not in the followers list T getEntity = checkFollowerDeleted(entityId, userId, authHeaders); + EntityInterface entityInterface = getEntityInterface(getEntity); assertEquals(totalFollowerCount, getEntityInterface(getEntity).getFollowers().size()); + + // Validate change events + validateChangeEvents(entityInterface, change.getDateTime(), EventType.ENTITY_UPDATED, + change.getChangeDescription(), authHeaders); } public T checkFollowerDeleted(UUID entityId, UUID userId, Map authHeaders) diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java index 1277deeb301..b59d2e4157c 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java @@ -203,6 +203,11 @@ public final class TestUtils { return readResponse(response, clz, Status.OK.getStatusCode()); } + public static T delete(WebTarget target, Class clz, Map headers) throws HttpResponseException { + final Response response = addHeaders(target, headers).delete(); + return readResponse(response, clz, Status.OK.getStatusCode()); + } + public static void delete(WebTarget target, Map headers) throws HttpResponseException { final Response response = addHeaders(target, headers).delete(); if (!HttpStatus.isSuccess(response.getStatus())) { @@ -222,8 +227,7 @@ public final class TestUtils { // FullyQualifiedName has "." as separator assertTrue(ref.getName().contains("."), "entity name is not fully qualified - " + ref.getName()); } - if (List.of("location") - .contains(ref.getName())) { + if (List.of("location").contains(ref.getName())) { ref.getName().contains(":/"); // FullyQualifiedName has ":/" as separator } }