[FrameworkBundle] Autoconfigure ORM attributes only if not done by DoctrineBundle #60001
Conversation
| }); | ||
|
|
||
| // DoctrineBundle autoconfigures attributes since version 2.14.0 | ||
| if (!array_key_exists(Entity::class, $container->getAutoconfiguredAttributes())) { |
There was a problem hiding this comment.
I'm not sure this is resilient if the DoctrineBundle is registered before FrameworkBundle. We can check the package version instead.
| if (!array_key_exists(Entity::class, $container->getAutoconfiguredAttributes())) { | |
| if (InstalledVersions::isInstalled('doctrine/doctrine-bundle') && version_compare(InstalledVersions::getVersion('doctrine/doctrine-bundle'), '2.14.0', '<')) { |
There was a problem hiding this comment.
this is not resilient at all irrespective of the order, because DI extensions receive a separate container builder (which is then merged into the main one), precisely to reduce the cases of writing logic that depends on the order (which is not guaranteed at all by Flex)
There was a problem hiding this comment.
Is the composer version check better?
| @@ -21,7 +21,7 @@ | |||
| "ext-xml": "*", | |||
| "symfony/cache": "^6.4|^7.0", | |||
| "symfony/config": "^7.3", | |||
| "symfony/dependency-injection": "^7.2", | |||
| "symfony/dependency-injection": "^7.3", | |||
There was a problem hiding this comment.
addExcludeTag was added in version 7.3 by #59704
maybe we can improve this instead? If we keep the current approach, we should use a feature test if possible. |
We use reflection on the closure. Creating a closure programmatically with a different list of arguments would be very complex. Instead, This issue was anticipated when the feature was introduced: #39897 (comment) |
|
New feature in a new PR: #60011 |
…nfiguration callbacks on the same class (GromNaN) This PR was squashed before being merged into the 7.3 branch. Discussion ---------- [DependencyInjection] Enable multiple attribute autoconfiguration callbacks on the same class | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | yes | Deprecations? | yes | Issues | Fix doctrine/DoctrineBundle#1868 (comment) | License | MIT Replace #60001 By having a list of callables for each attributes, we can enable merging definitions each having an autoconfiguration for the same attribute class. This is the case with the `#[Entity]` attribute in DoctrineBundle and FrameworkBundle. I have to deprecate `ContainerBuilder::getAutoconfiguredAttributes()` as its return type is `array<class-string, callable>`; so I added a new method `AttributeAutoconfigurationPass` that returns `array<class-string, callable[]>` in in order to use reflection on each callable in the compiler pass. Commits ------- e36fe60 [DependencyInjection] Enable multiple attribute autoconfiguration callbacks on the same class
An attribute cannot have multiple autoconfiguration callbacks registered. By excluding classes with the
#[Entity]and other ORM attributes in both FrameworkBundle (#59987) and DoctrineBundle (doctrine/DoctrineBundle#1868), the following exception is thrown: