]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-15132 Add a Quality Profile changelog on rule deletion
authorAntoine Vinot <antoine.vinot@sonarsource.com>
Thu, 2 Jun 2022 08:15:00 +0000 (10:15 +0200)
committersonartech <sonartech@sonarsource.com>
Thu, 9 Jun 2022 20:03:15 +0000 (20:03 +0000)
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java
server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java

index 2d342a401a72de9f876402ebd60f6c1e85e56f14..38c0c810cfa9acfd491424af66b6ab631eb6efff 100644 (file)
@@ -51,7 +51,6 @@ import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.api.utils.log.Profiler;
 import org.sonar.core.util.UuidFactory;
-import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.qualityprofile.ActiveRuleDto;
@@ -143,6 +142,7 @@ public class RegisterRules implements Startable {
       // FIXME lack of resiliency, active rules index is corrupted if rule index fails
       // to be updated. Only a single DB commit should be executed.
       ruleIndexer.commitAndIndex(dbSession, registerRulesContext.getAllModified().map(RuleDto::getUuid).collect(toSet()));
+      changes.forEach(arChange -> dbClient.qProfileChangeDao().insert(dbSession, arChange.toDto(null)));
       activeRuleIndexer.commitAndIndex(dbSession, changes);
       registerRulesContext.getRenamed().forEach(e -> LOG.info("Rule {} re-keyed to {}", e.getValue(), e.getKey().getKey()));
       profiler.stopDebug();
@@ -779,23 +779,29 @@ public class RegisterRules implements Startable {
    * If an extended repository do not exists anymore, then related active rules will be removed.
    */
   private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RegisterRulesContext recorder, List<RulesDefinition.Repository> context) {
-    List<String> repositoryKeys = context.stream()
-      .map(RulesDefinition.ExtendedRepository::key)
-      .collect(MoreCollectors.toList(context.size()));
-
+    Set<String> existingAndRenamedRepositories = getExistingAndRenamedRepositories(recorder, context);
     List<ActiveRuleChange> changes = new ArrayList<>();
     Profiler profiler = Profiler.create(Loggers.get(getClass()));
-    recorder.getRemoved().forEach(rule -> {
-      // SONAR-4642 Remove active rules only when repository still exists
-      if (repositoryKeys.contains(rule.getRepositoryKey())) {
+
+    recorder.getRemoved()
+      .filter(rule -> existingAndRenamedRepositories.contains(rule.getRepositoryKey()))
+      .forEach(rule -> {
+        // SONAR-4642 Remove active rules only when repository still exists
         profiler.start();
         changes.addAll(qProfileRules.deleteRule(dbSession, rule));
         profiler.stopDebug(format("Remove active rule for rule %s", rule.getKey()));
-      }
-    });
+      });
+
     return changes;
   }
 
+  private Set<String> getExistingAndRenamedRepositories(RegisterRulesContext recorder, Collection<RulesDefinition.Repository> context) {
+    return Stream.concat(
+        context.stream().map(RulesDefinition.ExtendedRepository::key),
+        recorder.getRenamed().map(Map.Entry::getValue).map(RuleKey::repository))
+      .collect(toSet());
+  }
+
   private void update(DbSession session, RuleDto rule) {
     rule.setUpdatedAt(system2.now());
     dbClient.ruleDao().update(session, rule);
index 39795de074fc1ef060e96453193aafd043927419..4f7c1add4f98305ba3f45ca039fd96b8bfb5f28e 100644 (file)
@@ -47,6 +47,10 @@ import org.sonar.core.util.UuidFactoryFast;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
+import org.sonar.db.qualityprofile.ActiveRuleDto;
+import org.sonar.db.qualityprofile.QProfileChangeDto;
+import org.sonar.db.qualityprofile.QProfileChangeQuery;
+import org.sonar.db.qualityprofile.QProfileDto;
 import org.sonar.db.rule.DeprecatedRuleKeyDto;
 import org.sonar.db.rule.RuleDescriptionSectionDto;
 import org.sonar.db.rule.RuleDto;
@@ -58,6 +62,7 @@ import org.sonar.server.es.SearchIdResult;
 import org.sonar.server.es.SearchOptions;
 import org.sonar.server.es.metadata.MetadataIndex;
 import org.sonar.server.plugins.ServerPluginRepository;
+import org.sonar.server.qualityprofile.ActiveRuleChange;
 import org.sonar.server.qualityprofile.QProfileRules;
 import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
 import org.sonar.server.rule.index.RuleIndex;
@@ -74,6 +79,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.tuple;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
@@ -91,6 +97,7 @@ import static org.sonar.api.server.rule.RulesDefinition.OwaspTop10Version.Y2021;
 import static org.sonar.db.rule.RuleDescriptionSectionDto.DEFAULT_KEY;
 import static org.sonar.db.rule.RuleDescriptionSectionDto.builder;
 import static org.sonar.db.rule.RuleDescriptionSectionDto.createDefaultRuleDescriptionSection;
+import static org.sonar.server.qualityprofile.ActiveRuleChange.Type.DEACTIVATED;
 
 @RunWith(DataProviderRunner.class)
 public class RegisterRulesTest {
@@ -126,8 +133,8 @@ public class RegisterRulesTest {
   private RuleIndexer ruleIndexer;
   private ActiveRuleIndexer activeRuleIndexer;
   private RuleIndex ruleIndex;
-  private RuleDescriptionSectionsGenerator ruleDescriptionSectionsGenerator = mock(RuleDescriptionSectionsGenerator.class);
-  private RuleDescriptionSectionsGeneratorResolver resolver = mock(RuleDescriptionSectionsGeneratorResolver.class);
+  private final RuleDescriptionSectionsGenerator ruleDescriptionSectionsGenerator = mock(RuleDescriptionSectionsGenerator.class);
+  private final RuleDescriptionSectionsGeneratorResolver resolver = mock(RuleDescriptionSectionsGeneratorResolver.class);
 
   @Before
   public void before() {
@@ -991,6 +998,34 @@ public class RegisterRulesTest {
       .hasMessage("The rule 'newKey1' of repository 'fake' is declared several times");
   }
 
+  @Test
+  public void removed_rule_should_appear_in_changelog() {
+    //GIVEN
+    QProfileDto qProfileDto = db.qualityProfiles().insert();
+    RuleDto ruleDto = db.rules().insert(RULE_KEY1);
+    db.qualityProfiles().activateRule(qProfileDto, ruleDto);
+    ActiveRuleChange arChange = new ActiveRuleChange(DEACTIVATED, ActiveRuleDto.createFor(qProfileDto, ruleDto), ruleDto);
+    when(qProfileRules.deleteRule(any(DbSession.class), eq(ruleDto))).thenReturn(List.of(arChange));
+    //WHEN
+    execute(context -> context.createRepository("fake", "java").done());
+    //THEN
+    List<QProfileChangeDto> qProfileChangeDtos = dbClient.qProfileChangeDao().selectByQuery(db.getSession(), new QProfileChangeQuery(qProfileDto.getKee()));
+    assertThat(qProfileChangeDtos).extracting(QProfileChangeDto::getRulesProfileUuid, QProfileChangeDto::getChangeType)
+      .contains(tuple(qProfileDto.getRulesProfileUuid(), "DEACTIVATED"));
+  }
+
+  @Test
+  public void removed_rule_should_be_deleted_when_renamed_repository() {
+    //GIVEN
+    RuleDto removedRuleDto = db.rules().insert(RuleKey.of("old_repo", "removed_rule"));
+    RuleDto renamedRuleDto = db.rules().insert(RuleKey.of("old_repo", "renamed_rule"));
+    //WHEN
+    execute(context -> createRule(context, "java", "new_repo", renamedRuleDto.getRuleKey(),
+      rule -> rule.addDeprecatedRuleKey(renamedRuleDto.getRepositoryKey(), renamedRuleDto.getRuleKey())));
+    //THEN
+    verify(qProfileRules).deleteRule(any(DbSession.class), eq(removedRuleDto));
+  }
+
   private void execute(RulesDefinition... defs) {
     ServerPluginRepository pluginRepository = mock(ServerPluginRepository.class);
     when(pluginRepository.getPluginKey(any(RulesDefinition.class))).thenReturn(FAKE_PLUGIN_KEY);