SONAR-10357 change rule key into id in QPROFILE_CHANGES#CHANGE_DATA

5 files changed, 318 insertions, 2 deletions

diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71.java
index bb67a4d88ed..3d762638d8b 100644
--- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71.java
+++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71.java
@@ -30,6 +30,8 @@ public class DbVersion71 implements DbVersion {
       .add(2000, "Delete settings defined in sonar.properties from PROPERTIES table", DeleteSettingsDefinedInSonarDotProperties.class)
       .add(2001, "Add scope to rules", AddRuleScope.class)
       .add(2002, "Set rules scope to MAIN", SetRuleScopeToMain.class)
-      .add(2003, "Make scope not nullable in rules", MakeScopeNotNullableInRules.class);
+      .add(2003, "Make scope not nullable in rules", MakeScopeNotNullableInRules.class)
+      .add(2004, "Use rule id in QPROFILE_CHANGES", UseRuleIdInQPChangesData.class)
+    ;
   }
 }
diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesData.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesData.java
new file mode 100644
index 00000000000..a2b674b975a
--- /dev/null
+++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesData.java
@@ -0,0 +1,85 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.platform.db.migration.version.v71;
+
+import java.sql.SQLException;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.api.utils.KeyValueFormat;
+import org.sonar.db.Database;
+import org.sonar.server.platform.db.migration.step.DataChange;
+import org.sonar.server.platform.db.migration.step.MassUpdate;
+import org.sonar.server.platform.db.migration.step.Select;
+import org.sonar.server.platform.db.migration.step.SqlStatement;
+
+public class UseRuleIdInQPChangesData extends DataChange {
+
+  private static final String RULE_KEY_DATA_FIELD = "ruleKey";
+  private static final String RULE_ID_DATA_FIELD = "ruleId";
+
+  public UseRuleIdInQPChangesData(Database db) {
+    super(db);
+  }
+
+  @Override
+  protected void execute(Context context) throws SQLException {
+    Map<String, Integer> ruleKeysById = context.prepareSelect("select id, plugin_name, plugin_rule_key from rules")
+      .list(row -> new Rule(row.getInt(1), RuleKey.of(row.getString(2), row.getString(3)).toString()))
+      .stream()
+      .collect(Collectors.toMap(r -> r.ruleKey, r -> r.ruleId));
+
+    MassUpdate massUpdate = context.prepareMassUpdate();
+    massUpdate.select("select kee,change_data from qprofile_changes");
+    massUpdate.update("update qprofile_changes set change_data=? where kee=?");
+    massUpdate.execute(((row, update) -> handle(row, update, ruleKeysById)));
+  }
+
+  private static boolean handle(Select.Row row, SqlStatement update, Map<String, Integer> ruleKeysById) throws SQLException {
+    String key = row.getString(1);
+    String data = row.getString(2);
+
+    Map<String, String> map = KeyValueFormat.parse(data);
+    String ruleKey = map.get(RULE_KEY_DATA_FIELD);
+    if (ruleKey == null) {
+      return false;
+    }
+
+    Integer ruleId = ruleKeysById.get(ruleKey);
+    if (ruleId != null) {
+      map.put(RULE_ID_DATA_FIELD, String.valueOf(ruleId));
+    }
+    map.remove(RULE_KEY_DATA_FIELD);
+
+    update.setString(1, KeyValueFormat.format(map));
+    update.setString(2, key);
+    return true;
+  }
+
+  private static final class Rule {
+    private final int ruleId;
+    private final String ruleKey;
+
+    private Rule(int ruleId, String ruleKey) {
+      this.ruleId = ruleId;
+      this.ruleKey = ruleKey;
+    }
+  }
+}
diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71Test.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71Test.java
index c65e93d43a8..e42251e445a 100644
--- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71Test.java
+++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/DbVersion71Test.java
@@ -36,7 +36,7 @@ public class DbVersion71Test {
 
   @Test
   public void verify_migration_count() {
-    verifyMigrationCount(underTest, 4);
+    verifyMigrationCount(underTest, 5);
   }
 
 }
diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest.java
new file mode 100644
index 00000000000..e8c018185c9
--- /dev/null
+++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest.java
@@ -0,0 +1,195 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.platform.db.migration.version.v71;
+
+import com.google.common.base.Preconditions;
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Random;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.sonar.api.utils.KeyValueFormat;
+import org.sonar.db.CoreDbTester;
+
+import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
+import static org.assertj.core.api.Assertions.assertThat;
+
+@RunWith(DataProviderRunner.class)
+public class UseRuleIdInQPChangesDataTest {
+
+  public static final String RULE_ID_DATA_FIELD = "ruleId";
+  public static final String RULE_KEY_DATA_FIELD = "ruleKey";
+  @Rule
+  public final CoreDbTester dbTester = CoreDbTester.createForSchema(UseRuleIdInQPChangesDataTest.class, "rules_and_qprofile_changes.sql");
+
+  private UseRuleIdInQPChangesData underTest = new UseRuleIdInQPChangesData(dbTester.database());
+
+  @Test
+  public void no_effect_if_tables_are_empty() throws SQLException {
+    underTest.execute();
+  }
+
+  @Test
+  public void qpChange_without_ruleKey_is_unchanged() throws SQLException {
+    String c1Data = insertQPChange("c1");
+    String c2Data = insertQPChange("c2", "random", "data", "for", "value");
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1")).isEqualTo(c1Data);
+    assertThat(reaQPChangeData("c2")).isEqualTo(c2Data);
+  }
+
+  @Test
+  public void qpChange_with_ruleKey_of_other_case_is_unchanged() throws SQLException {
+    int ruleId1 = insertRule("foo", "bar");
+    String c1Data = insertQPChange("c1", "RULEKEY", "notARuleKey");
+    String c2Data = insertQPChange("c2", "RULeKey", "foo:bar");
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1")).isEqualTo(c1Data);
+    assertThat(reaQPChangeData("c2")).isEqualTo(c2Data);
+  }
+
+  @Test
+  @UseDataProvider("nonExistingRuleKey")
+  public void qpChange_with_ruleKey_of_non_existing_rule_has_ruleKey_removed(String ruleKey) throws SQLException {
+    insertRule("foo", "bar");
+    insertQPChange("c1", RULE_KEY_DATA_FIELD, ruleKey);
+    insertQPChange("c2", "otherDataKey", "otherDataData", RULE_KEY_DATA_FIELD, ruleKey);
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1")).isEmpty();
+    assertThat(reaQPChangeData("c2")).contains("otherDataKey=otherDataData");
+  }
+
+  @DataProvider
+  public static Object[][] nonExistingRuleKey() {
+    return new Object[][] {
+      {"notARuleKey"},
+      {"bar:foo"},
+      {"foo:bar2"},
+    };
+  }
+
+  @Test
+  public void qpChange_with_ruleKey_of_existing_rule_is_replaced_with_ruleId() throws SQLException {
+    int ruleId1 = insertRule("foo", "bar");
+    int ruleId2 = insertRule("foo2", "bar");
+    int ruleId3 = insertRule("foo", "bar2");
+    insertQPChange("c1", RULE_KEY_DATA_FIELD, "foo:bar");
+    insertQPChange("c2", "otherDataKey", "otherDataData", RULE_KEY_DATA_FIELD, "foo2:bar");
+    insertQPChange("c3", RULE_KEY_DATA_FIELD, "foo:bar2", "otherDataKey2", "otherDataData2");
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1"))
+      .doesNotContain(RULE_KEY_DATA_FIELD)
+      .contains("ruleId=" + ruleId1);
+    assertThat(reaQPChangeData("c2"))
+      .contains("otherDataKey=otherDataData")
+      .doesNotContain(RULE_KEY_DATA_FIELD)
+      .contains("ruleId=" + ruleId2);
+    assertThat(reaQPChangeData("c3"))
+      .contains("otherDataKey2=otherDataData2")
+      .doesNotContain(RULE_KEY_DATA_FIELD)
+      .contains("ruleId=" + ruleId3);
+  }
+
+  @Test
+  public void qpChange_with_ruleId_is_unchanged() throws SQLException {
+    int ruleId1 = insertRule("foo", "bar");
+    String c1Data = insertQPChange("c1", RULE_ID_DATA_FIELD, "notAnIntButIgnored");
+    String c2Data = insertQPChange("c2", RULE_ID_DATA_FIELD, String.valueOf(ruleId1));
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1")).isEqualTo(c1Data);
+    assertThat(reaQPChangeData("c2")).isEqualTo(c2Data);
+  }
+
+  @Test
+  public void migration_is_reentrant() throws SQLException {
+    int ruleId1 = insertRule("foo", "bar");
+    insertQPChange("c1", RULE_KEY_DATA_FIELD, "foo:bar");
+    String c2Data = insertQPChange("c2", "RULEKEY", "notARuleKey");
+    insertQPChange("c3", RULE_KEY_DATA_FIELD, "nonExistingRuleKey");
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1"))
+      .doesNotContain(RULE_KEY_DATA_FIELD)
+      .contains("ruleId=" + ruleId1);
+    assertThat(reaQPChangeData("c2")).isEqualTo(c2Data);
+    assertThat(reaQPChangeData("c3")).isEmpty();
+
+    underTest.execute();
+
+    assertThat(reaQPChangeData("c1"))
+      .doesNotContain(RULE_KEY_DATA_FIELD)
+      .contains("ruleId=" + ruleId1);
+    assertThat(reaQPChangeData("c2")).isEqualTo(c2Data);
+    assertThat(reaQPChangeData("c3")).isEmpty();
+  }
+
+  private String reaQPChangeData(String qpChangeKey) {
+    return (String) dbTester.selectFirst("select change_data as \"DATA\"from qprofile_changes where kee='" + qpChangeKey + "'")
+      .get("DATA");
+  }
+
+  private int insertRule(String repo, String key) {
+    dbTester.executeInsert(
+      "RULES",
+      "PLUGIN_RULE_KEY", key,
+      "PLUGIN_NAME", repo);
+
+    Map<String, Object> row = dbTester.selectFirst("select id as \"ID\" from rules where plugin_rule_key='" + key + "' and plugin_name='" + repo + "'");
+    return ((Long) row.get("ID")).intValue();
+  }
+
+  private String insertQPChange(String kee, String... keysAndValues) {
+    String data = keysAndValues.length == 0 ? null : KeyValueFormat.format(toMap(keysAndValues));
+    dbTester.executeInsert(
+      "QPROFILE_CHANGES",
+      "KEE", kee,
+      "RULES_PROFILE_UUID", randomAlphanumeric(5),
+      "CHANGE_TYPE", randomAlphanumeric(6),
+      "CREATED_AT", new Random().nextInt(10_000),
+      "CHANGE_DATA", data);
+
+    return data;
+  }
+
+  private static Map<String, String> toMap(String[] keysAndValues) {
+    Preconditions.checkArgument(keysAndValues.length % 2 == 0);
+    Map<String, String> res = new HashMap<>();
+    for (int i = 0; i < keysAndValues.length; i++) {
+      res.put(keysAndValues[i++], keysAndValues[i]);
+    }
+    return res;
+  }
+}
diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest/rules_and_qprofile_changes.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest/rules_and_qprofile_changes.sql
new file mode 100644
index 00000000000..ea146a6d3c3
--- /dev/null
+++ b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest/rules_and_qprofile_changes.sql
@@ -0,0 +1,34 @@
+CREATE TABLE "RULES" (
+  "ID" INTEGER NOT NULL GENERATED BY DEFAULT AS IDENTITY (START WITH 1, INCREMENT BY 1),
+  "PLUGIN_KEY" VARCHAR(200),
+  "PLUGIN_RULE_KEY" VARCHAR(200) NOT NULL,
+  "PLUGIN_NAME" VARCHAR(255) NOT NULL,
+  "DESCRIPTION" VARCHAR(16777215),
+  "DESCRIPTION_FORMAT" VARCHAR(20),
+  "PRIORITY" INTEGER,
+  "IS_TEMPLATE" BOOLEAN DEFAULT FALSE,
+  "TEMPLATE_ID" INTEGER,
+  "PLUGIN_CONFIG_KEY" VARCHAR(200),
+  "NAME" VARCHAR(200),
+  "STATUS" VARCHAR(40),
+  "LANGUAGE" VARCHAR(20),
+  "DEF_REMEDIATION_FUNCTION" VARCHAR(20),
+  "DEF_REMEDIATION_GAP_MULT" VARCHAR(20),
+  "DEF_REMEDIATION_BASE_EFFORT" VARCHAR(20),
+  "GAP_DESCRIPTION" VARCHAR(4000),
+  "SYSTEM_TAGS" VARCHAR(4000),
+  "RULE_TYPE" TINYINT,
+  "CREATED_AT" BIGINT,
+  "UPDATED_AT" BIGINT
+);
+CREATE UNIQUE INDEX "RULES_REPO_KEY" ON "RULES" ("PLUGIN_NAME", "PLUGIN_RULE_KEY");
+
+CREATE TABLE "QPROFILE_CHANGES" (
+  "KEE" VARCHAR(40) NOT NULL PRIMARY KEY,
+  "RULES_PROFILE_UUID" VARCHAR(255) NOT NULL,
+  "CHANGE_TYPE" VARCHAR(20) NOT NULL,
+  "CREATED_AT" BIGINT NOT NULL,
+  "USER_LOGIN" VARCHAR(255),
+  "CHANGE_DATA" CLOB
+);
+CREATE INDEX "QP_CHANGES_RULES_PROFILE_UUID" ON "QPROFILE_CHANGES" ("RULES_PROFILE_UUID");