From 02233758a0ae6932ccc8cd148f2b9787359df18d Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 26 May 2017 12:32:25 +0200 Subject: [PATCH] SONAR-9327 delete permissions only for root when deleting a project/view --- .../org/sonar/db/purge/PurgeCommands.java | 22 ++++--- .../java/org/sonar/db/purge/PurgeDao.java | 6 ++ .../java/org/sonar/db/purge/PurgeMapper.java | 4 +- .../org/sonar/db/purge/PurgeMapper.xml | 10 +-- .../org/sonar/db/purge/PurgeCommandsTest.java | 62 +++++++++++++++++++ .../java/org/sonar/db/purge/PurgeDaoTest.java | 5 +- 6 files changed, 87 insertions(+), 22 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java index a2abbe53182..38e9e8e6140 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java @@ -60,6 +60,18 @@ class PurgeCommands { deleteAnalyses(purgeMapper.selectAnalysisIdsAndUuids(new PurgeSnapshotQuery().setComponentUuid(rootUuid))); } + void deletePermissions(long rootId) { + profiler.start("deletePermissions (group_roles)"); + purgeMapper.deleteComponentGroupRoles(rootId); + session.commit(); + profiler.stop(); + + profiler.start("deletePermissions (user_roles)"); + purgeMapper.deleteComponentUserRoles(rootId); + session.commit(); + profiler.stop(); + } + void deleteComponents(List componentIdUuids) { List> componentIdPartitions = Lists.partition(IdUuidPairs.ids(componentIdUuids), MAX_RESOURCES_PER_QUERY); List> componentUuidsPartitions = Lists.partition(IdUuidPairs.uuids(componentIdUuids), MAX_RESOURCES_PER_QUERY); @@ -79,16 +91,6 @@ class PurgeCommands { session.commit(); profiler.stop(); - profiler.start("deleteResourceGroupRoles (group_roles)"); - componentIdPartitions.forEach(purgeMapper::deleteComponentGroupRoles); - session.commit(); - profiler.stop(); - - profiler.start("deleteResourceUserRoles (user_roles)"); - componentIdPartitions.forEach(purgeMapper::deleteComponentUserRoles); - session.commit(); - profiler.stop(); - profiler.start("deleteResourceManualMeasures (manual_measures)"); componentUuidsPartitions.forEach(purgeMapper::deleteComponentManualMeasures); session.commit(); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java index 5ac77a792b7..8c411368257 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java @@ -154,6 +154,12 @@ public class PurgeDao implements Dao { private static void deleteProject(String rootUuid, PurgeMapper mapper, PurgeCommands commands) { List childrenIds = mapper.selectComponentsByProjectUuid(rootUuid); + long rootId = childrenIds.stream() + .filter(pair -> pair.getUuid().equals(rootUuid)) + .map(IdUuidPair::getId) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Couldn't find component for root uuid " + rootUuid)); + commands.deletePermissions(rootId); commands.deleteAnalyses(rootUuid); commands.deleteComponents(childrenIds); commands.deleteFileSources(rootUuid); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java index 08ec08c0a16..a0e3248320f 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java @@ -56,9 +56,9 @@ public interface PurgeMapper { void deleteComponents(@Param("componentUuids") List componentUuids); - void deleteComponentGroupRoles(@Param("componentIds") List componentIds); + void deleteComponentGroupRoles(@Param("rootId") long rootId); - void deleteComponentUserRoles(@Param("componentIds") List componentIds); + void deleteComponentUserRoles(@Param("rootId") long rootId); void deleteComponentManualMeasures(@Param("componentUuids") List componentUuids); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml index 67eec7c0f76..edec6e6e0e5 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml @@ -181,19 +181,13 @@ delete from group_roles where - resource_id in - - #{componentId} - + resource_id = #{rootId,jdbcType=INTEGER} delete from user_roles where - resource_id in - - #{componentId} - + resource_id = #{rootId,jdbcType=INTEGER} diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeCommandsTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeCommandsTest.java index 656a69f9cb4..c2586d6c7b3 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeCommandsTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeCommandsTest.java @@ -24,6 +24,11 @@ import org.junit.Rule; import org.junit.Test; import org.sonar.api.utils.System2; import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.permission.OrganizationPermission; +import org.sonar.db.user.GroupDto; +import org.sonar.db.user.UserDto; import static com.google.common.collect.Lists.newArrayList; import static java.util.Collections.singletonList; @@ -105,6 +110,63 @@ public class PurgeCommandsTest { assertThat(dbTester.countRowsOfTable("issue_changes")).isEqualTo(1); } + @Test + public void deletePermissions_deletes_permissions_of_public_project() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto project = dbTester.components().insertPublicProject(organization); + addPermissions(organization, project); + + PurgeCommands purgeCommands = new PurgeCommands(dbTester.getSession(), profiler); + purgeCommands.deletePermissions(project.getId()); + + assertThat(dbTester.countRowsOfTable("group_roles")).isEqualTo(2); + assertThat(dbTester.countRowsOfTable("user_roles")).isEqualTo(1); + } + + @Test + public void deletePermissions_deletes_permissions_of_private_project() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto project = dbTester.components().insertPrivateProject(organization); + addPermissions(organization, project); + + PurgeCommands purgeCommands = new PurgeCommands(dbTester.getSession(), profiler); + purgeCommands.deletePermissions(project.getId()); + + assertThat(dbTester.countRowsOfTable("group_roles")).isEqualTo(1); + assertThat(dbTester.countRowsOfTable("user_roles")).isEqualTo(1); + } + + @Test + public void deletePermissions_deletes_permissions_of_view() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto project = dbTester.components().insertView(organization); + addPermissions(organization, project); + + PurgeCommands purgeCommands = new PurgeCommands(dbTester.getSession(), profiler); + purgeCommands.deletePermissions(project.getId()); + + assertThat(dbTester.countRowsOfTable("group_roles")).isEqualTo(2); + assertThat(dbTester.countRowsOfTable("user_roles")).isEqualTo(1); + } + + private void addPermissions(OrganizationDto organization, ComponentDto root) { + if (!root.isPrivate()) { + dbTester.users().insertProjectPermissionOnAnyone("foo1", root); + dbTester.users().insertPermissionOnAnyone(organization, "not project level"); + } + + GroupDto group = dbTester.users().insertGroup(organization); + dbTester.users().insertProjectPermissionOnGroup(group, "bar", root); + dbTester.users().insertPermissionOnGroup(group, "not project level"); + + UserDto user = dbTester.users().insertUser(); + dbTester.users().insertProjectPermissionOnUser(user, "doh", root); + dbTester.users().insertPermissionOnUser(user, OrganizationPermission.SCAN); + + assertThat(dbTester.countRowsOfTable("group_roles")).isEqualTo(root.isPrivate() ? 2 : 4); + assertThat(dbTester.countRowsOfTable("user_roles")).isEqualTo(2); + } + /** * Test that SQL queries execution do not fail with a huge number of parameter */ diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java index 966708909a1..02ff99161e2 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java @@ -235,10 +235,11 @@ public class PurgeDaoTest { @Test public void deleteProject_deletes_webhook_deliveries() { - dbClient.webhookDeliveryDao().insert(dbSession, newWebhookDeliveryDto().setComponentUuid("P1").setUuid("D1")); + ComponentDto project = dbTester.components().insertPublicProject(); + dbClient.webhookDeliveryDao().insert(dbSession, newWebhookDeliveryDto().setComponentUuid(project.uuid()).setUuid("D1")); dbClient.webhookDeliveryDao().insert(dbSession, newWebhookDeliveryDto().setComponentUuid("P2").setUuid("D2")); - underTest.deleteProject(dbSession, "P1"); + underTest.deleteProject(dbSession, project.uuid()); assertThat(selectAllDeliveryUuids(dbTester, dbSession)).containsOnly("D2"); } -- 2.39.5