serializer icon indicating copy to clipboard operation
serializer copied to clipboard

Doctrine ArrayCollections are recreated when deserialized making orphan-removal unusable

Open danrot opened this issue 4 years ago • 8 comments

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? yes

I am using Deserialization to automatically create entities based on a JSON representation of it. The DoctrineObjectConstructor works just fine for that, except when the entities are used in combination with Doctrine's orphan-removal feature.

I think the problem boils down to this line. In my case the DoctrineObjectConstructor loads the desired object from the database, which is fine. But when deserializing into that object, it replaces existing ArrayCollections from Doctrine with new ArrayCollections (that's what's happening in the above line). This is a problem starting from Doctrine 2.6.0, because if this collection belongs to an association using the orphan-removal annotation, the elements will be deleted, even if they're added to the new collection afterwards (there are more details to this in a doctrine issue).

I think the fix would be to just clear the existing collection and fill it with the new data, instead of creating a new one. Would you accept a PR for that? Or is there any reason a new instance has to be returned?

danrot avatar Jul 15 '20 14:07 danrot

I'm willing to accept such feature/bug-fix, but I think that is pretty hard to implement. Making the diff of a doctrine collection can be quite hard.

goetas avatar Jul 17 '20 05:07 goetas

@goetas I think it might be enough to reuse the existing ArrayCollection, even if we call clear before assigning the new values.

I tried to implement that, but I don't know how to access properties of the existing object in this context. Is that possible at all?

danrot avatar Jul 31 '20 13:07 danrot

@goetas I think it might be enough to reuse the existing ArrayCollection, even if we call clear before assigning the new values.

I tried to implement that, but I don't know how to access properties of the existing object in this context. Is that possible at all?

Can maybe anybody else answer that question?

danrot avatar Aug 20 '20 09:08 danrot

@goetas I think it might be enough to reuse the existing ArrayCollection, even if we call clear before assigning the new values. I tried to implement that, but I don't know how to access properties of the existing object in this context. Is that possible at all?

During deserialization is not yet possible... we should add the current object in the context... but that might break other things..

goetas avatar Aug 23 '20 15:08 goetas

@goetas im trying to solve this Problem. Could u specify "other things" ? (here my WIP proof of concept: https://github.com/re2bit/serializer/commit/ff5d732ce261e78b49935883bcf02cb60f5da295)

re2bit avatar Oct 11 '21 07:10 re2bit

@re2bit hi, sorry for the delay. I had a look at your implementation, i think that is too much added complexity for such edge-case (maybe for some is not ad edgecase.. :( )

I see that is adding a lot of extra code in the serializer namespace just to handle some weirdness of doctrine... and i start to thing that is not worth the effort.

could doctrine accept a fix on their side for it? i saw that there is a ton of people having the same issue...

goetas avatar Oct 24 '21 19:10 goetas

~~@re2bit now that i've read better the doctrine issue, can this be the solution?~~ nvm, that is what you were doing...

goetas avatar Oct 24 '21 19:10 goetas

@re2bit I've left this comment. I think that if such change can be done, the PR can be accepted.

goetas avatar Oct 24 '21 19:10 goetas