Implement forward compatibility with Persistence 3

[greg0ire]

This PR aims at getting more compatible with Persistence 3 / see how much work is needed.

[greg0ire]

Locally I get

1) Doctrine\Tests\ORM\Functional\AdvancedDqlQueryTest::testAggregateWithHavingClause
Exception: [Doctrine\DBAL\Exception\NotNullConstraintViolationException] An exception occurred while executing a query: SQLSTATE[23000]: Integrity constraint violation: 19 NOT NULL constraint failed: company_employees.department

With queries:
4. SQL: '"ROLLBACK"' Params:
3. SQL: 'INSERT INTO company_employees (id, salary, department, startDate) VALUES (?, ?, ?, ?)' Params: 100000, 'IT', NULL, 'manager'
2. SQL: 'INSERT INTO company_cars (brand) VALUES (?)' Params: 'Caramba'
1. SQL: '"START TRANSACTION"' Params:

Trace:
/home/greg/dev/doctrine-orm/vendor/doctrine/dbal/src/Connection.php:1815
/home/greg/dev/doctrine-orm/vendor/doctrine/dbal/src/Connection.php:1750
/home/greg/dev/doctrine-orm/vendor/doctrine/dbal/src/Statement.php:184
/home/greg/dev/doctrine-orm/vendor/doctrine/dbal/src/Statement.php:221
/home/greg/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php:159
/home/greg/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1128
/home/greg/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:425
/home/greg/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:392
/home/greg/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/AdvancedDqlQueryTest.php:271
/home/greg/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/AdvancedDqlQueryTest.php:25
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/Framework/TestCase.php:1144
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/Framework/TestResult.php:726
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/Framework/TestCase.php:903
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/Framework/TestSuite.php:677
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/Framework/TestSuite.php:677
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/Framework/TestSuite.php:677
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:670
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/TextUI/Command.php:143
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/src/TextUI/Command.php:96
/home/greg/dev/doctrine-orm/vendor/phpunit/phpunit/phpunit:98
/home/greg/dev/doctrine-orm/vendor/bin/phpunit:110

I don't understand why.

[greg0ire]

I have installed a debugger, and as it turns out, the ORM is executing a query with an id: INSERT INTO company_employees (id, salary, department, startDate) VALUES (?, ?, ?, ?)

orm/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php

Lines 150 to 157 in 193c3ab

	$insertData = $this->prepareInsertData($entity);
	// Execute insert on root table
	$paramIndex = 1;
	foreach ($insertData[$rootTableName] as $columnName => $value) {
	$rootTableStmt->bindValue($paramIndex++, $value, $this->columnTypes[$columnName]);
	}

However $insertData["company_employees"] has no id:

   ▾ $insertData["company_employees"] = (array [4])
    \
     ⬦ $insertData["company_employees"]["salary"] = (int) 100000
     |
     ⬦ $insertData["company_employees"]["department"] = (string [2]) `IT`
     |
     ⬦ $insertData["company_employees"]["startDate"] = (null)
     |
     ⬦ $insertData["company_employees"]["discr"] = (string [7]) `manager`

The value for salary gets used for id, the value for departement, for salary, and so on.

That seems to be caused by the value of $rootTableName, which is company_persons with persistence 2, and company_employees for some reasons with persistence 3.

[greg0ire]

Ok, so I think the same PR that made this impossible to bisect because it breaks signature compatibility also breaks the code of AbstractClassMetadataFactory::getParentClasses() (see https://github.com/doctrine/persistence/pull/50/files#diff-ec77d2ded049e5791976987b1295eaadd51528aabeb9113cdb4f37807872ccaaR232)

The current implementation creates a variable $parentClasses then duplicates it:

    protected function getParentClasses(string $name)
    {
        // Collect parent classes, ignoring transient (not-mapped) classes.
        $parentClasses = [];


        $parentClasses = $this->getReflectionService()
            ->getParentClasses($name);


        foreach (array_reverse($parentClasses) as $parentClass) {
            if ($this->getDriver()->isTransient($parentClass)) {
                continue;
            }


            $parentClasses[] = $parentClass;
        }


        return $parentClasses;
    }

[greg0ire]

Blocked by doctrine/persistence#239 and a merge up

[greg0ire]

@derrabus @beberlei what do you think about the question I raise in 9a49aee ?

[derrabus]

I think, persistence should not be coupled to Doctrine Annotations at all. And tbh, that class does not actually need to read annotations. That's what I tried to solve in doctrine/persistence#217.

For now, you can get the attribute driver working again if you skip the call to the parant constructor.

--- a/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php	(revision 08eaba44ca081311495b4c153a9fab383863a06e)
+++ b/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php	(date 1645482841532)
@@ -46,7 +46,8 @@
             ));
         }
 
-        parent::__construct(new AttributeReader(), $paths);
+        $this->reader = new AttributeReader();
+        $this->addPaths($paths);
     }
 
     /**

[greg0ire]

I think we should resolve doctrine/persistence#217 before doing more work on this.

[greg0ire]

🤔 I don't reproduce the issue with https://phpstan.org/r/25a5c36c-067e-4306-9a8d-4433401d347a … let's try updating phpstan.

UPD: that does not help.

[greg0ire]

Fixed it locally by copy pasting phpdoc from EntityManagerInterface… don't ask me why it works 🤷‍♂️