SONAR-3618 improve support of the Views plugin

6 files changed, 64 insertions, 102 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/security/DefaultResourcePermissioning.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/security/DefaultResourcePermissioning.java
index 123869b49d0..8de64d8047c 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/security/DefaultResourcePermissioning.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/security/DefaultResourcePermissioning.java
@@ -19,13 +19,9 @@
  */
 package org.sonar.plugins.core.security;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.apache.ibatis.session.SqlSession;
 import org.sonar.api.BatchExtension;
-import org.sonar.api.Properties;
-import org.sonar.api.Property;
 import org.sonar.api.config.Settings;
-import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.security.DefaultGroups;
 import org.sonar.api.security.ResourcePermissioning;
@@ -36,23 +32,6 @@ import org.sonar.core.user.*;
 /**
  * @since 3.2
  */
-@Properties({
-  @Property(key = "sonar.role." + UserRole.ADMIN + ".TRK.defaultGroups",
-    name = "Default groups for project administrators",
-    defaultValue = DefaultGroups.ADMINISTRATORS,
-    global = false,
-    project = false),
-  @Property(key = "sonar.role." + UserRole.USER + ".TRK.defaultGroups",
-    name = "Default groups for project users",
-    defaultValue = DefaultGroups.USERS + "," + DefaultGroups.ANYONE,
-    global = false,
-    project = false),
-  @Property(key = "sonar.role." + UserRole.CODEVIEWER + ".TRK.defaultGroups",
-    name = "Default groups for project code viewers",
-    defaultValue = DefaultGroups.USERS + "," + DefaultGroups.ANYONE,
-    global = false,
-    project = false)
-})
 public class DefaultResourcePermissioning implements ResourcePermissioning, BatchExtension {
 
   private final Settings settings;
@@ -146,8 +125,7 @@ public class DefaultResourcePermissioning implements ResourcePermissioning, Batc
     UserMapper userMapper = session.getMapper(UserMapper.class);
     RoleMapper roleMapper = session.getMapper(RoleMapper.class);
 
-    String strategy = getStrategy(resource);
-    String[] groupNames = settings.getStringArrayBySeparator("sonar.role." + role + "." + strategy + ".defaultGroups", ",");
+    String[] groupNames = settings.getStringArrayBySeparator("sonar.role." + role + "." + resource.getQualifier() + ".defaultGroups", ",");
     for (String groupName : groupNames) {
       GroupRoleDto groupRole = new GroupRoleDto().setRole(role).setResourceId(new Long(resource.getId()));
       if (DefaultGroups.isAnyone(groupName)) {
@@ -160,7 +138,7 @@ public class DefaultResourcePermissioning implements ResourcePermissioning, Batc
       }
     }
 
-    String[] logins = settings.getStringArrayBySeparator("sonar.role." + role + "." + strategy + ".defaultUsers", ",");
+    String[] logins = settings.getStringArrayBySeparator("sonar.role." + role + "." + resource.getQualifier() + ".defaultUsers", ",");
     for (String login : logins) {
       UserDto user = userMapper.selectUserByLogin(login);
       if (user != null) {
@@ -168,28 +146,4 @@ public class DefaultResourcePermissioning implements ResourcePermissioning, Batc
       }
     }
   }
-
-  /**
-   * This is workaround to support old versions of the Views plugin.
-   * If the Views plugin does not define default permissions, then the standard permissions are re-used for new views.
-   */
-  @VisibleForTesting
-  String getStrategy(Resource resource) {
-    String qualifier = resource.getQualifier();
-    String result;
-    if (Qualifiers.PROJECT.equals(qualifier)) {
-      result = qualifier;
-
-    } else if (hasRoleSettings(UserRole.ADMIN, qualifier) || hasRoleSettings(UserRole.USER, qualifier) || hasRoleSettings(UserRole.CODEVIEWER, qualifier)) {
-      result = qualifier;
-    } else {
-      result = Qualifiers.PROJECT;
-    }
-    return result;
-  }
-
-  private boolean hasRoleSettings(String role, String qualifier) {
-    return settings.getString("sonar.role." + role + "." + qualifier + ".defaultGroups") != null
-      || settings.getString("sonar.role." + role + "." + qualifier + ".defaultUsers") != null;
-  }
 }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/security/DefaultResourcePermissioningTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/security/DefaultResourcePermissioningTest.java
index a67277fc376..50afeaaf526 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/security/DefaultResourcePermissioningTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/security/DefaultResourcePermissioningTest.java
@@ -20,19 +20,13 @@
 package org.sonar.plugins.core.security;
 
 import org.junit.Test;
-import org.sonar.api.Properties;
-import org.sonar.api.Property;
-import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.config.Settings;
 import org.sonar.api.resources.Project;
-import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.security.DefaultGroups;
 import org.sonar.core.persistence.AbstractDaoTestCase;
 
 import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 public class DefaultResourcePermissioningTest extends AbstractDaoTestCase {
 
@@ -83,8 +77,15 @@ public class DefaultResourcePermissioningTest extends AbstractDaoTestCase {
   public void grantDefaultRoles() {
     setupData("grantDefaultRoles");
 
-    Settings settings = new Settings(new PropertyDefinitions(DefaultResourcePermissioning.class));
+    Settings settings = new Settings();
+    settings.setProperty("sonar.role.admin.TRK.defaultGroups", "sonar-administrators");
+    settings.setProperty("sonar.role.admin.TRK.defaultUsers", "");
+    settings.setProperty("sonar.role.user.TRK.defaultGroups", "Anyone,sonar-users");
+    settings.setProperty("sonar.role.user.TRK.defaultUsers", "");
+    settings.setProperty("sonar.role.codeviewer.TRK.defaultGroups", "Anyone,sonar-users");
+    settings.setProperty("sonar.role.codeviewer.TRK.defaultUsers", "");
     DefaultResourcePermissioning permissioning = new DefaultResourcePermissioning(settings, getMyBatis());
+
     permissioning.grantDefaultRoles(project);
 
     checkTables("grantDefaultRoles", "user_roles", "group_roles");
@@ -134,43 +135,4 @@ public class DefaultResourcePermissioningTest extends AbstractDaoTestCase {
     // does not exist
     assertThat(permissioning.hasRoles(new Project("not_found"))).isFalse();
   }
-
-  @Test
-  public void use_default_project_roles_when_old_version_of_views_plugin() {
-    DefaultResourcePermissioning permissioning = new DefaultResourcePermissioning(new Settings(), getMyBatis());
-    Resource view = mock(Resource.class);
-    when(view.getQualifier()).thenReturn(Qualifiers.VIEW);
-
-    assertThat(permissioning.getStrategy(view)).isEqualTo(Qualifiers.PROJECT);
-  }
-
-  @Test
-  public void use_existing_view_roles() {
-    Settings settings = new Settings();
-    settings.setProperty("sonar.role.admin.VW.defaultUsers", "sonar-administrators");
-
-    DefaultResourcePermissioning permissioning = new DefaultResourcePermissioning(settings, getMyBatis());
-    Resource view = mock(Resource.class);
-    when(view.getQualifier()).thenReturn(Qualifiers.VIEW);
-
-    assertThat(permissioning.getStrategy(view)).isEqualTo(Qualifiers.VIEW);
-  }
-
-  @Test
-  public void use_existing_default_view_roles() {
-    Settings settings = new Settings(new PropertyDefinitions(RecentViewPlugin.class));
-
-    DefaultResourcePermissioning permissioning = new DefaultResourcePermissioning(settings, getMyBatis());
-    Resource view = mock(Resource.class);
-    when(view.getQualifier()).thenReturn(Qualifiers.VIEW);
-
-    assertThat(permissioning.getStrategy(view)).isEqualTo(Qualifiers.VIEW);
-  }
-
-  @Properties({
-    @Property(key = "sonar.role.user.VW.defaultUsers", defaultValue = "sonar-users", name = "")
-  })
-  static class RecentViewPlugin {
-
-  }
 }
\ No newline at end of file
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/security/DefaultResourcePermissioningTest/grantDefaultRoles-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/security/DefaultResourcePermissioningTest/grantDefaultRoles-result.xml
index 23b0c67e69c..f4154d15024 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/security/DefaultResourcePermissioningTest/grantDefaultRoles-result.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/security/DefaultResourcePermissioningTest/grantDefaultRoles-result.xml
@@ -12,9 +12,9 @@
   new rows : sonar-administrators (admin), sonar-users (user & codeviewer), Anyone (user & codeviewer),
    -->
   <group_roles id="3" group_id="100" resource_id="123" role="admin"/>
-  <group_roles id="4" group_id="101" resource_id="123" role="user"/>
-  <group_roles id="5" group_id="[null]" resource_id="123" role="user"/>
-  <group_roles id="6" group_id="101" resource_id="123" role="codeviewer"/>
-  <group_roles id="7" group_id="[null]" resource_id="123" role="codeviewer"/>
+  <group_roles id="4" group_id="[null]" resource_id="123" role="user"/>
+  <group_roles id="5" group_id="101" resource_id="123" role="user"/>
+  <group_roles id="6" group_id="[null]" resource_id="123" role="codeviewer"/>
+  <group_roles id="7" group_id="101" resource_id="123" role="codeviewer"/>
 
 </dataset>
\ No newline at end of file
diff --git a/sonar-core/src/main/resources/org/sonar/core/persistence/rows-h2.sql b/sonar-core/src/main/resources/org/sonar/core/persistence/rows-h2.sql
index 913e1ff6d9c..768c4f4dfe6 100644
--- a/sonar-core/src/main/resources/org/sonar/core/persistence/rows-h2.sql
+++ b/sonar-core/src/main/resources/org/sonar/core/persistence/rows-h2.sql
@@ -10,6 +10,28 @@ ALTER TABLE GROUP_ROLES ALTER COLUMN ID RESTART WITH 2;
 INSERT INTO GROUPS_USERS(USER_ID, GROUP_ID) VALUES (1, 1);
 INSERT INTO GROUPS_USERS(USER_ID, GROUP_ID) VALUES (1, 2);
 
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (1, 'sonar.role.admin.TRK.defaultGroups', 'sonar-administrators');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (2, 'sonar.role.admin.TRK.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (3, 'sonar.role.user.TRK.defaultGroups', 'Anyone,sonar-users');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (4, 'sonar.role.user.TRK.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (5, 'sonar.role.codeviewer.TRK.defaultGroups', 'Anyone,sonar-users');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (6, 'sonar.role.codeviewer.TRK.defaultUsers', '');
+
+-- COMPATIBILITY WITH OLD VERSIONS OF VIEWS PLUGIN -> see migration 320
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (7, 'sonar.role.admin.VW.defaultGroups', 'sonar-administrators');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (8, 'sonar.role.admin.VW.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (9, 'sonar.role.user.VW.defaultGroups', 'Anyone,sonar-users');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (10, 'sonar.role.user.VW.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (11, 'sonar.role.codeviewer.VW.defaultGroups', 'Anyone,sonar-users');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (12, 'sonar.role.codeviewer.VW.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (13, 'sonar.role.admin.SVW.defaultGroups', 'sonar-administrators');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (14, 'sonar.role.admin.SVW.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (15, 'sonar.role.user.SVW.defaultGroups', 'Anyone,sonar-users');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (16, 'sonar.role.user.SVW.defaultUsers', '');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (17, 'sonar.role.codeviewer.SVW.defaultGroups', 'Anyone,sonar-users');
+INSERT INTO PROPERTIES(ID, PROP_KEY, TEXT_VALUE) VALUES (18, 'sonar.role.codeviewer.SVW.defaultUsers', '');
+ALTER TABLE PROPERTIES ALTER COLUMN ID RESTART WITH 19;
+
 INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('1');
 INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('2');
 INSERT INTO SCHEMA_MIGRATIONS(VERSION) VALUES ('10');
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/roles_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/roles_helper.rb
index 95bba72992a..0516fc3a801 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/roles_helper.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/roles_helper.rb
@@ -47,13 +47,20 @@ module RolesHelper
   end
 
   def default_project_group_names(role, qualifier)
-    property_value=(controller.java_facade.getConfigurationValue("sonar.role.#{role}.#{qualifier}.defaultGroups")||'')
-    Api::Utils.insensitive_sort(property_value.split(','))
+    group_names=(controller.java_facade.getConfigurationValue("sonar.role.#{role}.#{qualifier}.defaultGroups")||'').split(',')
+
+    # verify that groups still exist
+    result = []
+    if group_names.size>0
+      groups = Group.find(:all, :conditions => ['name in (?)', group_names])
+      result = Api::Utils.insensitive_sort(groups.map{|g| g.name})
+      result = ['Anyone'].concat(result) if group_names.include? 'Anyone'
+    end
+    result
   end
 
   def default_project_users(role, qualifier)
-    property_value=(controller.java_facade.getConfigurationValue("sonar.role.#{role}.#{qualifier}.defaultUsers") || '')
-    logins=property_value.split(',')
+    logins=(controller.java_facade.getConfigurationValue("sonar.role.#{role}.#{qualifier}.defaultUsers") || '').split(',')
     users = User.find(:all, :conditions => ['login in (?) and active=?', logins, true])
     Api::Utils.insensitive_sort(users) { |user| user.name }
   end
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/320_move_default_roles.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/320_move_default_roles.rb
index df56495a5f4..75ee2d51658 100644
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/320_move_default_roles.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/320_move_default_roles.rb
@@ -49,11 +49,28 @@ class MoveDefaultRoles < ActiveRecord::Migration
       # upgrade from version < 3.2.
       move_groups
       move_users
+    else
+      create_default_groups('admin', 'TRK', 'sonar-administrators')
+      create_default_groups('user', 'TRK', 'Anyone,sonar-users')
+      create_default_groups('codeviewer', 'TRK', 'Anyone,sonar-users')
+
+      # Support old versions of Views plugin
+      create_default_groups('admin', 'VW', 'sonar-administrators')
+      create_default_groups('user', 'VW', 'Anyone,sonar-users')
+      create_default_groups('codeviewer', 'VW', 'Anyone,sonar-users')
+      create_default_groups('admin', 'SVW', 'sonar-administrators')
+      create_default_groups('user', 'SVW', 'Anyone,sonar-users')
+      create_default_groups('codeviewer', 'SVW', 'Anyone,sonar-users')
     end
   end
 
   private
 
+  def self.create_default_groups(role, qualifier, groups)
+    Property.create(:prop_key => "sonar.role.#{role}.#{qualifier}.defaultGroups", :text_value => groups)
+    Property.create(:prop_key => "sonar.role.#{role}.#{qualifier}.defaultUsers", :text_value => '')
+  end
+
   def self.move_groups
     groups_per_role={}
     group_roles = GroupRole.find(:all, :conditions => ['resource_id is null and role like ?', 'default-%'])