Remove embedded documents complexity

[jwage]

• Remove some of the additional complexity when working with embedded documents. Treat embedded documents like normal documents in the UnitOfWork as much as possible.
• Fix issue with ClassMetadata::associationMapping not inheriting associations correctly.
• Fix issue with re-persisting removed embedded document. Make it possible to re-persist removed embeded documents #719
• Fixes Document is upserted and then updated during one flush operation #1058, Fix changeset handling for new documents #999, Document is upserted and then updated during one flush operation #1059, UnitOfWork problem due to spl_object_hash collision #1017

[jmikola]

Type-hint $assoc as array here?

[jmikola]

@jwage: Is this still in progress?

[jwage]

@jmikola yes it is, I just haven't had time to continue working on it.

[jmikola]

[alcaeus]

Looks good to me. The issues I previously reported are gone. 👍

[malarzm]

@AronSzigetvari got any feedback for me by any chance? :)

[AronSzigetvari]

@malarzm, I have made a testing for the original problem and it seems to be OK, but I still have a failing test in the application test suite that I want to simplify to a minimal test or find out if we just used it wrong and had worked until now just because of some quirk.

[malarzm]

@AronSzigetvari sure, do you think you'll be able to pull it together today or tomorrow?

[AronSzigetvari]

@malarzm I am on it now so I very much hope so.

[malarzm]

@AronSzigetvari thank you very much! 👍

[AronSzigetvari]

@malarzm I have found the cause of our failing test, but I am not sure if it is a bug or not (a feature, or a fix of a previous bug):

Scenario: merging 2 carts. Cart is document, Item is embeddedDocument.
When we merged cart A and cart B, we added Items of cart B to cart A. Since it is the same Item object, after we call $dm->remove($cartB), we still have both items in $cartA->getItems(), but after calling $dm->flush(), the item that belongs to the removed $cartB is removed from $cartA, too.

I could make the test pass 2 different ways:

1. If I don't use the same Item object, but create a new one with the same data: $cartA($item->copy())
2. It also seems to work if we call $cartB->removeItem($item) when transferring items, so when we flush, the items doesn't belong to the removed parent document any more. (I am not sure if it is really OK, or may cause problems later)

It makes sense since remove is cascaded, but it used to work differently before, (however, persist should be also cascaded)
It is a BC for sure, but I am not sure it is a bug.

What do you think?

[malarzm]

@AronSzigetvari I would love to be able to just reuse embedded documents by passing them around to different documents but unfortunately we're not aware of any good solution for that :( I'd suggest going down 1), I'm personally always (deep) cloning objects that are about to change owner to ensure nothing will clash or have any side effects.

[malarzm]

Merged manually in b1089b7. @AronSzigetvari @blockjon @alcaeus @danez thank you very much for all your help!

[AronSzigetvari]

I have made a test that creates an owner document and an embedded document inside, then replaces the embedded, creates a new one with colliding hash value and checks the document state in the UOW. This fails with BETA10 and passes after this fix.
Probably too late to commit it in, since you have already merged the fix.

[malarzm]

@AronSzigetvari I've merged @danez test in 4817908 but if you think it's not enough feel free to submit a PR with passing test only :)

[AronSzigetvari]

@malarzm I have added a test, it is a bit similar to @danez 's, but it is a slightly different case (no detached documents were used)