Fix NPE when there's no unit on a requirement of a SQALE model

diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporter.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporter.java
index 817fe76a6ca29b6ddd689c97477fa6dcd67cfee5..e920d09a4cff9bc5c9637a5d8486b9f6d3488ec2 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporter.java
+++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporter.java
@@ -21,6 +21,7 @@
 package org.sonar.core.technicaldebt;
 
 import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.math.NumberUtils;
@@ -139,10 +140,10 @@ public class TechnicalDebtXMLImporter implements ServerExtension {
     return null;
   }
 
-  private void addRequirement(DefaultRequirement requirement, DefaultCharacteristic parent, ValidationMessages messages){
+  private void addRequirement(DefaultRequirement requirement, DefaultCharacteristic parent, ValidationMessages messages) {
     DefaultCharacteristic root = parent.parent();
     if (root == null) {
-      messages.addWarningText("Requirement '" + requirement.ruleKey()  + "' is ignored because it's defined directly under a root characteristic.");
+      messages.addWarningText("Requirement '" + requirement.ruleKey() + "' is ignored because it's defined directly under a root characteristic.");
     } else {
       requirement.setCharacteristic(parent);
       requirement.setRootCharacteristic(root);
@@ -236,11 +237,15 @@ public class TechnicalDebtXMLImporter implements ServerExtension {
       requirement.setFunction(function.getTextValue());
       if (factor != null) {
         requirement.setFactorValue(factor.getValue());
-        requirement.setFactorUnit(DefaultRequirement.toUnit(factor.getTextValue()));
+        if (!Strings.isNullOrEmpty(factor.getTextValue())) {
+          requirement.setFactorUnit(DefaultRequirement.toUnit(factor.getTextValue()));
+        }
       }
       if (offset != null) {
         requirement.setOffsetValue(offset.getValue());
-        requirement.setOffsetUnit(DefaultRequirement.toUnit(offset.getTextValue()));
+        if (!Strings.isNullOrEmpty(offset.getTextValue())) {
+          requirement.setOffsetUnit(DefaultRequirement.toUnit(offset.getTextValue()));
+        }
       }
       return requirement;
     }

diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java
index ae72bbf0e817c8335c6e63c3bc3bc2c6cdaca2a8..d2c76956d1b58c507e9e806ec645acf6aa5ffaea 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java
+++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java
@@ -270,11 +270,11 @@ public class CharacteristicDto implements Serializable {
 
   public static String fromUnit(@Nullable WorkDuration.UNIT unit) {
     if (unit != null) {
-      if (unit.equals(WorkDuration.UNIT.DAYS)) {
+      if (WorkDuration.UNIT.DAYS.equals(unit)) {
         return DAYS;
-      } else if (unit.equals(WorkDuration.UNIT.HOURS)) {
+      } else if (WorkDuration.UNIT.HOURS.equals(unit)) {
         return HOURS;
-      } else if (unit.equals(WorkDuration.UNIT.MINUTES)) {
+      } else if (WorkDuration.UNIT.MINUTES.equals(unit)) {
         return MINUTES;
       }
       throw new IllegalStateException("Invalid unit : " + unit);

diff --git a/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest.java b/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest.java
index 05138280dbf911d7fe9d78a74539e7f590743833..1b5133a94fbfac632bde36f65256f63803382a6b 100644 (file)
--- a/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest.java
@@ -68,6 +68,23 @@ public class TechnicalDebtXMLImporterTest {
     assertThat(sqale.characteristicByKey("READABILITY").parent().key()).isEqualTo("MAINTAINABILITY");
   }
 
+  @Test
+  public void use_default_unit_when_no_unit() {
+    TechnicalDebtRuleCache technicalDebtRuleCache = mockRuleCache();
+
+    String xml = getFileContent("use_default_unit_when_no_unit.xml");
+
+    ValidationMessages messages = ValidationMessages.create();
+    DefaultTechnicalDebtModel sqale = new TechnicalDebtXMLImporter().importXML(xml, messages, technicalDebtRuleCache);
+
+    DefaultCharacteristic memoryEfficiency = sqale.characteristicByKey("MEMORY_EFFICIENCY");
+    DefaultRequirement requirement = memoryEfficiency.requirements().get(0);
+    assertThat(requirement.factorValue()).isEqualTo(3);
+    assertThat(requirement.factorUnit()).isEqualTo(WorkDuration.UNIT.DAYS);
+    assertThat(requirement.offsetValue()).isEqualTo(1);
+    assertThat(requirement.offsetUnit()).isEqualTo(WorkDuration.UNIT.DAYS);
+  }
+
   @Test
   public void import_xml_with_linear_function() {
     TechnicalDebtRuleCache technicalDebtRuleCache = mockRuleCache();

diff --git a/sonar-core/src/test/resources/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest/use_default_unit_when_no_unit.xml b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest/use_default_unit_when_no_unit.xml
new file mode 100644 (file)
index 0000000..f83ca79
--- /dev/null
+++ b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest/use_default_unit_when_no_unit.xml
@@ -0,0 +1,52 @@
+<!--
+  ~ SonarQube, open source software quality management tool.
+  ~ Copyright (C) 2008-2013 SonarSource
+  ~ mailto:contact AT sonarsource DOT com
+  ~
+  ~ SonarQube is free software; you can redistribute it and/or
+  ~ modify it under the terms of the GNU Lesser General Public
+  ~ License as published by the Free Software Foundation; either
+  ~ version 3 of the License, or (at your option) any later version.
+  ~
+  ~ SonarQube is distributed in the hope that it will be useful,
+  ~ but WITHOUT ANY WARRANTY; without even the implied warranty of
+  ~ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  ~ Lesser General Public License for more details.
+  ~
+  ~ You should have received a copy of the GNU Lesser General Public License
+  ~ along with this program; if not, write to the Free Software Foundation,
+  ~ Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+  -->
+
+<sqale>
+  <chc>
+    <key>USABILITY</key>
+    <name>Usability</name>
+    <desc>Estimate usability</desc>
+  </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>3.0</val>
+        </prop>
+        <prop>
+          <key>remediationFunction</key>
+          <txt>linear</txt>
+        </prop>
+        <prop>
+          <key>offset</key>
+          <val>1.0</val>
+        </prop>
+      </chc>
+    </chc>
+  </chc>
+
+</sqale>

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/batch/internal/DefaultRequirement.java b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/batch/internal/DefaultRequirement.java
index c8828e61eb9c51fb6a879bfede459818035ff8ed..a028a567e73484c39a0b673262a5f2470ff16db0 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/batch/internal/DefaultRequirement.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/batch/internal/DefaultRequirement.java
@@ -191,22 +191,22 @@ public class DefaultRequirement implements Requirement {
   }
 
   public static WorkDuration.UNIT toUnit(String requirementUnit){
-    if (requirementUnit.equals(WorkUnit.DAYS)) {
+    if (WorkUnit.DAYS.equals(requirementUnit)) {
       return WorkDuration.UNIT.DAYS;
-    } else if (requirementUnit.equals(WorkUnit.HOURS)) {
+    } else if (WorkUnit.HOURS.equals(requirementUnit)) {
       return WorkDuration.UNIT.HOURS;
-    } else if (requirementUnit.equals(WorkUnit.MINUTES)) {
+    } else if (WorkUnit.MINUTES.equals(requirementUnit)) {
       return WorkDuration.UNIT.MINUTES;
     }
     throw new IllegalStateException("Invalid unit : " + requirementUnit);
   }
 
   private static String fromUnit(WorkDuration.UNIT unit){
-    if (unit.equals(WorkDuration.UNIT.DAYS)) {
+    if (WorkDuration.UNIT.DAYS.equals(unit)) {
       return WorkUnit.DAYS;
-    } else if (unit.equals(WorkDuration.UNIT.HOURS)) {
+    } else if (WorkDuration.UNIT.HOURS.equals(unit)) {
       return WorkUnit.HOURS;
-    } else if (unit.equals(WorkDuration.UNIT.MINUTES)) {
+    } else if (WorkDuration.UNIT.MINUTES.equals(unit)) {
       return WorkUnit.MINUTES;
     }
     throw new IllegalStateException("Invalid unit : " + unit);

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java
index c27f94cab74bbba2caa2d9325e1cdbdf71f1f825..f5a107452b33cacdb814dde0c874c279027a146b 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java
@@ -216,11 +216,11 @@ public class DefaultCharacteristic implements Characteristic {
 
   public static WorkDuration.UNIT toUnit(@Nullable String requirementUnit) {
     if (requirementUnit != null) {
-      if (requirementUnit.equals(WorkUnit.DAYS)) {
+      if (WorkUnit.DAYS.equals(requirementUnit)) {
         return WorkDuration.UNIT.DAYS;
-      } else if (requirementUnit.equals(WorkUnit.HOURS)) {
+      } else if (WorkUnit.HOURS.equals(requirementUnit)) {
         return WorkDuration.UNIT.HOURS;
-      } else if (requirementUnit.equals(WorkUnit.MINUTES)) {
+      } else if (WorkUnit.MINUTES.equals(requirementUnit)) {
         return WorkDuration.UNIT.MINUTES;
       }
       throw new IllegalStateException("Invalid unit : " + requirementUnit);
@@ -229,11 +229,11 @@ public class DefaultCharacteristic implements Characteristic {
   }
 
   private static String fromUnit(WorkDuration.UNIT unit){
-    if (unit.equals(WorkDuration.UNIT.DAYS)) {
+    if (WorkDuration.UNIT.DAYS.equals(unit)) {
       return WorkUnit.DAYS;
-    } else if (unit.equals(WorkDuration.UNIT.HOURS)) {
+    } else if (WorkDuration.UNIT.HOURS.equals(unit)) {
       return WorkUnit.HOURS;
-    } else if (unit.equals(WorkDuration.UNIT.MINUTES)) {
+    } else if (WorkDuration.UNIT.MINUTES.equals(unit)) {
       return WorkUnit.MINUTES;
     }
     throw new IllegalStateException("Invalid unit : " + unit);