SONAR-5360 When creating a custom rule, the rule key should be set

9 files changed, 132 insertions, 14 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/NewRule.java b/sonar-server/src/main/java/org/sonar/server/rule/NewRule.java
index c5f11cb43a0..4ae05bf8632 100644
--- a/sonar-server/src/main/java/org/sonar/server/rule/NewRule.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/NewRule.java
@@ -30,11 +30,21 @@ import java.util.Map;
 
 public class NewRule {
 
+  private String ruleKey;
   private RuleKey templateKey;
   private String name, htmlDescription, severity;
   private RuleStatus status;
   private final Map<String, String> parameters = Maps.newHashMap();
 
+  public String ruleKey() {
+    return ruleKey;
+  }
+
+  public NewRule setRuleKey(String ruleKey) {
+    this.ruleKey = ruleKey;
+    return this;
+  }
+
   @CheckForNull
   public RuleKey templateKey() {
     return templateKey;
diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleCreator.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleCreator.java
index e49f8315d7e..a2bd1f5733a 100644
--- a/sonar-server/src/main/java/org/sonar/server/rule/RuleCreator.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleCreator.java
@@ -24,7 +24,6 @@ import com.google.common.base.Strings;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
-import org.sonar.api.utils.System2;
 import org.sonar.check.Cardinality;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.rule.RuleDto;
@@ -34,11 +33,9 @@ import org.sonar.server.db.DbClient;
 public class RuleCreator implements ServerComponent {
 
   private final DbClient dbClient;
-  private final System2 system;
 
-  public RuleCreator(DbClient dbClient, System2 system) {
+  public RuleCreator(DbClient dbClient) {
     this.dbClient = dbClient;
-    this.system = system;
   }
 
   public RuleKey create(NewRule newRule) {
@@ -46,12 +43,15 @@ public class RuleCreator implements ServerComponent {
     try {
       RuleKey templateKey = newRule.templateKey();
       if (templateKey != null) {
-        RuleDto templateRule = dbClient.ruleDao().getByKey(dbSession, newRule.templateKey());
+        RuleDto templateRule = dbClient.ruleDao().getByKey(dbSession, templateKey);
         if (!Cardinality.MULTIPLE.equals(templateRule.getCardinality())) {
           throw new IllegalArgumentException("This rule is not a template rule: " + templateKey.toString());
         }
         validateRule(newRule);
-        RuleKey customRuleKey = createCustomRule(newRule, templateRule, dbSession);
+
+        RuleKey customRuleKey = RuleKey.of(templateRule.getRepositoryKey(), newRule.ruleKey());
+        checkRuleKeyUnicity(customRuleKey, dbSession);
+        createCustomRule(customRuleKey, newRule, templateRule, dbSession);
         dbSession.commit();
         return customRuleKey;
       }
@@ -62,6 +62,7 @@ public class RuleCreator implements ServerComponent {
   }
 
   private static void validateRule(NewRule newRule) {
+    validateRuleKey(newRule.ruleKey());
     if (Strings.isNullOrEmpty(newRule.name())) {
       throw new IllegalArgumentException("The name is missing");
     }
@@ -79,8 +80,23 @@ public class RuleCreator implements ServerComponent {
     }
   }
 
-  private RuleKey createCustomRule(NewRule newRule, RuleDto templateRuleDto, DbSession dbSession){
-    RuleKey ruleKey = RuleKey.of(templateRuleDto.getRepositoryKey(), templateRuleDto.getRuleKey() + "_" + system.now());
+  private static void validateRuleKey(String ruleKey) {
+    if (Strings.isNullOrEmpty(ruleKey)) {
+      throw new IllegalArgumentException("The rule key is missing");
+    } else {
+      if (!ruleKey.matches("^[\\w]+$")) {
+        throw new IllegalArgumentException(String.format("The rule key '%s' is invalid, it should only contains : a-z, 0-9, '_'", ruleKey));
+      }
+    }
+  }
+
+  private void checkRuleKeyUnicity(RuleKey ruleKey, DbSession dbSession){
+    if (dbClient.ruleDao().getNullableByKey(dbSession, ruleKey) != null) {
+      throw new IllegalArgumentException(String.format("A rule with the key '%s' already exits", ruleKey.rule()));
+    }
+  }
+
+  private RuleKey createCustomRule(RuleKey ruleKey, NewRule newRule, RuleDto templateRuleDto, DbSession dbSession){
     RuleDto ruleDto = RuleDto.createFor(ruleKey)
       .setParentId(templateRuleDto.getId())
       .setConfigKey(templateRuleDto.getConfigKey())
diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java b/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
index 91ada56961f..0046a95e645 100644
--- a/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
@@ -39,6 +39,7 @@ import org.sonar.server.search.BaseDoc;
  */
 public class CreateAction implements RequestHandler {
 
+  public static final String PARAM_KEY = "key";
   public static final String PARAM_NAME = "name";
   public static final String PARAM_DESCRIPTION = "html_description";
   public static final String PARAM_SEVERITY = "severity";
@@ -63,6 +64,11 @@ public class CreateAction implements RequestHandler {
       .setHandler(this);
 
     action
+      .createParam(PARAM_KEY)
+      .setDescription("Key of the rule")
+      .setExampleValue("Todo_should_not_be_used");
+
+    action
       .createParam(PARAM_TEMPLATE_KEY)
       .setDescription("Key of the template rule in order to create a custom rule")
       .setExampleValue("java:XPath");
@@ -100,6 +106,7 @@ public class CreateAction implements RequestHandler {
   public void handle(Request request, Response response) {
     String templateRuleKey = request.param(PARAM_TEMPLATE_KEY);
     NewRule newRule = new NewRule()
+      .setRuleKey(request.mandatoryParam(PARAM_KEY))
       .setTemplateKey(templateRuleKey != null ? RuleKey.parse(templateRuleKey) : null)
       .setName(request.mandatoryParam(PARAM_NAME))
       .setHtmlDescription(request.mandatoryParam(PARAM_DESCRIPTION))
diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleCreatorMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleCreatorMediumTest.java
index 4849a182ea2..8f02e61607b 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/RuleCreatorMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleCreatorMediumTest.java
@@ -71,6 +71,7 @@ public class RuleCreatorMediumTest {
 
     // Create custom rule
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setName("My custom")
       .setHtmlDescription("Some description")
@@ -83,6 +84,7 @@ public class RuleCreatorMediumTest {
 
     RuleDto rule = db.ruleDao().getNullableByKey(dbSession, customRuleKey);
     assertThat(rule).isNotNull();
+    assertThat(rule.getKey()).isEqualTo(RuleKey.of("java", "CUSTOM_RULE"));
     assertThat(rule.getParentId()).isEqualTo(templateRule.getId());
     assertThat(rule.getName()).isEqualTo("My custom");
     assertThat(rule.getDescription()).isEqualTo("Some description");
@@ -111,12 +113,88 @@ public class RuleCreatorMediumTest {
   }
 
   @Test
+  public void fail_to_create_custom_rule_when_missing_key() throws Exception {
+    // insert template rule
+    RuleDto templateRule = createTemplateRule();
+
+    NewRule newRule = new NewRule()
+      .setName("My custom")
+      .setTemplateKey(templateRule.getKey())
+      .setHtmlDescription("Some description")
+      .setSeverity(Severity.MAJOR)
+      .setStatus(RuleStatus.READY)
+      .setParameters(ImmutableMap.of("regex", "a.*"));
+
+    try {
+      creator.create(newRule);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("The rule key is missing");
+    }
+  }
+
+  @Test
+  public void fail_to_create_custom_rule_when_invalid_key() throws Exception {
+    // insert template rule
+    RuleDto templateRule = createTemplateRule();
+
+    NewRule newRule = new NewRule()
+      .setRuleKey("*INVALID*")
+      .setName("My custom")
+      .setTemplateKey(templateRule.getKey())
+      .setHtmlDescription("Some description")
+      .setSeverity(Severity.MAJOR)
+      .setStatus(RuleStatus.READY)
+      .setParameters(ImmutableMap.of("regex", "a.*"));
+
+    try {
+      creator.create(newRule);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("The rule key '*INVALID*' is invalid, it should only contains : a-z, 0-9, '_'");
+    }
+  }
+
+  @Test
+  public void fail_to_create_custom_rule_when_rule_key_already_exits() throws Exception {
+    // insert template rule
+    RuleDto templateRule = createTemplateRule();
+
+    // Create a custom rule
+    NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
+      .setName("My custom")
+      .setTemplateKey(templateRule.getKey())
+      .setHtmlDescription("Some description")
+      .setSeverity(Severity.MAJOR)
+      .setStatus(RuleStatus.READY)
+      .setParameters(ImmutableMap.of("regex", "a.*"));
+    creator.create(newRule);
+
+    try {
+      // Create another custom rule having same key
+      newRule = new NewRule()
+        .setRuleKey("CUSTOM_RULE")
+        .setName("My another custom")
+        .setTemplateKey(templateRule.getKey())
+        .setHtmlDescription("Some description")
+        .setSeverity(Severity.MAJOR)
+        .setStatus(RuleStatus.READY)
+        .setParameters(ImmutableMap.of("regex", "a.*"));
+      creator.create(newRule);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("A rule with the key 'CUSTOM_RULE' already exits");
+    }
+  }
+
+  @Test
   public void fail_to_create_custom_rule_when_missing_name() throws Exception {
     // insert template rule
     RuleDto templateRule = createTemplateRule();
 
-    // Create custom rule without name
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setHtmlDescription("Some description")
       .setSeverity(Severity.MAJOR)
@@ -136,8 +214,8 @@ public class RuleCreatorMediumTest {
     // insert template rule
     RuleDto templateRule = createTemplateRule();
 
-    // Create custom rule without description
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setName("My custom")
       .setSeverity(Severity.MAJOR)
@@ -157,8 +235,8 @@ public class RuleCreatorMediumTest {
     // insert template rule
     RuleDto templateRule = createTemplateRule();
 
-    // Create custom rule without description
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setName("My custom")
       .setHtmlDescription("Some description")
@@ -178,8 +256,8 @@ public class RuleCreatorMediumTest {
     // insert template rule
     RuleDto templateRule = createTemplateRule();
 
-    // Create custom rule without description
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setName("My custom")
       .setHtmlDescription("Some description")
@@ -200,8 +278,8 @@ public class RuleCreatorMediumTest {
     // insert template rule
     RuleDto templateRule = createTemplateRule();
 
-    // Create custom rule without description
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setName("My custom")
       .setHtmlDescription("Some description")
@@ -226,6 +304,7 @@ public class RuleCreatorMediumTest {
 
     // Create custom rule with unknown template rule
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(rule.getKey())
       .setName("My custom")
       .setHtmlDescription("Some description")
@@ -246,8 +325,8 @@ public class RuleCreatorMediumTest {
     // insert template rule
     RuleDto templateRule = createTemplateRule();
 
-    // Create custom rule
     NewRule newRule = new NewRule()
+      .setRuleKey("CUSTOM_RULE")
       .setTemplateKey(templateRule.getKey())
       .setName("My custom")
       .setHtmlDescription("Some description")
diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
index 0f5b80eb841..87c1d753bef 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
@@ -139,6 +139,7 @@ public class RuleServiceMediumTest {
 
     // Create custom rule
     NewRule newRule = new NewRule()
+      .setRuleKey("MY_CUSTOM")
       .setTemplateKey(templateRuleKey)
       .setName("My custom")
       .setHtmlDescription("Some description")
@@ -173,6 +174,7 @@ public class RuleServiceMediumTest {
 
     // Create custom rule
     NewRule newRule = new NewRule()
+      .setRuleKey("MY_CUSTOM")
       .setTemplateKey(templateRuleKey)
       .setName("My custom")
       .setHtmlDescription("Some description")
diff --git a/sonar-server/src/test/java/org/sonar/server/rule/ws/CreateActionMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/ws/CreateActionMediumTest.java
index 923f36e4ab4..4627de3acb6 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/ws/CreateActionMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/ws/CreateActionMediumTest.java
@@ -79,6 +79,7 @@ public class CreateActionMediumTest {
     session.commit();
 
     WsTester.TestRequest request = wsTester.newGetRequest("api/rules", "create")
+      .setParam("key", "MY_CUSTOM")
       .setParam("template_key", templateRule.getKey().toString())
       .setParam("name", "My custom rule")
       .setParam("html_description", "Description")
diff --git a/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java
index e2f054094a0..caefdad1159 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/ws/UpdateActionMediumTest.java
@@ -84,6 +84,7 @@ public class UpdateActionMediumTest {
 
     // Custom rule
     NewRule newRule = new NewRule()
+      .setRuleKey("MY_CUSTOM")
       .setTemplateKey(templateRule.getKey())
       .setName("Old custom")
       .setHtmlDescription("Old description")
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_custom_rule.json b/sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_custom_rule.json
index 53d351c6790..648e6c9b2fe 100644
--- a/sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_custom_rule.json
+++ b/sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_custom_rule.json
@@ -1,5 +1,6 @@
 {
   "rule": {
+    "key": "java:MY_CUSTOM",
     "repo": "java",
     "name": "My custom rule",
     "htmlDesc": "Description",
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ws/UpdateActionMediumTest/update_custom_rule.json b/sonar-server/src/test/resources/org/sonar/server/rule/ws/UpdateActionMediumTest/update_custom_rule.json
index 53d351c6790..648e6c9b2fe 100644
--- a/sonar-server/src/test/resources/org/sonar/server/rule/ws/UpdateActionMediumTest/update_custom_rule.json
+++ b/sonar-server/src/test/resources/org/sonar/server/rule/ws/UpdateActionMediumTest/update_custom_rule.json
@@ -1,5 +1,6 @@
 {
   "rule": {
+    "key": "java:MY_CUSTOM",
     "repo": "java",
     "name": "My custom rule",
     "htmlDesc": "Description",