[要望]BlogControllerにイベント beforeQueryParamsを追加したい

[katokaisya]

概要

baserCMS4系では、BlogPost.beforeFindでconditionsを上書きできたが、
baserCMS5系では BcBlog.BlogPosts.beforeFindでは、すでにクエリが出来上がっており、
クエリの追加しかできないため、上書きできるように、クエリ発行前のイベントがほしいです。
【例】blog_content_id を変更したい、など

baserCMS version : 5.1.x

TODO

• プルリクを送る予定

[seto1]

beforeFindとBcBlog.BlogPosts.beforeFindで取得できる情報が異なりますね。
baser5.0系だとBcBlog.BlogPosts.beforeFindでもクエリが取得できてたんですが、5.1系だとarrayになってます。

baserのイベント周りの調整が必要かもしれません。

<?php
namespace Test\Event;

use BaserCore\Event\BcModelEventListener;
use Cake\Event\EventInterface;

class TestModelEventListener extends BcModelEventListener
{
    public $events = [
        'beforeFind',
        'BcBlog.BlogPosts.beforeFind',
    ];

    public function beforeFind(EventInterface $event)
    {
        print_r($event->getData());
        exit;
        // Array
        // (
        //     [0] => Cake\ORM\Query\SelectQuery Object
    }

    public function bcBlogBlogPostsBeforeFind(EventInterface $event)
    {
        print_r($event->getData());
        exit;
        // Array
        // (
        //     [type] => all
        //     [options] => Array
        //         (
        //         )

        // )
    }
}

[ryuring]

@seto1

baser5.0系だとBcBlog.BlogPosts.beforeFindでもクエリが取得できてたんですが、5.1系だとarrayになってます。

下記コミットで変更となっているのが原因ようですね。5.0の最新だとすでにこのコミットが入っています。
e5d6db5

また、こちらのプルリクにもコメントを書きましたが、afterFindで書き換えるというのでもいいかもしれません。
#3635 (comment)

ただ、Cake本体側のイベントに渡すパラメーターが元々こちらなので、こちらに合わせた方がいいかもですね。

            $repository = $this->getRepository();
            $repository->dispatchEvent('Model.beforeFind', [
                $this,
                new ArrayObject($this->_options),
                !$this->isEagerLoaded(),
            ]);

[ryuring]

ん？ということは、元々 beforeFindはあったのか、、、

@uchin0 こちらのコミットの理由は何でしたっけ？
e5d6db5

[ryuring]

afterFindがなかったからか。ということは、beforeFindが２発発動されてしまう可能性が高い？（要調査）

[ryuring]

@seto1 調査しましたが、BcBlog.BlogPosts.beforeFind で、beforeFindが２回呼び出されています。

• SelectQueryのbeforeFind
• AppTableのbeforeFind

AppTableのbeforeFindを削除することを検討したいですね、、、

[ryuring]

@katokaisya 取り急ぎ参照を入れておきます。
#3635 (comment)

[ryuring]

現時点で分かっていること

where の第２引数に true を設定すると上書きとなる

$query->where(['BlogPosts.blog_content_id IN' => [1,2]], true);

元々のwhere条件を取得する

// QueryExpression として取得できる
$where = $query->clause('where');

が、実態であるQueryExpression::_conditions が参照不可であるため、既存の検索条件を解析できない。

つまり、既存の条件を参照しつつ、一部の条件だけを書き換えて、where() で上書きすることができない。

afterFindの呼び出し順が想定と違った

find() メソッドは一番最初に呼び出されるため、次の順番でイベントが発火している

• AppTableのbeforeFind
• AppTableのafterFind（このタイミングで where はセットされていない！）
• SelectQueryのbeforeFind

これらを踏まえて仕様の再検討が必要

[ryuring]

課題は３点

検索パラメーターの一部書き換えをどうやってやるか

加藤さんの提案のとおり、Queryオブジェクト作成前のタイミングで書き換えさせる方が良さそう。
ただ、Controllerのイベントにはしたくないため、サービスでのイベント実装を検討する？ beforeGetIndex ?

beforeFindの重複

AppTableのbeforeFindは削除した方が混乱をうまない

afterFindの呼び出し順

AppTableのbeforeFind を削除した場合、順番が逆となる。イベント名称を違うものに変更した方がよいかも

1. AppTableのafterFind
2. SelectQueryのbeforeFind

[seto1]

@ryuring

beforeFindの重複

解消したいですね。それで beforeFind と BcBlog.BlogPosts.beforeFind の挙動が揃うのであれば特に。

検索パラメーターの一部書き換えをどうやってやるか

既存のイベントで対応できるのに新しくイベントを増やしたくないという気持ちはありますね。
例えば検索条件のブログコンテンツIDの変更でしたら以下で対応可能です。

まあ、無理矢理感はあります。
関数化してもっと使いやすくすることは可能だと思いますけど。
クエリの書き換えの需要はあると思うので、なにかもっといいベストプラクティスがあればいいんですけど検索してもなかなか見つかりませんね。
イベントを増やすことでシンプルになるのであればありかもしれません。

<?php
namespace Test\Event;

use BaserCore\Event\BcModelEventListener;
use Cake\Database\ExpressionInterface;
use Cake\Database\Expression\ComparisonExpression;
use Cake\Database\Expression\QueryExpression;
use Cake\Event\EventInterface;
use Cake\ORM\Query;

class TestModelEventListener extends BcModelEventListener
{
    public $events = [
        'beforeFind',
    ];

    public function beforeFind(EventInterface $event)
    {
        $query = $event->getData(0);
        if ($query->getRepository()->getAlias() === 'BlogPosts') {
            $this->changeWhere($query);
        }
    }

    private function changeWhere($query)
    {
        if ($query instanceof ComparisonExpression) {
            $fieldName = $query->getField();
            // 検索条件のブログコンテンツID指定を2に変更
            if ($fieldName === 'BlogPosts.blog_content_id') {
                $query->setValue(2);
            }
            return $query;
        }
        if ($query instanceof Query) {
            $query = $query->clause('where');
        }
        if ($query instanceof QueryExpression) {
            return $query->iterateParts(function($condition) {
                return $this->changeWhere($condition);
            });
        }
        if ($query instanceof ExpressionInterface) {
            return $query->traverse(function($condition) {
                return $this->changeWhere($condition);
            });
        }
    }
}

[seto1]

サービスクラスのメソッドが呼び出される際に毎回発火するイベントがあるなら使いやすそうですけど難しそうですね。
https://www.php.net/manual/ja/language.oop5.magic.php

getIndexに個別でイベントを追加すると、サービスクラスの他の関数にもイベントを追加したくなってコード量が大幅に増えてしまいそうです。

もしくは、せっかくサービスクラスにInterfaceを使っているので、任意のクラスに変更できるようになればイベント以上に自由度が上がりますね。

[seto1]

■サービスクラスの切り替え
複数のプラグインから条件を変更したい場合、単にサービスクラスを切り替えるようにするだけでは難しい。

■where条件の上書き
where関数の3つめの引数にtrueを渡すと条件をリセットできるものの、ブログ記事の公開状態の条件などもリセットされてしまう。
https://discourse.cakephp.org/t/remove-where-condition-from-query/1900

■Queryを一度配列に変換して条件を変更しやすくする
複雑な条件が指定された場合に難しそう。

試作

public function beforeFind(EventInterface $event)
{
    $query = $event->getData(0);
    if ($query->getRepository()->getAlias() === 'BlogPosts') {
        $array = $this->whereToArray($query);
        $array['AND'][6]['BlogPosts.blog_content_id ='] = 2;
        $query->where($array, [], true);
        sqld($query);
    }
}

private function whereToArray($query)
{
    $query = clone $query;
    if ($query instanceof Query) {
        $array = [];
        $where = $query->clause('where');
        $conjunction = $where->getConjunction();
        $where->iterateParts(function($condition) use (&$array, $conjunction) {
            $array[$conjunction][] = $this->whereToArray($condition);
        });
        return $array;
    }
    if ($query instanceof ComparisonExpression) {
        return [
            $query->getField() . ' ' . $query->getOperator() => $query->getValue(),
        ];
    }
    if ($query instanceof QueryExpression) {
        $conjunction = $query->getConjunction();
        $array = [];
        $query->iterateParts(function($condition) use (&$array, $conjunction) {
            $array[$conjunction][] = $this->whereToArray($condition);
        });
        return $array;
    }
    if ($query instanceof ExpressionInterface) {
        $array = [];
        $query->traverse(function($condition) use (&$array) {
            $array[] = [
                $condition->getIdentifier() .  ' IS ' => $condition->getCollation(),
            ];
        });
        return $array;
    }
}

[ryuring]

@seto1 リセットしないでも下記のコードでいけました。汎用的ではないですが

if ($event->getData(0) instanceof \Cake\ORM\Query\SelectQuery) {
    $where = $event->getData(0)->clause('where');
    /** @var QueryExpression $where */
    $where->iterateParts(function($part, $key) {
        if ($part instanceof ComparisonExpression) {
            if ($part->getField() === 'BlogPosts.blog_content_id') {
                $part->setValue([1, 2]);
            }
        }
    });
}