diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java index 10bae280f9f..706ed3168bc 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/topics/TopicResource.java @@ -34,6 +34,7 @@ import org.openmetadata.catalog.jdbi3.TopicRepository; import org.openmetadata.catalog.resources.Collection; import org.openmetadata.catalog.security.CatalogAuthorizer; import org.openmetadata.catalog.security.SecurityUtil; +import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.EntityUtil.Fields; import org.openmetadata.catalog.util.RestUtil; @@ -84,6 +85,10 @@ public class TopicResource { private final TopicRepository dao; private final CatalogAuthorizer authorizer; + public static void addHref(UriInfo uriInfo, EntityReference ref) { + ref.withHref(RestUtil.getHref(uriInfo, TOPIC_COLLECTION_PATH, ref.getId())); + } + public static List addHref(UriInfo uriInfo, List topics) { Optional.ofNullable(topics).orElse(Collections.emptyList()).forEach(i -> addHref(uriInfo, i)); return topics; @@ -261,9 +266,11 @@ public class TopicResource { "{op:add, path: /b, value: val}" + "]")})) JsonPatch patch) throws IOException { - Topic topic = dao.patch(id, patch); + Fields fields = new Fields(FIELD_LIST, FIELDS); + Topic topic = dao.get(id, fields); SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, EntityUtil.getEntityReference(topic)); + topic = dao.patch(id, patch); return addHref(uriInfo, topic); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java index c9140fd8af5..e4f632dc780 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java @@ -48,6 +48,7 @@ import org.openmetadata.catalog.resources.services.database.DatabaseServiceResou import org.openmetadata.catalog.resources.services.messaging.MessagingServiceResource; import org.openmetadata.catalog.resources.teams.TeamResource; import org.openmetadata.catalog.resources.teams.UserResource; +import org.openmetadata.catalog.resources.topics.TopicResource; import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.Tag; import org.openmetadata.catalog.type.TagLabel; @@ -123,6 +124,9 @@ public final class EntityUtil { case Entity.DATABASE_SERVICE: DatabaseServiceResource.addHref(uriInfo, ref); break; + case Entity.TOPIC: + TopicResource.addHref(uriInfo, ref); + break; case Entity.MESSAGING_SERVICE: MessagingServiceResource.addHref(uriInfo, ref); break; 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 974da7ca2c0..f760cc084e4 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 @@ -812,8 +812,8 @@ public class TableResourceTest extends CatalogApplicationTest { List tableConstraints = List.of(new TableConstraint().withConstraintType(ConstraintType.UNIQUE) .withColumns(List.of(COLUMNS.get(0).getName()))); List tableTags = singletonList(USER_ADDRESS_TAG_LABEL); - table = patchTableAttributesAndCheck(table, "description", TEAM_OWNER1, TableType.Regular, tableConstraints, - tableTags, adminAuthHeaders()); + table = patchTableAttributesAndCheck(table, "description", TEAM_OWNER1, TableType.Regular, + tableConstraints, tableTags, adminAuthHeaders()); table.setOwner(TEAM_OWNER1); // Get rid of href and name returned in the response for owner // Replace description, tier, owner, tableType, tableConstraints @@ -1033,7 +1033,8 @@ public class TableResourceTest extends CatalogApplicationTest { List updatedExpectedList = new ArrayList<>(); updatedExpectedList.addAll(expectedList); for (TagLabel expected : expectedList) { - List derived = EntityUtil.getDerivedTags(expected, TagResourceTest.getTag(expected.getTagFQN(), adminAuthHeaders())); + List derived = EntityUtil.getDerivedTags(expected, TagResourceTest.getTag(expected.getTagFQN(), + adminAuthHeaders())); updatedExpectedList.addAll(derived); } updatedExpectedList = updatedExpectedList.stream().distinct().collect(Collectors.toList()); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java index fb06d852851..60e59438d5d 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java @@ -33,10 +33,12 @@ import org.openmetadata.catalog.entity.teams.Team; import org.openmetadata.catalog.entity.teams.User; import org.openmetadata.catalog.exception.CatalogExceptionMessage; import org.openmetadata.catalog.resources.services.MessagingServiceResourceTest; +import org.openmetadata.catalog.resources.tags.TagResourceTest; import org.openmetadata.catalog.resources.teams.TeamResourceTest; import org.openmetadata.catalog.resources.teams.UserResourceTest; import org.openmetadata.catalog.resources.topics.TopicResource.TopicList; import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.type.TagLabel; import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.TestUtils; @@ -47,10 +49,13 @@ import org.slf4j.LoggerFactory; import javax.json.JsonPatch; import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.Response.Status; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.stream.Collectors; +import static java.util.Collections.singletonList; 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; @@ -71,6 +76,10 @@ public class TopicResourceTest extends CatalogApplicationTest { public static EntityReference TEAM_OWNER1; public static EntityReference KAFKA_REFERENCE; public static EntityReference PULSAR_REFERENCE; + public static final TagLabel USER_ADDRESS_TAG_LABEL = new TagLabel().withTagFQN("User.Address"); + public static final TagLabel USER_BANK_ACCOUNT_TAG_LABEL = new TagLabel().withTagFQN("User.BankAccount"); + + @BeforeAll public static void setup(TestInfo test) throws HttpResponseException { @@ -388,22 +397,25 @@ public class TopicResourceTest extends CatalogApplicationTest { assertNull(topic.getDescription()); assertNull(topic.getOwner()); assertNotNull(topic.getService()); + List topicTags = singletonList(USER_ADDRESS_TAG_LABEL); - topic = getTopic(topic.getId(), "service,owner", adminAuthHeaders()); + topic = getTopic(topic.getId(), "service,owner,tags", adminAuthHeaders()); topic.getService().setHref(null); // href is readonly and not patchable // Add description, owner when previously they were null - topic = patchTopicAttributesAndCheck(topic, "description", TEAM_OWNER1, adminAuthHeaders()); + topic = patchTopicAttributesAndCheck(topic, "description", TEAM_OWNER1, topicTags, adminAuthHeaders()); topic.setOwner(TEAM_OWNER1); // Get rid of href and name returned in the response for owner topic.setService(KAFKA_REFERENCE); // Get rid of href and name returned in the response for service + topic.setTags(singletonList(USER_ADDRESS_TAG_LABEL)); // Replace description, tier, owner - topic = patchTopicAttributesAndCheck(topic, "description1", USER_OWNER1, adminAuthHeaders()); + topic = patchTopicAttributesAndCheck(topic, "description1", USER_OWNER1, topicTags, + adminAuthHeaders()); topic.setOwner(USER_OWNER1); // Get rid of href and name returned in the response for owner topic.setService(PULSAR_REFERENCE); // Get rid of href and name returned in the response for service // Remove description, tier, owner - patchTopicAttributesAndCheck(topic, null, null, adminAuthHeaders()); + patchTopicAttributesAndCheck(topic, null, null, topicTags, adminAuthHeaders()); } @Test @@ -517,7 +529,7 @@ public class TopicResourceTest extends CatalogApplicationTest { public static Topic createAndCheckTopic(CreateTopic create, Map authHeaders) throws HttpResponseException { Topic topic = createTopic(create, authHeaders); - validateTopic(topic, create.getDescription(), create.getOwner(), create.getService()); + validateTopic(topic, create.getDescription(), create.getOwner(), create.getService(), create.getTags()); return getAndValidate(topic.getId(), create, authHeaders); } @@ -525,7 +537,7 @@ public class TopicResourceTest extends CatalogApplicationTest { Status status, Map authHeaders) throws HttpResponseException { Topic updatedTopic = updateTopic(create, status, authHeaders); - validateTopic(updatedTopic, create.getDescription(), create.getOwner(), create.getService()); + validateTopic(updatedTopic, create.getDescription(), create.getOwner(), create.getService(), create.getTags()); // GET the newly updated topic and validate return getAndValidate(updatedTopic.getId(), create, authHeaders); @@ -537,12 +549,12 @@ public class TopicResourceTest extends CatalogApplicationTest { Map authHeaders) throws HttpResponseException { // GET the newly created topic by ID and validate Topic topic = getTopic(topicId, "service,owner", authHeaders); - validateTopic(topic, create.getDescription(), create.getOwner(), create.getService()); + validateTopic(topic, create.getDescription(), create.getOwner(), create.getService(), create.getTags()); // GET the newly created topic by name and validate String fqn = topic.getFullyQualifiedName(); topic = getTopicByName(fqn, "service,owner", authHeaders); - return validateTopic(topic, create.getDescription(), create.getOwner(), create.getService()); + return validateTopic(topic, create.getDescription(), create.getOwner(), create.getService(), create.getTags()); } public static Topic updateTopic(CreateTopic create, @@ -583,7 +595,8 @@ public class TopicResourceTest extends CatalogApplicationTest { } private static Topic validateTopic(Topic topic, String expectedDescription, EntityReference expectedOwner, - EntityReference expectedService) { + EntityReference expectedService, List expectedTags) + throws HttpResponseException { assertNotNull(topic.getId()); assertNotNull(topic.getHref()); assertEquals(expectedDescription, topic.getDescription()); @@ -602,25 +615,29 @@ public class TopicResourceTest extends CatalogApplicationTest { assertEquals(expectedService.getId(), topic.getService().getId()); assertEquals(expectedService.getType(), topic.getService().getType()); } + validateTags(expectedTags, topic.getTags()); return topic; } private Topic patchTopicAttributesAndCheck(Topic topic, String newDescription, - EntityReference newOwner, Map authHeaders) + EntityReference newOwner, + List tags, + Map authHeaders) throws JsonProcessingException, HttpResponseException { String topicJson = JsonUtils.pojoToJson(topic); // Update the topic attributes topic.setDescription(newDescription); topic.setOwner(newOwner); + topic.setTags(tags); // Validate information returned in patch response has the updates Topic updateTopic = patchTopic(topicJson, topic, authHeaders); - validateTopic(updateTopic, topic.getDescription(), newOwner, null); + validateTopic(updateTopic, topic.getDescription(), newOwner, null, tags); // GET the topic and Validate information returned - Topic getTopic = getTopic(topic.getId(), "service,owner", authHeaders); - validateTopic(getTopic, topic.getDescription(), newOwner, null); + Topic getTopic = getTopic(topic.getId(), "service,owner,tags", authHeaders); + validateTopic(getTopic, topic.getDescription(), newOwner, null, tags); return updateTopic; } @@ -714,7 +731,7 @@ public class TopicResourceTest extends CatalogApplicationTest { break; } } - assertTrue(followerFound, "Follower added was not found in table get response"); + assertTrue(followerFound, "Follower added was not found in topic get response"); // GET .../users/{userId} shows user as following table checkUserFollowing(userId, topic.getId(), true, authHeaders); @@ -762,4 +779,25 @@ public class TopicResourceTest extends CatalogApplicationTest { checkUserFollowing(userId, topicId, false, authHeaders); return getTopic; } + + private static void validateTags(List expectedList, List actualList) + throws HttpResponseException { + if (expectedList == null) { + return; + } + // When tags from the expected list is added to an entity, the derived tags for those tags are automatically added + // So add to the expectedList, the derived tags before validating the tags + List updatedExpectedList = new ArrayList<>(); + updatedExpectedList.addAll(expectedList); + for (TagLabel expected : expectedList) { + List derived = EntityUtil.getDerivedTags(expected, TagResourceTest.getTag(expected.getTagFQN(), + adminAuthHeaders())); + updatedExpectedList.addAll(derived); + } + updatedExpectedList = updatedExpectedList.stream().distinct().collect(Collectors.toList()); + + assertTrue(actualList.containsAll(updatedExpectedList)); + assertTrue(updatedExpectedList.containsAll(actualList)); + } + }