Skip to content

Commit

Permalink
TASK: Improve fusionGlobals handling in the Fusion FusionView
Browse files Browse the repository at this point in the history
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)
```
  • Loading branch information
mhsdesign committed Apr 22, 2024
1 parent 06570b5 commit 55b082a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 37 deletions.
69 changes: 34 additions & 35 deletions Classes/View/FusionView.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class FusionView extends AbstractView
protected $supportedOptions = [
'fusionPathPatterns' => [['resource://@package/Private/Fusion'], 'Fusion files that will be loaded if directories are given the Root.fusion is used.', 'array'],
'fusionPath' => [null, 'The Fusion path which should be rendered; derived from the controller and action names or set by the user.', 'string'],
'fusionGlobals' => [null, 'Additional global variables; merged together with the "request". Must only be specified at creation.', FusionGlobals::class],
'fusionGlobals' => [null, 'Additional Fusion global variables. The request must be assigned using `assign`. For regular variables please use `assign` as well.', 'array'],
'packageKey' => [null, 'The package key where the Fusion should be loaded from. If not given, is automatically derived from the current request.', 'string'],
'debugMode' => [false, 'Flag to enable debug mode of the Fusion runtime explicitly (overriding the global setting).', 'boolean'],
'enableContentCache' => [false, 'Flag to enable content caching inside Fusion (overriding the global setting).', 'boolean']
Expand Down Expand Up @@ -82,6 +82,12 @@ class FusionView extends AbstractView
*/
protected $fusionRuntime = null;

/**
* Via {@see assign} request using the "request" key,
* will be available also as Fusion global in the runtime.
*/
protected ?ActionRequest $assignedActionRequest = null;

/**
* Reset runtime cache if an option is changed
*
Expand All @@ -91,30 +97,37 @@ class FusionView extends AbstractView
*/
public function setOption($optionName, $value)
{
// todo do we want to allow to set the `fusionPathPatterns` after the first render?
$this->fusionPath = null;
parent::setOption($optionName, $value);
}

public function assign($key, $value)
{
if ($key === 'request') {
// the request cannot be used as "normal" fusion variable and must be treated as FusionGlobal
// to for example not cache it accidentally
// additionally we need it for special request based handling in the view
$this->assignedActionRequest = $value;
return $this;
}
return parent::assign($key, $value);
}

/**
* Legacy layer to make the `request` available in Fusion.
* Legacy layer to set the request for this view if not set already.
*
* Please use {@see setOption} and {@see FusionGlobals} instead:
* Please use {@see assign} with "request" instead
*
* $view->setOption('fusionGlobals', FusionGlobals::fromArray(['request' => $this->request]))
* $view->assign('request"', $this->request)
*
* @deprecated with Neos 9
*/
public function setControllerContext(ControllerContext $controllerContext)
{
/** @var FusionGlobals $fusionGlobals */
$fusionGlobals = $this->options['fusionGlobals'] ?? FusionGlobals::empty();
if ($fusionGlobals->has('request')) {
// no need for legacy layer as the request was already set.
return;
if (!$this->assignedActionRequest) {
$this->assignedActionRequest = $controllerContext->getRequest();
}
$this->setOption('fusionGlobals', $fusionGlobals->merge(FusionGlobals::fromArray(
['request' => $controllerContext->getRequest()]
)));
}

/**
Expand Down Expand Up @@ -185,14 +198,15 @@ public function initializeFusionRuntime()
{
if ($this->fusionRuntime === null) {
$this->loadFusion();

$fusionGlobals = $this->options['fusionGlobals'] ?? FusionGlobals::empty();
if (!$fusionGlobals instanceof FusionGlobals) {
throw new \InvalidArgumentException('View option "fusionGlobals" must be of type FusionGlobals', 1694252923947);
$additionalGlobals = FusionGlobals::fromArray($this->options['fusionGlobals'] ?? []);
if ($additionalGlobals->has('request')) {
throw new \RuntimeException(sprintf('The request cannot be set using the additional fusion globals. Please use $view->assign("request", ...) instead.'), 1708854895);
}
$this->fusionRuntime = $this->runtimeFactory->createFromConfiguration(
$this->parsedFusion,
$fusionGlobals
$this->assignedActionRequest
? $additionalGlobals->merge(FusionGlobals::fromArray(['request' => $this->assignedActionRequest]))
: $additionalGlobals
);
}
if (isset($this->options['debugMode'])) {
Expand Down Expand Up @@ -265,11 +279,10 @@ protected function getPackageKey()
if ($packageKey !== null) {
return $packageKey;
} else {
$request = $this->getRequestFromFusionGlobals();
if (!$request) {
if (!$this->assignedActionRequest) {
throw new \RuntimeException(sprintf('To resolve the @package in all fusionPathPatterns, either packageKey has to be specified, or the current request be available.'), 1708267874);
}
return $request->getControllerPackageKey();
return $this->assignedActionRequest->getControllerPackageKey();
}
}

Expand All @@ -285,7 +298,7 @@ protected function getFusionPathForCurrentRequest()
if ($fusionPath !== null) {
$this->fusionPath = $fusionPath;
} else {
$request = $this->getRequestFromFusionGlobals();
$request = $this->assignedActionRequest;
if (!$request) {
throw new \RuntimeException(sprintf('The option `fusionPath` was not set. Could not fallback to the current request.'), 1708267857);
}
Expand All @@ -300,18 +313,4 @@ protected function getFusionPathForCurrentRequest()
}
return $this->fusionPath;
}

private function getRequestFromFusionGlobals(): ?ActionRequest
{
/** @var FusionGlobals $fusionGlobals */
$fusionGlobals = $this->options['fusionGlobals'] ?? FusionGlobals::empty();
if (!$fusionGlobals instanceof FusionGlobals) {
throw new \InvalidArgumentException('View option "fusionGlobals" must be of type FusionGlobals', 1694252923947);
}
$actionRequest = $fusionGlobals->get('request');
if (!$actionRequest instanceof ActionRequest) {
return null;
}
return $actionRequest;
}
}
3 changes: 1 addition & 2 deletions Tests/Functional/View/FusionViewTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Neos\Flow\Mvc\ActionRequest;
use Neos\Flow\Tests\FunctionalTestCase;
use Neos\Fusion\Core\FusionGlobals;
use Neos\Fusion\View\FusionView;
use Psr\Http\Message\ResponseInterface;

Expand Down Expand Up @@ -106,7 +105,7 @@ protected function buildView($controllerObjectName, $controllerActionName)
$request->expects(self::any())->method('getControllerActionName')->will(self::returnValue($controllerActionName));

$view = new FusionView();
$view->setOption('fusionGlobals', FusionGlobals::fromArray(['request' => $request]));
$view->assign('request', $request);
$view->setFusionPathPattern(__DIR__ . '/Fixtures/Fusion');

return $view;
Expand Down

0 comments on commit 55b082a

Please sign in to comment.