refactor #175 Upgrade PagerfantaBundle to new version with B/C layer …

…(mbabker) This PR was merged into the 1.7-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | yes | Related tickets | Sylius/Sylius#10928 | License | MIT This is an alternative version of #169 with a B/C layer to try and map the bundle configuration from `WhiteOctoberPagerfantaBundle` to `BabDevPagerfantaBundle`. The B/C layer involves interfacing directly with the container in a few ways: - Using a custom container extension that can process the configuration under `white_october_pagerfanta` and map it to the `babdev_pagerfanta` configuration (since none of the child nodes were renamed, this part is pretty simple) and re-create the renamed `white_october_pagerfanta.default_view` parameter - Using a compiler pass to handle the `white_october_pagerfanta.view_factory.class` parameter that I fully dropped to change the `Pagerfanta\View\ViewFactory` class to something else if so desired and to alias the renamed `twig.extension.pagerfanta` and `white_october_pagerfanta.view_factory` service IDs (the aliases are deprecated in favor of the new bundle's service IDs) The one part of #169 (comment) not handled is a class mapping layer for the namespace change. It'd be easy to throw together a file with a bunch of `class_alias()` calls, but as also noted in the comment, a bunch of classes were made final so if someone were extending them then even the aliasing wouldn't save them from that API break. In addition to just upgrading to a supported bundle version that also removes one roadblock to full Symfony 5 support, there are a number of new features in the Pagerfanta library and/or PagerfantaBundle that could be interesting to Sylius users, including: - Customizable route generation logic (no longer hardcoded into the Twig extension) - Twig template support alongside the pre-existing PHP templates - Tailwind CSS support (I've shipped the Twig template for it, need to add the PHP version still) - At some point serializer support will land (at work we're manually building a lot of responses from our API with data out of the `Pagerfanta\Pagerfanta` class, at some point it's going to make sense to move that logic into the serializers (both Symfony's and JMS') and improve the pager's use in an API context) Commits ------- 586459f Upgrade PagerfantaBundle to new version with B/C layer
Sylius · Aug 31, 2020 · 52df400 · 52df400
2 parents 29e9c88 + 586459f
commit 52df400
Show file tree

Hide file tree

Showing 12 changed files with 302 additions and 3 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -31,6 +31,7 @@ install:
             symfony/form:$SYMFONY_VERSION \
             symfony/framework-bundle:$SYMFONY_VERSION \
             symfony/security-csrf:$SYMFONY_VERSION \
+            symfony/translation:$SYMFONY_VERSION \
             symfony/twig-bundle:$SYMFONY_VERSION \
             symfony/validator:$SYMFONY_VERSION \
             symfony/yaml:$SYMFONY_VERSION \

diff --git a/composer.json b/composer.json
@@ -22,6 +22,7 @@
     "require": {
         "php": "^7.2",
 
+        "babdev/pagerfanta-bundle": "^2.0",
         "doctrine/doctrine-bundle": "^1.12|^2.0",
         "doctrine/persistence": "^1.3",
         "friendsofsymfony/rest-bundle": "^3.0",
@@ -33,10 +34,10 @@
         "symfony/form": "^4.4|^5.0",
         "symfony/framework-bundle": "^4.4|^5.0",
         "symfony/security-csrf": "^4.4|^5.0",
+        "symfony/translation": "^4.4|^5.0",
         "symfony/twig-bundle": "^4.4|^5.0",
         "symfony/validator": "^4.4|^5.0",
         "symfony/yaml": "^4.4|^5.0",
-        "white-october/pagerfanta-bundle": "^1.0",
         "willdurand/hateoas-bundle": "^2.0",
         "winzou/state-machine-bundle": "^0.3|^0.4"
     },

diff --git a/docs/installation.md b/docs/installation.md
@@ -25,7 +25,7 @@ return [
     new FOS\RestBundle\FOSRestBundle(),
     new JMS\SerializerBundle\JMSSerializerBundle($this),
     new Sylius\Bundle\ResourceBundle\SyliusResourceBundle(),
-    new WhiteOctober\PagerfantaBundle\WhiteOctoberPagerfantaBundle(),
+    new BabDev\PagerfantaBundle\BabDevPagerfantaBundle(),
     new Bazinga\Bundle\HateoasBundle\BazingaHateoasBundle(),
     new winzou\Bundle\StateMachineBundle\winzouStateMachineBundle(),
 ];

diff --git a/phpstan.neon b/phpstan.neon
@@ -11,6 +11,7 @@ parameters:
 
     excludes_analyse:
         - %currentWorkingDirectory%/src/Bundle/DependencyInjection/Configuration.php
+        - %currentWorkingDirectory%/src/Bundle/DependencyInjection/PagerfantaConfiguration.php
         - %currentWorkingDirectory%/src/Bundle/DependencyInjection/Driver/Doctrine/DoctrineODMDriver.php
         - %currentWorkingDirectory%/src/Bundle/DependencyInjection/Driver/Doctrine/DoctrinePHPCRDriver.php
         - %currentWorkingDirectory%/src/Bundle/Doctrine/ODM/*

diff --git a/psalm.xml b/psalm.xml
@@ -69,6 +69,7 @@
         <PossiblyNullReference>
             <errorLevel type="suppress">
                 <file name="src/Bundle/DependencyInjection/Configuration.php" />
+                <file name="src/Bundle/DependencyInjection/PagerfantaConfiguration.php" />
                 <file name="src/Bundle/Routing/Configuration.php" />
             </errorLevel>
         </PossiblyNullReference>

diff --git a/src/Bundle/DependencyInjection/Compiler/PagerfantaBridgePass.php b/src/Bundle/DependencyInjection/Compiler/PagerfantaBridgePass.php
@@ -0,0 +1,59 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+@trigger_error(sprintf('The "%s" class is deprecated since Sylius 1.8. Migrate your Pagerfanta configuration from WhiteOctoberPagerfantaBundle to BabDevPagerfantaBundle, the configuration bridge will be removed in Sylius 2.0.', PagerfantaBridgePass::class), \E_USER_DEPRECATED);
+
+/**
+ * Compiler pass to bridge the configuration from WhiteOctoberPagerfantaBundle to BabDevPagerfantaBundle
+ *
+ * @internal
+ */
+final class PagerfantaBridgePass implements CompilerPassInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container): void
+    {
+        $this->changeViewFactoryClass($container);
+        $this->aliasRenamedServices($container);
+    }
+
+    private function changeViewFactoryClass(ContainerBuilder $container): void
+    {
+        if (!$container->hasParameter('white_october_pagerfanta.view_factory.class') || !$container->hasDefinition('pagerfanta.view_factory')) {
+            return;
+        }
+
+        $container->getDefinition('pagerfanta.view_factory')
+            ->setClass($container->getParameter('white_october_pagerfanta.view_factory.class'));
+    }
+
+    private function aliasRenamedServices(ContainerBuilder $container): void
+    {
+        if ($container->hasDefinition('pagerfanta.twig_extension')) {
+            $container->setAlias('twig.extension.pagerfanta', 'pagerfanta.twig_extension')
+                ->setDeprecated(true, 'The "%alias_id%" service alias is deprecated since Sylius 1.8, use the "pagerfanta.twig_extension" service ID instead.');
+        }
+
+        if ($container->hasDefinition('pagerfanta.view_factory')) {
+            $container->setAlias('white_october_pagerfanta.view_factory', 'pagerfanta.view_factory')
+                ->setDeprecated(true, 'The "%alias_id%" service alias is deprecated since Sylius 1.8, use the "pagerfanta.view_factory" service ID instead.');
+        }
+    }
+}
diff --git a/src/Bundle/DependencyInjection/PagerfantaConfiguration.php b/src/Bundle/DependencyInjection/PagerfantaConfiguration.php
@@ -0,0 +1,68 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\ResourceBundle\DependencyInjection;
+
+use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
+use Symfony\Component\Config\Definition\Builder\TreeBuilder;
+use Symfony\Component\Config\Definition\ConfigurationInterface;
+
+@trigger_error(sprintf('The "%s" class is deprecated since Sylius 1.8. Migrate your Pagerfanta configuration from WhiteOctoberPagerfantaBundle to BabDevPagerfantaBundle, the configuration bridge will be removed in Sylius 2.0.', PagerfantaConfiguration::class), \E_USER_DEPRECATED);
+
+/**
+ * Container configuration to bridge the configuration from WhiteOctoberPagerfantaBundle to BabDevPagerfantaBundle
+ *
+ * @internal
+ */
+final class PagerfantaConfiguration implements ConfigurationInterface
+{
+    public const EXCEPTION_STRATEGY_TO_HTTP_NOT_FOUND = 'to_http_not_found';
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getConfigTreeBuilder(): TreeBuilder
+    {
+        $treeBuilder = new TreeBuilder('white_october_pagerfanta');
+
+        /** @var ArrayNodeDefinition $rootNode */
+        $rootNode = $treeBuilder->getRootNode();
+
+        $this->addExceptionsStrategySection($rootNode);
+
+        $rootNode
+            ->children()
+                ->scalarNode('default_view')
+                    ->defaultValue('default')
+                ->end()
+            ->end()
+        ;
+
+        return $treeBuilder;
+    }
+
+    private function addExceptionsStrategySection(ArrayNodeDefinition $node): void
+    {
+        $node
+            ->children()
+                ->arrayNode('exceptions_strategy')
+                    ->addDefaultsIfNotSet()
+                    ->children()
+                        ->scalarNode('out_of_range_page')->defaultValue(self::EXCEPTION_STRATEGY_TO_HTTP_NOT_FOUND)->end()
+                        ->scalarNode('not_valid_current_page')->defaultValue(self::EXCEPTION_STRATEGY_TO_HTTP_NOT_FOUND)->end()
+                    ->end()
+                ->end()
+            ->end()
+        ;
+    }
+}
diff --git a/src/Bundle/DependencyInjection/PagerfantaExtension.php b/src/Bundle/DependencyInjection/PagerfantaExtension.php
@@ -0,0 +1,64 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\ResourceBundle\DependencyInjection;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
+use Symfony\Component\HttpKernel\DependencyInjection\Extension;
+
+@trigger_error(sprintf('The "%s" class is deprecated since Sylius 1.8. Migrate your Pagerfanta configuration from WhiteOctoberPagerfantaBundle to BabDevPagerfantaBundle, the configuration bridge will be removed in Sylius 2.0.', PagerfantaExtension::class), \E_USER_DEPRECATED);
+
+/**
+ * Container extension to bridge the configuration from WhiteOctoberPagerfantaBundle to BabDevPagerfantaBundle
+ *
+ * @internal
+ */
+final class PagerfantaExtension extends Extension implements PrependExtensionInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function getAlias(): string
+    {
+        return 'white_october_pagerfanta';
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getConfiguration(array $config, ContainerBuilder $container): PagerfantaConfiguration
+    {
+        return new PagerfantaConfiguration();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function load(array $config, ContainerBuilder $container): void
+    {
+        $config = $this->processConfiguration($this->getConfiguration($config, $container), $config);
+
+        $container->setParameter('white_october_pagerfanta.default_view', $config['default_view']);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function prepend(ContainerBuilder $container): void
+    {
+        $config = $this->processConfiguration($this->getConfiguration([], $container), $container->getExtensionConfig($this->getAlias()));
+
+        $container->prependExtensionConfig('babdev_pagerfanta', $config);
+    }
+}
diff --git a/src/Bundle/SyliusResourceBundle.php b/src/Bundle/SyliusResourceBundle.php
@@ -15,10 +15,12 @@
 
 use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\DoctrineTargetEntitiesResolverPass;
 use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\Helper\TargetEntitiesResolver;
+use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\PagerfantaBridgePass;
 use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\RegisterFormBuilderPass;
 use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\RegisterResourceRepositoryPass;
 use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\RegisterResourcesPass;
 use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\WinzouStateMachinePass;
+use Sylius\Bundle\ResourceBundle\DependencyInjection\PagerfantaExtension;
 use Symfony\Component\DependencyInjection\Compiler\PassConfig;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
@@ -43,6 +45,9 @@ public function build(ContainerBuilder $container): void
         $container->addCompilerPass(new RegisterResourceRepositoryPass());
         $container->addCompilerPass(new RegisterFormBuilderPass());
         $container->addCompilerPass(new WinzouStateMachinePass());
+
+        $container->registerExtension(new PagerfantaExtension());
+        $container->addCompilerPass(new PagerfantaBridgePass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -1); // Should run after all passes from BabDevPagerfantaBundle
     }
 
     /**

diff --git a/src/Bundle/Tests/DependencyInjection/Compiler/PagerfantaBridgePassTest.php b/src/Bundle/Tests/DependencyInjection/Compiler/PagerfantaBridgePassTest.php
@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\ResourceBundle\Tests\DependencyInjection\Compiler;
+
+use BabDev\PagerfantaBundle\Twig\PagerfantaExtension;
+use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase;
+use Pagerfanta\View\ViewFactory;
+use Sylius\Bundle\ResourceBundle\DependencyInjection\Compiler\PagerfantaBridgePass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+final class PagerfantaBridgePassTest extends AbstractCompilerPassTestCase
+{
+    /**
+     * @test
+     */
+    public function it_creates_aliased_services_and_changes_the_view_factory_class(): void
+    {
+        $this->registerService('pagerfanta.twig_extension', PagerfantaExtension::class);
+        $this->registerService('pagerfanta.view_factory', ViewFactory::class);
+
+        $this->setParameter('white_october_pagerfanta.view_factory.class', 'My\ViewFactory');
+
+        $this->compile();
+
+        $this->assertContainerBuilderHasAlias('twig.extension.pagerfanta', 'pagerfanta.twig_extension');
+        $this->assertContainerBuilderHasAlias('white_october_pagerfanta.view_factory', 'pagerfanta.view_factory');
+        $this->assertContainerBuilderHasService('pagerfanta.view_factory', 'My\ViewFactory');
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function registerCompilerPass(ContainerBuilder $container): void
+    {
+        $container->addCompilerPass(new PagerfantaBridgePass());
+    }
+}
diff --git a/src/Bundle/Tests/DependencyInjection/PagerfantaExtensionTest.php b/src/Bundle/Tests/DependencyInjection/PagerfantaExtensionTest.php
@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\ResourceBundle\Tests\DependencyInjection;
+
+use BabDev\PagerfantaBundle\DependencyInjection\BabDevPagerfantaExtension;
+use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase;
+use Sylius\Bundle\ResourceBundle\DependencyInjection\PagerfantaExtension;
+
+final class PagerfantaExtensionTest extends AbstractExtensionTestCase
+{
+    /**
+     * @test
+     */
+    public function it_prepends_the_white_october_bundle_configuration_to_the_babdev_bundle(): void
+    {
+        $bundleConfig = [
+            'default_view' => 'twitter_bootstrap',
+            'exceptions_strategy' => [
+                'out_of_range_page' => 'custom',
+                'not_valid_current_page' => 'to_http_not_found',
+            ],
+        ];
+
+        // Prepend config now to allow the prepend pass to work
+        $this->container->prependExtensionConfig('white_october_pagerfanta', $bundleConfig);
+
+        $this->load($bundleConfig);
+
+        $this->assertSame([$bundleConfig], $this->container->getExtensionConfig('babdev_pagerfanta'));
+        $this->assertContainerBuilderHasParameter('white_october_pagerfanta.default_view', $bundleConfig['default_view']);
+    }
+
+    protected function getContainerExtensions(): array
+    {
+        return [
+            new PagerfantaExtension(),
+            new BabDevPagerfantaExtension(),
+        ];
+    }
+}
diff --git a/src/Bundle/test/app/AppKernel.php b/src/Bundle/test/app/AppKernel.php
@@ -28,7 +28,7 @@ public function registerBundles()
             new JMS\SerializerBundle\JMSSerializerBundle(),
             new Doctrine\Bundle\DoctrineBundle\DoctrineBundle(),
             new Sylius\Bundle\ResourceBundle\SyliusResourceBundle(),
-            new WhiteOctober\PagerfantaBundle\WhiteOctoberPagerfantaBundle(),
+            new BabDev\PagerfantaBundle\BabDevPagerfantaBundle(),
             new Bazinga\Bundle\HateoasBundle\BazingaHateoasBundle(),
             new Symfony\Bundle\TwigBundle\TwigBundle(),
             new \winzou\Bundle\StateMachineBundle\winzouStateMachineBundle(),