]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5360 Add a parameter "prevent_reactivation" in order to not reactivate a remove...
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 16 Jun 2014 15:59:23 +0000 (17:59 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 16 Jun 2014 16:11:35 +0000 (18:11 +0200)
sonar-server/src/main/java/org/sonar/server/rule/NewRule.java
sonar-server/src/main/java/org/sonar/server/rule/ReactivationException.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/rule/RuleCreator.java
sonar-server/src/main/java/org/sonar/server/rule/ws/CreateAction.java
sonar-server/src/test/java/org/sonar/server/rule/RuleCreatorMediumTest.java
sonar-server/src/test/java/org/sonar/server/rule/ws/CreateActionMediumTest.java
sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_rule_with_prevent_reactivation_param_to_true.json [new file with mode: 0644]

index 3b2b2539a0aaaa78c11c5f72e1ac8a5d4597687d..f6c4cabaee5221e9278070468724cd2a0bdcc29f 100644 (file)
@@ -38,7 +38,7 @@ public class NewRule {
   private RuleStatus status;
   private final Map<String, String> parameters = Maps.newHashMap();
 
-  private boolean isCustomRule = false;
+  private boolean isCustom, isManual, preventReactivation = false;
 
   private NewRule() {
     // No direct call to constructor
@@ -110,19 +110,39 @@ public class NewRule {
     return this;
   }
 
+  public boolean isPreventReactivation() {
+    return preventReactivation;
+  }
+
+  /**
+   * When true, if the rule already exists in status REMOVED, an {@link ReactivationException} will be thrown
+   */
+  public NewRule setPreventReactivation(boolean preventReactivation) {
+    this.preventReactivation = preventReactivation;
+    return this;
+  }
+
   private void checkCustomRule(){
-    if (!isCustomRule) {
+    if (!isCustom) {
       throw new IllegalStateException("Not a custom rule");
     }
   }
 
+  public boolean isCustom() {
+    return isCustom;
+  }
+
+  public boolean isManual() {
+    return isManual;
+  }
+
   public static NewRule createForCustomRule(String customKey, RuleKey templateKey) {
     Preconditions.checkArgument(!Strings.isNullOrEmpty(customKey), "Custom key should be set");
     Preconditions.checkArgument(templateKey != null, "Template key should be set");
     NewRule newRule = new NewRule();
     newRule.ruleKey = customKey;
     newRule.templateKey = templateKey;
-    newRule.isCustomRule = true;
+    newRule.isCustom = true;
     return newRule;
   }
 
@@ -130,6 +150,7 @@ public class NewRule {
     Preconditions.checkArgument(!Strings.isNullOrEmpty(manualKey), "Manual key should be set");
     NewRule newRule = new NewRule();
     newRule.ruleKey = manualKey;
+    newRule.isManual = true;
     return newRule;
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ReactivationException.java b/sonar-server/src/main/java/org/sonar/server/rule/ReactivationException.java
new file mode 100644 (file)
index 0000000..06e3f09
--- /dev/null
@@ -0,0 +1,37 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 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.server.rule;
+
+import org.sonar.api.rule.RuleKey;
+
+public class ReactivationException extends RuntimeException {
+
+  private RuleKey ruleKey;
+
+  public ReactivationException(String s, RuleKey ruleKey) {
+    super(s);
+    this.ruleKey = ruleKey;
+  }
+
+  public RuleKey ruleKey() {
+    return ruleKey;
+  }
+}
index 48ad462dce329096e9f1371d44fcec7991ce80b7..7cd89b2811e6981962975473cf2d7d7442eabad2 100644 (file)
@@ -31,6 +31,8 @@ import org.sonar.core.rule.RuleParamDto;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.rule.index.RuleDoc;
 
+import javax.annotation.CheckForNull;
+
 public class RuleCreator implements ServerComponent {
 
   private final DbClient dbClient;
@@ -42,37 +44,60 @@ public class RuleCreator implements ServerComponent {
   public RuleKey create(NewRule newRule) {
     DbSession dbSession = dbClient.openSession(false);
     try {
-      RuleKey templateKey = newRule.templateKey();
-      // Creation of a custom rule
-      if (templateKey != null) {
-        RuleDto templateRule = dbClient.ruleDao().getByKey(dbSession, templateKey);
-        if (!templateRule.isTemplate()) {
-          throw new IllegalArgumentException("This rule is not a template rule: " + templateKey.toString());
-        }
-        validateCustomRule(newRule);
-
-        RuleKey customRuleKey = RuleKey.of(templateRule.getRepositoryKey(), newRule.ruleKey());
-        checkRuleKeyUnicity(customRuleKey, dbSession);
-        createCustomRule(customRuleKey, newRule, templateRule, dbSession);
-
-        dbSession.commit();
-        return customRuleKey;
-      } else {
-        // Creation of a manual rule
-        validateManualRule(newRule);
 
-        RuleKey customRuleKey = RuleKey.of(RuleDoc.MANUAL_REPOSITORY, newRule.ruleKey());
-        checkRuleKeyUnicity(customRuleKey, dbSession);
-        createManualRule(customRuleKey, newRule, dbSession);
+      if (newRule.isCustom()) {
+        return createCustomRule(newRule, dbSession);
+      }
 
-        dbSession.commit();
-        return customRuleKey;
+      if (newRule.isManual()) {
+        return createManualRule(newRule, dbSession);
       }
+
+      throw new IllegalStateException("Only custom rule and manual rule can be created");
     } finally {
       dbSession.close();
     }
   }
 
+  private RuleKey createCustomRule(NewRule newRule, DbSession dbSession){
+    RuleKey templateKey = newRule.templateKey();
+    if (templateKey == null) {
+      throw new IllegalArgumentException("Rule template key should not be null");
+    }
+    RuleDto templateRule = dbClient.ruleDao().getByKey(dbSession, templateKey);
+    if (!templateRule.isTemplate()) {
+      throw new IllegalArgumentException("This rule is not a template rule: " + templateKey.toString());
+    }
+    validateCustomRule(newRule);
+
+    RuleKey customRuleKey = RuleKey.of(templateRule.getRepositoryKey(), newRule.ruleKey());
+
+    RuleDto existingRule = loadRule(customRuleKey, dbSession);
+    if (existingRule != null) {
+      updateExistingRule(existingRule, newRule, dbSession);
+    } else {
+      createCustomRule(customRuleKey, newRule, templateRule, dbSession);
+    }
+
+    dbSession.commit();
+    return customRuleKey;
+  }
+
+  private RuleKey createManualRule(NewRule newRule, DbSession dbSession){
+    validateManualRule(newRule);
+
+    RuleKey customRuleKey = RuleKey.of(RuleDoc.MANUAL_REPOSITORY, newRule.ruleKey());
+    RuleDto existingRule = loadRule(customRuleKey, dbSession);
+    if (existingRule != null) {
+      updateExistingRule(existingRule, newRule, dbSession);
+    } else {
+      createManualRule(customRuleKey, newRule, dbSession);
+    }
+
+    dbSession.commit();
+    return customRuleKey;
+  }
+
   private static void validateCustomRule(NewRule newRule) {
     validateRuleKey(newRule.ruleKey());
     validateName(newRule);
@@ -117,10 +142,9 @@ public class RuleCreator implements ServerComponent {
     }
   }
 
-  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()));
-    }
+  @CheckForNull
+  private RuleDto loadRule(RuleKey ruleKey, DbSession dbSession){
+    return dbClient.ruleDao().getNullableByKey(dbSession, ruleKey);
   }
 
   private RuleKey createCustomRule(RuleKey ruleKey, NewRule newRule, RuleDto templateRuleDto, DbSession dbSession){
@@ -169,4 +193,17 @@ public class RuleCreator implements ServerComponent {
     return ruleKey;
   }
 
+  private void updateExistingRule(RuleDto ruleDto, NewRule newRule, DbSession dbSession){
+    if (ruleDto.getStatus().equals(RuleStatus.REMOVED)) {
+      if (newRule.isPreventReactivation()) {
+        throw new ReactivationException(String.format("A removed rule with the key '%s' already exits", ruleDto.getKey().rule()), ruleDto.getKey());
+      } else {
+        ruleDto.setStatus(RuleStatus.READY);
+        dbClient.ruleDao().update(dbSession, ruleDto);
+      }
+    } else {
+      throw new IllegalArgumentException(String.format("A rule with the key '%s' already exits", ruleDto.getKey().rule()));
+    }
+  }
+
 }
index 32a49c8aa6ae296a3ef3a3f959d426ad39face3d..3eb19d41e9d1c41982104158dc4d77bb96f3c12a 100644 (file)
@@ -30,11 +30,15 @@ import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.api.utils.text.JsonWriter;
 import org.sonar.server.exceptions.BadRequestException;
+import org.sonar.server.plugins.MimeTypes;
 import org.sonar.server.rule.NewRule;
+import org.sonar.server.rule.ReactivationException;
 import org.sonar.server.rule.Rule;
 import org.sonar.server.rule.RuleService;
 import org.sonar.server.search.BaseDoc;
 
+import java.io.OutputStreamWriter;
+
 /**
  * @since 4.4
  */
@@ -49,6 +53,8 @@ public class CreateAction implements RequestHandler {
   public static final String PARAM_TEMPLATE_KEY = "template_key";
   public static final String PARAMS = "params";
 
+  public static final String PARAM_PREVENT_REACTIVATION = "prevent_reactivation";
+
   private final RuleService service;
   private final RuleMapping mapping;
 
@@ -105,6 +111,12 @@ public class CreateAction implements RequestHandler {
 
     action.createParam(PARAMS)
       .setDescription("Parameters as semi-colon list of <key>=<value>, for example 'params=key1=v1;key2=v2' (Only for custom rule)");
+
+    action
+      .createParam(PARAM_PREVENT_REACTIVATION)
+      .setDescription("If set to true and if the rule has been deactivated (status 'REMOVED'), a status 409 will be returned")
+      .setDefaultValue(false)
+      .setBooleanPossibleValues();
   }
 
   @Override
@@ -115,25 +127,31 @@ public class CreateAction implements RequestHandler {
       throw new BadRequestException(String.format("Either '%s' or '%s' parameters should be set", PARAM_CUSTOM_KEY, PARAM_MANUAL_KEY));
     }
 
-    if (!Strings.isNullOrEmpty(customKey)) {
-      NewRule newRule = NewRule.createForCustomRule(customKey, RuleKey.parse(request.mandatoryParam(PARAM_TEMPLATE_KEY)))
-        .setName(request.mandatoryParam(PARAM_NAME))
-        .setHtmlDescription(request.mandatoryParam(PARAM_DESCRIPTION))
-        .setSeverity(request.mandatoryParam(PARAM_SEVERITY))
-        .setStatus(RuleStatus.valueOf(request.mandatoryParam(PARAM_STATUS)));
-      String params = request.param(PARAMS);
-      if (!Strings.isNullOrEmpty(params)) {
-        newRule.setParameters(KeyValueFormat.parse(params));
+    try {
+      if (!Strings.isNullOrEmpty(customKey)) {
+        NewRule newRule = NewRule.createForCustomRule(customKey, RuleKey.parse(request.mandatoryParam(PARAM_TEMPLATE_KEY)))
+          .setName(request.mandatoryParam(PARAM_NAME))
+          .setHtmlDescription(request.mandatoryParam(PARAM_DESCRIPTION))
+          .setSeverity(request.mandatoryParam(PARAM_SEVERITY))
+          .setStatus(RuleStatus.valueOf(request.mandatoryParam(PARAM_STATUS)))
+          .setPreventReactivation(request.paramAsBoolean(PARAM_PREVENT_REACTIVATION));
+        String params = request.param(PARAMS);
+        if (!Strings.isNullOrEmpty(params)) {
+          newRule.setParameters(KeyValueFormat.parse(params));
+        }
+        writeResponse(response, service.create(newRule));
       }
-      writeResponse(response, service.create(newRule));
-    }
 
-    if (!Strings.isNullOrEmpty(manualKey)) {
-      NewRule newRule = NewRule.createForManualRule(manualKey)
-        .setName(request.mandatoryParam(PARAM_NAME))
-        .setHtmlDescription(request.mandatoryParam(PARAM_DESCRIPTION))
-        .setSeverity(request.param(PARAM_SEVERITY));
-      writeResponse(response, service.create(newRule));
+      if (!Strings.isNullOrEmpty(manualKey)) {
+        NewRule newRule = NewRule.createForManualRule(manualKey)
+          .setName(request.mandatoryParam(PARAM_NAME))
+          .setHtmlDescription(request.mandatoryParam(PARAM_DESCRIPTION))
+          .setSeverity(request.param(PARAM_SEVERITY))
+          .setPreventReactivation(request.paramAsBoolean(PARAM_PREVENT_REACTIVATION));
+        writeResponse(response, service.create(newRule));
+      }
+    } catch (ReactivationException e) {
+      write409(response, e.ruleKey());
     }
   }
 
@@ -143,4 +161,15 @@ public class CreateAction implements RequestHandler {
     mapping.write((BaseDoc) rule, json);
     json.endObject().close();
   }
+
+  private void write409(Response response, RuleKey ruleKey) {
+    Rule rule = service.getByKey(ruleKey);
+
+    Response.Stream stream = response.stream();
+    stream.setStatus(409);
+    stream.setMediaType(MimeTypes.JSON);
+    JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output())).beginObject().name("rule");
+    mapping.write((BaseDoc) rule, json);
+    json.endObject().close();
+  }
 }
index 9dfd4e6fbebd70a041dc5b547a789f4baa3ac33c..cac6742e1b4d5e08937f7d2576c47e32267fee5e 100644 (file)
@@ -111,6 +111,86 @@ public class RuleCreatorMediumTest {
     assertThat(param.getDefaultValue()).isEqualTo("a.*");
   }
 
+  @Test
+  public void reactivate_custom_rule_if_already_exists_in_removed_status() throws Exception {
+    String key = "CUSTOM_RULE";
+
+    // insert template rule
+    RuleDto templateRule = createTemplateRule();
+
+    // insert a removed rule
+    RuleDto rule = dao.insert(dbSession, RuleTesting.newCustomRule(templateRule)
+      .setRuleKey(key)
+      .setStatus(RuleStatus.REMOVED)
+      .setName("Old name")
+      .setDescription("Old description")
+      .setSeverity(Severity.INFO));
+    dao.addRuleParam(dbSession, rule, dao.findRuleParamsByRuleKey(dbSession, templateRule.getKey()).get(0).setDefaultValue("a.*"));
+    dbSession.commit();
+    dbSession.clearCache();
+
+    // Create custom rule with same key, but with different values
+    NewRule newRule = NewRule.createForCustomRule(key, templateRule.getKey())
+      .setName("New name")
+      .setHtmlDescription("New description")
+      .setSeverity(Severity.MAJOR)
+      .setStatus(RuleStatus.READY)
+      .setParameters(ImmutableMap.of("regex", "c.*"));
+    RuleKey customRuleKey = creator.create(newRule);
+
+    dbSession.clearCache();
+
+    Rule result = ruleIndex.getByKey(customRuleKey);
+    assertThat(result.key()).isEqualTo(RuleKey.of("java", key));
+    assertThat(result.status()).isEqualTo(RuleStatus.READY);
+
+    // These values should be the same than before
+    assertThat(result.name()).isEqualTo("Old name");
+    assertThat(result.htmlDescription()).isEqualTo("Old description");
+    assertThat(result.severity()).isEqualTo(Severity.INFO);
+    assertThat(result.param("regex").defaultValue()).isEqualTo("a.*");
+
+    // Check that the id is the same
+    assertThat(db.ruleDao().getByKey(dbSession, result.key()).getId()).isEqualTo(rule.getId());
+  }
+
+  @Test
+  public void generate_reactivation_exception_when_rule_exists_in_removed_status_and_prevent_reactivation_parameter_is_true() throws Exception {
+    String key = "CUSTOM_RULE";
+
+    // insert template rule
+    RuleDto templateRule = createTemplateRule();
+
+    // insert a removed rule
+    RuleDto rule = dao.insert(dbSession, RuleTesting.newCustomRule(templateRule)
+      .setRuleKey(key)
+      .setStatus(RuleStatus.REMOVED)
+      .setName("Old name")
+      .setDescription("Old description")
+      .setSeverity(Severity.INFO));
+    dao.addRuleParam(dbSession, rule, dao.findRuleParamsByRuleKey(dbSession, templateRule.getKey()).get(0).setDefaultValue("a.*"));
+    dbSession.commit();
+    dbSession.clearCache();
+
+    // Create custom rule with same key, but with different values
+    NewRule newRule = NewRule.createForCustomRule(key, templateRule.getKey())
+      .setName("New name")
+      .setHtmlDescription("New description")
+      .setSeverity(Severity.MAJOR)
+      .setStatus(RuleStatus.READY)
+      .setParameters(ImmutableMap.of("regex", "c.*"))
+      .setPreventReactivation(true);
+
+    try {
+      creator.create(newRule);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(ReactivationException.class);
+      ReactivationException reactivationException = (ReactivationException) e;
+      assertThat(reactivationException.ruleKey()).isEqualTo(rule.getKey());
+    }
+  }
+
   @Test
   public void not_fail_to_create_custom_rule_when_a_param_is_missing() throws Exception {
     // insert template rule
@@ -366,6 +446,40 @@ public class RuleCreatorMediumTest {
     assertThat(rule.params()).isEmpty();
   }
 
+  @Test
+  public void reactivate_manual_rule_if_already_exists_in_removed_status() throws Exception {
+    String key = "MANUAL_RULE";
+
+    // insert a removed rule
+    RuleDto rule = dao.insert(dbSession, RuleTesting.newManualRule(key)
+      .setStatus(RuleStatus.REMOVED)
+      .setName("Old name")
+      .setDescription("Old description")
+      .setSeverity(Severity.INFO));
+    dbSession.commit();
+    dbSession.clearCache();
+
+    // Create a rule with the same key and with another name, description and severity
+    NewRule newRule = NewRule.createForManualRule(key)
+      .setName("New name")
+      .setHtmlDescription("New description");
+    RuleKey ruleKey = creator.create(newRule);
+
+    dbSession.clearCache();
+
+    Rule result = ruleIndex.getByKey(ruleKey);
+    assertThat(result.key()).isEqualTo(RuleKey.of("manual", key));
+    assertThat(result.status()).isEqualTo(RuleStatus.READY);
+
+    // Name, description and severity should be the same than before
+    assertThat(result.name()).isEqualTo("Old name");
+    assertThat(result.htmlDescription()).isEqualTo("Old description");
+    assertThat(result.severity()).isEqualTo(Severity.INFO);
+
+    // Check that the id is the same
+    assertThat(db.ruleDao().getByKey(dbSession, result.key()).getId()).isEqualTo(rule.getId());
+  }
+
   @Test
   public void fail_to_create_manual_rule_when_missing_key() throws Exception {
     try {
index 1c84857326bc4b453f31124318c407162fa14b64..f1060f762df1d05ff51bcbc7e2660c94e73eed8e 100644 (file)
@@ -27,6 +27,8 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.rule.RuleKey;
+import org.sonar.api.rule.RuleStatus;
+import org.sonar.api.rule.Severity;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.rule.RuleDto;
@@ -139,4 +141,32 @@ public class CreateActionMediumTest {
     }
   }
 
+  @Test
+  public void create_manual_rule_with_prevent_reactivation_param_to_true() throws Exception {
+    MockUserSession.set()
+      .setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)
+      .setLogin("me");
+
+    String key = "MY_MANUAL";
+
+    // insert a removed rule
+    tester.get(RuleDao.class).insert(session, RuleTesting.newManualRule(key)
+      .setStatus(RuleStatus.REMOVED)
+      .setName("My manual rule")
+      .setDescription("Description")
+      .setSeverity(Severity.MAJOR));
+    session.commit();
+    session.clearCache();
+
+    WsTester.TestRequest request = wsTester.newGetRequest("api/rules", "create")
+      .setParam("manual_key", key)
+      .setParam("name", "My manual rule")
+      .setParam("html_description", "Description")
+      .setParam("severity", "MAJOR")
+      .setParam("prevent_reactivation", "true");
+    request.execute()
+      .assertJson(getClass(), "create_rule_with_prevent_reactivation_param_to_true.json", false)
+      .assertStatus(409);
+  }
+
 }
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_rule_with_prevent_reactivation_param_to_true.json b/sonar-server/src/test/resources/org/sonar/server/rule/ws/CreateActionMediumTest/create_rule_with_prevent_reactivation_param_to_true.json
new file mode 100644 (file)
index 0000000..f3c87c6
--- /dev/null
@@ -0,0 +1,14 @@
+{
+  "rule": {
+    "key": "manual:MY_MANUAL",
+    "repo": "manual",
+    "name": "My manual rule",
+    "htmlDesc": "Description",
+    "severity": "MAJOR",
+    "status": "REMOVED",
+    "isTemplate": false,
+    "tags": [],
+    "sysTags": [],
+    "params": []
+  }
+}