Fixes #822 - Update Team version during PUT and POST operations

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<UUID> userIds) throws IOException {
-    // Query 1 - Validate user IDs
+    validateRelationships(team, userIds);
-    List<User> users = validateUsers(userIds);
+    storeTeam(team, false);
-
+    addRelationships(team);
-    // 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));
     return team;
   }
 
@@ -202,47 +197,50 @@ public abstract class TeamRepository {
     relationshipDAO().deleteAll(id);
   }
 
+  private void validateRelationships(Team team, List<UUID> 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<EntityReference> 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();
+    // Patch can't make changes to following fields. Ignore the changes
-    if (!updated.getId().equals(original.getId())) {
+    updated.withName(original.getName()).withId(original.getId());
-      throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "id"));
+    validateRelationships(updated, EntityUtil.getIDList(updated.getUsers()));
-    }
+    TeamRepository.TeamUpdater teamUpdater = new TeamRepository.TeamUpdater(original, updated, true);
-    if (!updated.getName().equals(original.getName())) {
+    teamUpdater.updateAll();
-      throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "name"));
+    teamUpdater.store();
-    }
-    if (updated.getDeleted() != original.getDeleted()) {
-      throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute("Team", "deleted"));
-    }
-    patchUsers(original, updated);
-    LOG.info("Updated user {}", JsonUtils.pojoToJson(updated));
-    List<EntityReference> 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);
   }
 
-  private void patchUsers(Team original, Team updated) throws IOException {
+  private List<EntityReference> validateUsers(List<UUID> userIds) throws IOException {
-    // Remove users from original and add users from updated
-    relationshipDAO().deleteFrom(original.getId().toString(), Relationship.CONTAINS.ordinal(), "user");
-    List<User> 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<User> validateUsers(List<UUID> userIds) throws IOException {
     if (userIds == null) {
       return null;
     }
-    List<User> users = new ArrayList<>();
+    List<EntityReference> users = new ArrayList<>();
     for (UUID id : userIds) {
-      User user = EntityUtil.validate(id.toString(), userDAO().findById(id.toString()), User.class);
+      users.add(EntityUtil.getEntityReference(EntityUtil.validate(id.toString(), userDAO().findById(id.toString()),
-      users.add(user);
+              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<TagLabel> 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<TagLabel> 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);
+    }
+  }
 }

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,
+  private Team patchTeamAttributesAndCheck(Team before, String displayName, String description, Profile profile,
-                                           List<User> users, Map<String, String> authHeaders)
+                                           List<User> users, Map<String, String> 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
+    Optional.ofNullable(before.getUsers()).orElse(Collections.emptyList()).forEach(t -> t.setHref(null)); // Remove href
-    String tableJson = JsonUtils.pojoToJson(team);
+    String tableJson = JsonUtils.pojoToJson(before);
 
     // Update the table attributes
-    team.setDisplayName(displayName);
+    before.setDisplayName(displayName);
-    team.setDescription(description);
+    before.setDescription(description);
-    team.setProfile(profile);
+    before.setProfile(profile);
-    team.setUsers(TeamRepository.toEntityReference(users));
+    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;
   }