Fix quality flaw

diff --git a/sonar-batch/src/main/java/org/sonar/batch/debt/IssueChangelogDebtCalculator.java b/sonar-batch/src/main/java/org/sonar/batch/debt/IssueChangelogDebtCalculator.java
index 1ce0e75482a023bc395f16fa9f93fa76ec734900..1e9ea1f8f8ab906f601025aa51893ca3773a0658 100644 (file)
--- a/sonar-batch/src/main/java/org/sonar/batch/debt/IssueChangelogDebtCalculator.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/debt/IssueChangelogDebtCalculator.java
@@ -58,7 +58,7 @@ public class IssueChangelogDebtCalculator implements BatchComponent {
   }
 
   @CheckForNull
-  private WorkDuration calculateNewTechnicalDebtValueFromChangelog(WorkDuration currentTechnicalDebtValue, Issue issue, Date periodDate) {
+  private WorkDuration calculateNewTechnicalDebtValueFromChangelog(@Nullable WorkDuration currentTechnicalDebtValue, Issue issue, Date periodDate) {
     List<FieldDiffs> changelog = technicalDebtHistory(issue);
     for (Iterator<FieldDiffs> iterator = changelog.iterator(); iterator.hasNext(); ) {
       FieldDiffs diff = iterator.next();
@@ -81,12 +81,15 @@ public class IssueChangelogDebtCalculator implements BatchComponent {
   /**
    * SONAR-5059
    */
-  private WorkDuration subtractNeverNegative(WorkDuration workDuration, WorkDuration toSubtractWith){
-    WorkDuration result = workDuration.subtract(toSubtractWith);
-    if (result.toSeconds() > 0) {
-      return result;
+  @CheckForNull
+  private WorkDuration subtractNeverNegative(@Nullable WorkDuration workDuration, WorkDuration toSubtractWith){
+    if (workDuration != null) {
+      WorkDuration result = workDuration.subtract(toSubtractWith);
+      if (result.toSeconds() > 0) {
+        return result;
+      }
     }
-    return workDurationFactory.createFromWorkingLong(0L);
+    return null;
   }
 
   private List<FieldDiffs> technicalDebtHistory(Issue issue) {

diff --git a/sonar-batch/src/test/java/org/sonar/batch/debt/IssueChangelogDebtCalculatorTest.java b/sonar-batch/src/test/java/org/sonar/batch/debt/IssueChangelogDebtCalculatorTest.java
new file mode 100644 (file)
index 0000000..b13c093
--- /dev/null
+++ b/sonar-batch/src/test/java/org/sonar/batch/debt/IssueChangelogDebtCalculatorTest.java
@@ -0,0 +1,163 @@
+/*
+ * 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.
+ */
+
+package org.sonar.batch.debt;
+
+import org.apache.commons.lang.time.DateUtils;
+import org.junit.Before;
+import org.junit.Test;
+import org.sonar.api.CoreProperties;
+import org.sonar.api.config.Settings;
+import org.sonar.api.issue.Issue;
+import org.sonar.api.issue.internal.DefaultIssue;
+import org.sonar.api.issue.internal.FieldDiffs;
+import org.sonar.api.utils.WorkDuration;
+import org.sonar.api.utils.WorkDurationFactory;
+
+import java.util.Date;
+
+import static com.google.common.collect.Lists.newArrayList;
+import static org.fest.assertions.Assertions.assertThat;
+
+public class IssueChangelogDebtCalculatorTest {
+
+  private static final int HOURS_IN_DAY = 8;
+  IssueChangelogDebtCalculator issueChangelogDebtCalculator;
+
+  Date rightNow = new Date();
+  Date elevenDaysAgo = DateUtils.addDays(rightNow, -11);
+  Date tenDaysAgo = DateUtils.addDays(rightNow, -10);
+  Date nineDaysAgo = DateUtils.addDays(rightNow, -9);
+  Date fiveDaysAgo = DateUtils.addDays(rightNow, -5);
+  Date fourDaysAgo = DateUtils.addDays(rightNow, -4);
+
+  WorkDuration oneDaysDebt = WorkDuration.createFromValueAndUnit(1, WorkDuration.UNIT.DAYS, HOURS_IN_DAY);
+  WorkDuration twoDaysDebt = WorkDuration.createFromValueAndUnit(2, WorkDuration.UNIT.DAYS, HOURS_IN_DAY);
+  WorkDuration fiveDaysDebt = WorkDuration.createFromValueAndUnit(5, WorkDuration.UNIT.DAYS, HOURS_IN_DAY);
+
+  @Before
+  public void setUp() throws Exception {
+    Settings settings = new Settings();
+    settings.setProperty(CoreProperties.HOURS_IN_DAY, Integer.toString(HOURS_IN_DAY));
+
+    issueChangelogDebtCalculator = new IssueChangelogDebtCalculator(new WorkDurationFactory(settings));
+  }
+
+  @Test
+  public void calculate_new_technical_debt_with_one_diff_in_changelog() throws Exception {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo).setTechnicalDebt(twoDaysDebt).setChanges(
+      newArrayList(
+        // changelog created at is null because it has just been created on the current analysis
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(oneDaysDebt), fromWorkDayDuration(twoDaysDebt)).setCreationDate(null)
+      )
+    );
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, rightNow)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(1, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, fiveDaysAgo)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(1, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, elevenDaysAgo)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(2, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+  }
+
+  @Test
+  public void calculate_new_technical_debt_with_many_diffs_in_changelog() throws Exception {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges(
+      newArrayList(
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(twoDaysDebt), fromWorkDayDuration(fiveDaysDebt)).setCreationDate(null),
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(oneDaysDebt), fromWorkDayDuration(twoDaysDebt)).setCreationDate(fourDaysAgo)
+      )
+    );
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, rightNow)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(3, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, fiveDaysAgo)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(4, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, elevenDaysAgo)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(5, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+  }
+
+  @Test
+  public void changelog_can_be_in_wrong_order() {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges(
+      newArrayList(
+        // 3rd
+        new FieldDiffs().setDiff("technicalDebt", null, fromWorkDayDuration(oneDaysDebt)).setCreationDate(nineDaysAgo),
+        // 1st
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(twoDaysDebt), fromWorkDayDuration(fiveDaysDebt)).setCreationDate(rightNow),
+        // 2nd
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(oneDaysDebt), fromWorkDayDuration(twoDaysDebt)).setCreationDate(fourDaysAgo)
+      )
+    );
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, fiveDaysAgo)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(4, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, elevenDaysAgo)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(5, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+  }
+
+  @Test
+  public void calculate_new_technical_debt_with_null_date() throws Exception {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo).setTechnicalDebt(twoDaysDebt).setChanges(
+      newArrayList(
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(oneDaysDebt), fromWorkDayDuration(twoDaysDebt)).setCreationDate(null)
+      )
+    );
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, null)).isEqualTo(
+      WorkDuration.createFromValueAndUnit(2, WorkDuration.UNIT.DAYS, HOURS_IN_DAY));
+  }
+
+  @Test
+  public void calculate_new_technical_debt_when_new_debt_is_null() throws Exception {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo).setTechnicalDebt(null).setChanges(
+      newArrayList(
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(oneDaysDebt), null).setCreationDate(null),
+        new FieldDiffs().setDiff("technicalDebt", null, fromWorkDayDuration(oneDaysDebt)).setCreationDate(nineDaysAgo)
+      )
+    );
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, rightNow)).isNull();
+  }
+
+  @Test
+  public void calculate_new_technical_debt_on_issue_without_technical_debt_and_without_changelog() {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo);
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, rightNow)).isNull();
+  }
+
+  @Test
+  public void not_return_negative_debt() {
+    Issue issue = new DefaultIssue().setKey("A").setCreationDate(tenDaysAgo).setTechnicalDebt(oneDaysDebt).setChanges(
+      newArrayList(
+        new FieldDiffs().setDiff("technicalDebt", fromWorkDayDuration(twoDaysDebt), fromWorkDayDuration(oneDaysDebt)).setCreationDate(null)
+      )
+    );
+
+    assertThat(issueChangelogDebtCalculator.calculateNewTechnicalDebt(issue, rightNow)).isNull();
+  }
+
+  private Long fromWorkDayDuration(WorkDuration workDayDuration) {
+    return workDayDuration.toLong();
+  }
+
+}