Fix SchemaTest::testChangeSerialFieldLength for Oracle (#322)

mondrake · Apr 25, 2023 · bf91442 · bf91442
1 parent caa452a
commit bf91442
Show file tree

Hide file tree

Showing 6 changed files with 391 additions and 39 deletions.
diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml
@@ -30,7 +30,7 @@ jobs:
 
     services:
       oracle:
-        image: gvenzl/oracle-xe:21
+        image: gvenzl/oracle-xe:slim-faststart
         env:
           ORACLE_PASSWORD: oracle
         ports:

diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -11,30 +11,43 @@ env:
     PHPUNIT_SKIP_CLASS: '[
       "Drupal\\KernelTests\\Core\\Cache\\ApcuBackendTest",
       "Drupal\\Tests\\file\\Functional\\FileAddPermissionsUpdateTest",
-      "Drupal\\Tests\\file\\Functional\\DownloadTest",
       "Drupal\\KernelTests\\Core\\Database\\SchemaUniquePrefixedKeysIndexTest"
     ]'
 
 jobs:
 
 #################################
 
-  sqlite-pdo:
-    name: "SQLite with PDO"
+  oracle-oci8:
+    name: "Oracle on Oci8"
     runs-on: ubuntu-20.04
     env:
-        DRUDBAL_ENV: "dbal/sqlite/file"
-        DBAL_URL: "sqlite://localhost/sites/drudbal.sqlite"
-        SIMPLETEST_DB: "dbal://localhost/sites/drudbal.sqlite?module=drudbal&dbal_driver=pdo_sqlite"
+        DRUDBAL_ENV: "dbal/oci8"
+        DBAL_URL: "oci8://DRUDBAL:[email protected]:1521/XE"
+        SIMPLETEST_DB: "dbal://DRUDBAL:[email protected]:1521/XE?module=drudbal&dbal_driver=oci8#dru"
+
+    services:
+      oracle:
+        image: gvenzl/oracle-xe:slim-faststart
+        env:
+          ORACLE_PASSWORD: oracle
+        ports:
+          - "1521:1521"
+        options: >-
+          --health-cmd healthcheck.sh
+          --health-interval 20s
+          --health-timeout 10s
+          --health-retries 10
 
     strategy:
       fail-fast: false
       matrix:
         php-version:
           - "8.1"
-#          - "8.2"
         test-args:
-          - "--group Database"
+          - "-v modules/contrib/drudbal/tests/src/Kernel/dbal/SchemaBisTest.php"
+          - "-v modules/contrib/drudbal/tests/src/Kernel/dbal/SchemaTest.php"
+#          - "--group Database"
 #          - "--group Entity"
 #          - "--group Cache,Config"
 #          - "--group field,Field"
@@ -47,6 +60,7 @@ jobs:
       with:
         php-version: "${{ matrix.php-version }}"
         coverage: "none"
+        extensions: "oci8"
         ini-values: "zend.assertions=1"
 
     - name: Checkout Drupal
@@ -63,21 +77,31 @@ jobs:
     - name: Install Composer dependencies
       run: |
         composer install --no-progress --ansi
+        composer config --no-plugins allow-plugins.composer/package-versions-deprecated true
 
     - name: Composer require DruDbal from local staging
       run: |
         git -C drudbal_staging checkout -b test-run-branch
         composer config repositories.test-run '{"type": "path", "url": "drudbal_staging", "options": {"symlink": false}}'
         composer require "mondrake/drudbal:dev-test-run-branch" --no-progress --ansi
 
-    - name: Install Drupal
+    - name: Patch doctrine/dbal
+      run: |
+        curl https://patch-diff.githubusercontent.com/raw/mondrake/dbal/pull/6.diff | patch -d vendor/doctrine/dbal -p1
+
+    - name: Create Oracle schema
       run: |
         cp modules/contrib/drudbal/tests/github/install_* .
+        php install_oracle.php
+
+    - name: Install Drupal
+      run: |
         vendor/bin/drush site-install standard --db-url=$SIMPLETEST_DB -y
         vendor/bin/drush runserver localhost:8080 --default-server=localhost:8080 &
-        sleep 1s
+        sleep 5s
 
     - name: Report installation
+      continue-on-error: true
       run: |
         php install_report.php
         vendor/bin/drush core:status

diff --git a/README.md b/README.md
@@ -95,3 +95,4 @@ tbd                                                             | UpdateTestBase
 [#2992274](https://www.drupal.org/project/drupal/issues/2992274) | Installer tests fail if contrib driver hides database credentials form fields |
 [#3256642](https://www.drupal.org/project/drupal/issues/3256642) | Autoload classes of database drivers modules' dependencies |
 [#3347497](https://www.drupal.org/project/drupal/issues/3347497) | Introduce a FetchModeTrait to allow emulating PDO fetch modes |
+[#3355841](https://www.drupal.org/project/drupal/issues/3355841) | Allow DriverSpecificSchemaTestBase::testChangePrimaryKeyToSerial to execute for non-core drivers |
diff --git a/src/Driver/Database/dbal/DbalExtension/Oci8Extension.php b/src/Driver/Database/dbal/DbalExtension/Oci8Extension.php
@@ -821,13 +821,8 @@ public function delegateAddField(&$primary_key_processed_by_extension, DbalSchem
     $sql[] = "ALTER TABLE $db_table ADD $db_field $column_definition";
 
     if ($drupal_field_specs['type'] === 'serial') {
-      /** @var OraclePlatform $platform */
-      $platform = $this->connection->getDbalPlatform();
-      $autoincrement_sql = $platform->getCreateAutoincrementSql($db_field, $db_table);
-      // Remove the auto primary key generation, which is the first element in
-      // the array.
-      array_shift($autoincrement_sql);
-      $sql = array_merge($sql, $autoincrement_sql);
+      $sql[] = $this->getAutoincrementSequenceSql($unquoted_db_table);
+      $sql[] = $this->getAutoincrementTriggerSql($unquoted_db_table, $unquoted_db_field);
     }
 
     if (!$has_primary_key && $db_primary_key_columns) {
@@ -907,20 +902,35 @@ public function delegateChangeField(&$primary_key_processed_by_extension, DbalSc
       $db_primary_key_columns[$key] = $new_db_field;
     }
 
+    // Drop primary key if needed.
     if ($drop_primary_key) {
       $db_pk_constraint = '';
       $this->delegateDropPrimaryKey($primary_key_processed_by_extension, $db_pk_constraint, $dbal_schema, $drupal_table_name);
       $has_primary_key = FALSE;
     }
 
+    $sql = [];
+
+    // Drop autoincrement setup if new field is serial and setup exists
+    // already.
+    if ($new_db_field_is_serial) {
+      $sequenceName = "\"" . $unquoted_db_table . "_SEQ\"";
+      $autoincrementIdentifierName = "\"" . $unquoted_db_table ."_AI_PK\"";
+
+      $currentSchema = $this->getDbalConnection()->createSchemaManager()->introspectSchema();
+      if ($currentSchema->hasSequence($sequenceName)) {
+        $sql[] = 'DROP TRIGGER ' . $autoincrementIdentifierName;
+        $sql[] = 'DROP SEQUENCE ' . $sequenceName;
+      }
+    }
+
     $temp_column = $this->getLimitedIdentifier(str_replace('-', '', 'tmp' . (new Uuid())->generate()));
     $not_null = $drupal_field_new_specs['not null'] ?? FALSE;
     $column_definition = str_replace($db_field, "\"$temp_column\"", $dbal_column_options['columnDefinition']);
     if ($not_null) {
       $column_definition = str_replace("NOT NULL", "NULL", $column_definition);
     }
 
-    $sql = [];
     $sql[] = "ALTER TABLE $db_table ADD \"$temp_column\" $column_definition";
     $sql[] = "UPDATE $db_table SET \"$temp_column\" = $db_field";
     $sql[] = "ALTER TABLE $db_table DROP COLUMN $db_field";
@@ -929,31 +939,19 @@ public function delegateChangeField(&$primary_key_processed_by_extension, DbalSc
       $sql[] = "ALTER TABLE $db_table MODIFY $new_db_field NOT NULL";
     }
 
-    if ($new_db_field_is_serial) {
-      $prev_max_sequence = (int) $this->connection->query("SELECT MAX({$db_field}) FROM {$db_table}")->fetchField();
-      /** @var OraclePlatform $platform */
-      $platform = $this->connection->getDbalPlatform();
-      $autoincrement_sql = $platform->getCreateAutoincrementSql($new_db_field, $db_table);
-      // Remove the auto primary key generation, which is the first element in
-      // the array.
-      array_shift($autoincrement_sql);
-      // Get the the sequence generation, which is the second element in the
-      // array.
-      $sequence_sql = array_shift($autoincrement_sql);
-      if ($prev_max_sequence) {
-        $sql[] = str_replace('START WITH 1', 'START WITH ' . $prev_max_sequence, $sequence_sql);
-      }
-      else {
-        $sql[] = $sequence_sql;
-      }
-      $sql = array_merge($sql, $autoincrement_sql);
-    }
-
+    // Add primary key if needed.
     if (!$has_primary_key && $db_primary_key_columns) {
       $db_pk_constraint = $db_pk_constraint ?? $unquoted_db_table . '_PK';
       $sql[] = "ALTER TABLE $db_table ADD CONSTRAINT $db_pk_constraint PRIMARY KEY (" . implode(', ', $db_primary_key_columns) . ")";
     }
 
+    // Add autoincrement if needed.
+    if ($new_db_field_is_serial) {
+      $prev_max_sequence = (int) $this->connection->query("SELECT MAX({$db_field}) FROM {$db_table}")->fetchField();
+      $sql[] = $this->getAutoincrementSequenceSql($unquoted_db_table, $prev_max_sequence + 1);
+      $sql[] = $this->getAutoincrementTriggerSql($unquoted_db_table, $unquoted_new_db_field);
+    }
+
     if (isset($drupal_field_new_specs['description'])) {
       $column_description = $this->connection->getDbalPlatform()->quoteStringLiteral($drupal_field_new_specs['description']);
       $sql[] = "COMMENT ON COLUMN $db_table.$new_db_field IS " . $column_description;
@@ -993,9 +991,40 @@ public function delegateDropPrimaryKey(bool &$primary_key_dropped_by_extension,
     assert(is_object($db_constraint));
     $primary_key_asset_name = $db_constraint->name;
     $exec = "ALTER TABLE $db_table DROP CONSTRAINT \"$primary_key_asset_name\"";
+    if ($this->getDebugging()) dump($exec);
     $this->getDbalConnection()->executeStatement($exec);
     $primary_key_dropped_by_extension = TRUE;
     return TRUE;
   }
 
+  private function getAutoincrementSequenceSql(string $unquotedTableName, int $start = 1): string {
+    return "CREATE SEQUENCE \"{$unquotedTableName}_SEQ\" START WITH {$start} MINVALUE {$start} INCREMENT BY 1";
+  }
+
+  private function getAutoincrementTriggerSql(string $unquotedTableName, string $unquotedColumnName): string {
+    $unquotedSequenceName = $unquotedTableName . '_SEQ';
+    return "
+CREATE OR REPLACE TRIGGER \"{$unquotedTableName}_AI_PK\"
+   BEFORE INSERT
+   ON \"{$unquotedTableName}\"
+   FOR EACH ROW
+DECLARE
+   pragma autonomous_transaction;
+   last_Sequence NUMBER;
+   last_InsertID NUMBER;
+BEGIN
+   IF (:NEW.\"{$unquotedColumnName}\" IS NULL) THEN
+      SELECT \"{$unquotedSequenceName}\".NEXTVAL INTO :NEW.\"{$unquotedColumnName}\" FROM DUAL;
+   ELSE
+      SELECT :NEW.\"{$unquotedColumnName}\" INTO last_InsertID FROM DUAL;
+      SELECT (\"{$unquotedSequenceName}\".NEXTVAL - 1) INTO last_Sequence FROM DUAL;
+      IF (last_InsertID > last_Sequence) THEN
+         EXECUTE IMMEDIATE 'alter sequence \"{$unquotedSequenceName}\" INCREMENT BY ' || TO_CHAR(last_InsertID - last_Sequence -1);
+         SELECT \"{$unquotedSequenceName}\".NEXTVAL INTO last_Sequence FROM DUAL;
+         EXECUTE IMMEDIATE 'alter sequence \"{$unquotedSequenceName}\" INCREMENT BY 1';
+      END IF;
+   END IF;
+END;";
+  }
+
 }
diff --git a/tests/github/drupal_patch.sh b/tests/github/drupal_patch.sh
@@ -12,3 +12,6 @@ git apply -v ./drudbal_staging/tests/github/2992274-local.patch
 
 #3347497 Introduce a FetchModeTrait to allow emulating PDO fetch modes
 curl https://git.drupalcode.org/project/drupal/-/merge_requests/3676.diff | git apply -v
+
+#3355841 Allow DriverSpecificSchemaTestBase::testChangePrimaryKeyToSerial to execute for non-core drivers
+curl https://www.drupal.org/files/issues/2023-04-23/3355841-2.patch | git apply -v