Wrong class instance when eager loading a nullable polymorphic relation with withDefault()

[bucaneer]

• Laravel Version: 5.7.24
• PHP Version: 7.1.26
• Database Driver & Version: MySQL 5.7.25

Description:

I have a use case for the MorphTo relation which sometimes involves setting an explicit morph type, but leaving the morph id blank. This way I can expect that accessing the relation will always provide an instance of the appropriate class. It works on individual instances, but fails with eager loading: instead of an instance of the specified morph type, you get an instance of the parent model.

Steps To Reproduce:

Minimal setup:

Schema::create('parent_models', function (Blueprint $table) {
    $table->increments('id');
    $table->integer('morph_id')->unsigned()->nullable();
    $table->string('morph_type')->unsigned()->nullable();
    $table->timestamps();
});

Schema::create('type_ones', function (Blueprint $table) {
    $table->increments('id');
    $table->timestamps();
});

class ParentModel extends Model
{
    protected $fillable = [
        'morph_id',
        'morph_type',
    ];

    public function morph() {
        return $this->morphTo()->withDefault();
    }
}

class TypeOne extends Model
{
    public function parent() {
        return $this->morphOne(ParentModel::class, 'morph');
    }
}

Reproduction in tinker:

>>> App\ParentModel::create(['morph_type' => App\TypeOne::class]);

>>> App\ParentModel::first()->morph
=> App\TypeOne {#2912} /* This is correct. */

>>> App\ParentModel::query()->with('morph')->first()
=> App\ParentModel {#2917
     id: "1",
     morph_id: null,
     morph_type: "App\TypeOne",
     created_at: "2019-01-31 11:56:36",
     updated_at: "2019-01-31 11:56:36",
     morph: App\ParentModel {#2920}, /* This is wrong. */
   }

Expected behavior:

Ideally, the ParentModel instance from App\ParentModel::query()->with('morph')->first() should have the morph relation set to an instance of App\TypeOne.

Alternatively, the eager loading process could ignore the morph relation when no id is present so that the correct instance could be lazy loaded on first access.

P.S.: The wrong behavior is also described in #24725, but whereas that involves an open-ended question about how to handle the edge case where morph type is also undefined, this issue is more clearly a bug: there are expected behaviors that the system is not following.

[driesvints]

It really doesn't makes sense to make any of those two columns nullable imo. If you have a specific example which you feel should be supported I suggest you open up an issue at laravel/ideas as this is more of a feature request then a bug.

I also believe we should look at maybe supporting the withDefault for this and hope to find some time for this in the upcoming weeks.

[bucaneer]

@driesvints I agree that this is a rare edge case, but I still think it is a bug:

1. there is inconsistent behavior between lazy loading and eager loading;
2. the behavior of lazy loading in this case makes intuitive sense;
3. the behavior of eager loading in this case is unexpected;
   => therefore, there is a bug with eager loading.

I'll try to summarize the use case I have for this without going into details of a complicated system that caters to a niche userbase. The ParentModel and multiple morph types (TypeOne, TypeTwo, ..., TypeN) have close functional links, but must be created separately. ParentModel instances are created first and are entirely managed by the system. It is possible to know which "flavor" (morph type) the ParentModel will have in advance, but the details of a particular TypeN instance have to be filled in later by a user. I use withDefault() so that even when the associated TypeN instance has not been created yet, I can still make calls like $parent->morph->isFilled() to check the state of the relation.

Technically, I could create blank instances of TypeN at the same type as I create ParentModel, but in that case instead of a couple nullable columns I would need several tables that are almost entirely nullable/defaultable. That would cause extra burden for validation and integrity checking, as well as database performance, so my current solution seems more elegant.

[driesvints]

I still think that this is just a matter of doing things differently and just associate the model with the polymorph at a later time.

[bucaneer]

Well, I have been able to rely on the lazy loading behavior for months of development before encountering the issue with eager loading, so there's too much work invested to change the approach at this point. Currently, I use the solution from the PR by extending the MorphTo class in my project.

Also, I don't understand how this is less of bug than #24725, which has the "bug" label despite being more of an idea. In fact, the approach used here - specifying a morph type without an id - is a straightforward (and already implemented for the most part) solution to the problem of how withDefault() should pick an appropriate class to instantiate in MorphTo relations.