DISCUSSION: How to avoid in fusion access to `$this->runtime->getControllerContext()->getResponse()`?

[mhsdesign]

Fusions PluginImplementation

neos-development-collection/Neos.Neos/Classes/Fusion/PluginImplementation.php

Lines 169 to 180 in d1eb6ab

	$parentResponse = $this->runtime->getControllerContext()->getResponse();
	$pluginResponse = new ActionResponse();
	$this->dispatcher->dispatch($this->buildPluginRequest(), $pluginResponse);
	// We need to make sure to not merge content up into the parent ActionResponse
	// because that would break the Fusion HttpResponse.
	$content = $pluginResponse->getContent();
	$pluginResponse->setContent('');
	$pluginResponse->mergeIntoParentResponse($parentResponse);
	return $content;

Fusion RuntimeFormImplementation

https://github.com/neos/fusion-form/blob/642f728976bd205205ce76180b4c701a67adb30c/Classes/Runtime/FusionObjects/RuntimeFormImplementation.php#L172-L174

I guess we would have to replace the controller context with a less ugly but still ugly object and pass that somehow to the view and then to the runtime?

final readonly class ActionMessages
{
     public function __construct(
         public ActionRequest $request,
         public ActionResponse $response
     ) {
     }
}

... or the runtime could return actual an ReponseInterface on the final rendering and could have some internal hacky state to collect the headers?
That would obviously only work for uncached segments to manipulate that global state but still. Basically a hole through space but first level support from fusion. And basically the same as we have now, but less ugliness exposed overall and the view interface doesnt need the controller context for fusion anymore :D

FusionView {
   render(): ReponseInterface
}

FusionRuntime {
   unsafeAdditionalDynamicHeaders: []

   renderResponse() {
      return new Response(render())->withHeaders(unsafeAdditionalDynamicHeaders)
   }
}

SomeFusionObjectImplementation {
   render() {
      // warning this only works if the object is evaluated consistently via mode=uncached.!!!
      runtime.unsafeAdditionalDynamicHeaders['Location'] = "localhost"
   }
}

[kitsunet]

That would obviously only work for uncached segments to manipulate that global state but still.

Wouldn't that be already the state we have now? If a cached fusion object manipulates the response, that will happen only on the first uncached render and not on subsequent ones.

[kitsunet]

How about just having a FusionRuntime->mergeResponse($additionalResponse) and internally the FusionRuntime carries a response around, which is merged in the HttpResponseImplementation?

SomeFusionObjectImplementation {
   render() {
      // warning this only works if the object is evaluated consistently via mode=uncached.!!!
      $myResponse = new ActionResponse();
      $myResponse->....doStuff/setHeaders
      $runtime->mergeResponse($myResponse);
   }
}

[mhsdesign]

Wouldn't that be already the state we have now? If a cached fusion object manipulates the response, that will happen only on the first uncached render and not on subsequent ones.

exactly, so no change here.

It worked actually out quite good and even with a working legacy layer so neos fusion form still works :D

And any code interacting with $this->runtime->getControllerContext()->getResponse(); :D

#4856

[mhsdesign]

Sadly it seems it doesnt work perfectly and there are test cases to proof it :D

The header, if existent in the the parent response is currently duplicated and not correctly overridden:

neos-development-collection/Neos.Neos/Tests/Unit/Fusion/PluginImplementationTest.php

Line 102 in 497b1a2

['key' => 'value'] // 'value, value'

The whole replaceHttpResponse / buildHttpResponse is a bit flaky, see neos/flow-development-collection#2492

I dont think its unfixable we just have to find a way.

[kitsunet]

I think multiple competing headers like that are a broken use case anyways. Headers by definition can appear multiple times, we can't really know if we should remove the first one and when not. IMHO the test is misleading the result might as well (correctly) be key => [value, value]

It shows a potential use case for the exceptions though, if you actualyl want to redirect the main request from within a plugin
you kinda have be able to throw to escape your plugin prison. Not sure how well that actually works with the code base right now as the plugin has it's own dispatch loop, but that could be a valid reason. I don't think status codes upmerging makes any sense as it's always unclear what should win. What if there are two plugins on the page? Either we allow it by excepting and thereby preventing the second plugin from even running, or we don't allow it at all.

[mhsdesign]

It shows a potential use case for the exceptions though, if you actualyl want to redirect the main request from within a plugin
you kinda have be able to throw to escape your plugin prison. Not sure how well that actually works with the code base right now as the plugin has it's own dispatch loop, but that could be a valid reason.

As far as i can tell that cannot work due to the own loop. A ForwardException will be catched and handled there and the forward will only affect the plugin subrequest and the main request is not forwarded. So i would not say that this is a reason for exceptions, especially considering that no one might need that feature...

I don't think status codes upmerging makes any sense as it's always unclear what should win. What if there are two plugins on the page? Either we allow it by excepting and thereby preventing the second plugin from even running, or we don't allow it at all.

well yes i agree it all is a bit muddy, but @mficzel relies on some behaviour to upmerge a redirect (with status code) for the Neos.Fusion.Form.Runtime:Redirect action:

https://github.com/neos/fusion-form/blob/642f728976bd205205ce76180b4c701a67adb30c/Classes/Runtime/Action/RedirectAction.php#L44

https://github.com/neos/fusion-form/blob/642f728976bd205205ce76180b4c701a67adb30c/Classes/Runtime/FusionObjects/RuntimeFormImplementation.php#L172-L174

but sure if there are other headers set or things rendered the redirect will just win without question (even in the fusion form, a returned action Message will be just ignored / never shown). Its wild west.

[mhsdesign]

I think we should keep at first the proposed unsafeAdditionalDynamicHeaders/ unsafeHttpResponseConstrains feature out of the game.

Fusion objects interacting with $this->runtime->getControllerContext()->getResponse()->set... are hacky as it is and changing the part $this->runtime->getControllerContext()->getResponse() to $this->runtime->bla is just syntactic sugar.

What is important that the controller context available will not be the actual controller context from the controller but self crafted by the fusion runtime which will be used to return the psr response.

At some point we might actually find out what we want to support, headers, redirects or status codes by plugin and by then we can decide on a nice api.