From bf91442ddb2a0fc4f9a47c8eb15d5f726dcc5734 Mon Sep 17 00:00:00 2001 From: mondrake Date: Tue, 25 Apr 2023 15:09:49 +0200 Subject: [PATCH] Fix SchemaTest::testChangeSerialFieldLength for Oracle (#322) --- .github/workflows/oracle.yml | 2 +- .github/workflows/pr.yml | 44 ++- README.md | 1 + .../dbal/DbalExtension/Oci8Extension.php | 85 +++-- tests/github/drupal_patch.sh | 3 + tests/src/Kernel/dbal/SchemaBisTest.php | 295 ++++++++++++++++++ 6 files changed, 391 insertions(+), 39 deletions(-) create mode 100644 tests/src/Kernel/dbal/SchemaBisTest.php diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index fa0cfa55b..38d61f0bc 100644 --- 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 index cafe6e6e0..a72faedac 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -11,7 +11,6 @@ env: PHPUNIT_SKIP_CLASS: '[ "Drupal\\KernelTests\\Core\\Cache\\ApcuBackendTest", "Drupal\\Tests\\file\\Functional\\FileAddPermissionsUpdateTest", - "Drupal\\Tests\\file\\Functional\\DownloadTest", "Drupal\\KernelTests\\Core\\Database\\SchemaUniquePrefixedKeysIndexTest" ]' @@ -19,22 +18,36 @@ 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:ORACLE@0.0.0.0:1521/XE" + SIMPLETEST_DB: "dbal://DRUDBAL:ORACLE@0.0.0.0: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,6 +77,7 @@ 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: | @@ -70,14 +85,23 @@ jobs: 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 index 7602db0da..7ebdd84e8 100644 --- 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 index 8d32cec98..86111483b 100644 --- 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,12 +902,28 @@ 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']); @@ -920,7 +931,6 @@ public function delegateChangeField(&$primary_key_processed_by_extension, DbalSc $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 index 7134eadb3..5fc5b3099 100755 --- 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 diff --git a/tests/src/Kernel/dbal/SchemaBisTest.php b/tests/src/Kernel/dbal/SchemaBisTest.php new file mode 100644 index 000000000..edb3eb022 --- /dev/null +++ b/tests/src/Kernel/dbal/SchemaBisTest.php @@ -0,0 +1,295 @@ +connection; + assert($connection instanceof DruDbalConnection); + return $connection; + } + + /** + * Returns the DruDbal schema. + */ + private function schema(): DruDbalSchema { + $schema = $this->schema; + assert($schema instanceof DruDbalSchema); + return $schema; + } + + /** + * {@inheritdoc} + */ + public function checkSchemaComment(string $description, string $table, string $column = NULL): void { + $comment = $this->schema()->getComment($table, $column); + // The schema comment truncation for mysql is different. + if ($this->connection()->databaseType() === 'mysql') { + $max_length = $column ? 255 : 60; + $description = Unicode::truncate($description, $max_length, TRUE, TRUE); + } + $this->assertSame($description, $comment, 'The comment matches the schema description.'); + } + + /** + * {@inheritdoc} + */ + protected function assertCollation(): void { + if ($this->connection()->databaseType() === 'mysql') { + // Make sure that varchar fields have the correct collations. + $columns = $this->connection()->query('SHOW FULL COLUMNS FROM {test_table}'); + $string_check = null; + $string_ascii_check = null; + foreach ($columns as $column) { + if ($column->Field == 'test_field_string') { + $string_check = $column->Collation; + } + if ($column->Field == 'test_field_string_ascii') { + $string_ascii_check = $column->Collation; + } + } + $this->assertMatchesRegularExpression('#^(utf8mb4_general_ci|utf8mb4_0900_ai_ci)$#', $string_check, 'test_field_string should have a utf8mb4_general_ci or a utf8mb4_0900_ai_ci collation, but it has not.'); + $this->assertSame('ascii_general_ci', $string_ascii_check, 'test_field_string_ascii should have a ascii_general_ci collation, but it has not.'); + } + } + + /** + * {@inheritdoc} + */ + protected function tryInsertExpectsIntegrityConstraintViolationException(string $tableName): void { + if ($this->connection()->databaseType() !== 'sqlite') { + parent::tryInsertExpectsIntegrityConstraintViolationException($tableName); + } + } + + /** + * Tests that indexes on string fields are limited to 191 characters on MySQL. + * + * @see \Drupal\mysql\Driver\Database\mysql\Schema::getNormalizedIndexes() + */ + public function testIndexLength(): void { + $this->markTestSkipped('test'); + } + + /** + * {@inheritdoc} + */ + public function testTableWithSpecificDataType(): void { + $this->markTestSkipped('test'); + } + + /** + * @covers \Drupal\drudbal\Driver\Database\dbal\Schema::introspectIndexSchema + * + * In this override, we need to change Oracle index names, since they cannot + * exceed the 30 chars limit in Oracle 11. + */ + public function testIntrospectIndexSchema(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests the findTables() method. + */ + public function testFindTables(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests handling of uppercase table names. + */ + public function testUpperCaseTableName(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests adding columns to an existing table with default and initial value. + * + * In this override, we need to change maximum precision in Oracle, that is + * 38, differently from other core databases. + */ + public function testSchemaAddFieldDefaultInitial(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests creating unsigned columns and data integrity thereof. + * + * In this override, we avoid testing insert on the serial column that in + * Drupal core raises an exception, but not in Oracle where a trigger forces + * the value to be next-in-sequence regardless of what is passed in. + */ + public function testUnsignedColumns(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests handling with reserved keywords for naming tables, fields and more. + */ + public function testReservedKeywordsForNaming(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests database interactions. + */ + public function testSchema(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests various schema changes' effect on the table's primary key. + * + * @param array $initial_primary_key + * The initial primary key of the test table. + * @param array $renamed_primary_key + * The primary key of the test table after renaming the test field. + * + * @dataProvider providerTestSchemaCreateTablePrimaryKey + * + * @covers ::addField + * @covers ::changeField + * @covers ::dropField + * @covers ::findPrimaryKeyColumns + */ + public function testSchemaChangePrimaryKey(array $initial_primary_key, array $renamed_primary_key): void { + $this->markTestSkipped('test'); + } + + /** + * Provides test cases for SchemaTest::testSchemaCreateTablePrimaryKey(). + * + * @return array + * An array of test cases for SchemaTest::testSchemaCreateTablePrimaryKey(). + */ + public function providerTestSchemaCreateTablePrimaryKey() { + $tests = []; + + $tests['simple_primary_key'] = [ + 'initial_primary_key' => ['test_field'], + 'renamed_primary_key' => ['test_field_renamed'], + ]; + $tests['composite_primary_key'] = [ + 'initial_primary_key' => ['test_field', 'other_test_field'], + 'renamed_primary_key' => ['test_field_renamed', 'other_test_field'], + ]; + $tests['composite_primary_key_different_order'] = [ + 'initial_primary_key' => ['other_test_field', 'test_field'], + 'renamed_primary_key' => ['other_test_field', 'test_field_renamed'], + ]; + + return $tests; + } + + /** + * Tests an invalid field specification as a primary key on table creation. + */ + public function testInvalidPrimaryKeyOnTableCreation(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests converting an int to a serial when the int column has data. + */ + public function testChangePrimaryKeyToSerial(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests adding an invalid field specification as a primary key. + */ + public function testInvalidPrimaryKeyAddition(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests changing the primary key with an invalid field specification. + */ + public function testInvalidPrimaryKeyChange(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests changing columns between types with default and initial values. + */ + public function testSchemaChangeFieldDefaultInitial(): void { + $this->markTestSkipped('test'); + } + + /** + * @covers ::findPrimaryKeyColumns + */ + public function testFindPrimaryKeyColumns(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests default values after altering table. + */ + public function testDefaultAfterAlter(): void { + $this->markTestSkipped('test'); + } + + /** + * Tests changing a field length. + */ + public function testChangeSerialFieldLength(): void { + $specification = [ + 'fields' => [ + 'id' => [ + 'type' => 'serial', + 'not null' => TRUE, + 'description' => 'Primary Key: Unique ID.', + ], + 'text' => [ + 'type' => 'text', + 'description' => 'A text field', + ], + ], + 'primary key' => ['id'], + ]; + $this->schema->createTable('change_serial_to_big', $specification); + + // Increase the size of the field. + $new_specification = [ + 'size' => 'big', + 'type' => 'serial', + 'not null' => TRUE, + 'description' => 'Primary Key: Unique ID.', + ]; + $this->schema->changeField('change_serial_to_big', 'id', 'id', $new_specification); + $this->assertTrue($this->schema->fieldExists('change_serial_to_big', 'id')); + + // Test if we can actually add a big int. + $id = $this->connection->insert('change_serial_to_big')->fields([ + 'id' => 21474836470, + ])->execute(); + + $id_two = $this->connection->insert('change_serial_to_big')->fields([ + 'text' => 'Testing for ID generation', + ])->execute(); + + $this->assertEquals($id + 1, $id_two); + } + +}