From 02b0d1b6048e5be2d698a60a3c754a3d2a36cfd4 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 18 May 2024 10:58:10 +0200 Subject: [PATCH 1/5] TASK: Fix nodeType related phpstan 8 errors --- Classes/Domain/Model/Changes/AbstractStructuralChange.php | 6 +++--- Classes/Domain/Model/Changes/Create.php | 2 +- Classes/Domain/Model/Changes/CreateAfter.php | 2 +- Classes/Domain/Model/Changes/CreateBefore.php | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Classes/Domain/Model/Changes/AbstractStructuralChange.php b/Classes/Domain/Model/Changes/AbstractStructuralChange.php index 111ffc96b1..f7a95dfae5 100644 --- a/Classes/Domain/Model/Changes/AbstractStructuralChange.php +++ b/Classes/Domain/Model/Changes/AbstractStructuralChange.php @@ -173,14 +173,14 @@ protected function finish(Node $node) } } - protected function findChildNodes(Node $node): Nodes + final protected function findChildNodes(Node $node): Nodes { // TODO REMOVE return $this->contentRepositoryRegistry->subgraphForNode($node) ->findChildNodes($node->aggregateId, FindChildNodesFilter::create()); } - protected function isNodeTypeAllowedAsChildNode(Node $parentNode, NodeTypeName $nodeTypeNameToCheck): bool + final protected function isNodeTypeAllowedAsChildNode(Node $parentNode, NodeTypeName $nodeTypeNameToCheck): bool { $nodeTypeManager = $this->contentRepositoryRegistry->get($parentNode->contentRepositoryId)->getNodeTypeManager(); @@ -196,7 +196,7 @@ protected function isNodeTypeAllowedAsChildNode(Node $parentNode, NodeTypeName $ } return $parentNodeType->allowsChildNodeType($nodeTypeToCheck); } - + assert($parentNode->name !== null); // were tethered $subgraph = $this->contentRepositoryRegistry->subgraphForNode($parentNode); $grandParentNode = $subgraph->findParentNode($parentNode->aggregateId); diff --git a/Classes/Domain/Model/Changes/Create.php b/Classes/Domain/Model/Changes/Create.php index 062f47dd45..18e571dd0e 100644 --- a/Classes/Domain/Model/Changes/Create.php +++ b/Classes/Domain/Model/Changes/Create.php @@ -40,7 +40,7 @@ public function canApply(): bool $subject = $this->getSubject(); $nodeTypeName = $this->getNodeTypeName(); - return $this->isNodeTypeAllowedAsChildNode($subject, $nodeTypeName); + return $subject && $nodeTypeName && $this->isNodeTypeAllowedAsChildNode($subject, $nodeTypeName); } /** diff --git a/Classes/Domain/Model/Changes/CreateAfter.php b/Classes/Domain/Model/Changes/CreateAfter.php index 686d69abfb..3e290c6e0a 100644 --- a/Classes/Domain/Model/Changes/CreateAfter.php +++ b/Classes/Domain/Model/Changes/CreateAfter.php @@ -38,7 +38,7 @@ public function canApply(): bool $parent = $this->findParentNode($this->subject); $nodeTypeName = $this->getNodeTypeName(); - return $parent && $this->isNodeTypeAllowedAsChildNode($parent, $nodeTypeName); + return $parent && $nodeTypeName && $this->isNodeTypeAllowedAsChildNode($parent, $nodeTypeName); } /** diff --git a/Classes/Domain/Model/Changes/CreateBefore.php b/Classes/Domain/Model/Changes/CreateBefore.php index 958d471691..ffaacce7e9 100644 --- a/Classes/Domain/Model/Changes/CreateBefore.php +++ b/Classes/Domain/Model/Changes/CreateBefore.php @@ -38,7 +38,7 @@ public function canApply(): bool $parent = $this->findParentNode($this->subject); $nodeTypeName = $this->getNodeTypeName(); - return $parent && $this->isNodeTypeAllowedAsChildNode($parent, $nodeTypeName); + return $parent && $nodeTypeName && $this->isNodeTypeAllowedAsChildNode($parent, $nodeTypeName); } /** From 0ca70b884cd6f9f8397f96eb6b107e9e5c07c756 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 18 May 2024 11:15:57 +0200 Subject: [PATCH 2/5] TASK: Fix some phpstan 8 errors --- .../Application/SyncWorkspace/ConflictsBuilder.php | 12 ++++++------ Classes/Domain/Model/Changes/CopyAfter.php | 9 ++++++--- Classes/Domain/Model/Changes/CopyBefore.php | 9 ++++++--- Classes/Domain/Model/Changes/CopyInto.php | 11 +++++++---- Classes/Domain/Model/Changes/MoveInto.php | 2 +- Classes/Domain/Model/RenderedNodeDomAddress.php | 1 + 6 files changed, 27 insertions(+), 17 deletions(-) diff --git a/Classes/Application/SyncWorkspace/ConflictsBuilder.php b/Classes/Application/SyncWorkspace/ConflictsBuilder.php index 12fbc046a8..3ebb3710e4 100644 --- a/Classes/Application/SyncWorkspace/ConflictsBuilder.php +++ b/Classes/Application/SyncWorkspace/ConflictsBuilder.php @@ -109,13 +109,13 @@ private function createConflictFromCommandThatFailedDuringRebase( $nodeAggregateId ); $affectedSite = $nodeAggregateId - ? $subgraph->findClosestNode( + ? $subgraph?->findClosestNode( $nodeAggregateId, FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_SITE) ) : null; $affectedDocument = $nodeAggregateId - ? $subgraph->findClosestNode( + ? $subgraph?->findClosestNode( $nodeAggregateId, FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT) ) @@ -214,13 +214,13 @@ private function acquireSubgraphFromCommand( $dimensionSpacePoint = $this->extractValidDimensionSpacePointFromNodeAggregate( $nodeAggregate ); - - if ($dimensionSpacePoint === null) { - return null; - } } } + if ($dimensionSpacePoint === null) { + return null; + } + return $contentGraph->getSubgraph( $dimensionSpacePoint, VisibilityConstraints::withoutRestrictions() diff --git a/Classes/Domain/Model/Changes/CopyAfter.php b/Classes/Domain/Model/Changes/CopyAfter.php index 16b1eadc9b..6f41906f1d 100644 --- a/Classes/Domain/Model/Changes/CopyAfter.php +++ b/Classes/Domain/Model/Changes/CopyAfter.php @@ -81,10 +81,13 @@ public function apply(): void $contentRepository->handle($command); + $newlyCreatedNodeId = $command->nodeAggregateIdMapping->getNewNodeAggregateId($subject->aggregateId); + assert($newlyCreatedNodeId !== null); // cannot happen $newlyCreatedNode = $this->contentRepositoryRegistry->subgraphForNode($parentNodeOfPreviousSibling) - ->findNodeById( - $command->nodeAggregateIdMapping->getNewNodeAggregateId($subject->aggregateId) - ); + ->findNodeById($newlyCreatedNodeId); + if (!$newlyCreatedNode) { + throw new \RuntimeException(sprintf('Node %s was not found after copy.', $newlyCreatedNodeId->value), 1716023308); + } $this->finish($newlyCreatedNode); // NOTE: we need to run "finish" before "addNodeCreatedFeedback" // to ensure the new node already exists when the last feedback is processed diff --git a/Classes/Domain/Model/Changes/CopyBefore.php b/Classes/Domain/Model/Changes/CopyBefore.php index 3353eaedc2..aeb01f567e 100644 --- a/Classes/Domain/Model/Changes/CopyBefore.php +++ b/Classes/Domain/Model/Changes/CopyBefore.php @@ -76,10 +76,13 @@ public function apply(): void $contentRepository->handle($command); + $newlyCreatedNodeId = $command->nodeAggregateIdMapping->getNewNodeAggregateId($subject->aggregateId); + assert($newlyCreatedNodeId !== null); // cannot happen $newlyCreatedNode = $this->contentRepositoryRegistry->subgraphForNode($parentNodeOfSucceedingSibling) - ->findNodeById( - $command->nodeAggregateIdMapping->getNewNodeAggregateId($subject->aggregateId) - ); + ->findNodeById($newlyCreatedNodeId); + if (!$newlyCreatedNode) { + throw new \RuntimeException(sprintf('Node %s was not found after copy.', $newlyCreatedNodeId->value), 1716023308); + } $this->finish($newlyCreatedNode); // NOTE: we need to run "finish" before "addNodeCreatedFeedback" // to ensure the new node already exists when the last feedback is processed diff --git a/Classes/Domain/Model/Changes/CopyInto.php b/Classes/Domain/Model/Changes/CopyInto.php index 194a3ca0f4..e5d1a1b6af 100644 --- a/Classes/Domain/Model/Changes/CopyInto.php +++ b/Classes/Domain/Model/Changes/CopyInto.php @@ -51,7 +51,7 @@ public function canApply(): bool { $parentNode = $this->getParentNode(); - return $parentNode && $this->isNodeTypeAllowedAsChildNode($parentNode, $this->subject->nodeTypeName); + return $this->subject && $parentNode && $this->isNodeTypeAllowedAsChildNode($parentNode, $this->subject->nodeTypeName); } public function getMode(): string @@ -82,10 +82,13 @@ public function apply(): void ); $contentRepository->handle($command); + $newlyCreatedNodeId = $command->nodeAggregateIdMapping->getNewNodeAggregateId($subject->aggregateId); + assert($newlyCreatedNodeId !== null); // cannot happen $newlyCreatedNode = $this->contentRepositoryRegistry->subgraphForNode($parentNode) - ->findNodeById( - $command->nodeAggregateIdMapping->getNewNodeAggregateId($subject->aggregateId), - ); + ->findNodeById($newlyCreatedNodeId); + if (!$newlyCreatedNode) { + throw new \RuntimeException(sprintf('Node %s was not found after copy.', $newlyCreatedNodeId->value), 1716023308); + } $this->finish($newlyCreatedNode); // NOTE: we need to run "finish" before "addNodeCreatedFeedback" // to ensure the new node already exists when the last feedback is processed diff --git a/Classes/Domain/Model/Changes/MoveInto.php b/Classes/Domain/Model/Changes/MoveInto.php index a3b7fbaaf2..284cae7e98 100644 --- a/Classes/Domain/Model/Changes/MoveInto.php +++ b/Classes/Domain/Model/Changes/MoveInto.php @@ -33,7 +33,7 @@ public function setParentContextPath(string $parentContextPath): void public function getParentNode(): ?Node { - if ($this->parentContextPath === null) { + if ($this->parentContextPath === null || !$this->getSubject()) { return null; } diff --git a/Classes/Domain/Model/RenderedNodeDomAddress.php b/Classes/Domain/Model/RenderedNodeDomAddress.php index 977d39387b..b13e8fcd74 100644 --- a/Classes/Domain/Model/RenderedNodeDomAddress.php +++ b/Classes/Domain/Model/RenderedNodeDomAddress.php @@ -84,6 +84,7 @@ public function getFusionPath() public function getFusionPathForContentRendering(): string { $fusionPathForContentRendering = $this->getFusionPath(); + /** @var string $fusionPathForContentRendering */ $fusionPathForContentRendering = preg_replace( '/(\/itemRenderer)\/([^<>\/]+)\/element(<[^>]+>)$/', '$1', From cbe2e898b9da0457d7dad0384bec5f8c567a73f4 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 18 May 2024 11:22:22 +0200 Subject: [PATCH 3/5] TASK: Add check around `setControllerContext` --- .../Model/Feedback/Operations/ReloadContentOutOfBand.php | 5 ++++- .../Model/Feedback/Operations/RenderContentOutOfBand.php | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php b/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php index 2db18e40d0..81cd60e676 100644 --- a/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php +++ b/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php @@ -136,7 +136,10 @@ protected function renderContent(ControllerContext $controllerContext): string $renderingMode = $this->renderingModeService->findByCurrentUser(); $view = $this->outOfBandRenderingViewFactory->resolveView(); - $view->setControllerContext($controllerContext); + if (method_exists($view, 'setControllerContext')) { + // deprecated + $view->setControllerContext($controllerContext); + } $view->setOption('renderingModeName', $renderingMode->name); $view->assign('value', $this->node); diff --git a/Classes/Domain/Model/Feedback/Operations/RenderContentOutOfBand.php b/Classes/Domain/Model/Feedback/Operations/RenderContentOutOfBand.php index 8f0ec05c96..d270fb33c6 100644 --- a/Classes/Domain/Model/Feedback/Operations/RenderContentOutOfBand.php +++ b/Classes/Domain/Model/Feedback/Operations/RenderContentOutOfBand.php @@ -190,7 +190,10 @@ protected function renderContent(ControllerContext $controllerContext): string $renderingMode = $this->renderingModeService->findByCurrentUser(); $view = $this->outOfBandRenderingViewFactory->resolveView(); - $view->setControllerContext($controllerContext); + if (method_exists($view, 'setControllerContext')) { + // deprecated + $view->setControllerContext($controllerContext); + } $view->setOption('renderingModeName', $renderingMode->name); $view->assign('value', $parentNode); From 2b08575ad2905ba212e753b039ac72af8bee5808 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 18 May 2024 11:40:31 +0200 Subject: [PATCH 4/5] WIP: PHPSTAN lvl 8 --- Classes/Controller/BackendController.php | 1 + Classes/Controller/BackendServiceController.php | 1 + .../Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php | 2 +- Classes/Fusion/Helper/WorkspaceHelper.php | 1 + phpstan.neon.dist | 2 +- 5 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index b70111a398..a70a803b48 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -146,6 +146,7 @@ public function indexAction(string $node = null) } $currentAccount = $this->securityContext->getAccount(); + assert($currentAccount !== null); $workspaceName = WorkspaceNameBuilder::fromAccountIdentifier($currentAccount->getAccountIdentifier()); try { diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index 7e0a09420b..7f93705291 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -418,6 +418,7 @@ public function changeBaseWorkspaceAction(string $targetWorkspaceName, string $d $nodeAddressFactory = NodeAddressFactory::create($contentRepository); $currentAccount = $this->securityContext->getAccount(); + assert($currentAccount !== null); $userWorkspaceName = WorkspaceNameBuilder::fromAccountIdentifier( $currentAccount->getAccountIdentifier() ); diff --git a/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php b/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php index 81cd60e676..52374833ad 100644 --- a/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php +++ b/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php @@ -127,7 +127,7 @@ public function serializePayload(ControllerContext $controllerContext): array protected function renderContent(ControllerContext $controllerContext): string { if (!is_null($this->node)) { - $cacheTags = $this->cachingHelper->nodeTag($this->getNode()); + $cacheTags = $this->cachingHelper->nodeTag($this->node); foreach ($cacheTags as $tag) { $this->contentCache->flushByTag($tag); } diff --git a/Classes/Fusion/Helper/WorkspaceHelper.php b/Classes/Fusion/Helper/WorkspaceHelper.php index 797a8d78b9..c22e35837f 100644 --- a/Classes/Fusion/Helper/WorkspaceHelper.php +++ b/Classes/Fusion/Helper/WorkspaceHelper.php @@ -56,6 +56,7 @@ class WorkspaceHelper implements ProtectedContextAwareInterface public function getPersonalWorkspace(ContentRepositoryId $contentRepositoryId): array { $currentAccount = $this->securityContext->getAccount(); + assert($currentAccount !== null); // todo use \Neos\Neos\Service\UserService::getPersonalWorkspaceName instead? $personalWorkspaceName = WorkspaceNameBuilder::fromAccountIdentifier($currentAccount->getAccountIdentifier()); diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 90b4b0c2db..d0cf7bd02d 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,4 +1,4 @@ parameters: - level: 7 + level: 8 paths: - Classes From 716966726afc1d131890a2c52055ee46d0847848 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 24 Jun 2024 15:33:47 +0200 Subject: [PATCH 5/5] TASK: Declare `ChangeInterface::getSubject` as non nullable --- .../SyncWorkspace/SyncWorkspaceCommand.php | 2 +- .../Controller/BackendServiceController.php | 7 ++- Classes/Domain/Model/AbstractChange.php | 26 ++++------ Classes/Domain/Model/ChangeInterface.php | 2 +- .../Changes/AbstractStructuralChange.php | 2 +- Classes/Domain/Model/Changes/CopyAfter.php | 5 +- Classes/Domain/Model/Changes/CopyBefore.php | 5 +- Classes/Domain/Model/Changes/CopyInto.php | 4 +- Classes/Domain/Model/Changes/Create.php | 4 +- Classes/Domain/Model/Changes/CreateAfter.php | 7 +-- Classes/Domain/Model/Changes/CreateBefore.php | 7 +-- Classes/Domain/Model/Changes/MoveAfter.php | 6 +-- Classes/Domain/Model/Changes/MoveBefore.php | 7 +-- Classes/Domain/Model/Changes/MoveInto.php | 7 +-- Classes/Domain/Model/Changes/Property.php | 6 +-- Classes/Domain/Model/Changes/Remove.php | 10 +--- .../Operations/ReloadContentOutOfBand.php | 52 +++++++++---------- .../Feedback/Operations/UpdateNodeInfo.php | 14 +++-- .../Operations/UpdateWorkspaceInfo.php | 24 +-------- .../ChangeCollectionConverter.php | 1 + 20 files changed, 74 insertions(+), 124 deletions(-) diff --git a/Classes/Application/SyncWorkspace/SyncWorkspaceCommand.php b/Classes/Application/SyncWorkspace/SyncWorkspaceCommand.php index 4a39375feb..f80dac1709 100644 --- a/Classes/Application/SyncWorkspace/SyncWorkspaceCommand.php +++ b/Classes/Application/SyncWorkspace/SyncWorkspaceCommand.php @@ -32,7 +32,7 @@ public function __construct( public ContentRepositoryId $contentRepositoryId, public WorkspaceName $workspaceName, - public DimensionSpacePoint $preferredDimensionSpacePoint, + public ?DimensionSpacePoint $preferredDimensionSpacePoint, public RebaseErrorHandlingStrategy $rebaseErrorHandlingStrategy ) { } diff --git a/Classes/Controller/BackendServiceController.php b/Classes/Controller/BackendServiceController.php index 7f93705291..279e0ee716 100644 --- a/Classes/Controller/BackendServiceController.php +++ b/Classes/Controller/BackendServiceController.php @@ -17,6 +17,7 @@ use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; use Neos\ContentRepository\Core\Feature\WorkspaceModification\Exception\WorkspaceIsNotEmptyException; use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Dto\RebaseErrorHandlingStrategy; +use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints; use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateCurrentlyDoesNotExist; use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint; @@ -477,17 +478,19 @@ public function changeBaseWorkspaceAction(string $targetWorkspaceName, string $d // todo ensure that https://github.com/neos/neos-ui/pull/3734 doesnt need to be refixed in Neos 9.0 $redirectNode = $documentNode; while (true) { + // @phpstan-ignore-next-line $redirectNodeInBaseWorkspace = $subgraph->findNodeById($redirectNode->aggregateId); if ($redirectNodeInBaseWorkspace) { break; } else { + // @phpstan-ignore-next-line $redirectNode = $subgraph->findParentNode($redirectNode->aggregateId); // get parent always returns Node if (!$redirectNode) { throw new \Exception( sprintf( 'Wasn\'t able to locate any valid node in rootline of node %s in the workspace %s.', - $documentNode->aggregateId->value, + $documentNode?->aggregateId->value, $targetWorkspaceName ), 1458814469 @@ -495,6 +498,8 @@ public function changeBaseWorkspaceAction(string $targetWorkspaceName, string $d } } } + /** @var Node $documentNode */ + /** @var Node $redirectNode */ // If current document node exists in the base workspace, then reload, else redirect if ($redirectNode->equals($documentNode)) { diff --git a/Classes/Domain/Model/AbstractChange.php b/Classes/Domain/Model/AbstractChange.php index 9320d79c34..345762651f 100644 --- a/Classes/Domain/Model/AbstractChange.php +++ b/Classes/Domain/Model/AbstractChange.php @@ -28,7 +28,7 @@ */ abstract class AbstractChange implements ChangeInterface { - protected ?Node $subject = null; + protected Node $subject; #[Flow\Inject] protected ContentRepositoryRegistry $contentRepositoryRegistry; @@ -56,7 +56,7 @@ final public function setSubject(Node $subject): void $this->subject = $subject; } - final public function getSubject(): ?Node + final public function getSubject(): Node { return $this->subject; } @@ -66,13 +66,11 @@ final public function getSubject(): ?Node */ final protected function updateWorkspaceInfo(): void { - if (!is_null($this->subject)) { - $subgraph = $this->contentRepositoryRegistry->subgraphForNode($this->subject); - $documentNode = $subgraph->findClosestNode($this->subject->aggregateId, FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT)); - if (!is_null($documentNode)) { - $updateWorkspaceInfo = new UpdateWorkspaceInfo($documentNode->contentRepositoryId, $documentNode->workspaceName); - $this->feedbackCollection->add($updateWorkspaceInfo); - } + $subgraph = $this->contentRepositoryRegistry->subgraphForNode($this->subject); + $documentNode = $subgraph->findClosestNode($this->subject->aggregateId, FindClosestNodeFilter::create(nodeTypes: NodeTypeNameFactory::NAME_DOCUMENT)); + if (!is_null($documentNode)) { + $updateWorkspaceInfo = new UpdateWorkspaceInfo($documentNode->contentRepositoryId, $documentNode->workspaceName); + $this->feedbackCollection->add($updateWorkspaceInfo); } } @@ -108,11 +106,9 @@ protected function reloadDocument(Node $node = null): void */ final protected function addNodeCreatedFeedback(Node $subject = null): void { - $node = $subject ?: $this->getSubject(); - if ($node) { - $nodeCreated = new NodeCreated(); - $nodeCreated->setNode($node); - $this->feedbackCollection->add($nodeCreated); - } + $node = $subject ?? $this->getSubject(); + $nodeCreated = new NodeCreated(); + $nodeCreated->setNode($node); + $this->feedbackCollection->add($nodeCreated); } } diff --git a/Classes/Domain/Model/ChangeInterface.php b/Classes/Domain/Model/ChangeInterface.php index de3728aa35..932b8f12d9 100644 --- a/Classes/Domain/Model/ChangeInterface.php +++ b/Classes/Domain/Model/ChangeInterface.php @@ -29,7 +29,7 @@ public function setSubject(Node $subject): void; /** * Get the subject */ - public function getSubject(): ?Node; + public function getSubject(): Node; /** * Checks whether this change can be applied to the subject diff --git a/Classes/Domain/Model/Changes/AbstractStructuralChange.php b/Classes/Domain/Model/Changes/AbstractStructuralChange.php index f7a95dfae5..f3dafdb559 100644 --- a/Classes/Domain/Model/Changes/AbstractStructuralChange.php +++ b/Classes/Domain/Model/Changes/AbstractStructuralChange.php @@ -102,7 +102,7 @@ public function getSiblingDomAddress(): ?RenderedNodeDomAddress */ public function getSiblingNode(): ?Node { - if ($this->siblingDomAddress === null || !$this->getSubject()) { + if ($this->siblingDomAddress === null) { return null; } diff --git a/Classes/Domain/Model/Changes/CopyAfter.php b/Classes/Domain/Model/Changes/CopyAfter.php index 6f41906f1d..a08e0ad744 100644 --- a/Classes/Domain/Model/Changes/CopyAfter.php +++ b/Classes/Domain/Model/Changes/CopyAfter.php @@ -30,9 +30,6 @@ class CopyAfter extends AbstractStructuralChange */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $siblingNode = $this->getSiblingNode(); if (is_null($siblingNode)) { return false; @@ -57,7 +54,7 @@ public function apply(): void : null; $subject = $this->subject; - if ($this->canApply() && $subject && !is_null($previousSibling) && !is_null($parentNodeOfPreviousSibling)) { + if ($this->canApply() && !is_null($previousSibling) && !is_null($parentNodeOfPreviousSibling)) { $succeedingSibling = null; try { $succeedingSibling = $this->findChildNodes($parentNodeOfPreviousSibling)->next($previousSibling); diff --git a/Classes/Domain/Model/Changes/CopyBefore.php b/Classes/Domain/Model/Changes/CopyBefore.php index aeb01f567e..d744f2b762 100644 --- a/Classes/Domain/Model/Changes/CopyBefore.php +++ b/Classes/Domain/Model/Changes/CopyBefore.php @@ -28,9 +28,6 @@ class CopyBefore extends AbstractStructuralChange */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $siblingNode = $this->getSiblingNode(); if (is_null($siblingNode)) { return false; @@ -57,7 +54,7 @@ public function apply(): void ? $this->findParentNode($succeedingSibling) : null; $subject = $this->subject; - if ($this->canApply() && !is_null($subject) && !is_null($succeedingSibling) + if ($this->canApply() && !is_null($succeedingSibling) && !is_null($parentNodeOfSucceedingSibling) ) { $contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId); diff --git a/Classes/Domain/Model/Changes/CopyInto.php b/Classes/Domain/Model/Changes/CopyInto.php index e5d1a1b6af..8e21e38346 100644 --- a/Classes/Domain/Model/Changes/CopyInto.php +++ b/Classes/Domain/Model/Changes/CopyInto.php @@ -51,7 +51,7 @@ public function canApply(): bool { $parentNode = $this->getParentNode(); - return $this->subject && $parentNode && $this->isNodeTypeAllowedAsChildNode($parentNode, $this->subject->nodeTypeName); + return $parentNode && $this->isNodeTypeAllowedAsChildNode($parentNode, $this->subject->nodeTypeName); } public function getMode(): string @@ -66,7 +66,7 @@ public function apply(): void { $subject = $this->getSubject(); $parentNode = $this->getParentNode(); - if ($parentNode && $subject && $this->canApply()) { + if ($parentNode && $this->canApply()) { $contentRepository = $this->contentRepositoryRegistry->get($subject->contentRepositoryId); $command = CopyNodesRecursively::createFromSubgraphAndStartNode( $contentRepository->getContentGraph($subject->workspaceName)->getSubgraph( diff --git a/Classes/Domain/Model/Changes/Create.php b/Classes/Domain/Model/Changes/Create.php index 18e571dd0e..431ab888be 100644 --- a/Classes/Domain/Model/Changes/Create.php +++ b/Classes/Domain/Model/Changes/Create.php @@ -40,7 +40,7 @@ public function canApply(): bool $subject = $this->getSubject(); $nodeTypeName = $this->getNodeTypeName(); - return $subject && $nodeTypeName && $this->isNodeTypeAllowedAsChildNode($subject, $nodeTypeName); + return $nodeTypeName && $this->isNodeTypeAllowedAsChildNode($subject, $nodeTypeName); } /** @@ -49,7 +49,7 @@ public function canApply(): bool public function apply(): void { $parentNode = $this->getSubject(); - if ($parentNode && $this->canApply()) { + if ($this->canApply()) { $this->createNode($parentNode, null); $this->updateWorkspaceInfo(); } diff --git a/Classes/Domain/Model/Changes/CreateAfter.php b/Classes/Domain/Model/Changes/CreateAfter.php index 3e290c6e0a..1d1a4a0e5d 100644 --- a/Classes/Domain/Model/Changes/CreateAfter.php +++ b/Classes/Domain/Model/Changes/CreateAfter.php @@ -32,9 +32,6 @@ public function getMode(): string */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $parent = $this->findParentNode($this->subject); $nodeTypeName = $this->getNodeTypeName(); @@ -46,9 +43,9 @@ public function canApply(): bool */ public function apply(): void { - $parentNode = $this->subject ? $this->findParentNode($this->subject) : null; + $parentNode = $this->findParentNode($this->subject); $subject = $this->subject; - if ($this->canApply() && !is_null($subject) && !is_null($parentNode)) { + if ($this->canApply() && !is_null($parentNode)) { $succeedingSibling = null; try { $succeedingSibling = $this->findChildNodes($parentNode)->next($subject); diff --git a/Classes/Domain/Model/Changes/CreateBefore.php b/Classes/Domain/Model/Changes/CreateBefore.php index ffaacce7e9..0907d07301 100644 --- a/Classes/Domain/Model/Changes/CreateBefore.php +++ b/Classes/Domain/Model/Changes/CreateBefore.php @@ -32,9 +32,6 @@ public function getMode(): string */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $parent = $this->findParentNode($this->subject); $nodeTypeName = $this->getNodeTypeName(); @@ -46,9 +43,9 @@ public function canApply(): bool */ public function apply(): void { - $parent = $this->subject ? $this->findParentNode($this->subject) : null; + $parent = $this->findParentNode($this->subject); $subject = $this->subject; - if ($this->canApply() && !is_null($subject) && !is_null($parent)) { + if ($this->canApply() && !is_null($parent)) { $this->createNode($parent, $subject->aggregateId); $this->updateWorkspaceInfo(); } diff --git a/Classes/Domain/Model/Changes/MoveAfter.php b/Classes/Domain/Model/Changes/MoveAfter.php index c5bb00a942..8f80c28472 100644 --- a/Classes/Domain/Model/Changes/MoveAfter.php +++ b/Classes/Domain/Model/Changes/MoveAfter.php @@ -30,9 +30,6 @@ class MoveAfter extends AbstractStructuralChange */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $sibling = $this->getSiblingNode(); if (is_null($sibling)) { return false; @@ -58,9 +55,8 @@ public function apply(): void $parentNodeOfPreviousSibling = $precedingSibling ? $this->findParentNode($precedingSibling) : null; // "subject" is the to-be-moved node $subject = $this->subject; - $parentNode = $this->subject ? $this->findParentNode($this->subject) : null; + $parentNode = $this->findParentNode($this->subject); if ($this->canApply() - && !is_null($subject) && !is_null($precedingSibling) && !is_null($parentNodeOfPreviousSibling) && !is_null($parentNode) diff --git a/Classes/Domain/Model/Changes/MoveBefore.php b/Classes/Domain/Model/Changes/MoveBefore.php index e2d813fa97..879c52fc35 100644 --- a/Classes/Domain/Model/Changes/MoveBefore.php +++ b/Classes/Domain/Model/Changes/MoveBefore.php @@ -29,9 +29,6 @@ class MoveBefore extends AbstractStructuralChange */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $siblingNode = $this->getSiblingNode(); if (is_null($siblingNode)) { return false; @@ -54,9 +51,9 @@ public function apply(): void $succeedingSibling = $this->getSiblingNode(); // "subject" is the to-be-moved node $subject = $this->subject; - $parentNode = $subject ? $this->findParentNode($subject) : null; + $parentNode = $this->findParentNode($subject); $succeedingSiblingParent = $succeedingSibling ? $this->findParentNode($succeedingSibling) : null; - if ($this->canApply() && !is_null($subject) && !is_null($succeedingSibling) + if ($this->canApply() && !is_null($succeedingSibling) && !is_null($parentNode) && !is_null($succeedingSiblingParent) ) { $precedingSibling = null; diff --git a/Classes/Domain/Model/Changes/MoveInto.php b/Classes/Domain/Model/Changes/MoveInto.php index 284cae7e98..74dbc07956 100644 --- a/Classes/Domain/Model/Changes/MoveInto.php +++ b/Classes/Domain/Model/Changes/MoveInto.php @@ -33,7 +33,7 @@ public function setParentContextPath(string $parentContextPath): void public function getParentNode(): ?Node { - if ($this->parentContextPath === null || !$this->getSubject()) { + if ($this->parentContextPath === null) { return null; } @@ -57,9 +57,6 @@ public function getMode(): string */ public function canApply(): bool { - if (is_null($this->subject)) { - return false; - } $parent = $this->getParentNode(); return $parent && $this->isNodeTypeAllowedAsChildNode($parent, $this->subject->nodeTypeName); @@ -74,7 +71,7 @@ public function apply(): void $parentNode = $this->getParentNode(); // "subject" is the to-be-moved node $subject = $this->subject; - if ($this->canApply() && $parentNode && $subject) { + if ($this->canApply() && $parentNode) { $otherParent = $this->contentRepositoryRegistry->subgraphForNode($subject) ->findParentNode($subject->aggregateId); diff --git a/Classes/Domain/Model/Changes/Property.php b/Classes/Domain/Model/Changes/Property.php index cf445895e2..dbff01dffc 100644 --- a/Classes/Domain/Model/Changes/Property.php +++ b/Classes/Domain/Model/Changes/Property.php @@ -126,7 +126,7 @@ public function getIsInline(): bool public function canApply(): bool { $propertyName = $this->getPropertyName(); - if (!$this->subject || !$propertyName) { + if (!$propertyName) { return false; } $nodeType = $this->getNodeType($this->subject); @@ -147,9 +147,9 @@ public function canApply(): bool public function apply(): void { $subject = $this->subject; - $nodeType = $subject ? $this->getNodeType($subject) : null; + $nodeType = $this->getNodeType($subject); $propertyName = $this->getPropertyName(); - if (is_null($subject) || is_null($nodeType) || is_null($propertyName) || $this->canApply() === false) { + if (is_null($nodeType) || is_null($propertyName) || $this->canApply() === false) { return; } diff --git a/Classes/Domain/Model/Changes/Remove.php b/Classes/Domain/Model/Changes/Remove.php index b56371ea37..1243b35b77 100644 --- a/Classes/Domain/Model/Changes/Remove.php +++ b/Classes/Domain/Model/Changes/Remove.php @@ -34,12 +34,6 @@ */ class Remove extends AbstractChange { - /** - * @Flow\Inject - * @var ContentCacheFlusher - */ - protected $contentCacheFlusher; - /** * Checks whether this change can be applied to the subject * @@ -47,7 +41,7 @@ class Remove extends AbstractChange */ public function canApply(): bool { - return !is_null($this->subject); + return true; } /** @@ -60,7 +54,7 @@ public function canApply(): bool public function apply(): void { $subject = $this->subject; - if ($this->canApply() && !is_null($subject)) { + if ($this->canApply()) { $parentNode = $this->findParentNode($subject); if (is_null($parentNode)) { throw new \InvalidArgumentException( diff --git a/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php b/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php index 52374833ad..ac9ba7e918 100644 --- a/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php +++ b/Classes/Domain/Model/Feedback/Operations/ReloadContentOutOfBand.php @@ -31,7 +31,7 @@ */ class ReloadContentOutOfBand extends AbstractFeedback { - protected ?Node $node = null; + protected Node $node; protected ?RenderedNodeDomAddress $nodeDomAddress; @@ -64,7 +64,7 @@ public function setNode(Node $node): void $this->node = $node; } - public function getNode(): ?Node + public function getNode(): Node { return $this->node; } @@ -86,7 +86,7 @@ public function getType(): string public function getDescription(): string { - return sprintf('Rendering of node "%s" required.', $this->node?->aggregateId->value); + return sprintf('Rendering of node "%s" required.', $this->node->aggregateId->value); } /** @@ -109,7 +109,7 @@ public function isSimilarTo(FeedbackInterface $feedback): bool */ public function serializePayload(ControllerContext $controllerContext): array { - if (!is_null($this->node) && !is_null($this->nodeDomAddress)) { + if (!is_null($this->nodeDomAddress)) { $contentRepository = $this->contentRepositoryRegistry->get($this->node->contentRepositoryId); $nodeAddressFactory = NodeAddressFactory::create($contentRepository); return [ @@ -126,32 +126,30 @@ public function serializePayload(ControllerContext $controllerContext): array */ protected function renderContent(ControllerContext $controllerContext): string { - if (!is_null($this->node)) { - $cacheTags = $this->cachingHelper->nodeTag($this->node); - foreach ($cacheTags as $tag) { - $this->contentCache->flushByTag($tag); + $cacheTags = $this->cachingHelper->nodeTag($this->node); + foreach ($cacheTags as $tag) { + $this->contentCache->flushByTag($tag); + } + + if ($this->nodeDomAddress) { + $renderingMode = $this->renderingModeService->findByCurrentUser(); + + $view = $this->outOfBandRenderingViewFactory->resolveView(); + if (method_exists($view, 'setControllerContext')) { + // deprecated + $view->setControllerContext($controllerContext); } + $view->setOption('renderingModeName', $renderingMode->name); + + $view->assign('value', $this->node); + $view->setRenderingEntryPoint($this->nodeDomAddress->getFusionPathForContentRendering()); - if ($this->nodeDomAddress) { - $renderingMode = $this->renderingModeService->findByCurrentUser(); - - $view = $this->outOfBandRenderingViewFactory->resolveView(); - if (method_exists($view, 'setControllerContext')) { - // deprecated - $view->setControllerContext($controllerContext); - } - $view->setOption('renderingModeName', $renderingMode->name); - - $view->assign('value', $this->node); - $view->setRenderingEntryPoint($this->nodeDomAddress->getFusionPathForContentRendering()); - - $content = $view->render(); - if ($content instanceof ResponseInterface) { - // todo should not happen, as we never render a full Neos.Neos:Page here? - return $content->getBody()->getContents(); - } - return $content->getContents(); + $content = $view->render(); + if ($content instanceof ResponseInterface) { + // todo should not happen, as we never render a full Neos.Neos:Page here? + return $content->getBody()->getContents(); } + return $content->getContents(); } return ''; diff --git a/Classes/Domain/Model/Feedback/Operations/UpdateNodeInfo.php b/Classes/Domain/Model/Feedback/Operations/UpdateNodeInfo.php index 7c2c13ee29..46b80d8b48 100644 --- a/Classes/Domain/Model/Feedback/Operations/UpdateNodeInfo.php +++ b/Classes/Domain/Model/Feedback/Operations/UpdateNodeInfo.php @@ -27,7 +27,7 @@ */ class UpdateNodeInfo extends AbstractFeedback { - protected ?Node $node = null; + protected Node $node; /** * @Flow\Inject @@ -68,7 +68,7 @@ public function recursive(): void $this->isRecursive = true; } - public function getNode(): ?Node + public function getNode(): Node { return $this->node; } @@ -80,7 +80,7 @@ public function getType(): string public function getDescription(): string { - return sprintf('Updated info for node "%s" is available.', $this->node?->aggregateId->value); + return sprintf('Updated info for node "%s" is available.', $this->node->aggregateId->value); } /** @@ -102,11 +102,9 @@ public function isSimilarTo(FeedbackInterface $feedback): bool */ public function serializePayload(ControllerContext $controllerContext): array { - return $this->node - ? [ - 'byContextPath' => $this->serializeNodeRecursively($this->node, $controllerContext->getRequest()) - ] - : []; + return [ + 'byContextPath' => $this->serializeNodeRecursively($this->node, $controllerContext->getRequest()) + ]; } /** diff --git a/Classes/Domain/Model/Feedback/Operations/UpdateWorkspaceInfo.php b/Classes/Domain/Model/Feedback/Operations/UpdateWorkspaceInfo.php index 03683668df..53577a573b 100644 --- a/Classes/Domain/Model/Feedback/Operations/UpdateWorkspaceInfo.php +++ b/Classes/Domain/Model/Feedback/Operations/UpdateWorkspaceInfo.php @@ -26,8 +26,6 @@ */ class UpdateWorkspaceInfo extends AbstractFeedback { - protected ?WorkspaceName $workspaceName = null; - /** * @Flow\Inject * @var WorkspaceService @@ -43,31 +41,17 @@ class UpdateWorkspaceInfo extends AbstractFeedback /** * UpdateWorkspaceInfo constructor. * - * @param WorkspaceName $workspaceName */ public function __construct( private readonly ContentRepositoryId $contentRepositoryId, - WorkspaceName $workspaceName = null + private readonly WorkspaceName $workspaceName ) { - $this->workspaceName = $workspaceName; - } - - /** - * Set the workspace - * - * @param Workspace $workspace - * @return void - * @deprecated - */ - public function setWorkspace(Workspace $workspace) - { - $this->workspaceName = $workspace->workspaceName; } /** * Getter for WorkspaceName */ - public function getWorkspaceName(): ?WorkspaceName + public function getWorkspaceName(): WorkspaceName { return $this->workspaceName; } @@ -115,10 +99,6 @@ public function isSimilarTo(FeedbackInterface $feedback) */ public function serializePayload(ControllerContext $controllerContext) { - if (!$this->workspaceName) { - return null; - } - $workspace = $this->workspaceProvider->provideForWorkspaceName( $this->contentRepositoryId, $this->workspaceName diff --git a/Classes/TypeConverter/ChangeCollectionConverter.php b/Classes/TypeConverter/ChangeCollectionConverter.php index 937ab18c0f..a452640d63 100644 --- a/Classes/TypeConverter/ChangeCollectionConverter.php +++ b/Classes/TypeConverter/ChangeCollectionConverter.php @@ -134,6 +134,7 @@ protected function convertChangeData(array $changeData, ContentRepositoryId $con $subjectContextPath = $changeData['subject']; $subject = $this->nodeService->findNodeBySerializedNodeAddress($subjectContextPath, $contentRepositoryId); + // we guard that `setSubject` gets a Node! if (is_null($subject)) { throw new \RuntimeException('Could not find node for subject "' . $subjectContextPath . '"', 1645657340); }