Laravel - StripeのWebhookで署名チェックする

stripe + webhookを実装しようと日本語記事を読んだら、どれも署名とか認証チェックみたいなことが考慮されていないので不安になって調べたメモ。

CSRFトークンのチェックを無効にしておく

本題とは関係ないけど、CSRFを無効にしておかないと302エラーになる。
なのでVerifyCsrfToken$exceptにwebhookのエンドポイントを入れておく。

<?php
# app/Http/Middleare/VerifyCsrfToken.php
# ...
    protected $except = [
        # こんな感じで無効にするエンドポイントを追加
        'stripe/wh/*',
    ];
# ...

署名チェックの実装

Stripeは公式ドキュメントが本当によく出来ていて、そこにそのまま書いてある。
Checking Webhook Signatures | Stripe

サンプルコードも載っているので特に補足もないけど、Laravelで書くならこんな感じ。

<?php
# ...
try {
    $content   = $req->getContent();
    $signature = $req->header('Stripe-Signature');
    // stripeのwebhook管理画面から取得した署名(configには自分で追加する必要あり)
    $wh_secret = config('services.stripe.webhook_secret');
    $event     = \Stripe\Webhook::constructEvent($content, $signature, $wh_secret);

    // $eventを使った何かしらの処理

    // レスポンスは何でもいいけど、管理画面上の表示が見やすいのでjsonにしている
    return response()->json('ok', 200);
} catch(\UnexpectedValueException $e) {
    return response()->json('Invalid payload', 400);
} catch(\Stripe\Error\SignatureVerification $e) {
    return response()->json('Invalid signature', 400);
} catch(\Throwable $e) {
    return response()->json($e->getMessage(), 500);
}
# ...

IPホワイトリストでもチェックできそう

実際に動作確認はしていないけどwebhookはこれらのIPから来るとか。
Domains and IP Addresses | Stripe

PCのディスプレイは複数あるよりも1つの方が作業に集中できる

f:id:kanno_kanno:20190329083953p:plain

結論

人それぞれ。

背景

むかしWindowsのデスクトップPCで仕事をしていたとき、デュアルディスプレイは当然だと思っていた。
当時の僕にとっては間違いなくデュアルディスプレイの方が生産性は高かった。ディスプレイが1つだとストレスがあった。現場によっては小さなディスプレイだったので尚更だった。

WindowsのノートPCを使っていた時も外付けモニタを用意して作業していたと思う。もう10年近く前なのでうろ覚えだが、この時は開発よりも設計寄りの仕事だった。

それからMacBook Proで仕事するようになると、開発の有無に関わらずデュアルディスプレイを使わなくなっていった。
会議や外出含めて持ち歩くことが多く、その度にケーブルを付け外ししたり、ウィンドウを配置し直すのが面倒に感じ始めた。

MacBookは11インチ(Air)、13インチ(Pro)、15インチ(Pro)を経験してきた。11インチのころは外付けモニタを使っていたかもしれないが、13インチ以上では使わないことが多かった。

外付けモニタを使わない開発スタイルが染み付いたものの、どこかでモヤモヤは消えなかった。
デュアルディスプレイを使った方が開発効率は上がるんじゃないだろうかと。あと単純にカッコいい。

一方でデュアルディスプレイなら必ずしも生産性が上げるわけではない、とも思い始めていた。

デスクトップにしろノートPCにしろ、シングルディスプレイだけでめちゃくちゃ生産性高い人を何人も見てきたからだ。
もちろん支給されていないとかではない。

シングルディスプレイの方が集中できる理由

たまに外付けモニタを試しては「やっぱ違う」と何となく感じていたんだけど、今日ハッキリと理由が分かった。

僕は基本的に各アプリを画面いっぱいに広げて作業する。
ブラウザも、Vimも、IDEも、ターミナルも、Slackも。
Finderとか一時的に使うアプリは例外で、適当な小さいサイズで使う。使ったらすぐに閉じる。

画面いっぱいにしているのは単純な表示領域の問題だけじゃなくて、「今はこの作業をする」と脳のスイッチを切り替えるためだ。

開発に限らず僕は自分を単細胞でマルチタスクが苦手だと思っているので、1つのことに集中するというスタイルが合っている。
むかしデュアルディスプレイを使っていたころはまだ、自分のそういう特性を理解していなかった。

デュアルディスプレイにすると目に飛び込んでくる情報量が増えるわけで、それだけ集中力が低下する。
別にリアルタイムで見る必要もないのにSlackやTwitterの画面を常に表示している必要はない。用事があれば通知は飛んでくるので反応が遅れるわけでもない。

開いているだけでいじらなくても、表示が更新されないようなアプリであっても、そこにあるというだけで意識を持っていかれる。
僕にデュアルディスプレイが向かない大きな理由はこれだと思う。

その上で、単純な操作性にしてもコストが高いと感じる。

シングルディスプレイならキーボード操作だけで切り替えが可能だが、デュアルディスプレイだとキーボード操作に加えて目線の移動が必要になる。これがどうにも落ち着かない。慣れかもしれないが。

あと僕は仮想デスクトップを活用していて、Macなら3本指のスワイプだけで簡単に切り替えられる。
デュアルディスプレイの場合はわざわざカーソルを行き来しないといけない。これも非常に面倒くさい。

仮想デスクトップの利用は、シングルディスプレイに馴染んだ理由の一つとして大きいと思う。

デュアルディスプレイの方が向いていそうな状況

恐らく僕にはしばらく縁がないけど、作業より監視や閲覧がメインになるとデュアル(マルチ)ディスプレイの方がいいと思う。
これらの時は情報量が必要になるし、アプリや画面の切り替えが頻繁に発生しそうなので。

最初に書いたとおり人それぞれだと思うが、自分にとってどうかを言語化できてスッキリした。

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();
        }
    });
}

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

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

おまけ

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

fish - rangerの多重起動を避ける設定

rangerというCLIのファイラがあるのを知って最近使っている。
参考: CLI で Linux ファイルマネージャ ranger を使うことのメモ

Macならbrew install rangerで入る。homebrewほんと便利。

そんなrangerの設定というかカスタマイズで「多重起動を避ける」というのがある。
上記の参考サイトではrangerの多重起動を避けるという項目で説明されている。

ここで書かれている設定はbash/zsh向けなので、fishの場合の書き方をメモ。

rangerの多重起動を避ける

bash/zshではこうなっている。

ranger() { [ -n "$RANGER_LEVEL" ] && exit || command ranger "$@"; }

fishでは次の定義を~/.config/fish/config.fishに定義する。
(~/.config/fish/functions/配下にファイルを作っても良いんだろうけど)

# ranger->サブシェル->rangerの多重起動を避ける
function ranger
  if set -q -x RANGER_LEVEL
    exit
  else
    command ranger $argv
  end
end

参考サイトにある通りRANGER_LEVEL変数が定義されていればexitして、それ以外(初回)なら普通にrangerを起動する。
注意としてはサブシェルからranger /tmpのように引数有りで起動してもexitして元のrangerに戻るだけなので、引数で渡したディレクトリで起動しない。

rangerのサブシェルの場合はプロンプトに表示

bash/zshではこうなっている。

[ -n "$RANGER_LEVEL" ] && PS1="(RANGER) $PS1"

fishでは次の定義を~/.config/fish/config.fishに定義する。
(~/.config/fish/functions/配下にファイルを作っても良いんだろうけど)

# rangerのサブシェルから起動された場合はpromptを変更する
if set -q -x RANGER_LEVEL
  set -g _origin_prompt (fish_prompt)
  function fish_prompt
    echo "(RANGER)$_origin_prompt"
  end
end
  1. 上記と同じようにRANGER_LEVELが設定されているかどうかで、通常のシェルかrangerサブシェルかを判断
  2. (fish_prompt)の実行結果、つまり既存のプロンプト表示を変数に保持する
    • -gでグローバル定義にしないとfish_prompt関数から見れなかった。ちょっと気持ち悪い
  3. fish_promptを再定義する

これで↓のように(RANGER)が付いた状態でプロンプトが表示される。

(RANGER) ~/.config/fish ⟩

PHPのテストで本当にdataProviderを使う必要があるのだろうか

PHPUnitにはdataProviderという機能がある。
パラメタライズドテスト、またはテーブルドリブン(駆動)テストで書くための仕組み。

僕はずっとこの機能が使いづらくて、普通にforeachで回す方が良いのではと思っている。
今日たまたまdataProviderを推奨している記事を続けて目にしたので、改めて考えを整理しておく。

dataProviderのメリット

foreachと比較したdataProviderのメリットはこちらの記事が分かりやすかったので引用する。
PHPUnitとデータプロバイダとテストケース生成

メリット1: setUp/tearDownが使える

確かにモックを使うテストならその方が助かる。

メリット2: どのデータでテストが落ちたのか分かりやすい

確かにforeachだとちょっと面倒。Goでテストを書いている時にt.Error()をいちいち書いていく感覚に似てる。

メリット3: 例外のテストとの組み合わせ

確かに例外が発生した段階で止まっちゃうと面倒。

dataProviderの(個人的)デメリット

上記のメリットはどれも同意するけど、個人的にはそれ以上にデメリットを感じてしまう。
それはテストの本体とdataProviderのメソッドが離れていること。ロジックとデータは近いところにある方が読みやすいと思うけど好みなのかもしれない。

例えばPHPUnitリポジトリにあるこのテストコード。
https://github.com/sebastianbergmann/phpunit/blob/cca308e970eebb1f21cd702071b09bfd40a1c705/tests/unit/Util/RegularExpressionTest.php

  1. testValidRegexのテストを読もうとする
  2. validRegexpProviderの定義を見る

という視線の移動と記憶が必要になる。
これなら↓のようにテスト本体とデータは一緒にあった方が個人的には読みやすい。

<?php
public function testValidRegex(): void
{
    foreach([
        ['#valid regexp#', 'valid regexp', 1],
        [';val.*xp;', 'valid regexp', 1],
        ['/val.*xp/i', 'VALID REGEXP', 1],
        ['/a val.*p/', 'valid regexp', 0],
    ] as $case) {
        $this->assertEquals($case[2], RegularExpression::safeMatch($case[1], $case[0]));
    }
}

(エラーメッセージとか一時変数に置くとかは省いている)

もちろんdataProviderの方が適切というケースもあると思うけど、同様に普通にforeachの方が良いケースもあると思う。何でもかんでもdataProviderを使うのは違う気がする。
このテストとか冗長なだけなのでは。
ただ「使うかどうかは個人の判断」にしちゃうとテストの書き方に統一性がなくなるから、プロジェクトとしては「必ず使う」でルール化した方がいいのかもしれない。

また個人的に読みにくいと感じる理由の一つは、dataProvider側だけを見ても値の意図が分からないから。

<?php
public function canonicalizeProvider(): array
{
    return [
        ['{"name":"John","age":"35"}', '{"age":"35","name":"John"}', false],
        ['{"name":"John","age":"35","kids":[{"name":"Petr","age":"5"}]}', '{"age":"35","kids":[{"age":"5","name":"Petr"}],"name":"John"}', false],
        ['"name":"John","age":"35"}', '{"age":"35","name":"John"}', true],
    ];
}

これはphpunit/tests/unit/Util/JsonTest.phpから拝借した。
それぞれの値の意図は、ここを見ただけでは分からない。
テストコード側の引数を見て初めて理解する。

<?php
public function testCanonicalize($actual, $expected, $expectError): void

テストコードとデータを行き来しないと読み解けない。これが僕にはつらい。

結局どっちが良いのか

最初に引用したdataProviderのメリットのうち2つは、「テストが落ちた時のメンテ性を上げる」ためのものだった。
僕があげたdataProviderのデメリットは「他人がコードを読む時」を想定したものだった。

つまり優劣というより重要視する観点が違うんだと思う。僕は可読性を大事にしている。でも「dataProviderの方が見やすい。foreachの方が見づらい」って人もいるだろうから、そうなるとforeachの方が良い理由はほとんどないのかな。