fix: transaction callbacks where not being called (#43)

* fix: transaction callbacks where not being called * fix stan Co-authored-by: 尾山貴康 <t-oyama@colopl.co.jp>
colopl · Sep 13, 2022 · 9b929c3 · 9b929c3
1 parent 4f1aefa
commit 9b929c3
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 2 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -22,3 +22,7 @@ parameters:
           path: src/Colopl/Spanner/Schema/Builder.php
         - message: '#^Parameter \#1 \$value of method Illuminate\\Database\\Schema\\Grammars\\Grammar::wrap\(\) expects Illuminate\\Database\\Query\\Expression\|string, Illuminate\\Database\\Schema\\ColumnDefinition given.$#'
           path: src/Colopl/Spanner/Schema/Grammar.php
+        - message: '#^Using nullsafe method call on non-nullable type Illuminate\\Database\\DatabaseTransactionsManager\. Use -> instead.$#'
+          path: src/Colopl/Spanner/Concerns/ManagesTransactions.php
+        - message: '#^Parameter \#1 \$connection of method Illuminate\\Database\\DatabaseTransactionsManager::.+\(\) expects string, string\|null given\.$#'
+          path: src/Colopl/Spanner/Concerns/ManagesTransactions.php
diff --git a/src/Colopl/Spanner/Concerns/ManagesTransactions.php b/src/Colopl/Spanner/Concerns/ManagesTransactions.php
@@ -36,7 +36,7 @@ trait ManagesTransactions
     /**
      * @var Transaction|null
      */
-    protected $currentTransaction;
+    protected ?Transaction $currentTransaction = null;
 
     protected bool $ignoreSessionNotFoundErrorOnRollback = false;
 
@@ -59,17 +59,27 @@ public function transaction(Closure $callback, $attempts = Database::MAX_RETRIES
             $return = $this->getSpannerDatabase()->runTransaction(function (Transaction $tx) use ($callback) {
                 try {
                     $this->currentTransaction = $tx;
+
                     $this->transactions++;
+
+                    $this->transactionsManager?->begin(
+                        $this->getName(), $this->transactions
+                    );
+
                     $this->fireConnectionEvent('beganTransaction');
+
                     $result = $callback($this);
+
                     $this->performSpannerCommit();
+
                     return $result;
                 } catch (Throwable $e) {
                     // if session is lost, there is no way to rollback transaction at all,
                     // so quietly ignore 'session not found' error in rollBack()
                     // and then abort current transaction and rerun everything again
                     $savedIgnoreError = $this->ignoreSessionNotFoundErrorOnRollback;
                     $exceptionToCheck = $e instanceof QueryException ? $e->getPrevious() : $e;
+
                     $this->ignoreSessionNotFoundErrorOnRollback =
                         $exceptionToCheck instanceOf NotFoundException
                         && $this->getSessionNotFoundMode() !== self::THROW_EXCEPTION
@@ -147,7 +157,7 @@ public function commit()
      * @return void
      * @throws AbortedException
      */
-    protected function performSpannerCommit()
+    protected function performSpannerCommit(): void
     {
         if ($this->transactions === 1 && $this->currentTransaction !== null) {
             $this->currentTransaction->commit();
@@ -157,6 +167,10 @@ protected function performSpannerCommit()
         if ($this->isTransactionFinished()) {
             $this->currentTransaction = null;
         }
+
+        if ($this->afterCommitCallbacksShouldBeExecuted()) {
+            $this->transactionsManager?->commit($this->getName());
+        }
     }
 
     /**

diff --git a/tests/Colopl/Spanner/TransactionTest.php b/tests/Colopl/Spanner/TransactionTest.php
@@ -166,6 +166,50 @@ public function testReadOnTransaction(): void
         $this->assertDatabaseHas($tableName, $insertRow);
     }
 
+    public function test_afterCommit(): void
+    {
+        $conn = $this->getDefaultConnection();
+
+        $tableName = self::TABLE_NAME_USER;
+        $qb = $conn->table($tableName);
+
+        $count = 0;
+        $conn->transaction(function (Connection $conn) use ($qb, &$count) {
+            $qb->insert(['userId' => $this->generateUuid(), 'name' => 't']);
+            $conn->afterCommit(static function() use (&$count) { $count++; });
+        });
+
+        // Should not be called on second try.
+        $conn->transaction(function (Connection $conn) use ($qb) {
+            $qb->insert(['userId' => $this->generateUuid(), 'name' => 't']);
+        });
+
+        $this->assertSame(1, $count);
+    }
+
+    public function test_afterCommit_not_called_on_rollback(): void
+    {
+        $conn = $this->getDefaultConnection();
+
+        $tableName = self::TABLE_NAME_USER;
+        $qb = $conn->table($tableName);
+
+        $count = 0;
+        try {
+            $conn->transaction(function (Connection $conn) use ($qb, &$count) {
+                $qb->insert(['userId' => $this->generateUuid(), 'name' => 't']);
+                $conn->afterCommit(static function() use (&$count) {
+                    $count++;
+                });
+                throw new RuntimeException('fail');
+            });
+        } catch (RuntimeException) {
+            // do nothing
+        }
+
+        $this->assertSame(0, $count);
+    }
+
     public function testRetrySuccess(): void
     {
         $conn = $this->getDefaultConnection();