[13.x] Check the class before calling relationLoaded in loadMissingRelationshipChain

[dxiiren]

loadMissingRelationshipChain() calls relationLoaded() on a value before it checks what that value is:

$this->filter(function ($model) use ($relation, $class) {
    return ! is_null($model) &&
        ! $model->relationLoaded($relation) &&
        $model::class === $class;
})->load($relation);

A relation doesn't have to hold a model or a collection of models. If it holds something else, that value isn't null and isn't a BaseCollection, so it gets past the filter, doesn't get collapsed by the check below, and the recursive call ends up calling relationLoaded() on it.

I hit this with a paginator. AbstractPaginator forwards unknown calls to the collection underneath it, so it comes out as:

BadMethodCallException: Method Illuminate\Database\Eloquent\Collection::relationLoaded does not exist.

Lighthouse sets a LengthAwarePaginator as the relation value when it resolves a paginated GraphQL relation:

https://github.com/nuwave/lighthouse/blob/master/src/Execution/ModelsLoader/PaginatedModelsLoader.php#L191

so any app using that hits this once Model::automaticallyEagerLoadRelationships() is turned on. It's reported there as nuwave/lighthouse#2679.

Swapping the two conditions makes the class check short circuit first. It's still an exact class comparison rather than instanceof, so anything that matched before still matches — the only change is that a value of some other type gets skipped instead of called into.

For anyone using a package that does this, automatic eager loading currently can't be turned on at all: the request 500s instead of rendering. After this they can use the feature, and relations holding something other than a model are simply left alone rather than crashing the request.

Nothing existing changes. The filter still matches the same models it did before, and the only values affected are ones that would have thrown.

Two tests. One walks a normal chain and checks the relations still get loaded — it passes before and after, so it shows the reorder doesn't change which models are matched. The other puts a paginator in a relation and walks a two step chain over it — that one fails on 13.x with the exception above and passes with this change.

         $this->filter(function ($model) use ($relation, $class) {
             return ! is_null($model) &&
-                ! $model->relationLoaded($relation) &&
-                $model::class === $class;
+                $model::class === $class &&
+                ! $model->relationLoaded($relation);
         })->load($relation);