Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Observers: updated triggers itself with an update and getDirty retains the previous updated field #27138

Closed
IlyasDeckers opened this issue Jan 11, 2019 · 6 comments

Comments

@IlyasDeckers
Copy link

  • Laravel Version: 5.7.*
  • PHP Version: 7.2.10
  • Database Driver & Version: Mysql 5.7

Description:

I don't know if this is a bug or expected behaviour, we've been looking at this with 3 devs and no one could provide a proper answer.

I have an UserObserver and every time a user gets updated and the active field updates to true a new password is generated and a welcome email is sent.

The function looks like this.

public function updated (User $user)
{
    if ($user->active && $user->isDirty('active')) {
        $password = generatePassword();
        $user->password = bcrypt($password);
        $user->save();

        $user->notify(
            new UserWelcomeNotification(
                $user->email,
                $password,
                new MailResource(Email::getMailInfo(23))
            )
        );
    }
}

As you can see in the if statement there is a check to see if the user is active and if the database field has been changed (isDirty()). If this is true a new password is being generated, hashed with bcrypt, stored and then send to the user via notifications. (mail)
As expected the password update triggers the observer method again, but now the isDirty('active) should return false. This does not happen, it returns true through all the iterations. This goes on until PHP timeouts or xdebug throws an exception that the nesting level of 256 is reached.

Why does $user->isDirty('active') return true throughout every update in this loop although the last update in the observer did not update the active field?

When I call $user->getDirty() returns this the first time the updated method is triggered.

array(2) {
  'active' =>
  bool(true)
  'updated_at' =>
  string(19) "2019-01-11 11:27:13"
}

From the second time it returns until the timeout is reached:

array(3) {
  'password' =>
  string(60) "$2y$10$rlAbpelKnT/yp5zFhXcjwelEKkDEx5SfNJWqL1LiDltRnHYBLINmK"
  'active' =>
  bool(true)
  'updated_at' =>
  string(19) "2019-01-11 11:27:13"
}

I've taken a look at Illuminate\Database\Concerns\HasAttributes there I have tried to backtrace what happens exactly with getDirty, isDirty, wasChanged and isChanged. Although this gave great insights I can not find the source of this behaviour. Can someone explain why the updated fields from the previous iterations are present until the timeout is reached? My guess goes to the fact that the whole process has one call stack and $this->attributes only gets empty a successful return or in this example PHP that cuts the process because of a timeout.

Steps To Reproduce:

  • Create an Observer & observe an update on a model
  • use an if statement with isDirty('first_field_name')
  • In the if statement do an update on the same model, but on a different field
  • isDirty('first_field_name') still returns true
  • getDirty() returns the first_field_name on all itterations
@staudenmeir
Copy link
Contributor

This is happening because the updated event is fired before syncOriginal() is called.

@driesvints
Copy link
Member

Well, think about it. The "updated" event happens after your model was updated. So any changes you made on your model are bound to still be picked up by the isDirty call. The fact that $user->active returns true is indeed because it was changed to true from the original update. The original changes aren't cleared or anything. Since you are constantly referencing the same object being passed to the updated method this is the expected behavior.

Anyway, this is more of a question for one of the below support channels. If you can actually identify this as a bug, feel free to report back.

@CptMisery
Copy link

I know OP didn't say "how to I make this not happen", but those responses are trash. The solution to this issue is to use $model->unsetEventDispatcher() before the update inside the observer.

@james-tcc
Copy link

I have just come across the same issue. In case it helps anyone else, my problem was calling $model->update([...$attributes]) within the Observer which produced an infinite loop:

public function updating(Model $model)
{
	if ($model->isDirty('some_attribute')) {
		if ($model->some_attribute == 'this_value') {
			$model->update(['some_other_attr' => 'another_value']);
		}
	}
}

The solution is to simply call $model->some_other_attr = 'another_value'. This way, 'some_other_attr' gets added to the attributes to be updated when it is synced.

@abidulrmdn
Copy link

Changing the value without even updating or saving will reflect on model in db.
Cause after it finishes the updated event it will sync back with the same object that the event ended up with.

@rezatati
Copy link

Hi
here is final solution
https://stackoverflow.com/questions/29407818/is-it-possible-to-temporarily-disable-event-in-laravel/51301753

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants