Change persistence ordering for OneToOne to allow making JoinColumns non-nullable#8703
Change persistence ordering for OneToOne to allow making JoinColumns non-nullable#8703rvanlaak wants to merge 2 commits intodoctrine:2.9.xfrom
Conversation
5cba2a7 to
cca8b89
Compare
…-nullable cascading relationship
…bug on non-nullable cascading relationship
|
I'm confused: how can a PR that changes only the |
|
The point would be that the tests in this PR should prove that the long-standing issues related to persistence ordering have been fixed by other changes/commits in the meanwhile, but that is something we should verify with a code review. I've also locally cherry-picked these tests on the 2.6 branch, and tests also pass over there. |
|
@rvanlaak do I understand correctly that this PR provides test cases to show other issues being fixed? If yes, it should not be too difficult to get these tests merged, and tests are always a good thing. Could you rebase this to 2.14.x? |
|
@mpdude yes it should. I see you also asked the other ticket creators (the ones mentioned on this PR description) to verify the situation, so let's also wait for their answer. |
|
I'd love to get your 👀 on #10547 |
|
@mpdude did check the changes and specifically the test cases on your PR, and they look extremely promising in solving all these commit order issues 👏 |
|
Feel free to also mark this PR as closed with your PR #10547 |
@Frikkle was the first to propose these tests in doctrine#6533. @rvanlaak followed up in doctrine#8703, making some adjustments.
@Frikkle was the first to propose these tests in doctrine#6533. @rvanlaak followed up in doctrine#8703, making some adjustments. Co-authored-by: Gabe van der Weijde <gabe.vanderweijde@triasinformatica.nl> Co-authored-by: Richard van Laak <rvanlaak@gmail.com> Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
closes #7006
closes #6499
closes #5538
closes #4230
The issue as discussed earlier in #7006, #6499, #5538 and #4230 makes it harder to encapsulate behavior inside domain objects, when following some basic principles of DDD aggregate roots. In other words, the aggregate should not leak it's child nor have a dependency on a em/repo for the child to get persisted.
To be clear on the workaround, setting the
@JoinColumnto nullable solves this, as Doctrine does take care of setting theidon the object at a later moment.In response to discussing a fix for #6499 we borrowed the test as @Frikkle proposed in #6533 and slightly adjusted it to demonstrate the issue on persisting cascading relationships.
Expected
The exception we should get on running the test, that would confirm that the persistence ordering is wrong:
Actual
There is pending discussion in #6533 on a proposed fix.
As the tests in this PR pass, that proves that the commit ordering works correctly.