Be part of JetBrains PHPverse 2026 on June 9 – a free online event bringing PHP devs worldwide together.

musicin3d's avatar

Correct way to save changes to an eloquent relationship

This question has a few parts. I'd greatly appreciate your help with any or all of them, since all the advice I was able to dig up was incomplete.

I'm looking for someone to point out my misunderstanding(s) or to offer better solutions than the ones I present.

Context

Assume we have a model, User, that has a many-to-many relationship with another model, Roles. The User::roles relationship is set up using belongsToMany() etc.

I have two use cases and two desired implementations. So I have a total of 4 possible scenarios I would like to clarify.

The two use cases are:

  1. Add or remove one role from the collection
  2. Replace the entire collection of roles in one go

The two desired implementations are:

  1. Update the in-memory collection and update the database at once
  2. Update the in-memory collection, do some processing, and then save the collection to the database

I understand that sync(), attach(), and detach() all update the database, but it appears that they do NOT also update the in-memory data structure. This requires you to manually update the data structure to match the change you just made in the database. I have a solution (which I'll share below), but it seems totally unreasonable to me that we can update both.

I've also noticed that replacing $user->roles via assignment will break $user->save().

I'm getting ahead of myself. For thoroughness, let me explain each scenario...

Adding and saving immediately

Consider the following, which demonstrates how one might expect this to work...

assert( empty($user->roles) );
$user->roles()->attach($role); // or ->sync($role, false)
assert( count($user->roles) == 1 );

This will fail because, the in-memory collection has not updated. You must do either of the following...

$user->roles->push($role);
// OR
$user->roles->load('roles);

This seems unintuitive. I cannot imagine a case when you would want to update the database but NOT the model whose purpose is to model the data. This API uses that very model to desynchronize itself with its own datasource... using a method named "sync!" Madness!

Ok, I'm breathing. Breathing slowly...

Adding and saving later

Perhaps the app needs to do some sort of processing on the collection before we go saving to the database all willy nilly.

assert( empty($user->roles) );
$user->roles->push($role);
assert( count($user->roles) == 1 );  // right, this makes sense so far
// proprietary, industry disrupting, secret codes here
$user->roles-()->sync($user->roles);
assert( {{role is saved to database}} );

This works! But it's a little ugly. I would not consider this to be "eloquent."

Replacing and saving immediately

Let's say we know what we want. We're strong, independent developers that don't need no existing data, or sanity checks, or stuffy business rules to hold us down.

assert( empty($user->roles) );
assert( count($newRoles) > 1 );
$user->roles = $newRoles;
$user->roles()->sync($user->roles); // jumping on the ugly train here
assert( count($user->roles) > 1 );
assert( {{roles are saved to database}} );
$user->save(); // SQL Exception!

Reassigning a relationship causes eloquent to see it as an real property on the model. So it tries to write to the "roles" column on the table, which doesn't exist.

And I really don't want to write a for loop every time I need to do this.

There is an alternative...

assert( empty($user->roles) );
assert( count($newRoles) > 1 );
$user->roles()->sync($newRoles);
assert( count($user->roles) > 1 ); // GAAH!!
assert( {{roles are saved to database}} );
$user->save();

Look familiar? Right. Moving along...

Replacing and saving later

So we've grown up a little now, and we've realized that we really do need to pass $user through some sort of processing before saving it. But we're still deciding exactly which roles the user is in.

assert( empty($user->roles) );
assert( count($newRoles) > 1 );
$user->roles = $newRoles;
// top secret, world changing ... erm, look, it's just consolidated compliance certification audit business rule validation or something
// i just do what they tell me
// #synergy
$user->roles()->sync($newRoles);
assert( count($user->roles) > 1 ); // GAAH!!
assert( {{roles are saved to database}} );
$user->save();

Aaand we're back here again. We could give in and do the for loop thing. But we'd have to clear the collection first, and I found no way to do that. So we have to do this...

assert( empty($user->roles) );
assert( count($newRoles) > 1 );
removeMissingRoles($user, $newRoles);
addNewRoles($user, $newRoles);
// ...
$user->roles()->sync($newRoles);
assert( count($user->roles) > 1 ); // GAAH!!
assert( {{roles are saved to database}} );
$user->save();

function removeMissingRoles(User $user, Collection|Roles[] $roles) : void {
    // remove from $user->roles where not in $roles
}
function addNewRoles(User $user, Collection|Roles[] $roles) : void {
    // for each $roles
        // push to $user->roles if not in $user->roles
}

Really? Really?! Do you know how bad that is? So bad, I won't even do it the honor of being written properly. Just no.

Help

Please tell me I'm missing something grossly, embarrassingly obvious.

Helping myself

In my real-world use case, I ended up not (yet) needing to do some processing before saving. Here's what I'm doing for now...

trait MoreModelFeatures {


    /**
     * Add the given model to the given relationship.
     * This will update both the database, and the in-memory collection.
     * @param $relationship
     * @param Model $model
     */
    public function relate($relationship, Model $model) {
        $this->$relationship->push($model);
        $this->$relationship()->sync($model, false);
    }

    /**
     * Add the given model to the given relationship.
     * This will update both the database, and the in-memory collection.
     * @param $relationship
     * @param Model $model
     */
    public function unrelate($relationship, Model $model) {
        $this->$relationship()->detach($model);
        $this->load($relationship);
    }


}

It is used as follows...

$user->relate('roles', $role);
// happy hour

This allows me to update the model, the whole model, and nothing but the model ... and the database. If I want to replace all roles, I can't. Not yet. I'm going to write something like this later...

public function relateAll($relationship, $models){
    $this->$relationship()->sync($d);
    $this->load($relationship);
}

...but I don't like how this version does a full read from the database instead of just updating it in place.

1 like
17 replies
Snapey's avatar

im puzzled by all the discussion and concerns of in-memory representation of roles?

everything is tore down and reloaded for each request cycle so why is there any need to wory about the effect of changing roles?

1 like
musicin3d's avatar

@SNAPEY - That is an interesting point. For a simple application that saves a users role and completes the request, this whole thing is irrelevant.

However, if those loaded roles are used for anything after that save, there is potential for some strange bugs that might take another developer quite a while to diagnose.

For example, what if some middleware uses the same user object?

If anyone wonders why on earth you would do that, I'd ask, "Why re-query the user's information every time you need it in the same request?" (Or is it cached it somehow? I'm not aware of that feature in eloquent. And even so, the same concern applies.)

Snapey's avatar

why, when changing roles are you then going on to do other things in the same request, and relying on the roles just set? You seem to be overthinking something that at best is an extreme edge case?

musicin3d's avatar

@SNAPEY -

You seem to be overthinking something that at best is an extreme edge case?

That very well may be true. It really seems odd to me, though, that a save-like operation on a model would ever put it out of sync with the database. ...especially when it's so easy to just update the data structure as well.

why, when changing roles are you then going on to do other things in the same request

Users and roles are just a familiar example. Sure, for this specific feature it's probably an edge case concern. But I know, in general, models can be updated mid request as an auxiliary function, not as the primary directive. Those models may very well be used again in the same request. Again, it just seems like an oversight that eloquent can't guarantee that the change you just made to your model is actually represented in your model. Edit: And on the other side, persisting an update in the collection requires something as ugly as $user->roles()->sync($user->roles);?

Another problem is that feature and integration tests have to manually refresh the model they are testing, or they have to test both the model and the database. Right?

Snapey's avatar

The model is easily reloaded with fresh() however the user model is a little different because it is used by the Authentication Guard

And on the other side, persisting an update in the collection requires something as ugly as $user->roles()->sync($user->roles);?

Why is this ugly? It seems very expressive to me,

The user(), their roles(), sync with this new list (array of roles)

This one step, removes any roles that are no longer mentioned and adds new roles.

Not sure why you are specifying $user->roles inside the sync function?

1 like
Snapey's avatar

missed this

a save-like operation on a model would ever put it out of sync with the database

Why is it out of sync? You change the model attributes, you call save(). Any dirty attributes are pushed to the database.

Model object and database are aligned.

The only thing that might not be is another instance of the same model loaded earlier in the request cycle (eg, the user model loaded at authentication). Its only out of sync if you load a new instance of User and don't update the Auth::user() instance

1 like
musicin3d's avatar

@SNAPEY -

Not sure why you are specifying $user->roles inside the sync function?

Hold up. Can you call sync() without a parameter? Will it just use whatever is already in that collection?

$user->roles->push($role);
$user->roles()->sync();

Why is it out of sync? You change the model attributes, you call save(). Any dirty attributes are pushed to the database.

I was under the impression that save() might not update relationships, but I can't remember why. Is that not so?

Snapey's avatar

no, sync requires an array of role ids you want to associate with the user

save on a model only updates the model. Relations are handled in their own right.

musicin3d's avatar

@SNAPEY -

no, sync requires an array of role ids you want to associate with the user

PHPDoc on the method says, "Sync the intermediate tables with a list of IDs or collection of models."

Furthermore, you can look at the source and see that it uses the parseIds() method internally, and that method specifically handles BaseCollection, Collection, and Model.

Indeed, it works if you pass a collection of models.

Snapey's avatar

I didn't know we were debating that, but clearly sync($user->roles) makes no sense

1 like
musicin3d's avatar

That's exactly where I am. But it works, and it seems to be the best way to persist the updated collection. The alternative would be to reduce the collection to a list of IDs, which is worse in my opinion.

$ids = $user->roles->pluck('id');
$user->roles()->sync($ids);
// vs
$user->roles()->sync($user->roles);

Both are unnecessarily verbose. It is not good to have to choose between these options. I'm hoping that this is due to my misunderstanding, not a poorly designed API!

In case anyone wonders what I'm getting at... Imagine you have some class, say a service, who implements some sort of business logic but is NOT responsible for writing to the database. It could update $user->roles, and then some other class would be responsible for saving those roles, as they are, to the database.

Without such an ability, the model would just be an overengineered wrapper around database calls. Now, that's not the case, and I'm not saying it is. I'm just saying this use case is important, and it seems like there should be a better solution.

That's all dealing with one major concern.

The other major concern is the case in which you want to update immediately and then process the model afterwards. We've already gone back and forth on this, and it doesn't seem to be very compelling for you.

The best solution (without requiring hierarchical changes), seems to be...

$user->roles()->sync( whatever ); // or attach()
$user->load('roles');

Again, this makes an extra call to the database to read back all of the roles, even though we already know what changed. Not only is this verbose, but it's also wasteful!

musicin3d's avatar

In short... So far, it seems there isn't great support for cases where you want to both process and persist a model in the same request. And that's odd.

DigDoug's avatar

Sorry to resurrect this, but I came across it in search and it's a serious and subtle problem that is absolutely going to bite you if you build a reasonably complex Laravel application. It's a dark pattern, and it's made worse by the differing functionality of the various methods of updating a relationship (eg "associate" DOES update the in-memory relationship, while "save" does not). The mental load to remember that difference leads to bugs and problems.

But to be fair, it's not a problem that can really be solved. Say you have two memory loaded models $car and $driver with a belongsTo/hasOne relationship ($car->driver, $driver->car). You can only use the eloquent method to update the relationship via one of them ($car->driver->associate($newCar), $driver->car->save($newDriver)). Even if associate and save both updated relationships, they don't know you've got another affected model in memory as well.

For this reason, we've gotten away from using the Eloquent methods, and generally just set the relationship IDs directly. We extend all of our Models from a base class that implements the following method, and when a developer saves a change that will impact a relationship, it is up to them to make sure they "loadLazy" those relationships. We do a "lazy" load, because the developer may not know whether the calling function ultimately cares about this updated relationship or not. In this way, the cost to reload it is only incurred if it is actually used.

So in the $car/$driver example, we might just do:

$car->driver_id = $driver->id;
$car->save();
$car->loadLazy('driver');
$driver->loadLazy('car');

Verbose? Yes. But don't do it and let me know later when you find your first bug caused by stale in-memory relationships.

/**
     * Lazily reload relations on the model
     *
     * @param array|string $relations
     * @return $this
     */
    public function loadLazy($relations)
    {
        if (is_string($relations)) $relations = func_get_args();
        
        $loadedRelations = $this->getRelations();
        
        foreach ($relations as $relation) {
            unset($loadedRelations[$relation]);
        }
        
        $this->setRelations($loadedRelations);
        
        return $this;
    ```
madman81's avatar

I just found this topic because I ran into a similar issue as @musicin3d and to me this is some strange behaviour of Laravel. In the example below, only the last assert is failing. This is because the relationship was touched before and never updated on the model again. In complex applications this is rather confusing and causing bugs (that are hard to find with unit tests).

Refreshing a relationship before using it will slow down the requests, so that's not a favourable solution. Could it be reconsidered to update the model as well?

<?php


use Illuminate\Database\Eloquent\Model;
use Tests\TestCase;

class Book extends Model
{

}

class Author extends Model
{

    public function books()
    {
        return $this->hasMany(Book::class);
    }
}

class RelationshipOutOfSyncTest extends TestCase
{
    public function testRelationship1()
    {
        $author = Author::create();

        $this->assertDatabaseCount('books', 0);

        $book = new Book;
        $author->books()->save($book);

        $this->assertDatabaseCount('books', 1);
        $this->assertCount(1, $author->books);
    }

    public function testRelationship2()
    {
        $author = Author::create();

        $this->assertDatabaseCount('books', 0);
        $this->assertCount(0, $author->books);

        $book = new Book;
        $author->books()->save($book);

        $this->assertDatabaseCount('books', 1);
        $this->assertCount(1, $author->books);
    }
}
krisi_gjika's avatar

@madman81 "only the last assert is failing" - yes, since here $this->assertCount(0, $author->books); you have already loaded the books relation of that author in memory. If it queried the database for every $author->books call, there would be no point of eager loading anything.

However if you want to update the in memory collection, you can easily do so yourself.

$author = Author::create();

$this->assertDatabaseCount('books', 0);
$this->assertCount(0, $author->books);

$book = new Book;
$author->books()->save($book);
$author->books->push($book);

// or in cases where the relation has not been loaded yet
$author = Author::create();

$this->assertDatabaseCount('books', 0);

$book = new Book;
$author->books()->save($book);
// $author->load('books');  load ALL books via query
$author->setRelation('books', (new Book)->newCollection()->push($book); // load only this book in memory
// OR $book->setRelation('author', $author); the other way around assuming books can have only one author

Please or to participate in this conversation.