]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5056 Fix issue when removing characteristic and rules linked with default chara...
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 20 Mar 2014 17:57:40 +0000 (18:57 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 20 Mar 2014 17:57:40 +0000 (18:57 +0100)
12 files changed:
sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java
sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java
sonar-batch/src/test/resources/org/sonar/batch/rule/RulesProviderTest/build_rules_with_default_characteristic_and_disable_characteristic.xml [new file with mode: 0644]
sonar-core/src/main/java/org/sonar/core/rule/RuleDao.java
sonar-core/src/main/java/org/sonar/core/rule/RuleDto.java
sonar-core/src/main/java/org/sonar/core/rule/RuleMapper.java
sonar-core/src/main/resources/org/sonar/core/rule/RuleMapper.xml
sonar-core/src/test/java/org/sonar/core/rule/RuleDaoTest.java
sonar-core/src/test/resources/org/sonar/core/rule/RuleDaoTest/select_by_characteristic_or_sub_characteristic_id.xml [deleted file]
sonar-core/src/test/resources/org/sonar/core/rule/RuleDaoTest/select_by_sub_characteristic_id.xml [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java

index 75afe603f4f31997ae8816aa993315c192ab48f2..701a5b9f81eed828bc6a528794d80989299dd9ba 100644 (file)
@@ -81,7 +81,7 @@ public class RulesProvider extends ProviderAdapter {
 
       if (ruleDto.hasCharacteristic()) {
         newRule.setDebtCharacteristic(effectiveCharacteristic(ruleDto, ruleKey, debtModel).key());
-        newRule.setDebtRemediationFunction(effectiveFunction(ruleDto, ruleKey, newRule, durations));
+        newRule.setDebtRemediationFunction(effectiveFunction(ruleDto, ruleKey, durations));
       }
 
       for (RuleParamDto ruleParamDto : paramDtosByRuleId.get(ruleDto.getId())) {
@@ -103,7 +103,7 @@ public class RulesProvider extends ProviderAdapter {
     return characteristic;
   }
 
-  private DebtRemediationFunction effectiveFunction(RuleDto ruleDto, RuleKey ruleKey, NewRule newRule, Durations durations) {
+  private DebtRemediationFunction effectiveFunction(RuleDto ruleDto, RuleKey ruleKey, Durations durations) {
     String function = ruleDto.getRemediationFunction();
     String defaultFunction = ruleDto.getDefaultRemediationFunction();
     if (function != null) {
index d87e6b3189b747e6baeb1ab723139a7d983741d4..08106710bfe932560279e86ea2c1e5b69433b8ad 100644 (file)
@@ -173,6 +173,17 @@ public class RulesProviderTest extends AbstractDaoTestCase {
     assertThat(rule.debtRemediationFunction()).isNull();
   }
 
+  @Test
+  public void build_rules_with_default_characteristic_and_disable_characteristic() throws Exception {
+    setupData("build_rules_with_default_characteristic_and_disable_characteristic");
+
+    Rules rules = provider.provide(ruleDao, debtModel, durations);
+
+    Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
+    assertThat(rule.debtCharacteristic()).isNull();
+    assertThat(rule.debtRemediationFunction()).isNull();
+  }
+
   @Test
   public void fail_if_characteristic_not_found() throws Exception {
     setupData("fail_if_characteristic_not_found");
diff --git a/sonar-batch/src/test/resources/org/sonar/batch/rule/RulesProviderTest/build_rules_with_default_characteristic_and_disable_characteristic.xml b/sonar-batch/src/test/resources/org/sonar/batch/rule/RulesProviderTest/build_rules_with_default_characteristic_and_disable_characteristic.xml
new file mode 100644 (file)
index 0000000..695d1df
--- /dev/null
@@ -0,0 +1,9 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="AvoidNull" plugin_name="checkstyle" name="Avoid Null" description="Should avoid NULL" status="READY" priority="1"
+         characteristic_id="-1" default_characteristic_id="103"
+         remediation_function="[null]" default_remediation_function="LINEAR"
+         remediation_factor="[null]" default_remediation_factor="2h"
+         remediation_offset="[null]" default_remediation_offset="[null]"/>
+
+</dataset>
index 0324dac5cb0cb7b6889e232979a7106fef327c84..74878a71ff6d723c2bfd3469383d8ade6f724f6a 100644 (file)
@@ -63,17 +63,17 @@ public class RuleDao implements BatchComponent, ServerComponent {
     return getMapper(session).selectNonManual();
   }
 
-  public List<RuleDto> selectByCharacteristicOrSubCharacteristicId(Integer characteristicOrSubCharacteristicId) {
+  public List<RuleDto> selectBySubCharacteristicId(Integer characteristicOrSubCharacteristicId) {
     SqlSession session = mybatis.openSession();
     try {
-      return selectByCharacteristicOrSubCharacteristicId(characteristicOrSubCharacteristicId, session);
+      return selectBySubCharacteristicId(characteristicOrSubCharacteristicId, session);
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
-  public List<RuleDto> selectByCharacteristicOrSubCharacteristicId(Integer characteristicOrSubCharacteristicId, SqlSession session) {
-    return getMapper(session).selectByCharacteristicOrSubCharacteristicId(characteristicOrSubCharacteristicId);
+  public List<RuleDto> selectBySubCharacteristicId(Integer characteristicOrSubCharacteristicId, SqlSession session) {
+    return getMapper(session).selectBySubCharacteristicId(characteristicOrSubCharacteristicId);
   }
 
   public List<RuleDto> selectOverridingDebt(List<String> repositories) {
index 86ab08a3dcd72719c82a5971eeac76209b173c61..e53232b57f913f432ef60c61552902f9c6ca9161 100644 (file)
@@ -315,7 +315,7 @@ public final class RuleDto {
   }
 
   public boolean hasCharacteristic(){
-    return (characteristicId != null && !RuleDto.DISABLED_CHARACTERISTIC_ID.equals(characteristicId)) || defaultCharacteristicId != null;
+    return ((characteristicId != null && !RuleDto.DISABLED_CHARACTERISTIC_ID.equals(characteristicId))) || (characteristicId == null && defaultCharacteristicId != null);
   }
 
   @Override
index 9b49070b0d09595f8c84218a33f261fc51ce5dc2..5976f9c2dd22bdb876952dfb90a09d108b43bd02 100644 (file)
@@ -30,7 +30,7 @@ public interface RuleMapper {
 
   List<RuleDto> selectNonManual();
 
-  List<RuleDto> selectByCharacteristicOrSubCharacteristicId(int id);
+  List<RuleDto> selectBySubCharacteristicId(int characteristicId);
 
   List<RuleDto> selectOverridingDebt(@Param("repositories") List<String> repositories);
 
index 7f61cd090d43815aad3494df46907245b462adf0..16d37d6a426220df5b6f62db9b6364de59bb65b3 100644 (file)
@@ -4,75 +4,74 @@
 <mapper namespace="org.sonar.core.rule.RuleMapper">
 
   <sql id="selectColumns">
-    id,
-    plugin_rule_key as "ruleKey",
-    plugin_name as "repositoryKey",
-    description,
-    status,
-    name,
-    plugin_config_key as "configKey",
-    priority as "severity",
-    cardinality,
-    language as "language",
-    parent_id as "parentId",
-    note_data as "noteData",
-    note_user_login as "noteUserLogin",
-    note_created_at as "noteCreatedAt",
-    note_updated_at as "noteUpdatedAt",
-    characteristic_id as "characteristicId",
-    default_characteristic_id as "defaultCharacteristicId",
-    remediation_function as "remediationFunction",
-    default_remediation_function as "defaultRemediationFunction",
-    remediation_factor as "remediationFactor",
-    default_remediation_factor as "defaultRemediationFactor",
-    remediation_offset as "remediationOffset",
-    default_remediation_offset as "defaultRemediationOffset",
-    effort_to_fix_description as "effortToFixDescription",
-    created_at as "createdAt",
-    updated_at as "updatedAt"
+    r.id,
+    r.plugin_rule_key as "ruleKey",
+    r.plugin_name as "repositoryKey",
+    r.description,
+    r.status,
+    r.name,
+    r.plugin_config_key as "configKey",
+    r.priority as "severity",
+    r.cardinality,
+    r.language as "language",
+    r.parent_id as "parentId",
+    r.note_data as "noteData",
+    r.note_user_login as "noteUserLogin",
+    r.note_created_at as "noteCreatedAt",
+    r.note_updated_at as "noteUpdatedAt",
+    r.characteristic_id as "characteristicId",
+    r.default_characteristic_id as "defaultCharacteristicId",
+    r.remediation_function as "remediationFunction",
+    r.default_remediation_function as "defaultRemediationFunction",
+    r.remediation_factor as "remediationFactor",
+    r.default_remediation_factor as "defaultRemediationFactor",
+    r.remediation_offset as "remediationOffset",
+    r.default_remediation_offset as "defaultRemediationOffset",
+    r.effort_to_fix_description as "effortToFixDescription",
+    r.created_at as "createdAt",
+    r.updated_at as "updatedAt"
   </sql>
 
   <select id="selectAll" resultType="Rule">
-    select <include refid="selectColumns"/> from rules
+    select <include refid="selectColumns"/> from rules r
   </select>
 
   <select id="selectEnablesAndNonManual" resultType="Rule">
-    select <include refid="selectColumns"/> from rules
+    select <include refid="selectColumns"/> from rules r
     <where>
-      and status != 'REMOVED'
-      and plugin_name != 'manual'
+      and r.status != 'REMOVED'
+      and r.plugin_name != 'manual'
     </where>
   </select>
 
   <select id="selectById" parameterType="Integer" resultType="Rule">
-    select <include refid="selectColumns"/> from rules WHERE id=#{id}
+    select <include refid="selectColumns"/> from rules r WHERE r.id=#{id}
   </select>
 
   <select id="selectByName" parameterType="String" resultType="Rule">
-    select <include refid="selectColumns"/> from rules WHERE name=#{name}
+    select <include refid="selectColumns"/> from rules r WHERE r.name=#{name}
   </select>
 
   <select id="selectNonManual" resultType="Rule">
-    select <include refid="selectColumns"/> from rules
-    where plugin_name != 'manual'
+    select <include refid="selectColumns"/> from rules r
+    where r.plugin_name != 'manual'
   </select>
 
-  <select id="selectByCharacteristicOrSubCharacteristicId" resultType="Rule">
-    select <include refid="selectColumns"/> from rules
+  <select id="selectBySubCharacteristicId" resultType="Rule">
+    select <include refid="selectColumns"/> from rules r
     <where>
-      and characteristic_id in (select c.id from characteristics c where (c.id=#{id} or c.parent_id=#{id}) and c.enabled=${_true})
-      and status!='REMOVED'
+      and (r.characteristic_id=#{characteristicId} or r.default_characteristic_id=#{characteristicId})
     </where>
   </select>
 
   <select id="selectOverridingDebt" resultType="Rule">
-    SELECT <include refid="selectColumns"/> FROM rules
+    SELECT <include refid="selectColumns"/> FROM rules r
     <where>
-      AND (characteristic_id is NOT NULL or remediation_function IS NOT NULL)
+      AND (r.characteristic_id is NOT NULL or r.remediation_function IS NOT NULL)
       <if test="repositories.size()>0">
         AND
         <foreach item="repo" index="index" collection="repositories" open="(" separator=" or " close=")">
-          plugin_name=#{repo}
+          r.plugin_name=#{repo}
         </foreach>
       </if>
     </where>
index 1c638ff976cc225d96dcfcb7bcc0dc48a7b9b0d1..48fc65093c05cfb5c8c2da9b679bc4898f813f47 100644 (file)
@@ -137,22 +137,21 @@ public class RuleDaoTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void select_by_characteristic_or_sub_characteristic_id(){
-    setupData("select_by_characteristic_or_sub_characteristic_id");
+  public void select_by_sub_characteristic_id(){
+    setupData("select_by_sub_characteristic_id");
 
     // Rules from sub characteristic
-    List<RuleDto> ruleDtos = dao.selectByCharacteristicOrSubCharacteristicId(2);
-    assertThat(ruleDtos).hasSize(1);
-    assertThat(ruleDtos.get(0).getId()).isEqualTo(1);
+    List<RuleDto> ruleDtos = dao.selectBySubCharacteristicId(3);
+    assertThat(ruleDtos).hasSize(3);
+    assertThat(idsFromRuleDtos(ruleDtos)).containsExactly(2, 4, 5);
 
-    // Rules from characteristic
-    ruleDtos = dao.selectByCharacteristicOrSubCharacteristicId(1);
-    assertThat(ruleDtos).hasSize(2);
-    assertThat(idsFromRuleDtos(ruleDtos)).containsExactly(1, 2);
+    // Nothing on root characteristic
+    ruleDtos = dao.selectBySubCharacteristicId(1);
+    assertThat(ruleDtos).isEmpty();
 
     // Rules from disabled characteristic
-    ruleDtos = dao.selectByCharacteristicOrSubCharacteristicId(11);
-    assertThat(ruleDtos).isEmpty();
+    ruleDtos = dao.selectBySubCharacteristicId(11);
+    assertThat(idsFromRuleDtos(ruleDtos)).containsExactly(3);
   }
 
   @Test
diff --git a/sonar-core/src/test/resources/org/sonar/core/rule/RuleDaoTest/select_by_characteristic_or_sub_characteristic_id.xml b/sonar-core/src/test/resources/org/sonar/core/rule/RuleDaoTest/select_by_characteristic_or_sub_characteristic_id.xml
deleted file mode 100644 (file)
index db7d5b3..0000000
+++ /dev/null
@@ -1,61 +0,0 @@
-<dataset>
-
-  <!-- Root characteristic -->
-  <characteristics id="1" kee="PORTABILITY" name="Portability" parent_id="[null]" characteristic_order="1"
-                   enabled="[true]"
-                   created_at="2013-11-20" updated_at="2013-11-22"/>
-
-  <!-- Sub characteristics of root characteristic -->
-  <characteristics id="2" kee="COMPILER_RELATED_PORTABILITY" name="Compiler related portability" parent_id="1" rcharacteristic_order="[null]"
-                   enabled="[true]"
-                   created_at="2013-11-20" updated_at="2013-11-22"/>
-  <characteristics id="3" kee="HARDWARE_RELATED_PORTABILITY" name="Hardware related portability " parent_id="1" rcharacteristic_order="[null]"
-                   enabled="[true]"
-                   created_at="2013-11-20" updated_at="2013-11-22"/>
-
-  <!-- Disabled root characteristic -->
-  <characteristics id="10" kee="DISABLED_ROOT_CHARACTERISTIC" name="Disabled root characteristic" parent_id="[null]" characteristic_order="2"
-                   enabled="[false]"
-                   created_at="2013-11-20" updated_at="2013-11-22"/>
-
-  <!-- Disabled characteristic -->
-  <characteristics id="11" kee="DISABLED_CHARACTERISTIC" name="Disabled characteristic" parent_id="10" characteristic_order="[null]"
-                   enabled="[false]"
-                   created_at="2013-11-20" updated_at="2013-11-22"/>
-
-  <!-- Rule linked to a sub characteristic -->
-  <rules id="1" plugin_rule_key="UselessImportCheck" plugin_name="squid" name="UselessImportCheck" description="Useless imports should be removed" status="READY"
-         characteristic_id="2" default_characteristic_id="50"
-         remediation_function="LINEAR_OFFSET" default_remediation_function="LINEAR_OFFSET"
-         remediation_factor="5d" default_remediation_factor="5d"
-         remediation_offset="10h" default_remediation_offset="10h" updated_at="2014-02-19"/>
-
-  <!-- Rule linked to a sub characteristic -->
-  <rules id="2" plugin_rule_key="LeftCurlyBraceStartLineCheck" plugin_name="squid" name="LeftCurlyBraceStartLineCheck" description="Left curly braces should be located at the beginning of lines of code" status="READY"
-         characteristic_id="3" default_characteristic_id="50"
-         remediation_function="LINEAR_OFFSET" default_remediation_function="LINEAR_OFFSET"
-         remediation_factor="5d" default_remediation_factor="5d"
-         remediation_offset="10h" default_remediation_offset="10h" updated_at="2014-02-19"/>
-
-  <!-- Rule linked to a disabled sub characteristic -> should never be returned -->
-  <rules id="3" plugin_rule_key="CallToFileDeleteOnExitMethod" plugin_name="squid" name="CallToFileDeleteOnExitMethod" description="CallToFileDeleteOnExitMethod" status="READY"
-         characteristic_id="11" default_characteristic_id="50"
-         remediation_function="LINEAR" default_remediation_function="LINEAR_OFFSET"
-         remediation_factor="5d" default_remediation_factor="5d"
-         remediation_offset="[null]" default_remediation_offset="10h" updated_at="2014-02-19"/>
-
-  <!-- Removed rule linked to one enable sub characteristic -> should never be returned -->
-  <rules id="4" plugin_rule_key="ObjectFinalizeOverridenCallsSuperFinalizeCheck" plugin_name="squid" name="ObjectFinalizeOverridenCallsSuperFinalizeCheck" description="super.finalize() should be called at the end of Object.finalize() implementations" status="REMOVED"
-         characteristic_id="3" default_characteristic_id="50"
-         remediation_function="LINEAR" default_remediation_function="LINEAR_OFFSET"
-         remediation_factor="5d" default_remediation_factor="5min"
-         remediation_offset="[null]" default_remediation_offset="10h" updated_at="2014-02-19"/>
-
-  <!-- Rule linked to a sub characteristic, but only default characteristic is linked -> should never be returned -->
-  <rules id="5" plugin_rule_key="RightCurlyBraceStartLineCheck" plugin_name="squid" name="RightCurlyBraceStartLineCheck" description="Right curly braces should be located at the beginning of lines of code" status="READY"
-         characteristic_id="[null]" default_characteristic_id="3"
-         remediation_function="[null]" default_remediation_function="LINEAR"
-         remediation_factor="[null]" default_remediation_factor="5d"
-         remediation_offset="[null]" default_remediation_offset="[null]" updated_at="2014-02-19"/>
-
-</dataset>
diff --git a/sonar-core/src/test/resources/org/sonar/core/rule/RuleDaoTest/select_by_sub_characteristic_id.xml b/sonar-core/src/test/resources/org/sonar/core/rule/RuleDaoTest/select_by_sub_characteristic_id.xml
new file mode 100644 (file)
index 0000000..b712b6e
--- /dev/null
@@ -0,0 +1,61 @@
+<dataset>
+
+  <!-- Root characteristic -->
+  <characteristics id="1" kee="PORTABILITY" name="Portability" parent_id="[null]" characteristic_order="1"
+                   enabled="[true]"
+                   created_at="2013-11-20" updated_at="2013-11-22"/>
+
+  <!-- Sub characteristics of root characteristic -->
+  <characteristics id="2" kee="COMPILER_RELATED_PORTABILITY" name="Compiler related portability" parent_id="1" rcharacteristic_order="[null]"
+                   enabled="[true]"
+                   created_at="2013-11-20" updated_at="2013-11-22"/>
+  <characteristics id="3" kee="HARDWARE_RELATED_PORTABILITY" name="Hardware related portability " parent_id="1" rcharacteristic_order="[null]"
+                   enabled="[true]"
+                   created_at="2013-11-20" updated_at="2013-11-22"/>
+
+  <!-- Disabled root characteristic -->
+  <characteristics id="10" kee="DISABLED_ROOT_CHARACTERISTIC" name="Disabled root characteristic" parent_id="[null]" characteristic_order="2"
+                   enabled="[false]"
+                   created_at="2013-11-20" updated_at="2013-11-22"/>
+
+  <!-- Disabled characteristic -->
+  <characteristics id="11" kee="DISABLED_CHARACTERISTIC" name="Disabled characteristic" parent_id="10" characteristic_order="[null]"
+                   enabled="[false]"
+                   created_at="2013-11-20" updated_at="2013-11-22"/>
+
+  <!-- Rule linked to a sub characteristic -->
+  <rules id="1" plugin_rule_key="UselessImportCheck" plugin_name="squid" name="UselessImportCheck" description="Useless imports should be removed" status="READY"
+         characteristic_id="2" default_characteristic_id="50"
+         remediation_function="LINEAR_OFFSET" default_remediation_function="LINEAR_OFFSET"
+         remediation_factor="5d" default_remediation_factor="5d"
+         remediation_offset="10h" default_remediation_offset="10h" updated_at="2014-02-19"/>
+
+  <!-- Rule linked to a sub characteristic -->
+  <rules id="2" plugin_rule_key="LeftCurlyBraceStartLineCheck" plugin_name="squid" name="LeftCurlyBraceStartLineCheck" description="Left curly braces should be located at the beginning of lines of code" status="READY"
+         characteristic_id="3" default_characteristic_id="50"
+         remediation_function="LINEAR_OFFSET" default_remediation_function="LINEAR_OFFSET"
+         remediation_factor="5d" default_remediation_factor="5d"
+         remediation_offset="10h" default_remediation_offset="10h" updated_at="2014-02-19"/>
+
+  <!-- Rule linked to a disabled sub characteristic -> should never be returned -->
+  <rules id="3" plugin_rule_key="CallToFileDeleteOnExitMethod" plugin_name="squid" name="CallToFileDeleteOnExitMethod" description="CallToFileDeleteOnExitMethod" status="READY"
+         characteristic_id="11" default_characteristic_id="50"
+         remediation_function="LINEAR" default_remediation_function="LINEAR_OFFSET"
+         remediation_factor="5d" default_remediation_factor="5d"
+         remediation_offset="[null]" default_remediation_offset="10h" updated_at="2014-02-19"/>
+
+  <!-- Removed rule linked to one enable sub characteristic -->
+  <rules id="4" plugin_rule_key="ObjectFinalizeOverridenCallsSuperFinalizeCheck" plugin_name="squid" name="ObjectFinalizeOverridenCallsSuperFinalizeCheck" description="super.finalize() should be called at the end of Object.finalize() implementations" status="REMOVED"
+         characteristic_id="3" default_characteristic_id="50"
+         remediation_function="LINEAR" default_remediation_function="LINEAR_OFFSET"
+         remediation_factor="5d" default_remediation_factor="5min"
+         remediation_offset="[null]" default_remediation_offset="10h" updated_at="2014-02-19"/>
+
+  <!-- Rule linked to a sub characteristic, but only default characteristic is linked  -->
+  <rules id="5" plugin_rule_key="RightCurlyBraceStartLineCheck" plugin_name="squid" name="RightCurlyBraceStartLineCheck" description="Right curly braces should be located at the beginning of lines of code" status="READY"
+         characteristic_id="[null]" default_characteristic_id="3"
+         remediation_function="[null]" default_remediation_function="LINEAR"
+         remediation_factor="[null]" default_remediation_factor="5d"
+         remediation_offset="[null]" default_remediation_offset="[null]" updated_at="2014-02-19"/>
+
+</dataset>
index 4b7c60047462dece9ee906b733895d64b40e532d..cbd9c4bcd6376e84872651e7e62e5e8e5a798318 100644 (file)
@@ -39,6 +39,7 @@ import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.UserSession;
 import org.sonar.server.util.Validation;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 import java.util.Date;
@@ -130,23 +131,13 @@ public class DebtModelOperations implements ServerComponent {
     try {
       final CharacteristicDto dto = findCharacteristic(characteristicId, session);
       int currentOrder = dto.getOrder();
-
-      // characteristics should be order by 'order'
-      List<CharacteristicDto> rootCharacteristics = dao.selectEnabledRootCharacteristics(session);
-      int currentPosition = Iterables.indexOf(rootCharacteristics, new Predicate<CharacteristicDto>() {
-        @Override
-        public boolean apply(CharacteristicDto input) {
-          return input.getKey().equals(dto.getKey());
-        }
-      });
-      Integer nextMove = moveUpOrDown ? (currentPosition > 0 ? currentPosition - 1 : null) : (currentPosition < rootCharacteristics.size()-1 ? currentPosition + 1 : null);
+      CharacteristicDto dtoToSwitchOrderWith = findCharacteristicToSwitchWith(dto, moveUpOrDown, session);
 
       // Do nothing when characteristic is already to the good location
-      if (nextMove == null) {
+      if (dtoToSwitchOrderWith == null) {
         return toCharacteristic(dto);
       }
 
-      CharacteristicDto dtoToSwitchOrderWith = Iterables.get(rootCharacteristics, nextMove);
       int nextOrder = dtoToSwitchOrderWith.getOrder();
       dtoToSwitchOrderWith.setOrder(currentOrder);
       dtoToSwitchOrderWith.setUpdatedAt(new Date(system2.now()));
@@ -163,6 +154,20 @@ public class DebtModelOperations implements ServerComponent {
     }
   }
 
+  @CheckForNull
+  private CharacteristicDto findCharacteristicToSwitchWith(final CharacteristicDto dto, final boolean moveUpOrDown, SqlSession session) {
+    // characteristics should be order by 'order'
+    List<CharacteristicDto> rootCharacteristics = dao.selectEnabledRootCharacteristics(session);
+    int currentPosition = Iterables.indexOf(rootCharacteristics, new Predicate<CharacteristicDto>() {
+      @Override
+      public boolean apply(CharacteristicDto input) {
+        return input.getKey().equals(dto.getKey());
+      }
+    });
+    Integer nextPosition = moveUpOrDown ? (currentPosition > 0 ? currentPosition - 1 : null) : (currentPosition < rootCharacteristics.size() - 1 ? currentPosition + 1 : null);
+    return nextPosition != null ? Iterables.get(rootCharacteristics, nextPosition) : null;
+  }
+
   /**
    * Disable characteristic and its sub characteristics or only sub characteristic.
    * Will also update every rules linked to sub characteristics by setting characteristic id to -1 and remove function, factor and offset.
@@ -174,27 +179,29 @@ public class DebtModelOperations implements ServerComponent {
     SqlSession session = mybatis.openBatchSession();
     try {
       CharacteristicDto characteristicOrSubCharacteristic = findCharacteristic(characteristicId, session);
-      disableDebtRules(
-        ruleDao.selectByCharacteristicOrSubCharacteristicId(characteristicOrSubCharacteristic.getId(), session),
-        updateDate,
-        session
-      );
-
+      // When root characteristic, browse sub characteristics and disable rule debt on each sub characteristic then disable it
       if (characteristicOrSubCharacteristic.getParentId() == null) {
         List<CharacteristicDto> subCharacteristics = dao.selectCharacteristicsByParentId(characteristicOrSubCharacteristic.getId(), session);
         for (CharacteristicDto subCharacteristic : subCharacteristics) {
-          disableCharacteristic(subCharacteristic, updateDate, session);
+          disableSubChracteristic(subCharacteristic, updateDate, session);
         }
+        disableCharacteristic(characteristicOrSubCharacteristic, updateDate, session);
+      } else {
+        // When sub characteristic, disable rule debt on the sub characteristic then disable it
+        disableSubChracteristic(characteristicOrSubCharacteristic, updateDate, session);
       }
-      disableCharacteristic(characteristicOrSubCharacteristic, updateDate, session);
-
       session.commit();
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
-  public void disableDebtRules(List<RuleDto> ruleDtos, Date updateDate, SqlSession session){
+  private void disableSubChracteristic(CharacteristicDto subCharacteristic, Date updateDate, SqlSession session) {
+    disableDebtRules(ruleDao.selectBySubCharacteristicId(subCharacteristic.getId(), session), updateDate, session);
+    disableCharacteristic(subCharacteristic, updateDate, session);
+  }
+
+  public void disableDebtRules(List<RuleDto> ruleDtos, Date updateDate, SqlSession session) {
     for (RuleDto ruleDto : ruleDtos) {
       ruleDto.setCharacteristicId(RuleDto.DISABLED_CHARACTERISTIC_ID);
       ruleDto.setRemediationFunction(null);
@@ -206,7 +213,7 @@ public class DebtModelOperations implements ServerComponent {
     }
   }
 
-  public void disableCharacteristic(CharacteristicDto characteristic, Date updateDate, SqlSession session){
+  public void disableCharacteristic(CharacteristicDto characteristic, Date updateDate, SqlSession session) {
     characteristic.setEnabled(false);
     characteristic.setUpdatedAt(updateDate);
     dao.update(characteristic, session);
index f3ab22fe6b3bc7709995a0abfcbecdc963f8d155..97570e977c3d3cdd649f376c925e38d6d41a4389 100644 (file)
@@ -303,7 +303,7 @@ public class DebtModelOperationsTest {
     BatchSession batchSession = mock(BatchSession.class);
     when(mybatis.openBatchSession()).thenReturn(batchSession);
 
-    when(ruleDao.selectByCharacteristicOrSubCharacteristicId(2, batchSession)).thenReturn(newArrayList(
+    when(ruleDao.selectBySubCharacteristicId(2, batchSession)).thenReturn(newArrayList(
       new RuleDto()
         .setCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationFactor("2h").setRemediationOffset("5min")
         .setDefaultCharacteristicId(10).setDefaultRemediationFunction("LINEAR_OFFSET").setDefaultRemediationFactor("4h").setDefaultRemediationOffset("15min")
@@ -344,8 +344,8 @@ public class DebtModelOperationsTest {
     BatchSession batchSession = mock(BatchSession.class);
     when(mybatis.openBatchSession()).thenReturn(batchSession);
 
-    when(ruleDao.selectByCharacteristicOrSubCharacteristicId(1, batchSession)).thenReturn(newArrayList(
-      new RuleDto().setCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationFactor("2h").setRemediationOffset("5min")
+    when(ruleDao.selectBySubCharacteristicId(subCharacteristicDto.getId(), batchSession)).thenReturn(newArrayList(
+      new RuleDto().setCharacteristicId(subCharacteristicDto.getId()).setRemediationFunction("LINEAR_OFFSET").setRemediationFactor("2h").setRemediationOffset("5min")
     ));
     when(dao.selectCharacteristicsByParentId(1, batchSession)).thenReturn(newArrayList(
       subCharacteristicDto