Fix dropping primary keys in DB migrations

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/sql/DropConstraintBuilder.java

@@ -51,6 +51,9 @@ public class DropConstraintBuilder {
   }
 
   public DropConstraintBuilder setName(String s) {
+    if (s.startsWith("pk_")) {
+      throw new IllegalArgumentException("This builder should not be used with primary keys");
+    }
     this.constraintName = s;
     return this;
   }

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/sql/DropPrimaryKeySqlGenerator.java

@@ -21,6 +21,7 @@ package org.sonar.server.platform.db.migration.sql;
 
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import org.sonar.db.Database;
 import org.sonar.db.dialect.Dialect;
@@ -30,6 +31,7 @@ import org.sonar.db.dialect.Oracle;
 import org.sonar.db.dialect.PostgreSql;
 
 import static java.lang.String.format;
+import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 
 public class DropPrimaryKeySqlGenerator {
@@ -44,11 +46,15 @@ public class DropPrimaryKeySqlGenerator {
   }
 
   public List<String> generate(String tableName, String columnName, boolean isAutoGenerated) throws SQLException {
+    return generate(tableName, singleton(columnName), isAutoGenerated);
+  }
+
+  public List<String> generate(String tableName, Collection<String> columnNames, boolean isAutoGenerated) throws SQLException {
     Dialect dialect = db.getDialect();
     String constraintName = dbConstraintFinder.findConstraintName(tableName);
     switch (dialect.getId()) {
       case PostgreSql.ID:
-        return generateForPostgresSql(tableName, columnName, constraintName);
+        return generateForPostgresSql(tableName, columnNames, constraintName);
       case MsSql.ID:
         return generateForMsSql(tableName, constraintName);
       case Oracle.ID:
@@ -60,17 +66,17 @@ public class DropPrimaryKeySqlGenerator {
     }
   }
 
-  private List<String> generateForPostgresSql(String tableName, String column, String constraintName) throws SQLException {
+  private List<String> generateForPostgresSql(String tableName, Collection<String> columns, String constraintName) throws SQLException {
     List<String> statements = new ArrayList<>();
-    statements.add(format("ALTER TABLE %s ALTER COLUMN %s DROP DEFAULT", tableName, column));
-
-    String sequence = dbConstraintFinder.getPostgresSqlSequence(tableName, column);
-    if (sequence != null) {
-      statements.add(format("DROP SEQUENCE %s", sequence));
+    for (String column : columns) {
+      statements.add(format("ALTER TABLE %s ALTER COLUMN %s DROP DEFAULT", tableName, column));
+      String sequence = dbConstraintFinder.getPostgresSqlSequence(tableName, column);
+      if (sequence != null) {
+        statements.add(format("DROP SEQUENCE %s", sequence));
+      }
     }
 
     statements.add(format(GENERIC_DROP_CONSTRAINT_STATEMENT, tableName, constraintName));
-
     return statements;
   }
 

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v86/DropOrganizationInRulesMetadata.java

@@ -20,22 +20,25 @@
 package org.sonar.server.platform.db.migration.version.v86;
 
 import java.sql.SQLException;
+import java.util.Arrays;
 import org.sonar.db.Database;
 import org.sonar.server.platform.db.migration.sql.AddPrimaryKeyBuilder;
 import org.sonar.server.platform.db.migration.sql.DropColumnsBuilder;
-import org.sonar.server.platform.db.migration.sql.DropConstraintBuilder;
+import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.step.DdlChange;
 
 public class DropOrganizationInRulesMetadata extends DdlChange {
   private static final String TABLE_NAME = "rules_metadata";
+  private final DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator;
 
-  public DropOrganizationInRulesMetadata(Database db) {
+  public DropOrganizationInRulesMetadata(Database db, DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator) {
     super(db);
+    this.dropPrimaryKeySqlGenerator = dropPrimaryKeySqlGenerator;
   }
 
   @Override
   public void execute(Context context) throws SQLException {
-    context.execute(new DropConstraintBuilder(getDialect()).setName("pk_rules_metadata").setTable(TABLE_NAME).build());
+    context.execute(dropPrimaryKeySqlGenerator.generate(TABLE_NAME, Arrays.asList("rule_uuid", "organization_uuid"), false));
     context.execute(new DropColumnsBuilder(getDialect(), TABLE_NAME, "organization_uuid").build());
     context.execute(new AddPrimaryKeyBuilder(TABLE_NAME, "rule_uuid").build());
   }

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v87/DropAlmAppInstallsTable.java

@@ -20,9 +20,7 @@
 package org.sonar.server.platform.db.migration.version.v87;
 
 import java.sql.SQLException;
-import java.util.Arrays;
 import org.sonar.db.Database;
-import org.sonar.server.platform.db.migration.sql.DropConstraintBuilder;
 import org.sonar.server.platform.db.migration.sql.DropIndexBuilder;
 import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.sql.DropTableBuilder;

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v87/DropOrgQualityGatesTable.java

@@ -21,21 +21,23 @@ package org.sonar.server.platform.db.migration.version.v87;
 
 import java.sql.SQLException;
 import org.sonar.db.Database;
-import org.sonar.server.platform.db.migration.sql.DropConstraintBuilder;
 import org.sonar.server.platform.db.migration.sql.DropIndexBuilder;
+import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.sql.DropTableBuilder;
 import org.sonar.server.platform.db.migration.step.DdlChange;
 
 public class DropOrgQualityGatesTable extends DdlChange {
   private static final String TABLE_NAME = "org_quality_gates";
+  private final DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator;
 
-  public DropOrgQualityGatesTable(Database db) {
+  public DropOrgQualityGatesTable(Database db, DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator) {
     super(db);
+    this.dropPrimaryKeySqlGenerator = dropPrimaryKeySqlGenerator;
   }
 
   @Override
   public void execute(Context context) throws SQLException {
-    context.execute(new DropConstraintBuilder(getDialect()).setTable(TABLE_NAME).setName("pk_org_quality_gates").build());
+    context.execute(dropPrimaryKeySqlGenerator.generate(TABLE_NAME, "uuid", false));
     context.execute(new DropIndexBuilder(getDialect()).setTable(TABLE_NAME).setName("uniq_org_quality_gates").build());
     context.execute(new DropTableBuilder(getDialect(), TABLE_NAME).build());
   }

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v87/DropOrganizationAlmBindingsTable.java

@@ -21,21 +21,23 @@ package org.sonar.server.platform.db.migration.version.v87;
 
 import java.sql.SQLException;
 import org.sonar.db.Database;
-import org.sonar.server.platform.db.migration.sql.DropConstraintBuilder;
 import org.sonar.server.platform.db.migration.sql.DropIndexBuilder;
+import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.sql.DropTableBuilder;
 import org.sonar.server.platform.db.migration.step.DdlChange;
 
 public class DropOrganizationAlmBindingsTable extends DdlChange {
   private static final String TABLE_NAME = "organization_alm_bindings";
+  private final DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator;
 
-  public DropOrganizationAlmBindingsTable(Database db) {
+  public DropOrganizationAlmBindingsTable(Database db, DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator) {
     super(db);
+    this.dropPrimaryKeySqlGenerator = dropPrimaryKeySqlGenerator;
   }
 
   @Override
   public void execute(Context context) throws SQLException {
-    context.execute(new DropConstraintBuilder(getDialect()).setTable(TABLE_NAME).setName("pk_organization_alm_bindings").build());
+    context.execute(dropPrimaryKeySqlGenerator.generate(TABLE_NAME, "uuid", false));
     context.execute(new DropIndexBuilder(getDialect()).setTable(TABLE_NAME).setName("org_alm_bindings_org").build());
     context.execute(new DropIndexBuilder(getDialect()).setTable(TABLE_NAME).setName("org_alm_bindings_install").build());
     context.execute(new DropTableBuilder(getDialect(), TABLE_NAME).build());

server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/sql/DropConstraintBuilderTest.java

@@ -0,0 +1,42 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2021 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.sql;
+
+import java.util.List;
+import org.junit.Assert;
+import org.junit.Test;
+import org.sonar.db.dialect.Oracle;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DropConstraintBuilderTest {
+  @Test
+  public void fail_if_constraint_name_starts_with_pk() {
+    DropConstraintBuilder builder = new DropConstraintBuilder(new Oracle());
+    Assert.assertThrows("This builder should not be used with primary keys", IllegalArgumentException.class, () -> builder.setName("pk_constraint"));
+  }
+
+  @Test
+  public void succeeds_for_oracle() {
+    DropConstraintBuilder builder = new DropConstraintBuilder(new Oracle());
+    List<String> queries = builder.setName("constraint1").setTable("table1").build();
+    assertThat(queries).containsOnly("ALTER TABLE table1 DROP CONSTRAINT constraint1");
+  }
+}

server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v86/DropOrganizationInRulesMetadataTest.java

@@ -23,18 +23,21 @@ import java.sql.SQLException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.db.CoreDbTester;
+import org.sonar.server.platform.db.migration.sql.DbPrimaryKeyConstraintFinder;
+import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.step.MigrationStep;
 
 public class DropOrganizationInRulesMetadataTest {
   @Rule
-  public CoreDbTester dbTester = CoreDbTester.createForSchema(DropOrganizationInRulesMetadataTest.class, "schema.sql");
+  public CoreDbTester db = CoreDbTester.createForSchema(DropOrganizationInRulesMetadataTest.class, "schema.sql");
+  private final DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator = new DropPrimaryKeySqlGenerator(db.database(), new DbPrimaryKeyConstraintFinder(db.database()));
 
-  private MigrationStep underTest = new DropOrganizationInRulesMetadata(dbTester.database());
+  private MigrationStep underTest = new DropOrganizationInRulesMetadata(db.database(), dropPrimaryKeySqlGenerator);
 
   @Test
   public void column_has_been_dropped() throws SQLException {
     underTest.execute();
-    dbTester.assertColumnDoesNotExist("rules_metadata", "organization_uuid");
-    dbTester.assertPrimaryKey("rules_metadata", "pk_rules_metadata", "rule_uuid");
+    db.assertColumnDoesNotExist("rules_metadata", "organization_uuid");
+    db.assertPrimaryKey("rules_metadata", "pk_rules_metadata", "rule_uuid");
   }
 }

server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v87/DropOrgQualityGatesTableTest.java

@@ -23,6 +23,8 @@ import java.sql.SQLException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.db.CoreDbTester;
+import org.sonar.server.platform.db.migration.sql.DbPrimaryKeyConstraintFinder;
+import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.step.MigrationStep;
 
 public class DropOrgQualityGatesTableTest {
@@ -30,8 +32,9 @@ public class DropOrgQualityGatesTableTest {
 
   @Rule
   public CoreDbTester db = CoreDbTester.createForSchema(DropOrgQualityGatesTableTest.class, "schema.sql");
+  private final DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator = new DropPrimaryKeySqlGenerator(db.database(), new DbPrimaryKeyConstraintFinder(db.database()));
 
-  private MigrationStep underTest = new DropOrgQualityGatesTable(db.database());
+  private MigrationStep underTest = new DropOrgQualityGatesTable(db.database(), dropPrimaryKeySqlGenerator);
 
   @Test
   public void execute() throws SQLException {

server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v87/DropOrganizationAlmBindingsTableTest.java

@@ -23,6 +23,8 @@ import java.sql.SQLException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.db.CoreDbTester;
+import org.sonar.server.platform.db.migration.sql.DbPrimaryKeyConstraintFinder;
+import org.sonar.server.platform.db.migration.sql.DropPrimaryKeySqlGenerator;
 import org.sonar.server.platform.db.migration.step.MigrationStep;
 
 public class DropOrganizationAlmBindingsTableTest {
@@ -30,8 +32,9 @@ public class DropOrganizationAlmBindingsTableTest {
 
   @Rule
   public CoreDbTester db = CoreDbTester.createForSchema(DropOrganizationAlmBindingsTableTest.class, "schema.sql");
+  private final DropPrimaryKeySqlGenerator dropPrimaryKeySqlGenerator = new DropPrimaryKeySqlGenerator(db.database(), new DbPrimaryKeyConstraintFinder(db.database()));
 
-  private MigrationStep underTest = new DropOrganizationAlmBindingsTable(db.database());
+  private MigrationStep underTest = new DropOrganizationAlmBindingsTable(db.database(), dropPrimaryKeySqlGenerator);
 
   @Test
   public void execute() throws SQLException {