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 0becfdeca375bfd2f518e744fe3d85ec9a6b9946..01aab36cadd8d6da009d65cd9e591ed585101b31 100644 (file)
--- 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<Integer, Set<SingleDeprecatedRuleKey>> 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<RuleRepositoryDto> 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<String> 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<RulesDefinition.ExtendedRepository> repositories) {
+    List<RulesDefinition.Rule> allRules = repositories.stream()
+      .flatMap(r -> r.rules().stream())
+      .collect(toList());
+
+    Set<RuleKey> definedRuleKeys = allRules.stream()
+      .map(r -> RuleKey.of(r.repository().key(), r.key()))
+      .collect(toSet());
+
+    List<RuleKey> definedDeprecatedRuleKeys = allRules.stream()
+      .flatMap(r -> r.deprecatedRuleKeys().stream())
+      .collect(toList());
+
+    Set<RuleKey> 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<RuleKey> 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 <T> Set<T> findDuplicates(Collection<T> list) {
+    Set<T> duplicates = new HashSet<>();
+    Set<T> 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 fbcf86f59f91028a37e4904e2b35b19554b56ed2..64cecd404e9eabfa4605516497d78d3fb165a5f2 100644 (file)
--- 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);