Laravel - DB::transaction中でDB::rollbackを呼んでいるとRefreshDatabaseを使っているテストでロールバックされない

環境

Laravel 5.5
PostgreSQL
- でも他のDBでも同じはず

現象

use RefreshDatabaseを使ってテスト毎にロールバックしているにも関わらず、データが残ってしまい後続のテストが落ちる。

原因

業務コードの方でDB::transaction()を使い、その中でDB::rollback()を呼んでいたため。
(例外時のテストを実行した時にテストデータが残ってしまった)

こういうコード。

<?php
// ...

try {
    \DB::transaction(function() {
        $isError = $this->updateHoge();
        if ($isError) {
            \DB::rollback();
            return;
        }
    });
} catch (\Throwable $e) {
    // ごにょごにょ
}

解決方法

DB::rollback()じゃなくて例外を発生させる。

<?php
// ...

try {
    \DB::transaction(function() {
        $isError = $this->updateHoge();
        if ($isError) {
            throw new \Exception('hogehoge');
        }
    });
} catch (\Throwable $e) {
    // ごにょごにょ
}

詳細

まずLaravelのトランザクションの仕組みについて。
これは内部的に「トランザクションの階層」を保持していて、commitやrollbackが呼ばれた時にはこのプロパティを見て「実際に処理するかどうか」を判断している。

トランザクションを開始すると階層が+1されて、rollbackやcommitをすると階層が-1される。

例えば階層が1の時にcommitが呼ばれればそのままcommitするが、2以上なら「現在の階層」をいじるだけで何もしない。

<?php
// laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:153
public function commit()
{
    if ($this->transactions == 1) {
        $this->getPdo()->commit();
    }

    $this->transactions = max(0, $this->transactions - 1);

    $this->fireConnectionEvent('committed');
}

次にDB::transaction()について。
これは引数で受け取った関数を実行したらcommitするようになっている。

<?php
// laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:20
try {
    return tap($callback($this), function ($result) {
        $this->commit();
    });
} catch (Exception $e) {
    $this->handleTransactionException(
        $e, $currentAttempt, $attempts
    );
} catch (Throwable $e) {
    $this->rollBack();

    throw $e;
}

つまりDB::transaction()内でDB::rollback()を呼んでreturnしても、最終的にcommitは呼ばれてしまう。
通常は問題ない。rollback()しているならこの時点で$this->transactionsは0になっているので、実際の$this->getPdo()->commit();は呼ばれないからだ。

問題が起きるのはタイトルにある通りテストでuse RefreshDatabaseを使っている時に起きる。
(あとたぶん業務コードでもネストしたトランザクションで書いていると問題が起きそうだけど、自分のケースではなかったので考えてない)

RefreshDatabaseをuseすると、TestCaseクラスのsetUpによってbeginDatabaseTransactionが呼ばれる。

<?php
// laravel/framework/src/Illuminate/Foundation/Testing/RefreshDatabase.php:68
public function beginDatabaseTransaction()
{
    $database = $this->app->make('db');

    foreach ($this->connectionsToTransact() as $name) {
        $database->connection($name)->beginTransaction();
    }

    $this->beforeApplicationDestroyed(function () use ($database) {
        foreach ($this->connectionsToTransact() as $name) {
            $database->connection($name)->rollBack();
        }
    });
}

テストケースの開始時にトランザクションを開始して、テストが終了する時にロールバックする。
ここまでの情報をまとめると、次のように動く。

RefreshDatabase::beginTransaction()によりtransactions=1になる
テストデータを入れる -- Aとする
業務コードを呼び出す
DB::transaction()によりtransactions=2になる
適当にデータ入れる -- Bとする
DB::rollback()を実行する
- -1してもtransactions=1なので(0じゃないので)、完全にロールバックしない
- スルーされるかセーブポイントに戻るかは環境次第。この例ではスルー扱いで進める
DB::transaction()を抜ける時にコミットが走る
- transactions=1なので、commitが走る => AとBがcommitされる
- commit後に-1してtransactions=0になる
RefreshDatabaseのrollbackが走るがtransactions=0だし、未コミットのデータもないので何も起きない
結果としてAやBが残る

おまけ

そもそもPHPだとクロージャを使いづらくてコードが汚く見えるので(useを使って引き継がないといけないせい)、素直にDB::transaction()じゃなくてDB::beginTransaction()によるtry-catchの方が良い気がしてきた。