From af4d2130fb1dc7312b52a8045039bb8a496c8e34 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 16 Dec 2013 11:40:20 +0100 Subject: [PATCH] SONAR-4535 Set default profile now use MyBatis --- .../qualityprofile/db/QualityProfileDao.java | 9 +++ .../db/QualityProfileMapper.java | 3 + .../db/QualityProfileMapper.xml | 8 +++ .../db/QualityProfileDaoTest.java | 17 +++++- .../qualityprofile/QProfileOperations.java | 57 +++++++++++++++---- .../server/qualityprofile/QProfiles.java | 15 +++-- .../app/controllers/api/api_controller.rb | 1 + .../controllers/api/profiles_controller.rb | 7 +-- .../app/controllers/application_controller.rb | 6 +- .../app/controllers/profiles_controller.rb | 17 +++--- .../main/webapp/WEB-INF/app/models/profile.rb | 5 -- .../app/views/profiles/_rename_form.html.erb | 3 +- .../QProfileOperationsTest.java | 39 ++++++++++--- .../server/qualityprofile/QProfilesTest.java | 17 ++++-- 14 files changed, 152 insertions(+), 52 deletions(-) diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java index a364ae1fcc7..5f8be747d25 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java @@ -43,6 +43,15 @@ public class QualityProfileDao implements ServerComponent { } } + public QualityProfileDto selectById(Integer id) { + SqlSession session = mybatis.openSession(); + try { + return session.getMapper(QualityProfileMapper.class).selectById(id); + } finally { + MyBatis.closeQuietly(session); + } + } + public QualityProfileDto selectByNameAndLanguage(String name, String language) { SqlSession session = mybatis.openSession(); try { diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileMapper.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileMapper.java index dcfe0b93337..743841117b4 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileMapper.java @@ -33,6 +33,9 @@ public interface QualityProfileMapper { @CheckForNull QualityProfileDto selectByNameAndLanguage(@Param("name") String name, @Param("language") String language); + @CheckForNull + QualityProfileDto selectById(@Param("id") Integer id); + void insert(QualityProfileDto dto); void update(QualityProfileDto dto); diff --git a/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/QualityProfileMapper.xml b/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/QualityProfileMapper.xml index cd44060bef0..7fce1aff56b 100644 --- a/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/QualityProfileMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/QualityProfileMapper.xml @@ -26,6 +26,14 @@ + + INSERT INTO rules_profiles (name, language, parent_name, version, used_profile) VALUES (#{name}, #{language}, #{parent}, #{version}, #{used}) diff --git a/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/QualityProfileDaoTest.java b/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/QualityProfileDaoTest.java index 69761aaef1d..d56d3da0e5d 100644 --- a/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/QualityProfileDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/QualityProfileDaoTest.java @@ -74,7 +74,22 @@ public class QualityProfileDaoTest extends AbstractDaoTestCase { assertThat(dto.getVersion()).isEqualTo(1); assertThat(dto.isUsed()).isFalse(); - assertThat(dao.selectByNameAndLanguage("Sonar way", "unknown")); + assertThat(dao.selectByNameAndLanguage("Sonar way", "unknown")).isNull(); + } + + @Test + public void select_by_id() { + setupData("shared"); + + QualityProfileDto dto = dao.selectById(1); + assertThat(dto.getId()).isEqualTo(1); + assertThat(dto.getName()).isEqualTo("Sonar way"); + assertThat(dto.getLanguage()).isEqualTo("java"); + assertThat(dto.getParent()).isNull(); + assertThat(dto.getVersion()).isEqualTo(1); + assertThat(dto.isUsed()).isFalse(); + + assertThat(dao.selectById(555)).isNull(); } @Test diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java index b6ed6b8c7a4..e66f93bcd56 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java @@ -38,9 +38,12 @@ import org.sonar.core.preview.PreviewCache; import org.sonar.core.qualityprofile.db.*; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.platform.PersistentSettings; import org.sonar.server.user.UserSession; import org.sonar.server.util.Validation; +import javax.annotation.CheckForNull; + import java.io.StringReader; import java.util.List; import java.util.Map; @@ -55,19 +58,21 @@ public class QProfileOperations implements ServerComponent { private final List exporters; private final List importers; private final PreviewCache dryRunCache; + private final PersistentSettings persistentSettings; - public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, PreviewCache dryRunCache) { - this(myBatis, dao, activeRuleDao, Lists.newArrayList(), Lists.newArrayList(), dryRunCache); + public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, PreviewCache dryRunCache, PersistentSettings persistentSettings) { + this(myBatis, dao, activeRuleDao, Lists.newArrayList(), Lists.newArrayList(), dryRunCache, persistentSettings); } public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, List exporters, List importers, - PreviewCache dryRunCache) { + PreviewCache dryRunCache, PersistentSettings persistentSettings) { this.myBatis = myBatis; this.dao = dao; this.activeRuleDao = activeRuleDao; this.exporters = exporters; this.importers = importers; this.dryRunCache = dryRunCache; + this.persistentSettings = persistentSettings; } public NewProfileResult newProfile(String name, String language, Map xmlProfilesByPlugin, UserSession userSession) { @@ -85,21 +90,26 @@ public class QProfileOperations implements ServerComponent { } result.setProfile(QProfile.from(dto)); sqlSession.commit(); + dryRunCache.reportGlobalModification(); } finally { MyBatis.closeQuietly(sqlSession); - dryRunCache.reportGlobalModification(); } return result; } - public void renameProfile(String name, String language, String newName, UserSession userSession) { - QualityProfileDto qualityProfile = validateRenameProfile(name, language, newName, userSession); + public void renameProfile(Integer profileId, String newName, UserSession userSession) { + QualityProfileDto qualityProfile = validateRenameProfile(profileId, newName, userSession); qualityProfile.setName(newName); dao.update(qualityProfile); } - private QualityProfileDto find(String name, String language) { - return dao.selectByNameAndLanguage(name, language); + public void updateDefaultProfile(Integer id, UserSession userSession) { + QualityProfileDto qualityProfile = validateUpdateDefaultProfile(id, userSession); + persistentSettings.saveProperty("sonar.profile." + qualityProfile.getLanguage(), qualityProfile.getName()); + } + + public void updateDefaultProfile(String name, String language, UserSession userSession) { + updateDefaultProfile(findNeverNull(name, language).getId(), userSession); } private List readProfilesFromXml(NewProfileResult result, Map xmlProfilesByPlugin) { @@ -175,13 +185,30 @@ public class QProfileOperations implements ServerComponent { } } - private QualityProfileDto validateRenameProfile(String name, String language, String newName, UserSession userSession) { + private QualityProfileDto validateRenameProfile(Integer profileId, String newName, UserSession userSession) { checkPermission(userSession); validateName(newName); - if (find(newName, language) != null) { + QualityProfileDto profileDto = findNeverNull(profileId); + if (!profileDto.getName().equals(newName) && find(newName, profileDto.getLanguage()) != null) { throw BadRequestException.ofL10n("quality_profiles.already_exists"); } + return profileDto; + } + + private QualityProfileDto validateUpdateDefaultProfile(Integer id, UserSession userSession) { + checkPermission(userSession); + return findNeverNull(id); + } + private QualityProfileDto findNeverNull(Integer id) { + QualityProfileDto qualityProfile = find(id); + if (qualityProfile == null) { + throw new NotFoundException("This quality profile does not exists."); + } + return qualityProfile; + } + + private QualityProfileDto findNeverNull(String name, String language) { QualityProfileDto qualityProfile = find(name, language); if (qualityProfile == null) { throw new NotFoundException("This quality profile does not exists."); @@ -189,6 +216,16 @@ public class QProfileOperations implements ServerComponent { return qualityProfile; } + @CheckForNull + private QualityProfileDto find(String name, String language) { + return dao.selectByNameAndLanguage(name, language); + } + + @CheckForNull + private QualityProfileDto find(Integer id) { + return dao.selectById(id); + } + private void validateName(String name) { if (Strings.isNullOrEmpty(name)) { throw BadRequestException.ofL10n("quality_profiles.please_type_profile_name"); diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java index 69dfc40c084..8cecacbb7ef 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java @@ -53,12 +53,19 @@ public class QProfiles implements ServerComponent { throw new UnsupportedOperationException(); } - public void renameProfile(String name, String language, String newName) { - operations.renameProfile(name, language, newName, UserSession.get()); + public void renameProfile(Integer id, String newName) { + operations.renameProfile(id, newName, UserSession.get()); } - public void updateDefaultProfile() { - throw new UnsupportedOperationException(); + public void updateDefaultProfile(Integer id) { + operations.updateDefaultProfile(id, UserSession.get()); + } + + /** + * Used by WS + */ + public void updateDefaultProfile(String name, String language) { + operations.updateDefaultProfile(name, language, UserSession.get()); } public void copyProfile() { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb index 4ce123abfd8..fb73adcb218 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb @@ -107,6 +107,7 @@ class Api::ApiController < ApplicationController render_error(message, 200) end + # Override def render_server_exception(exception) render_response(exception.httpCode, exception.getMessage) end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb index 8fdf52a9a29..4fe95efecab 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb @@ -75,12 +75,7 @@ class Api::ProfilesController < Api::ApiController # Since v.3.3 def set_as_default verify_post_request - access_denied unless has_role?(:profileadmin) - require_parameters :language, :name - - profile=Profile.find_by_name_and_language(params[:name], params[:language]) - not_found('Profile not found') unless profile - profile.set_as_default + Internal.qprofiles.updateDefaultProfile(params[:name], params[:language]) render_success end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb index 41c94f6e7d6..a38e8cc0098 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb @@ -181,7 +181,11 @@ class ApplicationController < ActionController::Base if exception.java_kind_of?(Java::OrgSonarServerExceptions::BadRequestException) && !exception.errors.empty? message += '
' + exception.errors.to_a.map{|error| error.text ? error.text : Api::Utils.message(error.l10nKey, :params => error.l10nParams)}.join('
') end - render :text => CGI.escapeHTML(message), :status => exception.httpCode + if request.xhr? + render :text => CGI.escapeHTML(message), :status => exception.httpCode + else + flash[:error] = message + end end def render_native_access_denied(exception) diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb index 4e3d0b2ce7c..46b86087d14 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb @@ -63,10 +63,7 @@ class ProfilesController < ApplicationController flash[:warning] = result.warnings.to_a[0...4].join('
') end rescue Java::OrgSonarServerExceptions::ServerException => error - flash[:error] = (error.getMessage ? error.getMessage : Api::Utils.message(error.l10nKey, :params => error.l10nParams.to_a)) - if error.java_kind_of?(Java::OrgSonarServerExceptions::BadRequestException) && !error.errors.empty? - flash[:error] += '
' + error.errors.to_a.map{|error| error.text ? error.text : Api::Utils.message(error.l10nKey, :params => error.l10nParams)}.join('
') - end + render_server_exception(error) end redirect_to :action => 'index' end @@ -88,11 +85,11 @@ class ProfilesController < ApplicationController # POST /profiles/set_as_default/ def set_as_default verify_post_request - access_denied unless has_role?(:profileadmin) - require_parameters 'id' - - profile = Profile.find(params[:id]) - profile.set_as_default + begin + Internal.qprofiles.updateDefaultProfile(params[:id].to_i) + rescue Java::OrgSonarServerExceptions::ServerException => error + render_server_exception(error) + end redirect_to :action => 'index' end @@ -336,7 +333,7 @@ class ProfilesController < ApplicationController verify_post_request verify_ajax_request - Internal.qprofiles.renameProfile(params[:name], params[:language], params[:new_name]) + Internal.qprofiles.renameProfile(params[:id].to_i, params[:new_name]) render :text => 'ok', :status => 200 end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb index 94183e6040c..202cc3d3a1f 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb @@ -61,11 +61,6 @@ class Profile < ActiveRecord::Base Property.value("sonar.profile.#{language}")==name end - def set_as_default - Property.set("sonar.profile.#{language}", name) - self - end - def active_by_rule_id(rule_id) active_hash_by_rule_id[rule_id] end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_rename_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_rename_form.html.erb index b9c935a26a9..cf0f9053583 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_rename_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_rename_form.html.erb @@ -1,6 +1,5 @@
- - +
diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java index b04fb400c52..a509e9be600 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java @@ -44,6 +44,7 @@ import org.sonar.core.qualityprofile.db.*; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.platform.PersistentSettings; import org.sonar.server.user.MockUserSession; import java.io.Reader; @@ -74,6 +75,9 @@ public class QProfileOperationsTest { @Mock PreviewCache dryRunCache; + @Mock + PersistentSettings persistentSettings; + List exporters = newArrayList(); List importers = newArrayList(); @@ -83,7 +87,7 @@ public class QProfileOperationsTest { @Before public void setUp() throws Exception { when(myBatis.openSession()).thenReturn(session); - operations = new QProfileOperations(myBatis, dao, activeRuleDao, exporters, importers, dryRunCache); + operations = new QProfileOperations(myBatis, dao, activeRuleDao, exporters, importers, dryRunCache, persistentSettings); } @Test @@ -199,9 +203,9 @@ public class QProfileOperationsTest { @Test public void rename_profile() throws Exception { - QualityProfileDto dto = new QualityProfileDto().setName("Default").setLanguage("java"); - when(dao.selectByNameAndLanguage(eq("Default"), anyString())).thenReturn(dto); - operations.renameProfile("Default", "java", "Default profile", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + QualityProfileDto dto = new QualityProfileDto().setId(1).setName("Default").setLanguage("java"); + when(dao.selectById(1)).thenReturn(dto); + operations.renameProfile(1, "Default profile", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); ArgumentCaptor profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class); verify(dao).update(profileArgument.capture()); @@ -213,7 +217,7 @@ public class QProfileOperationsTest { @Test public void fail_to_rename_profile_on_unknown_profile() throws Exception { try { - operations.renameProfile("Default", "java", "Default profile", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.renameProfile(1, "Default profile", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(NotFoundException.class); @@ -224,7 +228,7 @@ public class QProfileOperationsTest { @Test public void fail_to_rename_profile_when_missing_new_name() throws Exception { try { - operations.renameProfile("Default", "java", "", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.renameProfile(1, "", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(BadRequestException.class); @@ -235,13 +239,32 @@ public class QProfileOperationsTest { @Test public void fail_to_rename_profile_if_already_exists() throws Exception { try { - when(dao.selectByNameAndLanguage(eq("Default"), anyString())).thenReturn(new QualityProfileDto().setName("Default").setLanguage("java")); + when(dao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("Default").setLanguage("java")); when(dao.selectByNameAndLanguage(eq("New Default"), anyString())).thenReturn(new QualityProfileDto().setName("New Default").setLanguage("java")); - operations.renameProfile("Default", "java", "New Default", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.renameProfile(1, "New Default", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(BadRequestException.class); } verify(session, never()).commit(); } + + @Test + public void update_default_profile() throws Exception { + when(dao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); + + operations.updateDefaultProfile(1, MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + + verify(persistentSettings).saveProperty("sonar.profile.java", "My profile"); + } + + @Test + public void update_default_profile_from_name_and_language() throws Exception { + when(dao.selectByNameAndLanguage("My profile", "java")).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); + when(dao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); + + operations.updateDefaultProfile("My profile", "java", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + + verify(persistentSettings).saveProperty("sonar.profile.java", "My profile"); + } } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java index 690dce9a723..82e352eb7d8 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java @@ -74,13 +74,20 @@ public class QProfilesTest { @Test public void rename_profile() throws Exception { - qProfiles.renameProfile("Default", "java", "Default profile"); - verify(operations).renameProfile(eq("Default"), eq("java"), eq("Default profile"), any(UserSession.class)); + qProfiles.renameProfile(1, "Default profile"); + verify(operations).renameProfile(eq(1), eq("Default profile"), any(UserSession.class)); } - @Test(expected = UnsupportedOperationException.class) - public void testSetDefaultProfile() throws Exception { - qProfiles.updateDefaultProfile(); + @Test + public void update_default_profile() throws Exception { + qProfiles.updateDefaultProfile(1); + verify(operations).updateDefaultProfile(eq(1), any(UserSession.class)); + } + + @Test + public void update_default_profile_from_name_and_language() throws Exception { + qProfiles.updateDefaultProfile("Default", "java"); + verify(operations).updateDefaultProfile(eq("Default"), eq("java"), any(UserSession.class)); } @Test(expected = UnsupportedOperationException.class) -- 2.39.5