From e61c5f5e2d1e71d9f3dc5c562ad9be9fb1dd6c87 Mon Sep 17 00:00:00 2001 From: Antoine Vinot Date: Thu, 2 Jun 2022 10:15:00 +0200 Subject: [PATCH] SONAR-15132 Add a Quality Profile changelog on rule deletion --- .../org/sonar/server/rule/RegisterRules.java | 26 ++++++++----- .../sonar/server/rule/RegisterRulesTest.java | 39 ++++++++++++++++++- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java index 2d342a401a7..38c0c810cfa 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -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 removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RegisterRulesContext recorder, List context) { - List repositoryKeys = context.stream() - .map(RulesDefinition.ExtendedRepository::key) - .collect(MoreCollectors.toList(context.size())); - + Set existingAndRenamedRepositories = getExistingAndRenamedRepositories(recorder, context); List 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 getExistingAndRenamedRepositories(RegisterRulesContext recorder, Collection 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); diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java index 39795de074f..4f7c1add4f9 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RegisterRulesTest.java @@ -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 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); -- 2.39.5