OTOBANK Engineering Blog

オトバンクはコンテンツが大好きなエンジニアを募集しています！

トップ > Doctrine2 > PHPStan で Doctrine Criteria で使ってるフィールドを検証できるようにした

PHPStan で Doctrine Criteria で使ってるフィールドを検証できるようにした

Doctrine2 PHPStan PHP

全国1億2000万のDoctrineファンのみなさん。こんにちは @kalibora です。

Doctrine 使ってますか!! Eloquent な皆さんはここで帰っても大丈夫です。

さて、 Doctrine を使っているのであれば Criteria を使っているという方も多いかと思います。
Criteria ってどんなんだっけ？という方は

Doctrine2 四方山話 ( Fetch mode, Index by, Criteria について) - OTOBANK Engineering Blog の Criteria の項目。

または

Doctrine Criteriaの使いドコロ | QUARTETCOM TECH BLOG

あたりを読んでみてください。

僕は待ちます。

読みましたか？

OK! では続けます。

はい、そんな便利な Criteria ですが、私が思わずハマってしまった落とし穴がありました。
次節から実際のコードを使って確かめます。

エンティティ（スキーマ）の定義

サンプルとして使うエンティティは下記のように定義しました。

f:id:kalibora:20190225122126p:plain

顧客（Customer）が注文（Order）を複数持つ。というシンプルなもので、メソッドの意味は以下のとおりです。

  • Customer::order() : 顧客が商品を注文する
  • Customer::getRecentOrders() : 顧客の最新の注文を取得する（デフォルトは5件分）
    • 今回このメソッドで Criteria を使います
  • Order::__toString() : 注文内容を文字列化（デバッグ目的で注文の中身をダンプしたいため）

実際のソースコードはすべて こちら に置きましたが、実際のエンティティのソースコードを記述します（※名前空間など一部は省略します）と

Customer.php

<?php

/**
 * @ORM\Entity(repositoryClass="App\Repository\CustomerRepository")
 * @ORM\Table(name="customers")
 */
class Customer
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=255)
     */
    private $name;

    /**
     * @ORM\OneToMany(targetEntity="Order", mappedBy="customer", cascade={"persist", "remove"})
     */
    private $orders;

    public function __construct(string $name)
    {
        $this->name = $name;
        $this->orders = new ArrayCollection();
    }

    public function order(\DateTimeImmutable $date, string $product) : self
    {
        $this->orders[] = new Order($date, $product, $this);

        return $this;
    }

    public function getRecentOrders($limit = 5) : Collection
    {
        /* ここで Criteria を使ってるよ!!! */
        $criteria = Criteria::create()
            ->orderBy(['date' => Criteria::DESC])
            ->setMaxResults($limit)
        ;

        return $this->orders->matching($criteria);
    }
}

Order.php

<?php

/**
 * @ORM\Entity(repositoryClass="App\Repository\OrderRepository")
 * @ORM\Table(name="orders")
 */
class Order
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="date")
     */
    private $date;

    /**
     * @ORM\Column(type="string")
     */
    private $product;

    /**
     * @ORM\ManyToOne(targetEntity="Customer", inversedBy="orders")
     * @ORM\JoinColumn(name="customer_id", referencedColumnName="id")
     */
    private $customer;

    public function __construct(\DateTimeInterface $date, string $product, Customer $customer)
    {
        $this->date = $date;
        $this->product = $product;
        $this->customer = $customer;
    }

    public function __toString(): string
    {
        return sprintf('%s: %s - %s', $this->id, $this->date->format('Y-m-d'), $this->product);
    }
}

こんな感じです。次にサンプルで使うFixtureを定義します。

Fixture の定義

AppFixtures.php

<?php

class AppFixtures extends Fixture
{
    public function load(ObjectManager $manager)
    {
        $customer = (new Customer('kalibora'))
            ->order(new \DateTimeImmutable('2019-01-01'), 'おはぎ')
            ->order(new \DateTimeImmutable('2019-01-02'), 'ようかん')
            ->order(new \DateTimeImmutable('2019-01-03'), '茶饅頭')
            ->order(new \DateTimeImmutable('2019-01-04'), '芋ようかん')
            ->order(new \DateTimeImmutable('2019-01-05'), 'ようかん')
            ->order(new \DateTimeImmutable('2019-01-06'), 'とらまき')
            ->order(new \DateTimeImmutable('2019-01-07'), 'みたらし団子')
            ->order(new \DateTimeImmutable('2019-01-08'), 'とらまき')
            ->order(new \DateTimeImmutable('2019-01-09'), '磯辺団子')
            ->order(new \DateTimeImmutable('2019-01-10'), '茶饅頭')
        ;

        $manager->persist($customer);
        $manager->flush();
    }
}

はい。正月早々元旦から毎日和菓子を注文し続ける。というフィクスチャーになっております。

ここまででテストデータが揃ったので、次節から実際にコマンドを実行し、挙動を確認します。

コマンド実行

TestCriteriaCommand.php

<?php

namespace App\Command;

class TestCriteriaCommand extends Command
{
    protected static $defaultName = 'test:criteria';

    private $customerRepository;

    public function __construct(CustomerRepository $customerRepository)
    {
        $this->customerRepository = $customerRepository;

        parent::__construct();
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $customer = $this->customerRepository->findOneByName('kalibora');

        /* ここに注目 */
        foreach ($customer->getRecentOrders() as $order) {
            echo $order, PHP_EOL;
        }
    }
}

このように、 getRecentOrders を使い最新5件分の注文を出力すると、

$ ./bin/console test:criteria
10: 2019-01-10 - 茶饅頭
9: 2019-01-09 - 磯辺団子
8: 2019-01-08 - とらまき
7: 2019-01-07 - みたらし団子
6: 2019-01-06 - とらまき

はい、ちゃんと5件出力されました。いいですね。とても便利。

では次に私がハマったパターンのコマンドを実行してみます。

エラーとなるコマンドの実行

TestErrorCriteriaCommand.php

<?php

class TestErrorCriteriaCommand extends Command
{
    protected static $defaultName = 'test:error-criteria';

    private $customerRepository;

    public function __construct(CustomerRepository $customerRepository)
    {
        $this->customerRepository = $customerRepository;

        parent::__construct();
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $customer = $this->customerRepository->findOneByName('kalibora');

        /* ここに注目 */
        $customer->order(new \DateTimeImmutable('now'), 'ようかん');

        foreach ($customer->getRecentOrders() as $order) {
            echo $order, PHP_EOL;
        }
    }
}

分かりますかね。 getRecentOrders の前に最新の注文を1件追加しています。

このコマンドを実行してみます。

$ ./bin/console test:error-criteria

In ClosureExpressionVisitor.php line 90:

  Cannot access private property App\Entity\Order::$date
(snip)

はい。エラーになりました！
Order::$date は private なプロパティだからアクセスできないぜ！というエラーです。

なぜエラーになったか？

Order の定義をもう一度見てみましょう。

再掲: Order.php

<?php

/**
 * @ORM\Entity(repositoryClass="App\Repository\OrderRepository")
 * @ORM\Table(name="orders")
 */
class Order
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="date")
     */
    private $date;

    /**
     * @ORM\Column(type="string")
     */
    private $product;

    /**
     * @ORM\ManyToOne(targetEntity="Customer", inversedBy="orders")
     * @ORM\JoinColumn(name="customer_id", referencedColumnName="id")
     */
    private $customer;

    public function __construct(\DateTimeInterface $date, string $product, Customer $customer)
    {
        $this->date = $date;
        $this->product = $product;
        $this->customer = $customer;
    }

    public function __toString(): string
    {
        return sprintf('%s: %s - %s', $this->id, $this->date->format('Y-m-d'), $this->product);
    }
}

確かに $date は private ですね。
基本的にはプロパティは公開せず、必要に応じてアクセサを用意するのが鉄則なので、
プロパティの定義としてはこれでよいのですが、今回外からアクセスするためのアクセサがなかったためにエラーとなりました。

だから getDate() メソッドを定義してあげれば今回のエラーは出ず、望み通りの結果となります。

じゃあなんで1回めのコマンド実行ではエラーにならなかったか？

<?php
// (snip)

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $customer = $this->customerRepository->findOneByName('kalibora');

        /* この行があるとエラー、なければ成功。 $customer->order(new \DateTimeImmutable('now'), 'ようかん'); */

        foreach ($customer->getRecentOrders() as $order) {
            echo $order, PHP_EOL;
        }
    }

このようにDBからの取得後、さらに注文を追加してから getRecentOrders するとエラーになります。
追加しなければエラーになりません。

前者ではPHPのレイヤで（オブジェクトとしてロードされてから）Criteriaによる処理が走ります。
後者ではDBのレイヤで（SQLレベルで）Criteriaによる処理が走ります。

永続化される前のデータがある場合、DBのレイヤで処理することはできないのでこのような挙動になります。

PHPのレイヤではオブジェクトのアクセサを通して絞り込みやソートが行われるのでアクセサの定義が必須ですが、
DBのレイヤではDoctrineのORMの定義を通してSQLに変換されて絞り込みやソートが行われるのでアクセサは不要です。（もちろん @ORM\Column(type="date") のようなORMの定義は必要）

というわけで私はこのような事象にハマってしまったわけです。

PHPStan で解決する

なんだか 漏れのある抽象化 にやられているような気もしますが、
Criteria が便利であることも事実なのでここはエンジニアリングで解決しましょう。

ということで作りました。

otobank/phpstan-doctrine-criteria - Packagist

です。

これをインストールしてちょちょっと設定すると、
Criteriaで使用しているフィールドが、DBのレイヤでもPHPのレイヤでも存在する問題のないものだ。ということを検証してくれます。

ただし…

これを実現するために、オリジナルの Criteria クラスは使えないようにしています。

その理由ですが、 Criteria を定義したタイミングでは、どのエンティティのコレクションに適用するかが定まらず、
コレクションの matching メソッドに渡されたタイミングで初めてエンティティが定まります。

ということは、matching メソッドに渡された変数 $criteria の中身を知らないといけないことになり、
これを静的解析でやるのは難しいと判断しました。

代わりの解決方法として、 オリジナルの Criteria クラスを禁止し、
どのエンティティに対して適用するCriteriaなのかを定義の時点で定める TargetAwareCriteria というクラスを作り、
アプリケーション側では必ずそれを継承したものを使用する。という方式にしています。

これはオリジナルを使うよりも冗長になりますが、
一方でより明示的な Specification パターンになるようにも思うので、そんなに悪くない選択肢なのではないかなと思います。

ではまた。