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

概要

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

baserCMS version : 5.1.x

TODO

[x] プルリクを送る予定

Jul 26 '24 03:07 katokaisya

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
        //         (
        //         )

        // )
    }
}

Jul 26 '24 08:07 seto1

@seto1

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

下記コミットで変更となっているのが原因ようですね。5.0の最新だとすでにこのコミットが入っています。 https://github.com/baserproject/basercms/commit/e5d6db52a5184cc4d4cc0cdf4f44db49c46a2c3c

また、こちらのプルリクにもコメントを書きましたが、afterFindで書き換えるというのでもいいかもしれません。 https://github.com/baserproject/basercms/pull/3635#issuecomment-2252249890

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

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

Jul 26 '24 09:07 ryuring

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

@uchin0 こちらのコミットの理由は何でしたっけ？ https://github.com/baserproject/basercms/commit/e5d6db52a5184cc4d4cc0cdf4f44db49c46a2c3c

Jul 26 '24 09:07 ryuring

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

Jul 26 '24 09:07 ryuring

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

SelectQueryのbeforeFind
AppTableのbeforeFind

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

Jul 27 '24 05:07 ryuring

@katokaisya 取り急ぎ参照を入れておきます。 https://github.com/baserproject/basercms/pull/3635#issuecomment-2252844985

Jul 27 '24 06:07 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

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

Jul 27 '24 06:07 ryuring

課題は３点

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

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

beforeFindの重複

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

afterFindの呼び出し順

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

AppTableのafterFind
SelectQueryのbeforeFind

Jul 27 '24 06:07 ryuring

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

Jul 27 '24 09:07 seto1

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

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

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

Jul 27 '24 09:07 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;
    }
}

Aug 05 '24 07:08 seto1

@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]);
            }
        }
    });
}

Aug 10 '24 09:08 ryuring