]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5119 Do not display warning log if rule characteristic has been overriden
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 18 Mar 2014 10:04:47 +0000 (11:04 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 18 Mar 2014 10:04:47 +0000 (11:04 +0100)
12 files changed:
sonar-core/src/main/java/org/sonar/core/rule/RuleDto.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporter.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDao.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java
sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
sonar-server/src/main/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRules.java
sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_characteristic_not_found-result.xml
sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled-result.xml [new file with mode: 0644]
sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled.xml [new file with mode: 0644]
sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden-result.xml [new file with mode: 0644]
sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden.xml [new file with mode: 0644]

index b6b280cf96b3149c70abed1ffa6db6a7cfb673a7..52f79d26ce8d90f72c17ef188af6aef6a5b46583 100644 (file)
@@ -24,6 +24,7 @@ import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.commons.lang.builder.ReflectionToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
 import org.sonar.check.Cardinality;
+import org.sonar.core.technicaldebt.db.CharacteristicDto;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
@@ -32,8 +33,6 @@ import java.util.Date;
 
 public final class RuleDto {
 
-  public static final Integer DISABLED_CHARACTERISTIC_ID = -1;
-
   private Integer id;
   private String repositoryKey;
   private String ruleKey;
@@ -315,7 +314,7 @@ public final class RuleDto {
   }
 
   public boolean hasCharacteristic(){
-    return (characteristicId != null && !DISABLED_CHARACTERISTIC_ID.equals(characteristicId)) || defaultCharacteristicId != null;
+    return (characteristicId != null && !CharacteristicDto.DISABLED_CHARACTERISTIC_ID.equals(characteristicId)) || defaultCharacteristicId != null;
   }
 
   @Override
index aeb1a9655b0afe55a2cc0a01404f03fcf280419a..a6164c1c3e5d9bd09283d1df4411d7f05d89cad1 100644 (file)
@@ -38,7 +38,6 @@ import org.sonar.api.technicaldebt.batch.internal.DefaultCharacteristic;
 import org.sonar.api.technicaldebt.batch.internal.DefaultRequirement;
 import org.sonar.api.utils.ValidationMessages;
 import org.sonar.api.utils.internal.WorkDuration;
-import org.sonar.core.technicaldebt.db.CharacteristicDto;
 
 import javax.annotation.CheckForNull;
 import javax.xml.stream.XMLInputFactory;
@@ -227,7 +226,7 @@ public class TechnicalDebtXMLImporter implements ServerExtension {
       if ("linear_threshold".equals(functionKey)) {
         function.setTextValue(DefaultRequirement.FUNCTION_LINEAR);
         offset.setValue(0);
-        offset.setTextValue(CharacteristicDto.DAYS);
+        offset.setTextValue("d");
         messages.addWarningText(String.format("Linear with threshold function is no longer used, function of the requirement '%s' is replaced by linear.", requirement.ruleKey()));
       } else if ("constant_resource".equals(functionKey)) {
         messages.addWarningText(String.format("Constant/file function is no longer used, requirements '%s' are ignored.", requirement.ruleKey()));
index 9c927d47515d5c9915ec10698b7f579f16b1b8ce..42ef216b738b241f7d4e84f8f3d7020ca06a9a1d 100644 (file)
@@ -38,7 +38,7 @@ public class CharacteristicDao implements BatchComponent, ServerComponent {
   }
 
   /**
-   * @return enabled root characteristics, characteristics and requirements
+   * @return enabled root characteristics and characteristics
    *
    */
   public List<CharacteristicDto> selectEnabledCharacteristics() {
index fc394d4a1c166e33e1d060bfc503e23d2d9caa47..4f0b85be817e581b4fef2e4d836726e3e081d673 100644 (file)
@@ -30,9 +30,7 @@ import java.util.Date;
 
 public class CharacteristicDto implements Serializable {
 
-  public static final String DAYS = "d";
-  public static final String MINUTES = "mn";
-  public static final String HOURS = "h";
+  public static final Integer DISABLED_CHARACTERISTIC_ID = -1;
 
   private Integer id;
   private String kee;
@@ -52,22 +50,20 @@ public class CharacteristicDto implements Serializable {
     return this;
   }
 
-  @CheckForNull
   public String getKey() {
     return kee;
   }
 
-  public CharacteristicDto setKey(@Nullable String s) {
+  public CharacteristicDto setKey(String s) {
     this.kee = s;
     return this;
   }
 
-  @CheckForNull
   public String getName() {
     return name;
   }
 
-  public CharacteristicDto setName(@Nullable String s) {
+  public CharacteristicDto setName(String s) {
     this.name = s;
     return this;
   }
index d62ea4e72bd856cc0395dab2ed92b93294909332..6b82426ebaa309c71bacc5bb2c677b9edf473553 100644 (file)
@@ -46,6 +46,7 @@ import org.sonar.server.qualityprofile.ProfilesManager;
 import org.sonar.server.startup.RegisterDebtModel;
 
 import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 
 import java.util.*;
 
@@ -187,15 +188,14 @@ public class RuleRegistration implements Startable {
       .setUpdatedAt(buffer.now())
       .setStatus(ruleDef.status().name());
 
-    CharacteristicDto characteristic = findCharacteristic(characteristicDtos, ruleDef);
-    ruleDto.setDefaultCharacteristicId(characteristic != null ? characteristic.getId() : null)
-      .setEffortToFixL10nKey(ruleDef.effortToFixL10nKey());
-
+    CharacteristicDto characteristic = findCharacteristic(ruleDef, null, characteristicDtos);
     DebtRemediationFunction remediationFunction = ruleDef.debtRemediationFunction();
-    if (remediationFunction != null) {
-      ruleDto.setDefaultRemediationFunction(remediationFunction.type().name());
-      ruleDto.setDefaultRemediationFactor(remediationFunction.factor());
-      ruleDto.setDefaultRemediationOffset(remediationFunction.offset());
+    if (characteristic != null && remediationFunction != null) {
+      ruleDto.setDefaultCharacteristicId(characteristic.getId())
+        .setDefaultRemediationFunction(remediationFunction.type().name())
+        .setDefaultRemediationFactor(remediationFunction.factor())
+        .setDefaultRemediationOffset(remediationFunction.offset())
+        .setEffortToFixL10nKey(ruleDef.effortToFixL10nKey());
     }
 
     ruleDao.insert(ruleDto, sqlSession);
@@ -267,13 +267,14 @@ public class RuleRegistration implements Startable {
   private boolean mergeDebtDefinitions(RulesDefinition.Rule def, RuleDto dto, List<CharacteristicDto> characteristicDtos) {
     boolean changed = false;
 
-    CharacteristicDto characteristic = findCharacteristic(characteristicDtos, def);
+    CharacteristicDto characteristic = findCharacteristic(def, dto.getCharacteristicId(), characteristicDtos);
     // Debt definitions are set to null if the characteristic is null or unknown
+    boolean hasCharacteristic = characteristic != null;
+    DebtRemediationFunction debtRemediationFunction = characteristic != null ? def.debtRemediationFunction() : null;
     Integer characteristicId = characteristic != null ? characteristic.getId() : null;
-    DebtRemediationFunction debtRemediationFunction = def.debtRemediationFunction();
-    String remediationFactor = debtRemediationFunction != null ? debtRemediationFunction.factor() : null;
-    String remediationOffset = debtRemediationFunction != null ? debtRemediationFunction.offset() : null;
-    String effortToFixL10nKey = def.effortToFixL10nKey();
+    String remediationFactor = hasCharacteristic ? debtRemediationFunction.factor() : null;
+    String remediationOffset = hasCharacteristic ? debtRemediationFunction.offset() : null;
+    String effortToFixL10nKey = hasCharacteristic ? def.effortToFixL10nKey() : null;
 
     if (!ObjectUtils.equals(dto.getDefaultCharacteristicId(), characteristicId)) {
       dto.setDefaultCharacteristicId(characteristicId);
@@ -547,27 +548,31 @@ public class RuleRegistration implements Startable {
   }
 
   @CheckForNull
-  private CharacteristicDto findCharacteristic(List<CharacteristicDto> characteristicDtos, RulesDefinition.Rule ruleDef) {
-    final String key = ruleDef.debtCharacteristic();
+  private CharacteristicDto findCharacteristic(RulesDefinition.Rule ruleDef, @Nullable Integer overridingCharacteristicId, List<CharacteristicDto> characteristicDtos) {
+    String key = ruleDef.debtCharacteristic();
+    // Rule is not linked to a default characteristic or characteristic has been disabled by user, nothing to do
     if (key == null) {
-      // Rule is not linked to a characteristic, nothing to do
       return null;
     }
-    CharacteristicDto characteristicDto = Iterables.find(characteristicDtos, new Predicate<CharacteristicDto>() {
-      @Override
-      public boolean apply(CharacteristicDto input) {
-        String characteristicKey = input.getKey();
-        return characteristicKey != null && characteristicKey.equals(key);
-      }
-    }, null);
-
+    CharacteristicDto characteristicDto = findCharacteristic(key, characteristicDtos);
     if (characteristicDto == null) {
-      LOG.warn(String.format("Characteristic '%s' has not been found on rule '%s:%s'",
-        key, ruleDef.repository().name(), ruleDef.key()));
+      // Log a warning only if rule has not been overridden by user
+      if (overridingCharacteristicId == null) {
+        LOG.warn(String.format("Characteristic '%s' has not been found on rule '%s:%s'", key, ruleDef.repository().key(), ruleDef.key()));
+      }
     } else if (characteristicDto.getParentId() == null) {
-      throw MessageException.of(String.format("Rule '%s:%s' cannot be linked on the root characteristic '%s'",
-        ruleDef.repository().name(), ruleDef.key(), key));
+      throw MessageException.of(String.format("Rule '%s:%s' cannot be linked on the root characteristic '%s'", ruleDef.repository().key(), ruleDef.key(), key));
     }
     return characteristicDto;
   }
+
+  @CheckForNull
+  private CharacteristicDto findCharacteristic(final String key, List<CharacteristicDto> characteristicDtos) {
+    return Iterables.find(characteristicDtos, new Predicate<CharacteristicDto>() {
+      @Override
+      public boolean apply(CharacteristicDto input) {
+        return key.equals(input.getKey());
+      }
+    }, null);
+  }
 }
index 51fcac7192f14285119cae0f196eafb345a339fd..54232217ae2983d63b6136f17071030d2e9fd9a1 100644 (file)
@@ -34,7 +34,7 @@ import org.sonar.api.rules.Rule;
 import org.sonar.api.utils.Duration;
 import org.sonar.api.utils.System2;
 import org.sonar.core.persistence.Database;
-import org.sonar.core.rule.RuleDto;
+import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.core.technicaldebt.db.RequirementDao;
 import org.sonar.core.technicaldebt.db.RequirementDto;
 import org.sonar.server.db.migrations.MassUpdater;
@@ -174,7 +174,7 @@ public class CopyRequirementsFromCharacteristicsToRules {
 
       if (enabledRequirement == null && !Rule.STATUS_REMOVED.equals(ruleRow.getStatus())) {
         // If no requirements are enable, it means that the requirement has been disabled for this rule
-        updateStatement.setInt(1, RuleDto.DISABLED_CHARACTERISTIC_ID);
+        updateStatement.setInt(1, CharacteristicDto.DISABLED_CHARACTERISTIC_ID);
         updateStatement.setNull(2, Types.VARCHAR);
         updateStatement.setNull(3, Types.VARCHAR);
         updateStatement.setNull(4, Types.VARCHAR);
index e8bd583c621e498b47115ed80875bcb49981a416..40afa74f928f66db4888b053b9d49c7f2bbd4d35 100644 (file)
@@ -184,13 +184,35 @@ public class RuleRegistrationTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void set_no_characteristic_when_characteristic_not_found() {
+  public void set_no_default_characteristic_when_characteristic_not_found() {
     setupData("set_no_characteristic_when_characteristic_not_found");
+
     task.start();
+    // Warning log should be displayed
 
     checkTables("set_no_characteristic_when_characteristic_not_found", EXCLUDED_COLUMN_NAMES, "rules");
   }
 
+  @Test
+  public void set_no_default_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled() {
+    setupData("set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled");
+
+    task.start();
+    // No log should be displayed
+
+    checkTables("set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled", EXCLUDED_COLUMN_NAMES, "rules");
+  }
+
+  @Test
+  public void set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden() {
+    setupData("set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden");
+
+    task.start();
+    // No log should be displayed
+
+    checkTables("set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden", EXCLUDED_COLUMN_NAMES, "rules");
+  }
+
   @Test
   public void fail_when_rule_is_linked_on_root_characteristic() {
     setupData("ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic");
index 32fb4aae1773db76084b5fd04fdd8c9b698b6a98..a773983725fe4d736cd26dbbfcf5c589880af5cd 100644 (file)
@@ -3,10 +3,10 @@
   <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="config1" name="One" description="Description of One"
          status="READY" priority="4" cardinality="SINGLE" parent_id="[null]" language="java"
          characteristic_id="[null]" default_characteristic_id="[null]"
-         remediation_function="[null]" default_remediation_function="LINEAR_OFFSET"
-         remediation_factor="[null]" default_remediation_factor="5d"
-         remediation_offset="[null]" default_remediation_offset="10h"
-         effort_to_fix_l10n_key="squid.S115.effortToFix"/>
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
 
   <rules id="2" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="[null]" name="Two" description="Description of Two"
          status="DEPRECATED" priority="0" cardinality="SINGLE" parent_id="[null]" language="java"
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled-result.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled-result.xml
new file mode 100644 (file)
index 0000000..df6e027
--- /dev/null
@@ -0,0 +1,19 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="config1" name="One" description="Description of One"
+         status="READY" priority="4" cardinality="SINGLE" parent_id="[null]" language="java"
+         characteristic_id="-1" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+  <rules id="2" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="[null]" name="Two" description="Description of Two"
+         status="DEPRECATED" priority="0" cardinality="SINGLE" parent_id="[null]" language="java"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+</dataset>
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_characteristic_when_default_characteristic_not_found_and_overriding_characteristic_disabled.xml
new file mode 100644 (file)
index 0000000..69426df
--- /dev/null
@@ -0,0 +1,21 @@
+<dataset>
+
+  <characteristics id="999" kee="NEW" name="New" parent_id="1" characteristic_order="1" enabled="[true]"/>
+
+  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="old_config_key" name="old name" description="old description"
+         status="READY" priority="2" cardinality="SINGLE" parent_id="[null]"
+         characteristic_id="-1" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+  <rules id="2" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="old_config_key2" name="old name2" description="old description2"
+         status="READY" priority="1" cardinality="SINGLE" parent_id="[null]"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+</dataset>
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden-result.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden-result.xml
new file mode 100644 (file)
index 0000000..aa889da
--- /dev/null
@@ -0,0 +1,19 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="config1" name="One" description="Description of One"
+         status="READY" priority="4" cardinality="SINGLE" parent_id="[null]" language="java"
+         characteristic_id="3" default_characteristic_id="[null]"
+         remediation_function="LINEAR_OFFSET" default_remediation_function="[null]"
+         remediation_factor="5d" default_remediation_factor="[null]"
+         remediation_offset="10h" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+  <rules id="2" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="[null]" name="Two" description="Description of Two"
+         status="DEPRECATED" priority="0" cardinality="SINGLE" parent_id="[null]" language="java"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+</dataset>
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/set_no_default_characteristic_when_default_characteristic_not_found_but_characteristic_has_been_overridden.xml
new file mode 100644 (file)
index 0000000..cac7ab7
--- /dev/null
@@ -0,0 +1,23 @@
+<dataset>
+
+  <!-- User has removed the default characteristic used by rule1 and linked it to a a new characteristic-->
+  <characteristics id="2" kee="MEMORY_EFFICIENCY" name="Memory Efficiency" parent_id="1" characteristic_order="1" enabled="[false]"/>
+  <characteristics id="3" kee="COMPILER" name="Compiler" parent_id="1" characteristic_order="1" enabled="[true]"/>
+
+  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="old_config_key" name="old name" description="old description"
+         status="READY" priority="2" cardinality="SINGLE" parent_id="[null]"
+         characteristic_id="3" default_characteristic_id="[null]"
+         remediation_function="LINEAR_OFFSET" default_remediation_function="[null]"
+         remediation_factor="5d" default_remediation_factor="[null]"
+         remediation_offset="10h" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+  <rules id="2" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="old_config_key2" name="old name2" description="old description2"
+         status="READY" priority="1" cardinality="SINGLE" parent_id="[null]"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"/>
+
+</dataset>