9.0 Discussion Node property mapping in controllers

[mhsdesign]

in neos 8 one could leverage the property mapping to write a controller like this

public function someAction(NodeInterface $node): {}

and pass as node its fully qualified serialised representation: the old context path?node=/sites/neosdemo/features/multiple-columns@user-admin;language=en_US.

In Neos 9 this is not that easily doable as the NodeAdress (the closest to its old context path) is not fully qualified see #4564. The correct way would be:

public function indexAction(string $node) {
    $siteDetectionResult = SiteDetectionResult::fromRequest($this->request->getHttpRequest());
    $contentRepository = $this->contentRepositoryRegistry->get($siteDetectionResult->contentRepositoryId);
    $nodeAddress = NodeAddressFactory::create($contentRepository)->createFromUriString($node);
    $workspace = $contentRepository->getWorkspaceFinder()->findOneByName(
        $nodeAddress->workspaceName
    );
    $subgraph = $contentRepository->getContentGraph()->getSubgraph(
        $workspace->currentContentStreamId,
        $nodeAddress->dimensionSpacePoint,
        VisibilityConstraints::withoutRestrictions()
    );
    $node = $subgraph->findNodeById($nodeAddress->nodeAggregateId);

But due to a hack in the NewNodeConverter - accessing the bootstraps getActiveRequestHandler - we can make that magically work:

public function indexAction(Node $node) {

When i stumbled upon the NewNodeConverter, based on its comment, i assumed that its only used for fusion uncached serialisation. Thats why i opened this issue #4732 (and pr #4734) to remove fusion dependency to the node property mapper and the hack.

But it seems i only thought, being initially unaware of the hack, that the old property mapping style would and should not work. By the fact that we dont use it i was even more convinced. It seems there was until now no real discussion and decision?.

[mhsdesign]

Copied from #4734 (comment)
With this PR Neos and Fusion will completely work without the property mapper #4734

In my opinion its super important that people dont accidentally use the property mapper with the new nodes.
Allowing mapping would render all of our strictness and new explicitness useless as it would allow again magical nodes in controllers:

public function nodeAction(Node $node): {}

which will already now confusion for example about the nodes format when serialised ...

Lets to it rather explicit this time please :D

[mhsdesign]

@kitsunet wrote:

Allowing mapping would render all of our strictness and new explicitness useless as it would allow again magical nodes in controllers:

I think that is a core expectation though, countless projects will have controllers expecting a node and I don#t see why for the time being as the propertyMapper IS still the suggested and only way to do controller marshalling build into Flow, this should work

[mhsdesign]

In the previous weekly (9.2.) we discussed (bastian, christian, denny, me) that we DO NOT want to support someAction(Node $nodeIdentity).
The consent was that mapping entities magically was never a good idea.

Instead to still make it simpler to get a node in a controller we decided that we want to introduce a fully qualified NodeIdentity. By also encapsulating the ContentRepositoryId it becomes less tedious to transform, and its representation is irrelevant of the domains host.

Similarly as bastian put it here #4564 (comment), the introduction of the NodeIdentity with the overhauled routing we can allow patterns like these:

public function someAction(string $nodeIdentity): {
   $nodeIdentity = NodeIdentity::fromJsonString($nodeIdentity);
   // ...
}

// with little property converter, almost no gain
public function someAction(NodeIdentity $nodeIdentity): {
  $node = $this->someService->findNodeByIdentity($nodeIdentity);
}