SONAR-5056 Set Rule remediation factor and offset as String

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleDefinitions.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleDefinitions.java
index cbb5f213d1b4eabbea800a0ab9bd94b3403a4b92..a3d952ea567fa0d16bd722678051ab79a3837adf 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleDefinitions.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleDefinitions.java
@@ -389,13 +389,13 @@ public interface RuleDefinitions extends ServerExtension {
 
     public NewRule setRemediationFactor(@Nullable String remediationFactor) {
       // TODO validate format
-      this.remediationFactor = remediationFactor;
+      this.remediationFactor = StringUtils.deleteWhitespace(remediationFactor);
       return this;
     }
 
     public NewRule setRemediationOffset(@Nullable String remediationOffset) {
       // TODO validate format
-      this.remediationOffset = remediationOffset;
+      this.remediationOffset = StringUtils.deleteWhitespace(remediationOffset);
       return this;
     }
 

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleDefinitionsTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleDefinitionsTest.java
index a8c57ad30e0e0465a6801e3acca2890fe19123c8..e169b50a7d9a8e45d13d79b3b3f640a76c7ef0af 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleDefinitionsTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleDefinitionsTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.api.server.rule;
 
+import org.junit.Ignore;
 import org.junit.Test;
 import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleStatus;
@@ -167,6 +168,21 @@ public class RuleDefinitionsTest {
     assertThat(rule.name()).isEqualTo("NullPointer");
   }
 
+  @Test
+  public void sanitize_remediation_factor_and_offset() {
+    RuleDefinitions.NewRepository newFindbugs = context.newRepository("findbugs", "java");
+    newFindbugs.newRule("NPE")
+      .setName("Detect NPE")
+      .setHtmlDescription("NPE")
+      .setRemediationFactor("   1   h   ")
+      .setRemediationOffset(" 10  mi n ");
+    newFindbugs.done();
+
+    RuleDefinitions.Rule npeRule = context.repository("findbugs").rule("NPE");
+    assertThat(npeRule.remediationFactor()).isEqualTo("1h");
+    assertThat(npeRule.remediationOffset()).isEqualTo("10min");
+  }
+
   @Test
   public void extend_repository() {
     assertThat(context.extendedRepositories()).isEmpty();
@@ -301,4 +317,23 @@ public class RuleDefinitionsTest {
       assertThat(e).hasMessage("Status 'REMOVED' is not accepted on rule '[repository=findbugs, key=NPE]'");
     }
   }
+
+  @Test
+  @Ignore("TODO")
+  public void fail_if_bad_remediation_factor_or_offset() {
+    try {
+      context.newRepository("findbugs", "java").newRule("NPE").setRemediationFactor("ten hours");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Duration 'ten hours' is invalid, it should use the following sample format : 2d 10h 15min");
+    }
+
+    try {
+      context.newRepository("findbugs", "java").newRule("NPE").setRemediationOffset("ten hours");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Duration 'ten hours' is invalid, it should use the following sample format : 2d 10h 15min");
+    }
+  }
+
 }

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
index 3356625a8690132c37b488946ac1e10a6a3e0004..2ab48037f8e99ffc1d665204ea4fd9bbf6bd8d8b 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
@@ -225,6 +225,19 @@ public class RuleRegistration implements Startable {
       dto.setStatus(status);
       changed = true;
     }
+    if (!StringUtils.equals(dto.getLanguage(), def.repository().language())) {
+      dto.setLanguage(def.repository().language());
+      changed = true;
+    }
+    changed = mergeDebtRule(def, dto) || changed;
+    if (changed) {
+      dto.setUpdatedAt(buffer.now());
+    }
+    return changed;
+  }
+
+  private boolean mergeDebtRule(RuleDefinitions.Rule def, RuleDto dto){
+    boolean changed = false;
 
     // TODO add characteristic id change verification
 
@@ -246,14 +259,6 @@ public class RuleRegistration implements Startable {
       dto.setEffortToFixL10nKey(def.effortToFixL10nKey());
       changed = true;
     }
-
-    if (!StringUtils.equals(dto.getLanguage(), def.repository().language())) {
-      dto.setLanguage(def.repository().language());
-      changed = true;
-    }
-    if (changed) {
-      dto.setUpdatedAt(buffer.now());
-    }
     return changed;
   }