From: Eric Hartmann Date: Fri, 2 Feb 2018 17:19:23 +0000 (+0100) Subject: SONAR-10310 Fail on deprecated rule key duplicate X-Git-Tag: 7.5~1683 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6b45a96fc979c08e8ed599e767f45c8edd863a9c;p=sonarqube.git SONAR-10310 Fail on deprecated rule key duplicate --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java index 0becfdeca37..01aab36cadd 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -70,8 +71,10 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Sets.difference; +import static com.google.common.collect.Sets.intersection; import static java.lang.String.format; import static java.util.Collections.emptySet; +import static org.sonar.core.util.stream.MoreCollectors.toList; import static org.sonar.core.util.stream.MoreCollectors.toSet; /** @@ -116,6 +119,8 @@ public class RegisterRules implements Startable { RegisterRulesContext registerRulesContext = createRegisterRulesContext(dbSession); Map> existingDeprecatedRuleKeys = loadDeprecatedRuleKeys(dbSession); + verifyRuleKeyConsistency(repositories); + boolean orgsEnabled = organizationFlags.isEnabled(dbSession); for (RulesDefinition.ExtendedRepository repoDef : repositories) { if (languages.get(repoDef.language()) != null) { @@ -283,7 +288,7 @@ public class RegisterRules implements Startable { List dtos = repositories .stream() .map(r -> new RuleRepositoryDto(r.key(), r.language(), r.name())) - .collect(MoreCollectors.toList(repositories.size())); + .collect(toList(repositories.size())); dbClient.ruleRepositoryDao().insert(dbSession, dtos); dbSession.commit(); } @@ -549,7 +554,7 @@ public class RegisterRules implements Startable { // DeprecatedKeys that must be deleted List uuidsToBeDeleted = difference(deprecatedRuleKeysFromDB, deprecatedRuleKeysFromDefinition).stream() .map(SingleDeprecatedRuleKey::getUuid) - .collect(MoreCollectors.toList()); + .collect(toList()); dbClient.ruleDao().deleteDeprecatedRuleKeys(dbSession, uuidsToBeDeleted); @@ -699,4 +704,43 @@ public class RegisterRules implements Startable { dbClient.ruleDao().update(session, rule); } + + private static void verifyRuleKeyConsistency(List repositories) { + List allRules = repositories.stream() + .flatMap(r -> r.rules().stream()) + .collect(toList()); + + Set definedRuleKeys = allRules.stream() + .map(r -> RuleKey.of(r.repository().key(), r.key())) + .collect(toSet()); + + List definedDeprecatedRuleKeys = allRules.stream() + .flatMap(r -> r.deprecatedRuleKeys().stream()) + .collect(toList()); + + Set duplicates = findDuplicates(definedDeprecatedRuleKeys); + if (!duplicates.isEmpty()) { + throw new IllegalStateException(format("The following deprecated rule keys are declared at least twice [%s]", + duplicates.stream().map(RuleKey::toString).collect(Collectors.joining(",")))); + } + + Set intersection = intersection(new HashSet<>(definedRuleKeys), new HashSet<>(definedDeprecatedRuleKeys)).immutableCopy(); + if (!intersection.isEmpty()) { + throw new IllegalStateException(format("The following rule keys are declared both as deprecated and used key [%s]", + intersection.stream().map(RuleKey::toString).collect(Collectors.joining(",")))); + } + } + + private static Set findDuplicates(Collection list) { + Set duplicates = new HashSet<>(); + Set uniques = new HashSet<>(); + + list.stream().forEach(t -> { + if (!uniques.add(t)) { + duplicates.add(t); + } + }); + + return duplicates; + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java index fbcf86f59f9..64cecd404e9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java @@ -29,6 +29,7 @@ import java.util.stream.IntStream; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.junit.rules.ExpectedException; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.resources.Language; import org.sonar.api.resources.Languages; @@ -95,6 +96,8 @@ public class RegisterRulesTest { private System2 system = mock(System2.class); + @org.junit.Rule + public ExpectedException expectedException = ExpectedException.none(); @org.junit.Rule public DbTester dbTester = DbTester.create(system); @org.junit.Rule @@ -785,6 +788,93 @@ public class RegisterRulesTest { assertThat(deprecatedRuleKeys).hasSize(0); } + @Test + public void declaring_two_rules_with_same_deprecated_RuleKey_should_throw_ISE() { + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("The following deprecated rule keys are declared at least twice [fake:old]"); + + execute(context -> { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("newKey1") + .setName("One") + .setHtmlDescription("Description of One") + .setSeverity(BLOCKER) + .setInternalKey("config1") + .setTags("tag1", "tag2", "tag3") + .setType(RuleType.CODE_SMELL) + .addDeprecatedRuleKey("fake", "old") + .setStatus(RuleStatus.BETA); + repo.createRule("newKey2") + .setName("One") + .setHtmlDescription("Description of One") + .setSeverity(BLOCKER) + .setInternalKey("config1") + .setTags("tag1", "tag2", "tag3") + .setType(RuleType.CODE_SMELL) + .addDeprecatedRuleKey("fake", "old") + .setStatus(RuleStatus.BETA); + repo.done(); + }); + } + + @Test + public void declaring_a_rule_with_a_deprecated_RuleKey_still_used_should_throw_ISE() { + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("The following rule keys are declared both as deprecated and used key [fake:newKey1]"); + + execute(context -> { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("newKey1") + .setName("One") + .setHtmlDescription("Description of One") + .setSeverity(BLOCKER) + .setInternalKey("config1") + .setTags("tag1", "tag2", "tag3") + .setType(RuleType.CODE_SMELL) + .setStatus(RuleStatus.BETA); + + repo.createRule("newKey2") + .setName("One") + .setHtmlDescription("Description of One") + .setSeverity(BLOCKER) + .setInternalKey("config1") + .setTags("tag1", "tag2", "tag3") + .setType(RuleType.CODE_SMELL) + .addDeprecatedRuleKey("fake", "newKey1") + .setStatus(RuleStatus.BETA); + repo.done(); + }); + } + + @Test + public void declaring_a_rule_with_an_existing_RuleKey_still_used_should_throw_IAE() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("The rule 'newKey1' of repository 'fake' is declared several times"); + + execute(context -> { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("newKey1") + .setName("One") + .setHtmlDescription("Description of One") + .setSeverity(BLOCKER) + .setInternalKey("config1") + .setTags("tag1", "tag2", "tag3") + .setType(RuleType.CODE_SMELL) + .setStatus(RuleStatus.BETA); + + repo.createRule("newKey1") + .setName("One") + .setHtmlDescription("Description of One") + .setSeverity(BLOCKER) + .setInternalKey("config1") + .setTags("tag1", "tag2", "tag3") + .setType(RuleType.CODE_SMELL) + .addDeprecatedRuleKey("fake", "newKey1") + .setStatus(RuleStatus.BETA); + repo.done(); + }); + } + private void execute(RulesDefinition... defs) { ServerPluginRepository pluginRepository = mock(ServerPluginRepository.class); when(pluginRepository.getPluginKey(any(RulesDefinition.class))).thenReturn(FAKE_PLUGIN_KEY);