From c968fc891297834b5c5ecc1f7c1cf950732fc6e1 Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Sun, 14 Aug 2022 16:10:31 -0700 Subject: [PATCH] Fix #6603 Backend: Add support for delete API for threads (#6655) * Fix #6603 Backend: Add support for delete API for threads * Fix compilation error --- .../catalog/jdbi3/CollectionDAO.java | 11 +++-- .../catalog/jdbi3/FeedRepository.java | 21 ++++++++- .../catalog/resources/feeds/FeedResource.java | 30 ++++++++++++- .../ThreadResourceContext.java | 31 +++++++++++++ .../resources/feeds/FeedResourceTest.java | 44 +++++++++++++++++++ 5 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/ThreadResourceContext.java diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java index 1af641b2758..04332f775c8 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java @@ -578,6 +578,9 @@ public interface CollectionDAO { } } + @SqlUpdate("DELETE FROM thread_entity WHERE id = :id") + void delete(@Bind("id") String id); + @ConnectionAwareSqlUpdate(value = "UPDATE task_sequence SET id=LAST_INSERT_ID(id+1)", connectionType = MYSQL) @ConnectionAwareSqlUpdate(value = "UPDATE task_sequence SET id=(id+1) RETURNING id", connectionType = POSTGRES) void updateTaskId(); @@ -1081,11 +1084,11 @@ public interface CollectionDAO { @Bind("isResolved") boolean isResolved); @SqlQuery( - "SELECT entityLink, COUNT(id) count FROM thread_entity WHERE resolved = :resolved AND " - + "(:type IS NULL OR type = :type) AND id in (SELECT toId FROM entity_relationship WHERE " - + "(((fromEntity='user' AND fromId= :userId) OR " + "SELECT entityLink, COUNT(id) count FROM thread_entity WHERE resolved = :resolved AND (:type IS NULL OR type = :type) AND " + + "(entityId in (SELECT toId FROM entity_relationship WHERE " + + "((fromEntity='user' AND fromId= :userId) OR " + "(fromEntity='team' AND fromId IN ())) AND relation=8) OR " - + "(fromEntity='user' AND fromId= :userId AND toEntity='THREAD' AND relation IN (1,2))) " + + "id in (SELECT toId FROM entity_relationship WHERE (fromEntity='user' AND fromId= :userId AND toEntity='THREAD' AND relation IN (1,2)))) " + "GROUP BY entityLink") @RegisterRowMapper(CountFieldMapper.class) List> listCountByOwner( diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java index ed7bc13546b..5c2a010f0f7 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java @@ -505,8 +505,25 @@ public class FeedRepository { return new DeleteResponse<>(post, RestUtil.ENTITY_DELETED); } - public EntityReference getOwnerOfPost(Post post) { - User fromUser = dao.userDAO().findEntityByName(post.getFrom()); + @Transaction + public DeleteResponse deleteThread(Thread thread, String deletedByUser) throws IOException { + String id = thread.getId().toString(); + + // Delete all the relationships to other entities + dao.relationshipDAO().deleteAll(id, Entity.THREAD); + + // Delete all the field relationships to other entities + dao.fieldRelationshipDAO().deleteAllByPrefix(id); + + // Finally, delete the entity + dao.feedDAO().delete(id); + + LOG.info("{} deleted thread with id {}", deletedByUser, thread.getId()); + return new DeleteResponse<>(thread, RestUtil.ENTITY_DELETED); + } + + public EntityReference getOwnerReference(String username) { + User fromUser = dao.userDAO().findEntityByName(username); return fromUser.getEntityReference(); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java index 7aad68c3e85..e3507834f73 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java @@ -67,6 +67,7 @@ import org.openmetadata.catalog.security.Authorizer; import org.openmetadata.catalog.security.policyevaluator.OperationContext; import org.openmetadata.catalog.security.policyevaluator.PostResourceContext; import org.openmetadata.catalog.security.policyevaluator.ResourceContextInterface; +import org.openmetadata.catalog.security.policyevaluator.ThreadResourceContext; import org.openmetadata.catalog.type.CreateTaskDetails; import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.MetadataOperation; @@ -510,6 +511,33 @@ public class FeedResource { return response.toResponse(); } + @DELETE + @Path("/{threadId}") + @Operation( + operationId = "deleteThread", + summary = "Delete a thread", + tags = "feeds", + description = "Delete an existing thread and all its relationships.", + responses = { + @ApiResponse(responseCode = "200", description = "OK"), + @ApiResponse(responseCode = "404", description = "thread with {threadId} is not found"), + @ApiResponse(responseCode = "400", description = "Bad request") + }) + public Response deleteThread( + @Context SecurityContext securityContext, + @Parameter(description = "ThreadId of the thread to be deleted", schema = @Schema(type = "string")) + @PathParam("threadId") + String threadId) + throws IOException { + // validate and get the thread + Thread thread = dao.get(threadId); + // delete thread only if the admin/bot/author tries to delete it + OperationContext operationContext = new OperationContext(Entity.THREAD, MetadataOperation.DELETE); + ResourceContextInterface resourceContext = new ThreadResourceContext(dao.getOwnerReference(thread.getCreatedBy())); + authorizer.authorize(securityContext, operationContext, resourceContext, true); + return dao.deleteThread(thread, securityContext.getUserPrincipal().getName()).toResponse(); + } + @DELETE @Path("/{threadId}/posts/{postId}") @Operation( @@ -537,7 +565,7 @@ public class FeedResource { // delete post only if the admin/bot/author tries to delete it // TODO fix this OperationContext operationContext = new OperationContext(Entity.THREAD, MetadataOperation.DELETE); - ResourceContextInterface resourceContext = new PostResourceContext(dao.getOwnerOfPost(post)); + ResourceContextInterface resourceContext = new PostResourceContext(dao.getOwnerReference(post.getFrom())); authorizer.authorize(securityContext, operationContext, resourceContext, true); return dao.deletePost(thread, post, securityContext.getUserPrincipal().getName()).toResponse(); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/ThreadResourceContext.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/ThreadResourceContext.java new file mode 100644 index 00000000000..ea18ff21949 --- /dev/null +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/ThreadResourceContext.java @@ -0,0 +1,31 @@ +package org.openmetadata.catalog.security.policyevaluator; + +import java.io.IOException; +import java.util.List; +import org.openmetadata.catalog.EntityInterface; +import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.type.TagLabel; + +/** Conversation threads require special handling */ +public class ThreadResourceContext implements ResourceContextInterface { + private EntityReference owner; + + public ThreadResourceContext(EntityReference owner) { + this.owner = owner; + } + + @Override + public EntityReference getOwner() throws IOException { + return owner; + } + + @Override + public List getTags() throws IOException { + return null; + } + + @Override + public EntityInterface getEntity() throws IOException { + return null; + } +} diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java index e3ac736dd3f..7704ace98ad 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java @@ -311,6 +311,7 @@ public class FeedResourceTest extends CatalogApplicationTest { } // Test the /api/v1/feed/count API + assertEquals(userThreadCount, listThreads(USER_LINK, null, userAuthHeaders).getPaging().getTotal()); assertEquals(userThreadCount, listThreadsCount(USER_LINK, userAuthHeaders).getTotalCount()); assertEquals(tableDescriptionThreadCount, getThreadCount(TABLE_DESCRIPTION_LINK, userAuthHeaders)); assertEquals(tableColumnDescriptionThreadCount, getThreadCount(TABLE_COLUMN_LINK, userAuthHeaders)); @@ -1225,6 +1226,15 @@ public class FeedResourceTest extends CatalogApplicationTest { entityNotFound("Post", NON_EXISTENT_ENTITY)); } + @Test + void delete_thread_404() { + // Test with an invalid thread id + assertResponse( + () -> deleteThread(NON_EXISTENT_ENTITY, AUTH_HEADERS), + NOT_FOUND, + entityNotFound("Thread", NON_EXISTENT_ENTITY)); + } + @Test void delete_post_200() throws HttpResponseException { // Create a thread and add a post @@ -1247,6 +1257,20 @@ public class FeedResourceTest extends CatalogApplicationTest { assertEquals(0, getThread.getPostsCount()); } + @Test + void delete_thread_200() throws HttpResponseException { + // Create a thread + Thread thread = createAndCheck(create(), AUTH_HEADERS); + assertNotNull(thread); + + // delete the thread + Thread deletedThread = deleteThread(thread.getId(), AUTH_HEADERS); + assertEquals(thread.getId(), deletedThread.getId()); + + // Check if thread is not found + assertResponse(() -> getThread(thread.getId(), AUTH_HEADERS), NOT_FOUND, entityNotFound("Thread", thread.getId())); + } + @Test void delete_post_unauthorized_403() throws HttpResponseException { // Create a thread and add a post as admin user @@ -1266,6 +1290,22 @@ public class FeedResourceTest extends CatalogApplicationTest { permissionNotAllowed(USER.getName(), List.of(MetadataOperation.DELETE))); } + @Test + void delete_thread_unauthorized_403() throws HttpResponseException { + // Create a thread as admin user + CreateThread create = create(); + Thread thread = createAndCheck(create.withFrom(ADMIN_USER_NAME), ADMIN_AUTH_HEADERS); + assertNotNull(thread); + + // delete the thread using a different user who is not an admin + // Here thread author is ADMIN, and we try to delete as USER + UUID threadId = thread.getId(); + assertResponse( + () -> deleteThread(threadId, AUTH_HEADERS), + FORBIDDEN, + permissionNotAllowed(USER.getName(), List.of(MetadataOperation.DELETE))); + } + @Test void patch_post_reactions_200() throws IOException { // Create a thread and add a post @@ -1352,6 +1392,10 @@ public class FeedResourceTest extends CatalogApplicationTest { return TestUtils.post(getResource("feed/" + threadId + "/posts"), post, Thread.class, authHeaders); } + public static Thread deleteThread(UUID threadId, Map authHeaders) throws HttpResponseException { + return TestUtils.delete(getResource("feed/" + threadId), Thread.class, authHeaders); + } + public static Post deletePost(UUID threadId, UUID postId, Map authHeaders) throws HttpResponseException { return TestUtils.delete(getResource("feed/" + threadId + "/posts/" + postId), Post.class, authHeaders);