From 21d65415b3b95325dfe81a0ba48c4a43670eff11 Mon Sep 17 00:00:00 2001
From: Julien Lancelot <julien.lancelot@sonarsource.com>
Date: Wed, 26 Feb 2014 14:00:28 +0100
Subject: [PATCH] SONAR-5067 Fail when activating already activated rules on a
 profile

---
 .../org/sonar/api/profiles/RulesProfile.java  | 14 +++++++-
 .../sonar/api/profiles/RulesProfileTest.java  | 32 +++++++++++++------
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java b/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java
index 7836f493b89..c8ac0082627 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.api.profiles;
 
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.Transformer;
@@ -31,6 +33,7 @@ import org.sonar.api.rules.RulePriority;
 
 import javax.annotation.CheckForNull;
 import javax.persistence.*;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -367,7 +370,16 @@ public class RulesProfile implements Cloneable {
   /**
    * @param optionalSeverity if null, then the default rule severity is used
    */
-  public ActiveRule activateRule(Rule rule, RulePriority optionalSeverity) {
+  public ActiveRule activateRule(final Rule rule, RulePriority optionalSeverity) {
+    if (Iterables.any(activeRules, new Predicate<ActiveRule>() {
+      @Override
+      public boolean apply(ActiveRule input) {
+        return input.getRule().equals(rule);
+      }
+    })){
+      throw new IllegalStateException(String.format("The rule '%s:%s' is already activated on the profile '%s' (language '%s')",
+        rule.getRepositoryKey(), rule.getKey(), getName(), getLanguage()));
+    }
     ActiveRule activeRule = new ActiveRule();
     activeRule.setRule(rule);
     activeRule.setRulesProfile(this);
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/profiles/RulesProfileTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/profiles/RulesProfileTest.java
index c59a5f57ccb..3a31a73b060 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/profiles/RulesProfileTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/profiles/RulesProfileTest.java
@@ -23,9 +23,9 @@ import org.junit.Test;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RulePriority;
 
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
+import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
+
 
 public class RulesProfileTest {
 
@@ -35,8 +35,8 @@ public class RulesProfileTest {
     profile.activateRule(Rule.create("repo", "key1", "name1"), null);
     profile.activateRule(Rule.create("repo", "key2", "name2").setConfigKey("config2"), null);
 
-    assertNull(profile.getActiveRuleByConfigKey("repo", "unknown"));
-    assertThat(profile.getActiveRuleByConfigKey("repo", "config2").getRuleKey(), is("key2"));
+    assertThat(profile.getActiveRuleByConfigKey("repo", "unknown")).isNull();
+    assertThat(profile.getActiveRuleByConfigKey("repo", "config2").getRuleKey()).isEqualTo("key2");
   }
 
   @Test
@@ -44,7 +44,7 @@ public class RulesProfileTest {
     RulesProfile profile = RulesProfile.create();
     Rule rule = Rule.create("repo", "key1", "name1").setSeverity(RulePriority.CRITICAL);
     profile.activateRule(rule, null);
-    assertThat(profile.getActiveRule("repo", "key1").getSeverity(), is(RulePriority.CRITICAL));
+    assertThat(profile.getActiveRule("repo", "key1").getSeverity()).isEqualTo(RulePriority.CRITICAL);
   }
 
   @Test
@@ -52,12 +52,26 @@ public class RulesProfileTest {
     RulesProfile profile = RulesProfile.create();
     Rule rule = Rule.create("repo", "key1", "name1").setSeverity(RulePriority.CRITICAL);
     profile.activateRule(rule, RulePriority.MINOR);
-    assertThat(profile.getActiveRule("repo", "key1").getSeverity(), is(RulePriority.MINOR));
+    assertThat(profile.getActiveRule("repo", "key1").getSeverity()).isEqualTo(RulePriority.MINOR);
   }
 
   @Test
   public void defaultVersionIs1() {
-    RulesProfile profile = RulesProfile.create();    
-    assertThat(profile.getVersion(), is(1));
+    RulesProfile profile = RulesProfile.create();
+    assertThat(profile.getVersion()).isEqualTo(1);
+  }
+
+  @Test
+  public void fail_to_activate_already_activated_rule() {
+    RulesProfile profile = RulesProfile.create("Default", "java");
+    Rule rule = Rule.create("repo", "key1", "name1").setSeverity(RulePriority.CRITICAL);
+    profile.activateRule(rule, null);
+
+    try {
+      profile.activateRule(rule, null);
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("The rule 'repo:key1' is already activated on the profile 'Default' (language 'java')");
+    }
   }
 }
-- 
2.39.5