From 30846cabb2f198bb0c6d8f967dc4484ed0b83d6a Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Tue, 21 Apr 2020 14:01:32 -0400 Subject: [PATCH] Fixing a bug where the wrong ManagerRegistry was sometimes used in repositories This problem occurs if using DoctrineBundle 1.11 or lower, where the newer Persistence alias is not available. This also centralizes the creation of the repository classes to fix this in all places. --- .../SetDoctrineManagerRegistryClassPass.php | 30 +++++++++++++++++ src/Doctrine/EntityClassGenerator.php | 33 +++++++++++++++---- src/Doctrine/EntityRegenerator.php | 22 ++++--------- src/Maker/MakeEntity.php | 15 ++++++--- src/Maker/MakeUser.php | 7 ++-- src/MakerBundle.php | 2 ++ src/Resources/config/makers.xml | 2 ++ src/Resources/config/services.xml | 1 + tests/Doctrine/EntityRegeneratorTest.php | 8 ++++- 9 files changed, 89 insertions(+), 31 deletions(-) create mode 100644 src/DependencyInjection/CompilerPass/SetDoctrineManagerRegistryClassPass.php diff --git a/src/DependencyInjection/CompilerPass/SetDoctrineManagerRegistryClassPass.php b/src/DependencyInjection/CompilerPass/SetDoctrineManagerRegistryClassPass.php new file mode 100644 index 000000000..384b49a71 --- /dev/null +++ b/src/DependencyInjection/CompilerPass/SetDoctrineManagerRegistryClassPass.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\DependencyInjection\CompilerPass; + +use Doctrine\Persistence\ManagerRegistry; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; + +/** + * Helps determine which "ManagerRegistry" autowiring alias is available. + */ +class SetDoctrineManagerRegistryClassPass implements CompilerPassInterface +{ + public function process(ContainerBuilder $container) + { + if ($container->hasAlias(ManagerRegistry::class)) { + $definition = $container->getDefinition('maker.entity_class_generator'); + $definition->addMethodCall('setMangerRegistryClassName', [ManagerRegistry::class]); + } + } +} diff --git a/src/Doctrine/EntityClassGenerator.php b/src/Doctrine/EntityClassGenerator.php index 34f374924..f25aa642c 100644 --- a/src/Doctrine/EntityClassGenerator.php +++ b/src/Doctrine/EntityClassGenerator.php @@ -12,8 +12,8 @@ namespace Symfony\Bundle\MakerBundle\Doctrine; use Doctrine\Common\Persistence\ManagerRegistry as LegacyManagerRegistry; -use Doctrine\Persistence\ManagerRegistry; use Symfony\Bundle\MakerBundle\Generator; +use Symfony\Bundle\MakerBundle\Str; use Symfony\Bundle\MakerBundle\Util\ClassNameDetails; /** @@ -23,6 +23,7 @@ final class EntityClassGenerator { private $generator; private $doctrineHelper; + private $managerRegistryClassName = LegacyManagerRegistry::class; public function __construct(Generator $generator, DoctrineHelper $doctrineHelper) { @@ -51,19 +52,37 @@ public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $ ] ); - $entityAlias = strtolower($entityClassDetails->getShortName()[0]); - $this->generator->generateClass( + $this->generateRepositoryClass( $repoClassDetails->getFullName(), + $entityClassDetails->getFullName(), + $withPasswordUpgrade) + ; + + return $entityPath; + } + + public function generateRepositoryClass(string $repositoryClass, string $entityClass, bool $withPasswordUpgrade) + { + $shortEntityClass = Str::getShortClassName($entityClass); + $entityAlias = strtolower($shortEntityClass[0]); + $this->generator->generateClass( + $repositoryClass, 'doctrine/Repository.tpl.php', [ - 'entity_full_class_name' => $entityClassDetails->getFullName(), - 'entity_class_name' => $entityClassDetails->getShortName(), + 'entity_full_class_name' => $entityClass, + 'entity_class_name' => $shortEntityClass, 'entity_alias' => $entityAlias, 'with_password_upgrade' => $withPasswordUpgrade, - 'doctrine_registry_class' => interface_exists(ManagerRegistry::class) ? ManagerRegistry::class : LegacyManagerRegistry::class, + 'doctrine_registry_class' => $this->managerRegistryClassName, ] ); + } - return $entityPath; + /** + * Called by a compiler pass to inject the non-legacy value if available. + */ + public function setMangerRegistryClassName(string $managerRegistryClassName) + { + $this->managerRegistryClassName = $managerRegistryClassName; } } diff --git a/src/Doctrine/EntityRegenerator.php b/src/Doctrine/EntityRegenerator.php index 0569ebf85..b91974b55 100644 --- a/src/Doctrine/EntityRegenerator.php +++ b/src/Doctrine/EntityRegenerator.php @@ -11,15 +11,12 @@ namespace Symfony\Bundle\MakerBundle\Doctrine; -use Doctrine\Common\Persistence\ManagerRegistry as LegacyManagerRegistry; use Doctrine\Common\Persistence\Mapping\MappingException as CommonMappingException; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\MappingException; -use Doctrine\Persistence\ManagerRegistry; use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; -use Symfony\Bundle\MakerBundle\Str; use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator; /** @@ -30,13 +27,15 @@ final class EntityRegenerator private $doctrineHelper; private $fileManager; private $generator; + private $entityClassGenerator; private $overwrite; - public function __construct(DoctrineHelper $doctrineHelper, FileManager $fileManager, Generator $generator, bool $overwrite) + public function __construct(DoctrineHelper $doctrineHelper, FileManager $fileManager, Generator $generator, EntityClassGenerator $entityClassGenerator, bool $overwrite) { $this->doctrineHelper = $doctrineHelper; $this->fileManager = $fileManager; $this->generator = $generator; + $this->entityClassGenerator = $entityClassGenerator; $this->overwrite = $overwrite; } @@ -226,19 +225,10 @@ private function generateRepository(ClassMetadata $metadata) return; } - // duplication in MakeEntity - $entityClassName = Str::getShortClassName($metadata->name); - - $this->generator->generateClass( + $this->entityClassGenerator->generateRepositoryClass( $metadata->customRepositoryClassName, - 'doctrine/Repository.tpl.php', - [ - 'entity_full_class_name' => $metadata->name, - 'entity_class_name' => $entityClassName, - 'entity_alias' => strtolower($entityClassName[0]), - 'with_password_upgrade' => false, - 'doctrine_registry_class' => interface_exists(ManagerRegistry::class) ? ManagerRegistry::class : LegacyManagerRegistry::class, - ] + $metadata->name, + false ); $this->generator->writeChanges(); diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index 21c1360c2..a3d7ab030 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -47,8 +47,9 @@ final class MakeEntity extends AbstractMaker implements InputAwareMakerInterface private $fileManager; private $doctrineHelper; private $generator; + private $entityClassGenerator; - public function __construct(FileManager $fileManager, DoctrineHelper $doctrineHelper, string $projectDirectory, Generator $generator = null) + public function __construct(FileManager $fileManager, DoctrineHelper $doctrineHelper, string $projectDirectory, Generator $generator = null, EntityClassGenerator $entityClassGenerator = null) { $this->fileManager = $fileManager; $this->doctrineHelper = $doctrineHelper; @@ -60,6 +61,13 @@ public function __construct(FileManager $fileManager, DoctrineHelper $doctrineHe } else { $this->generator = $generator; } + + if (null === $entityClassGenerator) { + @trigger_error(sprintf('Passing a "%s" instance as 5th argument is mandatory since version 1.15.1', Generator::class), E_USER_DEPRECATED); + $this->entityClassGenerator = new EntityClassGenerator($generator, $this->doctrineHelper); + } else { + $this->entityClassGenerator = $entityClassGenerator; + } } public static function getCommandName(): string @@ -137,8 +145,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $classExists = class_exists($entityClassDetails->getFullName()); if (!$classExists) { - $entityClassGenerator = new EntityClassGenerator($generator, $this->doctrineHelper); - $entityPath = $entityClassGenerator->generateEntityClass( + $entityPath = $this->entityClassGenerator->generateEntityClass( $entityClassDetails, $input->getOption('api-resource') ); @@ -766,7 +773,7 @@ private function isClassInVendor(string $class): bool private function regenerateEntities(string $classOrNamespace, bool $overwrite, Generator $generator) { - $regenerator = new EntityRegenerator($this->doctrineHelper, $this->fileManager, $generator, $overwrite); + $regenerator = new EntityRegenerator($this->doctrineHelper, $this->fileManager, $generator, $this->entityClassGenerator, $overwrite); $regenerator->regenerateEntities($classOrNamespace); } diff --git a/src/Maker/MakeUser.php b/src/Maker/MakeUser.php index 33e3d908c..58569cc52 100644 --- a/src/Maker/MakeUser.php +++ b/src/Maker/MakeUser.php @@ -51,13 +51,15 @@ final class MakeUser extends AbstractMaker private $configUpdater; private $doctrineHelper; + private $entityClassGenerator; - public function __construct(FileManager $fileManager, UserClassBuilder $userClassBuilder, SecurityConfigUpdater $configUpdater, DoctrineHelper $doctrineHelper) + public function __construct(FileManager $fileManager, UserClassBuilder $userClassBuilder, SecurityConfigUpdater $configUpdater, DoctrineHelper $doctrineHelper, EntityClassGenerator $entityClassGenerator) { $this->fileManager = $fileManager; $this->userClassBuilder = $userClassBuilder; $this->configUpdater = $configUpdater; $this->doctrineHelper = $doctrineHelper; + $this->entityClassGenerator = $entityClassGenerator; } public static function getCommandName(): string @@ -139,8 +141,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // A) Generate the User class if ($userClassConfiguration->isEntity()) { - $entityClassGenerator = new EntityClassGenerator($generator, $this->doctrineHelper); - $classPath = $entityClassGenerator->generateEntityClass( + $classPath = $this->entityClassGenerator->generateEntityClass( $userClassNameDetails, false, // api resource $userClassConfiguration->hasPassword() && interface_exists(PasswordUpgraderInterface::class) // security user diff --git a/src/MakerBundle.php b/src/MakerBundle.php index b09e8e4a1..a55160cde 100644 --- a/src/MakerBundle.php +++ b/src/MakerBundle.php @@ -13,6 +13,7 @@ use Symfony\Bundle\MakerBundle\DependencyInjection\CompilerPass\MakeCommandRegistrationPass; use Symfony\Bundle\MakerBundle\DependencyInjection\CompilerPass\RemoveMissingParametersPass; +use Symfony\Bundle\MakerBundle\DependencyInjection\CompilerPass\SetDoctrineManagerRegistryClassPass; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -28,5 +29,6 @@ public function build(ContainerBuilder $container) // add a priority so we run before the core command pass $container->addCompilerPass(new MakeCommandRegistrationPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 10); $container->addCompilerPass(new RemoveMissingParametersPass()); + $container->addCompilerPass(new SetDoctrineManagerRegistryClassPass()); } } diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index 2af2ed531..a4d4244e7 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -35,6 +35,7 @@ %kernel.project_dir% + @@ -107,6 +108,7 @@ + diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index d11e32d0f..2a02a1f9f 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -48,6 +48,7 @@ + diff --git a/tests/Doctrine/EntityRegeneratorTest.php b/tests/Doctrine/EntityRegeneratorTest.php index 68f094a6a..71ac3d5e0 100644 --- a/tests/Doctrine/EntityRegeneratorTest.php +++ b/tests/Doctrine/EntityRegeneratorTest.php @@ -12,10 +12,12 @@ namespace Symfony\Bundle\MakerBundle\Tests\Doctrine; use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; +use Doctrine\Persistence\ManagerRegistry; use PHPUnit\Framework\TestCase; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; use Symfony\Bundle\MakerBundle\Doctrine\DoctrineHelper; +use Symfony\Bundle\MakerBundle\Doctrine\EntityClassGenerator; use Symfony\Bundle\MakerBundle\Doctrine\EntityRegenerator; use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; @@ -110,10 +112,14 @@ private function doTestRegeneration(string $sourceDir, Kernel $kernel, string $n $fileManager = new FileManager($fs, $autoloaderUtil, $tmpDir); $doctrineHelper = new DoctrineHelper('App\\Entity', $container->get('doctrine')); + $generator = new Generator($fileManager, 'App\\'); + $entityClassGenerator = new EntityClassGenerator($generator, $doctrineHelper); + $entityClassGenerator->setMangerRegistryClassName(ManagerRegistry::class); $regenerator = new EntityRegenerator( $doctrineHelper, $fileManager, - new Generator($fileManager, 'App\\'), + $generator, + $entityClassGenerator, $overwrite );