framework: Queued notification model serialization problem

Laravel Version: 5.3.18
PHP Version: 7.0.5

Description:

When a Notification is set to queue (it implements ShouldQueue, Queueable, SerializesModels) the via($notifiable) method doesn’t accept a call to a model passed into the notification constructor by returning a Undefined property: Illuminate\Contracts\Database\ModelIdentifier::$property

Steps To Reproduce:

create a notification

class MyNotification extends Notification implements ShouldQueue
{
    use Queueable;
    use SerializesModels;

    public $model;

    public function __construct($model)
    {
        $this->model = $model;
    }

    public function via($notifiable)
    {
        if($this->model->property == 1) {
            return ['mail'];
        } 
        return [];
    }
}

call a notification via

Notification::send($users, new MyNotification($model));

get a Undefined property: Illuminate\Contracts\Database\ModelIdentifier::$property error

About this issue

Original URL
State: closed
Created 8 years ago
Reactions: 2
Comments: 35 (15 by maintainers)

Most upvoted comments

I found the problem, you need to define it as protected in the notification class, like this:

class PaymentCreated extends Notification implements ShouldQueue
{
    use Queueable;

    protected $payment;

    public function __construct(Payment $payment)
    {
        $this->payment = $payment;
    }

I hope this helps.

hasandz on Mar 21, 2017

i have same issue, and i resolved, i juste remove SerializesModels in my classJob and the queue work fine

mehdirochdi on Jan 23, 2018

@lucadegasperi can you help me replicate? on a fresh installation can you replicate the issue?

themsaid on Nov 7, 2016

Not sure why this was closed, as it’s not fixed.

I’ve just spent hours tracking this issue down. After adding implements ShouldQueue to my notification, the model attributes are lost in the toMail and toArray methods:

I’m passing a model to the constructor and setting a public (or private/protected… makes no differernce) property. If I dump the model from the constructor, all attributes are there as expected, but if I dump the model from the toMail method, attributes are missing.

Obviously removing implements ShouldQueue fixes the problem, but slows the system. Is there a fix that doesn’t involve changes to Laravel source?

matbeard on Jun 8, 2018

I just experienced this problem myself. The problem was because my queue processor was still running the old code after deploying. You specifically need to terminate and restart your queue after your new code is released. If you’re using horizon f.e. you need to run horizon:purge & horizon:terminate right after activating a new release. That should be enough for your jobs to work properly with the SerializesModels trait.

driesvints on May 31, 2018

When sending the notification to multiple users (and getting the error above) what I saw is that the first time the via() method gets called it’s an instance of the model, the second time it’s an instance of Illuminate\Contracts\Database\ModelIdentifier this led me to believe there was a problem with the serialization / deserialization that happens when a notification is dispatched onto the bus.

lucadegasperi on Oct 13, 2016

See also https://github.com/laravel/framework/issues/26791 for more info about my issue how to reproduce and how to solve it.

acacha on Dec 8, 2018

@akalongman it prunes orphaned Horizon processes which aren’t managed anymore.

driesvints on Jun 1, 2018

I had the same issue when I had an event that fired 2 listeners. Each listener did not have any queue, instead, each listener fired a notification class, and each of the 2 notifications had a queue.

Once the first notification had implements ShouldQueue, the collection (with a join) I had passed to the listeners from the event gave the Undefined property: Illuminate\Contracts\Database\ModelIdentifier::$property and so the second queue would never run. Removing the implements ShouldQueue makes it work.

But since we need both notifications on the queue, what fixed it was removing SerializesModels from the event file. Now everything works with the queue as expected. Really not sure if this is a bug or what, since it works without the shouldQueue and only breaks with it!

MovingGifts on Mar 27, 2018

You’re right, @themsaid I’ll try as soon as I have time to put a demo together 😉

lucadegasperi on Nov 7, 2016