Fix missing Doctrine\Common\Persistence\ManagerRegistry class #3684

walva · 2020-08-25T21:09:30Z

Use Symfony Doctrine brigde namespace instead of Doctrine Common namespace for ManagerRegistry class

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tickets	#3683 1631 37935 (symfony issue) 691 667
License	MIT
Doc PR	api-platform/docs#...

Description
This issue appreared on Symfony today. It is fixed now, but i'm still unable to run composer require api. I wonder if there is something to do on api-platform side ?

How to reproduce
Create a new Symfony project with composer and add Api-Platform to it:

composer create-project symfony/skeleton new-project
cd new-project
composer require api

The last command no longer fail with the same error message as the Symfony linked issue.

Additionally:
I wasn't able to run the tests like described here. I'll have a look asap

weaverryan

We could/should also choose to conflict with doctrine/persistence < 1.3 - e.g. https://github.com/sensiolabs/SensioFrameworkExtraBundle/pull/691/files#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780R40

The reason is that 1.3 introduced the BC layer. So if you're using 1.3, then you WILL have class aliases for the new Doctrine\Persistence\ManagerRegistry. If you have an older version, then you would still be using Doctrine\Common\Persistence\ManagerRegistry. So the conflict guarantees the user will have the new one.

weaverryan · 2020-08-25T21:12:29Z

src/Bridge/Doctrine/Orm/Metadata/Property/DoctrineOrmPropertyMetadataFactory.php

@@ -15,7 +15,7 @@

 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
-use Doctrine\Common\Persistence\ManagerRegistry;
+use Symfony\Bridge\Doctrine\ManagerRegistry;


I think you actually want Doctrine\Persistence\ManagerRegistry - that's the lower-level interface that we're actually requiring. See https://github.com/sensiolabs/SensioFrameworkExtraBundle/pull/691/files#diff-c7d9812ffa6b2809dabcf5120f2d2ec1 as an example

Yes indeed that is a very good point. Using an abstract is way more restrictive than the interface itself. I though the bridge would manage the BC break by using the older or new version but it was an incorrect assumption.
I fear that Doctrine rollback the BC break which will again create some issue. I tried to use the method class_exists before using the use statement but that illegal in PHP. I'm a bit torn of the approach to take to be honest...

It seems the issue also impact Doctrine\Persistence\ObjectManager; (previously Doctrine\Common\Persistence\ObjectManager).
I need to figure out how to run the tests on my machine before going further (missing mondo db on my machine is the issue I think)

ben@SB3:/tmp/test/test (master) $ vendor/bin/simple-phpunit --stop-on-failure -vvv vendor/api-platform/core/tests/ PHP Fatal error: Trait 'ApiPlatform\Core\Tests\Bridge\Doctrine\Common\Filter\BooleanFilterTestTrait' not found in /tmp/test/test/vendor/api-platform/core/tests/Bridge/Doctrine/MongoDbOdm/Filter/BooleanFilterTest.php on line 25

If you clone the project and if you launch php -d memory_limit=-1 vendor/bin/phpunit tests/Bridge/Doctrine/MongoDbOdm/Filter without the mongodb extension, there should be no problem.

alanpoulain · 2020-08-26T08:21:13Z

I'm 👍 for adding a conflict for doctrine/persistence < 1.3.

walva · 2020-08-26T08:41:07Z

FYI, creating a new api-platform project in this order will work:

symfony new project
cd project
composer req orm
composer req doctrine/doctrine-bundle "2.1.0"
composer req api

alanpoulain · 2020-08-26T08:43:16Z

FYI, creating a new api-platform project in this order will work:
symfony new project
cd project
composer req orm
composer req doctrine/doctrine-bundle "2.1.0"
composer req api

Yes because, in 2.1.0, doctrine/persistence 2.0 is not allowed: https://github.com/doctrine/DoctrineBundle/blob/0fb513842c78b43770597ef3c487cdf79d944db3/composer.json#L30

walva · 2020-08-26T09:03:57Z

I don't why it closed automatically, I'm working on @alanpoulin proposal at the moment.

alanpoulain · 2020-08-26T09:59:52Z

@walva just adding the conflict is not enough. You should change the typehint like you have done before.

alanpoulain · 2020-08-26T10:00:43Z

composer.json

@@ -92,7 +92,8 @@
    },
    "conflict": {
        "doctrine/common": "<2.7",
-        "doctrine/mongodb-odm": "<2.0"
+        "doctrine/mongodb-odm": "<2.0",
+        "doctrine/persistence": ">1.3"


Suggested change

"doctrine/persistence": ">1.3"

"doctrine/persistence": "<1.3"

Ok got it !

Actually changing the namespace of Doctrine/Common/Persistence to Doctrine/Persistence will break if "doctrine/persistence": "<2.0".

But if I do so, I have this error:

composer install Loading composer repositories with package information Installing dependencies (including require-dev) from lock file Warning: The lock file is not up to date with the latest changes in composer.json. You may be getting outdated dependencies. It is recommended that you run `composer update` or `composer update <package name>`. Your requirements could not be resolved to an installable set of packages. Problem 1 - api-platform/core 2.5.x-dev conflicts with doctrine/persistence[1.3.0]. - doctrine/persistence 1.3.0 conflicts with api-platform/core[2.5.x-dev]. - Installation request for api-platform/core 2.5.x-dev -> satisfiable by api-platform/core[2.5.x-dev]. - Installation request for doctrine/persistence 1.3.0 -> satisfiable by doctrine/persistence[1.3.0].

Because of this, I'll stick with your proposition but I don't understand the root cause behind the scene.

To avoid doctrine/persistence 2.0 I added

"conflict": { "doctrine/common": ">=3.0" }

in my project. Now i can install the api platform. Could this work or do I miss something?

walva · 2020-08-26T12:35:19Z

I update composer.json by adding the conflict doctrine/persistence > 1.3 and all Doctrine\Common\Persistence to Doctrine\Persistence namespaces.
I started from upstream/2.5 then force pushed into my fork. I wasn't able to run the tests from 2.5:

php -d memory_limit=-1 vendor/bin/phpunit tests/
PHP Fatal error:  Declaration of ApiPlatform\Core\Tests\Fixtures\TestBundle\GraphQl\Type\Definition\DateTimeType::parseLiteral(GraphQL\Language\AST\Node $valueNode, ?array $variables = NULL) must be compatible with GraphQL\Type\Definition\LeafType::parseLiteral($valueNode, ?array $variables = NULL) in /tmp/test/test/vendor/api-platform/core/tests/Fixtures/TestBundle/GraphQl/Type/Definition/DateTimeType.php on line 81

alanpoulain · 2020-08-26T13:00:36Z

Try again by doing rm composer.lock && composer install before.
Please also fix the CS.

walva · 2020-08-26T13:44:58Z

Try again by doing rm composer.lock && composer install before.
Please also fix the CS.

Done !
Tests: 1779, Assertions: 12772, Warnings: 23, Skipped: 1.

alanpoulain · 2020-08-26T13:56:32Z

Thank you @walva and congratulations for your first contribution on API Platform 🙂
I hope the issue will be solved.
Now it needs @dunglas to make a patch release.

walva · 2020-08-26T15:17:14Z

Thank you @walva and congratulations for your first contribution on API Platform 🙂
I hope the issue will be solved.
Now it needs @dunglas to make a patch release.

Thanks for helping me @weaverryan and @alanpoulain. It's very motivating and I already learn a lot !

* 2.5: feat: bump JS deps Add changelog for 2.5.7 (#3691) fix: fix tests (#3688) Fix missing use statement (#3670) [MongoDB] Fix Behat tests (#3687) Fix missing Doctrine\Common\Persistence\ManagerRegistry class (#3684) [Swagger] Fix that subresource endpoints do not override custom endpoints #3676 - fixed subresource pagination support if primary resource has pagination disabled

weaverryan suggested changes Aug 25, 2020

View reviewed changes

weaverryan mentioned this pull request Aug 25, 2020

Using Symfony\Bridge\Doctrine\ManagerRegistry instead of Doctrine\Common\Persistence\ManagerRegistry #3683

Closed

walva force-pushed the fix_doctrine_bc_break branch from 46e3488 to cfbf95b Compare August 25, 2020 21:29

walva changed the title ~~Use Symfony Doctrine brigde namespace instead of Doctrine Common namespace for ManagerRegistry class~~ Fix missing Doctrine\Common\Persistence\ManagerRegistry class Aug 25, 2020

walva closed this Aug 26, 2020

walva force-pushed the fix_doctrine_bc_break branch from 0652da6 to d84282f Compare August 26, 2020 08:57

walva mentioned this pull request Aug 26, 2020

Avoid Doctrine persistance BC break by adding conflict doctrine/persistence >1.3 #3686

Closed

alanpoulain reopened this Aug 26, 2020

alanpoulain reviewed Aug 26, 2020

View reviewed changes

walva force-pushed the fix_doctrine_bc_break branch from ecd61f7 to 010c8b1 Compare August 26, 2020 12:32

walva force-pushed the fix_doctrine_bc_break branch from 010c8b1 to 9daa91e Compare August 26, 2020 12:38

Update namespaces according to doctrine/persistence 2.0

9501245

walva force-pushed the fix_doctrine_bc_break branch from 9daa91e to 9501245 Compare August 26, 2020 13:43

alanpoulain approved these changes Aug 26, 2020

View reviewed changes

alanpoulain merged commit 19de800 into api-platform:2.5 Aug 26, 2020

vincentchalamon mentioned this pull request Aug 28, 2020

ApiPlatform does not work with Symfony Skeleton #3692

Closed

	"doctrine/persistence": ">1.3"
	"doctrine/persistence": "<1.3"

Uh oh!

Fix missing Doctrine\Common\Persistence\ManagerRegistry class #3684

Fix missing Doctrine\Common\Persistence\ManagerRegistry class #3684

Uh oh!

Conversation

walva commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weaverryan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanpoulain commented Aug 26, 2020

Uh oh!

walva commented Aug 26, 2020

Uh oh!

alanpoulain commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walva commented Aug 26, 2020

Uh oh!

alanpoulain commented Aug 26, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walva Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walva commented Aug 26, 2020

Uh oh!

alanpoulain commented Aug 26, 2020

Uh oh!

walva commented Aug 26, 2020

Uh oh!

alanpoulain commented Aug 26, 2020

Uh oh!

walva commented Aug 26, 2020

Uh oh!

Uh oh!

walva commented Aug 25, 2020 •

edited

Loading

alanpoulain commented Aug 26, 2020 •

edited

Loading

walva Aug 26, 2020 •

edited

Loading