Zenn
🎻

[Symfony] @UniqueEntityアノテーションを使ったらエンティティが更新できなくなった件

2020/10/15に公開
PHP
Symfony
Doctrine
tech

@UniqueEntity を使ったら謎の挙動でフォーム経由の更新ができなくなりました

https://twitter.com/ttskch/status/1305428273820647424

https://twitter.com/ttskch/status/1305431648368910336

https://twitter.com/ttskch/status/1305492467223990283

https://twitter.com/ttskch/status/1305506984997740545

起こった現象を整理

  • handleRequest() 中に @UniqueEntity 制約がエラーになって変更内容が破棄されていた
  • にもかかわらず Form インスタンスにエラーは登録されておらず（ $form->getErrors() の結果が空）
  • $form->isValid() は true
  • なので、正常に処理が完了したような画面遷移をするけどデータは変更されていない、という結果になっていた

再現条件

厳密な再現条件まで調べる時間と気力がなかったのですが、少なくとも僕のケースでは

  • ある特定のエンティティにおいて、 @UniqueEntity 制約を使うと上記の現象が発生する
    • 他のエンティティでは @UniqueEntity が正常に機能している
  • @UniqueEntity を使うのをやめてカスタムバリデータを作って自力で重複チェックするようにしてみても、 そのバリデーション処理の中でリポジトリの findBy() メソッドを使うと同様の現象発生する
    • findAll() もNG、 find() はOK
  • findBy() を使わずにQueryBuilderを使う実装にしたら現象が発生しなくなる

という結果でした。謎すぎ。

今回のケースではリレーションプロパティに対して @UniqueEntity を使っていたので、それが原因かな？と一瞬思ったのですが、公式ドキュメント を見るとリレーションプロパティに対して @UniqueEntity を使う例が書かれているのでやっぱりこれは問題ないようです。

DoctrineかSymfony/Validatorのバグだと思うのですが、軽く検索した限りではissueを見つけることができませんでした😓

解決策

先述のとおり、

  • カスタムバリデータを作って自力で重複チェックする
  • ただしその際にリポジトリの findBy() メソッドは使わず、QueryBuilderを使って検索するようにする

で一応解決できました。

具体的なコードのイメージは以下のとおりです。

エンティティ

  use App\Repository\FooRepository;
+ use App\Validator\Constraints as AppAssert;
- use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
  
  /**
   * @ORM\Entity(repositoryClass=FooRepository::class)
-  * @UniqueEntity(fields="bar", message="そのbarはすでに他のfooと紐づいています")
   */
  class Foo
  {
      /**
       * @ORM\OneToOne(targetEntity=Bar::class, inversedBy="foo")
+      * @AppAssert\Foo\UniqueBar()
       */
      private $bar;
  
      // ...
  }

リポジトリ

  class FooRepository extends ServiceEntityRepository
  {
      public function __construct(ManagerRegistry $registry)
      {
          parent::__construct($registry, Foo::class);
      }
+
+     public function findByBar(Bar $bar, Foo $self = null)
+     {
+         $qb = $this->createQueryBuilder('f')
+             ->andWhere('f.bar = :bar')
+             ->setParameter('bar', $bar)
+         ;
+ 
+         if ($self) {
+             $qb
+                 ->andWhere('f != :self')
+                 ->setParameter('self', $self)
+             ;
+         }
+ 
+         return $qb->getQuery()->getResult();
+     }
  }

カスタムバリデータ

namespace App\Validator\Constraints\Foo;

use Symfony\Component\Validator\Constraint;

/**
 * @Annotation
 */
class UniqueBar extends Constraint
{
    public $message = 'このfooはすでに他のbarと紐づいています';
}

namespace App\Validator\Constraints\Foo;

use App\Entity\Foo;
use App\Repository\FooRepository;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
use Symfony\Component\Validator\Exception\UnexpectedValueException;

class UniqueBarValidator extends ConstraintValidator
{
    /**
     * @var FooRepository
     */
    private $fooRepository;

    public function __construct(FooRepository $fooRepository)
    {
        $this->fooRepository = $fooRepository;
    }

    public function validate($value, Constraint $constraint)
    {
        if (!$constraint instanceof UniqueBar) {
            throw new UnexpectedTypeException($constraint, UniqueBar::class);
        }

        $foo = $this->context->getObject();

        if (!$foo instanceof Foo) {
            throw new UnexpectedValueException($foo, Foo::class);
        }

        // findByを使うと謎現象が発生してしまう
//        foreach ($this->fooRepository->findBy(['bar' => $value]) as $found) {
//            if ($found->getId() !== $foo->getId()) {
//                $this->context->buildViolation($constraint->message)->addViolation();
//                return;
//            }
//        }

        // QueryBuilderを使えば謎現象は発生しない
        if ($this->fooRepository->findByBar($value, $foo)) {
            $this->context->buildViolation($constraint->message)->addViolation();
        }
    }
}

参考リンク

GitHubで編集を提案

Discussion