doctrine/archived-orm
1
0
Fork 0
mirror of https://github.com/doctrine/orm.git synced 2026-03-23 22:42:18 +01:00
Code Issues 5.3k Packages Projects Releases 100 Wiki Activity

Duplicate entries in CommitOrderCalculator output #7154

New Issue
Closed
opened 2026-01-22 15:45:41 +01:00 by admin · 3 comments
admin commented 2026-01-22 15:45:41 +01:00
Owner
Copy Link

Originally created by @sylfabre on GitHub (May 19, 2023).

Bug Report

Q A
BC Break no
Version 2.15.1

Summary

While investigating an insertion order issue with a complex entity relationships setup, I found out that CommitOrderCalculator may return a set with duplicate class entries which is creating a bug in my application.

Current behavior

CommitOrderCalculator may return a set with duplicate class entries.

How to reproduce

This test shows that there are 7 elements in the sorted set of testCommitOrdering4():

<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM;

use Doctrine\ORM\Internal\CommitOrderCalculator;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\Tests\OrmTestCase;

/**
 * Tests of the commit order calculation.
 *
 * IMPORTANT: When writing tests here consider that a lot of graph constellations
 * can have many valid orderings, so you may want to build a graph that has only
 * 1 valid order to simplify your tests.
 */
class CommitOrderCalculatorTest extends OrmTestCase
{
    /** @var CommitOrderCalculator */
    private $_calc;

    protected function setUp(): void
    {
        $this->_calc = new CommitOrderCalculator();
    }

    public function testCommitOrdering1(): void
    {
        $class1 = new ClassMetadata(NodeClass1::class);
        $class2 = new ClassMetadata(NodeClass2::class);
        $class3 = new ClassMetadata(NodeClass3::class);
        $class4 = new ClassMetadata(NodeClass4::class);
        $class5 = new ClassMetadata(NodeClass5::class);

        $this->_calc->addNode($class1->name, $class1);
        $this->_calc->addNode($class2->name, $class2);
        $this->_calc->addNode($class3->name, $class3);
        $this->_calc->addNode($class4->name, $class4);
        $this->_calc->addNode($class5->name, $class5);

        $this->_calc->addDependency($class1->name, $class2->name, 1);
        $this->_calc->addDependency($class2->name, $class3->name, 1);
        $this->_calc->addDependency($class3->name, $class4->name, 1);
        $this->_calc->addDependency($class5->name, $class1->name, 1);

        $sorted = $this->_calc->sort();

        // There is only 1 valid ordering for this constellation
        $correctOrder = [$class5, $class1, $class2, $class3, $class4];

        self::assertSame($correctOrder, $sorted);
    }

    public function testCommitOrdering2(): void
    {
        $class1 = new ClassMetadata(NodeClass1::class);
        $class2 = new ClassMetadata(NodeClass2::class);

        $this->_calc->addNode($class1->name, $class1);
        $this->_calc->addNode($class2->name, $class2);

        $this->_calc->addDependency($class1->name, $class2->name, 0);
        $this->_calc->addDependency($class2->name, $class1->name, 1);

        $sorted = $this->_calc->sort();

        // There is only 1 valid ordering for this constellation
        $correctOrder = [$class2, $class1];

        self::assertSame($correctOrder, $sorted);
    }

    public function testCommitOrdering3(): void
    {
        // this test corresponds to the GH7259Test::testPersistFileBeforeVersion functional test
        $class1 = new ClassMetadata(NodeClass1::class);
        $class2 = new ClassMetadata(NodeClass2::class);
        $class3 = new ClassMetadata(NodeClass3::class);
        $class4 = new ClassMetadata(NodeClass4::class);

        $this->_calc->addNode($class1->name, $class1);
        $this->_calc->addNode($class2->name, $class2);
        $this->_calc->addNode($class3->name, $class3);
        $this->_calc->addNode($class4->name, $class4);

        $this->_calc->addDependency($class4->name, $class1->name, 1);
        $this->_calc->addDependency($class1->name, $class2->name, 1);
        $this->_calc->addDependency($class4->name, $class3->name, 1);
        $this->_calc->addDependency($class1->name, $class4->name, 0);

        $sorted = $this->_calc->sort();

        // There are multiple valid orderings for this constellation, but
        // the class4, class1, class2 ordering is important to break the cycle
        // on the nullable link.
        $correctOrders = [
            [$class4, $class1, $class2, $class3],
            [$class4, $class1, $class3, $class2],
            [$class4, $class3, $class1, $class2],
        ];

        // We want to perform a strict comparison of the array
        self::assertContains($sorted, $correctOrders, '', false, true);
    }

    public function testCommitOrdering4(): void
    {
        // this test corresponds to the GH7259Test::testPersistFileBeforeVersion functional test
        $class1 = new ClassMetadata(NodeClass1::class);
        $class2 = new ClassMetadata(NodeClass2::class);
        $class3 = new ClassMetadata(NodeClass3::class);
        $class4 = new ClassMetadata(NodeClass4::class);
        $class5 = new ClassMetadata(NodeClass5::class);
        $class6 = new ClassMetadata(NodeClass6::class);

        $this->_calc->addNode($class1->name, $class1);
        $this->_calc->addNode($class2->name, $class2);
        $this->_calc->addNode($class3->name, $class3);
        $this->_calc->addNode($class4->name, $class4);
        $this->_calc->addNode($class5->name, $class5);
        $this->_calc->addNode($class6->name, $class6);

        $this->_calc->addDependency($class1->name, $class5->name, 1);
        $this->_calc->addDependency($class1->name, $class4->name, 1);
        $this->_calc->addDependency($class1->name, $class2->name, 1);
        $this->_calc->addDependency($class2->name, $class3->name, 1);
        $this->_calc->addDependency($class2->name, $class1->name, 0);
        $this->_calc->addDependency($class3->name, $class5->name, 0);
        $this->_calc->addDependency($class3->name, $class4->name, 0);
        $this->_calc->addDependency($class3->name, $class3->name, 1);
        $this->_calc->addDependency($class3->name, $class6->name, 1);
        $this->_calc->addDependency($class3->name, $class2->name, 0);
        $this->_calc->addDependency($class4->name, $class3->name, 1);
        $this->_calc->addDependency($class4->name, $class1->name, 0);
        $this->_calc->addDependency($class5->name, $class3->name, 1);
        $this->_calc->addDependency($class5->name, $class1->name, 0);
        $this->_calc->addDependency($class6->name, $class3->name, 0);

        $sorted = $this->_calc->sort();

        // There are multiple valid orderings for this constellation
        $correctOrders = [
            [$class1, $class2, $class4, $class5, $class3, $class6],
            [$class1, $class2, $class5, $class4, $class3, $class6],
            [$class1, $class4, $class2, $class5, $class3, $class6],
            [$class1, $class4, $class5, $class2, $class3, $class6],
            [$class1, $class5, $class2, $class4, $class3, $class6],
            [$class1, $class5, $class4, $class2, $class3, $class6],
        ];

        // We want to perform a strict comparison of the array
        self::assertContains($sorted, $correctOrders, '', false, true);
    }
}

class NodeClass1
{
}
class NodeClass2
{
}
class NodeClass3
{
}
class NodeClass4
{
}
class NodeClass5
{
}
class NodeClass6
{
}

Expected behavior

Correct behavior is having as many entries in the sorted set than in the initial node list.

Originally created by @sylfabre on GitHub (May 19, 2023). ### Bug Report | Q | A |------------ | ------ | BC Break | no | Version | 2.15.1 #### Summary While investigating an insertion order issue with a complex entity relationships setup, I found out that `CommitOrderCalculator` may return a set with duplicate class entries which is creating a bug in my application. #### Current behavior `CommitOrderCalculator` may return a set with duplicate class entries. #### How to reproduce This test shows that there are 7 elements in the sorted set of `testCommitOrdering4()`: ``` <?php declare(strict_types=1); namespace Doctrine\Tests\ORM; use Doctrine\ORM\Internal\CommitOrderCalculator; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\Tests\OrmTestCase; /** * Tests of the commit order calculation. * * IMPORTANT: When writing tests here consider that a lot of graph constellations * can have many valid orderings, so you may want to build a graph that has only * 1 valid order to simplify your tests. */ class CommitOrderCalculatorTest extends OrmTestCase { /** @var CommitOrderCalculator */ private $_calc; protected function setUp(): void { $this->_calc = new CommitOrderCalculator(); } public function testCommitOrdering1(): void { $class1 = new ClassMetadata(NodeClass1::class); $class2 = new ClassMetadata(NodeClass2::class); $class3 = new ClassMetadata(NodeClass3::class); $class4 = new ClassMetadata(NodeClass4::class); $class5 = new ClassMetadata(NodeClass5::class); $this->_calc->addNode($class1->name, $class1); $this->_calc->addNode($class2->name, $class2); $this->_calc->addNode($class3->name, $class3); $this->_calc->addNode($class4->name, $class4); $this->_calc->addNode($class5->name, $class5); $this->_calc->addDependency($class1->name, $class2->name, 1); $this->_calc->addDependency($class2->name, $class3->name, 1); $this->_calc->addDependency($class3->name, $class4->name, 1); $this->_calc->addDependency($class5->name, $class1->name, 1); $sorted = $this->_calc->sort(); // There is only 1 valid ordering for this constellation $correctOrder = [$class5, $class1, $class2, $class3, $class4]; self::assertSame($correctOrder, $sorted); } public function testCommitOrdering2(): void { $class1 = new ClassMetadata(NodeClass1::class); $class2 = new ClassMetadata(NodeClass2::class); $this->_calc->addNode($class1->name, $class1); $this->_calc->addNode($class2->name, $class2); $this->_calc->addDependency($class1->name, $class2->name, 0); $this->_calc->addDependency($class2->name, $class1->name, 1); $sorted = $this->_calc->sort(); // There is only 1 valid ordering for this constellation $correctOrder = [$class2, $class1]; self::assertSame($correctOrder, $sorted); } public function testCommitOrdering3(): void { // this test corresponds to the GH7259Test::testPersistFileBeforeVersion functional test $class1 = new ClassMetadata(NodeClass1::class); $class2 = new ClassMetadata(NodeClass2::class); $class3 = new ClassMetadata(NodeClass3::class); $class4 = new ClassMetadata(NodeClass4::class); $this->_calc->addNode($class1->name, $class1); $this->_calc->addNode($class2->name, $class2); $this->_calc->addNode($class3->name, $class3); $this->_calc->addNode($class4->name, $class4); $this->_calc->addDependency($class4->name, $class1->name, 1); $this->_calc->addDependency($class1->name, $class2->name, 1); $this->_calc->addDependency($class4->name, $class3->name, 1); $this->_calc->addDependency($class1->name, $class4->name, 0); $sorted = $this->_calc->sort(); // There are multiple valid orderings for this constellation, but // the class4, class1, class2 ordering is important to break the cycle // on the nullable link. $correctOrders = [ [$class4, $class1, $class2, $class3], [$class4, $class1, $class3, $class2], [$class4, $class3, $class1, $class2], ]; // We want to perform a strict comparison of the array self::assertContains($sorted, $correctOrders, '', false, true); } public function testCommitOrdering4(): void { // this test corresponds to the GH7259Test::testPersistFileBeforeVersion functional test $class1 = new ClassMetadata(NodeClass1::class); $class2 = new ClassMetadata(NodeClass2::class); $class3 = new ClassMetadata(NodeClass3::class); $class4 = new ClassMetadata(NodeClass4::class); $class5 = new ClassMetadata(NodeClass5::class); $class6 = new ClassMetadata(NodeClass6::class); $this->_calc->addNode($class1->name, $class1); $this->_calc->addNode($class2->name, $class2); $this->_calc->addNode($class3->name, $class3); $this->_calc->addNode($class4->name, $class4); $this->_calc->addNode($class5->name, $class5); $this->_calc->addNode($class6->name, $class6); $this->_calc->addDependency($class1->name, $class5->name, 1); $this->_calc->addDependency($class1->name, $class4->name, 1); $this->_calc->addDependency($class1->name, $class2->name, 1); $this->_calc->addDependency($class2->name, $class3->name, 1); $this->_calc->addDependency($class2->name, $class1->name, 0); $this->_calc->addDependency($class3->name, $class5->name, 0); $this->_calc->addDependency($class3->name, $class4->name, 0); $this->_calc->addDependency($class3->name, $class3->name, 1); $this->_calc->addDependency($class3->name, $class6->name, 1); $this->_calc->addDependency($class3->name, $class2->name, 0); $this->_calc->addDependency($class4->name, $class3->name, 1); $this->_calc->addDependency($class4->name, $class1->name, 0); $this->_calc->addDependency($class5->name, $class3->name, 1); $this->_calc->addDependency($class5->name, $class1->name, 0); $this->_calc->addDependency($class6->name, $class3->name, 0); $sorted = $this->_calc->sort(); // There are multiple valid orderings for this constellation $correctOrders = [ [$class1, $class2, $class4, $class5, $class3, $class6], [$class1, $class2, $class5, $class4, $class3, $class6], [$class1, $class4, $class2, $class5, $class3, $class6], [$class1, $class4, $class5, $class2, $class3, $class6], [$class1, $class5, $class2, $class4, $class3, $class6], [$class1, $class5, $class4, $class2, $class3, $class6], ]; // We want to perform a strict comparison of the array self::assertContains($sorted, $correctOrders, '', false, true); } } class NodeClass1 { } class NodeClass2 { } class NodeClass3 { } class NodeClass4 { } class NodeClass5 { } class NodeClass6 { } ```` #### Expected behavior Correct behavior is having as many entries in the sorted set than in the initial node list.
admin closed this issue 2026-01-22 15:45:41 +01:00
admin commented 2026-01-22 15:45:41 +01:00
Author
Owner
Copy Link

@sylfabre commented on GitHub (May 19, 2023):

https://github.com/doctrine/orm/pull/10714 is fixing this issue

@sylfabre commented on GitHub (May 19, 2023): https://github.com/doctrine/orm/pull/10714 is fixing this issue
admin commented 2026-01-22 15:45:42 +01:00
Author
Owner
Copy Link

@mpdude commented on GitHub (May 31, 2023):

Hey there 👋🏼,

I am actively working on major changes to the commit order computation. Could you please have a look at #10547 and/or try if the branch entity-level-commit-order (you can Composer-install it as dev-entity-level-commit-order!) solves the issues for you? If so, please leave a note in #10547.

@mpdude commented on GitHub (May 31, 2023): Hey there 👋🏼, I am actively working on major changes to the commit order computation. Could you please have a look at #10547 and/or try if the branch `entity-level-commit-order` (you can Composer-install it as `dev-entity-level-commit-order`!) solves the issues for you? If so, please leave a note in #10547.
admin commented 2026-01-22 15:45:42 +01:00
Author
Owner
Copy Link

@sylfabre commented on GitHub (Jul 30, 2023):

This issue will be fixed by https://github.com/doctrine/orm/pull/10547/files

Thanks @mpdude

@sylfabre commented on GitHub (Jul 30, 2023): This issue will be fixed by https://github.com/doctrine/orm/pull/10547/files Thanks @mpdude
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
BC Break
Bug
Can't Fix
Deprecation
Documentation
DQL
Duplicate
Failing Test
Hacktoberfest
Improvement
Improvement
Incomplete
Invalid
Missing Tests
New Feature
PR Created
pull-request
Mirrored from GitHub Pull Request
 Question
Regression
Status: Waiting feedback
Waiting feedback
WIP
Won't Fix
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
admin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#7154
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?