yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-09-25 17:04:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix #579: Address review comments

Browse Source
This commit is contained in:
Sriharsha Chintalapani 2021-09-25 09:48:17 -07:00
parent 2fd5ef55c8
commit c52fcd18fe
3 changed files with 2 additions and 162 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TaskRepository.java
View File

@ -257,14 +257,10 @@ public abstract class TaskRepository {
private Task setFields(Task task, Fields fields) throws IOException {
task.setOwner(fields.contains("owner") ? getOwner(task) : null);
task.setService(fields.contains("service") ? getService(task) : null);
task.setFollowers(fields.contains("followers") ? getFollowers(task) : null);
task.setTags(fields.contains("tags") ? getTags(task.getFullyQualifiedName()) : null);
return task;
}
private List<EntityReference> getFollowers(Task task) throws IOException {
return task == null ? null : EntityUtil.getFollowers(task.getId(), relationshipDAO(), userDAO());
}
private List<TagLabel> getTags(String fqn) {
return tagDAO().getTags(fqn);
@ -297,19 +293,6 @@ public abstract class TaskRepository {
}
}
@Transaction
public Status addFollower(String taskId, String userId) throws IOException {
EntityUtil.validate(taskId, taskDAO().findById(taskId), Task.class);
return EntityUtil.addFollower(relationshipDAO(), userDAO(), taskId, Entity.TASK, userId, Entity.USER) ?
Status.CREATED : Status.OK;
}
@Transaction
public void deleteFollower(String taskId, String userId) {
EntityUtil.validateUser(userDAO(), userId);
EntityUtil.removeFollower(relationshipDAO(), taskId, userId);
}
public interface TaskDAO {
@SqlUpdate("INSERT INTO task_entity (json) VALUES (:json)")
void insert(@Bind("json") String json);

42
catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/tasks/TaskResource.java
View File

@ -98,7 +98,6 @@ public class TaskResource {
task.setHref(RestUtil.getHref(uriInfo, TASK_COLLECTION_PATH, task.getId()));
EntityUtil.addHref(uriInfo, task.getOwner());
EntityUtil.addHref(uriInfo, task.getService());
EntityUtil.addHref(uriInfo, task.getFollowers());
return task;
}
@ -122,7 +121,7 @@ public class TaskResource {
}
}
static final String FIELDS = "taskConfig,owner,service,followers,tags";
static final String FIELDS = "taskConfig,owner,service,tags";
public static final List<String> FIELD_LIST = Arrays.asList(FIELDS.replaceAll(" ", "")
.split(","));
@ -284,45 +283,6 @@ public class TaskResource {
return Response.status(response.getStatus()).entity(task).build();
}
@PUT
@Path("/{id}/followers")
@Operation(summary = "Add a follower", tags = "tasks",
description = "Add a user identified by `userId` as followed of this task",
responses = {
@ApiResponse(responseCode = "200", description = "OK"),
@ApiResponse(responseCode = "404", description = "Task for instance {id} is not found")
})
public Response addFollower(@Context UriInfo uriInfo,
@Context SecurityContext securityContext,
@Parameter(description = "Id of the task", schema = @Schema(type = "string"))
@PathParam("id") String id,
@Parameter(description = "Id of the user to be added as follower",
schema = @Schema(type = "string"))
String userId) throws IOException, ParseException {
Fields fields = new Fields(FIELD_LIST, "followers");
Response.Status status = dao.addFollower(id, userId);
Task task = dao.get(id, fields);
return Response.status(status).entity(task).build();
}
@DELETE
@Path("/{id}/followers/{userId}")
@Operation(summary = "Remove a follower", tags = "tasks",
description = "Remove the user identified `userId` as a follower of the task.")
public Task deleteFollower(@Context UriInfo uriInfo,
@Context SecurityContext securityContext,
@Parameter(description = "Id of the task",
schema = @Schema(type = "string"))
@PathParam("id") String id,
@Parameter(description = "Id of the user being removed as follower",
schema = @Schema(type = "string"))
@PathParam("userId") String userId) throws IOException, ParseException {
Fields fields = new Fields(FIELD_LIST, "followers");
dao.deleteFollower(id, userId);
Task task = dao.get(id, fields);
return addHref(uriInfo, task);
}
@DELETE
@Path("/{id}")

105
catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/tasks/TaskResourceTest.java
View File

@ -62,7 +62,6 @@ import static javax.ws.rs.core.Response.Status.FORBIDDEN;
import static javax.ws.rs.core.Response.Status.NOT_FOUND;
import static javax.ws.rs.core.Response.Status.OK;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@ -75,7 +74,7 @@ import static org.openmetadata.catalog.util.TestUtils.adminAuthHeaders;
import static org.openmetadata.catalog.util.TestUtils.assertEntityPagination;
import static org.openmetadata.catalog.util.TestUtils.assertResponse;
import static org.openmetadata.catalog.util.TestUtils.authHeaders;
import static org.openmetadata.catalog.util.TestUtils.userAuthHeaders;
public class TaskResourceTest extends CatalogApplicationTest {
private static final Logger LOG = LoggerFactory.getLogger(TaskResourceTest.class);
@ -484,43 +483,6 @@ public class TaskResourceTest extends CatalogApplicationTest {
deleteTask(task.getId(), adminAuthHeaders());
}
@Test
public void put_addDeleteFollower_200(TestInfo test) throws HttpResponseException, URISyntaxException {
Task task = createAndCheckTask(create(test), adminAuthHeaders());
// Add follower to the task
User user1 = UserResourceTest.createUser(UserResourceTest.create(test, 1), userAuthHeaders());
addAndCheckFollower(task, user1.getId(), CREATED, 1, userAuthHeaders());
// Add the same user as follower and make sure no errors are thrown and return response is OK (and not CREATED)
addAndCheckFollower(task, user1.getId(), OK, 1, userAuthHeaders());
// Add a new follower to the task
User user2 = UserResourceTest.createUser(UserResourceTest.create(test, 2), userAuthHeaders());
addAndCheckFollower(task, user2.getId(), CREATED, 2, userAuthHeaders());
// Delete followers and make sure they are deleted
deleteAndCheckFollower(task, user1.getId(), 1, userAuthHeaders());
deleteAndCheckFollower(task, user2.getId(), 0, userAuthHeaders());
}
@Test
public void put_addDeleteInvalidFollower_200(TestInfo test) throws HttpResponseException, URISyntaxException {
Task task = createAndCheckTask(create(test), adminAuthHeaders());
// Add non existent user as follower to the task
HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
addAndCheckFollower(task, NON_EXISTENT_ENTITY, CREATED, 1, adminAuthHeaders()));
assertResponse(exception, NOT_FOUND, CatalogExceptionMessage.entityNotFound("User", NON_EXISTENT_ENTITY));
// Delete non existent user as follower to the task
exception = assertThrows(HttpResponseException.class, () ->
deleteAndCheckFollower(task, NON_EXISTENT_ENTITY, 1, adminAuthHeaders()));
assertResponse(exception, NOT_FOUND, CatalogExceptionMessage.entityNotFound("User", NON_EXISTENT_ENTITY));
}
@Test
public void delete_nonExistentTask_404() {
HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
@ -728,71 +690,6 @@ public class TaskResourceTest extends CatalogApplicationTest {
.withTaskUrl(new URI("http://localhost:0"));
}
public static void addAndCheckFollower(Task task, UUID userId, Status status, int totalFollowerCount,
Map<String, String> authHeaders) throws HttpResponseException {
WebTarget target = CatalogApplicationTest.getResource(String.format("tasks/%s/followers", task.getId()));
TestUtils.put(target, userId.toString(), status, authHeaders);
// GET .../tasks/{taskId} returns newly added follower
Task getTask = getTask(task.getId(), "followers", authHeaders);
assertEquals(totalFollowerCount, getTask.getFollowers().size());
TestUtils.validateEntityReference(getTask.getFollowers());
boolean followerFound = false;
for (EntityReference followers : getTask.getFollowers()) {
if (followers.getId().equals(userId)) {
followerFound = true;
break;
}
}
assertTrue(followerFound, "Follower added was not found in task get response");
// GET .../users/{userId} shows user as following table
checkUserFollowing(userId, task.getId(), true, authHeaders);
}
private static void checkUserFollowing(UUID userId, UUID taskId, boolean expectedFollowing,
Map<String, String> authHeaders) throws HttpResponseException {
// 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(taskId)) {
following = true;
break;
}
}
assertEquals(expectedFollowing, following, "Follower list for the user is invalid");
}
private void deleteAndCheckFollower(Task task, UUID userId, int totalFollowerCount,
Map<String, String> authHeaders) throws HttpResponseException {
WebTarget target = CatalogApplicationTest.getResource(String.format("tasks/%s/followers/%s",
task.getId(), userId));
TestUtils.delete(target, authHeaders);
Task getTask = checkFollowerDeleted(task.getId(), userId, authHeaders);
assertEquals(totalFollowerCount, getTask.getFollowers().size());
}
public static Task checkFollowerDeleted(UUID taskId, UUID userId, Map<String, String> authHeaders)
throws HttpResponseException {
Task getTask = getTask(taskId, "followers", authHeaders);
TestUtils.validateEntityReference(getTask.getFollowers());
boolean followerFound = false;
for (EntityReference followers : getTask.getFollowers()) {
if (followers.getId().equals(userId)) {
followerFound = true;
break;
}
}
assertFalse(followerFound, "Follower deleted is still found in table get response");
// GET .../users/{userId} shows user as following table
checkUserFollowing(userId, taskId, false, authHeaders);
return getTask;
}
private static void validateTags(List<TagLabel> expectedList, List<TagLabel> actualList)
throws HttpResponseException {
if (expectedList == null) {