michaeldyrynda / laravel-efficient-uuid

MIT License
304 stars 42 forks source link

Model with binary UUID can't be used in a Laravel Queue payload #22

Open rogatty opened 4 years ago

rogatty commented 4 years ago

dispatch(new MyJob(ModelWithBinaryUuid::create())) fails with:

message: "Unable to JSON encode payload. Error code: 5"
exception: "Illuminate\Queue\InvalidPayloadException"
file: "/vendor/laravel/framework/src/Illuminate/Queue/Queue.php"
line: 91

This is a know issue documented in https://github.com/laravel/framework/issues/15235 and https://github.com/laravel/framework/issues/12194. Laravel's Queue uses json_encode() which doesn't call model's toJson() method. Because of that https://github.com/michaeldyrynda/laravel-model-uuid/blob/79b20e5b2d6cae7b64e45858ea9202b7f26975a6/src/GeneratesUuid.php#L167-L175 is not called.

I don't have any good ideas how to solve it without modifying the Laravel's Queue code. Would be nice to add a note to docs about it, so people using this package don't have to do the research I did. My workaround will be to not pass the whole model to the Queue but only the attributes I need.

vadminn commented 4 years ago

Binary data, such as raw image contents, should be passed through the base64_encode function before being passed to a queued job. Otherwise, the job may not properly serialize to JSON when being placed on the queue.

Laravel Docs

@rogatty I would suggest not to provide the model instance to the Job constructor and get the model instance inside the handle() method.

For example, SQS has 256kb message limit and model instance can be really large.

rogatty commented 4 years ago

@vadim-nilov in general, I agree. There are times when it makes sense to pass an instance, though. If you do while using this library, you'll have a bad time debugging what's wrong. Hopefully people will search on GitHub and find this issue 🙂

robvankeilegom commented 2 years ago

Laravel only serializes the model identifier for queueable jobs. So as long as you still have a regular id alongside the uuid on the model (which you should have) you'll be able to pass the model instance to the job. This is the case since L5.0.

If your queued job accepts an Eloquent model in its constructor, only the identifier for the model will be serialized onto the queue. When the job is actually handled, the queue system will automatically re-retrieve the full model instance and its loaded relationships from the database. This approach to model serialization allows for much smaller job payloads to be sent to your queue driver.

https://laravel.com/docs/9.x/queues#class-structure

binaryfire commented 9 months ago

@robvankeilegom I don't think that's the case. I'm using the normal int id as the primary key but I'm still hitting this exception. It seems to happen if any binary column is present.

robvankeilegom commented 9 months ago

@binaryfire Do you have the Illuminate\Queue\SerializesModels trait on your job?

binaryfire commented 9 months ago

@robvankeilegom Yeah I do.

binaryfire commented 9 months ago

@binaryfire Here's a reproduction repo if you're curious: https://github.com/binaryfire/filament-import-export-bug

Log into the Filament dashboard, head to Products and create a couple, then try exporting to CSV. It'll throw the exception. But if you remove the binary uuid column from the User model it works fine. This is the job in question.

We've been trying to figure out a workaround but no joy. Might have to remove the binary column and just switch back to string...