SONAR-4463 Fixed quality flaws and updated permission template service layer

9 files changed, 49 insertions, 21 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/DaoUtils.java b/sonar-core/src/main/java/org/sonar/core/persistence/DaoUtils.java
index b715ac36950..1798e89e920 100644
--- a/sonar-core/src/main/java/org/sonar/core/persistence/DaoUtils.java
+++ b/sonar-core/src/main/java/org/sonar/core/persistence/DaoUtils.java
@@ -34,10 +34,7 @@ import org.sonar.core.resource.ResourceKeyUpdaterDao;
 import org.sonar.core.rule.RuleDao;
 import org.sonar.core.source.jdbc.SnapshotDataDao;
 import org.sonar.core.template.LoadedTemplateDao;
-import org.sonar.core.user.AuthorDao;
-import org.sonar.core.user.AuthorizationDao;
-import org.sonar.core.user.RoleDao;
-import org.sonar.core.user.UserDao;
+import org.sonar.core.user.*;
 
 import java.util.List;
 
@@ -64,6 +61,7 @@ public final class DaoUtils {
       IssueFilterFavouriteDao.class,
       LoadedTemplateDao.class,
       MeasureFilterDao.class,
+      PermissionDao.class,
       PropertiesDao.class,
       PurgeDao.class,
       ResourceIndexerDao.class,
diff --git a/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java b/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java
index 9f99630b65f..14deada2ff3 100644
--- a/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/user/PermissionDao.java
@@ -55,10 +55,15 @@ public class PermissionDao implements TaskExtension, ServerExtension {
     SqlSession session = myBatis.openSession();
     try {
       PermissionTemplateMapper mapper = session.getMapper(PermissionTemplateMapper.class);
-      permissionTemplate = mapper.selectTemplateUsersPermissions(templateName);
-      PermissionTemplateDto templateWithGroupsPermissions = mapper.selectTemplateGroupsPermissions(templateName);
-      permissionTemplate.setGroupsByPermission(templateWithGroupsPermissions.getGroupsPermissions());
-      session.commit();
+      permissionTemplate = mapper.selectByName(templateName);
+      PermissionTemplateDto templateUsersPermissions = mapper.selectTemplateUsersPermissions(templateName);
+      if(templateUsersPermissions != null) {
+        permissionTemplate.setUsersPermissions(templateUsersPermissions.getUsersPermissions());
+      }
+      PermissionTemplateDto templateGroupsPermissions = mapper.selectTemplateGroupsPermissions(templateName);
+      if(templateGroupsPermissions != null) {
+        permissionTemplate.setGroupsByPermission(templateGroupsPermissions.getGroupsPermissions());
+      }
     } finally {
       MyBatis.closeQuietly(session);
     }
diff --git a/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java
index 01e0c6e4d4c..de5aef22b0a 100644
--- a/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/user/PermissionDaoTest.java
@@ -63,6 +63,18 @@ public class PermissionDaoTest extends AbstractDaoTestCase {
   }
 
   @Test
+  public void should_select_empty_permission_template() throws Exception {
+    setupData("selectEmptyPermissionTemplate");
+    PermissionTemplateDto permissionTemplate = permissionDao.selectPermissionTemplate("my template");
+
+    assertThat(permissionTemplate).isNotNull();
+    assertThat(permissionTemplate.getName()).isEqualTo("my template");
+    assertThat(permissionTemplate.getDescription()).isEqualTo("my description");
+    assertThat(permissionTemplate.getUsersPermissions()).isNull();
+    assertThat(permissionTemplate.getGroupsPermissions()).isNull();
+  }
+
+  @Test
   public void should_permission_template_by_name() throws Exception {
     setupData("selectPermissionTemplate");
 
diff --git a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
index 7be63ada0cf..7854a6bd12a 100644
--- a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
+++ b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
@@ -135,11 +135,13 @@ public class InternalPermissionTemplateService implements ServerComponent {
 
   private void checkThatTemplateNameIsUnique(String name) {
     List<PermissionTemplateDto> existingTemplates = permissionDao.selectAllPermissionTemplates();
-    for (PermissionTemplateDto existingTemplate : existingTemplates) {
-      if(existingTemplate.getName().equals(name)) {
-        String errorMsg = "A template with that name already exists";
-        LOG.error(errorMsg);
-        throw new BadRequestException(errorMsg);
+    if(existingTemplates != null) {
+      for (PermissionTemplateDto existingTemplate : existingTemplates) {
+        if(existingTemplate.getName().equals(name)) {
+          String errorMsg = "A template with that name already exists";
+          LOG.error(errorMsg);
+          throw new BadRequestException(errorMsg);
+        }
       }
     }
   }
diff --git a/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplate.java b/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplate.java
index 97054b98576..a7d51d85355 100644
--- a/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplate.java
+++ b/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplate.java
@@ -27,6 +27,7 @@ import org.sonar.core.user.PermissionTemplateDto;
 import org.sonar.core.user.PermissionTemplateGroupDto;
 import org.sonar.core.user.PermissionTemplateUserDto;
 
+import javax.annotation.Nullable;
 import java.util.List;
 
 public class PermissionTemplate {
@@ -45,7 +46,10 @@ public class PermissionTemplate {
     groupsByPermission = HashMultimap.create();
   }
 
-  public static PermissionTemplate create(PermissionTemplateDto permissionTemplateDto) {
+  public static PermissionTemplate create(@Nullable PermissionTemplateDto permissionTemplateDto) {
+    if(permissionTemplateDto == null) {
+      return null;
+    }
     PermissionTemplate permissionTemplate =
       new PermissionTemplate(permissionTemplateDto.getId(), permissionTemplateDto.getName(), permissionTemplateDto.getDescription());
     if(permissionTemplateDto.getUsersPermissions() != null) {
diff --git a/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java b/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java
index 24ac25b5e56..9ab56eadb04 100644
--- a/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java
+++ b/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java
@@ -20,10 +20,14 @@
 
 package org.sonar.server.permission;
 
+import com.google.common.collect.Lists;
+import org.sonar.api.web.UserRole;
 import org.sonar.core.user.*;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.user.UserSession;
 
+import java.util.List;
+
 abstract class PermissionTemplateUpdater {
 
   private final String templateName;
@@ -72,7 +76,8 @@ abstract class PermissionTemplateUpdater {
   }
 
   private void validatePermission(String permission) {
-    if(!Permission.isValid(permission)) {
+    List<String> supportedPermissions = Lists.newArrayList(UserRole.ADMIN, UserRole.CODEVIEWER, UserRole.USER);
+    if(permission == null || !supportedPermissions.contains(permission)) {
       throw new BadRequestException("Invalid permission: " + permission);
     }
   }
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/internal.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/internal.rb
index ded7ff05eca..acb9d973e9f 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/internal.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/internal.rb
@@ -51,7 +51,7 @@ class Internal
   end
 
   def self.permission_templates
-    component(Java::OrgSonarServierPermission::InternalPermissionTemplateService.java_class)
+    component(Java::OrgSonarServerPermission::InternalPermissionTemplateService.java_class)
   end
 
   private
diff --git a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
index d7e9bf89028..5ade294c424 100644
--- a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
@@ -25,6 +25,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.api.web.UserRole;
 import org.sonar.core.user.*;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.user.MockUserSession;
@@ -38,7 +39,7 @@ public class InternalPermissionTemplateServiceTest {
 
   private static final String DEFAULT_NAME = "my template";
   private static final String DEFAULT_DESC = "my description";
-  private static final String DEFAULT_PERMISSION = Permission.DRY_RUN_EXECUTION.key();
+  private static final String DEFAULT_PERMISSION = UserRole.USER;
   private static final PermissionTemplateDto DEFAULT_TEMPLATE =
     new PermissionTemplateDto().setId(1L).setName(DEFAULT_NAME).setDescription(DEFAULT_DESC);
 
diff --git a/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java b/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java
index 9af7eeb66ab..e2eda87c932 100644
--- a/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java
@@ -24,6 +24,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.api.web.UserRole;
 import org.sonar.core.user.*;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
@@ -57,15 +58,15 @@ public class PermissionTemplateUpdaterTest {
     when(permissionDao.selectTemplateByName("my template")).thenReturn(new PermissionTemplateDto().setId(1L));
 
     PermissionTemplateUpdater updater =
-      new PermissionTemplateUpdater("my template", Permission.SCAN_EXECUTION.key(), "user", permissionDao, userDao) {
+      new PermissionTemplateUpdater("my template", UserRole.USER, "user", permissionDao, userDao) {
       @Override
       void doExecute(Long templateId, String permission) {
-        permissionDao.addUserPermission(1L, 1L, Permission.SCAN_EXECUTION.key());
+        permissionDao.addUserPermission(1L, 1L, UserRole.USER);
       }
     };
     updater.executeUpdate();
 
-    verify(permissionDao, times(1)).addUserPermission(1L, 1L, Permission.SCAN_EXECUTION.key());
+    verify(permissionDao, times(1)).addUserPermission(1L, 1L, UserRole.USER);
   }
 
   @Test
@@ -77,7 +78,7 @@ public class PermissionTemplateUpdaterTest {
     when(permissionDao.selectTemplateByName("my template")).thenReturn(null);
 
     PermissionTemplateUpdater updater =
-      new PermissionTemplateUpdater("my template", Permission.SCAN_EXECUTION.key(), "user", permissionDao, userDao) {
+      new PermissionTemplateUpdater("my template", UserRole.USER, "user", permissionDao, userDao) {
         @Override
         void doExecute(Long templateId, String permission) {
         }