Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3124: Activity Feed: Add support for deleting a post #3161

Merged
merged 4 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -38,6 +39,8 @@
import org.openmetadata.catalog.api.feed.ThreadCount;
import org.openmetadata.catalog.entity.feed.Thread;
import org.openmetadata.catalog.entity.teams.User;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
import org.openmetadata.catalog.exception.EntityNotFoundException;
import org.openmetadata.catalog.resources.feeds.FeedResource;
import org.openmetadata.catalog.resources.feeds.FeedUtil;
import org.openmetadata.catalog.resources.feeds.MessageParser;
Expand All @@ -49,6 +52,7 @@
import org.openmetadata.catalog.util.EntityUtil;
import org.openmetadata.catalog.util.JsonUtils;
import org.openmetadata.catalog.util.RestUtil;
import org.openmetadata.catalog.util.RestUtil.DeleteResponse;
import org.openmetadata.catalog.util.RestUtil.PatchResponse;

@Slf4j
Expand Down Expand Up @@ -144,12 +148,13 @@ private void storeMentions(Thread thread, String message) {
}

@Transaction
public Thread addPostToThread(String id, Post post) throws IOException {
public Thread addPostToThread(String id, Post post, String userName) throws IOException {
// Query 1 - validate the user posting the message
User fromUser = dao.userDAO().findEntityByName(post.getFrom());

// Query 2 - Find the thread
Thread thread = EntityUtil.validate(id, dao.feedDAO().findById(id), Thread.class);
thread.withUpdatedBy(userName).withUpdatedAt(System.currentTimeMillis());
FeedUtil.addPost(thread, post);

// TODO is rewriting entire json okay?
Expand Down Expand Up @@ -181,6 +186,31 @@ public Thread addPostToThread(String id, Post post) throws IOException {
return thread;
}

public Post getPostById(Thread thread, String postId) {
Optional<Post> post = thread.getPosts().stream().filter(p -> p.getId().equals(UUID.fromString(postId))).findAny();
if (post.isEmpty()) {
throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entityNotFound("Post", postId));
}
return post.get();
}

@Transaction
public DeleteResponse<Post> deletePost(Thread thread, Post post, String userName) throws IOException {
List<Post> posts = thread.getPosts();
// Remove the post to be deleted from the posts list
posts = posts.stream().filter(p -> !p.getId().equals(post.getId())).collect(Collectors.toList());
thread.withUpdatedAt(System.currentTimeMillis()).withUpdatedBy(userName).withPosts(posts);
// update the json document
dao.feedDAO().update(thread.getId().toString(), JsonUtils.pojoToJson(thread));

return new DeleteResponse<>(post, RestUtil.ENTITY_DELETED);
}

public EntityReference getOwnerOfPost(Post post) throws IOException {
User fromUser = dao.userDAO().findEntityByName(post.getFrom());
return Entity.getEntityReference(fromUser);
}

@Transaction
public ThreadCount getThreadsCount(String link, boolean isResolved) throws IOException {
ThreadCount threadCount = new ThreadCount();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import javax.validation.constraints.Max;
import javax.validation.constraints.Min;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.PATCH;
Expand All @@ -50,6 +51,7 @@
import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;
import javax.ws.rs.core.UriInfo;
import org.openmetadata.catalog.api.feed.CreatePost;
import org.openmetadata.catalog.api.feed.CreateThread;
import org.openmetadata.catalog.api.feed.ThreadCount;
import org.openmetadata.catalog.entity.feed.Thread;
Expand All @@ -58,6 +60,7 @@
import org.openmetadata.catalog.jdbi3.FeedRepository.FilterType;
import org.openmetadata.catalog.resources.Collection;
import org.openmetadata.catalog.security.Authorizer;
import org.openmetadata.catalog.security.SecurityUtil;
import org.openmetadata.catalog.type.Post;
import org.openmetadata.catalog.util.RestUtil;
import org.openmetadata.catalog.util.RestUtil.PatchResponse;
Expand All @@ -73,6 +76,7 @@ public class FeedResource {
public static final List<String> ALLOWED_FIELDS = getAllowedFields();

private final FeedRepository dao;
private final Authorizer authorizer;

private static List<String> getAllowedFields() {
JsonPropertyOrder propertyOrder = Thread.class.getAnnotation(JsonPropertyOrder.class);
Expand All @@ -92,6 +96,7 @@ public static Thread addHref(UriInfo uriInfo, Thread thread) {
public FeedResource(CollectionDAO dao, Authorizer authorizer) {
Objects.requireNonNull(dao, "FeedRepository must not be null");
this.dao = new FeedRepository(dao);
this.authorizer = authorizer;
}

static class ThreadList extends ResultList<Thread> {
Expand Down Expand Up @@ -259,14 +264,48 @@ public Response create(@Context UriInfo uriInfo, @Context SecurityContext securi
@ApiResponse(
responseCode = "200",
description = "The post",
content = @Content(mediaType = "application/json", schema = @Schema(implementation = Post.class))),
content = @Content(mediaType = "application/json", schema = @Schema(implementation = CreatePost.class))),
@ApiResponse(responseCode = "400", description = "Bad request")
})
public Response addPost(@Context UriInfo uriInfo, @PathParam("id") String id, @Valid Post post) throws IOException {
Thread thread = addHref(uriInfo, dao.addPostToThread(id, post));
public Response addPost(
@Context SecurityContext securityContext,
@Context UriInfo uriInfo,
@PathParam("id") String id,
@Valid CreatePost createPost)
throws IOException {
Post post = getPost(createPost);
Thread thread = addHref(uriInfo, dao.addPostToThread(id, post, securityContext.getUserPrincipal().getName()));
return Response.created(thread.getHref()).entity(thread).build();
}

@DELETE
@Path("/{threadId}/posts/{postId}")
@Operation(
summary = "Delete a post from its thread",
tags = "feeds",
description = "Delete a post from an existing thread.",
responses = {
@ApiResponse(responseCode = "200", description = "OK"),
@ApiResponse(responseCode = "404", description = "post with {postId} is not found"),
@ApiResponse(responseCode = "400", description = "Bad request")
})
public Response deletePost(
@Context SecurityContext securityContext,
@Parameter(description = "ThreadId of the post to be deleted", schema = @Schema(type = "string"))
@PathParam("threadId")
String threadId,
@Parameter(description = "PostId of the post to be deleted", schema = @Schema(type = "string"))
@PathParam("postId")
String postId)
throws IOException {
// validate and get thread & post
Thread thread = dao.get(threadId);
Post post = dao.getPostById(thread, postId);
// delete post only if the admin/bot/author tries to delete it
SecurityUtil.checkAdminOrBotOrOwner(authorizer, securityContext, dao.getOwnerOfPost(post));
return dao.deletePost(thread, post, securityContext.getUserPrincipal().getName()).toResponse();
}

@GET
@Path("/{id}/posts")
@Operation(
Expand Down Expand Up @@ -294,4 +333,12 @@ private Thread getThread(SecurityContext securityContext, CreateThread create) {
.withUpdatedBy(securityContext.getUserPrincipal().getName())
.withUpdatedAt(System.currentTimeMillis());
}

private Post getPost(CreatePost create) {
return new Post()
.withId(UUID.randomUUID())
.withMessage(create.getMessage())
.withFrom(create.getFrom())
.withPostTs(System.currentTimeMillis());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ private FeedUtil() {}

public static void addPost(Thread thread, Post post) {
// Add new post to the thread
post.setPostTs(System.currentTimeMillis());
thread.getPosts().add(post);
thread.withPostsCount(thread.getPosts().size());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"$id": "https://github.com/open-metadata/OpenMetadata/blob/main/catalog-rest-service/src/main/resources/json/schema/api/feed/createPost.json",
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "CreatePostRequest",
"description": "Create post request",
"type": "object",
"properties": {
"message": {
"description": "Message in markdown format. See markdown support for more details.",
"type": "string"
},
"from": {
"description": "Name of the User posting the message",
"type": "string"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If all users are in OpenMetadata internal db, should we use user id?

Copy link
Contributor

@sureshms sureshms Mar 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually not. Let's keep them as user names which are supposed to be unique and human readable. One reason why to use UUID is to identify an instance of an entity uniquely (entity with same name can be created and hence name might not be unique) and when permanent links are needed.

}
},
"required": ["message", "from"],
"additionalProperties": false
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
"type": "object",
"description": "Post within a feed.",
"properties": {
"id": {
"description": "Unique identifier that identifies the post.",
"$ref": "../../type/basic.json#/definitions/uuid"
},
"message": {
"description": "Message in markdown format. See markdown support for more details.",
"type": "string"
Expand All @@ -24,7 +28,7 @@
"type": "string"
}
},
"required": ["message", "from"],
"required": ["id", "message", "from"],
"additionalProperties": false
}
},
Expand Down Expand Up @@ -86,6 +90,6 @@
}
}
},
"required": ["id", "about", "posts"],
"required": ["id", "about", "message"],
"additionalProperties": false
}