Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 0 additions & 12 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -30570,12 +30570,6 @@ parameters:
count: 1
path: tests/bundle/Core/Fragment/DirectFragmentRendererTest.php

-
message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertInstanceOf\(\) with ''Symfony\\\\Component\\\\HttpFoundation\\\\Response'' and Symfony\\Component\\HttpFoundation\\Response will always evaluate to true\.$#'
identifier: method.alreadyNarrowedType
count: 4
path: tests/bundle/Core/Fragment/DirectFragmentRendererTest.php

-
message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Fragment\\FragmentListenerFactoryTest\:\:buildFragmentListenerProvider\(\) has no return type specified\.$#'
identifier: missingType.return
Expand Down Expand Up @@ -58764,12 +58758,6 @@ parameters:
count: 1
path: tests/lib/MVC/Symfony/Templating/RenderContentStrategyTest.php

-
message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertInstanceOf\(\) with ''Symfony\\\\Component\\\\HttpKernel\\\\Controller\\\\ControllerReference'' and Symfony\\Component\\HttpKernel\\Controller\\ControllerReference will always evaluate to true\.$#'
identifier: method.alreadyNarrowedType
count: 1
path: tests/lib/MVC/Symfony/Templating/RenderContentStrategyTest.php

-
message: '#^Anonymous function has an unused use \$content\.$#'
identifier: closure.unusedUse
Expand Down
2 changes: 2 additions & 0 deletions src/bundle/Core/Fragment/DirectFragmentRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ public function render(
if ($response instanceof Response) {
return $response;
} elseif ($response instanceof View) {
$response->addParameters($options['params'] ?? []);

return new Response($this->viewTemplateRenderer->render($response));
} elseif (is_string($response)) {
return new Response($response);
Expand Down
4 changes: 4 additions & 0 deletions src/bundle/Core/Fragment/InlineFragmentRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public function render($uri, Request $request, array $options = [])
if ($request->attributes->has('viewParametersString')) {
$uri->attributes['viewParametersString'] = $request->attributes->get('viewParametersString');
}
if ($options['params'] ?? false) {
Copy link
Member

Choose a reason for hiding this comment

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

Please refactor this so you don't rely on true-ish expression inside of the if statement. This works because of implicit cast which can lead to some bugs under some circumstances. What is the real condition making this usable? !is_empty && is_array?

$uri->attributes['params'] = $options['params'];
unset($options['params']);
Copy link
Member

Choose a reason for hiding this comment

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

This effectively changes the behavior which we can do in 6.0 ATM.

}
}

return $this->innerRenderer->render($uri, $request, $options);
Expand Down
2 changes: 1 addition & 1 deletion src/lib/MVC/Symfony/Templating/RenderContentStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function render(ValueObject $valueObject, RenderOptions $options): string

$renderer = $this->getFragmentRenderer($options->get('method', $this->defaultRenderer));

return $renderer->render($controllerReference, $currentRequest)->getContent();
return $renderer->render($controllerReference, $currentRequest, $options->has('params') ? ['params' => $options->get('params')] : [])->getContent();
Copy link
Member

Choose a reason for hiding this comment

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

Readability: please split this into multiple lines.

Personally I'd just go with:

Suggested change
return $renderer->render($controllerReference, $currentRequest, $options->has('params') ? ['params' => $options->get('params')] : [])->getContent();
return $renderer->render($controllerReference, $currentRequest, ['params' => $options->get('params', [])])->getContent();

and would've taken this into the account when processing further, effectively making params always present, reducing cognitive complexity.
But no strong feelings here.

}
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/MVC/Symfony/Templating/RenderLocationStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function render(ValueObject $valueObject, RenderOptions $options): string

$renderer = $this->getFragmentRenderer($options->get('method', $this->defaultRenderer));

return $renderer->render($controllerReference, $currentRequest)->getContent();
return $renderer->render($controllerReference, $currentRequest, $options->has('params') ? ['params' => $options->get('params')] : [])->getContent();
}
}

Expand Down
67 changes: 49 additions & 18 deletions tests/bundle/Core/Fragment/DirectFragmentRendererTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ public function testSubRequestBuilding(): void
$controllerResolver
->expects($this->any())
->method('getController')
->with($this->callback(function (Request $request) {
$this->assertEquals('/_fragment', $request->getPathInfo());
$this->assertEquals('some::controller', $request->attributes->get('_controller'));
$this->assertEquals('attribute_value', $request->attributes->get('some'));
$this->assertEquals('else', $request->attributes->get('something'));
$this->assertInstanceOf(SiteAccess::class, $request->attributes->get('siteaccess'));
$this->assertEquals('test', $request->attributes->get('siteaccess')->name);
->with($this->callback(static function (Request $request) {
self::assertEquals('/_fragment', $request->getPathInfo());
self::assertEquals('some::controller', $request->attributes->get('_controller'));
self::assertEquals('attribute_value', $request->attributes->get('some'));
self::assertEquals('else', $request->attributes->get('something'));
self::assertInstanceOf(SiteAccess::class, $request->attributes->get('siteaccess'));
self::assertEquals('test', $request->attributes->get('siteaccess')->name);

return true;
}))
Expand Down Expand Up @@ -65,8 +65,7 @@ public function testSubRequestBuilding(): void
$directFragmentRenderer = $this->getDirectFragmentRenderer($controllerResolver);
$response = $directFragmentRenderer->render($controllerReference, $request);

$this->assertInstanceOf(Response::class, $response);
$this->assertSame('rendered_response', $response->getContent());
self::assertSame('rendered_response', $response->getContent());
}

public function testControllerResponse(): void
Expand All @@ -82,11 +81,27 @@ public function testControllerResponse(): void
$directFragmentRenderer = $this->getDirectFragmentRenderer($controllerResolver);
$response = $directFragmentRenderer->render('', new Request(), []);

$this->assertInstanceOf(Response::class, $response);
$this->assertSame('response_body', $response->getContent());
self::assertSame('response_body', $response->getContent());
}

public function testControllerViewResponse(): void
/**
* @return iterable<array{0: array<string, string>|null}>
*/
public function controllerViewResponseDataProvider(): iterable
{
yield [[
'my_param1' => 'custom_data',
'my_param2' => 'foobar',
]];

yield [null];
}

/**
* @param array<string, string>|null $params
* @dataProvider controllerViewResponseDataProvider
*/
public function testControllerViewResponse(?array $params = null): void
{
$contentView = new ContentView();
$contentView->setTemplateIdentifier('template_identifier');
Expand All @@ -105,16 +120,33 @@ public function testControllerViewResponse(): void
->expects($this->once())
->method('render')
->with($contentView)
->willReturn('rendered_' . $contentView->getTemplateIdentifier());
->willReturnCallback(
static function (ContentView $cV) use ($params): string {
if ($params !== null) {
foreach ($params as $key => $value) {
self::assertArrayHasKey($key, $cV->getParameters());
}
}

return 'rendered_' . $cV->getTemplateIdentifier();
}
);

$directFragmentRenderer = $this->getDirectFragmentRenderer(
$controllerResolverMock,
$templateRendererMock
);
$response = $directFragmentRenderer->render('', new Request(), []);
$response = $directFragmentRenderer->render(
'',
new Request(),
[
'viewType' => 'line',
'method' => 'direct',
'params' => $params,
]
);

$this->assertInstanceOf(Response::class, $response);
$this->assertSame('rendered_template_identifier', $response->getContent());
self::assertSame('rendered_template_identifier', $response->getContent());
}

public function testControllerStringResponse(): void
Expand All @@ -130,8 +162,7 @@ public function testControllerStringResponse(): void
$directFragmentRenderer = $this->getDirectFragmentRenderer($controllerResolver);
$response = $directFragmentRenderer->render('', new Request(), []);

$this->assertInstanceOf(Response::class, $response);
$this->assertSame('some_prerendered_response', $response->getContent());
self::assertSame('some_prerendered_response', $response->getContent());
}

public function testControllerUnhandledStringResponse(): void
Expand Down
14 changes: 7 additions & 7 deletions tests/bundle/Core/Fragment/FragmentRendererBaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,22 @@ public function testRendererControllerReferenceWithCompoundMatcher(): Controller
->will($this->returnValue($expectedReturn));

$renderer = $this->getRenderer();
$this->assertSame($expectedReturn, $renderer->render($reference, $request, $options));
$this->assertArrayHasKey('serialized_siteaccess', $reference->attributes);
self::assertSame($expectedReturn, $renderer->render($reference, $request, $options));
self::assertArrayHasKey('serialized_siteaccess', $reference->attributes);
$serializedSiteAccess = json_encode($siteAccess);
$this->assertSame($serializedSiteAccess, $reference->attributes['serialized_siteaccess']);
$this->assertArrayHasKey('serialized_siteaccess_matcher', $reference->attributes);
$this->assertSame(
self::assertSame($serializedSiteAccess, $reference->attributes['serialized_siteaccess']);
self::assertArrayHasKey('serialized_siteaccess_matcher', $reference->attributes);
self::assertSame(
$this->getSerializer()->serialize(
$siteAccess->matcher,
'json',
[AbstractNormalizer::IGNORED_ATTRIBUTES => ['request', 'container', 'matcherBuilder']]
),
$reference->attributes['serialized_siteaccess_matcher']
);
$this->assertArrayHasKey('serialized_siteaccess_sub_matchers', $reference->attributes);
self::assertArrayHasKey('serialized_siteaccess_sub_matchers', $reference->attributes);
foreach ($siteAccess->matcher->getSubMatchers() as $subMatcher) {
$this->assertSame(
self::assertSame(
$this->getSerializer()->serialize(
$subMatcher,
'json',
Expand Down
62 changes: 47 additions & 15 deletions tests/bundle/Core/Fragment/InlineFragmentRendererTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Ibexa\Core\MVC\Symfony\Component\Serializer\SerializerTrait;
use Ibexa\Core\MVC\Symfony\SiteAccess;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Controller\ControllerReference;
use Symfony\Component\HttpKernel\Fragment\FragmentRendererInterface;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
Expand All @@ -22,7 +23,24 @@ class InlineFragmentRendererTest extends DecoratedFragmentRendererTest
{
use SerializerTrait;

public function testRendererControllerReference()
/**
* @return iterable<array{0: array<string, string>|null}>
*/
public function rendererControllerReferenceDataProvider(): iterable
{
yield [[
'my_param1' => 'custom_data',
'my_param2' => 'foobar',
]];

yield [null];
}

/**
* @param array<string, string>|null $params
* @dataProvider rendererControllerReferenceDataProvider
*/
public function testRendererControllerReference(?array $params = null)
{
$reference = new ControllerReference('FooBundle:bar:baz');
$matcher = new SiteAccess\Matcher\HostElement(1);
Expand All @@ -36,41 +54,55 @@ public function testRendererControllerReference()
$request->attributes->set('semanticPathinfo', '/foo/bar');
$request->attributes->set('viewParametersString', '/(foo)/bar');
$options = ['foo' => 'bar'];

$expectedReturn = '/_fragment?foo=bar';
$this->innerRenderer
->expects($this->once())
->method('render')
->with($reference, $request, $options)
->will($this->returnValue($expectedReturn));
->willReturnCallback(
static function (ControllerReference $url, Request $request, array $callBackOptions) use ($expectedReturn, $params, $options): Response {
if ($params !== null) {
self::assertEquals($params, $url->attributes['params']);
}
self::assertEquals($options, $callBackOptions);

return new Response($expectedReturn);
}
);

if ($params !== null) {
$options['params'] = $params;
}

$renderer = $this->getRenderer();
$this->assertSame($expectedReturn, $renderer->render($reference, $request, $options));
$this->assertTrue(isset($reference->attributes['serialized_siteaccess']));
self::assertEquals(new Response($expectedReturn), $renderer->render($reference, $request, $options));
self::assertTrue(isset($reference->attributes['serialized_siteaccess']));
$serializedSiteAccess = json_encode($siteAccess);
$this->assertSame($serializedSiteAccess, $reference->attributes['serialized_siteaccess']);
$this->assertTrue(isset($reference->attributes['serialized_siteaccess_matcher']));
$this->assertSame(
self::assertSame($serializedSiteAccess, $reference->attributes['serialized_siteaccess']);
self::assertTrue(isset($reference->attributes['serialized_siteaccess_matcher']));
self::assertSame(
$this->getSerializer()->serialize(
$siteAccess->matcher,
'json',
[AbstractNormalizer::IGNORED_ATTRIBUTES => ['request', 'container', 'matcherBuilder']]
),
$reference->attributes['serialized_siteaccess_matcher']
);
$this->assertTrue(isset($reference->attributes['semanticPathinfo']));
$this->assertSame('/foo/bar', $reference->attributes['semanticPathinfo']);
$this->assertTrue(isset($reference->attributes['viewParametersString']));
$this->assertSame('/(foo)/bar', $reference->attributes['viewParametersString']);
self::assertTrue(isset($reference->attributes['semanticPathinfo']));
self::assertSame('/foo/bar', $reference->attributes['semanticPathinfo']);
self::assertTrue(isset($reference->attributes['viewParametersString']));
self::assertSame('/(foo)/bar', $reference->attributes['viewParametersString']);
}

public function testRendererControllerReferenceWithCompoundMatcher(): ControllerReference
{
$reference = parent::testRendererControllerReferenceWithCompoundMatcher();

$this->assertArrayHasKey('semanticPathinfo', $reference->attributes);
$this->assertSame('/foo/bar', $reference->attributes['semanticPathinfo']);
$this->assertArrayHasKey('viewParametersString', $reference->attributes);
$this->assertSame('/(foo)/bar', $reference->attributes['viewParametersString']);
self::assertArrayHasKey('semanticPathinfo', $reference->attributes);
self::assertSame('/foo/bar', $reference->attributes['semanticPathinfo']);
self::assertArrayHasKey('viewParametersString', $reference->attributes);
self::assertSame('/(foo)/bar', $reference->attributes['viewParametersString']);

return $reference;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

abstract class BaseRenderStrategyTest extends TestCase
{
public function createRenderStrategy(
public static function createRenderStrategy(
string $typeClass,
array $fragmentRenderers,
string $defaultMethod = 'inline',
Expand Down
Loading
Loading