From 9316c3016fdca65613e1af37c630c04d393ae706 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Mon, 15 Mar 2021 15:53:53 -0500 Subject: [PATCH] SONAR-14549 Restoring backed up Quality Profile with old rule repositority/key doesn't work --- .../qualityprofile/QProfileBackuperImpl.java | 69 +++++++++++++------ .../QProfileBackuperImplTest.java | 66 ++++++++++++++++++ 2 files changed, 114 insertions(+), 21 deletions(-) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java index b274bb68fc2..e7cb97ea633 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java @@ -22,11 +22,12 @@ package org.sonar.server.qualityprofile; import java.io.Reader; import java.io.Writer; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Map; -import java.util.Objects; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -35,17 +36,18 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ServerSide; -import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.qualityprofile.ExportRuleDto; import org.sonar.db.qualityprofile.ExportRuleParamDto; import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.rule.DeprecatedRuleKeyDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.server.rule.NewCustomRule; import org.sonar.server.rule.RuleCreator; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.function.Function.identity; @ServerSide public class QProfileBackuperImpl implements QProfileBackuper { @@ -133,7 +135,7 @@ public class QProfileBackuperImpl implements QProfileBackuper { List importedRules = qProfile.getRules(); Map ruleDefinitionsByKey = getImportedRulesDefinitions(dbSession, importedRules); - checkIfRulesFromExternalEngines(ruleDefinitionsByKey); + checkIfRulesFromExternalEngines(ruleDefinitionsByKey.values()); Map customRulesDefinitions = createCustomRulesIfNotExist(dbSession, importedRules, ruleDefinitionsByKey); ruleDefinitionsByKey.putAll(customRulesDefinitions); @@ -144,17 +146,42 @@ public class QProfileBackuperImpl implements QProfileBackuper { return new QProfileRestoreSummary(targetProfile, changes); } + /** + * Returns map of rule definition for an imported rule key. + * The imported rule key may refer to a deprecated rule key, in which case the the RuleDefinitionDto will correspond to a different key (the new key). + */ private Map getImportedRulesDefinitions(DbSession dbSession, List rules) { - List ruleKeys = rules.stream() + Set ruleKeys = rules.stream() .map(ImportedRule::getRuleKey) - .collect(MoreCollectors.toList()); - return db.ruleDao().selectDefinitionByKeys(dbSession, ruleKeys) - .stream() - .collect(Collectors.toMap(RuleDefinitionDto::getKey, Function.identity())); + .collect(Collectors.toSet()); + Map rulesDefinitions = db.ruleDao().selectDefinitionByKeys(dbSession, ruleKeys).stream() + .collect(Collectors.toMap(RuleDefinitionDto::getKey, identity())); + + Set unrecognizedRuleKeys = ruleKeys.stream() + .filter(r -> !rulesDefinitions.containsKey(r)) + .collect(Collectors.toSet()); + + if (!unrecognizedRuleKeys.isEmpty()) { + Map deprecatedRuleKeysByUuid = db.ruleDao().selectAllDeprecatedRuleKeys(dbSession).stream() + .filter(r -> r.getNewRepositoryKey() != null && r.getNewRuleKey() != null) + .filter(r -> unrecognizedRuleKeys.contains(RuleKey.of(r.getOldRepositoryKey(), r.getOldRuleKey()))) + // ignore deprecated rule if the new rule key was already found in the list of imported rules + .filter(r -> !ruleKeys.contains(RuleKey.of(r.getNewRepositoryKey(), r.getNewRuleKey()))) + .collect(Collectors.toMap(DeprecatedRuleKeyDto::getRuleUuid, identity())); + + List rulesBasedOnDeprecatedKeys = db.ruleDao().selectDefinitionByUuids(dbSession, deprecatedRuleKeysByUuid.keySet()); + for (RuleDefinitionDto rule : rulesBasedOnDeprecatedKeys) { + DeprecatedRuleKeyDto deprecatedRuleKey = deprecatedRuleKeysByUuid.get(rule.getUuid()); + RuleKey oldRuleKey = RuleKey.of(deprecatedRuleKey.getOldRepositoryKey(), deprecatedRuleKey.getOldRuleKey()); + rulesDefinitions.put(oldRuleKey, rule); + } + } + + return rulesDefinitions; } - private static void checkIfRulesFromExternalEngines(Map ruleDefinitionsByKey) { - List externalRules = ruleDefinitionsByKey.values().stream() + private static void checkIfRulesFromExternalEngines(Collection ruleDefinitions) { + List externalRules = ruleDefinitions.stream() .filter(RuleDefinitionDto::isExternal) .collect(Collectors.toList()); @@ -173,7 +200,7 @@ public class QProfileBackuperImpl implements QProfileBackuper { if (!customRulesToCreate.isEmpty()) { return db.ruleDao().selectDefinitionByKeys(dbSession, ruleCreator.create(dbSession, customRulesToCreate)) .stream() - .collect(Collectors.toMap(RuleDefinitionDto::getKey, Function.identity())); + .collect(Collectors.toMap(RuleDefinitionDto::getKey, identity())); } return Collections.emptyMap(); } @@ -190,16 +217,16 @@ public class QProfileBackuperImpl implements QProfileBackuper { } private static List toRuleActivations(List rules, Map ruleDefinitionsByKey) { - return rules.stream() - .map(r -> { - RuleDefinitionDto ruleDefinition = ruleDefinitionsByKey.get(r.getRuleKey()); - if (ruleDefinition == null) { - return null; - } - return RuleActivation.create(ruleDefinition.getUuid(), r.getSeverity(), r.getParameters()); - }) - .filter(Objects::nonNull) - .collect(MoreCollectors.toList(rules.size())); + List activatedRule = new ArrayList<>(); + + for (ImportedRule r : rules) { + RuleDefinitionDto ruleDefinition = ruleDefinitionsByKey.get(r.getRuleKey()); + if (ruleDefinition == null) { + continue; + } + activatedRule.add(RuleActivation.create(ruleDefinition.getUuid(), r.getSeverity(), r.getParameters())); + } + return activatedRule; } private enum BackupActiveRuleComparator implements Comparator { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java index 765429591cc..cf560bf74e2 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java @@ -186,6 +186,72 @@ public class QProfileBackuperImplTest { assertThat(reset.calledActivations).isEmpty(); } + @Test + public void restore_detects_deprecated_rule_keys() { + String ruleUuid = db.rules().insert(RuleKey.of("sonarjs", "s001")).getUuid(); + db.rules().insertDeprecatedKey(c -> c.setRuleUuid(ruleUuid).setOldRuleKey("oldkey").setOldRepositoryKey("oldrepo")); + + Reader backup = new StringReader("" + + "foo" + + "js" + + "" + + "" + + "oldrepo" + + "oldkey" + + "BLOCKER" + + "" + + "barbaz" + + "" + + "" + + "" + + ""); + + underTest.restore(db.getSession(), backup, (String) null); + + assertThat(reset.calledActivations).hasSize(1); + RuleActivation activation = reset.calledActivations.get(0); + assertThat(activation.getSeverity()).isEqualTo("BLOCKER"); + assertThat(activation.getRuleUuid()).isEqualTo(ruleUuid); + assertThat(activation.getParameter("bar")).isEqualTo("baz"); + } + + @Test + public void restore_ignores_deprecated_rule_keys_if_new_key_is_already_present() { + String ruleUuid = db.rules().insert(RuleKey.of("sonarjs", "s001")).getUuid(); + db.rules().insertDeprecatedKey(c -> c.setRuleUuid(ruleUuid).setOldRuleKey("oldkey").setOldRepositoryKey("oldrepo")); + + Reader backup = new StringReader("" + + "foo" + + "js" + + "" + + "" + + "oldrepo" + + "oldkey" + + "MAJOR" + + "" + + "barbaz" + + "" + + "" + + "" + + "sonarjs" + + "s001" + + "BLOCKER" + + "" + + "bar2baz2" + + "" + + "" + + "" + + ""); + + underTest.restore(db.getSession(), backup, (String) null); + + assertThat(reset.calledActivations).hasSize(1); + RuleActivation activation = reset.calledActivations.get(0); + assertThat(activation.getSeverity()).isEqualTo("BLOCKER"); + assertThat(activation.getRuleUuid()).isEqualTo(ruleUuid); + assertThat(activation.getParameter("bar2")).isEqualTo("baz2"); + } + @Test public void restore_backup_on_profile_having_different_name() { Reader backup = new StringReader(EMPTY_BACKUP); -- 2.39.5