From b2ab634816dce76926da525a482383ab5cd6f4dc Mon Sep 17 00:00:00 2001
From: Simon Brandhof <simon.brandhof@sonarsource.com>
Date: Thu, 13 Oct 2016 00:54:42 +0200
Subject: [PATCH] SONAR-8134 remove RoleDao#deleteGroupRole()

replaced by GroupPermissionDao#delete()
---
 .../permission/GroupPermissionChanger.java    | 11 ++-
 .../db/permission/GroupPermissionDao.java     | 18 +++++
 .../db/permission/GroupPermissionMapper.java  |  3 +
 .../db/permission/PermissionRepository.java   |  9 ---
 .../main/java/org/sonar/db/user/RoleDao.java  |  9 ---
 .../java/org/sonar/db/user/RoleMapper.java    |  3 -
 .../db/permission/GroupPermissionMapper.xml   | 25 +++++++
 .../org/sonar/db/user/RoleMapper.xml          | 23 -------
 .../db/permission/GroupPermissionDaoTest.java | 68 +++++++++++++++++++
 .../permission/PermissionRepositoryTest.java  | 12 ----
 .../java/org/sonar/db/user/RoleDaoTest.java   | 25 -------
 .../java/org/sonar/db/user/UserDbTester.java  |  8 +++
 .../should_delete_group_permission-result.xml | 30 --------
 .../should_delete_group_permission.xml        | 35 ----------
 14 files changed, 127 insertions(+), 152 deletions(-)
 delete mode 100644 sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission-result.xml
 delete mode 100644 sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission.xml

diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java b/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java
index 32d34717fa5..f2c382a5463 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/GroupPermissionChanger.java
@@ -58,12 +58,11 @@ public class GroupPermissionChanger {
         break;
       case REMOVE:
         checkAdminUsersExistOutsideTheRemovedGroup(dbSession, change);
-        GroupPermissionDto deletedDto = new GroupPermissionDto()
-          .setRole(change.getPermission())
-          .setOrganizationUuid(change.getOrganizationUuid())
-          .setGroupId(change.getGroupIdOrAnyone().getId())
-          .setResourceId(change.getNullableProjectId());
-        dbClient.roleDao().deleteGroupRole(deletedDto, dbSession);
+        dbClient.groupPermissionDao().delete(dbSession,
+          change.getPermission(),
+          change.getOrganizationUuid(),
+          change.getGroupIdOrAnyone().getId(),
+          change.getNullableProjectId());
         break;
       default:
         throw new UnsupportedOperationException("Unsupported permission change: " + change.getOperation());
diff --git a/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionDao.java b/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionDao.java
index 37e375b4b46..c1685e39776 100644
--- a/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionDao.java
+++ b/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionDao.java
@@ -120,6 +120,24 @@ public class GroupPermissionDao implements Dao {
     mapper(dbSession).deleteByRootComponentId(rootComponentId);
   }
 
+  /**
+   * Delete a single permission. It can be:
+   * <ul>
+   *   <li>a global permission granted to a group</li>
+   *   <li>a global permission granted to anyone</li>
+   *   <li>a permission granted to a group for a project</li>
+   *   <li>a permission granted to anyone for a project</li>
+   * </ul>
+   * @param dbSession
+   * @param permission the kind of permission
+   * @param organizationUuid UUID of organization, even if parameter {@code groupId} is not null
+   * @param groupId if null, then anyone, else id of group
+   * @param rootComponentId if null, then global permission, else id of root component (project)
+   */
+  public void delete(DbSession dbSession, String permission, String organizationUuid, @Nullable Long groupId, @Nullable Long rootComponentId) {
+    mapper(dbSession).delete(permission, organizationUuid, groupId, rootComponentId);
+  }
+
   private static GroupPermissionMapper mapper(DbSession session) {
     return session.getMapper(GroupPermissionMapper.class);
   }
diff --git a/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionMapper.java b/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionMapper.java
index ca005bcc617..5a8138776c9 100644
--- a/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionMapper.java
+++ b/sonar-db/src/main/java/org/sonar/db/permission/GroupPermissionMapper.java
@@ -61,4 +61,7 @@ public interface GroupPermissionMapper {
   void insert(GroupPermissionDto dto);
 
   void deleteByRootComponentId(@Param("rootComponentId") long componentId);
+
+  void delete(@Param("permission") String permission, @Param("organizationUuid") String organizationUuid,
+    @Nullable @Param("groupId") Long groupId, @Nullable @Param("rootComponentId") Long rootComponentId);
 }
diff --git a/sonar-db/src/main/java/org/sonar/db/permission/PermissionRepository.java b/sonar-db/src/main/java/org/sonar/db/permission/PermissionRepository.java
index 53b33564296..d909d0ff0fe 100644
--- a/sonar-db/src/main/java/org/sonar/db/permission/PermissionRepository.java
+++ b/sonar-db/src/main/java/org/sonar/db/permission/PermissionRepository.java
@@ -56,15 +56,6 @@ public class PermissionRepository {
     this.settings = settings;
   }
 
-  public void deleteGroupPermission(@Nullable Long resourceId, @Nullable Long groupId, String permission, DbSession session) {
-    GroupPermissionDto groupRole = new GroupPermissionDto()
-      .setRole(permission)
-      .setGroupId(groupId)
-      .setResourceId(resourceId);
-    updateProjectAuthorizationDate(session, resourceId);
-    dbClient.roleDao().deleteGroupRole(groupRole, session);
-  }
-
   /**
    * For each modification of permission on a project, update the authorization_updated_at to help ES reindex only relevant changes
    */
diff --git a/sonar-db/src/main/java/org/sonar/db/user/RoleDao.java b/sonar-db/src/main/java/org/sonar/db/user/RoleDao.java
index 04d8e82bde2..2eb62c0efc6 100644
--- a/sonar-db/src/main/java/org/sonar/db/user/RoleDao.java
+++ b/sonar-db/src/main/java/org/sonar/db/user/RoleDao.java
@@ -24,7 +24,6 @@ import javax.annotation.Nullable;
 import org.sonar.api.security.DefaultGroups;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
-import org.sonar.db.permission.GroupPermissionDto;
 
 public class RoleDao implements Dao {
 
@@ -45,14 +44,6 @@ public class RoleDao implements Dao {
     return session.getMapper(RoleMapper.class).selectGroupPermissions(groupName, resourceId, DefaultGroups.isAnyone(groupName));
   }
 
-  /**
-   * @deprecated does not support organizations on anyone groups
-   */
-  @Deprecated
-  public void deleteGroupRole(GroupPermissionDto groupRole, DbSession session) {
-    mapper(session).deleteGroupRole(groupRole);
-  }
-
   private static int countResourceGroupRoles(DbSession session, Long resourceId) {
     return mapper(session).countResourceGroupRoles(resourceId);
   }
diff --git a/sonar-db/src/main/java/org/sonar/db/user/RoleMapper.java b/sonar-db/src/main/java/org/sonar/db/user/RoleMapper.java
index 783514c5faf..23d154d5cbe 100644
--- a/sonar-db/src/main/java/org/sonar/db/user/RoleMapper.java
+++ b/sonar-db/src/main/java/org/sonar/db/user/RoleMapper.java
@@ -22,7 +22,6 @@ package org.sonar.db.user;
 import java.util.List;
 import javax.annotation.Nullable;
 import org.apache.ibatis.annotations.Param;
-import org.sonar.db.permission.GroupPermissionDto;
 
 /**
  * @since 3.2
@@ -36,8 +35,6 @@ public interface RoleMapper {
    */
   List<String> selectGroupPermissions(@Param("groupName") String groupName, @Nullable @Param("resourceId") Long resourceId, @Param("isAnyOneGroup") Boolean isAnyOneGroup);
 
-  void deleteGroupRole(GroupPermissionDto dto);
-
   int countResourceGroupRoles(Long resourceId);
 
   int countResourceUserRoles(long resourceId);
diff --git a/sonar-db/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml b/sonar-db/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
index 065111f7bfe..dff26a62a71 100644
--- a/sonar-db/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
+++ b/sonar-db/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml
@@ -168,4 +168,29 @@
     delete from group_roles
     where resource_id=#{rootComponentId,jdbcType=BIGINT}
   </delete>
+
+  <delete id="delete" parameterType="map">
+    delete from group_roles
+    where
+    role=#{permission,jdbcType=VARCHAR} and
+    organization_uuid=#{organizationUuid,jdbcType=VARCHAR} and
+    <choose>
+      <when test="rootComponentId != null">
+        resource_id=#{rootComponentId,jdbcType=BIGINT}
+      </when>
+      <otherwise>
+        resource_id is null
+      </otherwise>
+    </choose>
+    and
+    <choose>
+      <when test="groupId != null">
+        group_id=#{groupId,jdbcType=BIGINT}
+      </when>
+      <otherwise>
+        group_id is null
+      </otherwise>
+    </choose>
+  </delete>
+
 </mapper>
diff --git a/sonar-db/src/main/resources/org/sonar/db/user/RoleMapper.xml b/sonar-db/src/main/resources/org/sonar/db/user/RoleMapper.xml
index 27a020c8dce..0ac0d435512 100644
--- a/sonar-db/src/main/resources/org/sonar/db/user/RoleMapper.xml
+++ b/sonar-db/src/main/resources/org/sonar/db/user/RoleMapper.xml
@@ -49,29 +49,6 @@
     </where>
   </select>
 
-  <delete id="deleteGroupRole" parameterType="map">
-    DELETE FROM group_roles
-    WHERE role=#{role}
-    AND
-    <choose>
-      <when test="resourceId != null">
-        resource_id=#{resourceId}
-      </when>
-      <otherwise>
-        resource_id IS NULL
-      </otherwise>
-    </choose>
-    AND
-    <choose>
-      <when test="groupId != null">
-        group_id=#{groupId}
-      </when>
-      <otherwise>
-        group_id IS NULL
-      </otherwise>
-    </choose>
-  </delete>
-
   <select id="countResourceUserRoles" parameterType="long" resultType="int">
     SELECT count(1)
     FROM user_roles WHERE resource_id=#{id}
diff --git a/sonar-db/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java b/sonar-db/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java
index b1b4cc869f7..014fafcbb69 100644
--- a/sonar-db/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java
@@ -333,4 +333,72 @@ public class GroupPermissionDaoTest {
     assertThat(db.countSql("select count(id) from group_roles where resource_id=" + project1.getId())).isEqualTo(0);
     assertThat(db.countRowsOfTable("group_roles")).isEqualTo(2);
   }
+
+  @Test
+  public void delete_global_permission_from_group() {
+    GroupDto group1 = db.users().insertGroup();
+    ComponentDto project1 = db.components().insertProject();
+    db.users().insertPermissionOnAnyone("perm1");
+    db.users().insertPermissionOnGroup(group1, "perm2");
+    db.users().insertProjectPermissionOnGroup(group1, "perm3", project1);
+    db.users().insertProjectPermissionOnAnyone("perm4", project1);
+
+    underTest.delete(dbSession, "perm2", group1.getOrganizationUuid(), group1.getId(), null);
+    dbSession.commit();
+
+    assertThatNoPermission("perm2");
+    assertThat(db.countRowsOfTable("group_roles")).isEqualTo(3);
+  }
+
+  @Test
+  public void delete_global_permission_from_anyone() {
+    GroupDto group1 = db.users().insertGroup();
+    ComponentDto project1 = db.components().insertProject();
+    db.users().insertPermissionOnAnyone("perm1");
+    db.users().insertPermissionOnGroup(group1, "perm2");
+    db.users().insertProjectPermissionOnGroup(group1, "perm3", project1);
+    db.users().insertProjectPermissionOnAnyone("perm4", project1);
+
+    underTest.delete(dbSession, "perm1", group1.getOrganizationUuid(), null, null);
+    dbSession.commit();
+
+    assertThatNoPermission("perm1");
+    assertThat(db.countRowsOfTable("group_roles")).isEqualTo(3);
+  }
+
+  @Test
+  public void delete_project_permission_from_group() {
+    GroupDto group1 = db.users().insertGroup();
+    ComponentDto project1 = db.components().insertProject();
+    db.users().insertPermissionOnAnyone("perm1");
+    db.users().insertPermissionOnGroup(group1, "perm2");
+    db.users().insertProjectPermissionOnGroup(group1, "perm3", project1);
+    db.users().insertProjectPermissionOnAnyone("perm4", project1);
+
+    underTest.delete(dbSession, "perm3", group1.getOrganizationUuid(), group1.getId(), project1.getId());
+    dbSession.commit();
+
+    assertThatNoPermission("perm3");
+    assertThat(db.countRowsOfTable("group_roles")).isEqualTo(3);
+  }
+
+  @Test
+  public void delete_project_permission_from_anybody() {
+    GroupDto group1 = db.users().insertGroup();
+    ComponentDto project1 = db.components().insertProject();
+    db.users().insertPermissionOnAnyone("perm1");
+    db.users().insertPermissionOnGroup(group1, "perm2");
+    db.users().insertProjectPermissionOnGroup(group1, "perm3", project1);
+    db.users().insertProjectPermissionOnAnyone("perm4", project1);
+
+    underTest.delete(dbSession, "perm4", group1.getOrganizationUuid(), null, project1.getId());
+    dbSession.commit();
+
+    assertThatNoPermission("perm4");
+    assertThat(db.countRowsOfTable("group_roles")).isEqualTo(3);
+  }
+
+  private void assertThatNoPermission(String permission) {
+    assertThat(db.countSql("select count(id) from group_roles where role='" + permission + "'")).isEqualTo(0);
+  }
 }
diff --git a/sonar-db/src/test/java/org/sonar/db/permission/PermissionRepositoryTest.java b/sonar-db/src/test/java/org/sonar/db/permission/PermissionRepositoryTest.java
index 5b53fef921f..ecdf59dd0b9 100644
--- a/sonar-db/src/test/java/org/sonar/db/permission/PermissionRepositoryTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/permission/PermissionRepositoryTest.java
@@ -110,18 +110,6 @@ public class PermissionRepositoryTest {
     dbTester.assertDbUnitTable(getClass(), "apply_default_permission_template-result.xml", "user_roles", "user_id", "resource_id", "role");
   }
 
-  @Test
-  public void should_delete_group_permission() {
-    dbTester.prepareDbUnit(getClass(), "should_delete_group_permission.xml");
-
-    underTest.deleteGroupPermission(PROJECT.getId(), 100L, UserRole.USER, session);
-    session.commit();
-
-    dbTester.assertDbUnitTable(getClass(), "should_delete_group_permission-result.xml", "group_roles", "group_id", "resource_id", "role");
-    dbTester.assertDbUnitTable(getClass(), "should_delete_group_permission-result.xml", "projects", "authorization_updated_at");
-    checkAuthorizationUpdatedAtIsUpdated();
-  }
-
   @Test
   public void would_user_have_permission_with_default_permission_template() {
     UserDto user = dbTester.users().insertUser();
diff --git a/sonar-db/src/test/java/org/sonar/db/user/RoleDaoTest.java b/sonar-db/src/test/java/org/sonar/db/user/RoleDaoTest.java
index e092e46465a..843cd398aa9 100644
--- a/sonar-db/src/test/java/org/sonar/db/user/RoleDaoTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/user/RoleDaoTest.java
@@ -31,7 +31,6 @@ import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDbTester;
 import org.sonar.db.component.ComponentDto;
-import org.sonar.db.permission.GroupPermissionDto;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.sonar.db.user.GroupTesting.newGroupDto;
@@ -117,30 +116,6 @@ public class RoleDaoTest {
     assertThat(underTest.selectGroupPermissions(db.getSession(), "sonar-users", 1L)).containsOnly(UserRole.CODEVIEWER);
   }
 
-  @Test
-  public void delete_global_group_permission() {
-    db.prepareDbUnit(getClass(), "globalGroupPermissions.xml");
-
-    GroupPermissionDto groupRoleToDelete = new GroupPermissionDto().setGroupId(100L).setRole(GlobalPermissions.QUALITY_PROFILE_ADMIN);
-
-    underTest.deleteGroupRole(groupRoleToDelete, db.getSession());
-    db.getSession().commit();
-
-    db.assertDbUnit(getClass(), "globalGroupPermissions-result.xml", "group_roles");
-  }
-
-  @Test
-  public void delete_resource_group_permission() {
-    db.prepareDbUnit(getClass(), "resourceGroupPermissions.xml");
-
-    GroupPermissionDto groupRoleToDelete = new GroupPermissionDto().setGroupId(100L).setRole(UserRole.CODEVIEWER).setResourceId(1L);
-
-    underTest.deleteGroupRole(groupRoleToDelete, db.getSession());
-    db.getSession().commit();
-
-    db.assertDbUnit(getClass(), "resourceGroupPermissions-result.xml", "group_roles");
-  }
-
   @Test
   public void delete_all_group_permissions_by_group_id() {
     db.prepareDbUnit(getClass(), "deleteGroupPermissionsByGroupId.xml");
diff --git a/sonar-db/src/test/java/org/sonar/db/user/UserDbTester.java b/sonar-db/src/test/java/org/sonar/db/user/UserDbTester.java
index a6501d831ca..a2a75166c45 100644
--- a/sonar-db/src/test/java/org/sonar/db/user/UserDbTester.java
+++ b/sonar-db/src/test/java/org/sonar/db/user/UserDbTester.java
@@ -71,6 +71,14 @@ public class UserDbTester {
     return insertGroup(group);
   }
 
+  /**
+   * Create group in default organization
+   */
+  public GroupDto insertGroup() {
+    GroupDto group = newGroupDto().setOrganizationUuid(db.getDefaultOrganization().getUuid());
+    return insertGroup(group);
+  }
+
   public GroupDto insertGroup(GroupDto dto) {
     db.getDbClient().groupDao().insert(db.getSession(), dto);
     db.commit();
diff --git a/sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission-result.xml b/sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission-result.xml
deleted file mode 100644
index efd14a5c8fb..00000000000
--- a/sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission-result.xml
+++ /dev/null
@@ -1,30 +0,0 @@
-<dataset>
-
-  <groups id="100"
-          name="devs"
-          organization_uuid="org1"/>
-
-  <group_roles id="1"
-               group_id="100"
-               resource_id="123"
-               role="admin"
-               organization_uuid="org1"/>
-
-  <projects uuid="A"
-            uuid_path="NOT_USED"
-            root_uuid="A"
-            scope="PRJ"
-            qualifier="TRK"
-            kee="org.struts:struts"
-            name="Struts"
-            description="the description"
-            long_name="Apache Struts"
-            enabled="[true]"
-            language="java"
-            copy_component_uuid="[null]"
-            developer_uuid="[null]"
-            path="[null]"
-            authorization_updated_at="123456789"
-            id="123"/>
-
-</dataset>
diff --git a/sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission.xml b/sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission.xml
deleted file mode 100644
index a66c2de8f72..00000000000
--- a/sonar-db/src/test/resources/org/sonar/db/permission/PermissionRepositoryTest/should_delete_group_permission.xml
+++ /dev/null
@@ -1,35 +0,0 @@
-<dataset>
-
-  <groups id="100"
-          name="devs"
-          organization_uuid="org1"/>
-
-  <group_roles id="1"
-               group_id="100"
-               resource_id="123"
-               role="admin"
-               organization_uuid="org1"/>
-  <group_roles id="2"
-               group_id="100"
-               resource_id="123"
-               role="user"
-               organization_uuid="org1"/>
-
-  <projects uuid="A"
-            uuid_path="NOT_USED"
-            root_uuid="A"
-            scope="PRJ"
-            qualifier="TRK"
-            kee="org.struts:struts"
-            name="Struts"
-            description="the description"
-            long_name="Apache Struts"
-            enabled="[true]"
-            language="java"
-            copy_component_uuid="[null]"
-            developer_uuid="[null]"
-            path="[null]"
-            authorization_updated_at="123456789"
-            id="123"/>
-
-</dataset>
-- 
2.39.5