From a774a62aec4318d3db04aaa8bc0a4cf741959f03 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 11 Jun 2014 10:43:46 +0200 Subject: [PATCH] SONAR-5360 When creating a custom rule, the rule key should be set --- .../java/org/sonar/server/rule/NewRule.java | 10 ++ .../org/sonar/server/rule/RuleCreator.java | 32 +++++-- .../sonar/server/rule/ws/CreateAction.java | 7 ++ .../server/rule/RuleCreatorMediumTest.java | 91 +++++++++++++++++-- .../server/rule/RuleServiceMediumTest.java | 2 + .../rule/ws/CreateActionMediumTest.java | 1 + .../rule/ws/UpdateActionMediumTest.java | 1 + .../create_custom_rule.json | 1 + .../update_custom_rule.json | 1 + 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 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"; @@ -62,6 +63,11 @@ public class CreateAction implements RequestHandler { .setPost(true) .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") @@ -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"); @@ -110,13 +112,89 @@ public class RuleCreatorMediumTest { assertThat(param.getDefaultValue()).isEqualTo("a.*"); } + @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", -- 2.39.5