Zenn
🚨

Laravel の Model が Undefined Property エラーを吐いてくれない件をなんとかする

2020/10/23に公開
Laravel
PHP
tech

検証環境

PHP 7.4
Laravel 8.10.0

前提知識

Post が posts テーブルに紐づけられたモデルとして、

$post = App\Models\Post::findOrFail(1);
$author = $post->author;

としたとき、$author には何が入るでしょうか。

  1. Post モデルに getAuthorAttribute() メソッドが定義されている場合、その戻り値

  2. Post モデルに、author() メソッドが定義されている場合、それが他のモデルとのリレーションを返すメソッドであれば、そのリレーション

  3. posts テーブルに author カラムが存在すれば、その値

上記のいずれかが、上記の優先順位で返されます。

何が問題か

問題は上記1〜3のいずれにも該当しないとき。

例外を投げてくれるかと思いきや、しれっとNullが返ってくるのです。

これが厄介で、プロパティ名のスペルミスがあった時や、
カラム名を変更してプログラムの修正が漏れていた時、

実行時エラーにならないため気づかず、隠れたバグを生みがちです。

直接画面に表示するような値ならまだ気づきやすいのですが、
条件判定に使っているような場合、本当にハマります。えぇ。

ソース探索

上記の挙動から、Model の基底クラスが
マジックメソッド __get() を実装していることは容易に想像できます。

というわけで、Model クラスを直撃

// Illuminate\Database\Eloquent\Model

public function __get($key)
{
    return $this->getAttribute($key);
}

まさかの、トレイトに丸投げ。

// Illuminate\Database\Eloquent\Concerns\HasAttributes

public function getAttribute($key)
{
    if (! $key) {
        return;
    }

    // If the attribute exists in the attribute array or has a "get" mutator we will
    // get the attribute's value. Otherwise, we will proceed as if the developers
    // are asking for a relationship's value. This covers both types of values.
    if (array_key_exists($key, $this->attributes) ||
        array_key_exists($key, $this->casts) ||
        $this->hasGetMutator($key) ||
        $this->isClassCastable($key)) {
        return $this->getAttributeValue($key);
    }

    // Here we will determine if the model base class itself contains this given key
    // since we don't want to treat any of those methods as relationships because
    // they are all intended as helper methods and none of these are relations.
    if (method_exists(self::class, $key)) {
        return;
    }

    return $this->getRelationValue($key);
}

しれっと return している2箇所も気になりますが、
ここは本題ではないので置いておきましょう。

プロパティの存在チェック＋αののち、
最後は条件判定なしでリレーション返却用のメソッドへパス。

public function getRelationValue($key)
{
    // If the key already exists in the relationships array, it just means the
    // relationship has already been loaded, so we'll just return it out of
    // here because there is no need to query within the relations twice.
    if ($this->relationLoaded($key)) {
        return $this->relations[$key];
    }

    // If the "attribute" exists as a method on the model, we will just assume
    // it is a relationship and will load and return results from the query
    // and hydrate the relationship's value on the "relationships" array.
    if (method_exists($this, $key) ||
        (static::$relationResolvers[get_class($this)][$key] ?? null)) {
        return $this->getRelationshipFromMethod($key);
    }
}

はい、ここです。
2つの if 文をすり抜けると、return null どころか何も返さない放置プレー。

対応

諸悪の根源をオーバーライドした基底クラスを作ります。

namespace App\Models\Shared;

use Illuminate\Database\Eloquent\Model;

abstract class BaseModel extends Model
{
    /**
     * オーバーライド
     * Illuminate\Database\Eloquent\Concerns\HasAttributes::getRelationValue
     * 存在しないキーに対して例外を発するように
     */
    public function getRelationValue($key)
    {
        // If the key already exists in the relationships array, it just means the
        // relationship has already been loaded, so we'll just return it out of
        // here because there is no need to query within the relations twice.
        if ($this->relationLoaded($key)) {
            return $this->relations[$key];
        }

        // If the "attribute" exists as a method on the model, we will just assume
        // it is a relationship and will load and return results from the query
        // and hydrate the relationship's value on the "relationships" array.
        if (method_exists($this, $key) ||
            (static::$relationResolvers[get_class($this)][$key] ?? null)) {
            return $this->getRelationshipFromMethod($key);
        }

        $class = static::class;
        throw new \Exception("プロパティ {$key} は {$class} に存在しません。きっとスペルミスです");
    }
}

そして各モデルクラスは、BaseModel を継承するようにします。

namespace App\Models;

use App\Models\Shared\BaseModel;

class Post extends BaseModel // <- BaseModelを継承
{
    protected $fillable = ["title", "user_id"];

    public function user()
    {
        return $this->belongsTo('App\Models\User');
    }

    public function getHogeAttribute()
    {
        return "HOGE";
    }
}

検証

// App\Http\Controllers\TestController

$post = Post::find(1);

var_dump($post->title); // postsテーブルのtitleカラムの値
var_dump($post->user); // リレーション先のUserオブジェクト
var_dump($post->hoge); // "HOGE"
var_dump($post->foo); // 例外発生!!

これで潜在バグが減ってハッピー。

まとめ

むしろこれを標準の挙動にして欲しい。

何か不都合があるのでしょうか。

Discussion