Closed albe closed 3 years ago
sorenmalling
commented
4 years ago I've been giving RoutingComponent a try-out. It's fairly tight copuled to PrepareMvcRequestComponent and DispatchComponent and even tied to SecurityEntryPointComponent, from inside the DispatchComponent.
Is this "MVC Dispatching Middleware" something we should focus on here and now while we have the engine open?
sorenmalling
commented
4 years ago @albe Is the approach correct for #2153 and #2154 with a single commit per middleware and Settings.yaml change?
albe
commented
4 years ago I've been giving RoutingComponent a try-out. It's fairly tight copuled to PrepareMvcRequestComponent and DispatchComponent and even tied to SecurityEntryPointComponent, from inside the DispatchComponent.
Let's take a look at that together maybe.
Is this "MVC Dispatching Middleware" something we should focus on here and now while we have the engine open?
If we have finished PSR-15 and everything around that is safe & sound. But low prio and from a gut feeling it will probably not happen for 7.0
Is the approach correct for #2153 and #2154 with a single commit per middleware and Settings.yaml change?
Sure, why not. We could have attempted to do all in one PR, but this can easily grow out of hand, so this is maybe the most efficient way for review. Just need to make sure all those PRs integrate well together, so we should merge and then rebase the others every once in a while.
sorenmalling
commented
4 years ago Let's take a look at that together maybe.
Tuesday and wednesday er good for me - how about you?
Just need to make sure all those PRs integrate well together, so we should merge and then rebase the others every once in a while
Great idea - let's rebase on top of other middleware commits before merging the next 👍
bwaidelich
commented
4 years ago I just had a great session with @albe and @sorenmalling about the fact that ActionResponse::setComponentParameter() won't work with the middleware approach since it is only possible to pass information (other than the HTTP response) down into the middleware chain, but the Dispatch MW has to be the inner most.
A few options we discussed:
...similar to the ComponentContext...
We don't like that solution because it defeats the idea of having a clean PSR-15 implementation
This would introduce a new level of complexity – and probably only for really edgy edge cases that can be solved otherwise (see below)
ActionResponse::setComponentParameter()Our favorite solution for now, even if more breaking (see comment regarding b/c)
ActionResponse::setComponentParameter()
$componentClassName parameter is not ReplaceHttpResponseComponent or SetHeaderComponentActionResponse::setHeader()
ActionResponse::setComponentParameter(SetHeaderComponent::class, 'foo', 'bar') into ActionResponse::setHeader('foo', 'bar')ActionResponse::replaceHttpResponse()
ActionResponse::setComponentParameter(ReplaceHttpResponseComponent::class, ReplaceHttpResponseComponent::PARAMETER_RESPONSE, $response) into ActionResponse::replaceHttpResponse($response)DispatchMiddleware (see below)
ServerRequest.withAttribute() to communicate with inner middlewaresThis is how the DispatchMiddleware could look like:
<?php
namespace Neos\Flow\Mvc;
/*
* This file is part of the Neos.Flow package.
*
* (c) Contributors of the Neos Project - www.neos.io
*
* This package is Open Source Software. For the full copyright and license
* information, please view the LICENSE file which was distributed with this
* source code.
*/
use Neos\Flow\Annotations as Flow;
use Neos\Flow\Http\ServerRequestAttributes;
use Neos\Flow\Security\Context;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
/**
* A dispatch component
*/
class DispatchMiddleware implements MiddlewareInterface
{
/**
* @Flow\Inject
* @var Dispatcher
*/
protected $dispatcher;
/**
* @Flow\Inject
* @var ActionRequestFactory
*/
protected $actionRequestFactory;
/**
* @Flow\Inject
* @var Context
*/
protected $securityContext;
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
$routingMatchResults = $request->getAttribute(ServerRequestAttributes::ROUTING_RESULTS) ?? [];
$actionRequest = $this->actionRequestFactory->createActionRequest($request, $routingMatchResults);
// can we move this somewhere else?
$this->securityContext->setRequest($actionRequest);
$actionResponse = new ActionResponse();
$this->dispatcher->dispatch($actionRequest, $actionResponse);
return $actionResponse->buildHttpResponse();
}
}diff --git a/Neos.Flow/Classes/Mvc/ActionResponse.php b/Neos.Flow/Classes/Mvc/ActionResponse.php
index 8cbbced88..538b3ccef 100644
--- a/Neos.Flow/Classes/Mvc/ActionResponse.php
+++ b/Neos.Flow/Classes/Mvc/ActionResponse.php
@@ -1,14 +1,16 @@
<?php
namespace Neos\Flow\Mvc;
+use GuzzleHttp\Psr7\Response;
+use GuzzleHttp\Psr7\Stream;
+use Neos\Flow\Annotations as Flow;
+use Neos\Flow\Http\Component\ReplaceHttpResponseComponent;
+use Neos\Flow\Http\Component\SetHeaderComponent;
use Neos\Flow\Http\Cookie;
use Psr\Http\Message\ResponseInterface;
-use function GuzzleHttp\Psr7\stream_for;
-use Neos\Flow\Annotations as Flow;
-use Neos\Flow\Http\Component\ComponentContext;
use Psr\Http\Message\StreamInterface;
use Psr\Http\Message\UriInterface;
-use GuzzleHttp\Psr7\Stream;
+use function GuzzleHttp\Psr7\stream_for;
/**
* The minimal MVC response object.
@@ -26,9 +28,14 @@ final class ActionResponse
protected $content;
/**
- * @var array
+ * @var ResponseInterface|null
*/
- protected $componentParameters = [];
+ protected $replacedHttpResponse;
+
+ /**
+ * @var string[][]
+ */
+ protected $httpHeaders = [];
/**
* @var UriInterface
@@ -147,32 +154,51 @@ final class ActionResponse
* @param string $parameterName
* @param mixed $value
* @return void
- * @api
+ * @deprecated since Flow 7.0 ... (todo explain)
*/
public function setComponentParameter(string $componentClassName, string $parameterName, $value): void
{
- if (!isset($this->componentParameters[$componentClassName])) {
- $this->componentParameters[$componentClassName] = [];
+ if ($componentClassName === SetHeaderComponent::class) {
+ $this->setHttpHeader($parameterName, $value);
+ return;
}
- $this->componentParameters[$componentClassName][$parameterName] = $value;
+ if ($componentClassName === ReplaceHttpResponseComponent::class && $parameterName === ReplaceHttpResponseComponent::PARAMETER_RESPONSE) {
+ $this->replaceHttpResponse($value);
+ return;
+ }
+ throw new \InvalidArgumentException('todo');
}
/**
- * @return string
+ * TODO document
+ *
+ * @param string $headerName
+ * @param string|string[] $headerValue
*/
- public function getContent(): string
+ public function setHttpHeader(string $headerName, $headerValue): void
{
- $content = $this->content->getContents();
- $this->content->rewind();
- return $content;
+ // todo validate $headerValue (string|string[])
+ $this->httpHeaders[$headerName] = $headerValue;
}
/**
- * @return array
+ * TODO document
+ *
+ * @param ResponseInterface $response
*/
- public function getComponentParameters(): array
+ public function replaceHttpResponse(ResponseInterface $response): void
{
- return $this->componentParameters;
+ $this->replacedHttpResponse = $response;
+ }
+
+ /**
+ * @return string
+ */
+ public function getContent(): string
+ {
+ $content = $this->content->getContents();
+ $this->content->rewind();
+ return $content;
}
/**
@@ -220,11 +246,11 @@ final class ActionResponse
if ($this->redirectUri !== null) {
$actionResponse->setRedirectUri($this->redirectUri);
}
-
- foreach ($this->componentParameters as $componentClass => $parameters) {
- foreach ($parameters as $parameterName => $parameterValue) {
- $actionResponse->setComponentParameter($componentClass, $parameterName, $parameterValue);
- }
+ if ($this->replacedHttpResponse !== null) {
+ $actionResponse->replaceHttpResponse($this->replacedHttpResponse);
+ }
+ foreach ($this->httpHeaders as $headerName => $headerValue) {
+ $actionResponse->setHttpHeader($headerName, $headerValue);
}
foreach ($this->cookies as $cookie) {
$actionResponse->setCookie($cookie);
@@ -234,57 +260,14 @@ final class ActionResponse
}
/**
- * @param ComponentContext $componentContext
- * @return ComponentContext
- */
- public function mergeIntoComponentContext(ComponentContext $componentContext): ComponentContext
- {
- $httpResponse = $componentContext->getHttpResponse();
- if ($this->statusCode !== null) {
- $httpResponse = $httpResponse->withStatus($this->statusCode);
- }
-
- if ($this->hasContent()) {
- $httpResponse = $httpResponse->withBody($this->content);
- }
-
- if ($this->contentType) {
- $httpResponse = $httpResponse->withHeader('Content-Type', $this->contentType);
- }
-
- if ($this->redirectUri) {
- $httpResponse = $httpResponse->withHeader('Location', (string)$this->redirectUri);
- }
-
- foreach ($this->componentParameters as $componentClassName => $componentParameterGroup) {
- foreach ($componentParameterGroup as $parameterName => $parameterValue) {
- $componentContext->setParameter($componentClassName, $parameterName, $parameterValue);
- }
- }
-
- foreach ($this->cookies as $cookie) {
- $httpResponse = $httpResponse->withAddedHeader('Set-Cookie', (string)$cookie);
- }
-
- $componentContext->replaceHttpResponse($httpResponse);
-
- return $componentContext;
- }
-
- /**
- * Note this is a special use case method that will apply the internal properties (Content-Type, StatusCode, Location, Set-Cookie and Content)
- * to the given PSR-7 Response and return a modified response. This is used to merge the ActionResponse properties into a possible HttpResponse
- * created in a View (see ActionController::renderView()) because those would be overwritten otherwise. Note that any component parameters will
- * still run through the component chain and will not be propagated here.
- *
- * WARNING: Should this ActionResponse contain body content it would replace any content in the given HttpReponse.
+ * TODO document
*
- * @param ResponseInterface $httpResponse
* @return ResponseInterface
* @internal
*/
- public function applyToHttpResponse(ResponseInterface $httpResponse): ResponseInterface
+ public function buildHttpResponse(): ResponseInterface
{
+ $httpResponse = $this->replacedHttpResponse ?? new Response();
if ($this->statusCode !== null) {
$httpResponse = $httpResponse->withStatus($this->statusCode);
}
@@ -305,9 +288,14 @@ final class ActionResponse
$httpResponse = $httpResponse->withAddedHeader('Set-Cookie', (string)$cookie);
}
+ foreach ($this->httpHeaders as $headerName => $headerValue) {
+ $httpResponse = $httpResponse->withHeader($headerName, $headerValue);
+ }
+
return $httpResponse;
}
+
/**
* Does this action response have content?
*
This is a follow-up to #1889 that is meant to track the status of "phasing out" the existing HTTP Components.
This is the current (as of Flow 6.2) list of existing configured HTTP components. The order to convert them should be starting from pre-process, since the PSR-15 middlewares invoke the component chain. Hence the first step is the
TrustedProxiesComponent. Also, some Components can probably be merged together (SessionRequest+Response), some need to be moved into the MVC stack maybe. We'll need to play and adapt there.