analogueorm / analogue

Analogue ORM : Data Mapper ORM for Laravel/PHP
MIT License
632 stars 51 forks source link

5.5 entities with generated proxies are unable to serialize due to a closure #236

Open michaelsadlon opened 6 years ago

michaelsadlon commented 6 years ago

I was working on upgrading to 5.5 and ran into some issues with the serialization. To be exact I upgraded analogue/orm from 5.4.0 to 5.5.18.

Basically in 5.5 the EntityProxy was replaced with a new ProxyFactory and when creating the generated class there is a closure inside. PHP is unable to serialize closures and fails with an exception:

Serialization of 'Closure' is not allowed

I ran into this when passing in my User entity to Notification::send(...) or when calling $user->notify(...). My User entity is still using the 5.4 base class and implements Notifiable. The serialization issue happened when the Queue tried to serialize the job (which contains the notifiable user).

I tried to work around this by implementing the Serializable interface and calling $this->toArray() and such but was not able to cover all the cases. Also upon a fresh boot Analogue ORM would throw an exception due to not being able to serialize() the dummy entity. Removing my overrides restored the original behavior.

I hacked a workaround by manually unsetting any proxied entites or collections but this isn't maintainable. I can probably separate User from Notifiable and create another class to deal with this but it all worked in 5.4 without issue.

My questions are:

I would greatly appreciate any insight into this problem. Maybe I'm just doing something wrong but since this all worked in 5.4 it feels like a regression.

RemiCollin commented 6 years ago

@michaelsadlon thanks for reporting this.

The proxy library we implemented in 5.5 doesn't support serialization indeed : see https://github.com/Ocramius/ProxyManager/issues/342

I guess we should use the same strategy Eloquent uses to deal with this issue (Eloquent object are not serializable either and uses a 'SerializesModels' trait to convert model to its primary key and fetch it from the database on wakeup).

michaelsadlon commented 6 years ago

Thanks for the reply!

I noticed the SerializesModels trait and was wondering if that was also the case for Eloquent. Good to have confirmation. The only other case where I need to serialize a model is as a property on a job or notification but I believe SerializesModels already handles this.

I've put my 5.5 upgrade on hold for the moment but if I come back to it I'll try the strategy you mentioned.