-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
!!! FEATURE: Render ResponseInterface
directly in Fusion Views
#4856
!!! FEATURE: Render ResponseInterface
directly in Fusion Views
#4856
Conversation
/** | ||
* @test | ||
*/ | ||
public function fusionViewCanReturnHttpResponseFromHttpMessagePrototype() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is actually a bugfix as the FusionView dindt handle Neos.Fusion:Http.Message
before. Only the Neos FusionView did.
protected function parsePotentialRawHttpResponse($output) | ||
{ | ||
if ($this->isRawHttpResponse($output)) { | ||
return Message::parseResponse($output); | ||
} | ||
|
||
return $output; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this hack could be removed from the Neos FusionView and the view will use the new Runtime::renderResponse which will handle this.
* @deprecated {@see self::getControllerContext()} | ||
* @internal | ||
*/ | ||
public function setControllerContext(ControllerContext $controllerContext): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method was only introduced in 9.0 dev thats why we can so easily remove it: #4425
@@ -169,23 +167,10 @@ public function setControllerContext(ControllerContext $controllerContext): void | |||
*/ | |||
public function getControllerContext(): ControllerContext | |||
{ | |||
if (isset($this->controllerContext)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic was only introduced in 9.0 dev thats why we can so easily change it: #4425
428afca
to
a64bd6c
Compare
ResponseInterface
directly in Fusion Runtime
a64bd6c
to
5d4eb41
Compare
5fbe56b
to
27313f9
Compare
27313f9
to
12bf4c7
Compare
12bf4c7
to
b737c01
Compare
ResponseInterface
directly in Fusion RuntimeResponseInterface
directly in Fusion Runtime
I can confirm that a Though it needs an additional patch as plugins are currently broken on 9.0-dev anyways: #4909 Things i tested in the controller:
|
ResponseInterface
directly in Fusion RuntimeResponseInterface
directly in Fusion Views
bf09d77
to
e2f7562
Compare
198c755
to
17caac0
Compare
…o allow setting headers dynamically Instead, the legacy layer > $this->runtime->getControllerContext()->getResponse(); should be continued to be used by fusion forms and fusion plugin impl.
…s->get('request');`
…yControllerContext
Previously the pattern was that the utmost caller of the runtime would unwrap the exception. To simplify this, as the runtime now has a single entry point, we add this behaviour into the runtime.
... to centralise tricking phpstan
This will be discussed separately and not part of the change of the Neos Node `FusionView`
…`ControllerContext` `getArguments` removed without replacement.
…ead of `mixed` This will make the `Neos.Fusion:Http.Message` prototype work out of the box. This was also done to return a value allowed by the view: `string|ActionResponse|ResponseInterface|StreamInterface|\Stringable` Adjusts AbstractFusionObjectTest::buildView to the runtime directly. We still use a `ViewInterface` here as this allows us to not rewrite all our tests :D We will rewrite them either way in behat some time.
…ot a string After a discussion with Christian we found it to be a more workable behaviour than throwing an exception.
Initially introduced the option expected an actual instance of `FusionGlobals`. This is a flawed design as only primitives can be set for example via `Views.yaml` Additionally, the `FusionView` is forward compatible for this change neos/flow-development-collection#3286 The `fusionGlobals` must not be used for setting the request but rather ``` $view->assign('request"', $this->request) ```
This magic behaviour is not expected and will rather lead to bugs instead of help the integrator to use fusion. The action controllers `renderView` would previously just silently ignore this case and render a blank page. Instead, to force to write correct functioning entry points, we throw an exception like: > Fusion entry path "root" is expected to render a compatible http response body: string|\Stringable|null. Got array instead.
- this will make the change less breaking due to allowing an implicit string cast - adjust to fluidViews will now return a stream
361f2d0
to
13077e6
Compare
Co-authored-by: Wilhelm Behncke <[email protected]>
…using a hacky anonymous class
13077e6
to
2cf100f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!
Neos Adjustments for neos/flow-development-collection#3286
StreamInterface
or anResponseInterface
StreamInterface
or anResponseInterface
The fusion runtime will not accept the controller context from outside anymore, but instead has its own legacy layer.
The legacy layer only works in combination with the new entry point in the fusion runtime:
Fusion's
FusionView
adjustments:The current view behaves incorrect. The ViewInterface allows to return
string|ActionResponse|ResponseInterface|StreamInterface|\Stringable
But we violate this by returning the raw runtime value instead ... which for all we now could be a string but might also be just as annotated anything else (
mixed
) not included in the allowed return types, like raw arrays or non string-able objects or integers.Also previously when using the
Neos.Fusion:Http.Message
in combination with the view it would actually render the http response as string, soHTTP/
was part of the content and visible in the dom (This is currently broken since Neos 8 and before).To streamline the behaviour and fix these specialities, the
FusionView
will return if applicable aResponseInterface
and for simple (string) cases a psrStreamInterface
. Both cases have to be anticipated on call site.The change is not expected to be super breaking if a
StreamInterface
will be returned as for classes with loose typing this will be converted via__toString
.This is also necessary for
Neos.Neos:Plugin
andNeos.Fusion.Form
to work correctly.Resolves: #4855
Upgrade instructions
Mutating the response of the Fusion Runtime (
$this->getRuntime()->getControllerContext()->getResponse()
) is highly internal and only allowed for Neos special cases as the fusion plugin implementation and fusion forms. This will be removed when we find a better concept internally!Also other usages of the
getControllerContext
are deprecated like accessing the request the following way:Instead, the new fusion globals #4425 should be used (which will also guarantee that the request in EEL
${request}
is the same as when fetched from a Fusion object:If you use the Fusion Runtime directly, which is internal, you should know that using
Runtime::render
as entry point will fail to renderNeos.Neos:Plugin
,Neos.Fusion.Form
and also lead to weird behaviour when rendering aNeos.Fusion:Http.Message
.The Fusion view should be used for interacting with Fusion, but if you must, you can leverage
Runtime::renderEntryPathWithContext
to render a full page with all special cases.Review instructions / WHY
The WHY of this PR might not seem obvious as a lot of pieces fall here into place.
Neos.Neos:Plugin
for sub-request linkNeos.Fusion:Form
to redirect on success to another page link$runtime->setHeader()
) we will not change the syntax but deprecate it and internalise it.renderEntryPathWithContext
)ViewInterface
will be adjusted in Flow to force returning either aStreamInterface
or anResponseInterface
, and also thesetControllerContext
will not be necessary to implement / call any longer on views.Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions