From a6169a65ed6ea9d9e2b2665734b6d42a3282e6bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 13 Oct 2021 00:26:29 +0200 Subject: [PATCH 1/2] Allow users to customise Symfony's ContainerBuilder Under certain conditions, users might be interested in extending the container builder to modify its behaviour in the way that compiler passes aren't able to do. --- src/ContainerBuilder.php | 41 +++++++++++++++++-------- src/Generator.php | 14 ++++++--- test/CompilerTest.php | 18 +++++++++++ test/ContainerBuilderTest.php | 35 ++++++++++++++++++--- test/CustomContainerBuilderForTests.php | 16 ++++++++++ test/GeneratorTest.php | 19 ++++++++++++ 6 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 test/CustomContainerBuilderForTests.php diff --git a/src/ContainerBuilder.php b/src/ContainerBuilder.php index d8fc7cb6..73dd2db8 100644 --- a/src/ContainerBuilder.php +++ b/src/ContainerBuilder.php @@ -9,6 +9,7 @@ use Symfony\Component\Config\ConfigCache; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\PassConfig; +use Symfony\Component\DependencyInjection\ContainerBuilder as SymfonyBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use function assert; @@ -37,38 +38,54 @@ public static function default(string $configurationFile, string $namespace): se return self::xml($configurationFile, $namespace); } - public static function xml(string $configurationFile, string $namespace): self - { + /** @param class-string|null $builderClass */ + public static function xml( + string $configurationFile, + string $namespace, + ?string $builderClass = null, + ): self { return new self( new ContainerConfiguration($namespace), - new Generators\Xml($configurationFile), + new Generators\Xml($configurationFile, $builderClass), new ParameterBag(), ); } - public static function php(string $configurationFile, string $namespace): self - { + /** @param class-string|null $builderClass */ + public static function php( + string $configurationFile, + string $namespace, + ?string $builderClass = null, + ): self { return new self( new ContainerConfiguration($namespace), - new Generators\Php($configurationFile), + new Generators\Php($configurationFile, $builderClass), new ParameterBag(), ); } - public static function yaml(string $configurationFile, string $namespace): self - { + /** @param class-string|null $builderClass */ + public static function yaml( + string $configurationFile, + string $namespace, + ?string $builderClass = null, + ): self { return new self( new ContainerConfiguration($namespace), - new Generators\Yaml($configurationFile), + new Generators\Yaml($configurationFile, $builderClass), new ParameterBag(), ); } - public static function delegating(string $configurationFile, string $namespace): self - { + /** @param class-string|null $builderClass */ + public static function delegating( + string $configurationFile, + string $namespace, + ?string $builderClass = null, + ): self { return new self( new ContainerConfiguration($namespace), - new Generators\Delegating($configurationFile), + new Generators\Delegating($configurationFile, $builderClass), new ParameterBag(), ); } diff --git a/src/Generator.php b/src/Generator.php index b1b39ae7..f8a2700f 100644 --- a/src/Generator.php +++ b/src/Generator.php @@ -13,10 +13,16 @@ abstract class Generator { private Compiler $compiler; + /** @var class-string */ + private string $builderClass; - public function __construct(private string $configurationFile) - { - $this->compiler = new Compiler(); + /** @param class-string|null $builderClass */ + public function __construct( + private string $configurationFile, + ?string $builderClass = null, + ) { + $this->compiler = new Compiler(); + $this->builderClass = $builderClass ?? SymfonyBuilder::class; } /** @@ -41,7 +47,7 @@ private function loadContainer(ContainerConfiguration $config, ConfigCache $dump public function initializeContainer(ContainerConfiguration $config): SymfonyBuilder { - $container = new SymfonyBuilder(); + $container = new $this->builderClass(); $container->addResource(new FileResource($this->configurationFile)); $loader = $this->getLoader($container, $config->getPaths()); diff --git a/test/CompilerTest.php b/test/CompilerTest.php index 0589bac2..12edb562 100644 --- a/test/CompilerTest.php +++ b/test/CompilerTest.php @@ -14,6 +14,7 @@ use SplFileInfo; use Symfony\Component\Config\ConfigCache; use Symfony\Component\DependencyInjection\Compiler\PassConfig; +use Symfony\Component\DependencyInjection\Container; use function bin2hex; use function count; @@ -168,6 +169,23 @@ public function compilationShouldBeSkippedWhenFileAlreadyExists(): void self::assertCount(1, $generatedFiles); } + /** @test */ + public function compileShouldUseCustomContainerBuilders(): void + { + $compiler = new Compiler(); + $compiler->compile( + $this->config, + $this->dump, + new Yaml(__FILE__, CustomContainerBuilderForTests::class), + ); + + $container = include $this->dumpDir . '/AppContainer.php'; + + self::assertInstanceOf(Container::class, $container); + self::assertTrue($container->hasParameter('built-with-very-special-builder')); + self::assertTrue($container->getParameter('built-with-very-special-builder')); + } + /** @return PHPGenerator */ private function getGeneratedFiles(?string $dir = null): PHPGenerator { diff --git a/test/ContainerBuilderTest.php b/test/ContainerBuilderTest.php index 8debb4f0..6374c16d 100644 --- a/test/ContainerBuilderTest.php +++ b/test/ContainerBuilderTest.php @@ -52,8 +52,11 @@ public function configureDependencies(): void * @covers ::__construct * @covers ::setDefaultConfiguration */ - public function namedConstructorsShouldSimplifyTheObjectCreation(string $method, Generator $generator): void - { + public function namedConstructorsShouldSimplifyTheObjectCreation( + string $method, + Generator $generator, + ?string $builderClass = null, + ): void { $expected = new ContainerBuilder( new ContainerConfiguration('Lcobucci\\DependencyInjection'), $generator, @@ -61,10 +64,10 @@ public function namedConstructorsShouldSimplifyTheObjectCreation(string $method, ); // @phpstan-ignore-next-line - self::assertEquals($expected, ContainerBuilder::$method(__FILE__, __NAMESPACE__)); + self::assertEquals($expected, ContainerBuilder::$method(__FILE__, __NAMESPACE__, $builderClass)); } - /** @return iterable */ + /** @return iterable}> */ public function supportedFormats(): iterable { yield 'default' => ['default', new Generators\Xml(__FILE__)]; @@ -72,6 +75,30 @@ public function supportedFormats(): iterable yield 'yaml' => ['yaml', new Generators\Yaml(__FILE__)]; yield 'php' => ['php', new Generators\Php(__FILE__)]; yield 'delegating' => ['delegating', new Generators\Delegating(__FILE__)]; + + yield 'xml with custom builder' => [ + 'xml', + new Generators\Xml(__FILE__, CustomContainerBuilderForTests::class), + CustomContainerBuilderForTests::class, + ]; + + yield 'yaml with custom builder' => [ + 'yaml', + new Generators\Yaml(__FILE__, CustomContainerBuilderForTests::class), + CustomContainerBuilderForTests::class, + ]; + + yield 'php with custom builder' => [ + 'php', + new Generators\Php(__FILE__, CustomContainerBuilderForTests::class), + CustomContainerBuilderForTests::class, + ]; + + yield 'delegating with custom builder' => [ + 'delegating', + new Generators\Delegating(__FILE__, CustomContainerBuilderForTests::class), + CustomContainerBuilderForTests::class, + ]; } /** diff --git a/test/CustomContainerBuilderForTests.php b/test/CustomContainerBuilderForTests.php new file mode 100644 index 00000000..d4441147 --- /dev/null +++ b/test/CustomContainerBuilderForTests.php @@ -0,0 +1,16 @@ +parameterBag->set('built-with-very-special-builder', true); + + parent::compile($resolveEnvPlaceholders); + } +} diff --git a/test/GeneratorTest.php b/test/GeneratorTest.php index 2c700ac7..409d2933 100644 --- a/test/GeneratorTest.php +++ b/test/GeneratorTest.php @@ -49,6 +49,25 @@ public function initializeContainerShouldAddTheConfigurationFileAsAResource(): v self::assertEquals([new FileResource(__FILE__)], $container->getResources()); } + /** + * @test + * + * @covers ::__construct + * @covers ::initializeContainer + */ + public function initializeContainerCanOptionallyUseACustomClass(): void + { + $generator = $this->getMockForAbstractClass( + Generator::class, + [__FILE__, CustomContainerBuilderForTests::class], + ); + + self::assertInstanceOf( + CustomContainerBuilderForTests::class, + $generator->initializeContainer(new ContainerConfiguration('Me\\MyApp')), + ); + } + /** * @test * From 9d997c1a830d8c12db3d83e49b36ced3828bef34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Mon, 18 Oct 2021 23:53:10 +0200 Subject: [PATCH 2/2] Remove flaky test Depending on the order of the test suite execution we were having issues as the compiled container for `CompilerTest` was using the same FQCN as the `GeneratorTest`. This makes sure we use different namespaces for the generated containers, avoiding those conflicts. It also plugs vfsStream to remove the file system dependency on `CompilerTest`. Running PHPUnit using the flag `--random-order-seed=1634593025` was enough to reproduce the issue. --- test/CompilerTest.php | 20 +++++--------------- test/GeneratorTest.php | 12 ++++++------ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/test/CompilerTest.php b/test/CompilerTest.php index 12edb562..4cf329ee 100644 --- a/test/CompilerTest.php +++ b/test/CompilerTest.php @@ -16,15 +16,11 @@ use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\Container; -use function bin2hex; use function count; -use function exec; use function file_get_contents; use function file_put_contents; use function iterator_to_array; use function mkdir; -use function random_bytes; -use function realpath; /** * @covers \Lcobucci\DependencyInjection\Compiler @@ -53,7 +49,7 @@ final class CompilerTest extends TestCase public function configureDependencies(): void { vfsStream::setup( - 'tests', + 'tests-compilation', null, ['services.yml' => 'services: { testing: { class: stdClass } }'], ); @@ -67,8 +63,8 @@ public function configureDependencies(): void $this->dump = new ConfigCache($this->dumpDir . '/AppContainer.php', false); $this->config = new ContainerConfiguration( - 'Me\\MyApp', - [vfsStream::url('tests/services.yml')], + 'Me\\CompilationTest', + [vfsStream::url('tests-compilation/services.yml')], [ [$this->parameters, PassConfig::TYPE_BEFORE_OPTIMIZATION], [[MakeServicesPublic::class, []], PassConfig::TYPE_BEFORE_OPTIMIZATION], @@ -80,18 +76,12 @@ public function configureDependencies(): void private function createDumpDirectory(): string { - $dir = __DIR__ . '/../tmp/' . bin2hex(random_bytes(5)) . '/me_myapp'; + $dir = vfsStream::url('tests-compilation/tmp/me_myapp'); mkdir($dir, 0777, true); return $dir; } - /** @after */ - public function cleanUpDumpDirectory(): void - { - exec('rm -rf ' . realpath($this->dumpDir . '/../../')); - } - /** @test */ public function compileShouldCreateMultipleFilesForDevelopmentMode(): void { @@ -143,7 +133,7 @@ public function compileShouldTrackChangesOnTheConfigurationFile(): void public function compileShouldAllowForLazyServices(): void { file_put_contents( - vfsStream::url('tests/services.yml'), + vfsStream::url('tests-compilation/services.yml'), 'services: { testing: { class: stdClass, lazy: true } }', ); diff --git a/test/GeneratorTest.php b/test/GeneratorTest.php index 409d2933..2fb93f41 100644 --- a/test/GeneratorTest.php +++ b/test/GeneratorTest.php @@ -79,19 +79,19 @@ public function initializeContainerCanOptionallyUseACustomClass(): void public function generateShouldCompileAndLoadTheContainer(): void { vfsStream::setup( - 'tests', + 'tests-generation', null, ['services.yml' => 'services: { testing: { class: stdClass, public: true } }'], ); $config = new ContainerConfiguration( - 'Me\\MyApp', - [vfsStream::url('tests/services.yml')], + 'Me\\GenerationTest', + [vfsStream::url('tests-generation/services.yml')], [ [new ParameterBag(['app.devmode' => true]), PassConfig::TYPE_BEFORE_OPTIMIZATION], [ new DumpXmlContainer( - new ConfigCache(vfsStream::url('tests/dump.xml'), true), + new ConfigCache(vfsStream::url('tests-generation/dump.xml'), true), ), PassConfig::TYPE_AFTER_REMOVING, -255, @@ -99,7 +99,7 @@ public function generateShouldCompileAndLoadTheContainer(): void ], ); - $dump = new ConfigCache(vfsStream::url('tests/container.php'), false); + $dump = new ConfigCache(vfsStream::url('tests-generation/container.php'), false); $this->generator->method('getLoader')->willReturnCallback( static function (SymfonyBuilder $container, array $paths): YamlFileLoader { @@ -113,6 +113,6 @@ static function (SymfonyBuilder $container, array $paths): YamlFileLoader { $container = $this->generator->generate($config, $dump); self::assertInstanceOf(stdClass::class, $container->get('testing')); - self::assertFileExists(vfsStream::url('tests/dump.xml')); + self::assertFileExists(vfsStream::url('tests-generation/dump.xml')); } }