]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4535 Quality profile renaming is now done in Java
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 13 Dec 2013 15:47:30 +0000 (16:47 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 13 Dec 2013 15:47:30 +0000 (16:47 +0100)
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java
sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_rename_form.html.erb
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java

index ff262ffefc9f11230128bc1b32c3ad7242cbe3da..95a42a10dfb8dea663ac7f9427db20c179276714 100644 (file)
@@ -37,6 +37,7 @@ import org.sonar.core.persistence.MyBatis;
 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.user.UserSession;
 import org.sonar.server.util.Validation;
 
@@ -70,11 +71,10 @@ public class QProfileOperations implements ServerComponent {
   }
 
   public NewProfileResult newProfile(String name, String language, Map<String, String> xmlProfilesByPlugin, UserSession userSession) {
-    userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN);
-    validate(name, language);
+    validateNewProfile(name, language, userSession);
 
     NewProfileResult result = new NewProfileResult();
-    List<RulesProfile> importProfiles = readProfiles(result, xmlProfilesByPlugin);
+    List<RulesProfile> importProfiles = readProfilesFromXml(result, xmlProfilesByPlugin);
 
     SqlSession sqlSession = myBatis.openSession();
     try {
@@ -91,17 +91,17 @@ public class QProfileOperations implements ServerComponent {
     }
   }
 
-  public void validate(String name, String language){
-    if (Strings.isNullOrEmpty(name)) {
-      throw BadRequestException.ofL10n("quality_profiles.please_type_profile_name");
-    }
-    Validation.checkMandatoryParameter(language, "language");
-    if (dao.selectByNameAndLanguage(name, language) != null) {
-      throw BadRequestException.ofL10n("quality_profiles.already_exists");
-    }
+  public void renameProfile(String name, String language, String newName, UserSession userSession) {
+    QualityProfileDto qualityProfile = validateRenameProfile(name, language, newName, userSession);
+    qualityProfile.setName(newName);
+    dao.update(qualityProfile);
   }
 
-  private List<RulesProfile> readProfiles(NewProfileResult result, Map<String, String> xmlProfilesByPlugin) {
+  private QualityProfileDto find(String name, String language) {
+    return dao.selectByNameAndLanguage(name, language);
+  }
+
+  private List<RulesProfile> readProfilesFromXml(NewProfileResult result, Map<String, String> xmlProfilesByPlugin) {
     List<RulesProfile> profiles = newArrayList();
     ValidationMessages messages = ValidationMessages.create();
     for (Map.Entry<String, String> entry : xmlProfilesByPlugin.entrySet()) {
@@ -165,4 +165,37 @@ public class QProfileOperations implements ServerComponent {
       .setValue(activeRuleParam.getValue());
   }
 
+  private void validateNewProfile(String name, String language, UserSession userSession) {
+    checkPermission(userSession);
+    validateName(name);
+    Validation.checkMandatoryParameter(language, "language");
+    if (find(name, language) != null) {
+      throw BadRequestException.ofL10n("quality_profiles.already_exists");
+    }
+  }
+
+  private QualityProfileDto validateRenameProfile(String name, String language, String newName, UserSession userSession) {
+    checkPermission(userSession);
+    validateName(newName);
+    if (find(newName, language) != null) {
+      throw BadRequestException.ofL10n("quality_profiles.already_exists");
+    }
+
+    QualityProfileDto qualityProfile = find(name, language);
+    if (qualityProfile == null) {
+      throw new NotFoundException("This quality profile does not exists.");
+    }
+    return qualityProfile;
+  }
+
+  private void validateName(String name) {
+    if (Strings.isNullOrEmpty(name)) {
+      throw BadRequestException.ofL10n("quality_profiles.please_type_profile_name");
+    }
+  }
+
+  private void checkPermission(UserSession userSession) {
+    userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN);
+  }
+
 }
index de6a590e05807985693e5c946a18d8a92bd66165..a17f8a758d70739c54f4ade78f3e6823151f8bfe 100644 (file)
@@ -53,8 +53,8 @@ public class QProfiles implements ServerComponent {
     throw new UnsupportedOperationException();
   }
 
-  public void renameProfile() {
-    throw new UnsupportedOperationException();
+  public void renameProfile(String name, String language, String newName) {
+    operations.renameProfile(name, language, newName, UserSession.get());
   }
 
   public void setDefaultProfile() {
index e07c0b62e000526b0a204353730e8b6f855619bb..41c94f6e7d635fcc24ddfe12dae671b1fb2971eb 100644 (file)
@@ -178,6 +178,9 @@ class ApplicationController < ActionController::Base
 
   def render_server_exception(exception)
     message = (exception.getMessage ? exception.getMessage : Api::Utils.message(exception.l10nKey, :params => exception.l10nParams.to_a))
+    if exception.java_kind_of?(Java::OrgSonarServerExceptions::BadRequestException) && !exception.errors.empty?
+      message += '<br/>' + exception.errors.to_a.map{|error| error.text ? error.text : Api::Utils.message(error.l10nKey, :params => error.l10nParams)}.join('<br/>')
+    end
     render :text => CGI.escapeHTML(message), :status => exception.httpCode
   end
 
index 9d9de03820f3d515069f281b984ccf0de7241a38..149e7afbc1a0ae02dc027d4a091e0035cb304dd8 100644 (file)
@@ -64,21 +64,15 @@ class ProfilesController < ApplicationController
       end
 
       # TODO manage these exceptions at a higher level
-    rescue NativeException => exception
-      if exception.cause.java_kind_of? Java::OrgSonarServerExceptions::ServerException
-        error = exception.cause
-        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] += '<br/>' + error.errors.to_a.map{|error| error.text ? error.text : Api::Utils.message(error.l10nKey, :params => error.l10nParams)}.join('<br/>')
-        end
-      else
-        flash[:error] = 'Error when creating the quality profile : ' + exception.cause.getMessage
+    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] += '<br/>' + error.errors.to_a.map{|error| error.text ? error.text : Api::Utils.message(error.l10nKey, :params => error.l10nParams)}.join('<br/>')
       end
     end
     redirect_to :action => 'index'
   end
 
-
   # POST /profiles/delete/<id>
   def delete
     verify_post_request
@@ -343,16 +337,9 @@ class ProfilesController < ApplicationController
   def rename
     verify_post_request
     verify_ajax_request
-    access_denied unless has_role?(:profileadmin)
-    require_parameters 'id'
 
-    @profile = Profile.find(params[:id])
-
-    if @profile.rename(params[:name]).errors.empty?
-      render :text => 'ok', :status => 200
-    else
-      render :partial => 'profiles/rename_form', :status => 400
-    end
+    Internal.qprofiles.renameProfile(params[:name], params[:language], params[:new_name])
+    render :text => 'ok', :status => 200
   end
 
   # GET /profiles/compare?id1=<profile1 id>&id2=<profile2 id>
index 4d2a24c5ab3446e420729cba00b47a1e7060f3dd..b9c935a26a9cf00b8942ed0dc539b2b8e23745d5 100644 (file)
@@ -1,19 +1,18 @@
 <form id="rename-profile-form" method="post" action="profiles/rename">
-  <input type="hidden" name="id" value="<%= @profile.id -%>"/>
+  <input type="hidden" name="name" value="<%= @profile.name -%>"/>
+  <input type="hidden" name="language" value="<%=  @profile.language -%>"/>
   <fieldset>
 
     <div class="modal-head">
-      <h2>Rename Profile: <%= h @profile.name_was -%></h2>
+      <h2>Rename Profile: <%= h @profile.name -%></h2>
     </div>
 
     <div class="modal-body">
-      <% @profile.errors.each do |attr, msg| %>
-        <p class="error"><%= h msg -%></p>
-      <% end %>
+      <div class="modal-error"/>
 
       <div class="modal-field">
-        <label for="name">New name <em class="mandatory">*</em></label>
-        <input id="new-name" name="name" type="text" size="50" maxlength="100" autofocus="autofocus"/>
+        <label for="new_name">New name <em class="mandatory">*</em></label>
+        <input id="new-name" name="new_name" type="text" size="50" maxlength="100" autofocus="autofocus"/>
       </div>
     </div>
     <div class="modal-foot">
@@ -24,4 +23,4 @@
 </form>
 <script>
   $j("#rename-profile-form").modalForm();
-</script>
\ No newline at end of file
+</script>
index a66523f0fd261d36e51ecc37357b0d0cd813fb0f..b04fb400c52e903fd842c8e251ef4397801d669b 100644 (file)
@@ -43,6 +43,7 @@ import org.sonar.core.preview.PreviewCache;
 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.user.MockUserSession;
 
 import java.io.Reader;
@@ -86,12 +87,19 @@ public class QProfileOperationsTest {
   }
 
   @Test
-  public void new_profile() throws Exception {
+  public void create_profile() throws Exception {
     NewProfileResult result = operations.newProfile("Default", "java", Maps.<String, String>newHashMap(), MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
     assertThat(result.profile().name()).isEqualTo("Default");
     assertThat(result.profile().language()).isEqualTo("java");
 
     verify(dao).insert(any(QualityProfileDto.class), eq(session));
+
+    ArgumentCaptor<QualityProfileDto> profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class);
+    verify(dao).insert(profileArgument.capture(), eq(session));
+    assertThat(profileArgument.getValue().getName()).isEqualTo("Default");
+    assertThat(profileArgument.getValue().getLanguage()).isEqualTo("java");
+    assertThat(profileArgument.getValue().getVersion()).isEqualTo(1);
+    assertThat(profileArgument.getValue().isUsed()).isFalse();
   }
 
   @Test
@@ -130,7 +138,7 @@ public class QProfileOperationsTest {
   }
 
   @Test
-  public void new_profile_from_xml_plugin() throws Exception {
+  public void create_profile_from_xml_plugin() throws Exception {
     RulesProfile profile = RulesProfile.create("Default", "java");
     Rule rule = Rule.create("pmd", "rule1");
     rule.createParameter("paramKey");
@@ -188,4 +196,52 @@ public class QProfileOperationsTest {
     }
     verify(session, never()).commit();
   }
+
+  @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));
+
+    ArgumentCaptor<QualityProfileDto> profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class);
+    verify(dao).update(profileArgument.capture());
+
+    assertThat(profileArgument.getValue().getName()).isEqualTo("Default profile");
+    assertThat(profileArgument.getValue().getLanguage()).isEqualTo("java");
+  }
+
+  @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));
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(NotFoundException.class);
+    }
+    verify(dao, never()).update(any(QualityProfileDto.class));
+  }
+
+  @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));
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(BadRequestException.class);
+    }
+    verify(dao, never()).update(any(QualityProfileDto.class));
+  }
+
+  @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.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));
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(BadRequestException.class);
+    }
+    verify(session, never()).commit();
+  }
 }
index b021c68f6daab2b3518a6179621eef834f573f67..5294b7ca6420d46b64a95fb048640362594d832b 100644 (file)
@@ -73,9 +73,10 @@ public class QProfilesTest {
     qProfiles.deleteProfile();
   }
 
-  @Test(expected = UnsupportedOperationException.class)
-  public void testRenameProfile() throws Exception {
-    qProfiles.renameProfile();
+  @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));
   }
 
   @Test(expected = UnsupportedOperationException.class)