Fix rules handling with duplicated internalKey

g711ab · Jul 29, 2015 · 23699dc · 23699dc
1 parent 33851f1
commit 23699dc
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 12 deletions.
diff --git a/sonar-batch/src/main/java/org/sonar/batch/rule/RuleFinderCompatibility.java b/sonar-batch/src/main/java/org/sonar/batch/rule/RuleFinderCompatibility.java
@@ -111,8 +111,7 @@ private Collection<Rule> byKey(RuleQuery query) {
   }
 
   private Collection<Rule> byInternalKey(RuleQuery query) {
-    Rule rule = toRule(rules.findByInternalKey(query.getRepositoryKey(), query.getConfigKey()));
-    return rule != null ? Arrays.asList(rule) : Collections.<Rule>emptyList();
+    return Collections2.transform(rules.findByInternalKey(query.getRepositoryKey(), query.getConfigKey()), RuleTransformer);
   }
 
   @CheckForNull

diff --git a/...r-batch/src/test/java/org/sonar/batch/mediumtest/preview/PreviewAndReportsMediumTest.java b/...r-batch/src/test/java/org/sonar/batch/mediumtest/preview/PreviewAndReportsMediumTest.java
@@ -70,6 +70,7 @@ private static Long date(String date) {
     .addDefaultQProfile("xoo", "Sonar Way")
     .addRules(new XooRulesDefinition())
     .addRule(new Rule("manual:MyManualIssue", "manual", "MyManualIssue", "My manual issue", "MAJOR", null))
+    .addRule(new Rule("manual:MyManualIssueDup", "manual", "MyManualIssue", "My manual issue", "MAJOR", null))
     .activateRule(new ActiveRule("xoo", "OneIssuePerLine", null, "One issue per line", "MAJOR", null, "xoo"))
     .activateRule(new ActiveRule("manual", "MyManualIssue", null, "My manual issue", "MAJOR", null, null))
     .setPreviousAnalysisDate(new Date())

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
@@ -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
@@ -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,29 +33,44 @@
 import javax.annotation.concurrent.Immutable;
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 
 @Immutable
 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 @@ public Collection<Rule> findByRepository(String repository) {
   }
 
   @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
@@ -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;
+  }
+}