SONAR-4463 Fixed quality issues

4 files changed, 31 insertions, 13 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java b/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java
index 35506cd04ef..eb8febb0fc5 100644
--- a/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java
+++ b/sonar-core/src/main/java/org/sonar/core/resource/DefaultResourcePermissions.java
@@ -156,8 +156,9 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte
 
   private List<String> getEligibleGroups(String role, PermissionTemplateDto permissionTemplate) {
     List<String> eligibleGroups = new ArrayList<String>();
-    if(permissionTemplate.getGroupsPermissions() != null) {
-      for (PermissionTemplateGroupDto groupPermission : permissionTemplate.getGroupsPermissions()) {
+    List<PermissionTemplateGroupDto> groupsPermissions = permissionTemplate.getGroupsPermissions();
+    if(groupsPermissions != null) {
+      for (PermissionTemplateGroupDto groupPermission : groupsPermissions) {
         if(role.equals(groupPermission.getPermission())) {
           String groupName = groupPermission.getGroupName() != null ? groupPermission.getGroupName() : DefaultGroups.ANYONE;
           eligibleGroups.add(groupName);
@@ -169,8 +170,9 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte
 
   private List<String> getEligibleUsers(String role, PermissionTemplateDto permissionTemplate) {
     List<String> eligibleUsers = new ArrayList<String>();
-    if(permissionTemplate.getUsersPermissions() != null) {
-      for (PermissionTemplateUserDto userPermission : permissionTemplate.getUsersPermissions()) {
+    List<PermissionTemplateUserDto> usersPermissions = permissionTemplate.getUsersPermissions();
+    if(usersPermissions != null) {
+      for (PermissionTemplateUserDto userPermission : usersPermissions) {
         if(role.equals(userPermission.getPermission())) {
           eligibleUsers.add(userPermission.getUserLogin());
         }
@@ -187,7 +189,7 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte
 
     String defaultTemplateId = settings.getString("sonar.permission.template.default");
     if(StringUtils.isBlank(defaultTemplateId)) {
-      throw new RuntimeException("At least one default permission template should be defined");
+      throw new IllegalStateException("At least one default permission template should be defined");
     }
     return getTemplateWithPermissions(defaultTemplateId);
   }
@@ -195,11 +197,11 @@ public class DefaultResourcePermissions implements ResourcePermissions, TaskExte
   private PermissionTemplateDto getTemplateWithPermissions(String templateId) {
     PermissionTemplateDto permissionTemplateDto = permissionDao.selectTemplateById(Long.parseLong(templateId));
     if(permissionTemplateDto == null) {
-      throw new RuntimeException("Could not retrieve permission template with id " + templateId);
+      throw new IllegalArgumentException("Could not retrieve permission template with id " + templateId);
     }
     PermissionTemplateDto templateWithPermissions = permissionDao.selectPermissionTemplate(permissionTemplateDto.getName());
     if(templateWithPermissions == null) {
-      throw new RuntimeException("Could not retrieve permissions for template with id " + templateId);
+      throw new IllegalArgumentException("Could not retrieve permissions for template with id " + templateId);
     }
     return templateWithPermissions;
   }
diff --git a/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java b/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java
index 3a2bc1b9095..93db1524086 100644
--- a/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java
+++ b/sonar-core/src/main/java/org/sonar/core/user/PermissionTemplateDto.java
@@ -68,7 +68,7 @@ public class PermissionTemplateDto {
     return usersPermissions;
   }
 
-  public PermissionTemplateDto setUsersPermissions(List<PermissionTemplateUserDto> usersPermissions) {
+  public PermissionTemplateDto setUsersPermissions(@Nullable List<PermissionTemplateUserDto> usersPermissions) {
     this.usersPermissions = usersPermissions;
     return this;
   }
@@ -78,7 +78,7 @@ public class PermissionTemplateDto {
     return groupsPermissions;
   }
 
-  public PermissionTemplateDto setGroupsByPermission(List<PermissionTemplateGroupDto> groupsPermissions) {
+  public PermissionTemplateDto setGroupsByPermission(@Nullable List<PermissionTemplateGroupDto> groupsPermissions) {
     this.groupsPermissions = groupsPermissions;
     return this;
   }
diff --git a/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java b/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java
index ab69bc5eaa5..d3442d46283 100644
--- a/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/resource/DefaultResourcePermissionsTest.java
@@ -20,7 +20,9 @@
 package org.sonar.core.resource;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.api.config.Settings;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
@@ -38,6 +40,9 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase {
   private Settings settings;
   private DefaultResourcePermissions permissions;
 
+  @Rule
+  public ExpectedException throwable = ExpectedException.none();
+
   @Before
   public void initResourcePermissions() {
     settings = new Settings();
@@ -159,4 +164,11 @@ public class DefaultResourcePermissionsTest extends AbstractDaoTestCase {
     // does not exist
     assertThat(permissions.hasRoles(new Project("not_found"))).isFalse();
   }
+
+  @Test
+  public void should_fail_when_no_default_template_is_defined() throws Exception {
+    throwable.expect(IllegalStateException.class);
+
+    permissions.grantDefaultRoles(project);
+  }
 }
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 2b392f97b07..20cd6888bcb 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
@@ -52,14 +52,18 @@ public class PermissionTemplate {
     }
     PermissionTemplate permissionTemplate =
       new PermissionTemplate(permissionTemplateDto.getId(), permissionTemplateDto.getName(), permissionTemplateDto.getDescription());
-    if(permissionTemplateDto.getUsersPermissions() != null) {
-      for (PermissionTemplateUserDto userPermission : permissionTemplateDto.getUsersPermissions()) {
+
+    List<PermissionTemplateUserDto> usersPermissions = permissionTemplateDto.getUsersPermissions();
+    if(usersPermissions != null) {
+      for (PermissionTemplateUserDto userPermission : usersPermissions) {
         permissionTemplate.registerUserPermission(permissionTemplateDto.getId(), userPermission.getUserId(),
           userPermission.getUserName(), userPermission.getUserLogin(), userPermission.getPermission());
       }
     }
-    if(permissionTemplateDto.getGroupsPermissions() != null) {
-      for (PermissionTemplateGroupDto groupPermission : permissionTemplateDto.getGroupsPermissions()) {
+
+    List<PermissionTemplateGroupDto> groupsPermissions = permissionTemplateDto.getGroupsPermissions();
+    if(groupsPermissions != null) {
+      for (PermissionTemplateGroupDto groupPermission : groupsPermissions) {
         permissionTemplate.registerGroupPermission(groupPermission.getPermission(), permissionTemplateDto.getId(),
           groupPermission.getGroupId(), groupPermission.getGroupName());
       }