"children that may not actually be Model instances, or that don't actually exist"?

[iamjrock]

Terrific Cascade Deletion package, thank you for your excellent work!

I'm struggling to understand what the piece of code explained by this comment is protecting us from.

// To protect against potential relationship defaults,
// filter out any children that may not actually be
// Model instances, or that don't actually exist.

What is meant by "potential relationship defaults" in this context please?

[patrickcarlohickman]

@iamjrock The belongsTo, hasOne, hasOneThrough, and morphOne relationships allow you to specify a default return value if a related record doesn't exist. This allows you to follow the Null Object pattern. The documentation on this is here: https://laravel.com/docs/10.x/eloquent-relationships#default-models.

So, imagine you have a hasOne relationship defined:

public function phone(): HasOne
{
    return $this->hasOne(Phone::class);
}

With this relationship, it will either return a Phone instance if one exists, or null if there is no related phone. Now, lets define it to return a default model:

public function phone(): HasOne
{
    return $this->hasOne(Phone::class)->withDefault();
}

With the addition of the call to withDefault(), this relationship with always return a Phone instance. If a related phone exists, you will get the related Phone instance, but if there is no related phone, you will now get a new empty Phone instance instead of getting null.

Furthermore, the withDefault() method lets you customize the returned default. You can pass in an array of attributes, and those attributes will be set on the returned model. Additionally, you can pass in a Closure to have full control over the returned default value. The issue there is that there is no protection to enforce the type of value returned from the Closure. Because of this, even though this isn't the intended use, you could define a closure that doesn't even return a Model:

public function phone(): HasOne
{
    return $this->hasOne(Phone::class)
        ->withDefault(function (Model $newPhone, Model $parentUser) {
            return 'asdf';
        });
}

With this definition, if a related phone exists, you will get the related Phone instance, as expected. However, if there is no related phone, you will get the string 'asdf'.

Given all of that, here is the code in question:

                    $children = $model->getCascadeDeletesRelationQuery($relationName)->get();

                    // To protect against potential relationship defaults,
                    // filter out any children that may not actually be
                    // Model instances, or that don't actually exist.
                    $children = $children->filter(function ($child) {
                        return $child instanceof Model && $child->exists;
                    })->all();

Since the withDefault() method allows a relationship to return a model that doesn't exist in the database, or a value that isn't even a model, the filter() method is used to ensure that we only look at Model instances that exist in the database.

---

All of that said, I don't think this is actually needed anymore. I believe when the code was originally written, it used the actual value returned by the relationship. However, the current code as written does not:

$children = $model->getCascadeDeletesRelationQuery($relationName)->get();

This gets the relationship query and manually calls get() on it, so we will always be getting a collection of the related data in the database. Since we've manually called get() on the relationship query, and didn't actually use the relationship methods to get the data, the getResults() method on the relationship isn't actually called and this bypasses the default defined by withDefault().

This can probably be removed, but some tests should be added to verify.

[iamjrock]

Understood, thx for the detailed explanation, your time is valuable!