diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java index da28c42f87d..8c10214ecd0 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java @@ -16,24 +16,28 @@ package org.openmetadata.catalog.jdbi3; +import com.fasterxml.jackson.core.JsonProcessingException; import org.openmetadata.catalog.entity.teams.Team; import org.openmetadata.catalog.entity.teams.User; import org.openmetadata.catalog.exception.CatalogExceptionMessage; import org.openmetadata.catalog.exception.EntityNotFoundException; +import org.openmetadata.catalog.jdbi3.ChartRepository.ChartDAO; import org.openmetadata.catalog.jdbi3.DashboardRepository.DashboardDAO; import org.openmetadata.catalog.jdbi3.DatabaseRepository.DatabaseDAO; import org.openmetadata.catalog.jdbi3.MetricsRepository.MetricsDAO; -import org.openmetadata.catalog.jdbi3.ReportRepository.ReportDAO; -import org.openmetadata.catalog.jdbi3.TableRepository.TableDAO; -import org.openmetadata.catalog.jdbi3.UserRepository.UserDAO; -import org.openmetadata.catalog.jdbi3.TopicRepository.TopicDAO; -import org.openmetadata.catalog.jdbi3.ChartRepository.ChartDAO; -import org.openmetadata.catalog.jdbi3.TaskRepository.TaskDAO; import org.openmetadata.catalog.jdbi3.ModelRepository.ModelDAO; import org.openmetadata.catalog.jdbi3.PipelineRepository.PipelineDAO; +import org.openmetadata.catalog.jdbi3.ReportRepository.ReportDAO; +import org.openmetadata.catalog.jdbi3.TableRepository.TableDAO; +import org.openmetadata.catalog.jdbi3.TaskRepository.TaskDAO; +import org.openmetadata.catalog.jdbi3.TopicRepository.TopicDAO; +import org.openmetadata.catalog.jdbi3.UserRepository.UserDAO; import org.openmetadata.catalog.resources.teams.TeamResource; import org.openmetadata.catalog.resources.teams.TeamResource.TeamList; import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.type.TagLabel; +import org.openmetadata.catalog.util.EntityInterface; +import org.openmetadata.catalog.util.EntityUpdater; import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.EntityUtil.Fields; import org.openmetadata.catalog.util.JsonUtils; @@ -114,18 +118,9 @@ public abstract class TeamRepository { @Transaction public Team create(Team team, List userIds) throws IOException { - // Query 1 - Validate user IDs - List users = validateUsers(userIds); - - // Query 2 - add team into team_entity - Note that no team href or relationship attributes are stored in json - teamDAO().insert(JsonUtils.pojoToJson(team)); - - // Query 3 - Add relationship Team -- contains --> User - for (User user : Optional.ofNullable(users).orElse(Collections.emptyList())) { - addUserRelationship(team, user); - } - - team.setUsers(toEntityReference(users)); + validateRelationships(team, userIds); + storeTeam(team, false); + addRelationships(team); return team; } @@ -202,47 +197,50 @@ public abstract class TeamRepository { relationshipDAO().deleteAll(id); } + private void validateRelationships(Team team, List userIds) throws IOException { + team.setUsers(validateUsers(userIds)); + } + + private void addRelationships(Team team) { + for (EntityReference user : Optional.ofNullable(team.getUsers()).orElse(Collections.emptyList())) { + relationshipDAO().insert(team.getId().toString(), user.getId().toString(), "team", "user", + Relationship.CONTAINS.ordinal()); + } + } + + private void storeTeam(Team team, boolean update) throws JsonProcessingException { + // Relationships and fields such as href are derived and not stored as part of json + List users = team.getUsers(); + + // Don't store users, href as JSON. Build it on the fly based on relationships + team.withUsers(null).withHref(null); + + if (update) { + teamDAO().update(team.getId().toString(), JsonUtils.pojoToJson(team)); + } else { + teamDAO().insert(JsonUtils.pojoToJson(team)); + } + + // Restore the relationships + team.withUsers(users); + } private void patch(Team original, Team updated) throws IOException { - String teamId = original.getId().toString(); - if (!updated.getId().equals(original.getId())) { - throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "id")); - } - if (!updated.getName().equals(original.getName())) { - throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "name")); - } - if (updated.getDeleted() != original.getDeleted()) { - throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "deleted")); - } - patchUsers(original, updated); - LOG.info("Updated user {}", JsonUtils.pojoToJson(updated)); - List newUsers = updated.getUsers(); - updated.setUsers(null); - LOG.info("Updated user {}", JsonUtils.pojoToJson(updated)); - teamDAO().update(teamId, JsonUtils.pojoToJson(updated)); // Update the stored JSON - updated.setUsers(newUsers); + // Patch can't make changes to following fields. Ignore the changes + updated.withName(original.getName()).withId(original.getId()); + validateRelationships(updated, EntityUtil.getIDList(updated.getUsers())); + TeamRepository.TeamUpdater teamUpdater = new TeamRepository.TeamUpdater(original, updated, true); + teamUpdater.updateAll(); + teamUpdater.store(); } - private void patchUsers(Team original, Team updated) throws IOException { - // Remove users from original and add users from updated - relationshipDAO().deleteFrom(original.getId().toString(), Relationship.CONTAINS.ordinal(), "user"); - List validatedUsers = new ArrayList<>(); - for (EntityReference user : Optional.ofNullable(updated.getUsers()).orElse(Collections.emptyList())) { - String userId = user.getId().toString(); - validatedUsers.add(EntityUtil.validate(userId, userDAO().findById(userId), User.class)); - relationshipDAO().insert(updated.getId().toString(), user.getId().toString(), - "team", "user", Relationship.CONTAINS.ordinal()); - } - updated.setUsers(toEntityReference(validatedUsers)); - } - - private List validateUsers(List userIds) throws IOException { + private List validateUsers(List userIds) throws IOException { if (userIds == null) { return null; } - List users = new ArrayList<>(); + List users = new ArrayList<>(); for (UUID id : userIds) { - User user = EntityUtil.validate(id.toString(), userDAO().findById(id.toString()), User.class); - users.add(user); + users.add(EntityUtil.getEntityReference(EntityUtil.validate(id.toString(), userDAO().findById(id.toString()), + User.class))); } return users; } @@ -271,11 +269,6 @@ public abstract class TeamRepository { metricsDAO(), dashboardDAO(), reportDAO(), topicDAO(), chartDAO(), taskDAO(), modelDAO(), pipelineDAO()); } - private void addUserRelationship(Team team, User user) { - relationshipDAO().insert(team.getId().toString(), user.getId().toString(), "team", "user", - Relationship.CONTAINS.ordinal()); - } - public interface TeamDAO { @SqlUpdate("INSERT INTO team_entity (json) VALUES (:json)") void insert(@Bind("json") String json); @@ -310,4 +303,88 @@ public abstract class TeamRepository { @SqlUpdate("UPDATE team_entity SET json = :json WHERE id = :id") void update(@Bind("id") String id, @Bind("json") String json); } + + static class TeamEntityInterface implements EntityInterface { + private final Team team; + + TeamEntityInterface(Team Team) { + this.team = Team; + } + + @Override + public UUID getId() { + return team.getId(); + } + + @Override + public String getDescription() { + return team.getDescription(); + } + + @Override + public String getDisplayName() { + return team.getDisplayName(); + } + + @Override + public EntityReference getOwner() { return null; } + + @Override + public String getFullyQualifiedName() { return null; } + + @Override + public List getTags() { return null; } + + @Override + public void setDescription(String description) { team.setDescription(description); } + + @Override + public void setDisplayName(String displayName) { + team.setDisplayName(displayName); + } + + @Override + public void setTags(List tags) { } + } + + /** + * Handles entity updated from PUT and POST operation. + */ + public class TeamUpdater extends EntityUpdater { + final Team orig; + final Team updated; + + public TeamUpdater(Team orig, Team updated, boolean patchOperation) { + super(new TeamRepository.TeamEntityInterface(orig), new TeamRepository.TeamEntityInterface(updated), patchOperation, relationshipDAO(), + null); + this.orig = orig; + this.updated = updated; + } + + public void updateAll() throws IOException { + // Update operation can't undelete a user + if (updated.getDeleted() != orig.getDeleted()) { + throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "deleted")); + } + super.updateAll(); + updateUsers(); + } + + public void updateUsers() throws IOException { + // TODO cleanup + // Remove users from original and add users from updated + relationshipDAO().deleteFrom(orig.getId().toString(), Relationship.CONTAINS.ordinal(), "user"); + + for (EntityReference user : Optional.ofNullable(updated.getUsers()).orElse(Collections.emptyList())) { + relationshipDAO().insert(updated.getId().toString(), user.getId().toString(), + "team", "user", Relationship.CONTAINS.ordinal()); + } + update("users", orig.getUsers(), updated.getUsers()); + } + + public void store() throws IOException { + updated.setVersion(getNewVersion(orig.getVersion())); + storeTeam(updated, true); + } + } } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java index 7fec7dd1130..56c6fff9849 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java @@ -33,6 +33,7 @@ import org.openmetadata.catalog.type.ImageList; import org.openmetadata.catalog.type.Profile; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.TestUtils; +import org.openmetadata.catalog.util.TestUtils.UpdateType; import org.openmetadata.common.utils.JsonSchemaUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +60,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityNotFound; import static org.openmetadata.catalog.resources.teams.UserResourceTest.createUser; +import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE; +import static org.openmetadata.catalog.util.TestUtils.UpdateType.NO_CHANGE; import static org.openmetadata.catalog.util.TestUtils.adminAuthHeaders; import static org.openmetadata.catalog.util.TestUtils.assertEntityPagination; import static org.openmetadata.catalog.util.TestUtils.assertResponse; @@ -322,29 +325,6 @@ public class TeamResourceTest extends CatalogApplicationTest { assertResponse(exception, NOT_FOUND, entityNotFound("Team", TestUtils.NON_EXISTENT_ENTITY)); } - @Test - public void patch_teamIDChange_400(TestInfo test) throws HttpResponseException, JsonProcessingException { - // Ensure team ID can't be changed using patch - Team team = createTeam(create(test), adminAuthHeaders()); - UUID oldTeamId = team.getId(); - String teamJson = JsonUtils.pojoToJson(team); - team.setId(UUID.randomUUID()); - HttpResponseException exception = assertThrows(HttpResponseException.class, () -> - patchTeam(oldTeamId, teamJson, team, adminAuthHeaders())); - assertResponse(exception, BAD_REQUEST, CatalogExceptionMessage.readOnlyAttribute("Team", "id")); - } - - @Test - public void patch_teamNameChange_400(TestInfo test) throws HttpResponseException, JsonProcessingException { - // Ensure team name can't be changed using patch - Team team = createTeam(create(test), adminAuthHeaders()); - String teamJson = JsonUtils.pojoToJson(team); - team.setName("newName"); - HttpResponseException exception = assertThrows(HttpResponseException.class, () -> - patchTeam(teamJson, team, adminAuthHeaders())); - assertResponse(exception, BAD_REQUEST, CatalogExceptionMessage.readOnlyAttribute("Team", "name")); - } - @Test public void patch_teamDeletedDisallowed_400(TestInfo test) throws HttpResponseException, JsonProcessingException { // Ensure team deleted attribute can't be changed using patch @@ -376,17 +356,17 @@ public class TeamResourceTest extends CatalogApplicationTest { // Add previously absent attributes team = patchTeamAttributesAndCheck(team, "displayName", "description", profile, users, - adminAuthHeaders()); + adminAuthHeaders(), MINOR_UPDATE); // Replace the attributes users = Arrays.asList(user1, user3); // user2 dropped and user3 is added profile = new Profile().withImages(new ImageList().withImage(URI.create("http://image1.com"))); team = patchTeamAttributesAndCheck(team, "displayName1", "description1", profile, users, - adminAuthHeaders()); + adminAuthHeaders(), MINOR_UPDATE); // Remove the attributes patchTeamAttributesAndCheck(team, null, null, null, null, - adminAuthHeaders()); + adminAuthHeaders(), MINOR_UPDATE); } @Test @@ -408,7 +388,7 @@ public class TeamResourceTest extends CatalogApplicationTest { HttpResponseException exception = assertThrows(HttpResponseException.class, () -> patchTeamAttributesAndCheck(team, "displayName", "description", profile, users, - authHeaders("test@open-metadata.org"))); + authHeaders("test@open-metadata.org"), NO_CHANGE)); assertResponse(exception, FORBIDDEN, "Principal: CatalogPrincipal{name='test'} is not admin"); } // @Test @@ -534,25 +514,26 @@ public class TeamResourceTest extends CatalogApplicationTest { return patchTeam(updated.getId(), originalJson, updated, authHeaders); } - private Team patchTeamAttributesAndCheck(Team team, String displayName, String description, Profile profile, - List users, Map authHeaders) + private Team patchTeamAttributesAndCheck(Team before, String displayName, String description, Profile profile, + List users, Map authHeaders, UpdateType updateType) throws JsonProcessingException, HttpResponseException { String updatedBy = TestUtils.getPrincipal(authHeaders); - Optional.ofNullable(team.getUsers()).orElse(Collections.emptyList()).forEach(t -> t.setHref(null)); // Remove href - String tableJson = JsonUtils.pojoToJson(team); + Optional.ofNullable(before.getUsers()).orElse(Collections.emptyList()).forEach(t -> t.setHref(null)); // Remove href + String tableJson = JsonUtils.pojoToJson(before); // Update the table attributes - team.setDisplayName(displayName); - team.setDescription(description); - team.setProfile(profile); - team.setUsers(TeamRepository.toEntityReference(users)); + before.setDisplayName(displayName); + before.setDescription(description); + before.setProfile(profile); + before.setUsers(TeamRepository.toEntityReference(users)); // Validate information returned in patch response has the updates - Team updatedTeam = patchTeam(tableJson, team, authHeaders); + Team updatedTeam = patchTeam(tableJson, before, authHeaders); validateTeam(updatedTeam, description, displayName, profile, users, updatedBy); + TestUtils.validateUpdate(before.getVersion(), updatedTeam.getVersion(), updateType); // GET the table and Validate information returned - Team getTeam = getTeam(team.getId(), "users,profile", authHeaders); + Team getTeam = getTeam(before.getId(), "users,profile", authHeaders); validateTeam(getTeam, description, displayName, profile, users, updatedBy); return getTeam; }