From 87ce20cf7935ac9ab5fb048d109be09967d7751b Mon Sep 17 00:00:00 2001 From: sureshms Date: Wed, 10 Nov 2021 14:41:42 -0800 Subject: [PATCH] Added tests for entityUpdated event for entities upated using PUT --- .../catalog/CatalogApplication.java | 16 ++--- .../java/org/openmetadata/catalog/Entity.java | 1 - .../catalog/events/ChangeEventHandler.java | 5 -- .../catalog/jdbi3/EntityRepository.java | 2 - .../catalog/resources/EntityResourceTest.java | 66 ++++++++----------- .../databases/TableResourceTest.java | 3 +- .../locations/LocationResourceTest.java | 19 +----- .../resources/teams/TeamResourceTest.java | 22 +------ .../resources/teams/UserResourceTest.java | 12 +--- .../openmetadata/catalog/util/TestUtils.java | 24 ++++--- 10 files changed, 58 insertions(+), 112 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/CatalogApplication.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/CatalogApplication.java index b1b4b1a8ea7..7ebaaf7231b 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/CatalogApplication.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/CatalogApplication.java @@ -82,14 +82,14 @@ public class CatalogApplication extends Application { final JdbiFactory factory = new JdbiFactory(); final Jdbi jdbi = factory.build(environment, catalogConfig.getDataSourceFactory(), "mysql3"); - SqlLogger sqlLogger = new SqlLogger() { - @Override - public void logAfterExecution(StatementContext context) { - LOG.info("sql {}, parameters {}, timeTaken {} ms", context.getRenderedSql(), - context.getBinding().toString(), context.getElapsedTime(ChronoUnit.MILLIS)); - } - }; - jdbi.setSqlLogger(sqlLogger); +// SqlLogger sqlLogger = new SqlLogger() { +// @Override +// public void logAfterExecution(StatementContext context) { +// LOG.info("sql {}, parameters {}, timeTaken {} ms", context.getRenderedSql(), +// context.getBinding().toString(), context.getElapsedTime(ChronoUnit.MILLIS)); +// } +// }; +// jdbi.setSqlLogger(sqlLogger); // Register Authorizer diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java index 8bbd8306600..ce3f439033a 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java @@ -82,7 +82,6 @@ public final class Entity { public static void registerEntity(String entity, EntityDAO dao, EntityRepository entityRepository) { - System.out.println("Registering entity " + entity); daoMap.put(entity.toLowerCase(Locale.ROOT), dao); entityRepositoryMap.put(entity, entityRepository); } 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 5d1bda88987..9906b74fa42 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 @@ -58,7 +58,6 @@ public class ChangeEventHandler implements EventHandler { if (responseCode == Status.CREATED.getStatusCode()) { EntityInterface entityInterface = Entity.getEntityInterface(entity); EntityReference entityReference = Entity.getEntityReference(entity); - System.out.println("Entity created at " + entityInterface.getUpdatedAt().getTime()); changeEvent = new ChangeEvent() .withEventType(EventType.ENTITY_CREATED) .withEntityId(entityInterface.getId()) @@ -72,7 +71,6 @@ public class ChangeEventHandler implements EventHandler { } else if (changeType.equals(RestUtil.ENTITY_UPDATED)) { EntityInterface entityInterface = Entity.getEntityInterface(entity); EntityReference entityReference = Entity.getEntityReference(entity); - System.out.println("Entity updated at " + entityInterface.getUpdatedAt().getTime()); changeEvent = new ChangeEvent() .withEventType(EventType.ENTITY_UPDATED) .withEntityId(entityInterface.getId()) @@ -91,9 +89,6 @@ public class ChangeEventHandler implements EventHandler { if (changeEvent != null) { dao.changeEventDAO().insert(JsonUtils.pojoToJson(changeEvent)); - System.out.println("Adding change event " + changeEvent); - } else { - System.out.println("Change event not recorded"); } } catch(Exception e) { LOG.error("Failed to capture change event for method {} due to {}", method, e); 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 3a267317988..32dbc6fe69e 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 @@ -484,10 +484,8 @@ public abstract class EntityRepository { JsonUtils.pojoToJson(original.getEntity())); // Store the new version - System.out.println("Storing new version"); EntityRepository.this.store(updated.getEntity(), true); } else { - System.out.println("Restoring old version"); updated.setUpdateDetails(original.getUpdatedBy(), original.getUpdatedAt()); } } 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 e731667a643..47178a91805 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 @@ -620,7 +620,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { validateCreatedEntity(getEntity, create, authHeaders); // Validate that change event was created - validateChangeEvents(entityInterface, EventType.ENTITY_CREATED, authHeaders); + validateChangeEvents(entityInterface, EventType.ENTITY_CREATED, null, authHeaders); return entity; } @@ -643,7 +643,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { if (updateType != NO_CHANGE) { EventType expectedEventType = updateType == UpdateType.CREATED ? EventType.ENTITY_CREATED : EventType.ENTITY_UPDATED; - validateChangeEvents(entityInterface, expectedEventType, authHeaders); + validateChangeEvents(entityInterface, expectedEventType, changeDescription, authHeaders); } return updated; } @@ -717,21 +717,24 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { TestUtils.validateUpdate(expectedChange.getPreviousVersion(), updatedEntityInterface.getVersion(), updateType); if (updateType != UpdateType.NO_CHANGE) { - ChangeDescription change = updatedEntityInterface.getChangeDescription(); - assertEquals(expectedChange.getPreviousVersion(), change.getPreviousVersion()); - - assertFieldLists(expectedChange.getFieldsAdded(), change.getFieldsAdded()); - assertFieldLists(expectedChange.getFieldsUpdated(), change.getFieldsUpdated()); - assertFieldLists(expectedChange.getFieldsDeleted(), change.getFieldsDeleted()); + assertChangeDescription(expectedChange, updatedEntityInterface.getChangeDescription()); } } + private void assertChangeDescription(ChangeDescription expected, ChangeDescription actual) throws IOException { + assertEquals(expected.getPreviousVersion(), actual.getPreviousVersion()); + assertFieldLists(expected.getFieldsAdded(), actual.getFieldsAdded()); + assertFieldLists(expected.getFieldsUpdated(), actual.getFieldsUpdated()); + assertFieldLists(expected.getFieldsDeleted(), actual.getFieldsDeleted()); + } + /** * This method validates the change event created after POST, PUT, and PATCH operations * and ensures entityCreate, entityUpdated, and entityDeleted change events are created in the system with * valid date. */ protected final void validateChangeEvents(EntityInterface entityInterface, EventType expectedEventType, + ChangeDescription expectedChangeDescription, Map authHeaders) throws IOException { ResultList changeEvents = getChangeEvents(entityName, entityName, null, entityInterface.getUpdatedAt(), authHeaders); @@ -741,12 +744,13 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { // Top most changeEvent corresponds to the update ChangeEvent changeEvent = changeEvents.getData().get(0); assertEquals(expectedEventType, changeEvent.getEventType()); + assertEquals(entityName, changeEvent.getEntityType()); + assertEquals(entityInterface.getId(), changeEvent.getEntityId()); + assertEquals(entityInterface.getVersion(), changeEvent.getCurrentVersion()); + assertEquals(entityInterface.getUpdatedBy(), changeEvent.getUserName()); + assertEquals(entityInterface.getUpdatedAt().getTime(), changeEvent.getDateTime().getTime()); - assertEquals(changeEvent.getDateTime().getTime(), entityInterface.getUpdatedAt().getTime()); - assertEquals(changeEvent.getEntityId(), entityInterface.getId()); - assertEquals(changeEvent.getCurrentVersion(), entityInterface.getVersion()); - assertEquals(changeEvent.getUserName(), entityInterface.getUpdatedBy()); - assertEquals(changeEvent.getEntityType(), entityName); + // previous, entity, changeDescription if (expectedEventType == EventType.ENTITY_CREATED) { assertEquals(changeEvent.getEventType(), EventType.ENTITY_CREATED); @@ -755,9 +759,12 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { compareEntities(entityInterface.getEntity(), JsonUtils.readValue((String)changeEvent.getEntity(), entityClass), authHeaders); } else if (expectedEventType == EventType.ENTITY_UPDATED) { - // TODO + 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()); } } @@ -851,15 +858,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { throw new IllegalArgumentException("Invalid owner type " + owner.getType()); } - boolean owning = false; - for (EntityReference owns : ownsList) { - TestUtils.validateEntityReference(owns); - if (owns.getId().equals(entityId)) { - owning = true; - break; - } - } - assertEquals(expectedOwning, owning, "Ownership not correct in the owns list for " + owner.getType()); + TestUtils.existsInEntityReferenceList(ownsList, entityId, expectedOwning); } } @@ -875,14 +874,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { assertEquals(totalFollowerCount, followers.size()); TestUtils.validateEntityReference(followers); - boolean followerFound = false; - for (EntityReference follower : followers) { - if (follower.getId().equals(userId)) { - followerFound = true; - break; - } - } - assertTrue(followerFound, "Follower added was not found in " + entityClass + " get response"); + TestUtils.existsInEntityReferenceList(followers, userId, true); // GET .../users/{userId} shows user as following the entity checkUserFollowing(userId, entityId, true, authHeaders); @@ -905,14 +897,7 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { T getEntity = getEntity(entityId, authHeaders); List followers = getEntityInterface(getEntity).getFollowers(); TestUtils.validateEntityReference(followers); - boolean followerFound = false; - for (EntityReference follower : followers) { - if (follower.getId().equals(userId)) { - followerFound = true; - break; - } - } - assertFalse(followerFound, "Follower deleted is still found in " + entityClass + " get response"); + TestUtils.existsInEntityReferenceList(followers, userId, false); // GET .../users/{userId} shows user as following the entity checkUserFollowing(userId, entityId, false, authHeaders); @@ -941,4 +926,5 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { list.getData().forEach(entity -> LOG.info("{} {}", entityClass, getEntityInterface(entity).getFullyQualifiedName())); LOG.info("before {} after {} ", list.getPaging().getBefore(), list.getPaging().getAfter()); } + } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java index 18680ac139c..a5c84a5a385 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java @@ -281,10 +281,9 @@ public class TableResourceTest extends EntityResourceTest { // Test PUT operation - put operation to create CreateTable create2 = create(test, 2).withColumns(Arrays.asList(c1, c2)).withName("put_complexColumnType"); - System.out.println("Put to create"); Table table2 = updateAndCheckEntity(create2, CREATED, adminAuthHeaders(), UpdateType.CREATED, null); + // Test PUT operation again without any change - System.out.println("Put with no change"); ChangeDescription change = getChangeDescription(table2.getVersion()); updateAndCheckEntity(create2, Status.OK, adminAuthHeaders(), NO_CHANGE, change); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java index 7bb1b06da4a..908fe397a5c 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java @@ -71,6 +71,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityNotFound; import static org.openmetadata.catalog.exception.CatalogExceptionMessage.readOnlyAttribute; +import static org.openmetadata.catalog.util.TestUtils.existsInEntityReferenceList; import static org.openmetadata.catalog.util.TestUtils.NON_EXISTENT_ENTITY; import static org.openmetadata.catalog.util.TestUtils.adminAuthHeaders; import static org.openmetadata.catalog.util.TestUtils.assertEntityPagination; @@ -745,14 +746,7 @@ public class LocationResourceTest extends CatalogApplicationTest { // GET .../users/{userId} shows user as following location boolean following = false; User user = UserResourceTest.getUser(userId, "follows", authHeaders); - for (EntityReference follows : user.getFollows()) { - TestUtils.validateEntityReference(follows); - if (follows.getId().equals(locationId)) { - following = true; - break; - } - } - assertEquals(expectedFollowing, following, "Follower list for the user is invalid"); + existsInEntityReferenceList(user.getFollows(), locationId, expectedFollowing); } private void deleteAndCheckFollower(Location location, UUID userId, int totalFollowerCount, @@ -769,14 +763,7 @@ public class LocationResourceTest extends CatalogApplicationTest { throws HttpResponseException { Location getLocation = getLocation(locationId, "followers", authHeaders); TestUtils.validateEntityReference(getLocation.getFollowers()); - boolean followerFound = false; - for (EntityReference followers : getLocation.getFollowers()) { - if (followers.getId().equals(userId)) { - followerFound = true; - break; - } - } - assertFalse(followerFound, "Follower deleted is still found in location get response"); + existsInEntityReferenceList(getLocation.getFollowers(), locationId, false); // GET .../users/{userId} shows user as following location checkUserFollowing(userId, locationId, false, authHeaders); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java index 26a55d589b2..3a0fb329a65 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java @@ -45,7 +45,6 @@ import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Optional; @@ -348,14 +347,7 @@ public class TeamResourceTest extends EntityResourceTest { assertEquals(expectedUsers.size(), team.getUsers().size()); for (EntityReference user : team.getUsers()) { TestUtils.validateEntityReference(user); - boolean foundUser = false; - for (EntityReference expected : expectedUsers) { - if (expected.getId().equals(user.getId())) { - foundUser = true; - break; - } - } - assertTrue(foundUser); + TestUtils.existsInEntityReferenceList(expectedUsers, user.getId(), true); } } TestUtils.validateEntityReference(team.getOwns()); @@ -431,16 +423,8 @@ public class TeamResourceTest extends EntityResourceTest { List actualUsers = Optional.ofNullable(team.getUsers()).orElse(Collections.emptyList()); if (!expectedUsers.isEmpty()) { assertEquals(expectedUsers.size(), actualUsers.size()); - for (EntityReference user : actualUsers) { - TestUtils.validateEntityReference(user); - boolean foundUser = false; - for (EntityReference expectedEntity : expectedUsers) { - if (expectedEntity.getId().equals(user.getId())) { - foundUser = true; - break; - } - } - assertTrue(foundUser); + for (EntityReference user : expectedUsers) { + TestUtils.existsInEntityReferenceList(actualUsers, user.getId(), true); } } TestUtils.validateEntityReference(team.getOwns()); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java index 91833ea9aee..1a38d046696 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java @@ -539,16 +539,8 @@ public class UserResourceTest extends EntityResourceTest { if (!expectedTeams.isEmpty()) { assertEquals(expectedTeams.size(), user.getTeams().size()); - for (EntityReference team : user.getTeams()) { - TestUtils.validateEntityReference(team); - boolean foundTeam = false; - for (EntityReference expected : expectedTeams) { - if (expected.getId().equals(team.getId())) { - foundTeam = true; - break; - } - } - assertTrue(foundTeam); + for (EntityReference team : expectedTeams) { + TestUtils.existsInEntityReferenceList(user.getTeams(), team.getId(), true); } } if (createRequest.getProfile() != null) { 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 f6ee2639421..1277deeb301 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 @@ -39,7 +39,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -274,14 +273,7 @@ public final class TestUtils { // GET .../users/{userId} shows user as following table boolean following = false; User user = UserResourceTest.getUser(userId, "follows", authHeaders); - for (EntityReference follows : user.getFollows()) { - TestUtils.validateEntityReference(follows); - if (follows.getId().equals(entityId)) { - following = true; - break; - } - } - assertEquals(expectedFollowing, following, "Follower list for the user is invalid"); + existsInEntityReferenceList(user.getFollows(), entityId, expectedFollowing); } public static String getPrincipal(Map authHeaders) { @@ -305,4 +297,18 @@ public final class TestUtils { assertEquals(Math.round((previousVersion + 1.0) * 10.0)/10.0, newVersion); // Major version change } } + + public static void existsInEntityReferenceList(List list, UUID id, boolean expectedExistsInList) { + boolean exists = false; + for (EntityReference ref : list) { + validateEntityReference(ref); + if (ref.getId().equals(id)) { + exists = true; + break; + } + } + assertEquals(expectedExistsInList, exists, + "Entry exists in list - expected:" + expectedExistsInList + " actual:" + exists); + } + }