SONAR-5175 When deleting a group, rows related to this group in perm_templates_groups table should be deleted

9 files changed, 94 insertions, 7 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
index b6e2538ae0c..97b1798ada7 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
@@ -23,13 +23,20 @@ package org.sonar.server.permission;
 import com.google.common.collect.Lists;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.api.ServerComponent;
+import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.permission.PermissionTemplateDao;
 import org.sonar.core.permission.PermissionTemplateDto;
+import org.sonar.core.persistence.DbSession;
+import org.sonar.core.persistence.MyBatis;
+import org.sonar.core.user.GroupDto;
 import org.sonar.core.user.UserDao;
 import org.sonar.server.exceptions.BadRequestException;
+import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.user.UserSession;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Pattern;
@@ -40,11 +47,13 @@ import java.util.regex.PatternSyntaxException;
  */
 public class InternalPermissionTemplateService implements ServerComponent {
 
+  private final MyBatis myBatis;
   private final PermissionTemplateDao permissionTemplateDao;
   private final UserDao userDao;
   private final PermissionFinder finder;
 
-  public InternalPermissionTemplateService(PermissionTemplateDao permissionTemplateDao, UserDao userDao, PermissionFinder finder) {
+  public InternalPermissionTemplateService(MyBatis myBatis, PermissionTemplateDao permissionTemplateDao, UserDao userDao, PermissionFinder finder) {
+    this.myBatis = myBatis;
     this.permissionTemplateDao = permissionTemplateDao;
     this.userDao = userDao;
     this.finder = finder;
@@ -145,6 +154,21 @@ public class InternalPermissionTemplateService implements ServerComponent {
     updater.executeUpdate();
   }
 
+  public void removeGroupFromTemplates(String groupName) {
+    UserSession.get().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN);
+    DbSession session = myBatis.openSession(false);
+    try {
+      GroupDto group = userDao.selectGroupByName(groupName, session);
+      if (group == null) {
+        throw new NotFoundException("Group does not exists : " + groupName);
+      }
+      permissionTemplateDao.removeByGroup(group.getId(), session);
+      session.commit();
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+  }
+
   private void validateTemplateName(@Nullable Long templateId, String templateName) {
     if (StringUtils.isEmpty(templateName)) {
       String errorMsg = "Name can't be blank";
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
index 276f056b089..c50acc61604 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
@@ -31,6 +31,8 @@ import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.web.UserRole;
 import org.sonar.core.permission.*;
+import org.sonar.core.persistence.DbSession;
+import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.user.GroupDto;
 import org.sonar.core.user.UserDao;
 import org.sonar.core.user.UserDto;
@@ -61,6 +63,9 @@ public class InternalPermissionTemplateServiceTest {
   @Mock
   PermissionFinder finder;
 
+  @Mock
+  DbSession session;
+
   InternalPermissionTemplateService service;
 
   @Rule
@@ -69,7 +74,10 @@ public class InternalPermissionTemplateServiceTest {
   @Before
   public void setUp() {
     MockUserSession.set().setLogin("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
-    service = new InternalPermissionTemplateService(permissionTemplateDao, userDao, finder);
+
+    MyBatis myBatis = mock(MyBatis.class);
+    when(myBatis.openSession(false)).thenReturn(session);
+    service = new InternalPermissionTemplateService(myBatis, permissionTemplateDao, userDao, finder);
   }
 
   @Test
@@ -339,6 +347,16 @@ public class InternalPermissionTemplateServiceTest {
     verifyZeroInteractions(userDao);
   }
 
+  @Test
+  public void should_remove_group_from_templates() throws Exception {
+    GroupDto groupDto = new GroupDto().setId(1L).setName("group");
+    when(userDao.selectGroupByName("group", session)).thenReturn(groupDto);
+
+    service.removeGroupFromTemplates("group");
+
+    verify(permissionTemplateDao).removeByGroup(eq(1L), eq(session));
+  }
+
   private PermissionTemplateUserDto buildUserPermission(String userName, String permission) {
     return new PermissionTemplateUserDto().setUserName(userName).setPermission(permission);
   }
diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb
index 4322e4876f0..51a6a8063ba 100644
--- a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb
+++ b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb
@@ -75,8 +75,11 @@ class GroupsController < ApplicationController
     verify_post_request
     require_parameters :id
     group = Group.find(params[:id])
-    if group.destroy
-      flash[:notice] = 'Group is deleted.'
+    call_backend do
+      Internal.permission_templates.removeGroupFromTemplates(group.name)
+      if group.destroy
+        flash[:notice] = 'Group is deleted.'
+      end
     end
     to_index(group.errors, nil)
   end
diff --git a/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateDao.java b/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateDao.java
index 9ec979bc283..c28804d6d9b 100644
--- a/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateDao.java
@@ -32,6 +32,7 @@ import org.sonar.core.persistence.MyBatis;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.text.Normalizer;
 import java.util.Date;
 import java.util.List;
@@ -244,6 +245,13 @@ public class PermissionTemplateDao implements TaskComponent, ServerComponent {
     }
   }
 
+  /**
+   * Remove a group from all templates (used when removing a group)
+   */
+  public void removeByGroup(Long groupId, SqlSession session) {
+      session.getMapper(PermissionTemplateMapper.class).deleteByGroupId(groupId);
+  }
+
   private String generateTemplateKee(String name, Date timeStamp) {
     if (PermissionTemplateDto.DEFAULT.getName().equals(name)) {
       return PermissionTemplateDto.DEFAULT.getKee();
diff --git a/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateMapper.java b/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateMapper.java
index 63c2b7b9ff5..79e50415ffd 100644
--- a/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/permission/PermissionTemplateMapper.java
@@ -48,4 +48,6 @@ public interface PermissionTemplateMapper {
   void insertGroupPermission(PermissionTemplateGroupDto permissionTemplateGroup);
 
   void deleteGroupPermission(PermissionTemplateGroupDto permissionTemplateGroup);
+
+  void deleteByGroupId(long groupId);
 }
diff --git a/sonar-core/src/main/resources/org/sonar/core/permission/PermissionTemplateMapper.xml b/sonar-core/src/main/resources/org/sonar/core/permission/PermissionTemplateMapper.xml
index 7791fb0aa89..2ec027daa89 100644
--- a/sonar-core/src/main/resources/org/sonar/core/permission/PermissionTemplateMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/permission/PermissionTemplateMapper.xml
@@ -61,6 +61,11 @@
     </choose>
   </delete>
 
+  <delete id="deleteByGroupId" parameterType="long">
+    DELETE FROM perm_templates_groups
+    WHERE group_id = #{groupId}
+  </delete>
+
   <select id="selectUsers" parameterType="map" resultType="UserWithPermission">
     SELECT u.login as login, u.name as name, ptu.permission_reference as permission
     FROM users u
diff --git a/sonar-core/src/test/java/org/sonar/core/permission/PermissionTemplateDaoTest.java b/sonar-core/src/test/java/org/sonar/core/permission/PermissionTemplateDaoTest.java
index 5a71651c35e..0bb6f825edd 100644
--- a/sonar-core/src/test/java/org/sonar/core/permission/PermissionTemplateDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/permission/PermissionTemplateDaoTest.java
@@ -20,6 +20,7 @@
 
 package org.sonar.core.permission;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
@@ -33,23 +34,28 @@ import java.util.Date;
 import java.util.List;
 
 import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 public class PermissionTemplateDaoTest extends AbstractDaoTestCase {
 
   Date now;
   PermissionTemplateDao permissionTemplateDao;
+  DbSession session;
   System2 system = mock(System2.class);
 
   @Before
   public void setUpDao() throws ParseException {
+    session = getMyBatis().openSession(false);
     now = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").parse("2013-01-02 01:04:05");
     when(system.now()).thenReturn(now.getTime());
     permissionTemplateDao = new PermissionTemplateDao(getMyBatis(), system);
   }
 
+  @After
+  public void after() {
+    this.session.close();
+  }
+
   @Test
   public void should_create_permission_template() throws Exception {
     setupData("createPermissionTemplate");
@@ -207,6 +213,15 @@ public class PermissionTemplateDaoTest extends AbstractDaoTestCase {
   }
 
   @Test
+  public void remove_by_group() throws Exception {
+    setupData("remove_by_group");
+    permissionTemplateDao.removeByGroup(2L, session);
+    session.commit();
+
+    checkTable("remove_by_group", "perm_templates_groups", "id", "template_id", "group_id", "permission_reference");
+  }
+
+  @Test
   public void should_add_group_permission_with_null_name() throws Exception {
     setupData("addNullGroupPermissionToTemplate");
     permissionTemplateDao.addGroupPermission(1L, null, "new_permission");
diff --git a/sonar-core/src/test/resources/org/sonar/core/permission/PermissionTemplateDaoTest/remove_by_group-result.xml b/sonar-core/src/test/resources/org/sonar/core/permission/PermissionTemplateDaoTest/remove_by_group-result.xml
new file mode 100644
index 00000000000..94ab2fde7ed
--- /dev/null
+++ b/sonar-core/src/test/resources/org/sonar/core/permission/PermissionTemplateDaoTest/remove_by_group-result.xml
@@ -0,0 +1,5 @@
+<dataset>
+  <permission_templates id="1" name="my template" kee="my_template_20130102_030405" description="my description"/>
+  <perm_templates_users/>
+  <perm_templates_groups id="1" template_id="1" group_id="1" permission_reference="remaining_permission_other_group"/>
+</dataset>
diff --git a/sonar-core/src/test/resources/org/sonar/core/permission/PermissionTemplateDaoTest/remove_by_group.xml b/sonar-core/src/test/resources/org/sonar/core/permission/PermissionTemplateDaoTest/remove_by_group.xml
new file mode 100644
index 00000000000..2ff5815d855
--- /dev/null
+++ b/sonar-core/src/test/resources/org/sonar/core/permission/PermissionTemplateDaoTest/remove_by_group.xml
@@ -0,0 +1,7 @@
+<dataset>
+  <permission_templates id="1" name="my template" kee="my_template_20130102_030405" description="my description"/>
+  <perm_templates_users/>
+  <perm_templates_groups id="1" template_id="1" group_id="1" permission_reference="remaining_permission_other_group"/>
+  <perm_templates_groups id="2" template_id="1" group_id="2" permission_reference="permission_to_remove"/>
+  <perm_templates_groups id="3" template_id="1" group_id="2" permission_reference="permission_to_remove2"/>
+</dataset>