!!! FEATURE: Overhaul node uri building

[mhsdesign]

Resolves: #4552

Introduces a new API for node-uribuilding to replace the LinkingService
Currently Neos9-Beta10 does already contain a similar new NodeUriBuilder but this change overhauls the API to not expose the implementations of the Flow UriBuilder.

Upgrade instructions

The new NodeUriBuilder is not a singleton and thus cannot be injected but must be created via the NodeUriBuilderFactory with the current ActionRequest at hand.

Simple case:

// before
#[Flow\Inject]
protected LinkingService $linkingService;

public function uriForNode(Node $node, ControllerContext $controllerContext): string
{
    return $this->linkingService
        ->createNodeUri(
            controllerContext: $controllerContext,
            node: $node
        );
}

// after
#[Flow\Inject]
protected NodeUriBuilderFactory $nodeUriBuilderFactory;

public function uriForNode(Node $node, ActionRequest $actionRequest): UriInterface
{
    return $this->nodeUriBuilderFactory
        ->forActionRequest($actionRequest)
        ->uriFor(
            NodeAddress::fromNode($node)
        );
}

Advanced case:

// before
#[Flow\Inject]
protected LinkingService $linkingService;

public function uriForNode(Node $node, ControllerContext $controllerContext): string
{
    return $this->linkingService
        ->createNodeUri(
            controllerContext: $controllerContext,
            node: $node,
            format: 'json',
            absolute: true,
            arguments: ['simple-query' => 'abc'],
            section: 'my-anchor'
        );
}

// after
#[Flow\Inject]
protected NodeUriBuilderFactory $nodeUriBuilderFactory;

public function uriForNode(Node $node, ActionRequest $actionRequest): UriInterface
{
    $resolvedUri = $this->nodeUriBuilderFactory
        ->forActionRequest($actionRequest)
        ->uriFor(
            NodeAddress::fromNode($node),
            Options::create(forceAbsolute: true)->withCustomFormat('json')
        );

    return UriHelper::uriWithAdditionalQueryParameters($resolvedUri, ['simple-query' => 'abc'])
        ->withFragment('my-anchor');
}

Also notice that the PHP api for uri building will no longer keep the current format of the action request by default unlike in the Fusion api.
If this behaviour is required it should be implemented via $options->withCustomFormat($actionRequest->getFormat());

Review instructions

Currently in 9.0-dev the NodeUriBuilder is a not well though out abstraction. It has a really shallow implementation and doest abstract the uribuilder away, but rather exposes it to be set from outside. That makes it impossible to swap out the implementation and also the api is questionable. How do i really set the format option or add additional queryparameters?

With the overhaul of the api and lately new introduced Shared\NodeAddress we can go one step further and not use the legacy FrontendRouting\NodeAdress any further. The new address will fill its place and make thus also cross site linking will be possible: #4441

We decided against adding the query parameters to be encoded as array option to the NodeUriBuilder Options and promote to use the helper UriHelper::uriWithAdditionalQueryParameters.
It is better to have it separated into a helper because its not core PSR standard behaviour (where the query is just a string) and anything further like http_build_query is php magic which does not obey an official standard.
Also another argument is that its just string manipulation after the resolve was successful for the same reason a fragment cannot be provided as option but must be set via the PSR Api withFragment.

Adjustment prs:

• TASK: Adjust to introduction of Shared\NodeAddress and new NodeUriBuilder
• TASK: adjust to overhauled node uri building

Related discussions:

• !!! FEATURE: Introduce ActionUriBuilder flow-development-collection#2744
• RFC CLI compatible context for uri building flow-development-collection#3354
• FEATURE: introduce UriHelper to work with query parameters flow-development-collection#3316
• Split up NodeAddress into dedicated classes #4564
• Split up NodeAddress into dedicated classes #4564

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[mhsdesign]

i also noticed that the current format will NOT be takend from the actionRequest so one would need to take care of this themselves is this ok?

[mhsdesign]

But i guess the Fusion helpers should default to the requests format? Either in php or better via

format = ${request.format}

what do you think? ...

[bwaidelich]

Good question, I'm not sure about that. When I played with HTMX, I used a custom format for the HX requests (see https://github.com/bwaidelich/Wwwision.HtmxTest/blob/main/Resources%2FPrivate%2FFusion%2FComponents%2FHtmxUri.fusion)
In the results I would not want this format to be used for all links implicitly

[kitsunet]

Currently it's the expected behavior, I have had it in both directions in projects. Sometimes I was happy it would automatically keep the xml I had in hte current request without me having to check it at every link, other times I would have wished to ignore it and use html by default 🤷

[mhsdesign]

yeah i guess for fusion (especially as the magic is currently still part of the Neos.Fusion:ActionUri) i think it would make sense to keep it for the Neos.Neos:NodeUri as well.

Ill vote for making it a default value on fusion side so its directly visible to the human eye? or should it be part of the fusion php implementation?

edit on the other hand this is core behaviour and has to be documented via comment. Otherwise it would be weird that we access the request differently in two places (once for the uri builder and then for the format)

[bwaidelich]

My gut feeling is still: html is like the 99% case and I would want to explicitly change that if I wanted to create links to other resources in the same format (e.g. in a JSON response I might want to link to other JSON resources, but I would expect the node URI to still have the default format:

{
  "title": "foo",
  "contactUri": "https://domain.tld/contact",
  "includedResources": {
    "contacts": [
      {"id": "123", "href": "/api/contacts/123.json"}
    ]
  }
}

Or maybe I can override the default format for the RoutingDefaults per controller/action?

[mhsdesign]

I agree that the magic of keeping the format either has helped (by knowing it beforehand) or worse lead to confusion and i had to debug many times (because i forgot) why uri building failed ... the @format: json part in the exception dump cat Data... tends to go unnoticed for the untrained eye.
Still for this change it would be cruel to change behaviour like this ever so slightly and id prefer a dedicated discussion and separate pr.
I think we only have to agree that the PHP Node Uri building api doesnt do it by default and how to implement this magic again for the Fusion prototypes.

[mficzel]

That makes sense ... to me.

I would be fine with simply adding a default format of "html" to the fusion prototypes that is documented.

That way you only have to read the fusion documentation to get it and not know about some magic behavior that works until you forget it and then causes trouble.

[mhsdesign]

Im not against removing the format by request magic but id prefer a separate pr for that (and probably wont do it myself right now because i cant sell it as well as you do :D)
I guess this pr is a step in the right direction by removing that feature from the core node uribuilding and making it opt in from the outside. That means we can eventually get rid of it!

[kitsunet]

Giving this 👀
Might do some more testing tomorrow, if it doesn't get merged.

[mficzel]

+1 By 👀 and 🧪

Should be tested with: neos/neos-ui#3802 as otherwise you end up with errors in the backend.

[mficzel]

\o/ ... yeha that alone is worth it ... if only this would be the main fe route. I fear this needs appendExceedingArguments a while longer

[mficzel]

That makes sense ... to me.

I would be fine with simply adding a default format of "html" to the fusion prototypes that is documented.

That way you only have to read the fusion documentation to get it and not know about some magic behavior that works until you forget it and then causes trouble.

[mhsdesign]

Thank you so much for all the reviews and discussions ;) Glad to have this merged!!! One step closer.