Merge pull request #3 from Flowpack/1-links-in-texts-are-rendered-without-trailing-slash

TASK: render links to nodes with trailing slash

Author: t-heuser

Classes/Helper/BlocklistHelper.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\SeoRouting\Helper;
+
+use Neos\Flow\Annotations as Flow;
+use Psr\Http\Message\UriInterface;
+
+#[Flow\Scope('singleton')]
+class BlocklistHelper
+{
+    #[Flow\Inject]
+    protected ConfigurationHelper $configurationHelper;
+
+    public function isUriInBlocklist(UriInterface $uri): bool
+    {
+        $path = $uri->getPath();
+        foreach ($this->configurationHelper->getBlocklist() as $rawPattern => $active) {
+            $pattern = '/' . str_replace('/', '\/', $rawPattern) . '/';
+
+            if (! $active) {
+                continue;
+            }
+
+            if (preg_match($pattern, $path) === 1) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

Classes/Helper/ConfigurationHelper.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\SeoRouting\Helper;
+
+use Neos\Flow\Annotations as Flow;
+
+#[Flow\Scope('singleton')]
+class ConfigurationHelper
+{
+    /** @var array{enable: array{trailingSlash: bool, toLowerCase: bool}, statusCode?: int} */
+    #[Flow\InjectConfiguration(path: 'redirect')]
+    protected array $configuration;
+
+    /** @var array{string: bool} */
+    #[Flow\InjectConfiguration(path: 'blocklist')]
+    protected array $blocklist;
+
+    public function isTrailingSlashEnabled(): bool
+    {
+        return $this->configuration['enable']['trailingSlash'] ?? false;
+    }
+
+    public function isToLowerCaseEnabled(): bool
+    {
+        return $this->configuration['enable']['toLowerCase'] ?? false;
+    }
+
+    public function getStatusCode(): int
+    {
+        return $this->configuration['statusCode'] ?? 301;
+    }
+
+    /**
+     * @return array{string: bool}
+     */
+    public function getBlocklist(): array
+    {
+        return $this->blocklist;
+    }
+}

Classes/Helper/LowerCaseHelper.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\SeoRouting\Helper;
+
+use Neos\Flow\Annotations\Scope;
+use Psr\Http\Message\UriInterface;
+
+#[Scope('singleton')]
+class LowerCaseHelper
+{
+    public function convertPathToLowerCase(UriInterface $uri): UriInterface
+    {
+        $loweredPath = strtolower($uri->getPath());
+
+        if ($uri->getPath() === $loweredPath) {
+            return $uri;
+        }
+
+        // bypass links to files
+        if (array_key_exists('extension', pathinfo($uri->getPath()))) {
+            return $uri;
+        }
+
+        return $uri->withPath($loweredPath);
+    }
+}

Classes/Helper/TrailingSlashHelper.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\SeoRouting\Helper;
+
+use Neos\Flow\Annotations\Scope;
+use Psr\Http\Message\UriInterface;
+
+#[Scope('singleton')]
+class TrailingSlashHelper
+{
+    public function appendTrailingSlash(UriInterface $uri): UriInterface
+    {
+        // bypass links without path
+        if (strlen($uri->getPath()) === 0) {
+            return $uri;
+        }
+
+        // bypass links to files
+        if (array_key_exists('extension', pathinfo($uri->getPath()))) {
+            return $uri;
+        }
+
+        // bypass mailto and tel links
+        if (in_array($uri->getScheme(), ['mailto', 'tel'], true)) {
+            return $uri;
+        }
+
+        return $uri->withPath(rtrim($uri->getPath(), '/') . '/');
+    }
+}

Classes/LinkingServiceAspect.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\SeoRouting;
+
+use Flowpack\SeoRouting\Helper\BlocklistHelper;
+use Flowpack\SeoRouting\Helper\ConfigurationHelper;
+use Flowpack\SeoRouting\Helper\TrailingSlashHelper;
+use GuzzleHttp\Psr7\Exception\MalformedUriException;
+use GuzzleHttp\Psr7\Uri;
+use Neos\Flow\Annotations as Flow;
+use Neos\Flow\Aop\JoinPointInterface;
+use Neos\Neos\Service\LinkingService;
+
+#[Flow\Aspect]
+class LinkingServiceAspect
+{
+    #[Flow\Inject]
+    protected TrailingSlashHelper $trailingSlashHelper;
+
+    #[Flow\Inject]
+    protected ConfigurationHelper $configurationHelper;
+
+    #[Flow\Inject]
+    protected BlocklistHelper $blocklistHelper;
+
+    /**
+     * This ensures that all internal links are rendered with a trailing slash.
+     */
+    #[Flow\Around('method(' . LinkingService::class . '->createNodeUri())')]
+    public function appendTrailingSlashToNodeUri(JoinPointInterface $joinPoint): string
+    {
+        /** @var string $result */
+        $result = $joinPoint->getAdviceChain()->proceed($joinPoint);
+
+        if (! $this->configurationHelper->isTrailingSlashEnabled()) {
+            return $result;
+        }
+
+        try {
+            $uri = new Uri($result);
+        } catch (MalformedUriException) {
+            return $result;
+        }
+
+        if ($this->blocklistHelper->isUriInBlocklist($uri)) {
+            return $result;
+        }
+
+        return (string)$this->trailingSlashHelper->appendTrailingSlash($uri);
+    }
+}

Classes/RoutingMiddleware.php

@@ -4,108 +4,65 @@
 
 namespace Flowpack\SeoRouting;
 
+use Flowpack\SeoRouting\Helper\BlocklistHelper;
+use Flowpack\SeoRouting\Helper\ConfigurationHelper;
+use Flowpack\SeoRouting\Helper\LowerCaseHelper;
+use Flowpack\SeoRouting\Helper\TrailingSlashHelper;
 use Neos\Flow\Annotations as Flow;
 use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
-use Psr\Http\Message\UriFactoryInterface;
-use Psr\Http\Message\UriInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 
-final class RoutingMiddleware implements MiddlewareInterface
+class RoutingMiddleware implements MiddlewareInterface
 {
     #[Flow\Inject]
     protected ResponseFactoryInterface $responseFactory;
 
     #[Flow\Inject]
-    protected UriFactoryInterface $uriFactory;
+    protected ConfigurationHelper $configurationHelper;
 
-    /** @var array{enable: array{trailingSlash: bool, toLowerCase: bool}, statusCode?: int} */
-    #[Flow\InjectConfiguration(path: 'redirect')]
-    protected array $configuration;
+    #[Flow\Inject]
+    protected BlocklistHelper $blocklistHelper;
+
+    #[Flow\Inject]
+    protected TrailingSlashHelper $trailingSlashHelper;
 
-    /** @var array{string: bool} */
-    #[Flow\InjectConfiguration(path: 'blocklist')]
-    protected array $blocklist;
+    #[Flow\Inject]
+    protected LowerCaseHelper $lowerCaseHelper;
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
-        $isTrailingSlashEnabled = $this->configuration['enable']['trailingSlash'] ?? false;
-        $isToLowerCaseEnabled = $this->configuration['enable']['toLowerCase'] ?? false;
+        $isTrailingSlashEnabled = $this->configurationHelper->isTrailingSlashEnabled();
+        $isToLowerCaseEnabled = $this->configurationHelper->isToLowerCaseEnabled();
 
         $uri = $request->getUri();
 
         if (! $isTrailingSlashEnabled && ! $isToLowerCaseEnabled) {
             return $handler->handle($request);
         }
 
-        if ($this->matchesBlocklist($uri)) {
+        if ($this->blocklistHelper->isUriInBlocklist($uri)) {
             return $handler->handle($request);
         }
 
         $oldPath = $uri->getPath();
 
         if ($isTrailingSlashEnabled) {
-            $uri = $this->handleTrailingSlash($uri);
+            $uri = $this->trailingSlashHelper->appendTrailingSlash($uri);
         }
 
         if ($isToLowerCaseEnabled) {
-            $uri = $this->handleToLowerCase($uri);
+            $uri = $this->lowerCaseHelper->convertPathToLowerCase($uri);
         }
 
         if ($uri->getPath() === $oldPath) {
             return $handler->handle($request);
         }
 
-        $response = $this->responseFactory->createResponse($this->configuration['statusCode'] ?? 301);
+        $response = $this->responseFactory->createResponse($this->configurationHelper->getStatusCode());
 
         return $response->withAddedHeader('Location', (string)$uri);
     }
-
-    private function handleTrailingSlash(UriInterface $uri): UriInterface
-    {
-        if (strlen($uri->getPath()) === 0) {
-            return $uri;
-        }
-
-        if (array_key_exists('extension', pathinfo($uri->getPath()))) {
-            return $uri;
-        }
-
-        return $uri->withPath(rtrim($uri->getPath(), '/') . '/')
-            ->withQuery($uri->getQuery())
-            ->withFragment($uri->getFragment());
-    }
-
-    private function handleToLowerCase(UriInterface $uri): UriInterface
-    {
-        $loweredPath = strtolower($uri->getPath());
-
-        if ($uri->getPath() === $loweredPath) {
-            return $uri;
-        }
-
-        $newUri = str_replace($uri->getPath(), $loweredPath, (string)$uri);
-
-        return $this->uriFactory->createUri($newUri);
-    }
-
-    private function matchesBlocklist(UriInterface $uri): bool
-    {
-        $path = $uri->getPath();
-        foreach ($this->blocklist as $rawPattern => $active) {
-            $pattern = '/' . str_replace('/', '\/', $rawPattern) . '/';
-
-            if (! $active) {
-                continue;
-            }
-
-            if (preg_match($pattern, $path) === 1) {
-                return true;
-            }
-        }
-
-        return false;
-    }
 }

README.md

@@ -26,14 +26,15 @@ This package allows you to enforce a trailing slash and/or lower case urls in Fl
 
 This package has 2 main features:
 
-- **trailingSlash**: ensure that all links ends with a trailing slash (e.g. `example.com/test/` instead of
-  `example.com/test`)
+- **trailingSlash**: ensure that all rendered internal links in the frontend end with a trailing slash (e.g. `example.
+com/test/` instead of `example.com/test`) and all called URLs without trailing slash will be redirected to the same
+  page with a trailing slash
 - **toLowerCase**: ensure that camelCase links gets redirected to lowercase (e.g. `example.com/lowercase` instead of
   `example.com/lowerCase`)
 
 You can de- and activate both of them.
 
-Another small feature is to restrict all _new_ neos pages to have a lowercased `uriPathSegment`. This is done by
+Another small feature is to restrict all _new_ Neos pages to have a lowercased `uriPathSegment`. This is done by
 extending the `NodeTypes.Document.yaml`.
 
 ## Installation
@@ -50,8 +51,8 @@ In the standard configuration we have activated the trailingSlash (to redirect a
 with / at the end) and do all redirects with a 301 http status.
 
 *Note: The lowercase redirect is deactivated by default, because you have to make sure, that there is
-no `uriPathSegment`
-with camelCase or upperspace letters - this would lead to redirects in the neverland.*
+no Neos page with an `uriPathSegment` with camelCase or upperspace letters - this would lead to redirects in the
+neverland.*
 
 ```
 Flowpack:

Tests/Unit/Helper/BlocklistHelperTest.php

@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\SeoRouting\Tests\Unit\Helper;
+
+use Flowpack\SeoRouting\Helper\BlocklistHelper;
+use Flowpack\SeoRouting\Helper\ConfigurationHelper;
+use GuzzleHttp\Psr7\Uri;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+
+#[CoversClass(BlocklistHelper::class)]
+class BlocklistHelperTest extends TestCase
+{
+    #[DataProvider('urlDataProvider')]
+    public function testIsUriInBlocklist(string $input, bool $expected): void
+    {
+        $blocklistHelper = new BlocklistHelper();
+        $configurationHelperMock = $this->createMock(ConfigurationHelper::class);
+
+        $configurationHelperMock->expects($this->once())->method('getBlocklist')->willReturn(
+            ['/neos.*' => false, '.*test.*' => true]
+        );
+
+        $reflection = new ReflectionClass($blocklistHelper);
+        $property = $reflection->getProperty('configurationHelper');
+        $property->setValue($blocklistHelper, $configurationHelperMock);
+
+        $uri = new Uri($input);
+
+        self::assertSame($expected, $blocklistHelper->isUriInBlocklist($uri));
+    }
+
+    /**
+     * @return array{array{string, bool}}
+     */
+    public static function urlDataProvider(): array
+    {
+        return [
+            ['https://test.de/neos', false],
+            ['https://test.de/neos/test', true],
+            ['https://neos.de/foo', false],
+        ];
+    }
+}