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 5b1a290a251..51528594448 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 @@ -106,6 +106,11 @@ public final class Entity { public static final String AIRFLOW_PIPELINE = "airflowPipeline"; public static final String WEBHOOK = "webhook"; + // + // List of entities whose changes should not be published to the Activity Feed + // + public static final List ACTIVITY_FEED_EXCLUDED_ENTITIES = List.of(USER, TEAM, ROLE, POLICY, BOTS); + private Entity() {} public static void registerEntity( @@ -185,6 +190,16 @@ public final class Entity { return !entityType.equals(TEAM); } + /** + * Returns true if the change events of the given entity type should be published to the activity feed. + * + * @param entityType Type of the entity. + * @return true if change events of the entity should be published to activity feed, false otherwise + */ + public static boolean shouldDisplayEntityChangeOnFeed(@NonNull String entityType) { + return !ACTIVITY_FEED_EXCLUDED_ENTITIES.contains(entityType); + } + public static EntityInterface getEntityInterface(T entity) { if (entity == null) { return null; 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 8bbc86e1cc5..333a21411e2 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 @@ -72,10 +72,15 @@ public class ChangeEventHandler implements EventHandler { // Add a new thread to the entity for every change event // for the event to appear in activity feeds - List threads = getThreads(responseContext, changeEvent); - if (threads != null) { - for (var thread : threads) { - feedDao.create(thread); + if (Entity.shouldDisplayEntityChangeOnFeed(changeEvent.getEntityType())) { + List threads = getThreads(responseContext); + if (threads != null) { + for (var thread : threads) { + // Don't create a thread if there is no message + if (!thread.getMessage().isEmpty()) { + feedDao.create(thread); + } + } } } } @@ -171,7 +176,7 @@ public class ChangeEventHandler implements EventHandler { .withCurrentVersion(changeEvent.getCurrentVersion()); } - private List getThreads(ContainerResponseContext responseContext, ChangeEvent changeEvent) { + private List getThreads(ContainerResponseContext responseContext) { Object entity = responseContext.getEntity(); if (entity == null) { return null; // Response has no entity to produce change event from @@ -184,14 +189,14 @@ public class ChangeEventHandler implements EventHandler { return null; } - return getThreads(entity, entityInterface.getChangeDescription(), changeEvent); + return getThreads(entity, entityInterface.getChangeDescription()); } - private List getThreads(Object entity, ChangeDescription changeDescription, ChangeEvent changeEvent) { + private List getThreads(Object entity, ChangeDescription changeDescription) { List threads = new ArrayList<>(); var entityInterface = Entity.getEntityInterface(entity); - Map messages = ChangeEventParser.getFormattedMessages(changeDescription, entity, changeEvent); + Map messages = ChangeEventParser.getFormattedMessages(changeDescription, entity); // Create an automated thread for (var link : messages.keySet()) { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java index baaef11b869..e7430568b95 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java @@ -32,7 +32,6 @@ import org.apache.commons.lang.StringUtils; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.resources.feeds.MessageParser.EntityLink; import org.openmetadata.catalog.type.ChangeDescription; -import org.openmetadata.catalog.type.ChangeEvent; import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.FieldChange; @@ -46,8 +45,7 @@ public final class ChangeEventParser { DELETE } - public static Map getFormattedMessages( - ChangeDescription changeDescription, Object entity, ChangeEvent changeEvent) { + public static Map getFormattedMessages(ChangeDescription changeDescription, Object entity) { // Store a map of entityLink -> message Map messages; @@ -82,7 +80,7 @@ public final class ChangeEventParser { } private static String getFieldValue(Object fieldValue) { - if (fieldValue != null) { + if (fieldValue != null && !fieldValue.toString().isEmpty()) { try { // Check if field value is a json string JsonValue json = JsonUtils.readJson(fieldValue.toString()); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/ChangeEventParserTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/ChangeEventParserTest.java index 3ee264de557..834d0d2367e 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/ChangeEventParserTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/ChangeEventParserTest.java @@ -33,7 +33,6 @@ import org.openmetadata.catalog.CatalogApplicationTest; import org.openmetadata.catalog.resources.databases.TableResourceTest; import org.openmetadata.catalog.resources.feeds.MessageParser.EntityLink; import org.openmetadata.catalog.type.ChangeDescription; -import org.openmetadata.catalog.type.ChangeEvent; import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.FieldChange; import org.openmetadata.catalog.type.TagLabel; @@ -57,16 +56,14 @@ public class ChangeEventParserTest extends CatalogApplicationTest { @Test void testFormattedMessages() throws JsonProcessingException { ChangeDescription changeDescription = new ChangeDescription(); - ChangeEvent changeEvent = new ChangeEvent(); // Simulate updating tags of an entity from tag1 -> tag2 FieldChange addTag = new FieldChange(); addTag.withName("tags").withNewValue("tag2"); FieldChange deleteTag = new FieldChange(); deleteTag.withName("tags").withOldValue("tag1"); changeDescription.withFieldsAdded(List.of(addTag)).withFieldsDeleted(List.of(deleteTag)).withPreviousVersion(1.0); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(1.0).withCurrentVersion(1.1); - Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, messages.size()); TagLabel tag1 = new TagLabel(); @@ -78,8 +75,7 @@ public class ChangeEventParserTest extends CatalogApplicationTest { addTag.withNewValue(JsonUtils.pojoToJson(List.of(tag2))); deleteTag.withOldValue(JsonUtils.pojoToJson(List.of(tag1))); - Map jsonMessages = - ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + Map jsonMessages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, jsonMessages.size()); // The entity links and values of both the messages should be the same @@ -89,7 +85,6 @@ public class ChangeEventParserTest extends CatalogApplicationTest { @Test void testEntityReferenceFormat() throws JsonProcessingException { ChangeDescription changeDescription = new ChangeDescription(); - ChangeEvent changeEvent = new ChangeEvent(); // Simulate adding owner to a table EntityReference entityReference = new EntityReference(); entityReference.withId(UUID.randomUUID()).withName("user1").withDisplayName("User One"); @@ -97,9 +92,8 @@ public class ChangeEventParserTest extends CatalogApplicationTest { addOwner.withName("owner").withNewValue(JsonUtils.pojoToJson(entityReference)); changeDescription.withFieldsAdded(List.of(addOwner)).withPreviousVersion(1.0); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(1.0).withCurrentVersion(1.1); - Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, messages.size()); assertEquals("Added **owner**: `User One`", messages.values().iterator().next()); @@ -108,15 +102,13 @@ public class ChangeEventParserTest extends CatalogApplicationTest { @Test void testUpdateOfString() { ChangeDescription changeDescription = new ChangeDescription(); - ChangeEvent changeEvent = new ChangeEvent(); // Simulate a change of description in table FieldChange updateDescription = new FieldChange(); updateDescription.withName("description").withNewValue("new description").withOldValue("old description"); changeDescription.withFieldsUpdated(List.of(updateDescription)).withPreviousVersion(1.0); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(1.1); - Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, messages.size()); assertEquals( @@ -135,11 +127,8 @@ public class ChangeEventParserTest extends CatalogApplicationTest { .withFieldsDeleted(List.of(deleteDescription)) .withPreviousVersion(1.0); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(1.1); - // now test if both the type of updates give the same message - Map updatedMessages = - ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + Map updatedMessages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, updatedMessages.size()); assertEquals(messages.keySet().iterator().next(), updatedMessages.keySet().iterator().next()); @@ -149,7 +138,6 @@ public class ChangeEventParserTest extends CatalogApplicationTest { @Test void testMajorSchemaChange() { ChangeDescription changeDescription = new ChangeDescription(); - ChangeEvent changeEvent = new ChangeEvent(); // Simulate a change of column name in table FieldChange addColumn = new FieldChange(); addColumn @@ -167,9 +155,8 @@ public class ChangeEventParserTest extends CatalogApplicationTest { .withFieldsAdded(List.of(addColumn)) .withFieldsDeleted(List.of(deleteColumn)) .withPreviousVersion(1.3); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(2.3); - Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + Map messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, messages.size()); assertEquals( @@ -186,9 +173,8 @@ public class ChangeEventParserTest extends CatalogApplicationTest { .withFieldsAdded(List.of(addColumn)) .withFieldsDeleted(List.of(deleteColumn)) .withPreviousVersion(1.3); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(2.3); - messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, messages.size()); assertEquals( @@ -205,9 +191,8 @@ public class ChangeEventParserTest extends CatalogApplicationTest { .withFieldsAdded(List.of(addColumn)) .withFieldsDeleted(List.of(deleteColumn)) .withPreviousVersion(1.4); - changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(2.4); - messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent); + messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE); assertEquals(1, messages.size()); assertEquals(