diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 3ec55af..ecafed3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -104,7 +104,7 @@ jobs: continue-on-error: true run: vendor/bin/phpunit -c core --color=always ${{ matrix.test-args }} - - uses: actions/upload-artifact@v3 - with: - name: test-results - path: sites/simpletest/browser_output +# - uses: actions/upload-artifact@v3 +# with: +# name: test-results +# path: sites/simpletest/browser_output diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1b32498..9fea9bb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-20.04 strategy: - fail-fast: true + fail-fast: false matrix: php-version: - "8.1" diff --git a/README.md b/README.md index 1090b3c..be52153 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ Requires patches for the following issues to be applied: Issue | Description -------------------|----------------------------------------------------------------------------------------------| #3110546 | Allow contributed modules (mostly database drivers) to override tests in core | +#3364706 | Refactor transactions | Known issues diff --git a/src/Driver/Database/mysqli/Connection.php b/src/Driver/Database/mysqli/Connection.php index 912824b..1c62269 100644 --- a/src/Driver/Database/mysqli/Connection.php +++ b/src/Driver/Database/mysqli/Connection.php @@ -4,12 +4,14 @@ use Drupal\Core\Database\Connection as BaseConnection; use Drupal\Core\Database\Database; +use Drupal\Core\Database\Transaction\TransactionManagerInterface; use Drupal\Core\Database\TransactionNameNonUniqueException; use Drupal\Core\Database\TransactionNoActiveException; use Drupal\Core\Database\TransactionOutOfOrderException; use Drupal\mysql\Driver\Database\mysql\Connection as BaseMySqlConnection; use Drupal\mysqli\Driver\Database\mysqli\Parser\Parser; use Drupal\mysqli\Driver\Database\mysqli\Parser\Visitor; + /** * MySQLi implementation of \Drupal\Core\Database\Connection. */ @@ -181,151 +183,6 @@ public function lastInsertId(?string $name = NULL): string { return $this->connection->insert_id; } - /** - * {@inheritdoc} - */ - public function pushTransaction($name) { -// global $xxx; if ($xxx) dump(['pushTransaction in', $name]); - if (isset($this->transactionLayers[$name])) { - throw new TransactionNameNonUniqueException($name . " is already in use."); - } - // If we're already in a transaction then we want to create a savepoint - // rather than try to create another transaction. - if ($this->inTransaction()) { -// if ($xxx) dump(['pushTransaction savepoint', $name]); - $this->connection->savepoint($name); - } - else { -// if ($xxx) dump(['pushTransaction begin_transaction', $name]); - $this->connection->begin_transaction(0, $name); - } - $this->transactionLayers[$name] = $name; -// if ($xxx) dump(['pushTransaction out', $this->transactionLayers]); - } - - /** - * {@inheritdoc} - * - * mysqli does not support query('RELEASE SAVEPOINT ' . $name), we - * need to use direct rollback on the connection. - */ - protected function popCommittableTransactions() { -// global $xxx; if ($xxx) dump(['popCommittableTransactions in', $this->transactionLayers]); - // Commit all the committable layers. - foreach (array_reverse($this->transactionLayers) as $name => $active) { - // Stop once we found an active transaction. - if ($active) { - break; - } - - // If there are no more layers left then we should commit. - unset($this->transactionLayers[$name]); - if (empty($this->transactionLayers)) { -//dump(['popCommittableTransactions 1', $name]); - $this->doCommit(); - } - else { -//dump(['popCommittableTransactions 2', $name]); - if (!$this->connection->release_savepoint($name)) { -//dump(['popCommittableTransactions 3', $name]); - $this->transactionLayers = []; - $this->doCommit(); - } - } - } -// if ($xxx) dump(['popCommittableTransactions out', $this->transactionLayers]); - } - - /** - * {@inheritdoc} - * - * mysqli does not support query('ROLLBACK TO SAVEPOINT ' . $savepoint), we - * need to use direct rollback on the connection. - */ - public function rollBack($savepoint_name = 'drupal_transaction') { -// global $xxx; if ($xxx) dump(['rollBack in', $savepoint_name, $this->transactionLayers]); - if (!$this->inTransaction()) { - throw new TransactionNoActiveException(); - } - // A previous rollback to an earlier savepoint may mean that the savepoint - // in question has already been accidentally committed. - if (!isset($this->transactionLayers[$savepoint_name])) { - throw new TransactionNoActiveException(); - } - - // We need to find the point we're rolling back to, all other savepoints - // before are no longer needed. If we rolled back other active savepoints, - // we need to throw an exception. - $rolled_back_other_active_savepoints = FALSE; - while ($savepoint = array_pop($this->transactionLayers)) { - if ($savepoint == $savepoint_name) { - // If it is the last the transaction in the stack, then it is not a - // savepoint, it is the transaction itself so we will need to roll back - // the transaction rather than a savepoint. - if (empty($this->transactionLayers)) { -//dump(['rollBack 2', $savepoint_name, $this->transactionLayers]); - break; - } -//dump($this->query('SELECT * FROM {test}')->fetchAll()); -// $success = $this->connection->rollback(0, $savepoint); - $success = $this->connection->query('ROLLBACK TO SAVEPOINT ' . $savepoint_name); -//dump(['rollBack 3', $savepoint_name, $this->transactionLayers, $success]); -//dump($this->query('SELECT * FROM {test}')->fetchAll()); - $this->popCommittableTransactions(); - if ($rolled_back_other_active_savepoints) { - throw new TransactionOutOfOrderException(); - } - return; - } - else { -//dump(['rollBack 4', $savepoint, $savepoint_name, $this->transactionLayers]); - $rolled_back_other_active_savepoints = TRUE; - } -// if ($xxx) dump(['rollBack out', $savepoint_name, $this->transactionLayers]); - } - - // Notify the callbacks about the rollback. - $callbacks = $this->rootTransactionEndCallbacks; - $this->rootTransactionEndCallbacks = []; - foreach ($callbacks as $callback) { - call_user_func($callback, FALSE); - } - -//dump(['in rollback 1']); - if (!$this->connection->rollBack()) { -//dump(['in rollback 2']); - trigger_error('Invalid rollback', E_USER_WARNING); - } - if ($rolled_back_other_active_savepoints) { - throw new TransactionOutOfOrderException(); - } - } - - /** - * {@inheritdoc} - */ - protected function doCommit() { - try { - $this->connection->commit(); - $success = TRUE; - } - catch (\mysqli_sql_exception $e) { - $success = FALSE; - } - - if (!empty($this->rootTransactionEndCallbacks)) { - $callbacks = $this->rootTransactionEndCallbacks; - $this->rootTransactionEndCallbacks = []; - foreach ($callbacks as $callback) { - call_user_func($callback, $success); - } - } - - if (!$success) { - throw new TransactionCommitFailedException(); - } - } - /** * @todo */ @@ -356,4 +213,18 @@ public function exceptionHandler() { return new ExceptionHandler(); } + /** + * {@inheritdoc} + */ + protected function driverTransactionManager(): TransactionManagerInterface { + return new TransactionManager($this); + } + + /** + * {@inheritdoc} + */ + public function startTransaction($name = '') { + return $this->transactionManager()->push($name); + } + } diff --git a/src/Driver/Database/mysqli/TransactionManager.php b/src/Driver/Database/mysqli/TransactionManager.php new file mode 100644 index 0000000..64718d3 --- /dev/null +++ b/src/Driver/Database/mysqli/TransactionManager.php @@ -0,0 +1,60 @@ +connection->getClientConnection()->begin_transaction(); + } + + /** + * {@inheritdoc} + */ + protected function addClientSavepoint(string $name): bool { + return $this->connection->getClientConnection()->savepoint($name); + } + + /** + * {@inheritdoc} + */ + protected function rollbackClientSavepoint(string $name): bool { + // Mysqli does not have a rollback_to_savepoint method, and it does not + // allow a prepared statement for 'ROLLBACK TO SAVEPOINT', so we need to + // fallback to query on the client connection directly. + return (bool) $this->connection->getClientConnection()->query('ROLLBACK TO SAVEPOINT ' . $name); + } + + /** + * {@inheritdoc} + */ + protected function releaseClientSavepoint(string $name): bool { + return $this->connection->getClientConnection()->release_savepoint($name); + } + + /** + * {@inheritdoc} + */ + protected function rollbackClientTransaction(): bool { + return $this->connection->getClientConnection()->rollback(); + } + + /** + * {@inheritdoc} + */ + protected function commitClientTransaction(): bool { + return $this->connection->getClientConnection()->commit(); + } + +} diff --git a/tests/github/drupal_patch.sh b/tests/github/drupal_patch.sh index 99faf77..83544a1 100755 --- a/tests/github/drupal_patch.sh +++ b/tests/github/drupal_patch.sh @@ -3,5 +3,8 @@ #3110546 Allow contributed modules (mostly database drivers) to override tests in core curl https://git.drupalcode.org/project/drupal/-/merge_requests/291.diff | git apply -v +#3364706 Refactor transactions +curl https://git.drupalcode.org/project/drupal/-/merge_requests/4101.diff | git apply -v + # Extra patch # git apply -v ./mysqli_staging/tests/github/extra_patch.patch diff --git a/tests/src/Kernel/mysqli/TransactionTest.php b/tests/src/Kernel/mysqli/TransactionTest.php index 523a24b..cf5130f 100644 --- a/tests/src/Kernel/mysqli/TransactionTest.php +++ b/tests/src/Kernel/mysqli/TransactionTest.php @@ -104,4 +104,13 @@ public function testTransactionWithDdlStatement() { } } + /** + * Tests deprecation of Connection methods. + * + * @group legacy + */ + public function testConnectionDeprecations(): void { + $this->markTestSkipped('Skipping this for mysqli'); + } + }