SONAR-5056 Create DebtRemediationFunction API on batch side

4 files changed, 47 insertions, 76 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
index 106cf3bada2..751aa64eedd 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
@@ -21,13 +21,9 @@ package org.sonar.batch.issue;
 
 import com.google.common.base.Objects;
 import com.google.common.base.Strings;
-import org.sonar.api.batch.rule.ActiveRule;
-import org.sonar.api.batch.rule.ActiveRules;
-import org.sonar.api.batch.rule.Rule;
-import org.sonar.api.batch.rule.Rules;
+import org.sonar.api.batch.rule.*;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.resources.Project;
-import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Violation;
 import org.sonar.api.utils.Duration;
@@ -111,25 +107,25 @@ public class ModuleIssues {
     if (issue.severity() == null) {
       issue.setSeverity(activeRule.severity());
     }
-    if (rule.characteristic() != null) {
+    if (rule.debtCharacteristic() != null) {
       issue.setDebt(calculateDebt(rule, issue.effortToFix()));
     }
   }
 
   private Duration calculateDebt(Rule rule, @Nullable Double effortToFix) {
-    if (RemediationFunction.CONSTANT_ISSUE.equals(rule.function()) && effortToFix != null) {
+    DebtRemediationFunction function = rule.debtRemediationFunction();
+    if (DebtRemediationFunction.Type.CONSTANT_ISSUE.equals(function.type()) && effortToFix != null) {
       throw new IllegalArgumentException("Rule '" + rule.key() + "' can not use 'Constant/issue' remediation function " +
         "because this rule does not have a fixed remediation cost.");
     }
     Duration result = Duration.create(0);
-    Duration factor = rule.factor();
-    Duration offset = rule.offset();
+    Duration factor = function.factor();
+    Duration offset = function.offset();
 
     if (factor != null) {
       int effortToFixValue = Objects.firstNonNull(effortToFix, 1).intValue();
       result = factor.multiply(effortToFixValue);
     }
-
     if (offset != null) {
       result = result.add(offset);
     }
diff --git a/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java b/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java
index 5732f129663..2d0f1294e8d 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/rule/RulesProvider.java
@@ -25,10 +25,10 @@ import com.google.common.collect.ListMultimap;
 import org.picocontainer.injectors.ProviderAdapter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.sonar.api.batch.rule.DebtRemediationFunction;
 import org.sonar.api.batch.rule.Rules;
 import org.sonar.api.batch.rule.internal.NewRule;
 import org.sonar.api.batch.rule.internal.RulesBuilder;
-import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.technicaldebt.batch.Characteristic;
@@ -80,8 +80,8 @@ public class RulesProvider extends ProviderAdapter {
       // TODO should we set metadata ?
 
       if (ruleDto.hasCharacteristic()) {
-        newRule.setCharacteristic(characteristic(ruleDto, ruleKey, debtModel).key());
-        setFunction(ruleDto, newRule, ruleKey, durations);
+        newRule.setDebtCharacteristic(effectiveCharacteristic(ruleDto, ruleKey, debtModel).key());
+        newRule.setDebtRemediationFunction(effectiveFunction(ruleDto, ruleKey, newRule, durations));
       }
 
       for (RuleParamDto ruleParamDto : paramDtosByRuleId.get(ruleDto.getId())) {
@@ -92,27 +92,7 @@ public class RulesProvider extends ProviderAdapter {
     return rulesBuilder.build();
   }
 
-  private void setFunction(RuleDto ruleDto, NewRule newRule, RuleKey ruleKey, Durations durations) {
-    String function = ruleDto.getRemediationFunction();
-    String factor = ruleDto.getRemediationFactor();
-    String offset = ruleDto.getRemediationOffset();
-
-    String defaultFunction = ruleDto.getDefaultRemediationFunction();
-    String defaultFactor = ruleDto.getDefaultRemediationFactor();
-    String defaultOffset = ruleDto.getDefaultRemediationOffset();
-
-    if (function != null) {
-      newRule.setFunction(function(function, ruleKey));
-      newRule.setFactor(factor != null ? durations.decode(factor) : null);
-      newRule.setOffset(offset != null ? durations.decode(offset) : null);
-    } else {
-      newRule.setFunction(function(defaultFunction, ruleKey));
-      newRule.setFactor(defaultFactor != null ? durations.decode(defaultFactor) : null);
-      newRule.setOffset(defaultOffset != null ? durations.decode(defaultOffset) : null);
-    }
-  }
-
-  private Characteristic characteristic(RuleDto ruleDto, RuleKey ruleKey, TechnicalDebtModel debtModel) {
+  private Characteristic effectiveCharacteristic(RuleDto ruleDto, RuleKey ruleKey, TechnicalDebtModel debtModel) {
     Integer characteristicId = ruleDto.getCharacteristicId();
     Integer defaultCharacteristicId = ruleDto.getDefaultCharacteristicId();
     Integer effectiveCharacteristicId = characteristicId != null ? characteristicId : defaultCharacteristicId;
@@ -123,10 +103,22 @@ public class RulesProvider extends ProviderAdapter {
     return characteristic;
   }
 
-  private RemediationFunction function(@Nullable String function, RuleKey ruleKey) {
-    if (function == null) {
+  private DebtRemediationFunction effectiveFunction(RuleDto ruleDto, RuleKey ruleKey, NewRule newRule, Durations durations) {
+    String function = ruleDto.getRemediationFunction();
+    String defaultFunction = ruleDto.getDefaultRemediationFunction();
+    if (function != null) {
+      return createDebtRemediationFunction(function, ruleDto.getRemediationFactor(), ruleDto.getRemediationOffset(), durations);
+    } else if (defaultFunction != null) {
+      return createDebtRemediationFunction(ruleDto.getDefaultRemediationFunction(), ruleDto.getDefaultRemediationFactor(), ruleDto.getDefaultRemediationOffset(), durations);
+    } else {
       throw new IllegalStateException(String.format("Remediation function should not be null on rule '%s'", ruleKey));
     }
-    return RemediationFunction.valueOf(function);
   }
+
+  private DebtRemediationFunction createDebtRemediationFunction(String function, @Nullable String factor, @Nullable String offset, Durations durations) {
+    return DebtRemediationFunction.create(DebtRemediationFunction.Type.valueOf(function),
+      factor != null ? durations.decode(factor) : null,
+      offset != null ? durations.decode(offset) : null);
+  }
+
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
index 492b4061b1d..f5b920c3b3c 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
@@ -26,13 +26,13 @@ import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.sonar.api.batch.rule.DebtRemediationFunction;
 import org.sonar.api.batch.rule.internal.ActiveRulesBuilder;
 import org.sonar.api.batch.rule.internal.RulesBuilder;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.resources.JavaFile;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
-import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.rules.RulePriority;
@@ -250,9 +250,8 @@ public class ModuleIssuesTest {
   public void set_debt_with_linear_function() throws Exception {
     ruleBuilder.add(SQUID_RULE_KEY)
       .setName(SQUID_RULE_NAME)
-      .setCharacteristic("COMPILER_RELATED_PORTABILITY")
-      .setFunction(RemediationFunction.LINEAR)
-      .setFactor(Duration.create(10L));
+      .setDebtCharacteristic("COMPILER_RELATED_PORTABILITY")
+      .setDebtRemediationFunction(DebtRemediationFunction.createLinear(Duration.create(10L)));
     activeRulesBuilder.activate(SQUID_RULE_KEY).setSeverity(Severity.INFO);
     initModuleIssues();
 
@@ -277,10 +276,8 @@ public class ModuleIssuesTest {
   public void set_debt_with_linear_with_offset_function() throws Exception {
     ruleBuilder.add(SQUID_RULE_KEY)
       .setName(SQUID_RULE_NAME)
-      .setCharacteristic("COMPILER_RELATED_PORTABILITY")
-      .setFunction(RemediationFunction.LINEAR_OFFSET)
-      .setFactor(Duration.create(10L))
-      .setOffset(Duration.create(25L));
+      .setDebtCharacteristic("COMPILER_RELATED_PORTABILITY")
+      .setDebtRemediationFunction(DebtRemediationFunction.createLinearWithOffset(Duration.create(10L), Duration.create(25L)));
     activeRulesBuilder.activate(SQUID_RULE_KEY).setSeverity(Severity.INFO);
     initModuleIssues();
 
@@ -305,9 +302,8 @@ public class ModuleIssuesTest {
   public void set_debt_with_constant_issue_function() throws Exception {
     ruleBuilder.add(SQUID_RULE_KEY)
       .setName(SQUID_RULE_NAME)
-      .setCharacteristic("COMPILER_RELATED_PORTABILITY")
-      .setFunction(RemediationFunction.CONSTANT_ISSUE)
-      .setOffset(Duration.create(10L));
+      .setDebtCharacteristic("COMPILER_RELATED_PORTABILITY")
+      .setDebtRemediationFunction(DebtRemediationFunction.createConstantPerIssue(Duration.create(10L)));
     activeRulesBuilder.activate(SQUID_RULE_KEY).setSeverity(Severity.INFO);
     initModuleIssues();
 
@@ -332,9 +328,8 @@ public class ModuleIssuesTest {
   public void fail_to_set_debt_with_constant_issue_function_when_effort_to_fix_is_set() throws Exception {
     ruleBuilder.add(SQUID_RULE_KEY)
       .setName(SQUID_RULE_NAME)
-      .setCharacteristic("COMPILER_RELATED_PORTABILITY")
-      .setFunction(RemediationFunction.CONSTANT_ISSUE)
-      .setOffset(Duration.create(25L));
+      .setDebtCharacteristic("COMPILER_RELATED_PORTABILITY")
+      .setDebtRemediationFunction(DebtRemediationFunction.createConstantPerIssue(Duration.create(25L)));
     activeRulesBuilder.activate(SQUID_RULE_KEY).setSeverity(Severity.INFO);
     initModuleIssues();
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java b/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java
index 01108161c10..d87e6b3189b 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/rule/RulesProviderTest.java
@@ -25,11 +25,11 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.sonar.api.batch.rule.DebtRemediationFunction;
 import org.sonar.api.batch.rule.Rule;
 import org.sonar.api.batch.rule.RuleParam;
 import org.sonar.api.batch.rule.Rules;
 import org.sonar.api.config.Settings;
-import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.technicaldebt.batch.internal.DefaultCharacteristic;
@@ -111,10 +111,8 @@ public class RulesProviderTest extends AbstractDaoTestCase {
     Rules rules = provider.provide(ruleDao, debtModel, durations);
 
     Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
-    assertThat(rule.characteristic()).isEqualTo("EFFICIENCY");
-    assertThat(rule.function()).isEqualTo(RemediationFunction.LINEAR_OFFSET);
-    assertThat(rule.factor()).isEqualTo(Duration.decode("5d", 8));
-    assertThat(rule.offset()).isEqualTo(Duration.decode("10h", 8));
+    assertThat(rule.debtCharacteristic()).isEqualTo("EFFICIENCY");
+    assertThat(rule.debtRemediationFunction()).isEqualTo(DebtRemediationFunction.createLinearWithOffset(Duration.decode("5d", 8), Duration.decode("10h", 8)));
   }
 
   @Test
@@ -124,10 +122,8 @@ public class RulesProviderTest extends AbstractDaoTestCase {
     Rules rules = provider.provide(ruleDao, debtModel, durations);
 
     Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
-    assertThat(rule.characteristic()).isEqualTo("PORTABILITY");
-    assertThat(rule.function()).isEqualTo(RemediationFunction.LINEAR);
-    assertThat(rule.factor()).isEqualTo(Duration.decode("2h", 8));
-    assertThat(rule.offset()).isNull();
+    assertThat(rule.debtCharacteristic()).isEqualTo("PORTABILITY");
+    assertThat(rule.debtRemediationFunction()).isEqualTo(DebtRemediationFunction.createLinear(Duration.decode("2h", 8)));
   }
 
   @Test
@@ -138,10 +134,8 @@ public class RulesProviderTest extends AbstractDaoTestCase {
 
     // As both default columns and user columns on debt are set, user debt columns should be used
     Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
-    assertThat(rule.characteristic()).isEqualTo("PORTABILITY");
-    assertThat(rule.function()).isEqualTo(RemediationFunction.LINEAR);
-    assertThat(rule.factor()).isEqualTo(Duration.decode("2h", 8));
-    assertThat(rule.offset()).isNull();
+    assertThat(rule.debtCharacteristic()).isEqualTo("PORTABILITY");
+    assertThat(rule.debtRemediationFunction()).isEqualTo(DebtRemediationFunction.createLinear(Duration.decode("2h", 8)));
   }
 
   @Test
@@ -152,10 +146,8 @@ public class RulesProviderTest extends AbstractDaoTestCase {
 
     // As both default columns and user columns on debt are set, user debt columns should be used
     Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
-    assertThat(rule.characteristic()).isEqualTo("PORTABILITY");
-    assertThat(rule.function()).isEqualTo(RemediationFunction.LINEAR);
-    assertThat(rule.factor()).isEqualTo(Duration.decode("2h", 8));
-    assertThat(rule.offset()).isNull();
+    assertThat(rule.debtCharacteristic()).isEqualTo("PORTABILITY");
+    assertThat(rule.debtRemediationFunction()).isEqualTo(DebtRemediationFunction.createLinear(Duration.decode("2h", 8)));
   }
 
   @Test
@@ -166,10 +158,8 @@ public class RulesProviderTest extends AbstractDaoTestCase {
 
     // As both default columns and user columns on debt are set, user debt columns should be used
     Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
-    assertThat(rule.characteristic()).isEqualTo("PORTABILITY");
-    assertThat(rule.function()).isEqualTo(RemediationFunction.LINEAR);
-    assertThat(rule.factor()).isEqualTo(Duration.decode("2h", 8));
-    assertThat(rule.offset()).isNull();
+    assertThat(rule.debtCharacteristic()).isEqualTo("PORTABILITY");
+    assertThat(rule.debtRemediationFunction()).isEqualTo(DebtRemediationFunction.createLinear(Duration.decode("2h", 8)));
   }
 
   @Test
@@ -179,10 +169,8 @@ public class RulesProviderTest extends AbstractDaoTestCase {
     Rules rules = provider.provide(ruleDao, debtModel, durations);
 
     Rule rule = rules.find(RuleKey.of("checkstyle", "AvoidNull"));
-    assertThat(rule.characteristic()).isNull();
-    assertThat(rule.function()).isNull();
-    assertThat(rule.factor()).isNull();
-    assertThat(rule.offset()).isNull();
+    assertThat(rule.debtCharacteristic()).isNull();
+    assertThat(rule.debtRemediationFunction()).isNull();
   }
 
   @Test