Merge pull request #3239 from acrobat/redirect-performance

[RedirectBundle] Improve performance of the redirect router

UPGRADE-6.3.md

@@ -6,6 +6,39 @@ General
 
 - The supported Symfony version is 5.4.
 
+RedirectBundle
+--------------
+
+- The redirect-bundle has now some improved logic in the router itself. Using the old router logic is deprecated and the new will be the default in 7.0.
+  To enable the new and improved router, set the `kunstmaan_redirect.enable_improved_router` config to `true`.
+
+  This improved router has the following changes:
+  - Huge speed improvement with larger sets of cms redirects. See [#3239](https://github.com/Kunstmaan/KunstmaanBundlesCMS/pull/3239)
+  - Each redirect path in the database should start with a `/` so the improved redirect lookup will work. When migrating an existing website
+    to the new router setup, execute the following queries to prefix all redirects.
+    ```sql
+    UPDATE kuma_redirects SET origin = CONCAT('/', origin) WHERE origin NOT LIKE '/%';
+    UPDATE kuma_redirects SET target = CONCAT('/', target) WHERE target NOT LIKE '/%' AND target NOT LIKE '%://%';
+    ```
+  - Changed the wildcard redirect to be more correct for all usecases. See this comparisson in behaviour
+
+Old:
+
+| Origin  | Target  | Request url    | Result           |
+|---------|---------|----------------|------------------|
+| /news/* | /blog   | /news/article1 | /blog/article1   |
+| /news/* | /blog/* | /news/article1 | /blog/*/article1 |
+| /news   | /blog   | /news          | /blog            |
+
+New:
+
+| Origin  | Target  | Request url    | Result         |
+|---------|---------|----------------|----------------|
+| /news/* | /blog   | /news/article1 | /blog          |
+| /news/* | /blog/* | /news/article1 | /blog/article1 |
+| /news   | /blog   | /news          | /blog          |
+
+
 UtilitiesBundle
 ---------------
 

src/Kunstmaan/RedirectBundle/DependencyInjection/Configuration.php

@@ -19,6 +19,7 @@ public function getConfigTreeBuilder()
         $rootNode
             ->children()
                 ->scalarNode('redirect_entity')->defaultValue(Redirect::class)->end()
+                ->booleanNode('enable_improved_router')->defaultFalse()->end()
             ->end()
         ;
 

src/Kunstmaan/RedirectBundle/DependencyInjection/KunstmaanRedirectExtension.php

@@ -18,5 +18,14 @@ public function load(array $configs, ContainerBuilder $container)
         $loader->load('services.yml');
 
         $container->setParameter('kunstmaan_redirect.redirect.class', $config['redirect_entity']);
+
+        $enableImprovedRouter = $config['enable_improved_router'] ?? false;
+        $container->setParameter('.kunstmaan_redirect.enable_improved_router', $enableImprovedRouter);
+
+        if (!$enableImprovedRouter) {
+            trigger_deprecation('kunstmaan/redirect-bundle', '6.3', 'Not setting the "kunstmaan_redirect.enable_improved_router" config to true is deprecated, it will always be true in 7.0.');
+        }
+
+        $container->findDefinition('kunstmaan_redirect.redirectrouter')->addMethodCall('enableImprovedRouter', [$enableImprovedRouter]);
     }
 }

src/Kunstmaan/RedirectBundle/Form/EventSubscriber/RedirectPathNormalizerFormEventSubscriber.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace Kunstmaan\RedirectBundle\Form\EventSubscriber;
+
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\Form\Event\PreSubmitEvent;
+use Symfony\Component\Form\FormEvents;
+
+final class RedirectPathNormalizerFormEventSubscriber implements EventSubscriberInterface
+{
+    public static function getSubscribedEvents(): array
+    {
+        return [
+            FormEvents::PRE_SUBMIT => 'normalizeRedirectPath',
+        ];
+    }
+
+    public function normalizeRedirectPath(PreSubmitEvent $event): void
+    {
+        $path = $event->getData();
+        if (!$path) {
+            return;
+        }
+
+        if (null !== parse_url($path, \PHP_URL_SCHEME)) {
+            return;
+        }
+
+        $event->setData('/' . ltrim(trim($path), '/'));
+    }
+}

src/Kunstmaan/RedirectBundle/Form/RedirectAdminType.php

@@ -2,6 +2,7 @@
 
 namespace Kunstmaan\RedirectBundle\Form;
 
+use Kunstmaan\RedirectBundle\Form\EventSubscriber\RedirectPathNormalizerFormEventSubscriber;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
 use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
@@ -54,6 +55,9 @@ public function buildForm(FormBuilderInterface $builder, array $options)
             'label' => 'redirect.form.redirect.note.label',
             'required' => false,
         ]);
+
+        $builder->get('origin')->addEventSubscriber(new RedirectPathNormalizerFormEventSubscriber());
+        $builder->get('target')->addEventSubscriber(new RedirectPathNormalizerFormEventSubscriber());
     }
 
     /**

src/Kunstmaan/RedirectBundle/Repository/RedirectRepository.php

@@ -3,7 +3,30 @@
 namespace Kunstmaan\RedirectBundle\Repository;
 
 use Doctrine\ORM\EntityRepository;
+use Kunstmaan\RedirectBundle\Entity\Redirect;
 
 class RedirectRepository extends EntityRepository
 {
+    public function findByRequestPathAndDomain(string $path, string $domain): ?Redirect
+    {
+        $conn = $this->_em->getConnection();
+        $qb = $conn->createQueryBuilder();
+
+        $qb
+            ->select('redirect.id')
+            ->from('kuma_redirects', 'redirect')
+            // This allows to easily match the current request path with wildcard origin values in the redirect table.
+            ->where(':path LIKE REPLACE(redirect.origin, \'*\', \'%\')')
+            ->andWhere($qb->expr()->or('redirect.domain IS NULL', 'redirect.domain = \'\'', 'redirect.domain = :domain'))
+            ->setParameter('path', $path)
+            ->setParameter('domain', $domain)
+        ;
+
+        $redirectId = method_exists($qb, 'executeQuery') ? $qb->executeQuery()->fetchOne() : $qb->execute()->fetchColumn();
+        if (null === $redirectId) {
+            return null;
+        }
+
+        return $this->find($redirectId);
+    }
 }

src/Kunstmaan/RedirectBundle/Router/RedirectRouter.php

@@ -5,6 +5,7 @@
 use Doctrine\Persistence\ObjectRepository;
 use Kunstmaan\AdminBundle\Helper\DomainConfigurationInterface;
 use Kunstmaan\RedirectBundle\Entity\Redirect;
+use Kunstmaan\RedirectBundle\Repository\RedirectRepository;
 use Symfony\Component\Routing\Exception\RouteNotFoundException;
 use Symfony\Component\Routing\Matcher\UrlMatcher;
 use Symfony\Component\Routing\RequestContext;
@@ -20,12 +21,18 @@ class RedirectRouter implements RouterInterface
     /** @var RouteCollection */
     private $routeCollection;
 
-    /** @var ObjectRepository */
+    /** @var RedirectRepository */
     private $redirectRepository;
 
     /** @var DomainConfigurationInterface */
     private $domainConfiguration;
 
+    /**
+     * @internal
+     * @var bool
+     */
+    private $enableImprovedRouter = false;
+
     public function __construct(ObjectRepository $redirectRepository, DomainConfigurationInterface $domainConfiguration)
     {
         $this->redirectRepository = $redirectRepository;
@@ -83,6 +90,8 @@ public function generate($name, $parameters = [], $referenceType = self::ABSOLUT
      */
     public function match($pathinfo)
     {
+        $this->initRouteCollection($pathinfo);
+
         $urlMatcher = new UrlMatcher($this->getRouteCollection(), $this->getContext());
 
         return $urlMatcher->match($pathinfo);
@@ -96,6 +105,7 @@ public function match($pathinfo)
     public function getRouteCollection()
     {
         if (\is_null($this->routeCollection)) {
+            // NEXT_MAJOR: Remove initRoutes logic
             $this->routeCollection = new RouteCollection();
             $this->initRoutes();
         }
@@ -216,4 +226,60 @@ public function setContext(RequestContext $context)
     {
         $this->context = $context;
     }
+
+    private function initRouteCollection(string $pathInfo): void
+    {
+        if (false === $this->enableImprovedRouter) {
+            trigger_deprecation('kunstmaan/redirect-bundle', '6.3', 'Not enabling the improved router is deprecated and the changed and improved redirect logic will be the default in 7.0. Set the "kunstmaan_redirect.enable_improved_router" config to true.');
+
+            return;
+        }
+
+        if (null !== $this->routeCollection) {
+            return;
+        }
+
+        $this->routeCollection = new RouteCollection();
+
+        $domain = $this->domainConfiguration->getHost();
+        $redirect = $this->redirectRepository->findByRequestPathAndDomain($pathInfo, $domain);
+        if (null === $redirect) {
+            return;
+        }
+
+        $routePath = str_contains($redirect->getOrigin(), '/*') ? $pathInfo : $redirect->getOrigin();
+        $targetPath = $redirect->getTarget();
+        if (str_contains($redirect->getOrigin(), '/*') && str_contains($redirect->getTarget(), '/*')) {
+            $origin = rtrim($redirect->getOrigin(), '/*');
+            $target = rtrim($redirect->getTarget(), '/*');
+            $targetPath = str_replace($origin, $target, $pathInfo);
+        }
+
+        $needsUtf8 = false;
+        foreach ([$routePath, $targetPath] as $item) {
+            if (preg_match('/[\x80-\xFF]/', $item)) {
+                $needsUtf8 = true;
+
+                break;
+            }
+        }
+
+        $route = new Route($routePath, [
+            '_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
+            'path' => $targetPath,
+            'permanent' => $redirect->isPermanent(),
+        ], [], ['utf8' => $needsUtf8]);
+
+        $this->routeCollection->add('_redirect_route_' . $redirect->getId(), $route);
+    }
+
+    /**
+     * NEXT_MAJOR: Remove method/property
+     *
+     * @interal
+     */
+    public function enableImprovedRouter(bool $enabled): void
+    {
+        $this->enableImprovedRouter = $enabled;
+    }
 }

src/Kunstmaan/RedirectBundle/Tests/DependencyInjection/ConfigurationTest.php

@@ -21,8 +21,13 @@ protected function getConfiguration()
 
     public function testProcessedValueContainsRequiredValue()
     {
-        $array = [];
+        $array = [
+            'enable_improved_router' => true,
+        ];
 
-        $this->assertProcessedConfigurationEquals([$array], ['redirect_entity' => Redirect::class]);
+        $this->assertProcessedConfigurationEquals([$array], [
+            'redirect_entity' => Redirect::class,
+            'enable_improved_router' => true,
+        ]);
     }
 }

src/Kunstmaan/RedirectBundle/Tests/DependencyInjection/KunstmaanRedirectExtensionTest.php

@@ -22,15 +22,28 @@ protected function getContainerExtensions(): array
 
     public function testRedirectClassParameterWithConfigValue()
     {
-        $this->load(['redirect_entity' => 'RedirectTestEntity']);
+        $this->load([
+            'redirect_entity' => 'RedirectTestEntity',
+            'enable_improved_router' => true,
+        ]);
 
         $this->assertContainerBuilderHasParameter('kunstmaan_redirect.redirect.class', 'RedirectTestEntity');
     }
 
     public function testDefaultRedirectClassParameter()
     {
-        $this->load();
+        $this->load(['enable_improved_router' => true]);
 
         $this->assertContainerBuilderHasParameter('kunstmaan_redirect.redirect.class', Redirect::class);
     }
+
+    /**
+     * @group legacy
+     */
+    public function testImprovedRouterConfigDeprecation()
+    {
+        $this->expectDeprecation('Since kunstmaan/redirect-bundle 6.3: Not setting the "kunstmaan_redirect.enable_improved_router" config to true is deprecated, it will always be true in 7.0.');
+
+        $this->load();
+    }
 }