SONAR-15260 when changing a quality profile parent, the overridden rules severit is preserved

2 files changed, 64 insertions, 6 deletions

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java
index aa491de41c9..0e245f0fd8f 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java
@@ -22,6 +22,7 @@ package org.sonar.server.qualityprofile;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.sonar.api.utils.System2;
 import org.sonar.core.util.stream.MoreCollectors;
@@ -79,15 +80,18 @@ public class QProfileTreeImpl implements QProfileTree {
     }
 
     checkRequest(!isDescendant(dbSession, profile, parent), "Descendant profile '%s' can not be selected as parent of '%s'", parent.getKee(), profile.getKee());
-    changes.addAll(removeParent(dbSession, profile));
 
     // set new parent
     profile.setParentKee(parent.getKee());
     db.qualityProfileDao().update(dbSession, profile);
 
+    List<OrgActiveRuleDto> activeRules = db.activeRuleDao().selectByProfile(dbSession, profile);
     List<OrgActiveRuleDto> parentActiveRules = db.activeRuleDao().selectByProfile(dbSession, parent);
-    Collection<String> ruleUuids = parentActiveRules.stream().map(ActiveRuleDto::getRuleUuid).collect(MoreCollectors.toArrayList());
-    RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleUuids);
+
+    changes = getChangesFromRulesToBeRemoved(dbSession, profile, getRulesDifference(activeRules, parentActiveRules));
+
+    Collection<String> parentRuleUuids = parentActiveRules.stream().map(ActiveRuleDto::getRuleUuid).collect(MoreCollectors.toArrayList());
+    RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, parentRuleUuids);
 
     for (ActiveRuleDto parentActiveRule : parentActiveRules) {
       try {
@@ -102,6 +106,16 @@ public class QProfileTreeImpl implements QProfileTree {
     return changes;
   }
 
+  private static List<OrgActiveRuleDto> getRulesDifference(Collection<OrgActiveRuleDto> rulesCollection1, Collection<OrgActiveRuleDto> rulesCollection2) {
+    Collection<String> rulesCollection2Uuids = rulesCollection2.stream()
+      .map(ActiveRuleDto::getRuleUuid)
+      .collect(MoreCollectors.toArrayList());
+
+    return rulesCollection1.stream()
+      .filter(rule -> !rulesCollection2Uuids.contains(rule.getRuleUuid()))
+      .collect(Collectors.toList());
+  }
+
   private List<ActiveRuleChange> removeParent(DbSession dbSession, QProfileDto profile) {
     List<ActiveRuleChange> changes = new ArrayList<>();
     if (profile.getParentKee() == null) {
@@ -112,10 +126,19 @@ public class QProfileTreeImpl implements QProfileTree {
     db.qualityProfileDao().update(dbSession, profile);
 
     List<OrgActiveRuleDto> activeRules = db.activeRuleDao().selectByProfile(dbSession, profile);
-    Collection<String> ruleUuids = activeRules.stream().map(ActiveRuleDto::getRuleUuid).collect(MoreCollectors.toArrayList());
+    changes = getChangesFromRulesToBeRemoved(dbSession, profile, activeRules);
+
+    qualityProfileChangeEventService.distributeRuleChangeEvent(List.of(profile), changes, profile.getLanguage());
+    return changes;
+  }
+
+  private List<ActiveRuleChange> getChangesFromRulesToBeRemoved(DbSession dbSession, QProfileDto profile, List<OrgActiveRuleDto> rules) {
+    List<ActiveRuleChange> changes = new ArrayList<>();
+
+    Collection<String> ruleUuids = rules.stream().map(ActiveRuleDto::getRuleUuid).collect(MoreCollectors.toArrayList());
     RuleActivationContext context = ruleActivator.createContextForUserProfile(dbSession, profile, ruleUuids);
 
-    for (OrgActiveRuleDto activeRule : activeRules) {
+    for (OrgActiveRuleDto activeRule : rules) {
       if (ActiveRuleDto.INHERITED.equals(activeRule.getInheritance())) {
         changes.addAll(ruleActivator.deactivate(dbSession, context, activeRule.getRuleUuid(), true));
 
@@ -128,7 +151,6 @@ public class QProfileTreeImpl implements QProfileTree {
       }
     }
 
-    qualityProfileChangeEventService.distributeRuleChangeEvent(List.of(profile), changes, profile.getLanguage());
     return changes;
   }
 
diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java
index 7cf8aec8d4b..fe22127a786 100644
--- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java
+++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/QProfileTreeImplTest.java
@@ -184,6 +184,42 @@ public class QProfileTreeImplTest {
   }
 
   @Test
+  public void change_parent_keep_overridden_rules() {
+    RuleDto parentRule = createJavaRule();
+    RuleDto childRule = createJavaRule();
+
+    QProfileDto parentProfile1 = createProfile(parentRule);
+    List<ActiveRuleChange> changes = activate(parentProfile1, RuleActivation.create(parentRule.getUuid()));
+    assertThat(changes).hasSize(1);
+
+    QProfileDto childProfile = createProfile(childRule);
+    changes = activate(childProfile, RuleActivation.create(childRule.getUuid()));
+    assertThat(changes).hasSize(1);
+
+    changes = underTest.setParentAndCommit(db.getSession(), childProfile, parentProfile1);
+    assertThat(changes).hasSize(1);
+    assertThatRuleIsActivated(childProfile, parentRule, changes, parentRule.getSeverityString(), INHERITED, emptyMap());
+    assertThatRuleIsActivated(childProfile, childRule, null, childRule.getSeverityString(), null, emptyMap());
+    verify(qualityProfileChangeEventService, times(2)).distributeRuleChangeEvent(any(), any(), eq(childProfile.getLanguage()));
+
+    RuleActivation activation = RuleActivation.create(parentRule.getUuid(), BLOCKER, null);
+    changes = activate(childProfile, activation);
+    assertThat(changes).hasSize(1);
+    assertThatRuleIsUpdated(childProfile, parentRule, BLOCKER, ActiveRuleInheritance.OVERRIDES, emptyMap());
+    assertThatRuleIsActivated(childProfile, childRule, null, childRule.getSeverityString(), null, emptyMap());
+
+    QProfileDto parentProfile2 = createProfile(parentRule);
+    changes = activate(parentProfile2, RuleActivation.create(parentRule.getUuid()));
+    assertThat(changes).hasSize(1);
+
+    changes = underTest.setParentAndCommit(db.getSession(), childProfile, parentProfile2);
+    assertThat(changes).isEmpty();
+    assertThatRuleIsUpdated(childProfile, parentRule, BLOCKER, ActiveRuleInheritance.OVERRIDES, emptyMap());
+    assertThatRuleIsActivated(childProfile, childRule, null, childRule.getSeverityString(), null, emptyMap());
+    verify(qualityProfileChangeEventService, times(4)).distributeRuleChangeEvent(any(), any(), eq(childProfile.getLanguage()));
+  }
+
+  @Test
   public void activation_errors_are_ignored_when_setting_a_parent() {
     RuleDto rule1 = createJavaRule();
     RuleDto rule2 = createJavaRule();