Fix the handling of circular references in the commit order calculator #7260
Conversation
|
Note that removing the call to persist |
stof
force-pushed
the
regression_commit_order
branch
2 times, most recently
from
722005f
to
4b72667
Compare
|
@guilhermeblanco I added another reproducer in a unit test at the commit order calculator level, based on what the UnitOfWork would be configuring in it in the failing functional test. I kept the 2 functional tests I added previously, as they show high-level usage (and so would be helpful in case of a future refactoring of this part to ensure things keep working). |
When finding a circular dependencies, we must ensure that all dependencies of a node have been visited before adding it to the sorted list.
stof
force-pushed
the
regression_commit_order
branch
from
4b72667
to
568c2d3
Compare
|
@guilhermeblanco the last commit in this PR is fixing the bug, with the idea I suggested in #7259 (comment) Can you validate whether this approach makes sense ? |
stof
changed the title
|
@stof That totally makes sense. It took me a while to even revive the whole operation/approach, but your solution is appropriate. Very well done! Thanks!!! |
|
I'm allowing someone else merge as I got very detached from 2.X land and don't have full context on merge process now between 2.5, 2.6 and 2.7. |
|
@guilhermeblanco 2.5 is not maintained anymore (except maybe for security patches, but I'm not even sure). regarding 2.6 vs 2.7, I don't know what is the merge process for maintainers (but this bugfix should definitely go in the next 2.6 patch release as it fixes a regression) |
|
Thanks @stof. 🚢 Will appear in 2.6.3. |
v2.6.3 [](https://travis-ci.org/doctrine/doctrine2) This release provides fixes for many things, specially: - Regression in commit order calculation - BC-break in `EntityManager#find()` using optimistic lock outside of transaction - PHP 7.3 compatibility issues -------------------------------------------- - Total issues resolved: **8** - Total pull requests resolved: **26** - Total contributors: **26** Documentation ------------- - [7472: fix incorrect phpdoc typehint](doctrine#7472) thanks to @seferov - [7465: Fixes tiny typo in the 'Working with DateTime instances' documentation](doctrine#7465) thanks to @unguul - [7444: Fixed URLs of doctrine-mapping.xsd in docs](doctrine#7444) thanks to @Naitsirch - [7441: $hydrationMode throughout can be a string as well as int (for custom modes)](doctrine#7441) thanks to @asgrim - [7435: Fix a typo on Documentation](doctrine#7435) thanks to @oguzdumanoglu - [7434: Removed FAQ paragraph stating public variables are disallowed](doctrine#7434) thanks to @Naitsirch and @flaushi - [7423: Update association-mapping.rst](doctrine#7423) thanks to @ThomasLandauer - [7421: JIRA to Github issues on Limitations and Known Issues](doctrine#7421) thanks to @seferov - [7412: Some formatting improvements](doctrine#7412) thanks to @ThomasLandauer - [7411: Autoload error when following the Getting Started Guide](doctrine#7411) thanks to @ThomasLandauer - [7401: &doctrine#91;docs&doctrine#93; Fix docblock in `inheritance-mapping.rst`](doctrine#7401) thanks to @bobdenotter - [7397: Update getting-started.rst](doctrine#7397) thanks to @eibt - [7394: Class 'Doctrine\Common\Persistence\Mapping\Driver\AnnotationDriver' not found](doctrine#7394) thanks to @ekosynth - [7378: Typo fix](doctrine#7378) thanks to @BenMorel - [7377: Fix query andX doctype](doctrine#7377) thanks to @sserbin - [7374: Deprecation message in documentation for YAML](doctrine#7374) thanks to @SenseException and @iltar - [7360: Document getPartialReference() properly](doctrine#7360) thanks to @lcobucci Bug --- - [7471: Fix parameter value processing for objects with unloaded metadata](doctrine#7471) thanks to @alcaeus - [7367: Fix for BC break in 2.6.2 when calling EM::find() with LockMode::OPTIMISTIC outside of a TX](doctrine#7367) thanks to @timdev - [7328: Handle removed parameters by tree walker in Paginator](doctrine#7328) thanks to @plfort - [7325: Make code php 7.3 lint-compatible](doctrine#7325) thanks to @paxal - [7317: &doctrine#91;XML&doctrine#93; Fix default value of many-to-many order-by to ASC](doctrine#7317) thanks to @alexdenvir - [7260: Fix the handling of circular references in the commit order calculator](doctrine#7260) thanks to @stof - [6830: fix applying column options on foreign key columns](doctrine#6830) thanks to @Tobion Improvement ----------- - [7428: CI: Test against PHP 7.3](doctrine#7428) thanks to @Majkl578 - [7363: Fix compatibility with phan](doctrine#7363) thanks to @philippe-unitiz - [7345: Correct DOMDocument constructor in test](doctrine#7345) thanks to @guilliamxavier - [7307: Fix remaining usages of deprecated ClassLoader and Inflector from doctrine/common](doctrine#7307) thanks to @Majkl578 and @simonwelsh
](https://travis-ci.org/doctrine/doctrine2){kind=link}
This fixes #7259
On 2.5, both tests are passing. On 2.6.2, the first one is failing while the second one is passing. The only difference between them is the order of
persist($fileVersion)andpersist($file)