From 5db787d457fdf31c73322d9dad0b8dc726ddb2d1 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 16 Aug 2013 10:54:22 +0200 Subject: [PATCH] SONAR-4475 Revert commit 3554f0dddf that makes tests fail --- .../resources/org/sonar/l10n/core.properties | 1 - .../java/org/sonar/core/user/RoleDao.java | 10 ------- .../java/org/sonar/core/user/RoleMapper.java | 4 +-- .../org/sonar/core/user/RoleMapper.xml | 24 ----------------- .../java/org/sonar/core/user/RoleDaoTest.java | 15 ----------- .../should_count_user_with_permission.xml | 25 ------------------ .../org/sonar/api/utils/MessageException.java | 26 ------------------- .../sonar/api/utils/MessageExceptionTest.java | 14 ---------- .../permission/InternalPermissionService.java | 7 ----- .../app/controllers/groups_controller.rb | 4 +-- .../app/controllers/users_controller.rb | 17 ++++++------ .../main/webapp/javascripts/application.js | 2 +- .../InternalPermissionServiceTest.java | 19 -------------- 13 files changed, 14 insertions(+), 154 deletions(-) delete mode 100644 sonar-core/src/test/resources/org/sonar/core/user/RoleDaoTest/should_count_user_with_permission.xml diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties index 01df59dce07..eb291c817c1 100644 --- a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -2233,7 +2233,6 @@ global_permissions.scan.desc=Ability to execute analyses, and to get all setting global_permissions.dryRunScan=Execute Local Analysis (Dry Run) global_permissions.dryRunScan.desc=Ability to execute local (dry run) analyses without pushing the results to the server, and to get all settings required to perform a local analysis. This permission does not include the ability to access secured settings such as the scm account password, the jira account password, and so on.
\ This permission is required to execute a local analysis in Eclipse or via the Issues Report plugin. -global_permissions.error.need_at_lest_one_admin=At least one admin must be defined. #------------------------------------------------------------------------------ # diff --git a/sonar-core/src/main/java/org/sonar/core/user/RoleDao.java b/sonar-core/src/main/java/org/sonar/core/user/RoleDao.java index b936f0e3308..c8bc741db1b 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/RoleDao.java +++ b/sonar-core/src/main/java/org/sonar/core/user/RoleDao.java @@ -144,14 +144,4 @@ public class RoleDao implements TaskComponent, ServerComponent { MyBatis.closeQuietly(session); } } - - public int countUserWithPermission(String permission) { - SqlSession session = mybatis.openSession(); - try { - RoleMapper mapper = session.getMapper(RoleMapper.class); - return mapper.countUserWithPermission(permission); - } finally { - MyBatis.closeQuietly(session); - } - } } diff --git a/sonar-core/src/main/java/org/sonar/core/user/RoleMapper.java b/sonar-core/src/main/java/org/sonar/core/user/RoleMapper.java index 7bd9fb0315e..0fcaac7973b 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/RoleMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/user/RoleMapper.java @@ -21,6 +21,7 @@ package org.sonar.core.user; import org.apache.ibatis.annotations.Param; +import javax.annotation.Nullable; import java.util.List; /** @@ -48,6 +49,5 @@ public interface RoleMapper { int countUserRoles(Long resourceId); - int countUserWithPermission(@Param("permission") String permission); - + List countSystemAdministrators(@Nullable @Param("groupName") String groupName); } diff --git a/sonar-core/src/main/resources/org/sonar/core/user/RoleMapper.xml b/sonar-core/src/main/resources/org/sonar/core/user/RoleMapper.xml index be4b7671b5d..3dba348d55a 100644 --- a/sonar-core/src/main/resources/org/sonar/core/user/RoleMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/user/RoleMapper.xml @@ -88,28 +88,4 @@ SELECT count(id) FROM group_roles WHERE resource_id=#{id} - - - diff --git a/sonar-core/src/test/java/org/sonar/core/user/RoleDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/RoleDaoTest.java index 427c2aee456..6d0c9b0830e 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/RoleDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/RoleDaoTest.java @@ -76,19 +76,4 @@ public class RoleDaoTest extends AbstractDaoTestCase { checkTable("groupPermissions", "group_roles", "group_id", "role"); } - @Test - public void should_count_user_with_permission() { - setupData("should_count_user_with_permission"); - - RoleDao dao = new RoleDao(getMyBatis()); - - // 1 user have role 'admin', 1 user owns to the 'admin' group - assertThat(dao.countUserWithPermission("admin")).isEqualTo(2); - // 1 user have role 'profileadmin' - assertThat(dao.countUserWithPermission("profileadmin")).isEqualTo(1); - // 1 user owns to the 'user' group - assertThat(dao.countUserWithPermission("user")).isEqualTo(1); - assertThat(dao.countUserWithPermission("unknown")).isEqualTo(0); - } - } diff --git a/sonar-core/src/test/resources/org/sonar/core/user/RoleDaoTest/should_count_user_with_permission.xml b/sonar-core/src/test/resources/org/sonar/core/user/RoleDaoTest/should_count_user_with_permission.xml deleted file mode 100644 index 8fe58e3c7d3..00000000000 --- a/sonar-core/src/test/resources/org/sonar/core/user/RoleDaoTest/should_count_user_with_permission.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/MessageException.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/MessageException.java index 7a703557b5a..25290dac9b2 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/MessageException.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/MessageException.java @@ -19,14 +19,6 @@ */ package org.sonar.api.utils; -import javax.annotation.CheckForNull; -import javax.annotation.Nullable; - -import java.util.Collection; -import java.util.Collections; - -import static com.google.common.collect.Lists.newArrayList; - /** * Runtime exception for "functional" errors. It aims to be displayed to end-users, without any technical information * like stack traces. @@ -35,19 +27,10 @@ import static com.google.common.collect.Lists.newArrayList; */ public class MessageException extends RuntimeException { - private String l10nKey; - private Collection l10nParams; - public MessageException(String s) { super(s); } - public MessageException(@Nullable String message, @Nullable String l10nKey, @Nullable Object[] l10nParams) { - super(message); - this.l10nKey = l10nKey; - this.l10nParams = l10nParams == null ? Collections.emptyList() : Collections.unmodifiableCollection(newArrayList(l10nParams)); - } - /** * Does not fill in the stack trace * @@ -63,14 +46,5 @@ public class MessageException extends RuntimeException { return getMessage(); } - @CheckForNull - public String l10nKey() { - return l10nKey; - } - - @CheckForNull - public Collection l10nParams() { - return l10nParams; - } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/MessageExceptionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/MessageExceptionTest.java index f036069412f..0b734416e94 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/utils/MessageExceptionTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/MessageExceptionTest.java @@ -47,18 +47,4 @@ public class MessageExceptionTest { assertThat(writer.toString()).isEqualTo(message + System.getProperty("line.separator")); } } - - @Test - public void should_create_exception_with_status_and_l10n_message_with_param(){ - MessageException exception = new MessageException(null, "key", new String[]{"value"}); - assertThat(exception.l10nKey()).isEqualTo("key"); - assertThat(exception.l10nParams()).containsOnly("value"); - } - - @Test - public void should_create_exception_with_status_and_l10n_message_without_param(){ - MessageException exception = new MessageException(null, "key", null); - assertThat(exception.l10nKey()).isEqualTo("key"); - assertThat(exception.l10nParams()).isEmpty(); - } } diff --git a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java index c6e22f94435..3fdcc998ab8 100644 --- a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java +++ b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java @@ -24,7 +24,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.ServerComponent; import org.sonar.api.security.DefaultGroups; -import org.sonar.api.utils.MessageException; import org.sonar.core.permission.ComponentPermissionFacade; import org.sonar.core.permission.Permission; import org.sonar.core.user.*; @@ -72,12 +71,6 @@ public class InternalPermissionService implements ServerComponent { } } - public void checkAtLeatOneSysAdminExists(){ - if (roleDao.countUserWithPermission(Permission.SYSTEM_ADMIN.key()) == 0){ - throw new MessageException(null, "global_permissions.error.need_at_lest_one_admin", null); - } - } - private void applyPermissionTemplate(String templateKey, String componentId) { permissionFacade.applyPermissionTemplate(templateKey, Long.parseLong(componentId)); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb index 1105fa5292b..f2d5fd5d878 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb @@ -23,7 +23,7 @@ class GroupsController < ApplicationController before_filter :admin_required def index - @groups = Group.all(:order => 'name') + @groups = Group.find(:all, :order => 'name') if params[:id] @group = Group.find(params[:id]) else @@ -72,7 +72,7 @@ class GroupsController < ApplicationController end def to_index(errors, id) - unless errors.empty? + if !errors.empty? flash[:error] = errors.full_messages.join("
\n") end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb index 98bcbad7e30..97807f9a749 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb @@ -130,16 +130,17 @@ class UsersController < ApplicationController end def destroy - begin - user = User.find(params[:id]) - Api.users.deactivate(user.login) + @user = User.find(params[:id]) + + if current_user.id==@user.id + flash[:error] = 'Please log in with another user in order to delete yourself.' + + else + Api.users.deactivate(@user.login) flash[:notice] = 'User is deleted.' - rescue Java::OrgSonarServerExceptions::HttpException => exception - error = exception.cause - flash[:error] = (error.getMessage ? error.getMessage : Api::Utils.message(error.l10nKey, :params => error.l10nParams.to_a)) end - redirect_to(:action => 'index', :id => nil) + to_index(@user.errors, nil) end def select_group @@ -157,7 +158,7 @@ class UsersController < ApplicationController end def to_index(errors, id) - unless errors.empty? + if !errors.empty? flash[:error] = errors.full_messages.join("
\n") end diff --git a/sonar-server/src/main/webapp/javascripts/application.js b/sonar-server/src/main/webapp/javascripts/application.js index 501fd589a24..773170b1a38 100644 --- a/sonar-server/src/main/webapp/javascripts/application.js +++ b/sonar-server/src/main/webapp/javascripts/application.js @@ -320,8 +320,8 @@ function openModalWindow(url, options) { $j('input[type=submit]', obj).removeAttr('disabled'); errorElt.show(); errorElt.html(xhr.responseText); - } else { // otherwise replace modal window by the returned text + } else { $j("#modal").html(xhr.responseText); } } diff --git a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java index 04d8daceb6a..6dd2f92fca6 100644 --- a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java @@ -30,7 +30,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.security.DefaultGroups; -import org.sonar.api.utils.MessageException; import org.sonar.core.permission.ComponentPermissionFacade; import org.sonar.core.permission.Permission; import org.sonar.core.user.*; @@ -41,8 +40,6 @@ import org.sonar.server.user.MockUserSession; import java.util.Map; -import static org.fest.assertions.Assertions.assertThat; -import static org.fest.assertions.Fail.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.*; @@ -194,22 +191,6 @@ public class InternalPermissionServiceTest { verify(permissionFacade).applyPermissionTemplate("my_template_key", 3L); } - @Test - public void should_check_at_least_one_sys_admin_exists() throws Exception { - // One admin exists -> all is ok - when(roleDao.countUserWithPermission(anyString())).thenReturn(1); - service.checkAtLeatOneSysAdminExists(); - - // No admin -> fail - try { - when(roleDao.countUserWithPermission(anyString())).thenReturn(0); - service.checkAtLeatOneSysAdminExists(); - fail(); - } catch (Exception e){ - assertThat(e).isInstanceOf(MessageException.class); - } - } - protected static class MatchesUserRole extends BaseMatcher { private final UserRoleDto referenceDto; -- 2.39.5