SONAR-10357 store rule id in QP changelog instead of rule key

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java

@@ -28,6 +28,7 @@ import org.apache.commons.lang.StringUtils;
 import org.sonar.db.qualityprofile.ActiveRuleDto;
 import org.sonar.db.qualityprofile.ActiveRuleKey;
 import org.sonar.db.qualityprofile.QProfileChangeDto;
+import org.sonar.db.rule.RuleDefinitionDto;
 
 public class ActiveRuleChange {
 
@@ -39,25 +40,32 @@ public class ActiveRuleChange {
 
   private final Type type;
   private final ActiveRuleKey key;
+  private final int ruleId;
   private String severity = null;
   private ActiveRule.Inheritance inheritance = null;
   private final Map<String, String> parameters = new HashMap<>();
 
-  public ActiveRuleChange(Type type, ActiveRuleDto activeRule) {
+  public ActiveRuleChange(Type type, ActiveRuleDto activeRule, RuleDefinitionDto ruleDefinition) {
     this.type = type;
     this.key = activeRule.getKey();
+    this.ruleId = ruleDefinition.getId();
     this.activeRule = activeRule;
   }
 
-  public ActiveRuleChange(Type type, ActiveRuleKey key) {
+  public ActiveRuleChange(Type type, ActiveRuleKey key, RuleDefinitionDto ruleDefinition) {
     this.type = type;
     this.key = key;
+    this.ruleId = ruleDefinition.getId();
   }
 
   public ActiveRuleKey getKey() {
     return key;
   }
 
+  public int getRuleId() {
+    return ruleId;
+  }
+
   public Type getType() {
     return type;
   }
@@ -113,7 +121,7 @@ public class ActiveRuleChange {
     dto.setRulesProfileUuid(getKey().getRuleProfileUuid());
     dto.setLogin(login);
     Map<String, String> data = new HashMap<>();
-    data.put("ruleKey", getKey().getRuleKey().toString());
+    data.put("ruleId", String.valueOf(getRuleId()));
 
     parameters.entrySet().stream()
       .filter(param -> !param.getKey().isEmpty())
@@ -134,6 +142,7 @@ public class ActiveRuleChange {
     return MoreObjects.toStringHelper(this)
       .add("type", type)
       .add("key", key)
+      .add("ruleId", ruleId)
       .add("severity", severity)
       .add("inheritance", inheritance)
       .add("parameters", parameters)

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileInsertImpl.java

@@ -155,7 +155,7 @@ public class BuiltInQProfileInsertImpl implements BuiltInQProfileInsert {
 
     List<ActiveRuleParamDto> paramDtos = insertActiveRuleParams(dbSession, activeRule, ruleKey, dto);
 
-    ActiveRuleChange change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, dto);
+    ActiveRuleChange change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, dto, ruleDefinitionDto);
     change.setSeverity(dto.getSeverityString());
     paramDtos.forEach(paramDto -> change.setParameter(paramDto.getKey(), paramDto.getValue()));
     return change;

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRulesImpl.java

@@ -102,7 +102,7 @@ public class QProfileRulesImpl implements QProfileRules {
     List<Integer> activeRuleIds = new ArrayList<>();
     db.activeRuleDao().selectByRuleIdOfAllOrganizations(dbSession, rule.getId()).forEach(ar -> {
       activeRuleIds.add(ar.getId());
-      changes.add(new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, ar));
+      changes.add(new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, ar, rule));
     });
 
     db.activeRuleDao().deleteByIds(dbSession, activeRuleIds);

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileTreeImpl.java

@@ -114,10 +114,11 @@ public class QProfileTreeImpl implements QProfileTree {
         changes.addAll(ruleActivator.deactivate(dbSession, context, activeRule.getRuleKey(), true));
 
       } else if (ActiveRuleDto.OVERRIDES.equals(activeRule.getInheritance())) {
+        context.reset(activeRule.getRuleKey());
         activeRule.setInheritance(null);
         activeRule.setUpdatedAt(system2.now());
         db.activeRuleDao().update(dbSession, activeRule);
-        changes.add(new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRule).setInheritance(null));
+        changes.add(new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRule, context.getRule().get()).setInheritance(null));
       }
     }
     return changes;

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java

@@ -94,7 +94,7 @@ public class RuleActivator {
         return changes;
       }
       // new activation
-      change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, activeRuleKey);
+      change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, activeRuleKey, rule);
       applySeverityAndParamToChange(activation, context, change);
       if (context.isCascading() || isSameAsParent(change, context)) {
         change.setInheritance(ActiveRule.Inheritance.INHERITED);
@@ -105,7 +105,7 @@ public class RuleActivator {
         // propagating to descendants, but child profile already overrides rule -> stop propagation
         return changes;
       }
-      change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRuleKey);
+      change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activeRuleKey, rule);
       if (context.isCascading() && activeRule.get().getInheritance() == null) {
         // activate on child, then on parent -> mark child as overriding parent
         change.setInheritance(ActiveRule.Inheritance.OVERRIDES);
@@ -333,7 +333,7 @@ public class RuleActivator {
 
     ActiveRuleChange change;
     checkRequest(force || context.isCascading() || activeRule.get().getInheritance() == null, "Cannot deactivate inherited rule '%s'", context.getRule().get().getKey());
-    change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, activeRule.get());
+    change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, activeRule.get(), context.getRule().get());
     changes.add(change);
     persist(change, context, dbSession);
 

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ChangelogAction.java

@@ -19,15 +19,15 @@
  */
 package org.sonar.server.qualityprofile.ws;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
 import org.sonar.api.resources.Languages;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.server.ws.Request;
@@ -129,8 +129,8 @@ public class ChangelogAction implements QProfileWsAction {
         .prop("authorLogin", change.getUserLogin())
         .prop("authorName", change.getUserName())
         .prop("action", change.getType())
-        .prop("ruleKey", change.getRuleKey() == null ? null : change.getRuleKey().toString())
-        .prop("ruleName", change.getRuleName());
+        .prop("ruleKey", change.getRuleKey().map(RuleKey::toString).orElse(null))
+        .prop("ruleName", change.getRuleName().orElse(null));
       writeParameters(json, change);
       json.endObject();
     }
@@ -166,15 +166,22 @@ public class ChangelogAction implements QProfileWsAction {
       .stream()
       .collect(java.util.stream.Collectors.toMap(UserDto::getLogin, UserDto::getName));
 
-    Set<RuleKey> ruleKeys = changes.stream().filter(c -> c.ruleKey != null).map(c -> c.ruleKey).collect(MoreCollectors.toSet());
-    Map<RuleKey, String> ruleNamesByKeys = dbClient.ruleDao()
-      .selectDefinitionByKeys(dbSession, Lists.newArrayList(ruleKeys))
+    Set<Integer> ruleIds = changes.stream()
+      .map(c -> c.ruleId)
+      .filter(Objects::nonNull)
+      .collect(MoreCollectors.toSet());
+    Map<Integer, RuleDefinitionDto> ruleDefinitionsById = dbClient.ruleDao()
+      .selectDefinitionByIds(dbSession, Lists.newArrayList(ruleIds))
       .stream()
-      .collect(java.util.stream.Collectors.toMap(RuleDefinitionDto::getKey, RuleDefinitionDto::getName));
+      .collect(MoreCollectors.uniqueIndex(RuleDefinitionDto::getId));
 
     changes.forEach(c -> {
       c.userName = userNamesByLogins.get(c.userLogin);
-      c.ruleName = ruleNamesByKeys.get(c.ruleKey);
+      RuleDefinitionDto ruleDefinitionDto = ruleDefinitionsById.get(c.ruleId);
+      if (ruleDefinitionDto != null) {
+        c.ruleKey = ruleDefinitionDto.getKey();
+        c.ruleName = ruleDefinitionDto.getName();
+      }
     });
   }
 
@@ -186,6 +193,7 @@ public class ChangelogAction implements QProfileWsAction {
     private String userLogin;
     private String userName;
     private String inheritance;
+    private Integer ruleId;
     private RuleKey ruleKey;
     private String ruleName;
     private final Map<String, String> params = new HashMap<>();
@@ -193,20 +201,6 @@ public class ChangelogAction implements QProfileWsAction {
     private Change() {
     }
 
-    @VisibleForTesting
-    Change(String key, String type, long at, @Nullable String severity, @Nullable String userLogin,
-      @Nullable String userName, @Nullable String inheritance, @Nullable RuleKey ruleKey, @Nullable String ruleName) {
-      this.key = key;
-      this.type = type;
-      this.at = at;
-      this.severity = severity;
-      this.userLogin = userLogin;
-      this.userName = userName;
-      this.inheritance = inheritance;
-      this.ruleKey = ruleKey;
-      this.ruleName = ruleName;
-    }
-
     public String getKey() {
       return key;
     }
@@ -235,13 +229,16 @@ public class ChangelogAction implements QProfileWsAction {
       return inheritance;
     }
 
-    public RuleKey getRuleKey() {
-      return ruleKey;
+    public Optional<Integer> getRuleId() {
+      return Optional.ofNullable(ruleId);
     }
 
-    @CheckForNull
-    public String getRuleName() {
-      return ruleName;
+    public Optional<RuleKey> getRuleKey() {
+      return Optional.ofNullable(ruleKey);
+    }
+
+    public Optional<String> getRuleName() {
+      return Optional.ofNullable(ruleName);
     }
 
     public long getCreatedAt() {
@@ -261,9 +258,9 @@ public class ChangelogAction implements QProfileWsAction {
       change.at = dto.getCreatedAt();
       // see content of data in class org.sonar.server.qualityprofile.ActiveRuleChange
       change.severity = data.get("severity");
-      String ruleKey = data.get("ruleKey");
-      if (ruleKey != null) {
-        change.ruleKey = RuleKey.parse(ruleKey);
+      String ruleId = data.get("ruleId");
+      if (ruleId != null) {
+        change.ruleId = Integer.parseInt(ruleId);
       }
       change.inheritance = data.get("inheritance");
       data.entrySet().stream()

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleChangeTest.java

@@ -19,11 +19,13 @@
  */
 package org.sonar.server.qualityprofile;
 
+import java.util.Random;
 import org.junit.Test;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.db.qualityprofile.ActiveRuleKey;
 import org.sonar.db.qualityprofile.QProfileChangeDto;
 import org.sonar.db.qualityprofile.QProfileDto;
+import org.sonar.db.rule.RuleDefinitionDto;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.sonar.db.qualityprofile.QualityProfileTesting.newQualityProfileDto;
@@ -37,12 +39,14 @@ public class ActiveRuleChangeTest {
   public void toDto() {
     QProfileDto profile = newQualityProfileDto();
     ActiveRuleKey key = ActiveRuleKey.of(profile, RuleKey.of("P1", "R1"));
-    ActiveRuleChange underTest = new ActiveRuleChange(ACTIVATED, key);
+    int ruleId = new Random().nextInt(963);
+    ActiveRuleChange underTest = new ActiveRuleChange(ACTIVATED, key, new RuleDefinitionDto().setId(ruleId));
 
     QProfileChangeDto result = underTest.toDto(A_LOGIN);
 
     assertThat(result.getChangeType()).isEqualTo(ACTIVATED.name());
     assertThat(result.getRulesProfileUuid()).isEqualTo(profile.getRulesProfileUuid());
     assertThat(result.getLogin()).isEqualTo(A_LOGIN);
+    assertThat(result.getDataAsMap().get("ruleId")).isEqualTo(String.valueOf(ruleId));
   }
 }

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileInsertImplTest.java

@@ -24,9 +24,7 @@ import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.rule.Severity;
-import org.sonar.api.rules.RulePriority;
 import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition;
 import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.NewBuiltInQualityProfile;
 import org.sonar.api.utils.System2;
@@ -189,7 +187,7 @@ public class BuiltInQProfileInsertImplTest {
 
     QProfileChangeQuery changeQuery = new QProfileChangeQuery(profile.getKee());
     QProfileChangeDto change = db.getDbClient().qProfileChangeDao().selectByQuery(dbSession, changeQuery).stream()
-      .filter(c -> c.getDataAsMap().get("ruleKey").equals(rule.getKey().toString()))
+      .filter(c -> c.getDataAsMap().get("ruleId").equals(String.valueOf(rule.getId())))
       .findFirst()
       .get();
     assertThat(change.getChangeType()).isEqualTo(ActiveRuleChange.Type.ACTIVATED.name());
@@ -200,10 +198,6 @@ public class BuiltInQProfileInsertImplTest {
     assertThat(change.getDataAsMap().get("severity")).isEqualTo(expectedSeverity);
   }
 
-  private static void activeRule(RulesProfile apiProfile, RuleDefinitionDto rule, RulePriority severity) {
-    apiProfile.activateRule(org.sonar.api.rules.Rule.create(rule.getRepositoryKey(), rule.getRuleKey()), severity);
-  }
-
   private QProfileDto verifyProfileInDb(OrganizationDto organization, BuiltInQProfile builtIn) {
     QProfileDto profileOnOrg1 = db.getDbClient().qualityProfileDao().selectByNameAndLanguage(dbSession, organization, builtIn.getName(), builtIn.getLanguage());
     assertThat(profileOnOrg1.getLanguage()).isEqualTo(builtIn.getLanguage());

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQualityProfilesUpdateListenerTest.java

@@ -21,6 +21,7 @@ package org.sonar.server.qualityprofile;
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
+import java.util.Random;
 import org.assertj.core.groups.Tuple;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -29,6 +30,7 @@ import org.sonar.api.notifications.Notification;
 import org.sonar.api.resources.Language;
 import org.sonar.api.resources.Languages;
 import org.sonar.db.qualityprofile.ActiveRuleKey;
+import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.server.notification.NotificationManager;
 import org.sonar.server.qualityprofile.BuiltInQualityProfilesNotification.Profile;
 
@@ -159,10 +161,15 @@ public class BuiltInQualityProfilesUpdateListenerTest {
 
   private Tuple addProfile(Multimap<QProfileName, ActiveRuleChange> profiles, Languages languages, ActiveRuleChange.Type type) {
     String profileName = randomLowerCaseText();
+    int ruleId1 = new Random().nextInt(952);
+    int ruleId2 = new Random().nextInt(952);
     Language language = newLanguage(randomLowerCaseText(), randomLowerCaseText());
     languages.add(language);
     profiles.putAll(new QProfileName(language.getKey(), profileName),
-      asList(new ActiveRuleChange(type, ActiveRuleKey.parse("qp:repo:rule1")), new ActiveRuleChange(type, ActiveRuleKey.parse("qp:repo:rule2"))));
+      asList(new ActiveRuleChange(
+        type,
+        ActiveRuleKey.parse("qp:repo:rule1"), new RuleDefinitionDto().setId(ruleId1)),
+        new ActiveRuleChange(type, ActiveRuleKey.parse("qp:repo:rule2"), new RuleDefinitionDto().setId(ruleId2))));
     return tuple(profileName, language.getKey(), language.getName(), 2);
   }
 

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/index/ActiveRuleIndexerTest.java

@@ -101,7 +101,7 @@ public class ActiveRuleIndexerTest {
     ActiveRuleDto ar2 = db.qualityProfiles().activateRule(profile2, rule1);
     ActiveRuleDto ar3 = db.qualityProfiles().activateRule(profile2, rule2);
 
-    commitAndIndex(ar1, ar2);
+    commitAndIndex(rule1, ar1, ar2);
 
     verifyOnlyIndexed(ar1, ar2);
     assertThatEsQueueTableIsEmpty();
@@ -122,7 +122,7 @@ public class ActiveRuleIndexerTest {
     ActiveRuleDto ar = db.qualityProfiles().activateRule(profile1, rule1);
     es.lockWrites(INDEX_TYPE_ACTIVE_RULE);
 
-    commitAndIndex(ar);
+    commitAndIndex(rule1, ar);
 
     EsQueueDto expectedItem = EsQueueDto.create(INDEX_TYPE_ACTIVE_RULE.format(), "" + ar.getId(), "activeRuleId", ar.getRuleKey().toString());
     assertThatEsQueueContainsExactly(expectedItem);
@@ -135,7 +135,7 @@ public class ActiveRuleIndexerTest {
     assertThat(es.countDocuments(INDEX_TYPE_ACTIVE_RULE)).isEqualTo(1);
 
     db.getDbClient().activeRuleDao().delete(db.getSession(), ar.getKey());
-    commitAndIndex(ar);
+    commitAndIndex(rule1, ar);
 
     assertThat(es.countDocuments(INDEX_TYPE_ACTIVE_RULE)).isEqualTo(0);
     assertThatEsQueueTableIsEmpty();
@@ -187,9 +187,9 @@ public class ActiveRuleIndexerTest {
       .containsExactlyInAnyOrder(Tuple.tuple(expected.getDocId(), expected.getDocIdType(), expected.getDocRouting()));
   }
 
-  private void commitAndIndex(ActiveRuleDto... ar) {
+  private void commitAndIndex(RuleDefinitionDto rule, ActiveRuleDto... ar) {
     underTest.commitAndIndex(db.getSession(), stream(ar)
-      .map(a -> new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, a))
+      .map(a -> new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, a, rule))
       .collect(Collectors.toList()));
   }
 

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ChangelogActionTest.java

@@ -51,6 +51,7 @@ import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
 import org.sonar.test.JsonAssert;
 
+import static java.lang.String.valueOf;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.sonar.test.JsonAssert.assertJson;
 import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_KEY;
@@ -109,7 +110,7 @@ public class ChangelogActionTest {
     insertChange(profile, c -> c.setRulesProfileUuid(profileUuid)
       .setLogin(user1.getLogin())
       .setChangeType(ActiveRuleChange.Type.ACTIVATED.name())
-      .setData(ImmutableMap.of("severity", "CRITICAL", "ruleKey", rule1.getKey().toString())));
+      .setData(ImmutableMap.of("severity", "CRITICAL", "ruleId", valueOf(rule1.getId()))));
 
     system2.setNow(DateUtils.parseDateTime("2015-02-23T17:58:18+0100").getTime());
     RuleDefinitionDto rule2 = dbTester.rules().insert(RuleKey.of("squid", "S2162"), r -> r.setName("\"equals\" methods should be symmetric and work for subclasses"));
@@ -117,7 +118,7 @@ public class ChangelogActionTest {
     QProfileChangeDto change2 = insertChange(profile, c -> c.setRulesProfileUuid(profileUuid)
       .setLogin(user2.getLogin())
       .setChangeType(ActiveRuleChange.Type.DEACTIVATED.name())
-      .setData(ImmutableMap.of("ruleKey", rule2.getKey().toString())));
+      .setData(ImmutableMap.of("ruleId", valueOf(rule2.getId()))));
 
     system2.setNow(DateUtils.parseDateTime("2014-09-12T15:20:46+0200").getTime());
     RuleDefinitionDto rule3 = dbTester.rules().insert(RuleKey.of("squid", "S00101"), r -> r.setName("Class names should comply with a naming convention"));
@@ -125,7 +126,7 @@ public class ChangelogActionTest {
     QProfileChangeDto change3 = insertChange(profile, c -> c.setRulesProfileUuid(profileUuid)
       .setLogin(user3.getLogin())
       .setChangeType(ActiveRuleChange.Type.ACTIVATED.name())
-      .setData(ImmutableMap.of("severity", "MAJOR", "param_format", "^[A-Z][a-zA-Z0-9]*$", "ruleKey", rule3.getKey().toString())));
+      .setData(ImmutableMap.of("severity", "MAJOR", "param_format", "^[A-Z][a-zA-Z0-9]*$", "ruleId", valueOf(rule3.getId()))));
 
     dbTester.commit();
 
@@ -285,8 +286,10 @@ public class ChangelogActionTest {
   @Test
   public void return_change_with_all_fields() {
     QProfileDto profile = dbTester.qualityProfiles().insert(organization);
+    RuleDefinitionDto rule1 = dbTester.rules().insert(RuleKey.of("java", "S001"));
+
     Map<String, Object> data = ImmutableMap.of(
-      "ruleKey", "java:S001",
+      "ruleId", valueOf(rule1.getId()),
       "severity", "MINOR",
       "inheritance", ActiveRule.Inheritance.INHERITED.name(),
       "param_foo", "foo_value",