Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

9.0 Avoid getActiveRequestHandler in node property mapper for uncached fusion rendering #4732

Closed
mhsdesign opened this issue Nov 12, 2023 · 3 comments · Fixed by #4734
Closed
Labels

Comments

@mhsdesign
Copy link
Member

mhsdesign commented Nov 12, 2023

For fusions uncached context variables (in the uncached mode) to work one needs a property mapper to serialize the variable to a string see code.

This is problematic for nodes in neos 9 as its string representation (the NodeAdress) doesnt serialize the content repository id: #4441

Previously in Neos 8.3 this was not a problem as a node could be easily resolved from a context path as there was no ambiguity of multiple content repositories.

To solve this obstacle (temporary?) we currently have this NewNodeConverter which has quite a side effect to get a content repository out of the blue:

$activeRequestHandler = $this->bootstrap->getActiveRequestHandler();
$contentRepositoryId = ContentRepositoryId::fromString('default');
if ($activeRequestHandler instanceof RequestHandler) {
$httpRequest = $activeRequestHandler->getHttpRequest();
$siteDetectionResult = SiteDetectionResult::fromRequest($httpRequest);
$contentRepositoryId = $siteDetectionResult->contentRepositoryId;
}

Using the activeRequest of the bootstrap like this will make it impossible to render fusion from the cli.

This fix was introduced with 763f293

I would not like to ship this code for Neos 9 final release as this property mapping would also allow implicit code like this to work:

public function nodeAction(Node $node): {}

This might be dangerous as its abstracts away the request, to content repository id mapping and besides is not testable as it doesnt use the action request of the controller but the one from the bootstrap.

-> extracted discussion to here: #4873

mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Nov 12, 2023
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Nov 12, 2023
@mhsdesign
Copy link
Member Author

@kitsunet wrote

Why do we not have a fully qualified node serialization format, that would help it I guess

Thats what i implemented here: #4734

but i guess maybe this should rather be part of the cr itself see NodeIdentity in #4564

@kitsunet
Copy link
Member

I am not so good with discussion here where here is not "there" 😂 too many place overflow...
Anyways yes I would rpefer a more unified solution, that also potentially gets rid of the \Neos\Neos\FrontendRouting\NodeAddress::serializeForUri in favor of a fully qualified format.

@mficzel
Copy link
Member

mficzel commented Nov 12, 2023

We will have to add the cr id to a node address anyways. So why not do it sooner than later.

@mhsdesign mhsdesign changed the title 9.0 Avoid getActiveRequestHandler in new node property mapper for uncached fusion rendering 9.0 Avoid getActiveRequestHandler in node property mapper for uncached fusion rendering Nov 12, 2023
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Feb 3, 2024
@mhsdesign mhsdesign moved this to Under Review 👀 in Neos 9.0 Release Board Feb 8, 2024
@mhsdesign mhsdesign added the 9.0 label Feb 8, 2024
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Feb 11, 2024
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Feb 13, 2024
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Feb 14, 2024
@github-project-automation github-project-automation bot moved this from Under Review 👀 to Done ✅ in Neos 9.0 Release Board Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
3 participants