Merge pull request #77 from neos/task/adjustToOverhauledNodeUriBuilding

TASK: Adjust to introduction of Shared\NodeAddress and new NodeUriBuilder

.github/workflows/tests.yml

@@ -20,7 +20,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php-versions: ['8.2']
+        php-versions: ['8.2', '8.3']
 
     services:
       mariadb:
@@ -86,12 +86,18 @@ jobs:
         git -C ../${{ env.PACKAGE_FOLDER }} checkout -b build
         composer config repositories.package '{ "type": "path", "url": "../${{ env.PACKAGE_FOLDER }}", "options": { "symlink": false } }'
         composer require --no-update --no-interaction neos/redirecthandler-neosadapter:"dev-build as dev-${PACKAGE_TARGET_VERSION}"
+        composer require --dev --no-update --no-interaction phpstan/phpstan:^1.11
 
     - name: Composer Install
       run: |
         cd ${NEOS_BASE_FOLDER}
         composer update --no-interaction --no-progress
 
+    - name: Linting
+      run: |
+        cd ${NEOS_BASE_FOLDER}/Packages/Application/Neos.RedirectHandler.NeosAdapter
+        composer run lint:phpstan
+
     - name: Setup Flow configuration
       run: |
         cd ${NEOS_BASE_FOLDER}

Classes/CatchUpHook/DocumentUriPathProjectionHook.php

@@ -5,6 +5,7 @@
 use Neos\ContentRepository\Core\Projection\CatchUpHookInterface;
 use Neos\ContentRepository\Core\ContentRepository;
 use Neos\ContentRepository\Core\EventStore\EventInterface;
+use Neos\ContentRepository\Core\SharedModel\Node\NodeAddress;
 use Neos\EventStore\Model\EventEnvelope;
 use Neos\ContentRepository\Core\Feature\NodeModification\Event\NodePropertiesWereSet;
 use Neos\ContentRepository\Core\Feature\NodeMove\Event\NodeAggregateWasMoved;
@@ -14,10 +15,7 @@
 use Neos\Neos\FrontendRouting\Projection\DocumentUriPathFinder;
 use Neos\Neos\FrontendRouting\Projection\DocumentNodeInfo;
 use Neos\Neos\FrontendRouting\Exception\NodeNotFoundException;
-use Neos\Neos\FrontendRouting\NodeAddress;
-use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint;
 use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
-use Neos\Neos\FrontendRouting\NodeAddressFactory;
 
 final class DocumentUriPathProjectionHook implements CatchUpHookInterface
 {
@@ -85,8 +83,7 @@ private function onBeforeNodeAggregateWasRemoved(NodeAggregateWasRemoved $event)
 
             $this->nodeRedirectService->appendAffectedNode(
                 $node,
-                $this->getNodeAddress($event->contentStreamId, $dimensionSpacePoint, $node->getNodeAggregateId()),
-                $this->contentRepository->id
+                NodeAddress::create($this->contentRepository->id, $event->workspaceName, $dimensionSpacePoint, $node->getNodeAggregateId())
             );
             $this->documentNodeInfosBeforeRemoval[$dimensionSpacePoint->hash][] = $node;
 
@@ -95,8 +92,7 @@ private function onBeforeNodeAggregateWasRemoved(NodeAggregateWasRemoved $event)
                 function ($descendantOfNode) use ($event, $dimensionSpacePoint) {
                     $this->nodeRedirectService->appendAffectedNode(
                         $descendantOfNode,
-                        $this->getNodeAddress($event->contentStreamId, $dimensionSpacePoint, $descendantOfNode->getNodeAggregateId()),
-                        $this->contentRepository->id
+                        NodeAddress::create($this->contentRepository->id, $event->workspaceName, $dimensionSpacePoint, $descendantOfNode->getNodeAggregateId())
                     );
                     $this->documentNodeInfosBeforeRemoval[$dimensionSpacePoint->hash][] = $descendantOfNode;
                 },
@@ -144,6 +140,9 @@ private function onAfterNodePropertiesWereSet(NodePropertiesWereSet $event): voi
         );
     }
 
+    /**
+     * @param \Closure(DocumentNodeInfo $nodeInfo, NodeAddress $nodeAddress):void $closure
+     */
     private function handleNodePropertiesWereSet(NodePropertiesWereSet $event, \Closure $closure): void
     {
         if (!$this->isLiveContentStream($event->contentStreamId)) {
@@ -162,13 +161,12 @@ private function handleNodePropertiesWereSet(NodePropertiesWereSet $event, \Clos
                 continue;
             }
 
-            $closure($node, $this->getNodeAddress($event->contentStreamId, $affectedDimensionSpacePoint, $node->getNodeAggregateId()), $this->contentRepository->id);
+            $closure($node, NodeAddress::create($this->contentRepository->id, $event->workspaceName, $affectedDimensionSpacePoint, $node->getNodeAggregateId()));
 
             $descendantsOfNode = $this->getState()->getDescendantsOfNode($node);
             array_map(fn (DocumentNodeInfo $descendantOfNode) => $closure(
                 $descendantOfNode,
-                $this->getNodeAddress($event->contentStreamId, $affectedDimensionSpacePoint, $descendantOfNode->getNodeAggregateId()),
-                $this->contentRepository->id
+                NodeAddress::create($this->contentRepository->id, $event->workspaceName, $affectedDimensionSpacePoint, $descendantOfNode->getNodeAggregateId())
             ), iterator_to_array($descendantsOfNode));
         }
     }
@@ -189,6 +187,9 @@ private function onAfterNodeAggregateWasMoved(NodeAggregateWasMoved $event): voi
         );
     }
 
+    /**
+     * @param \Closure(DocumentNodeInfo $nodeInfo, NodeAddress $nodeAddress):void $closure
+     */
     private function handleNodeWasMoved(NodeAggregateWasMoved $event, \Closure $closure): void
     {
         if (!$this->isLiveContentStream($event->contentStreamId)) {
@@ -202,13 +203,12 @@ private function handleNodeWasMoved(NodeAggregateWasMoved $event, \Closure $clos
                 continue;
             }
 
-            $closure($node, $this->getNodeAddress($event->contentStreamId, $interdimensionalSibling->dimensionSpacePoint, $node->getNodeAggregateId()), $this->contentRepository->id);
+            $closure($node, NodeAddress::create($this->contentRepository->id, $event->workspaceName, $interdimensionalSibling->dimensionSpacePoint, $node->getNodeAggregateId()));
 
             $descendantsOfNode = $this->getState()->getDescendantsOfNode($node);
             array_map(fn (DocumentNodeInfo $descendantOfNode) => $closure(
                 $descendantOfNode,
-                $this->getNodeAddress($event->contentStreamId, $interdimensionalSibling->dimensionSpacePoint, $descendantOfNode->getNodeAggregateId()),
-                $this->contentRepository->id
+                NodeAddress::create($this->contentRepository->id, $event->workspaceName, $interdimensionalSibling->dimensionSpacePoint, $descendantOfNode->getNodeAggregateId())
             ), iterator_to_array($descendantsOfNode));
         }
     }
@@ -231,16 +231,4 @@ private function findNodeByIdAndDimensionSpacePointHash(NodeAggregateId $nodeAgg
             return null;
         }
     }
-
-    protected function getNodeAddress(
-        ContentStreamId $contentStreamId,
-        DimensionSpacePoint $dimensionSpacePoint,
-        NodeAggregateId $nodeAggregateId,
-    ): NodeAddress {
-        return NodeAddressFactory::create($this->contentRepository)->createFromContentStreamIdAndDimensionSpacePointAndNodeAggregateId(
-            $contentStreamId,
-            $dimensionSpacePoint,
-            $nodeAggregateId
-        );
-    }
 }

Classes/Service/NodeRedirectService.php

@@ -13,12 +13,15 @@
  * source code.
  */
 
+use Neos\ContentRepository\Core\SharedModel\Node\NodeAddress;
 use Neos\Flow\Annotations as Flow;
 use Neos\Flow\Persistence\PersistenceManagerInterface;
 use Neos\Neos\Domain\Model\Domain;
 use Neos\Neos\Domain\Model\SiteNodeName;
 use Neos\Neos\Domain\Repository\SiteRepository;
+use Neos\Neos\FrontendRouting\NodeUriBuilderFactory;
 use Neos\RedirectHandler\Storage\RedirectStorageInterface;
+use Psr\Http\Message\UriInterface;
 use Psr\Log\LoggerInterface;
 use Neos\ContentRepository\Core\NodeType\NodeType;
 use Neos\Neos\FrontendRouting\NodeUriBuilder;
@@ -27,7 +30,6 @@
 use Neos\Neos\FrontendRouting\SiteDetection\SiteDetectionResult;
 use Neos\Flow\Mvc\ActionRequest;
 use Neos\Neos\FrontendRouting\Projection\DocumentNodeInfo;
-use Neos\Neos\FrontendRouting\NodeAddress;
 use Neos\ContentRepository\Core\NodeType\NodeTypeName;
 use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry;
 use GuzzleHttp\Psr7\Uri;
@@ -45,7 +47,13 @@ final class NodeRedirectService
     const STATUS_CODE_TYPE_REDIRECT = 'redirect';
     const STATUS_CODE_TYPE_GONE = 'gone';
 
+    /**
+     * @var array<string,array{node:DocumentNodeInfo,url:UriInterface}|null>
+     */
     private array $affectedNodes = [];
+    /**
+     * @var array<string,list<string>>
+     */
     private array $hostnamesRuntimeCache = [];
 
     #[Flow\Inject]
@@ -76,57 +84,52 @@ final class NodeRedirectService
     protected array $restrictByNodeType = [];
 
     public function __construct(
-        protected RedirectStorageInterface $redirectStorage,
-        protected PersistenceManagerInterface $persistenceManager,
-        protected ContentRepositoryRegistry $contentRepositoryRegistry,
-        protected SiteRepository $siteRepository,
+        private readonly RedirectStorageInterface $redirectStorage,
+        private readonly PersistenceManagerInterface $persistenceManager,
+        private readonly ContentRepositoryRegistry $contentRepositoryRegistry,
+        private readonly SiteRepository $siteRepository,
+        private readonly NodeUriBuilderFactory $nodeUriBuilderFactory
     ) {
     }
 
     /**
      * Collects affected nodes before they got moved or removed.
-     *
-     * @throws \Neos\Flow\Http\Exception
-     * @throws \Neos\Flow\Mvc\Routing\Exception\MissingActionNameException
      */
-    public function appendAffectedNode(DocumentNodeInfo $nodeInfo, NodeAddress $nodeAddress, ContentRepositoryId $contentRepositoryId): void
+    public function appendAffectedNode(DocumentNodeInfo $nodeInfo, NodeAddress $nodeAddress): void
     {
         try {
-            $this->affectedNodes[$this->createAffectedNodesKey($nodeInfo, $contentRepositoryId)] = [
+            $this->affectedNodes[$this->createAffectedNodesKey($nodeInfo, $nodeAddress->contentRepositoryId)] = [
                 'node' => $nodeInfo,
-                'url' => $this->getNodeUriBuilder($nodeInfo->getSiteNodeName(), $contentRepositoryId)->uriFor($nodeAddress),
+                'url' => $this->getNodeUriBuilder($nodeInfo->getSiteNodeName(), $nodeAddress->contentRepositoryId)->uriFor($nodeAddress),
             ];
         } catch (NoMatchingRouteException $exception) {
         }
     }
 
     /**
      * Creates redirects for given node and uses the collected affected nodes to determine the source of the new redirect target.
-     *
-     * @throws \Neos\Flow\Http\Exception
-     * @throws \Neos\Flow\Mvc\Routing\Exception\MissingActionNameException
      */
-    public function createRedirectForAffectedNode(DocumentNodeInfo $nodeInfo, NodeAddress $nodeAddress, ContentRepositoryId $contentRepositoryId): void
+    public function createRedirectForAffectedNode(DocumentNodeInfo $nodeInfo, NodeAddress $nodeAddress): void
     {
         if (!$this->enableAutomaticRedirects) {
             return;
         }
 
-        $affectedNode = $this->affectedNodes[$this->createAffectedNodesKey($nodeInfo, $contentRepositoryId)] ?? null;
+        $affectedNode = $this->affectedNodes[$this->createAffectedNodesKey($nodeInfo, $nodeAddress->contentRepositoryId)] ?? null;
         if ($affectedNode === null) {
             return;
         }
-        unset($this->affectedNodes[$this->createAffectedNodesKey($nodeInfo, $contentRepositoryId)]);
+        unset($this->affectedNodes[$this->createAffectedNodesKey($nodeInfo, $nodeAddress->contentRepositoryId)]);
 
         /** @var Uri $oldUri */
         $oldUri = $affectedNode['url'];
-        $nodeType = $this->getNodeType($contentRepositoryId, $nodeInfo->getNodeTypeName());
+        $nodeType = $this->getNodeType($nodeAddress->contentRepositoryId, $nodeInfo->getNodeTypeName());
 
-        if ($this->isRestrictedByNodeType($nodeType) || $this->isRestrictedByOldUri($oldUri->getPath())) {
+        if (!$nodeType || $this->isRestrictedByNodeType($nodeType) || $this->isRestrictedByOldUri($oldUri->getPath())) {
             return;
         }
         try {
-            $newUri = $this->getNodeUriBuilder($nodeInfo->getSiteNodeName(), $contentRepositoryId)->uriFor($nodeAddress);
+            $newUri = $this->getNodeUriBuilder($nodeInfo->getSiteNodeName(), $nodeAddress->contentRepositoryId)->uriFor($nodeAddress);
         } catch (NoMatchingRouteException $exception) {
             // We can't build an uri for given node, so we can't create any redirect. E.g.: Node is disabled.
             return;
@@ -155,7 +158,7 @@ public function createRedirectForRemovedAffectedNode(DocumentNodeInfo $nodeInfo,
         $oldUri = $affectedNode['url'];
         $nodeType = $this->getNodeType($contentRepositoryId, $nodeInfo->getNodeTypeName());
 
-        if ($this->isRestrictedByNodeType($nodeType) || $this->isRestrictedByOldUri($oldUri->getPath())) {
+        if (!$nodeType || $this->isRestrictedByNodeType($nodeType) || $this->isRestrictedByOldUri($oldUri->getPath())) {
             return;
         }
 
@@ -164,7 +167,7 @@ public function createRedirectForRemovedAffectedNode(DocumentNodeInfo $nodeInfo,
         $this->persistenceManager->persistAll();
     }
 
-    protected function getNodeType(ContentRepositoryId $contentRepositoryId, NodeTypeName $nodeTypeName): NodeType
+    protected function getNodeType(ContentRepositoryId $contentRepositoryId, NodeTypeName $nodeTypeName): ?NodeType
     {
         return $this->contentRepositoryRegistry->get($contentRepositoryId)->getNodeTypeManager()->getNodeType($nodeTypeName);
     }
@@ -179,15 +182,12 @@ protected function getNodeUriBuilder(SiteNodeName $siteNodeName, ContentReposito
         // Generate a custom request when the current request was triggered from CLI
         $baseUri = 'http://localhost';
 
-        // Prevent `index.php` appearing in generated redirects
-        putenv('FLOW_REWRITEURLS=1');
-
         $httpRequest = new ServerRequest('POST', $baseUri);
 
         $httpRequest = (SiteDetectionResult::create($siteNodeName, $contentRepositoryId))->storeInRequest($httpRequest);
         $actionRequest = ActionRequest::fromHttpRequest($httpRequest);
 
-        return NodeUriBuilder::fromRequest($actionRequest);
+        return $this->nodeUriBuilderFactory->forActionRequest($actionRequest);
     }
 
     /**
@@ -283,7 +283,7 @@ protected function isRestrictedByOldUri(string $oldUriPath): bool
 
     /**
      * Collects all hostnames from the Domain entries attached to the current site.
-     * @return array<string, array<string>>
+     * @return list<string>
      */
     protected function getHostnames(SiteNodeName $siteNodeName): array
     {

composer.json

@@ -10,6 +10,7 @@
         "neos/flow": "^9.0"
     },
     "scripts": {
+        "lint:phpstan": "../../../bin/phpstan analyse",
         "test:behat-cli": "../../../bin/behat -f progress --strict --no-interaction",
         "test:behavioral:stop-on-failure": "@test:behat-cli -c Tests/Behavior/behat.yml.dist -vvv --stop-on-failure"
     },

phpstan.neon.dist

@@ -0,0 +1,4 @@
+parameters:
+    level: 8
+    paths:
+        - Classes