From b6be3b9fb90076f5216f82f014ad3d52bfcc5b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20J=C3=A4ger?= Date: Thu, 15 Oct 2020 12:07:23 +0200 Subject: [PATCH 1/2] Use Symfony service to register templates instead of TwigUtil TwigUtil uses a static SplObjectStorage when registering templates. This leads to memory leaks when you have a lot of funtional tests. Registering templates through a service avoids that memory leak. --- .../Compiler/BuildConfigsPass.php | 6 +- DependencyInjection/PayumExtension.php | 5 ++ Resources/config/twig.xml | 12 +++ .../Compiler/BuildConfigsPassTest.php | 33 ++++++- .../PayumExtensionTest.php | 85 +++++++++++++++++-- Tests/Functional/Twig/PathRegistrarTest.php | 44 ++++++++++ Twig/PathRegistrar.php | 43 ++++++++++ 7 files changed, 218 insertions(+), 10 deletions(-) create mode 100644 Resources/config/twig.xml create mode 100644 Tests/Functional/Twig/PathRegistrarTest.php create mode 100644 Twig/PathRegistrar.php diff --git a/DependencyInjection/Compiler/BuildConfigsPass.php b/DependencyInjection/Compiler/BuildConfigsPass.php index 3364b004..06aebe16 100644 --- a/DependencyInjection/Compiler/BuildConfigsPass.php +++ b/DependencyInjection/Compiler/BuildConfigsPass.php @@ -23,8 +23,10 @@ public function process(ContainerBuilder $container) $builder = $container->getDefinition('payum.builder'); if ($container->hasDefinition('twig')) { - $config = ['twig.env' => '@twig']; - + $config = [ + 'twig.env' => '@twig', + 'twig.register_paths' => '@payum.twig.path_registrar' + ]; $builder->addMethodCall('addCoreGatewayFactoryConfig', [$config]); } diff --git a/DependencyInjection/PayumExtension.php b/DependencyInjection/PayumExtension.php index 7e33367d..1192acb6 100644 --- a/DependencyInjection/PayumExtension.php +++ b/DependencyInjection/PayumExtension.php @@ -53,6 +53,11 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('commands.xml'); $loader->load('form.xml'); + $bundles = $container->getParameter('kernel.bundles'); + if (isset($bundles['TwigBundle'])) { + $loader->load('twig.xml'); + } + if ($container->getParameter('kernel.debug')) { $loader->load('debug.xml'); } diff --git a/Resources/config/twig.xml b/Resources/config/twig.xml new file mode 100644 index 00000000..9328fa38 --- /dev/null +++ b/Resources/config/twig.xml @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/Tests/DependencyInjection/Compiler/BuildConfigsPassTest.php b/Tests/DependencyInjection/Compiler/BuildConfigsPassTest.php index 8979b777..7d7af16e 100644 --- a/Tests/DependencyInjection/Compiler/BuildConfigsPassTest.php +++ b/Tests/DependencyInjection/Compiler/BuildConfigsPassTest.php @@ -212,4 +212,35 @@ public function testShouldAddConfig(array $tagAttributes, $expected) $this->assertEquals($expected, $builder->getMethodCalls()); } -} \ No newline at end of file + + public function testShouldAddTwigPathRegistrar() + { + $builder = new Definition(); + $twig = new Definition(); + + $container = new ContainerBuilder(); + $container->setDefinition('payum.builder', $builder); + $container->setDefinition('twig', $twig); + $pass = new BuildConfigsPass(); + + $pass->process($container); + + $calls = $builder->getMethodCalls(); + + $this->assertEquals(# + $calls, + [ + [ + 'addCoreGatewayFactoryConfig', + [ + [ + 'twig.env' => '@twig', + 'twig.register_paths' => '@payum.twig.path_registrar' + ] + ] + ] + ] + ); + + } +} diff --git a/Tests/Functional/DependencyInjection/PayumExtensionTest.php b/Tests/Functional/DependencyInjection/PayumExtensionTest.php index 939a85f9..36b8d13f 100644 --- a/Tests/Functional/DependencyInjection/PayumExtensionTest.php +++ b/Tests/Functional/DependencyInjection/PayumExtensionTest.php @@ -89,6 +89,7 @@ public function shouldUsePayumBuilderServiceToBuildPayumService() $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); $extension = new PayumExtension; @@ -137,6 +138,7 @@ public function shouldSetGatewayConfigStorageToPayumBuilderIfConfigured() $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); $extension = new PayumExtension; @@ -186,21 +188,22 @@ public function shouldWrapGatewayConfigStorageByEncryptionDecoratorWhenDefuseEnc $configs = array($config); - $container = new ContainerBuilder(); - $container->setParameter('kernel.debug', false); + $containerBuilder = new ContainerBuilder(); + $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); $extension = new PayumExtension; - $extension->load($configs, $container); + $extension->load($configs, $containerBuilder); - $this->assertTrue($container->hasDefinition('payum.dynamic_gateways.cypher')); - $cypher = $container->getDefinition('payum.dynamic_gateways.cypher'); + $this->assertTrue($containerBuilder->hasDefinition('payum.dynamic_gateways.cypher')); + $cypher = $containerBuilder->getDefinition('payum.dynamic_gateways.cypher'); $this->assertSame(DefuseCypher::class, $cypher->getClass()); $this->assertSame('aSecretKey', $cypher->getArgument(0)); - $this->assertTrue($container->hasDefinition('payum.dynamic_gateways.encrypted_config_storage')); + $this->assertTrue($containerBuilder->hasDefinition('payum.dynamic_gateways.encrypted_config_storage')); - $storage = $container->getDefinition('payum.dynamic_gateways.encrypted_config_storage'); + $storage = $containerBuilder->getDefinition('payum.dynamic_gateways.encrypted_config_storage'); $this->assertSame(CryptoStorageDecorator::class, $storage->getClass()); $this->assertSame('payum.dynamic_gateways.encrypted_config_storage.inner', (string) $storage->getArgument(0)); $this->assertSame('payum.dynamic_gateways.cypher', (string) $storage->getArgument(1)); @@ -243,6 +246,7 @@ public function shouldConfigureSonataAdminClassForGatewayConfigModelSetInStorage $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); $extension = new PayumExtension; @@ -301,6 +305,7 @@ public function shouldInjectCypherToForGatewayConfigAdmin() $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); $extension = new PayumExtension; @@ -351,6 +356,7 @@ public function shouldNotConfigureSonataAdminClassForGatewayConfigIfDisabled() $containerBuilder = new ContainerBuilder(); $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); $extension = new PayumExtension; @@ -358,6 +364,71 @@ public function shouldNotConfigureSonataAdminClassForGatewayConfigIfDisabled() $this->assertFalse($containerBuilder->hasDefinition('payum.dynamic_gateways.gateway_config_admin')); } + + /** + * @test + */ + public function shouldConfigureTwigPathRegistrar() + { + $config = array( + 'security' => array( + 'token_storage' => array( + 'Payum\Core\Model\Token' => array( + 'filesystem' => array( + 'storage_dir' => sys_get_temp_dir(), + 'id_property' => 'hash' + ) + ) + ) + ), + 'gateways' => array(), + ); + + $configs = array($config); + + $containerBuilder = new ContainerBuilder(); + $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', ['TwigBundle' => 'TwigBundle']); + + $extension = new PayumExtension; + + $extension->load($configs, $containerBuilder); + + $this->assertTrue($containerBuilder->hasDefinition('payum.twig.path_registrar')); + } + + /** + * @test + */ + public function shouldNotConfigureTwigPathRegistrarWithoutTwig() + { + $config = array( + 'security' => array( + 'token_storage' => array( + 'Payum\Core\Model\Token' => array( + 'filesystem' => array( + 'storage_dir' => sys_get_temp_dir(), + 'id_property' => 'hash' + ) + ) + ) + ), + 'gateways' => array(), + ); + + $configs = array($config); + + $containerBuilder = new ContainerBuilder(); + $containerBuilder->setParameter('kernel.debug', false); + $containerBuilder->setParameter('kernel.bundles', []); + + $extension = new PayumExtension; + + $extension->load($configs, $containerBuilder); + + $this->assertFalse($containerBuilder->hasDefinition('payum.twig.path_registrar')); + } + } class TestGatewayConfig implements GatewayConfigInterface diff --git a/Tests/Functional/Twig/PathRegistrarTest.php b/Tests/Functional/Twig/PathRegistrarTest.php new file mode 100644 index 00000000..633c1ae0 --- /dev/null +++ b/Tests/Functional/Twig/PathRegistrarTest.php @@ -0,0 +1,44 @@ +twig = static::$container->get('twig'); + $this->payum = static::$container->get('payum'); + } + + + public function testPathsAreConfigured() + { + $this->payum->getGateway('barGateway'); + + $templateContent = $this->twig->render('@PayumPaypalExpressCheckout/confirmOrder.html.twig'); + + $this->assertContains( + '', + $templateContent + ); + } + +} diff --git a/Twig/PathRegistrar.php b/Twig/PathRegistrar.php new file mode 100644 index 00000000..f47cc838 --- /dev/null +++ b/Twig/PathRegistrar.php @@ -0,0 +1,43 @@ +fileLoader = new FilesystemLoader(); + + $currentLoader = $twig->getLoader(); + if ($currentLoader instanceof ChainLoader) { + $currentLoader->addLoader($this->fileLoader); + } else { + $twig->setLoader(new ChainLoader([$currentLoader, $this->fileLoader])); + } + } + + public function register(ArrayObject $config) + { + $paths = $config['payum.paths']; + + foreach ($paths as $namespace => $path) { + $this->fileLoader->addPath($path, $namespace); + } + } + + public function __invoke() + { + return call_user_func_array([$this, 'register'], func_get_args()); + } +} From 7c390ddb9c886a1f1ca0b3225027463d22f354e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20J=C3=A4ger?= Date: Mon, 19 Oct 2020 20:51:10 +0200 Subject: [PATCH 2/2] fix test for symfony 5 --- Tests/Functional/Twig/PathRegistrarTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Tests/Functional/Twig/PathRegistrarTest.php b/Tests/Functional/Twig/PathRegistrarTest.php index 633c1ae0..c6256ff2 100644 --- a/Tests/Functional/Twig/PathRegistrarTest.php +++ b/Tests/Functional/Twig/PathRegistrarTest.php @@ -33,7 +33,10 @@ public function testPathsAreConfigured() { $this->payum->getGateway('barGateway'); - $templateContent = $this->twig->render('@PayumPaypalExpressCheckout/confirmOrder.html.twig'); + $templateContent = $this->twig->render( + '@PayumPaypalExpressCheckout/confirmOrder.html.twig', + ['layout' => false] + ); $this->assertContains( '',