[5.6] Allow to set opposite relation automatically

[mnabialek]

If approved this would give 2 benefits:

• lower number of database queries
• save memory and keep consistent object for base model and related model relation

First example - let's assume we have company and user with 1:1 relationship and run code like this:

$company = Company::with('owner')->first();
$ownerCompany = $company->owner->company;
$company->delete();
$company->name = 'Modified';

dd($company->name, $ownerCompany->name, $company->exists, $ownerCompany->exists);

As a result we get:

"Modified"
"Mitchell-Harber"
false
true

what seems quite strange but this is because $company and $ownerCompany are not the same object.

Now let's assume we have 1 to many relationships (user has multiple companies) and we have code like this:

$users = User::get();
$users->load('companies');

foreach ($users as $user) {
    echo $user->email."<br />";
    foreach ($user->companies as $company) {
        echo $company->name.' ( '.$company->owner->email.' )'."<br />";
    }
}

(inner foreach might be considered as Blade partial). Assuming we have 10 users, each with 20 companies we would execute 202 queries here (200 too much). Of course in above case we could load relationship like this:

$users->load('companies.owner');

but it still produces one not necessary query.

The idea is to add additional withOpposite method when defining a relationship which will cause that same model will be reused and no additional queries will be run like so:

public function owner()
{
    return $this->belongsTo(User::class, 'owner_id')->withOpposite('company');
}

public function companies()
{
    return $this->hasMany(Company::class, 'owner_id')->withOpposite('owner');
}

public function company()
{
    return $this->hasOne(Company::class, 'owner_id')->withOpposite('owner');
}

Now running:

$company = Company::with('owner')->first();
$ownerCompany = $company->owner->company;
$company->delete();
$company->name = 'Modified';

dd($company->name, $ownerCompany->name, $company->exists, $ownerCompany->exists);
exit;

wouldn't execute an additional query and get the result:

"Modified"
"Modified"
false
false

as expected.

Also running:

$company = Company::with('owner')->first();
echo $company->owner->company->name;

or

$users = User::with('companies')->get();

foreach ($users as $user) {
    echo $user->email."<br />";
    foreach ($user->companies as $company) {
        echo $company->name.' ( '.$company->owner->email.' )'."<br />";
    }
}

or

$users = User::get();
$users->load('companies');

foreach ($users as $user) {
    echo $user->email."<br />";
    foreach ($user->companies as $company) {
        echo $company->name.' ( '.$company->owner->email.' )'."<br />";
    }
}

or

$users = User::all();

foreach ($users as $user) {
    echo $user->email."<br />";
    foreach ($user->companies as $company) {
        echo $company->name.' ( '.$company->owner->email.' )'."<br />";
    }
}

would execute minimum expected queries (in last code we don't use eager loading so it will produce more queries than in previous examples).

In this PR autoloading opposite relationship was introduced only into HasMany, BelongsTo and HasOne relationships but if approved and makes sense it could be added maybe to some more of them. Also maybe some additional code or tests should be updated.

This mechanism is opt-in - without adding ->withOpposite(...) to the relationship it should work as it's working now.

[sisve]

You should probably tag this as a work-in-progress since it breaks a lot of existing tests and there's no new test introduced for this functionality.

[mnabialek]

@sisve Thanks, good point. I've marked this as work in progress. As I mentioned this is initial version and could be improved if general idea will be accepted to add this to core.

[sisve]

If that's the point; have you considered a branch in your own repository, and rallying support in #internals on Slack or https://github.com/laravel/internals/? Taylor has recently gone on a 0-open-prs spree, and having one with no activity because you wait for someone to evaluate it, while it breaks stuff, doesn't look good for the longevity of this pr...

I believe that this type of functionality has been attempted before, and decline for various reasons. Have you read what happened there? There's no references to those attempts in your post.

[mnabialek]

@sisve I haven't found something similar (but maybe there is something like this). I can move it to internals but I believe @taylorotwell need to decide whether something like this is desired to be added to core or not.

[deleugpn]

Might be relevant: #22401, #20011, #20053, #6720, #6718

This has been suggested before and while the concept as a whole is interesting I haven't seen an implementation I was thrilled with. I know Rails tried an identity map on their ActiveRecord before and ended up pulling it out entirely if memory serves me right.

[mnabialek]

@deleugpn Thanks for relevant links. I haven't seen them before but it seems I'm not the only one who would like to have it added to Laravel :)

[deleugpn]

It's definitely something I would love to have, but I do feel like it's not easily feasible. Feels like we want an expensive magic.

[Miguel-Serejo]

As long as it's opt-in, and it's not as expensive as trying to do it yourself (or doing redundant eager loads like 'users.account.user'), I would very much like to see this implemented.

[mnabialek]

@taylorotwell What's your opinion about it now? Are you going to merge this or rather not?

[sisve]

There are still no tests.

[mnabialek]

@sisve Yes, but there is no point to write tests if it won't be accepted at all

[taylorotwell]

Not going to be merged, especially without tests. Sorry.