Message class subclass cannot be serialized
karelvasicek opened this issue · 13 comments
@karelvasicek See comments raise by @SamMousa: c656520#commitcomment-137621946
Why do you want to extend the class? Can you provide a use case of where extending class is the best way?
All properties have getters right? You could implement a serializer that uses those.
Why do you want to extend the class?
I posted an example in #58 (comment) . We extend the Message class and put corresponding (serialized) objects to the queue where messages are handled.
All properties have getters right?
I might be missing something, but private Email $email; does not have any getter.
Anyhow, after a new (important) private property was added to the base Message class, would we need to change the serializing logic again in our subclass? Not very convenient way of development, TBH. Even now we have to use a hack using reflections.
I wonder what's wrong with protected properties in framework libraries.
It makes them part of the public API. So any change to them then becomes a break in backwards compatibility.
So any change to them then becomes a break in backwards compatibility.
Is the recently added __sleep() needed then? It looks like a major BC break to me.
We extend the Message class and put corresponding (serialized) objects to the queue where messages are handled.
As a hack you can copy the two properties and __sleep() to your class to make it working. But as a long term solution you need to change the logic.
Permanent solution would be not to abuse the class. For example taking a second look at your example I wonder why do you need invoiceId in the message class. I think it is not a good design. Can you explain the use of invoiceId in your context?
I agree; it's not a good thing to add at all. Unfortunately I cant follow every commit in every repo where I contribute; so bad things are bound to happen.
In my opinion you should not be serializing objects you do not own. What happens if any of the libraries get updated while serialized in the queue?
What I do for mail, for example, is create my own value object with just the stuff my code uses and that is serializable. Then I serialize that and the task runner uses the library code to create a message object and sends it using the mailer.
Serialization is risky and with Yii2 comes with extra caveats because the message object also has a reference to the mailer component which cannot / should not be serialized; upon deserialization it tries to "magically" retrieve the mailer from the application, but your console application might be configured differently.
I wonder what's wrong with
protectedproperties in framework libraries.
It prevents library developers from changing or removing the fields without breaking things since someone might have overriden. But if they are private then you are sure that no one is affected if you decided to remove it and replace with a different design
Thanks guys!
abuse the class
Different approaches could be used, of course, but extending a class and putting into the queue does not look like an abuse to me.
Can you explain the use of invoiceId in your context?
It was a simple example. We use more properties. But as for the invoiceId, after the message is put into the queue (and serialized by Yii queue), it is handled by queue worker and PDF invoice is created (and attached) according to that invoiceId.
It was a simple example. We use more properties. But as for the
invoiceId, after the message is put into the queue (and serialized by Yii queue), it is handled by queue worker and PDF invoice is created (and attached) according to thatinvoiceId.
Well, that sounds like, again "abusing" this class for what it should not do. The class is for Message. I suggest you serialize the class then use some ways to add metadata. One example would be storing JSON version, with message property representing serialized message, invoice_id representing invoice and so on. The send the JSON string to message Queue
or other similar ways, which will avoid changing the class internals while doing the job
not look like an abuse to me.
It is. You're misusing fundamental OO concepts. You should not be extending 3rd party classes (I realize the irony since Yii2 often forces you to extend it's base classes). Use composition over inheritance.
In this scenario however even composition does not solve your serialization problem.
You're applying all kind of hacks to fix this "problem", but you're going about it wrong.
Instead of trying to make some other object do what you need, why not create a new DTO (Data Transfer Object) that does just what you need:
final readonly class PdfInvoiceMail {
public function __construct(
public string $toEmail,
public string $body,
public int $invoiceId
);
}Put that in the queue, as you can see your frontend code doesn't even need to know what kind of mailer implementation you use.
Then in the task handler use that object to interact with whatever API you use to send emails, so there you can use mailer->compose(..)->send() etc.
Closing since what @SamMousa proposed makes perfect sense to me. @karelvasicek please check if the approach is good for you.