Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DoctrineBridge] DoctrineLoader relies on AutoMappingTrait #41796

Closed
m-vo opened this issue Jun 22, 2021 · 7 comments
Closed

[DoctrineBridge] DoctrineLoader relies on AutoMappingTrait #41796

m-vo opened this issue Jun 22, 2021 · 7 comments
Labels
Bug DependencyInjection DoctrineBridge Status: Needs Review

Comments

@m-vo
Copy link
Contributor

m-vo commented Jun 22, 2021 •
edited

Symfony version(s) affected: >= 4.4

Description
DoctrineExtension#loadValidatorLoader registers the DoctrineLoader as a service. This class uses the Symfony\Component\Validator\Mapping\Loader\AutoMappingTrait which might not be available.

As soon as this service gets autloaded at container build time* this will produce a fatal error under PHP8.1:

PHP Fatal error:  During class fetch: Uncaught ReflectionException: Class "Symfony\Component\Validator\Mapping\Loader\AutoMappingTrait" not found while loading "Symfony\Bridge\Doctrine\Validator\DoctrineLoader`

*) one scenario is for instance reading class annotations/attributes and thereby calling $container->getReflectionClass()

How to reproduce

  1. Set up a Sf application with doctrine-bundle installed.
  2. Add a compiler pass that does the following: 
    foreach ($container->getDefinitions() as $definition) {
  $container->getReflectionClass($definition->getClass(), false);
}
  3. Build the container.

This reminded me a bit of #32395. 🙂 Maybe we could mark the service as synthetic if the AutoMappingTrait is not available or register a dummy instead?
@m-vo m-vo added the Bug label Jun 22, 2021
@m-vo m-vo mentioned this issue Jun 22, 2021
Merged
@derrabus
Copy link
Member

derrabus commented Jun 22, 2021 •
edited

Registering a service with a class that cannot be autoloaded is fine in general, as long as you don't wire it. If you think that this service should not be registered at all, please open an issue here: https://github.com/doctrine/DoctrineBundle/issues

If you think that this is a bug of the DependencyInjection component, you're in the right place, however. 🙂

@m-vo
Copy link
Contributor Author

m-vo commented Jun 22, 2021

You're right I guess. 🙂 My brain was probably assuming that the extension was part of the symfony/doctrine-bridge as well.
I'm going to open up an issue over at DoctrineBundle.

Though you could argue that the bridge then should either require its dependencies or tolerate missing them? (Not easy with traits, unfortunately.)

@m-vo m-vo mentioned this issue Jun 22, 2021
Closed
@derrabus
Copy link
Member

Though you could argue that the bridge then should either require its dependencies or tolerate missing them?

The latter is the case. The bridge is a collection of glue code for various Symfony components. If we made all dependencies mandatory, the bridge would pull a lot of components that you probably don't use. The alternative would be that we have a symfony/form-doctrine-bridge, a symfony/validator-doctrine-bridge, a symfony/uid-doctrine-bridge and so on. We would end up micro-managing tiny packages.

@m-vo m-vo changed the title [DoctrineBundle] DoctrineLoader relies on AutoMappingTrait [DoctrineBridge] DoctrineLoader relies on AutoMappingTrait Jun 23, 2021
@m-vo
Copy link
Contributor Author

m-vo commented Jun 23, 2021

To me it looks like symfony/validator does not seem to be needed for the DoctrineLoader class - other than for the trait.

It's just a few lines. Wdyt about pulling the code in to make it more resilient?

symfony/src/Symfony/Component/Validator/Mapping/Loader/AutoMappingTrait.php

Lines 24 to 33 in e1b81e3

private function isAutoMappingEnabledForClass(ClassMetadata $metadata, string $classValidatorRegexp = null): bool
{
// Check if AutoMapping constraint is set first
if (AutoMappingStrategy::NONE !== $strategy = $metadata->getAutoMappingStrategy()) {
return AutoMappingStrategy::ENABLED === $strategy;
}
// Fallback on the config
return null !== $classValidatorRegexp && preg_match($classValidatorRegexp, $metadata->getClassName());
}

@derrabus
Copy link
Member

To me it looks like symfony/validator does not seem to be needed for the DoctrineLoader class - other than for the trait.

That is not correct. The class implements LoaderInterface which also comes from this package. And honestly, without the validator, the whole class is pretty much useless.

@m-vo
Copy link
Contributor Author

m-vo commented Jun 30, 2021

Let's fix this over at DoctrineBundle then.

@m-vo m-vo closed this as completed Jun 30, 2021
@kbond kbond mentioned this issue Nov 16, 2021
Closed
@brzuchal
Copy link
Contributor

brzuchal commented Dec 2, 2021 •
edited

I don't use Validator component in my project but this issue hits me, looks like the loader is loaded anyway.

Bumping up doctrine/doctrine-bundle to v2.4.3 helped me.

bendavies added a commit to bendavies/ux that referenced this issue Jun 15, 2023
@bendavies
set minimum version of doctrine/doctrine-bundle to 2.4.3
2a02152 
resolves symfony/symfony#41796
bendavies added a commit to bendavies/ux that referenced this issue Jun 19, 2023
@bendavies
set minimum version of doctrine/doctrine-bundle to 2.4.3
cd35d73 
resolves symfony/symfony#41796
bendavies added a commit to bendavies/ux that referenced this issue Jun 19, 2023
@bendavies
set minimum version of doctrine/doctrine-bundle to 2.4.3
da6f7ca 
resolves symfony/symfony#41796
bendavies added a commit to bendavies/ux that referenced this issue Jun 19, 2023
@bendavies
set minimum version of doctrine/doctrine-bundle to 2.4.3
8defe95 
resolves symfony/symfony#41796
bendavies added a commit to bendavies/ux that referenced this issue Jun 19, 2023
@bendavies
set minimum version of doctrine/doctrine-bundle to 2.4.3
eadfe7a 
resolves symfony/symfony#41796
bendavies added a commit to bendavies/ux that referenced this issue Jun 19, 2023
@bendavies
set minimum version of doctrine/doctrine-bundle to 2.4.3
12afaf7 
resolves symfony/symfony#41796
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection DoctrineBridge Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@derrabus @brzuchal @m-vo @carsonbot