[DDC-3444] Refresh cascading of lazy loaded associations does not work

[martijndwars]

As described in DDC-1987, cascading of a refresh to lazy loaded associations does not work. I designed a test case to illustrate this problem.

The test case uses Gearman to simulate concurrent requests in a single test. This makes things a bit harder to understand, but I had no idea how to simulate concurrency without it. Please let me know how I can improve the test case.

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3444

We use Jira to track the state of pull requests and the versions they got
included in.

[adicosoi]

Hello,

I know this is somehow off-topic but I can really use some help.

There is an issue with refresh on OneToMany relations with orphanRemoval=true.

When removeElement is called on the collection the elements is correctly added on the UnitOfWork orphanRemovals. When the main entity is refreshed, the collection is also correctly refreshed, due to cascade refresh, but if the flush is called the element of the collection is removed anyway, because although the collection is correctly refreshed, the UnitOfWork orphanRemovals is not unset for this entity.

Any suggestions or workarounds for this issue.

Thank you

[Ocramius]

All tests pass when this patch is applied to master, therefore either the test is not exposing the issue correctly or it was fixed.

@martijndwars please check it again if you can.

[martijndwars]

I believe the bug still exists @Ocramius. I tried to provide a clearer test case. It still uses two models: Order which has a one-to-many association with Item. When I refresh Order, I expect that its associated Items are refreshed as well. However, only a refresh for Order is triggered.

The test case is cluttered with a lot of boilerplate and I had to change some fields from private to protected to be able test these things. Not sure I was allowed to do so, but I did not know how to test it differently.

[stof]

Please don't change the visibility just for testing purpose. This is very bad. Changing the visibility to protected impacts the maintenance of the library (we have to provide BC on protected things, or at least try to provide it)

[stof]

You should write a test reproducing the actual issue you had (which does not involve accessing internal properties) to ensure things work.

[beberlei]

-1 on the private => protected

[fprochazka]

@beberlei 👍 agreed

[martijndwars]

Thanks for the suggestions, I reverted my last commit. My original test case reproduces the issue I had. However, because it involves concurrency it is a bit hard (and slow) to run the tests. I currently run the following commands in this order (note the two worker instances):

1. vendor/bin/phpunit tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1987TestWorker.php
2. vendor/bin/phpunit tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1987TestWorker.php
3. vendor/bin/phpunit tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1987Test.php

When manually running the above commands the unit test fails. TravisCI will probably skip the test, because it won't have the Gearman workers running.

[yellow1912]

It seems like this issue is still in the latest version of Doctrine?

[Ocramius]

@yellow1912 since it's not closed: yes, very likely.

[zluiten]

Provided a fix for this in #6798