Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Laravel 5.5 - SerializesModels on event causes ModelIdentifier error?

I have a laravel event with a few listeners. Some listeners or their notifications (depending if they are time consuming) are implementing the ShouldQueue so they run in the background on a redis queue. The event uses SerializesModels by default, but when one of the passed models to the event is the logged in user and we fire it, like this:

$user = $this->user(); // logged in user instance
event(new UserCreatedPost($user, $post, $modelX, $modelY));

I am not able to access the user's followers in the respective listener, to check whether or not to notify them if they exist:

// In listener handle method
public function handle(UserCreatedPost $event){
    $followers = $event->user->followers()->get();
}

I get this error:

Call to undefined method Illuminate\Contracts\Database\ModelIdentifier::followers()

The only way I was able to get it to work was to add event wakeup below:

public function handle(UserCreatedPost $event){
    // This fixes it, as it unserializes all the models
    // (even though we only need this model to be unserialized, not all of them)
    $event->__wakeup();


    $followers = $event->user->followers()->first();
    // If at least one follower exists send queued notification
    // else exit
}

I do use the $user instance in a few other listeners under the same event and other event listeners. I don't even know if the $user should be serialized in the first place at all, but it is a model, so the parent event SerializesModels trait automatically serializes all the models (and I don't know of any way to make this specific model not be serialized while other models yes).

Is there a better way to be able to access the $user in the listeners without having to do the wakeup call at all? I have many events with listeners, and just got to implementing queues now, so I really don't want to add that wakeup to all areas in all files that the error would appear, but I do want to queue some of the listeners or their notifications. An alternative would be to remove the event SerializesModels trait and not even worry about that error showing up under this or any other listener I have yet to discover this error in. Are there any issues that can arise, like performance for example or others, by implementing the alternative approach? Any better way?

like image 606
Wonka Avatar asked Mar 30 '18 20:03

Wonka


1 Answers

__wakeup() (defined in SerializesModels) should actually be called by the framework when it unserializes events to execute their queued listeners. The whole purpose of this trait is that it only stores the model identifier in the serialized string (and not other properties) and reloads the model from the database when being unserialized. This is done to save space in the queue, but also because queue processing is delayed and properties can potentially change. So you never want to serialize the whole model with all its properties for queue processing.

This also means, you can do the very same thing manually, if you want. Instead of passing model objects to the event, you just pass some identifiers to it instead and load the models yourself (which isn't much more than Model::find($id), right?).

$user = $this->user(); // logged in user instance
event(new UserCreatedPost($user->id, $post->id, $modelX->id, $modelY->id));

And in the listener:

// In listener handle method
public function handle(UserCreatedPost $event){
    $user = User::find($event->user);
    $followers = $user->followers()->get();

    // the other stuff you want to do...
}

I have to admit that I'm not familiar with queued event listeners yet, only with queued jobs and similar. But from what you describe, I would say there might be a bug in the framework somewhere. Actually queued is not the listener but a job that calls the listener, called CallQueuedListener.

If you take the behavior Laravel utilizes for queued listeners, you could also use the same approach yourself: Don't queue the event listener, but let the listener create a queued job that sends the notifications. Maybe this works better.

like image 90
Namoshek Avatar answered Sep 17 '22 06:09

Namoshek