Verified Commit d322b132 authored by Lee Rowlands's avatar Lee Rowlands
Browse files

Issue #3440848 by mondrake, quietone, daffie: Ensure post transaction...

Issue #3440848 by mondrake, quietone, daffie: Ensure post transaction callbacks are only at the end of the root Drupal transaction

(cherry picked from commit f276f6a9)
parent 42a97ff4
Loading
Loading
Loading
Loading
Loading
+44 −6
Original line number Diff line number Diff line
@@ -23,6 +23,26 @@
 */
abstract class TransactionManagerBase implements TransactionManagerInterface {

  /**
   * The ID of the root Transaction object.
   *
   * The unique identifier of the first 'root' transaction object created, when
   * the stack is empty.
   *
   * Normally, during the transaction stack lifecycle only one 'root'
   * Transaction object is processed. Any post transaction callbacks are only
   * processed during its destruction. However, there are cases when there
   * could be multiple 'root' transaction objects in the stack. For example: a
   * 'root' transaction object is opened, then a DDL statement is executed in a
   * database that does not support transactional DDL, and because of that,
   * another 'root' is opened before the original one is closed.
   *
   * Keeping track of the first 'root' created allows us to process the post
   * transaction callbacks only during its destruction and not during
   * destruction of another one.
   */
  private ?string $rootId = NULL;

  /**
   * The stack of Drupal transactions currently active.
   *
@@ -234,11 +254,19 @@ public function push(string $name = ''): Transaction {
      throw new TransactionNameNonUniqueException("A transaction named {$name} is already in use. Active stack: " . $this->dumpStackItemsAsString());
    }

    // Define a unique ID for the transaction.
    $id = uniqid('', TRUE);

    // Do the client-level processing.
    if ($this->stackDepth() === 0) {
      $this->beginClientTransaction();
      $type = StackItemType::Root;
      $this->setConnectionTransactionState(ClientConnectionTransactionState::Active);
      // Only set ::rootId if there's not one set already, which may happen in
      // case of broken transactions.
      if ($this->rootId === NULL) {
        $this->rootId = $id;
      }
    }
    else {
      // If we're already in a Drupal transaction then we want to create a
@@ -248,9 +276,6 @@ public function push(string $name = ''): Transaction {
      $type = StackItemType::Savepoint;
    }

    // Define an unique id for the transaction.
    $id = uniqid('', TRUE);

    // Add an item on the stack, increasing its depth.
    $this->addStackItem($id, new StackItem($name, $type));

@@ -262,6 +287,18 @@ public function push(string $name = ''): Transaction {
   * {@inheritdoc}
   */
  public function unpile(string $name, string $id): void {
    // If this is a 'root' transaction, and it is voided (that is, no longer in
    // the stack), then the transaction on the database is no longer active. An
    // action such as a rollback, or a DDL statement, was executed that
    // terminated the database transaction. So, we can process the post
    // transaction callbacks.
    if (!isset($this->stack()[$id]) && isset($this->voidedItems[$id]) && $this->rootId === $id) {
      $this->processPostTransactionCallbacks();
      $this->rootId = NULL;
      unset($this->voidedItems[$id]);
      return;
    }

    // If the $id does not correspond to the one in the stack for that $name,
    // we are facing an orphaned Transaction object (for example in case of a
    // DDL statement breaking an active transaction). That should be listed in
@@ -289,6 +326,10 @@ public function unpile(string $name, string $id): void {
        // If this was the root Drupal transaction, we can commit the client
        // transaction.
        $this->processRootCommit();
        if ($this->rootId === $id) {
          $this->processPostTransactionCallbacks();
          $this->rootId = NULL;
        }
      }
      else {
        // The stack got corrupted.
@@ -420,7 +461,6 @@ protected function getConnectionTransactionState(): ClientConnectionTransactionS
   * Processes the root transaction rollback.
   */
  protected function processRootRollback(): void {
    $this->processPostTransactionCallbacks();
    $this->rollbackClientTransaction();
  }

@@ -432,7 +472,6 @@ protected function processRootRollback(): void {
   */
  protected function processRootCommit(): void {
    $clientCommit = $this->commitClientTransaction();
    $this->processPostTransactionCallbacks();
    if (!$clientCommit) {
      throw new TransactionCommitFailedException();
    }
@@ -534,7 +573,6 @@ public function voidClientTransaction(): void {
      $this->voidStackItem((string) $i);
    }
    $this->setConnectionTransactionState(ClientConnectionTransactionState::Voided);
    $this->processPostTransactionCallbacks();
  }

}
+46 −4
Original line number Diff line number Diff line
@@ -777,18 +777,60 @@ public function testRootTransactionEndCallbackCalledOnCommit(): void {
  /**
   * Tests post-transaction callback executes after transaction rollback.
   */
  public function testRootTransactionEndCallbackCalledOnRollback(): void {
  public function testRootTransactionEndCallbackCalledAfterRollbackAndDestruction(): void {
    $transaction = $this->createRootTransaction('', FALSE);
    $this->connection->transactionManager()->addPostTransactionCallback([$this, 'rootTransactionCallback']);
    $this->insertRow('row');
    $this->assertNull($this->postTransactionCallbackAction);

    // Callbacks are processed only when destructing the transaction.
    // Executing a rollback is not sufficient by itself.
    $transaction->rollBack();
    $this->assertSame('rtcRollback', $this->postTransactionCallbackAction);
    $this->assertNull($this->postTransactionCallbackAction);
    $this->assertRowAbsent('rtcCommit');
    $this->assertRowAbsent('rtcRollback');
    $this->assertRowAbsent('row');

    // Destruct the transaction.
    unset($transaction);

    // The post-transaction callback should now have inserted a 'rtcRollback'
    // row.
    $this->assertSame('rtcRollback', $this->postTransactionCallbackAction);
    $this->assertRowAbsent('rtcCommit');
    $this->assertRowPresent('rtcRollback');
    $this->assertRowAbsent('row');
    // The row insert should be missing since the client rollback occurs after
    // the processing of the callbacks.
  }

  /**
   * Tests post-transaction callback executes after a DDL statement.
   */
  public function testRootTransactionEndCallbackCalledAfterDdlAndDestruction(): void {
    $transaction = $this->createRootTransaction('', FALSE);
    $this->connection->transactionManager()->addPostTransactionCallback([$this, 'rootTransactionCallback']);
    $this->insertRow('row');
    $this->assertNull($this->postTransactionCallbackAction);

    // Callbacks are processed only when destructing the transaction.
    // Executing a DDL statement is not sufficient itself.
    // We cannot use truncate here, since it has protective code to fall back
    // to a transactional delete when in transaction. We drop an unrelated
    // table instead.
    $this->connection->schema()->dropTable('test_people');
    $this->assertNull($this->postTransactionCallbackAction);
    $this->assertRowAbsent('rtcCommit');
    $this->assertRowAbsent('rtcRollback');
    $this->assertRowPresent('row');

    // Destruct the transaction.
    unset($transaction);

    // The post-transaction callback should now have inserted a 'rtcCommit'
    // row.
    $this->assertSame('rtcCommit', $this->postTransactionCallbackAction);
    $this->assertRowPresent('rtcCommit');
    $this->assertRowAbsent('rtcRollback');
    $this->assertRowPresent('row');
  }

  /**