Fix rules handling with duplicated internalKey

3 files changed, 102 insertions, 10 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rules.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rules.java
index f05b73e8324..b54d676db91 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rules.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/Rules.java
@@ -49,6 +49,6 @@ public interface Rules {
    */
   Collection<Rule> findByRepository(String repository);
 
-  Rule findByInternalKey(String repository, String internalKey);
+  Collection<Rule> findByInternalKey(String repository, String internalKey);
 
 }
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRules.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRules.java
index 1ef36b3e63d..90e9d24d155 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRules.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/rule/internal/DefaultRules.java
@@ -19,8 +19,10 @@
  */
 package org.sonar.api.batch.rule.internal;
 
-import org.sonar.api.batch.rule.Rule;
 import com.google.common.collect.ImmutableTable;
+
+import com.google.common.collect.HashBasedTable;
+import org.sonar.api.batch.rule.Rule;
 import com.google.common.collect.Table;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ListMultimap;
@@ -31,6 +33,8 @@ import org.sonar.api.rule.RuleKey;
 import javax.annotation.concurrent.Immutable;
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 
 @Immutable
@@ -38,22 +42,35 @@ class DefaultRules implements Rules {
 
   // TODO use disk-backed cache (persistit) instead of full in-memory cache ?
   private final ListMultimap<String, Rule> rulesByRepository;
-  private final Table<String, String, Rule> rulesByRepositoryAndInternalKey;
+  private final ImmutableTable<String, String, List<Rule>> rulesByRepositoryAndInternalKey;
 
   DefaultRules(Collection<NewRule> newRules) {
     ImmutableListMultimap.Builder<String, Rule> builder = ImmutableListMultimap.builder();
-    ImmutableTable.Builder<String, String, Rule> tableBuilder = ImmutableTable.builder();
+    Table<String, String, List<Rule>> tableBuilder = HashBasedTable.create();
 
     for (NewRule newRule : newRules) {
       DefaultRule r = new DefaultRule(newRule);
       builder.put(r.key().repository(), r);
-      if (r.internalKey() != null) {
-        tableBuilder.put(r.key().repository(), r.internalKey(), r);
-      }
+      addToTable(tableBuilder, r);
     }
 
     rulesByRepository = builder.build();
-    rulesByRepositoryAndInternalKey = tableBuilder.build();
+    rulesByRepositoryAndInternalKey = ImmutableTable.copyOf(tableBuilder);
+  }
+
+  private void addToTable(Table<String, String, List<Rule>> table, DefaultRule r) {
+    if (r.internalKey() == null) {
+      return;
+    }
+
+    List<Rule> ruleList = table.get(r.key().repository(), r.internalKey());
+    
+    if(ruleList == null) {
+      ruleList = new LinkedList<>();
+    }
+    
+    ruleList.add(r);
+    table.put(r.key().repository(), r.internalKey(), ruleList);
   }
 
   @Override
@@ -78,7 +95,9 @@ class DefaultRules implements Rules {
   }
 
   @Override
-  public Rule findByInternalKey(String repository, String internalKey) {
-    return rulesByRepositoryAndInternalKey.get(repository, internalKey);
+  public Collection<Rule> findByInternalKey(String repository, String internalKey) {
+    List<Rule> rules = rulesByRepositoryAndInternalKey.get(repository, internalKey);
+    
+    return rules != null ? rules : Collections.<Rule>emptyList();
   }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/DefaultRulesTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/DefaultRulesTest.java
new file mode 100644
index 00000000000..62c066463d3
--- /dev/null
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/DefaultRulesTest.java
@@ -0,0 +1,73 @@
+/*
+ * 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.api.batch.rule.internal;
+
+import org.sonar.api.rule.RuleKey;
+import org.junit.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DefaultRulesTest {
+  @Test
+  public void testRepeatedInternalKey() {
+    List<NewRule> newRules = new LinkedList<>();
+    newRules.add(createRule("key1", "repo", "internal"));
+    newRules.add(createRule("key2", "repo", "internal"));
+    
+    DefaultRules rules = new DefaultRules(newRules);
+    assertThat(rules.findByInternalKey("repo", "internal")).hasSize(2);
+    assertThat(rules.find(RuleKey.of("repo", "key1"))).isNotNull();
+    assertThat(rules.find(RuleKey.of("repo", "key2"))).isNotNull();
+    assertThat(rules.findByRepository("repo")).hasSize(2);
+  }
+  
+  @Test
+  public void testNonExistingKey() {
+    List<NewRule> newRules = new LinkedList<>();
+    newRules.add(createRule("key1", "repo", "internal"));
+    newRules.add(createRule("key2", "repo", "internal"));
+    
+    DefaultRules rules = new DefaultRules(newRules);
+    assertThat(rules.findByInternalKey("xx", "xx")).hasSize(0);
+    assertThat(rules.find(RuleKey.of("xxx", "xx"))).isNull();
+    assertThat(rules.findByRepository("xxxx")).hasSize(0);
+  }
+  
+  @Test
+  public void testRepeatedRule() {
+    List<NewRule> newRules = new LinkedList<>();
+    newRules.add(createRule("key", "repo", "internal"));
+    newRules.add(createRule("key", "repo", "internal"));
+    
+    DefaultRules rules = new DefaultRules(newRules);
+    assertThat(rules.find(RuleKey.of("repo", "key"))).isNotNull();
+  }
+  
+  private NewRule createRule(String key, String repo, String internalKey) {
+    RuleKey ruleKey = RuleKey.of(repo, key);
+    NewRule newRule = new NewRule(ruleKey);
+    newRule.setInternalKey(internalKey);
+    
+    return newRule;
+  }
+}