SONAR-6873 Ignore quality model having zero remediation cost during import

3 files changed, 128 insertions, 53 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/debt/DebtRulesXMLImporter.java b/server/sonar-server/src/main/java/org/sonar/server/debt/DebtRulesXMLImporter.java
index 91ebb132d9f..94dc86b1c77 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/debt/DebtRulesXMLImporter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/debt/DebtRulesXMLImporter.java
@@ -23,14 +23,6 @@ package org.sonar.server.debt;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
-import java.io.Reader;
-import java.io.StringReader;
-import java.util.List;
-import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-import javax.xml.stream.XMLInputFactory;
-import javax.xml.stream.XMLStreamException;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.math.NumberUtils;
 import org.codehaus.stax2.XMLInputFactory2;
@@ -39,23 +31,30 @@ import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.server.ServerSide;
-import org.sonar.api.server.debt.DebtRemediationFunction;
 import org.sonar.api.utils.Duration;
 import org.sonar.api.utils.ValidationMessages;
-import org.sonar.server.debt.DebtModelXMLExporter.RuleDebt;
+import org.sonar.server.debt.DebtModelXMLExporter.*;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+
+import java.io.Reader;
+import java.io.StringReader;
+import java.util.List;
 
 import static com.google.common.collect.Lists.newArrayList;
-import static org.sonar.server.debt.DebtModelXMLExporter.CHARACTERISTIC;
-import static org.sonar.server.debt.DebtModelXMLExporter.CHARACTERISTIC_KEY;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY_COEFFICIENT;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY_FUNCTION;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY_KEY;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY_OFFSET;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY_TEXT_VALUE;
-import static org.sonar.server.debt.DebtModelXMLExporter.PROPERTY_VALUE;
-import static org.sonar.server.debt.DebtModelXMLExporter.REPOSITORY_KEY;
-import static org.sonar.server.debt.DebtModelXMLExporter.RULE_KEY;
+import static java.lang.Boolean.FALSE;
+import static java.lang.Boolean.TRUE;
+import static javax.xml.stream.XMLInputFactory.*;
+import static org.apache.commons.lang.StringUtils.isNotBlank;
+import static org.sonar.api.server.debt.DebtRemediationFunction.Type.CONSTANT_ISSUE;
+import static org.sonar.api.server.debt.DebtRemediationFunction.Type.LINEAR;
+import static org.sonar.api.utils.Duration.MINUTE;
+import static org.sonar.server.debt.DebtCharacteristicsXMLImporter.convertKey;
+import static org.sonar.server.debt.DebtModelXMLExporter.*;
 
 /**
  * Import rules debt definitions from an XML
@@ -89,10 +88,10 @@ public class DebtRulesXMLImporter {
 
   private static SMInputFactory initStax() {
     XMLInputFactory xmlFactory = XMLInputFactory2.newInstance();
-    xmlFactory.setProperty(XMLInputFactory.IS_COALESCING, Boolean.TRUE);
-    xmlFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, Boolean.FALSE);
-    xmlFactory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
-    xmlFactory.setProperty(XMLInputFactory.IS_VALIDATING, Boolean.FALSE);
+    xmlFactory.setProperty(IS_COALESCING, TRUE);
+    xmlFactory.setProperty(IS_NAMESPACE_AWARE, FALSE);
+    xmlFactory.setProperty(SUPPORT_DTD, FALSE);
+    xmlFactory.setProperty(IS_VALIDATING, FALSE);
     return new SMInputFactory(xmlFactory);
   }
 
@@ -110,7 +109,7 @@ public class DebtRulesXMLImporter {
         RuleDebt ruleDebt = processRule(validationMessages, cursor);
         if (ruleDebt != null && parentKey != null) {
           if (rootKey != null) {
-            ruleDebt.setSubCharacteristicKey(DebtCharacteristicsXMLImporter.convertKey(parentKey));
+            ruleDebt.setSubCharacteristicKey(convertKey(parentKey));
             ruleDebts.add(ruleDebt);
           } else {
             validationMessages.addWarningText("Rule '" + ruleDebt.ruleKey() + "' is ignored because it's defined directly under a root characteristic.");
@@ -134,7 +133,7 @@ public class DebtRulesXMLImporter {
         ruleKey = cursor.collectDescendantText().trim();
       }
     }
-    if (StringUtils.isNotBlank(ruleRepositoryKey) && StringUtils.isNotBlank(ruleKey)) {
+    if (isNotBlank(ruleRepositoryKey) && isNotBlank(ruleKey)) {
       return createRule(RuleKey.of(ruleRepositoryKey, ruleKey), properties, validationMessages);
     }
     return null;
@@ -160,7 +159,7 @@ public class DebtRulesXMLImporter {
         }
       } else if (StringUtils.equals(node, PROPERTY_TEXT_VALUE)) {
         textValue = c.collectDescendantText().trim();
-        textValue = "mn".equals(textValue) ? Duration.MINUTE : textValue;
+        textValue = "mn".equals(textValue) ? MINUTE : textValue;
       }
     }
     return new Property(key, value, textValue);
@@ -169,13 +168,11 @@ public class DebtRulesXMLImporter {
   @CheckForNull
   private static RuleDebt createRule(RuleKey ruleKey, Properties properties, ValidationMessages validationMessages) {
     Property function = properties.function();
-    if (function != null) {
-
-      Property coefficientProperty = properties.coefficient();
-      String coefficient = coefficientProperty != null ? coefficientProperty.toDuration() : null;
-      Property offsetProperty = properties.offset();
-      String offset = offsetProperty != null ? offsetProperty.toDuration() : null;
-
+    Property coefficientProperty = properties.coefficient();
+    String coefficient = coefficientProperty == null ? null : coefficientProperty.toDuration();
+    Property offsetProperty = properties.offset();
+    String offset = offsetProperty == null ? null : offsetProperty.toDuration();
+    if (function != null && (coefficient != null || offset != null)) {
       return createRuleDebt(ruleKey, function.getTextValue(), coefficient, offset, validationMessages);
     }
     return null;
@@ -183,28 +180,29 @@ public class DebtRulesXMLImporter {
 
   @CheckForNull
   private static RuleDebt createRuleDebt(RuleKey ruleKey, String function, @Nullable String coefficient, @Nullable String offset, ValidationMessages validationMessages) {
+    if ("constant_resource".equals(function)) {
+      validationMessages.addWarningText(String.format("Constant/file function is no longer used, technical debt definitions on '%s' are ignored.", ruleKey));
+      return null;
+    }
     if ("linear_threshold".equals(function) && coefficient != null) {
       validationMessages.addWarningText(String.format("Linear with threshold function is no longer used, remediation function of '%s' is replaced by linear.", ruleKey));
-      return new RuleDebt().setRuleKey(ruleKey).setFunction(DebtRemediationFunction.Type.LINEAR.name()).setCoefficient(coefficient);
-    } else if ("constant_resource".equals(function)) {
-      validationMessages.addWarningText(String.format("Constant/file function is no longer used, technical debt definitions on '%s' are ignored.", ruleKey));
-    } else if (DebtRemediationFunction.Type.CONSTANT_ISSUE.name().equalsIgnoreCase(function) && coefficient != null && offset == null) {
-      return new RuleDebt().setRuleKey(ruleKey).setFunction(DebtRemediationFunction.Type.CONSTANT_ISSUE.name()).setOffset(coefficient);
-    } else {
-      return new RuleDebt().setRuleKey(ruleKey).setFunction(function.toUpperCase()).setCoefficient(coefficient).setOffset(offset);
+      return createRuleDebt(ruleKey, LINEAR.name(), coefficient, null, validationMessages);
     }
-    return null;
+    if (CONSTANT_ISSUE.name().equalsIgnoreCase(function) && coefficient != null && offset == null) {
+      return createRuleDebt(ruleKey, CONSTANT_ISSUE.name(), null, coefficient, validationMessages);
+    }
+    return new RuleDebt().setRuleKey(ruleKey).setFunction(function.toUpperCase()).setCoefficient(coefficient).setOffset(offset);
   }
 
   private static class Properties {
-    List<Property> properties;
+    List<Property> list;
 
     public Properties() {
-      this.properties = newArrayList();
+      this.list = newArrayList();
     }
 
     public Properties add(Property property) {
-      this.properties.add(property);
+      this.list.add(property);
       return this;
     }
 
@@ -221,7 +219,7 @@ public class DebtRulesXMLImporter {
     }
 
     private Property find(String key) {
-      return Iterables.find(properties, new PropertyMathKey(key), null);
+      return Iterables.find(list, new PropertyMatchKey(key), null);
     }
   }
 
@@ -259,10 +257,10 @@ public class DebtRulesXMLImporter {
     }
   }
 
-  private static class PropertyMathKey implements Predicate<Property> {
+  private static class PropertyMatchKey implements Predicate<Property> {
     private final String key;
 
-    public PropertyMathKey(String key) {
+    public PropertyMatchKey(String key) {
       this.key = key;
     }
 
@@ -272,5 +270,4 @@ public class DebtRulesXMLImporter {
     }
   }
 
-
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/debt/DebtRulesXMLImporterTest.java b/server/sonar-server/src/test/java/org/sonar/server/debt/DebtRulesXMLImporterTest.java
index 233d486c2b0..66bc76acbe2 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/debt/DebtRulesXMLImporterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/debt/DebtRulesXMLImporterTest.java
@@ -21,14 +21,13 @@
 package org.sonar.server.debt;
 
 import com.google.common.io.Resources;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
 import org.junit.Test;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.server.debt.DebtRemediationFunction;
 import org.sonar.api.utils.ValidationMessages;
 
-import java.nio.charset.StandardCharsets;
-import java.util.List;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static org.sonar.server.debt.DebtModelXMLExporter.RuleDebt;
@@ -181,6 +180,14 @@ public class DebtRulesXMLImporterTest {
   }
 
   @Test
+  public void ignore_remediation_cost_having_zero_value() throws Exception {
+    String xml = getFileContent("ignore_remediation_cost_having_zero_value.xml");
+
+    List<RuleDebt> results = importer.importXML(xml, validationMessages);
+    assertThat(results).isEmpty();
+  }
+
+  @Test
   public void ignore_deprecated_constant_per_file_function() throws Exception {
     String xml = getFileContent("ignore_deprecated_constant_per_file_function.xml");
 
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/debt/DebtRulesXMLImporterTest/ignore_remediation_cost_having_zero_value.xml b/server/sonar-server/src/test/resources/org/sonar/server/debt/DebtRulesXMLImporterTest/ignore_remediation_cost_having_zero_value.xml
new file mode 100644
index 00000000000..2165a603341
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/debt/DebtRulesXMLImporterTest/ignore_remediation_cost_having_zero_value.xml
@@ -0,0 +1,71 @@
+<sqale>
+  <chc>
+    <key>USABILITY</key>
+    <name>Usability</name>
+    <desc>Estimate usability</desc>
+    <chc>
+      <rule-repo>squid</rule-repo>
+      <rule-key>S001</rule-key>
+      <prop>
+        <key>offset</key>
+        <val>0.0</val>
+        <txt>min</txt>
+      </prop>
+      <prop>
+        <key>remediationFunction</key>
+        <txt>constant_issue</txt>
+      </prop>
+    </chc>
+  </chc>
+  <chc>
+    <key>EFFICIENCY</key>
+    <name>Efficiency</name>
+    <chc>
+      <key>MEMORY_EFFICIENCY</key>
+      <name>Memory use</name>
+      <chc>
+        <rule-repo>checkstyle</rule-repo>
+        <rule-key>Regexp</rule-key>
+        <prop>
+          <key>remediationFactor</key>
+          <val>0.0</val>
+          <txt>h</txt>
+        </prop>
+        <prop>
+          <key>remediationFunction</key>
+          <txt>linear</txt>
+        </prop>
+      </chc>
+    </chc>
+  </chc>
+  <chc>
+    <key>PORTABILITY</key>
+    <name>Portability</name>
+    <chc>
+      <key>COMPILER_RELATED_PORTABILITY</key>
+      <name>Compiler related portability</name>
+    </chc>
+    <chc>
+      <key>HARDWARE_RELATED_PORTABILITY</key>
+      <name>Hardware related portability</name>
+      <chc>
+        <rule-repo>checkstyle</rule-repo>
+        <rule-key>Regexp2</rule-key>
+        <prop>
+          <key>remediationFunction</key>
+          <txt>linear_offset</txt>
+        </prop>
+        <prop>
+          <key>remediationFactor</key>
+          <val>0.0</val>
+          <txt>d</txt>
+        </prop>
+        <prop>
+          <key>offset</key>
+          <val>0.0</val>
+          <txt>d</txt>
+        </prop>
+      </chc>
+    </chc>
+  </chc>
+</sqale>