Laravel5.7 で不等号を使ってリレーションする方法

Question

Laravel5.7, MySQL5.7 で見積書、および請求書を生成、管理するシステムを作成しています。
リレーション周辺の知見をいただきたく質問しました。

Laravel 職人のご意見をお聞かせください。

具体的には、見積モデルの「見積書発行日」時点での消費税モデルをどうリレーションするのか？自分が採用したアプローチの他にもっと良い方法はないのか？
という疑問です。

なぜ悩んでいるかを列挙すると、以下の通りです。

• リレーションはできるだけすっきりと書きたい。（可読性のため）
• トリッキーなことはしたくない。（Laravelの作法には従おう）
• 発行日が変更されたとき、正しい消費税率を関連づけたい。できれば発行日が変更されても「よしなに」税率も変更される。
• ビジネスロジックがブラックボックスになりがちだから、SQLのトリガーは使いたくない。

Model, migration

問題になる部分の関連 Modelは Estimate, Tax で、今のところ、以下のような構造です。

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

class CreateEstimatesTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('estimates', function (Blueprint $table) {
            $table->bigIncrements('id')->comment('ID');
            $table->unsignedBigInteger('company_id')->comment('会社ID');
            $table->unsignedBigInteger('client_id')->comment('顧客ID');
            $table->unsignedBigInteger('tax_id')->comment('税率ID');
            $table->string('estimate_number')->nullable()->comment('見積番号');
            $table->string('title')->comment('件名');
            $table->date('publish_dt')->comment('発行年月日');
            $table->date('expiration_dt')->nullable()->comment('見積有効期限');
            $table->string('condition')->nullable()->comment('支払条件');
            $table->string('extra', 1000)->nullable()->comment('備考');
            $table->timestamps();
            $table->softDeletes()->comment('削除日時');

            $table->foreign('company_id')->references('id')->on('companies');
            $table->foreign('client_id')->references('id')->on('clients');
            $table->foreign('tax_id')->references('id')->on('taxes');
        });
    }
}

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

class CreateTaxesTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('taxes', function (Blueprint $table) {
            $table->bigIncrements('id')->comment('ID');
            $table->unsignedTinyInteger('rate')->comment('税率');
            $table->date('start_dt')->comment('開始年月日');
            $table->date('end_dt')->nullable()->comment('終了年月日');
            $table->timestamps();
            $table->softDeletes()->comment('削除日時');
        });
    }
}

ここで、Estimate と Tax において、素のSQLであれば、estimates.tax_id を使わずとも、

SELECT * FROM estimates 
INNER JOIN taxes 
ON (estimates.publish_dt BETWEEN taxes.start_dt AND taxes.end_dt)
 OR (estimates.publish_dt >= taxes.start_dt AND taxes.end_dt IS NULL)

と、リレーションを取ることが可能です。

コントローラーから見積書一覧を取得するときに、withを使ってアクセスしたいので、

<?php

namespace App\Http\Controllers;

use App\Models\Estimate;

class EstimateController extends Controller
{
    public function index()
    {
        $estimates = Estimate::with(['tax'])
            ->orderByDesc('updated_at')
            ->orderByDesc('id')
            ->paginate();
        return view('estimates.index', compact('estimates'));
    }
}

まず最初に、このSQLを表現するリレーションを表現できないのか？
と調べましたが、もちろん以下の形では表現できません。

<?php

use Illuminate\Database\Eloquent\Model;

class Estimate extends Model
{
    public function tax(): BelongsTo
    {
        return $this->belongsTo(Tax::class, 'publish_dt'); // NG
    }
}

また、以下のように表現しても、できなくはないですが、
一覧表示するときにループの中で毎回SQLを発行してしまうので、良いアプローチではないと思います。

<?php

use Illuminate\Database\Eloquent\Model;

class Estimate extends Model
{
    public function tax()
    {
        return Tax::whereDate('start_dt', '<=', $this->publish_dt)
                  ->where(function($query){
                    $query->whereDate('end_dt', '>=', $this->publish_dt)
                      ->orWhereNull('end_dt')
                  })->first();
    }
}

暫定的に採用したアプローチ

estimatesにtax_id を追加し、素直にbelongsTo でリレーションし、
EstimateObserver クラスで creating , updating を定義しています。

<?php

namespace App\Observers;

use App\Models\Estimate;
use App\Models\Tax;

class EstimateObserver
{
    public function creating(Estimate $estimate)
    {
        if (is_null($estimate->publish_dt)) {
            return;
        }
        $publish_dt = $estimate->publish_dt;
        $estimate->tax_id = Tax::whereDate('start_dt', '<=', $publish_dt)
            ->where(function ($query) use ($publish_dt) {
                $query->whereNull('end_dt')
                    ->orWhereDate('end_dt', '>', $publish_dt);
            })->first()->id;
    }
    
    public function updating(Estimate $estimate)
    {
        if (is_null($estimate->publish_dt)) {
            return;
        }
        $publish_dt = $estimate->publish_dt;
        $estimate->tax_id = Tax::whereDate('start_dt', '<=', $publish_dt)
            ->where(function ($query) use ($publish_dt) {
                $query->whereNull('end_dt')
                    ->orWhereDate('end_dt', '>', $publish_dt);
            })->first()->id;
    }
}

Comments

[m.ts10806]

回答依頼いただいておいて申し訳ないのですが、
まだ「職人」というほど使いこなせてないですし（5系は業務未経験）、
Kosuke_Shibuyaさんが出されている以上のアプローチは現状出てこないですね・・すみません

[xenbeat]

最終的には「ご自身で採用したアプローチ」をやめて「ベストアンサーのアプローチ」をそのまま採用されたという理解でよろしいでしょうか？

色んなトレードオフの中で最終的にKosuke_Shibuyaさんがこの問題に対して出した答え（解決した方法）を共有いただけますでしょうか。

Answer 1

あー，これ普通にリレーション自作案件ですね。「トリッキーなことはしたくない」というのは重々承知ですが，Laravel 本家が不等号を使った比較に対応していない以上リレーションを自作するのはやむを得ないと思います。再利用性を高くしたかったら Relation 継承クラスを返り値にしないといけないですし。

【Laravel】 第2回 Eloquent ソースコードリーディング - リレーションの取得 - Qiita

class HasTax extends Relation
{
    protected $model;
    protected $dateColumn;

    public function __construct(Model $model, string $dateColumn)
    {
        $this->model = $model; 
        $this->dateColumn = $dateColumn; 
    }

    protected function getDate(Model $model)
    {
        return $model->{$this->dateColumn};
    }

    public function addConstraints()
    {
        if (static::$constraints) {
            $this->query->whereDate('start_dt', '<=', $this->getDate($this->model));
            $this->query->where(function (Builder $query) {
                $query
                    ->whereDate('end_dt', '>=', $this->getDate($this->model))
                    ->orWhereNull('end_dt');
            });
        }
    }

    public function addEagerConstraints(array $models)
    {
        $this->query->where(function (Builder $query) use ($models) {
            foreach ($models as $model) {
                $query->orWhere(function (Builder $query) use ($model) {
                    $query->whereDate('start_dt', '<=', $this->getDate($model));
                    $query->where(function (Builder $query) {
                        $query
                            ->whereDate('end_dt', '>=', $this->getDate($model))
                            ->orWhereNull('end_dt');
                    });
                });
            }
        });
    }

    public function initRelation(array $models, $relation)
    {
        foreach ($models as $model) {
            $model->setRelation($relation, null);
        }
        return $models;
    }

    public function match(array $models, Collection $taxes, $relation)
    {
        foreach ($models as $model) {
            $date = $model->{$this->dateColumn};

            foreach ($taxes as $tax) {
                if ($tax->start_dt <= $date && ($date <= $tax->end_dt || $tax->end_dt === null)) {
                    $model->setRelation($relattion, $tax);
                    break;
                }
            }
        }

        return $models;
    }

    public function getResults()
    {
        return $this->query->first();
    }
}

これでモデル側で

return new HasTax($this, 'publish_dt');

すればリレーション定義完了です。hasMany hasOne belongsTo にならって hasTax を定義しておくと更にベター。

Comments

[mpyw]

動作未検証なので細かいミスはご容赦ください

[mpyw]

<del>あ… getEager() も定義しないといけないな。修正します</del>

<ins>そんなことはなかった</ins>

[mpyw]

match メソッドで，foreach の中で foreach 回してるのが若干嫌な気もしますが，内側の foreach はほぼ無視できる程度のオーバーヘッドなので気にする必要はないでしょう。消費税そんなに変わってないのでｗｗ

[退会済みユーザー]

回答ありがとうございます。
あー、なるほどと感心しました。
HasTaxクラスをどこに書くのがベストプラクティスだろうかと模索中です。
採用するかどうかは未定ですが、実際にやってみて判断します。

artisan vendor:publish でasset のようにオーバーライドできれば採用！なんですが、後々のメンテナンス性、学習コストとトレードオフかなあ。

[mpyw]

app/Models ディレクトリを切っているのであれば

・app/Models/Relations

無いのであれば

・app/Relations
・app/Database/Eloquent/Relations

あたりが候補ですかね。

[mpyw]

確かに学習コストはだいぶ上がってしまいますね。しかし1回これを書いてしまえば他のところはものすごくきれいになるので、メリットもあります。トレードオフなので、出来るだけいい落とし所を探してください。

[mpyw]

【Laravel】 Eager Loading でネストしたリレーションをカウントする - Qiita https://qiita.com/mpyw/items/ccbffdd07330fd9f1c39

（↑正直これとかやりすぎ感あるけど、リレーション定義頑張ったおかげで今も商用環境でカウンターキャッシュを一切使わずとも爆速でレスポンスが返っているのでやって悪かったとは思ってない）

Answer 2

検討されている仕様が汲み取れていないかもしれないですし、そのようなシステムの開発経験がないため色々と考慮漏れがあるかもしれませんが、個人的な考え方が少しでも参考になればと思い恐縮ながら回答させていただきます。

見積モデルの「見積書発行日」時点での消費税モデルをどうリレーションするのか？

Laravelというより設計の話になりますが、そもそもリレーションする必要がないと思っています。

自分が採用したアプローチの他にもっと良い方法はないのか？

私ならtax_idの代わりにtax_rateのようなカラムを持ち、作成・更新時点の税率をそのまま記録すると思います。EstimateObserverはそのまま残しても良いと思いますが、作成時と更新時で次のロジックが重複するので

$estimate->tax_id = Tax::whereDate('start_dt', '<=', $publish_dt)
            ->where(function ($query) use ($publish_dt) {
                $query->whereNull('end_dt')
                    ->orWhereDate('end_dt', '>', $publish_dt);
            })->first()->id;

次のようにTaxのモデルを作って、そこに上記と同様の税率取得ロジックを閉じ込めちゃって、estimatesの作成時と更新時にそのメソッドの戻り値を$estimate->tax_rateに設定すれば良いと思います。

class Tax extends Model
{
    public function 税率を取得($date=現在日): int
    {
        ...
    }
}

あとは余談ですが私ならestimatesにstatusカラムも持つと思います。
発行前までは「下書き」として保持して、発行後に「発行済み」に更新、その後一切更新できないようにします。
もし「発行済み」の後に内容を更新したい場合は「破棄」して再作成させるようにします。
「発行済み」の見積書は相手に渡ったりして、業務上の重要な記録となるので、その業務記録が改ざん出来ないように設計します。
リレーションは必要ないというのも上記の考えに基づくものです。

Comments

[退会済みユーザー]

回答ありがとうございます.
ご提案の方法だと、estimates テーブルの tax_rate に0, 3, 5, 8, 10 以外の不正なデータが入り込む余地ができてしまいますね。7というデータが入らないようにする別のロジックが必要になったり、不正データがないことを担保する別の仕組みが必要になりそうです。

[xenbeat]

トリッキーな方法でリレーションを貼ったとしても、POST時にはデータベース側（外部キー制約は使えないですよね？）ではなく、どのみちアプリケーション側でデータベースに問い合わせるなりして例外処理を適切にすべきかと思いますがいかがでしょうか。

その処理含めTaxのモデルに税率取得処理を切り出すというご提案です。

[退会済みユーザー]

> 外部キー制約は使えないですよね？
質問文では割愛しましたが、使ってますよ。

[xenbeat]

estimates.tax_id = tax.idという制約ですよね？

[退会済みユーザー]

そうです。そうです。
「暫定的なアプローチ」で実装している方法では、です。

[xenbeat]

ですよね。
ただその制約だけでは不十分だと（データベース側だけで担保するのは難しい）思うのですが。

念のため確認ですが、見積書作成/更新時の税率はユーザー入力ではなくシステムで「正しい税率（そのときに適用されるべき税率）」を自動設定という認識でよろしいでしょうか？

EstimateObserverの「creating」と「updating」でtax_idを設定されているのと、質問に記載されている次の要件からそのような認識でいるのですが。

> 発行日が変更されたとき、正しい消費税率を関連づけたい。できれば発行日が変更されても「よしなに」税率も変更される。

[退会済みユーザー]

発行日はユーザー入力で、税率はシステムでの自動設定となります。

[xenbeat]

> 発行日はユーザー入力で、税率はシステムでの自動設定となります
その認識は合ってそうです。

tax_idでもtax_rateでも、ユーザー入力の「発行日」から「正しい税率（そのときに適用されるべき税率）」を導き出すのはアプリケーションのロジックですよね。

データベースの制約（estimates.tax_id = tax.id）では税率が「（例）5, 8, 10」の中のどれかであるというのが担保できるだけだと思うので、それを含め正しい税率（そのときに適用されるべき税率）かどうかというのはアプリケーション側のロジック、テストで担保するのが良いと思った次第です。
で、そのロジックをTaxというモデルに書いたらどうかという提案です。

否定しているわけではなく、あくまでも私ならこうするかもという話なので参考程度に捉えていただければ幸いです。

Answer 3

リレーション(belongsToやhasMany）にwhere文をくっつける事ができます。

return $this->belongsTo(Tax::class)->whereDate('start_dt', '<=', $this->publish_dt)
                  ->where(function($query){
                    $query->whereDate('end_dt', '>=', $this->publish_dt)
                      ->orWhereNull('end_dt')
                  });

こんな感じで対応できないでしょうか？な、withできるかは不明です

Comments

[退会済みユーザー]

回答ありがとうございます。
いただいた方法では、なくても良いはずの tax_id が必要になり、またpublish_dt 変更時にtax_id を変更する別の手段が発生しますね。
実際に試したところ、eager ロードは使えず,、with を使用することができませんでした。
$this->publish_dt が null になってしまうためです。