From 886c70d2d5cc2898f5d5230f70b9e02d6ca0f94b Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 18 Feb 2014 08:39:56 +0100 Subject: [PATCH] Fix NPE when there's no unit on a requirement of a SQALE model --- .../TechnicalDebtXMLImporter.java | 13 +++-- .../technicaldebt/db/CharacteristicDto.java | 6 +-- .../TechnicalDebtXMLImporterTest.java | 17 ++++++ .../use_default_unit_when_no_unit.xml | 52 +++++++++++++++++++ .../batch/internal/DefaultRequirement.java | 12 ++--- .../internal/DefaultCharacteristic.java | 12 ++--- 6 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 sonar-core/src/test/resources/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest/use_default_unit_when_no_unit.xml 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 817fe76a6ca..e920d09a4cf 100644 --- 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 ae72bbf0e81..d2c76956d1b 100644 --- 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 05138280dbf..1b5133a94fb 100644 --- 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 index 00000000000..f83ca7952da --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/TechnicalDebtXMLImporterTest/use_default_unit_when_no_unit.xml @@ -0,0 +1,52 @@ + + + + + USABILITY + Usability + Estimate usability + + + EFFICIENCY + Efficiency + + MEMORY_EFFICIENCY + Memory use + + checkstyle + Regexp + + remediationFactor + 3.0 + + + remediationFunction + linear + + + offset + 1.0 + + + + + + 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 c8828e61eb9..a028a567e73 100644 --- 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 c27f94cab74..f5a107452b3 100644 --- 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); -- 2.39.5