]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4535 Revert active rule use Java facade
authorJulien Lancelot <julien.lancelot@gmail.com>
Tue, 24 Dec 2013 16:17:12 +0000 (17:17 +0100)
committerJulien Lancelot <julien.lancelot@gmail.com>
Tue, 24 Dec 2013 16:20:12 +0000 (17:20 +0100)
14 files changed:
sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDao.java
sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java
sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleMapper.java
sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml
sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest.java
sonar-core/src/test/resources/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest/select_parent.xml [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleOperations.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/main/webapp/WEB-INF/app/views/new_rules_configuration/_rule.html.erb
sonar-server/src/main/webapp/WEB-INF/app/views/new_rules_configuration/_rule_param.html.erb
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java

index 069b07d7cf6fe76e530b6b29e65436a337f8ae10..6bbd825ebf278e1629c41f232076b286dce9c4d9 100644 (file)
@@ -61,6 +61,19 @@ public class ActiveRuleDao implements ServerComponent {
     return session.getMapper(ActiveRuleMapper.class).selectByProfileAndRule(profileId, ruleId);
   }
 
+  public ActiveRuleDto selectParent(Integer activeRuleId) {
+    SqlSession session = mybatis.openSession();
+    try {
+      return selectParent(activeRuleId, session);
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+  }
+
+  public ActiveRuleDto selectParent(Integer activeRuleId, SqlSession session) {
+    return session.getMapper(ActiveRuleMapper.class).selectParent(activeRuleId);
+  }
+
   public ActiveRuleParamDto selectParamById(Integer activeRuleParamId) {
     SqlSession session = mybatis.openSession();
     try {
index 5b9176acd3fc4cde8effb70350a57bf759bcf886..29f024110e2fcff7f708a76759f0bb1d1c624d7c 100644 (file)
 
 package org.sonar.core.qualityprofile.db;
 
+import org.apache.commons.lang.StringUtils;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import java.util.Date;
 
 public class ActiveRuleDto {
 
+  public static final String INHERITED = "INHERITED";
+  public static final String OVERRIDES = "OVERRIDES";
+
   private Integer id;
   private Integer profileId;
   private Integer ruleId;
@@ -121,4 +126,13 @@ public class ActiveRuleDto {
     this.noteData = noteData;
     return this;
   }
+
+  public boolean isInherited() {
+    return StringUtils.equals(INHERITED, inheritance);
+  }
+
+  public boolean doesOverride() {
+    return StringUtils.equals(OVERRIDES, inheritance);
+  }
+
 }
index 02d9a9b36049bf64740ffcfac84fb1f661cffce1..e8a1333664d4d5a3de247a2e85c2965b086be6f8 100644 (file)
@@ -35,6 +35,9 @@ public interface ActiveRuleMapper {
 
   List<ActiveRuleDto> selectByRuleId(Integer ruleId);
 
+  @CheckForNull
+  ActiveRuleDto selectParent(Integer id);
+
   void insert(ActiveRuleDto dto);
 
   void update(ActiveRuleDto dto);
index d7e0e5476b5e5fb7ec3b7b2a068dc5f52b174958..051e74fbdef8bd825266d61fe18692a5001ba336 100644 (file)
     SELECT <include refid="activeRuleColumns"/>
     FROM active_rules a
     <where>
-      AND rule_id=#{ruleId}
+      AND a.rule_id=#{ruleId}
     </where>
   </select>
 
+  <select id="selectParent" parameterType="Integer" resultType="ActiveRule">
+    SELECT <include refid="activeRuleColumns"/>
+    FROM active_rules a
+    INNER JOIN rules_profiles profile_parent ON profile_parent.id=a.profile_id
+    INNER JOIN rules_profiles profile_child ON profile_child.parent_name=profile_parent.name and profile_child.language=profile_parent.language
+    INNER JOIN active_rules active_rule_child ON active_rule_child.profile_id=profile_child.id and active_rule_child.id=#{childId} AND a.rule_id=active_rule_child.rule_id
+  </select>
+
   <select id="selectParamById" parameterType="Integer" resultType="ActiveRuleParam">
     SELECT <include refid="activeRuleParamColumns"/>
     FROM active_rule_parameters p
index a2af9d31c67752118676fdef0f076e0f5bb265a7..dbdf7d9934a0d18dbd68fde5715e8a1544994b68 100644 (file)
@@ -88,6 +88,14 @@ public class ActiveRuleDaoTest extends AbstractDaoTestCase {
     assertThat(result).hasSize(2);
   }
 
+  @Test
+  public void select_parent() {
+    setupData("select_parent");
+
+    ActiveRuleDto result = dao.selectParent(1);
+    assertThat(result.getId()).isEqualTo(2);
+  }
+
   @Test
   public void select_param_by_id() {
     setupData("shared");
diff --git a/sonar-core/src/test/resources/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest/select_parent.xml b/sonar-core/src/test/resources/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest/select_parent.xml
new file mode 100644 (file)
index 0000000..962dc31
--- /dev/null
@@ -0,0 +1,21 @@
+<dataset>
+
+  <!-- Active rule child -->
+  <active_rules id="1" profile_id="1" rule_id="11" failure_level="2" inheritance="INHERITED"
+    note_created_at="2013-12-18" note_updated_at="2013-12-18" note_user_login="henry" note_data="some note"/>
+
+  <!-- Active rule parent -->
+  <active_rules id="2" profile_id="2" rule_id="11" failure_level="0" inheritance="[null]"
+    note_created_at="2013-12-18" note_updated_at="2013-12-18" note_user_login="john" note_data="other note"/>
+
+  <!-- Other Active rule on parent -->
+  <active_rules id="3" profile_id="2" rule_id="12" failure_level="1" inheritance="[null]"
+    note_created_at="2013-12-18" note_updated_at="2013-12-18" note_user_login="henry" note_data="other note"/>
+
+  <rules_profiles id="1" name="Child" language="java" parent_name="Parent" version="1"
+                  used_profile="[false]"/>
+
+  <rules_profiles id="2" name="Parent" language="java" parent_name="[null]" version="1"
+                  used_profile="[false]"/>
+
+</dataset>
index 1a304d0c442daed5f31fda2c8caf613faf26263f..fe28647c3b11b9d9e36ac2440e320711dc3d1e39 100644 (file)
@@ -23,10 +23,11 @@ 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;
 import com.google.common.collect.Multimap;
 import org.apache.commons.lang.math.NumberUtils;
 import org.apache.ibatis.session.SqlSession;
+import org.elasticsearch.common.base.Predicate;
+import org.elasticsearch.common.collect.Iterables;
 import org.sonar.api.PropertyType;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.rule.Severity;
@@ -50,6 +51,8 @@ import org.sonar.server.user.UserSession;
 import java.util.Date;
 import java.util.List;
 
+import static com.google.common.collect.Lists.newArrayList;
+
 public class QProfileActiveRuleOperations implements ServerComponent {
 
   private final MyBatis myBatis;
@@ -90,7 +93,7 @@ public class QProfileActiveRuleOperations implements ServerComponent {
       activeRuleDao.insert(activeRule, session);
 
       List<RuleParamDto> ruleParams = ruleDao.selectParameters(rule.getId(), session);
-      List<ActiveRuleParamDto> activeRuleParams = Lists.newArrayList();
+      List<ActiveRuleParamDto> activeRuleParams = newArrayList();
       for (RuleParamDto ruleParam : ruleParams) {
         ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto()
           .setActiveRuleId(activeRule.getId())
@@ -121,10 +124,7 @@ public class QProfileActiveRuleOperations implements ServerComponent {
       activeRuleDao.update(activeRule, session);
       session.commit();
 
-      RuleInheritanceActions actions = profilesManager.ruleSeverityChanged(activeRule.getProfileId(), activeRule.getId(),
-        RulePriority.valueOfInt(oldSeverity), RulePriority.valueOf(newSeverity),
-        userSession.name());
-      reindexInheritanceResult(actions, session);
+      notifySeverityChanged(activeRule, newSeverity, Severity.get(oldSeverity), session, userSession);
     } finally {
       MyBatis.closeQuietly(session);
     }
@@ -174,9 +174,7 @@ public class QProfileActiveRuleOperations implements ServerComponent {
       activeRuleDao.deleteParameter(activeRuleParam.getId(), session);
       session.commit();
 
-      RuleInheritanceActions actions = profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), activeRuleParam.getKey(), activeRuleParam.getValue(),
-        null, userSession.name());
-      reindexInheritanceResult(actions, session);
+      notifyParamsDeleted(activeRule, newArrayList(activeRuleParam), session, userSession);
     } finally {
       MyBatis.closeQuietly(session);
     }
@@ -204,6 +202,79 @@ public class QProfileActiveRuleOperations implements ServerComponent {
     }
   }
 
+  public void revertActiveRule(ActiveRuleDto activeRule, UserSession userSession) {
+    checkPermission(userSession);
+
+    if (activeRule.doesOverride()) {
+      SqlSession session = myBatis.openSession();
+      try {
+        RuleInheritanceActions actions = new RuleInheritanceActions();
+
+        ActiveRuleDto parent = activeRuleDao.selectParent(activeRule.getId(), session);
+
+        // Restore all parameters from parent
+        List<ActiveRuleParamDto> parentParams = activeRuleDao.selectParamsByActiveRuleId(parent.getId(), session);
+        List<ActiveRuleParamDto> activeRuleParams = activeRuleDao.selectParamsByActiveRuleId(activeRule.getId(), session);
+        List<ActiveRuleParamDto> newParams = newArrayList();
+        List<String> paramKeys = newArrayList();
+        for (ActiveRuleParamDto param : activeRuleParams) {
+          final String key = param.getKey();
+          ActiveRuleParamDto parentParam = Iterables.find(parentParams, new Predicate<ActiveRuleParamDto>() {
+            @Override
+            public boolean apply(ActiveRuleParamDto activeRuleParamDto) {
+              return activeRuleParamDto.getKey().equals(key);
+            }
+          }, null);
+          if (parentParam != null && !Strings.isNullOrEmpty(parentParam.getValue())) {
+            String oldValue = param.getValue();
+            String newValue = parentParam.getValue();
+            param.setValue(newValue);
+            activeRuleDao.update(param, session);
+            session.commit();
+            newParams.add(param);
+            actions.add(profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), key, oldValue, newValue, getLoggedName(userSession)));
+          } else {
+            activeRuleDao.deleteParameter(param.getId(), session);
+            session.commit();
+            actions.add(profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), key, param.getValue(), null, userSession.name()));
+          }
+          paramKeys.add(key);
+        }
+        for (ActiveRuleParamDto parentParam : parentParams) {
+          if (!paramKeys.contains(parentParam.getKey())) {
+            ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setActiveRuleId(activeRule.getId())
+              .setKey(parentParam.getKey()).setValue(parentParam.getValue()).setRulesParameterId(parentParam.getRulesParameterId());
+            activeRuleDao.insert(activeRuleParam, session);
+            session.commit();
+            newParams.add(activeRuleParam);
+            actions.add(profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), parentParam.getKey(), null, parentParam.getValue(), userSession.name()));
+          }
+        }
+
+        // Restore severity from parent
+        Integer oldSeverity = activeRule.getSeverity();
+        Integer newSeverity = parent.getSeverity();
+        if (!oldSeverity.equals(newSeverity)) {
+          activeRule.setSeverity(newSeverity);
+          activeRuleDao.update(activeRule, session);
+          session.commit();
+          actions.add(profilesManager.ruleSeverityChanged(activeRule.getProfileId(), activeRule.getId(),
+            RulePriority.valueOf(Severity.get(oldSeverity)), RulePriority.valueOf(Severity.get(newSeverity)), userSession.name()));
+        }
+
+        reindexInheritanceResult(actions, session);
+
+        // Update inheritance
+        activeRule.setInheritance(ActiveRuleDto.INHERITED);
+        activeRuleDao.update(activeRule, session);
+        session.commit();
+        reindexActiveRule(activeRule, newParams);
+      } finally {
+        MyBatis.closeQuietly(session);
+      }
+    }
+  }
+
   public void updateActiveRuleNote(ActiveRuleDto activeRule, String note, UserSession userSession) {
     checkPermission(userSession);
     Date now = new Date(system.now());
@@ -242,6 +313,22 @@ public class QProfileActiveRuleOperations implements ServerComponent {
     }
   }
 
+  private void notifyParamsDeleted(ActiveRuleDto activeRule, List<ActiveRuleParamDto> params, SqlSession session, UserSession userSession) {
+    RuleInheritanceActions actions = new RuleInheritanceActions();
+    for (ActiveRuleParamDto activeRuleParam : params) {
+      actions.add(profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), activeRuleParam.getKey(), activeRuleParam.getValue(),
+        null, userSession.name()));
+    }
+    reindexInheritanceResult(actions, session);
+  }
+
+  private void notifySeverityChanged(ActiveRuleDto activeRule, String newSeverity, String oldSeverity, SqlSession session, UserSession userSession) {
+    RuleInheritanceActions actions = profilesManager.ruleSeverityChanged(activeRule.getProfileId(), activeRule.getId(),
+      RulePriority.valueOf(oldSeverity), RulePriority.valueOf(newSeverity),
+      userSession.name());
+    reindexInheritanceResult(actions, session);
+  }
+
   private void reindexInheritanceResult(RuleInheritanceActions actions, SqlSession session) {
     ruleRegistry.deleteActiveRules(actions.idsToDelete());
     List<ActiveRuleDto> activeRules = activeRuleDao.selectByIds(actions.idsToIndex(), session);
@@ -253,7 +340,11 @@ public class QProfileActiveRuleOperations implements ServerComponent {
   }
 
   private void reindexActiveRule(ActiveRuleDto activeRuleDto, SqlSession session) {
-    ruleRegistry.save(activeRuleDto, activeRuleDao.selectParamsByActiveRuleId(activeRuleDto.getId(), session));
+    reindexActiveRule(activeRuleDto, activeRuleDao.selectParamsByActiveRuleId(activeRuleDto.getId(), session));
+  }
+
+  private void reindexActiveRule(ActiveRuleDto activeRuleDto, List<ActiveRuleParamDto> params) {
+    ruleRegistry.save(activeRuleDto, params);
   }
 
   private void checkPermission(UserSession userSession) {
index 68c2542a7cc6e511671729fb6b845b8d00be27ba..22b36c321d7c0688d894dcb7a4bd3aa7881937db 100644 (file)
@@ -205,7 +205,7 @@ public class QProfileRuleOperations implements ServerComponent {
   }
 
   private void reindexRule(RuleDto rule, SqlSession session) {
-    ruleRegistry.save(rule, ruleDao.selectParameters(rule.getId(), session));
+    reindexRule(rule, ruleDao.selectParameters(rule.getId(), session));
   }
 
   private void reindexRule(RuleDto rule, List<RuleParamDto> ruleParams) {
index f9f04c8cafcef5607f1dd46a7f946ef8cfffdff8..b03481c5f7ee2ac47ea98c98ac938b59d6647745 100644 (file)
@@ -92,7 +92,6 @@ public class QProfiles implements ServerComponent {
   // ACTIVE RULES
   // bulk activate all
   // bulk deactivate all
-  // revert modification on active rule with inheritance -->
   // active rule parameter validation (only Integer types are checked)
 
 
@@ -184,20 +183,6 @@ public class QProfiles implements ServerComponent {
 
   // ACTIVE RULES
 
-  /**
-   * Used to load ancestor active rule of an active rule
-   * <p/>
-   * TODO Ancestor active rules should be integrated into QProfileRule or elsewhere in order to load all ancestor active rules once a time
-   */
-  @CheckForNull
-  public QProfileRule activeRuleByProfileAndRule(QProfile profile, QProfileRule rule) {
-    ActiveRuleDto activeRule = findActiveRule(profile.id(), rule.id());
-    if (activeRule != null) {
-      return rules.getFromActiveRuleId(activeRule.getId());
-    }
-    return null;
-  }
-
   public QProfileRuleResult searchActiveRules(ProfileRuleQuery query, Paging paging) {
     return rules.searchActiveRules(query, paging);
   }
@@ -234,21 +219,6 @@ public class QProfiles implements ServerComponent {
     return activeRuleChanged(qualityProfile, rule);
   }
 
-  /**
-   * Used to load ancestor param of an active rule param
-   * <p/>
-   * TODO Ancestor params should be integrated into QProfileRuleParam or elsewhere in order to load all ancestor params once a time
-   */
-  @CheckForNull
-  public ActiveRuleParamDto activeRuleParam(QProfileRule rule, String key) {
-    Integer activeRuleId = rule.activeRuleId();
-    if (activeRuleId == null) {
-      throw new IllegalArgumentException("Active rule id can't be null");
-    }
-    return findActiveRuleParam(activeRuleId, key);
-
-  }
-
   public ActiveRuleChanged updateActiveRuleParam(int profileId, int activeRuleId, String key, @Nullable String value) {
     String sanitizedValue = Strings.emptyToNull(value);
     QualityProfileDto qualityProfile = findNotNull(profileId);
@@ -267,6 +237,13 @@ public class QProfiles implements ServerComponent {
     return activeRuleChanged(qualityProfile, activeRule);
   }
 
+  public ActiveRuleChanged revertActiveRule(int profileId, int activeRuleId) {
+    QualityProfileDto qualityProfile = findNotNull(profileId);
+    ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId);
+    activeRuleOperations.revertActiveRule(activeRule, UserSession.get());
+    return activeRuleChanged(qualityProfile, activeRule);
+  }
+
   public QProfileRule updateActiveRuleNote(int activeRuleId, String note) {
     ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId);
     String sanitizedNote = Strings.emptyToNull(note);
@@ -284,6 +261,36 @@ public class QProfiles implements ServerComponent {
     return rules.getFromActiveRuleId(activeRule.getId());
   }
 
+  /**
+   * Used to load ancestor param of an active rule param
+   * <p/>
+   * TODO Ancestor params should be integrated into QProfileRuleParam or elsewhere in order to load all ancestor params once a time
+   */
+  @CheckForNull
+  public ActiveRuleParamDto activeRuleParam(QProfileRule rule, String key) {
+    Integer activeRuleId = rule.activeRuleId();
+    if (activeRuleId == null) {
+      throw new IllegalArgumentException("Active rule id can't be null");
+    }
+    return findActiveRuleParam(activeRuleId, key);
+
+  }
+
+  /**
+   * Used to load ancestor active rule of an active rule
+   * <p/>
+   * TODO Ancestor active rules should be integrated into QProfileRule or elsewhere in order to load all ancestor active rules once a time
+   */
+  @CheckForNull
+  public QProfileRule parentActiveRule(QProfileRule rule) {
+    ActiveRuleDto parent = activeRuleDao.selectParent(rule.activeRuleId());
+    if (parent != null) {
+      return rules.getFromActiveRuleId(parent.getId());
+    }
+    return null;
+  }
+
+
   // RULES
 
   public QProfileRule updateRuleNote(int activeRuleId, int ruleId, String note) {
index ff31ca691109cb0d588c0485e4566219bdc6fcdc..037714bbc472a51deb935e6f43a94b1f81c9572c 100644 (file)
@@ -94,12 +94,16 @@ class NewRulesConfigurationController < ApplicationController
   #
   def revert_rule
     verify_post_request
-    access_denied unless has_role?(:profileadmin)
     require_parameters :id, :active_rule_id
-    id = params[:id].to_i
+
+    profile_id = params[:id].to_i
     rule_id = params[:active_rule_id].to_i
-    java_facade.revertRule(id, rule_id, current_user.name)
-    redirect_to request.query_parameters.merge({:action => 'index', :id => params[:id], :commit => nil})
+
+    call_backend do
+      Internal.quality_profiles.revertActiveRule(profile_id, rule_id)
+    end
+
+    redirect_to request.query_parameters.merge({:action => 'index', :id => profile_id, :commit => nil})
   end
 
 
index 7da9d39c5f722bacc733d02c0e8a8a94229fee78..005adfc2467791b0cab5c3322a0f61370cb3ffb6 100644 (file)
     </div>
 
     <%
-       # TODO Ancestor active rule should be integrated in the result rule to prevent bunch of SQL queries to load ancestor profile and ancestor active rule for each actuive rules!
-       ancestor_profile = Internal.quality_profiles.parent(profile)
-       ancestor_active_rule = Internal.quality_profiles.activeRuleByProfileAndRule(profile, rule) if ancestor_profile && (rule.inherited? || rule.overrides?)
+       # TODO Ancestor active rule should be integrated in the result rule to prevent bunch of SQL queries to load ancestor profile and ancestor active rule for each active rules!
+       ancestor_active_rule = Internal.quality_profiles.parentActiveRule(rule) if (rule.inherited? || rule.overrides?)
        if ancestor_active_rule || !rule.params.empty?
+         ancestor_profile = Internal.quality_profiles.parent(profile)
     %>
       <table width="100%" class="table spacer-bottom bordered background-gray">
         <%
index 4b09f1b9124dbb9ddaa2885873eb1f56784432f8..bf39d4b41fe3965e0abd7c6433ce46d5ac813f0f 100644 (file)
@@ -31,7 +31,7 @@
 
   <% if !rule.nil? && rule.overrides? && ancestor_active_rule
        # TODO Ancestor active rule param should be integrated in the result to prevent bunch of SQL queries to load ancestor param for each active rule params!
-       ancestor_param = Internal.quality_profiles.activeRuleParam(rule, parameter.key)
+       ancestor_param = Internal.quality_profiles.activeRuleParam(ancestor_active_rule, parameter.key)
        ancestor_value = ancestor_param && ancestor_param.value ? ancestor_param.value : ''
   %>
     <% if ancestor_value != param_value %>
index e96b51b17c9147c9de6b03810731a8d722b15b2a..7e7dc8b03f37077b68e5ca30dfd7706f9698d9b8 100644 (file)
@@ -65,6 +65,7 @@ import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyList;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.isA;
+import static org.mockito.Mockito.anyListOf;
 import static org.mockito.Mockito.*;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -211,6 +212,56 @@ public class QProfileActiveRuleOperationsTest {
     verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class));
   }
 
+  @Test
+  public void create_active_rule_param() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
+    RuleParamDto ruleParam = new RuleParamDto().setRuleId(10).setName("max").setDefaultValue("20").setType(PropertyType.INTEGER.name());
+    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(ruleParam);
+    when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("Nicolas"))).thenReturn(new RuleInheritanceActions());
+
+    operations.createActiveRuleParam(activeRule, "max", "30", authorizedUserSession);
+
+    ArgumentCaptor<ActiveRuleParamDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleParamDto.class);
+    verify(activeRuleDao).insert(argumentCaptor.capture(), eq(session));
+    assertThat(argumentCaptor.getValue().getKey()).isEqualTo("max");
+    assertThat(argumentCaptor.getValue().getValue()).isEqualTo("30");
+    assertThat(argumentCaptor.getValue().getActiveRuleId()).isEqualTo(5);
+
+    verify(session).commit();
+    verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("Nicolas"));
+    verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class));
+  }
+
+  @Test
+  public void fail_to_create_active_rule_if_no_rule_param() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
+    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(null);
+    try {
+      operations.createActiveRuleParam(activeRule, "max", "30", authorizedUserSession);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class);
+    }
+    verify(activeRuleDao, never()).insert(any(ActiveRuleParamDto.class), eq(session));
+    verifyZeroInteractions(profilesManager);
+  }
+
+  @Test
+  public void fail_to_create_active_rule_if_type_is_invalid() throws Exception {
+    RuleParamDto ruleParam = new RuleParamDto().setRuleId(10).setName("max").setDefaultValue("20").setType(PropertyType.INTEGER.name());
+    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(ruleParam);
+
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
+    try {
+      operations.createActiveRuleParam(activeRule, "max", "invalid integer", authorizedUserSession);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(BadRequestException.class);
+    }
+    verify(activeRuleDao, never()).insert(any(ActiveRuleParamDto.class), eq(session));
+    verifyZeroInteractions(profilesManager);
+  }
+
   @Test
   public void update_active_rule_param() throws Exception {
     ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
@@ -248,53 +299,125 @@ public class QProfileActiveRuleOperationsTest {
   }
 
   @Test
-  public void create_active_rule_param() throws Exception {
-    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
-    RuleParamDto ruleParam = new RuleParamDto().setRuleId(10).setName("max").setDefaultValue("20").setType(PropertyType.INTEGER.name());
-    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(ruleParam);
-    when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("Nicolas"))).thenReturn(new RuleInheritanceActions());
+  public void revert_active_rule_with_severity_to_update() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setInheritance(ActiveRuleDto.OVERRIDES);
+    ActiveRuleDto parent = new ActiveRuleDto().setId(4).setProfileId(1).setRuleId(10).setSeverity(2);
+    when(activeRuleDao.selectParent(5, session)).thenReturn(parent);
 
-    operations.createActiveRuleParam(activeRule, "max", "30", authorizedUserSession);
+    when(profilesManager.ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("Nicolas"))).thenReturn(new RuleInheritanceActions());
 
-    ArgumentCaptor<ActiveRuleParamDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleParamDto.class);
-    verify(activeRuleDao).insert(argumentCaptor.capture(), eq(session));
-    assertThat(argumentCaptor.getValue().getKey()).isEqualTo("max");
-    assertThat(argumentCaptor.getValue().getValue()).isEqualTo("30");
-    assertThat(argumentCaptor.getValue().getActiveRuleId()).isEqualTo(5);
+    operations.revertActiveRule(activeRule, authorizedUserSession);
 
-    verify(session).commit();
-    verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("Nicolas"));
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao, times(2)).update(argumentCaptor.capture(), eq(session));
+    List<ActiveRuleDto> activeRulesChanged = argumentCaptor.getAllValues();
+    assertThat(activeRulesChanged.get(0).getSeverity()).isEqualTo(2);
+    assertThat(activeRulesChanged.get(1).getInheritance()).isEqualTo(ActiveRuleDto.INHERITED);
+
+    verify(session, times(2)).commit();
+    verify(profilesManager).ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("Nicolas"));
     verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class));
+    verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class));
   }
 
   @Test
-  public void fail_to_create_active_rule_if_no_rule_param() throws Exception {
-    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
-    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(null);
-    try {
-      operations.createActiveRuleParam(activeRule, "max", "30", authorizedUserSession);
-      fail();
-    } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalArgumentException.class);
-    }
-    verify(activeRuleDao, never()).insert(any(ActiveRuleParamDto.class), eq(session));
-    verifyZeroInteractions(profilesManager);
+  public void revert_active_rule_with_param_to_update() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setInheritance(ActiveRuleDto.OVERRIDES);
+    when(activeRuleDao.selectParamsByActiveRuleId(eq(5), eq(session))).thenReturn(newArrayList(
+      new ActiveRuleParamDto().setId(102).setActiveRuleId(5).setKey("max").setValue("20")
+    ));
+
+    ActiveRuleDto parent = new ActiveRuleDto().setId(4).setProfileId(1).setRuleId(10).setSeverity(1);
+    when(activeRuleDao.selectParent(5, session)).thenReturn(parent);
+    when(activeRuleDao.selectParamsByActiveRuleId(eq(4), eq(session))).thenReturn(newArrayList(
+      new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("15")
+    ));
+
+    when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("15"), eq("Nicolas"))).thenReturn(new RuleInheritanceActions());
+
+    operations.revertActiveRule(activeRule, authorizedUserSession);
+
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao).update(argumentCaptor.capture(), eq(session));
+    assertThat(argumentCaptor.getValue().getInheritance()).isEqualTo(ActiveRuleDto.INHERITED);
+
+    ArgumentCaptor<ActiveRuleParamDto> paramCaptor = ArgumentCaptor.forClass(ActiveRuleParamDto.class);
+    verify(activeRuleDao).update(paramCaptor.capture(), eq(session));
+    assertThat(paramCaptor.getValue().getId()).isEqualTo(102);
+    assertThat(paramCaptor.getValue().getKey()).isEqualTo("max");
+    assertThat(paramCaptor.getValue().getValue()).isEqualTo("15");
+
+    verify(session, times(2)).commit();
+    verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("15"), eq("Nicolas"));
+    verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class));
+    verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class));
   }
 
   @Test
-  public void fail_to_create_active_rule_if_type_is_invalid() throws Exception {
-    RuleParamDto ruleParam = new RuleParamDto().setRuleId(10).setName("max").setDefaultValue("20").setType(PropertyType.INTEGER.name());
-    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(ruleParam);
+  public void revert_active_rule_with_param_to_delete() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setInheritance(ActiveRuleDto.OVERRIDES);
+    when(activeRuleDao.selectParamsByActiveRuleId(eq(5), eq(session))).thenReturn(newArrayList(
+      new ActiveRuleParamDto().setId(103).setActiveRuleId(5).setKey("format").setValue("abc"))
+    );
 
-    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
-    try {
-      operations.createActiveRuleParam(activeRule, "max", "invalid integer", authorizedUserSession);
-      fail();
-    } catch (Exception e) {
-      assertThat(e).isInstanceOf(BadRequestException.class);
-    }
-    verify(activeRuleDao, never()).insert(any(ActiveRuleParamDto.class), eq(session));
+    ActiveRuleDto parent = new ActiveRuleDto().setId(4).setProfileId(1).setRuleId(10).setSeverity(1);
+    when(activeRuleDao.selectParent(5, session)).thenReturn(parent);
+
+    when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("format"), eq("abc"), eq((String) null), eq("Nicolas"))).thenReturn(new RuleInheritanceActions());
+
+    operations.revertActiveRule(activeRule, authorizedUserSession);
+
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao).update(argumentCaptor.capture(), eq(session));
+    assertThat(argumentCaptor.getValue().getInheritance()).isEqualTo(ActiveRuleDto.INHERITED);
+
+    verify(activeRuleDao).deleteParameter(103, session);
+
+    verify(session, times(2)).commit();
+    verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("format"), eq("abc"), eq((String) null), eq("Nicolas"));
+    verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class));
+    verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class));
+  }
+
+  @Test
+  public void revert_active_rule_with_param_to_create() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setInheritance(ActiveRuleDto.OVERRIDES);
+
+    ActiveRuleDto parent = new ActiveRuleDto().setId(4).setProfileId(1).setRuleId(10).setSeverity(1);
+    when(activeRuleDao.selectParent(5, session)).thenReturn(parent);
+    when(activeRuleDao.selectParamsByActiveRuleId(eq(4), eq(session))).thenReturn(newArrayList(
+      new ActiveRuleParamDto().setId(101).setActiveRuleId(5).setKey("minimum").setValue("2"))
+    );
+
+    when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("minimum"), eq((String) null), eq("2"), eq("Nicolas"))).thenReturn(new RuleInheritanceActions());
+
+    operations.revertActiveRule(activeRule, authorizedUserSession);
+
+    ArgumentCaptor<ActiveRuleDto> argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class);
+    verify(activeRuleDao).update(argumentCaptor.capture(), eq(session));
+    assertThat(argumentCaptor.getValue().getInheritance()).isEqualTo(ActiveRuleDto.INHERITED);
+
+    ArgumentCaptor<ActiveRuleParamDto> paramCaptor = ArgumentCaptor.forClass(ActiveRuleParamDto.class);
+    verify(activeRuleDao).insert(paramCaptor.capture(), eq(session));
+    assertThat(paramCaptor.getValue().getKey()).isEqualTo("minimum");
+    assertThat(paramCaptor.getValue().getValue()).isEqualTo("2");
+
+    verify(session, times(2)).commit();
+    verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("minimum"), eq((String) null), eq("2"), eq("Nicolas"));
+    verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class));
+    verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class));
+  }
+
+  @Test
+  public void no_revert_when_active_rule_do_not_override() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setInheritance(null);
+
+    operations.revertActiveRule(activeRule, authorizedUserSession);
+
+    verifyZeroInteractions(activeRuleDao);
+    verifyZeroInteractions(session);
     verifyZeroInteractions(profilesManager);
+    verifyZeroInteractions(ruleRegistry);
   }
 
   @Test
index 8a30360ec39134788c023a9a1ee33f3a6abd3421..da2e32536fcbd5c4e761d4e87aba9633fa740075 100644 (file)
@@ -283,20 +283,20 @@ public class QProfilesTest {
   }
 
   @Test
-  public void active_rule_by_profile_and_rule() throws Exception {
-    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
-    when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(activeRule);
+  public void parent_active_rule() throws Exception {
+    ActiveRuleDto parent = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
+    when(activeRuleDao.selectParent(6)).thenReturn(parent);
+
     QProfileRule rule = mock(QProfileRule.class);
     when(rule.id()).thenReturn(10);
+    when(rule.activeRuleId()).thenReturn(6);
 
     QProfileRule ruleResult = mock(QProfileRule.class);
     when(rules.getFromActiveRuleId(5)).thenReturn(ruleResult);
 
-    QProfileRule result = qProfiles.activeRuleByProfileAndRule(new QProfile().setId(1), rule);
+    QProfileRule result = qProfiles.parentActiveRule(rule);
 
     assertThat(result).isEqualTo(ruleResult);
-
-    assertThat(qProfiles.activeRuleByProfileAndRule(new QProfile().setId(55), rule)).isNull();
   }
 
   @Test
@@ -480,6 +480,17 @@ public class QProfilesTest {
     verifyZeroInteractions(service);
   }
 
+  @Test
+  public void revert_active_rule() throws Exception {
+    when(qualityProfileDao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"));
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(50);
+    when(activeRuleDao.selectById(50)).thenReturn(activeRule);
+
+    qProfiles.revertActiveRule(1, 50);
+
+    verify(activeRuleOperations).revertActiveRule(eq(activeRule), any(UserSession.class));
+  }
+
   @Test
   public void create_active_rule_note() throws Exception {
     ActiveRuleDto activeRule = new ActiveRuleDto().setId(50);