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.

Sign up for GitHub

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: Introduce routingArguments and queryParameters to Fusion link prototypes to eventually replace arguments and additionalParams #3914

Open
wants to merge 4 commits into
base: 8.4
Choose a base branch
from
Open

FEATURE: Introduce routingArguments and queryParameters to Fusion link prototypes to eventually replace arguments and additionalParams #3914

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f8bd9a
FEATURE: Introduce `routingArguments` and `queryParameters` to Fusion…
mficzel Oct 7, 2022
47a96b0
Merge remote-tracking branch 'origin/8.4' into feature/routingArgumen…
mhsdesign Oct 22, 2024
b09c77e
TASK: Add NodeUri test
mhsdesign Oct 22, 2024
8f0245e
TASK: Adjust deprecation notices and introduce routingArguments for N…
mhsdesign Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 55 additions & 4 deletions Neos.Fusion/Classes/FusionObjects/ActionUriImplementation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
* source code.
*/

use GuzzleHttp\Psr7\Uri;
use Neos\Flow\Mvc\ActionRequest;
use Neos\Flow\Mvc\Routing\UriBuilder;
use Neos\Fusion\Exception\RuntimeException;
use Neos\Utility\Arrays;

/**
* A Fusion ActionUri object
Expand Down Expand Up @@ -85,10 +88,22 @@ public function getAction(): ?string
return $this->fusionValue('action');
}

/**
* Controller arguments that are to be handled by the router
*
* @return array
*/
public function getRoutingArguments(): array
{
$routingArguments = $this->fusionValue('routingArguments');
return is_array($routingArguments) ? $routingArguments: [];
}

/**
* Controller arguments
*
* @return array|null
* @deprecated to be removed with Neos 9
*/
public function getArguments(): ?array
{
Expand Down Expand Up @@ -120,16 +135,29 @@ public function getSection(): ?string
* Additional query parameters that won't be prefixed like $arguments (overrule $arguments)
*
* @return array|null
* @deprecated to be removed with Neos 9
*/
public function getAdditionalParams(): ?array
{
return $this->fusionValue('additionalParams');
}

/**
* Query parameters that are appended to the url
*
* @return array
*/
public function getQueryParameters(): array
{
$queryParameters = $this->fusionValue('queryParameters');
return is_array($queryParameters) ? $queryParameters: [];
}

/**
* Arguments to be removed from the URI. Only active if addQueryString = true
*
* @return array|null
* @deprecated to be removed with Neos 9
*/
public function getArgumentsToBeExcludedFromQueryString(): ?array
{
Expand All @@ -140,6 +168,7 @@ public function getArgumentsToBeExcludedFromQueryString(): ?array
* If true, the current query parameters will be kept in the URI
*
* @return boolean
* @deprecated to be removed with Neos 9
*/
public function isAddQueryString(): bool
{
Expand All @@ -157,12 +186,21 @@ public function isAbsolute(): bool
}

/**
* @return string
* @return UriBuilder
*/
public function evaluate()
public function createUriBuilder(): UriBuilder
{
$uriBuilder = new UriBuilder();
$uriBuilder->setRequest($this->getRequest());
return $uriBuilder;
}

/**
* @return string
*/
public function evaluate()
{
$uriBuilder = $this->createUriBuilder();

$format = $this->getFormat();
if ($format !== null) {
Expand Down Expand Up @@ -195,13 +233,26 @@ public function evaluate()
}

try {
return $uriBuilder->uriFor(
$arguments = $this->getArguments();
$routingArguments = $this->getRoutingArguments();
if ($arguments && $routingArguments) {
throw new RuntimeException('Neos.Fusion:ActionUri does not allow to combine "arguments" and "routingArguments"', 1665431866);
}
$uriString = $uriBuilder->uriFor(
$this->getAction(),
$this->getArguments(),
$routingArguments ?: $arguments ?: [],
$this->getController(),
$this->getPackage(),
$this->getSubpackage()
);
$queryParameters = $this->getQueryParameters();
if (empty($queryParameters)) {
return $uriString;
}
$uri = new Uri($uriString);
Copy link
Member

@mhsdesign mhsdesign Feb 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would look great in a flow uri utility or service, but I know the hassle since it’s in another repo… it might be enough time to pull it up (and you have my +1)

Advantage beeign that we can super easily tests this ugly code snippet isolated against all scenarios.

… or we can always later adjust it too (which is more likely a never thought ^^)

Copy link
Member Author

@mficzel mficzel Feb 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually this code will be replaced by an all new action-uri-builder on the flow side. See neos/flow-development-collection#2744 (not finished and still in discussion). Once this is in place ActionUri should be adjusted to use this internally.

In the meantime this pr allows to handle the negative effects of additionalParams ending up in the routingCache and introduces the distinction between routingArguments and queryParameters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay i see then its totally fine with inlining this temporarily ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI now with neos/flow-development-collection#3316 in place we can actually use a proper helper instead ;)

parse_str($uri->getQuery(), $queryParametersFromRouting);
$mergedQueryParameters = Arrays::arrayMergeRecursiveOverrule($queryParametersFromRouting, $queryParameters);
return (string)$uri->withQuery(http_build_query($mergedQueryParameters, '', '&'));
} catch (\Exception $exception) {
return $this->runtime->handleRenderingException($this->path, $exception);
}
Expand Down
2 changes: 2 additions & 0 deletions Neos.Fusion/Resources/Private/Fusion/Prototypes/ActionUri.fusion
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ prototype(Neos.Fusion:ActionUri) {
action = null
format = null
section = null
routingArguments = Neos.Fusion:DataStructure
queryParameters = Neos.Fusion:DataStructure
additionalParams = Neos.Fusion:DataStructure
arguments = Neos.Fusion:DataStructure
argumentsToBeExcludedFromQueryString = Neos.Fusion:DataStructure
Expand Down
249 changes: 249 additions & 0 deletions Neos.Fusion/Tests/Unit/FusionObjects/ActionUriImplementationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,249 @@
<?php
namespace Neos\Fusion\Tests\Unit\FusionObjects;

/*
* This file is part of the Neos.Fusion 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\Mvc\Routing\UriBuilder;
use Neos\Flow\Tests\UnitTestCase;
use Neos\Fusion\Core\Runtime;
use Neos\Fusion\FusionObjects\ActionUriImplementation;

/**
* Testcase for the Fusion ActionUri object
*/
class ActionUriImplementationTest extends UnitTestCase
{
/**
* @var ActionUriImplementation
*/
protected $mockActionUriImplementation;

/**
* @var UriBuilder
*/
protected $mockUriBuilder;

/**
* @var Runtime
*/
protected $mockRuntime;

public function setUp(): void
{
$this->mockRuntime = $this->getMockBuilder(Runtime::class)->disableOriginalConstructor()->getMock();
$this->mockUriBuilder = $this->getMockBuilder(UriBuilder::class)->disableOriginalConstructor()->getMock();

$methodsToMock = [
'getRequest',
'getPackage',
'getSubpackage',
'getController',
'getAction',
'getRoutingArguments',
'getArguments',
'getFormat',
'getSection',
'getAdditionalParams',
'getQueryParameters',
'isAbsolute',
'getArgumentsToBeExcludedFromQueryString',
'isAddQueryString',
'createUriBuilder'
];

$this->mockActionUriImplementation = $this->getMockBuilder(ActionUriImplementation::class)->disableOriginalConstructor()->onlyMethods($methodsToMock)->getMock();
$this->mockActionUriImplementation->expects($this->once())->method('createUriBuilder')->willReturn($this->mockUriBuilder);
$this->inject($this->mockActionUriImplementation, 'runtime', $this->mockRuntime);
$this->inject($this->mockActionUriImplementation, 'path', '/example/path');
}

/**
* @return void
* @test
*/
public function actionIsPassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockUriBuilder->expects($this->once())->method('uriFor')->with('hello', [], null, null, null)->willReturn("http://example.com");
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function formatIsPassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getFormat')->willReturn('square');
$this->mockUriBuilder->expects($this->once())->method('setFormat')->with('square');
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function additionalParamsArePassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getAdditionalParams')->willReturn(['nudel' => 'suppe']);
$this->mockUriBuilder->expects($this->once())->method('setArguments')->with(['nudel' => 'suppe']);
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function argumentsToBeExcludedFromQueryStringArePassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getArgumentsToBeExcludedFromQueryString')->willReturn(['nudel', 'suppe']);
$this->mockUriBuilder->expects($this->once())->method('setArgumentsToBeExcludedFromQueryString')->with(['nudel', 'suppe']);
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function absoluteIsPassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('isAbsolute')->willReturn(true);
$this->mockUriBuilder->expects($this->once())->method('setCreateAbsoluteUri')->with(true);
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function sectionIsPassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getSection')->willReturn('something');
$this->mockUriBuilder->expects($this->once())->method('setSection')->with('something');
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function addQueryStringIsPassedToTheUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('isAddQueryString')->willReturn(true);
$this->mockUriBuilder->expects($this->once())->method('setAddQueryString')->with(true);
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function actionPackageAndArgumentsArePassedToUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getArguments')->willReturn(['test' => 123]);
$this->mockActionUriImplementation->expects($this->once())->method('getRoutingArguments')->willReturn([]);
$this->mockActionUriImplementation->expects($this->once())->method('getController')->willReturn('Special');
$this->mockActionUriImplementation->expects($this->once())->method('getPackage')->willReturn('Vendor.Package');
$this->mockActionUriImplementation->expects($this->once())->method('getSubpackage')->willReturn('Stuff');

$this->mockUriBuilder->expects($this->once())->method('uriFor')->with('hello', ['test' => 123], 'Special', 'Vendor.Package', 'Stuff')->willReturn("http://example.com");
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function actionPackageAndRoutingArgumentsArePassedToUriBuilder()
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getArguments')->willReturn([]);
$this->mockActionUriImplementation->expects($this->once())->method('getRoutingArguments')->willReturn(['test' => 123]);
$this->mockActionUriImplementation->expects($this->once())->method('getController')->willReturn('Special');
$this->mockActionUriImplementation->expects($this->once())->method('getPackage')->willReturn('Vendor.Package');
$this->mockActionUriImplementation->expects($this->once())->method('getSubpackage')->willReturn('Stuff');

$this->mockUriBuilder->expects($this->once())->method('uriFor')->with('hello', ['test' => 123], 'Special', 'Vendor.Package', 'Stuff')->willReturn("http://example.com");
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function actionUriUsesArguments(): void
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getArguments')->willReturn(['foo' => 'bar']);
$this->mockActionUriImplementation->expects($this->once())->method('getRoutingArguments')->willReturn([]) ;
$this->mockUriBuilder->expects($this->once())->method('uriFor')->with('hello', ['foo' => 'bar'], null, null, null)->willReturn("http://hostname");
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function actionUriUsesRoutingArguments(): void
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getArguments')->willReturn([]);
$this->mockActionUriImplementation->expects($this->once())->method('getRoutingArguments')->willReturn(['foo' => 'bar']) ;
$this->mockUriBuilder->expects($this->once())->method('uriFor')->with('hello', ['foo' => 'bar'], null, null, null)->willReturn("http://hostname");
$this->mockActionUriImplementation->evaluate();
}

/**
* @return void
* @test
*/
public function actionUriThrowsExceptionIfRoutingArgumentAreUsedTogetherWithArguments(): void
{
$this->mockActionUriImplementation->expects($this->once())->method('getArguments')->willReturn(['bar' => 'baz']);
$this->mockActionUriImplementation->expects($this->once())->method('getRoutingArguments')->willReturn(['foo' => 'bar']) ;
$this->mockUriBuilder->expects($this->never())->method('uriFor');
$this->mockRuntime->expects($this->once())->method('handleRenderingException');
$this->mockActionUriImplementation->evaluate();
}

public function queryParameterAppendingDataProvider(): array
{
return [
['https://example.com', ['foo' => 'bar'], 'https://example.com?foo=bar'],
['https://example.com?foo=bar', ['bar' => 'baz'], 'https://example.com?foo=bar&bar=baz'],
['https://example.com?foo=bar', ['foo' => 'bam'], 'https://example.com?foo=bam'],
['https://example.com', ['foo' => ['bar' => 'baz']], 'https://example.com?foo%5Bbar%5D=baz'],
['https://example.com?foo=bar', ['foo' => ['bar' => 'baz']], 'https://example.com?foo%5Bbar%5D=baz'],
['https://example.com?foo[bar]=baz', ['foo' => ['blah' => 'blubb']], 'https://example.com?foo%5Bbar%5D=baz&foo%5Bblah%5D=blubb']
];
}

/**
* @return void
* @dataProvider queryParameterAppendingDataProvider
* @test
*/
public function actionUriAppendsQueryParametersToUri($uriFromLinking, $queryParameters, $expectedFinalUri): void
{
$this->mockActionUriImplementation->expects($this->once())->method('getAction')->willReturn('hello');
$this->mockActionUriImplementation->expects($this->once())->method('getQueryParameters')->willReturn($queryParameters);
$this->mockUriBuilder->expects($this->once())->method('uriFor')->with('hello', [], null, null, null)->willReturn($uriFromLinking);
$actualResult = $this->mockActionUriImplementation->evaluate();
$this->assertEquals($expectedFinalUri, $actualResult);
}
}
Loading
Loading