From 697cdc1de821864d77a507bdb3834f1239066c57 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 30 Jan 2018 16:15:11 +0100 Subject: [PATCH] SONAR-10357 change rule key into id in QPROFILE_CHANGES#CHANGE_DATA --- .../db/migration/version/v71/DbVersion71.java | 4 +- .../version/v71/UseRuleIdInQPChangesData.java | 85 ++++++++ .../version/v71/DbVersion71Test.java | 2 +- .../v71/UseRuleIdInQPChangesDataTest.java | 195 ++++++++++++++++++ .../rules_and_qprofile_changes.sql | 34 +++ 5 files changed, 318 insertions(+), 2 deletions(-) create mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesData.java create mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest.java create mode 100644 server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v71/UseRuleIdInQPChangesDataTest/rules_and_qprofile_changes.sql 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 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 ruleKeysById) throws SQLException { + String key = row.getString(1); + String data = row.getString(2); + + Map 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 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 toMap(String[] keysAndValues) { + Preconditions.checkArgument(keysAndValues.length % 2 == 0); + Map 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"); -- 2.39.5