ModelIdentifier instead of Laravel Model error in Listener depending on Listeners order for a same event.

[acacha]

• Laravel Version: 5.7.16
• PHP Version: 7.2.10-0ubuntu0.18.04.1
• Database Driver & Version:

Description:

ModelIdentifier object is passed to Listeners instead of real Laravel Model in some cases depending on order of Listeners for same Event.

Steps To Reproduce:

git clone https://github.com/acacha/ModelIndentifierProblem
cd ModelIndentifierProblem
composer install
php artisan migrate
cp .env.example .env
php artisan key:generate

Config database in .env file and run:

php artisan migrate

Run Laravel Valet or similar and execute route /error:

http://ModelIndentifierProblem1.test/error

Error:

Undefined property: Illuminate\Contracts\Database\ModelIdentifier::$user

Will be throwed

Change listeners order in EventServiceProvider from:

IncidentStored::class => [
            LogIncidentStored::class,
            SendIncidentCreatedEmail::class,
        ],

to:

IncidentStored::class => [
            SendIncidentCreatedEmail::class,
            LogIncidentStored::class,
        ],

And problem dissapears.

[koenhoeijmakers]

I think i've narrowed it down.

The Event gets passed directly to the Queue and because it is Serializeable it gets serialized as Jobs are also Serializeable, and because objects are mutable by default in php you'll get the serialized event in your next listener.

I first thought that this only happens on the Sync queue as thats actually just the event bus, but this happens on redis aswel.

Not sure on how to fix it though, maybe clone every object before it is going to be serialized?

[koenhoeijmakers]

I might have a fix, but i'm not sure if this problem only exists for this use case.
As in when the job that is being queued has a property that uses the trait Illuminate\Queue\SerializesModels.

I'll create a wip PR soon.

[acacha]

@koenhoeijmakers @driesvints Hello any news about that issue?

[koenhoeijmakers]

I haven't really figured out how to write proper tests for this, but the issue has passed by in my thoughts a few times which raises the following question.

Should we really apply a fix for this or would the consensus be "Don't queue a job that has a property which uses the trait Illuminate\Queue\SerializesModels"?

[acacha]

Ok I understand what you say it makes sense...

[acacha]

In the repo provided by myself the problem is with any Listener after LogIncidentStored. It seems LogIncidentStored listener (https://github.com/acacha/ModelIndentifierProblem/blob/master/app/Listeners/LogIncidentStored.php) is the cause of the error maybe because is an event listener that dispatch another event?

[taylorotwell]

Fixed by updating to PHP 7.4 which uses the new __serialize() and __unserialize() methods on SerializesModels.php which do not mutate the existing properties.

[yllndritb]

I think I have the same problem with php ^8.0 and Laravel 8.x. Any ideas | solutions?