From 23699dc534e3978abc10c643566715a345296b2c Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Wed, 29 Jul 2015 11:14:18 +0200 Subject: [PATCH] Fix rules handling with duplicated internalKey --- .../batch/rule/RuleFinderCompatibility.java | 3 +- .../preview/PreviewAndReportsMediumTest.java | 1 + .../java/org/sonar/api/batch/rule/Rules.java | 2 +- .../api/batch/rule/internal/DefaultRules.java | 37 +++++++--- .../batch/rule/internal/DefaultRulesTest.java | 73 +++++++++++++++++++ 5 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 sonar-plugin-api/src/test/java/org/sonar/api/batch/rule/internal/DefaultRulesTest.java 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 index 285d5e6668f..715dc526b0a 100644 --- 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 @@ public class RuleFinderCompatibility implements RuleFinder { } private Collection byInternalKey(RuleQuery query) { - Rule rule = toRule(rules.findByInternalKey(query.getRepositoryKey(), query.getConfigKey())); - return rule != null ? Arrays.asList(rule) : Collections.emptyList(); + return Collections2.transform(rules.findByInternalKey(query.getRepositoryKey(), query.getConfigKey()), RuleTransformer); } @CheckForNull diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/preview/PreviewAndReportsMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/preview/PreviewAndReportsMediumTest.java index acf1f5b588b..9dae8576ce3 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/preview/PreviewAndReportsMediumTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/preview/PreviewAndReportsMediumTest.java @@ -70,6 +70,7 @@ public class PreviewAndReportsMediumTest { .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 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 findByRepository(String repository); - Rule findByInternalKey(String repository, String internalKey); + Collection 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 rulesByRepository; - private final Table rulesByRepositoryAndInternalKey; + private final ImmutableTable> rulesByRepositoryAndInternalKey; DefaultRules(Collection newRules) { ImmutableListMultimap.Builder builder = ImmutableListMultimap.builder(); - ImmutableTable.Builder tableBuilder = ImmutableTable.builder(); + Table> 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> table, DefaultRule r) { + if (r.internalKey() == null) { + return; + } + + List 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 findByInternalKey(String repository, String internalKey) { + List rules = rulesByRepositoryAndInternalKey.get(repository, internalKey); + + return rules != null ? rules : Collections.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 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 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 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; + } +} -- 2.39.5