]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4535 update and delete active rule note now uses Java facade
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 20 Dec 2013 15:16:14 +0000 (16:16 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 20 Dec 2013 15:16:14 +0000 (16:16 +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/new_rules_configuration_controller.rb
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java

index 78a2d1efaa189144f0a0367a86ca15b4f1432c6f..45006c20a37094b2d0fda95015eb7cd31c51a822 100644 (file)
@@ -20,6 +20,7 @@
 
 package org.sonar.server.qualityprofile;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Lists;
@@ -34,6 +35,7 @@ import org.sonar.api.rules.ActiveRule;
 import org.sonar.api.rules.ActiveRuleParam;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RulePriority;
+import org.sonar.api.utils.System2;
 import org.sonar.api.utils.ValidationMessages;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.MyBatis;
@@ -52,6 +54,7 @@ import org.sonar.server.user.UserSession;
 import javax.annotation.CheckForNull;
 
 import java.io.StringReader;
+import java.util.Date;
 import java.util.List;
 import java.util.Map;
 
@@ -72,18 +75,28 @@ public class QProfileOperations implements ServerComponent {
   private final ProfileRules profileRules;
   private final ProfilesManager profilesManager;
 
+  private final System2 system;
+
   /**
    * Used by pico when no plugin provide profile exporter / importer
    */
   public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, PropertiesDao propertiesDao,
                             PreviewCache dryRunCache, RuleRegistry ruleRegistry, ProfilesManager profilesManager, ProfileRules profileRules) {
     this(myBatis, dao, activeRuleDao, ruleDao, propertiesDao, Lists.<ProfileImporter>newArrayList(), dryRunCache, ruleRegistry,
-      profilesManager, profileRules);
+      profilesManager, profileRules, System2.INSTANCE);
   }
 
   public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, PropertiesDao propertiesDao,
                             List<ProfileImporter> importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry,
                             ProfilesManager profilesManager, ProfileRules profileRules) {
+    this(myBatis, dao, activeRuleDao, ruleDao, propertiesDao, Lists.<ProfileImporter>newArrayList(), dryRunCache, ruleRegistry,
+      profilesManager, profileRules, System2.INSTANCE);
+  }
+
+  @VisibleForTesting
+  QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, PropertiesDao propertiesDao,
+                            List<ProfileImporter> importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry,
+                            ProfilesManager profilesManager, ProfileRules profileRules, System2 system) {
     this.myBatis = myBatis;
     this.dao = dao;
     this.activeRuleDao = activeRuleDao;
@@ -94,6 +107,7 @@ public class QProfileOperations implements ServerComponent {
     this.ruleRegistry = ruleRegistry;
     this.profilesManager = profilesManager;
     this.profileRules = profileRules;
+    this.system = system;
   }
 
   public NewProfileResult newProfile(String name, String language, Map<String, String> xmlProfilesByPlugin, UserSession userSession) {
@@ -213,7 +227,12 @@ public class QProfileOperations implements ServerComponent {
 
     SqlSession session = myBatis.openSession();
     try {
-      createActiveRuleParam(activeRule, key, value, session);
+      RuleParamDto ruleParam = ruleDao.selectParamByRuleAndKey(activeRule.getRulId(), key, session);
+      if (ruleParam == null) {
+        throw new IllegalArgumentException("No rule param found");
+      }
+      ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setActiveRuleId(activeRule.getId()).setKey(key).setValue(value).setRulesParameterId(ruleParam.getId());
+      activeRuleDao.insert(activeRuleParam, session);
       session.commit();
       profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), key, null, value, userSession.name());
     } finally {
@@ -250,14 +269,30 @@ public class QProfileOperations implements ServerComponent {
     }
   }
 
-  private ActiveRuleParamDto createActiveRuleParam(ActiveRuleDto activeRule, String key, String value, SqlSession session) {
-    RuleParamDto ruleParam = ruleDao.selectParamByRuleAndKey(activeRule.getRulId(), key, session);
-    if (ruleParam == null) {
-      throw new IllegalArgumentException("No rule param found");
+  public void updateActiveRuleNote(ActiveRuleDto activeRule, String note, UserSession userSession) {
+    checkPermission(userSession);
+    Date now = new Date(system.now());
+
+    if (activeRule.getNoteData() == null) {
+      activeRule.setNoteCreatedAt(now);
+      activeRule.setNoteUserLogin(userSession.login());
     }
-    ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setActiveRuleId(activeRule.getId()).setKey(key).setValue(value).setRulesParameterId(ruleParam.getId());
-    activeRuleDao.insert(activeRuleParam, session);
-    return activeRuleParam;
+    activeRule.setNoteUpdatedAt(now);
+    activeRule.setNoteData(note);
+    activeRuleDao.update(activeRule);
+    // TODO notify E/S of active rule change
+  }
+
+  public void deleteActiveRuleNote(ActiveRuleDto activeRule, UserSession userSession) {
+    checkPermission(userSession);
+
+    activeRule.setNoteUpdatedAt(new Date(system.now()));
+    activeRule.setNoteData(null);
+    activeRule.setNoteUserLogin(null);
+    activeRule.setNoteCreatedAt(null);
+    activeRule.setNoteUpdatedAt(null);
+    activeRuleDao.update(activeRule);
+    // TODO notify E/S of active rule change
   }
 
   private List<RulesProfile> readProfilesFromXml(NewProfileResult result, Map<String, String> xmlProfilesByPlugin) {
index 0b7895e34ef8111310ad2fe9c659f8d509e24cb8..a0f697c165a77045f10dc4ad9c5e16046b02042e 100644 (file)
@@ -84,9 +84,8 @@ public class QProfiles implements ServerComponent {
   // deactivate a rule (only E/S indexing)
   // update severity (only E/S indexing)
   // update parameter on a active rule (only E/S indexing)
-  // add note on an active rule
-  // delete note on an active rule
-  // edit note on an active rule
+  // update note on an active rule (only E/S indexing)
+  // delete note on an active rule (only E/S indexing)
   // extends extension of a rule
   //
   // TEMPLATE RULES
@@ -217,10 +216,23 @@ public class QProfiles implements ServerComponent {
     } else if (activeRuleParam != null && sanitizedValue == null) {
       operations.deleteActiveRuleParam(activeRule, activeRuleParam, userSession);
     } else if (activeRuleParam != null) {
-      operations.updateActiveRuleParam(activeRule, activeRuleParam, value, UserSession.get());
+      operations.updateActiveRuleParam(activeRule, activeRuleParam, value, userSession);
     }
   }
 
+  public void updateActiveRuleNote(int activeRuleId, String note) {
+    String sanitizedNote = Strings.emptyToNull(note);
+    ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId);
+    if (sanitizedNote != null) {
+      operations.updateActiveRuleNote(activeRule, sanitizedNote, UserSession.get());
+    }
+  }
+
+  public void deleteActiveRuleNote(int activeRuleId) {
+    ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId);
+    operations.deleteActiveRuleNote(activeRule, UserSession.get());
+  }
+
   //
   // Quality profile validation
   //
index b2a271342cbf6f2b356a211a741b5733b7d67787..5a7a60b64173d1e05c4c5a8cc47b4ae67b706f1f 100644 (file)
@@ -340,29 +340,28 @@ class NewRulesConfigurationController < ApplicationController
 
   def update_active_rule_note
     verify_post_request
-    access_denied unless has_role?(:profileadmin)
     require_parameters :active_rule_id, :note
-    active_rule = ActiveRule.find(params[:active_rule_id])
-    note = active_rule.note
-    unless note
-      note = ActiveRuleNote.new({:active_rule => active_rule})
-      # set the note on the rule to avoid reloading the rule
-      active_rule.note = note
+
+    call_backend do
+      Internal.quality_profiles.updateActiveRuleNote(params[:active_rule_id].to_i, params[:note])
     end
-    note.text = params[:note]
-    note.user_login = current_user.login
-    note.save!
+
+    # TODO use a QProfileRule instead of rails objects
+    active_rule = ActiveRule.find(params[:active_rule_id])
     render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :profile => active_rule.rules_profile}
   end
 
 
   def delete_active_rule_note
     verify_post_request
-    access_denied unless has_role?(:profileadmin)
     require_parameters :active_rule_id
+
+    call_backend do
+      Internal.quality_profiles.deleteActiveRuleNote(params[:active_rule_id].to_i)
+    end
+
+    # TODO use a QProfileRule instead of rails objects
     active_rule = ActiveRule.find(params[:active_rule_id])
-    active_rule.note.destroy if active_rule.note
-    active_rule.note = nil
     render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :profile => active_rule.rules_profile}
   end
 
index 51010b97e75383e5e320e91a28550f44996ac55b..b6a79144dcc8be71466d829e5b8221c73d758bb6 100644 (file)
@@ -37,6 +37,8 @@ import org.sonar.api.rule.Severity;
 import org.sonar.api.rules.ActiveRule;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RulePriority;
+import org.sonar.api.utils.DateUtils;
+import org.sonar.api.utils.System2;
 import org.sonar.api.utils.ValidationMessages;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.MyBatis;
@@ -55,6 +57,7 @@ import org.sonar.server.user.MockUserSession;
 
 import java.io.Reader;
 import java.util.Collection;
+import java.util.Date;
 import java.util.List;
 import java.util.Map;
 
@@ -62,7 +65,11 @@ import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
-import static org.mockito.Matchers.*;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyCollection;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyListOf;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.*;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -98,6 +105,9 @@ public class QProfileOperationsTest {
   @Mock
   ProfilesManager profilesManager;
 
+  @Mock
+  System2 system;
+
   List<ProfileImporter> importers = newArrayList();
 
   QProfileOperations operations;
@@ -119,7 +129,7 @@ public class QProfileOperationsTest {
     }).when(activeRuleDao).insert(any(ActiveRuleDto.class), any(SqlSession.class));
 
     operations = new QProfileOperations(myBatis, qualityProfileDao, activeRuleDao, ruleDao, propertiesDao, importers, dryRunCache, ruleRegistry, profilesManager,
-      profileRules);
+      profileRules, system);
   }
 
   @Test
@@ -384,6 +394,58 @@ public class QProfileOperationsTest {
     verifyZeroInteractions(profilesManager);
   }
 
+  @Test
+  public void update_active_rule_note_when_no_note() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setNoteCreatedAt(null).setNoteData(null);
+
+    long now = System.currentTimeMillis();
+    doReturn(now).when(system).now();
 
+    operations.updateActiveRuleNote(activeRule, "My note", MockUserSession.create().setLogin("nicolas").setName("Nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
+
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao).update(argumentCaptor.capture());
+    assertThat(argumentCaptor.getValue().getNoteData()).isEqualTo("My note");
+    assertThat(argumentCaptor.getValue().getNoteUserLogin()).isEqualTo("nicolas");
+    assertThat(argumentCaptor.getValue().getNoteCreatedAt().getTime()).isEqualTo(now);
+    assertThat(argumentCaptor.getValue().getNoteUpdatedAt().getTime()).isEqualTo(now);
+  }
+
+  @Test
+  public void update_active_rule_note_when_already_note() throws Exception {
+    Date createdAt = DateUtils.parseDate("2013-12-20");
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1)
+      .setNoteCreatedAt(createdAt).setNoteData("My previous note").setNoteUserLogin("nicolas");
+
+    long now = System.currentTimeMillis();
+    doReturn(now).when(system).now();
+
+    operations.updateActiveRuleNote(activeRule, "My new note", MockUserSession.create().setLogin("guy").setName("Guy").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
+
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao).update(argumentCaptor.capture());
+    assertThat(argumentCaptor.getValue().getNoteData()).isEqualTo("My new note");
+    assertThat(argumentCaptor.getValue().getNoteUserLogin()).isEqualTo("nicolas");
+    assertThat(argumentCaptor.getValue().getNoteCreatedAt()).isEqualTo(createdAt);
+    assertThat(argumentCaptor.getValue().getNoteUpdatedAt().getTime()).isEqualTo(now);
+  }
+
+  @Test
+  public void delete_active_rule_note() throws Exception {
+    Date createdAt = DateUtils.parseDate("2013-12-20");
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setNoteCreatedAt(createdAt).setNoteData("My note");
+
+    long now = System.currentTimeMillis();
+    doReturn(now).when(system).now();
+
+    operations.deleteActiveRuleNote(activeRule, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
+
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao).update(argumentCaptor.capture());
+    assertThat(argumentCaptor.getValue().getNoteData()).isNull();
+    assertThat(argumentCaptor.getValue().getNoteUserLogin()).isNull();
+    assertThat(argumentCaptor.getValue().getNoteCreatedAt()).isNull();
+    assertThat(argumentCaptor.getValue().getNoteUpdatedAt()).isNull();
+  }
 
 }
index 2376ba7101fea1f9f1d1aa7bdebe9286232a5449..098d6af11bd48740546b61fe85d96d62ac4ac923 100644 (file)
@@ -405,4 +405,34 @@ public class QProfilesTest {
     verifyZeroInteractions(service);
   }
 
+  @Test
+  public void create_active_rule_note() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(50);
+    when(activeRuleDao.selectById(50)).thenReturn(activeRule);
+
+    qProfiles.updateActiveRuleNote(50, "My note");
+
+    verify(service).updateActiveRuleNote(eq(activeRule), eq("My note"), any(UserSession.class));
+  }
+
+  @Test
+  public void do_nothing_when_create_active_rule_note_with_empty_note() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(50);
+    when(activeRuleDao.selectById(50)).thenReturn(activeRule);
+
+    qProfiles.updateActiveRuleNote(50, "");
+
+    verifyZeroInteractions(service);
+  }
+
+  @Test
+  public void delete_active_rule_note() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(50);
+    when(activeRuleDao.selectById(50)).thenReturn(activeRule);
+
+    qProfiles.deleteActiveRuleNote(50);
+
+    verify(service).deleteActiveRuleNote(eq(activeRule), any(UserSession.class));
+  }
+
 }