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.

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

IBX-5821: Fixed an issue where incomplete request object was passed over to route Matcher #319

Merged
merged 3 commits into from
Jan 30, 2024
Merged
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
20 changes: 0 additions & 20 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -26390,11 +26390,6 @@ parameters:
count: 2
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouterTest\\:\\:generateRouter\\(\\) has parameter \\$mockedMethods with no value type specified in iterable type array\\.$#"
count: 1
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouterTest\\:\\:getExpectedRequestContext\\(\\) has no return type specified\\.$#"
count: 1
Expand All @@ -26405,11 +26400,6 @@ parameters:
count: 1
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouterTest\\:\\:getRouterClass\\(\\) has no return type specified\\.$#"
count: 1
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouterTest\\:\\:providerGenerateNoSiteAccess\\(\\) has no return type specified\\.$#"
count: 1
Expand All @@ -26420,11 +26410,6 @@ parameters:
count: 1
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouterTest\\:\\:providerGetContextBySimplifiedRequest\\(\\) return type has no value type specified in iterable type array\\.$#"
count: 1
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouterTest\\:\\:testGenerateNoSiteAccess\\(\\) has no return type specified\\.$#"
count: 1
Expand Down Expand Up @@ -26470,11 +26455,6 @@ parameters:
count: 2
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Unable to resolve the template type T in call to method PHPUnit\\\\Framework\\\\TestCase\\:\\:getMockBuilder\\(\\)$#"
count: 1
path: tests/bundle/Core/Routing/DefaultRouterTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\Bundle\\\\Core\\\\Routing\\\\JsRouting\\\\ExposedRoutesExtractorTest\\:\\:getDataForTestGetBaseUrl\\(\\) return type has no value type specified in iterable type iterable\\.$#"
count: 1
Expand Down
10 changes: 9 additions & 1 deletion src/bundle/Core/Routing/DefaultRouter.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,15 @@ public function setSiteAccessRouter(SiteAccessRouterInterface $siteAccessRouter)

public function matchRequest(Request $request)
{
return $this->match($request->attributes->get('semanticPathinfo', $request->getPathInfo()));
if ($request->attributes->has('semanticPathinfo')) {
$request = $request->duplicate();
$request->server->set(
'REQUEST_URI',
$request->attributes->get('semanticPathinfo')
);
}

return parent::matchRequest($request);
}

public function generate($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH)
Expand Down
89 changes: 53 additions & 36 deletions tests/bundle/Core/Routing/DefaultRouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Routing\Matcher\UrlMatcherInterface;
use Symfony\Component\Routing\RequestContext;

class DefaultRouterTest extends TestCase
Expand All @@ -37,19 +38,22 @@ protected function setUp(): void
$this->requestContext = new RequestContext();
}

/**
* @return class-string<\Ibexa\Bundle\Core\Routing\DefaultRouter>
*/
protected function getRouterClass()
{
return DefaultRouter::class;
}

/**
* @param array $mockedMethods
* @param array<string> $mockedMethods
*
* @return \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Bundle\Core\Routing\DefaultRouter
* @return \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter
*/
protected function generateRouter(array $mockedMethods = [])
{
/** @var \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Bundle\Core\Routing\DefaultRouter $router */
/** @var \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter $router */
$router = $this
->getMockBuilder($this->getRouterClass())
->setConstructorArgs([$this->container, 'foo', [], $this->requestContext])
Expand All @@ -67,15 +71,21 @@ public function testMatchRequestWithSemanticPathinfo()
$request = Request::create($pathinfo);
$request->attributes->set('semanticPathinfo', $semanticPathinfo);

/** @var \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Bundle\Core\Routing\DefaultRouter $router */
$router = $this->generateRouter(['match']);

/** @var \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter $router */
$router = $this->generateRouter(['getMatcher']);
$matchedParameters = ['_controller' => 'AcmeBundle:myAction'];
$router
->expects($this->once())

$matcher = $this->createMock(UrlMatcherInterface::class);
$matcher->expects(self::once())
->method('match')
->with($semanticPathinfo)
->will($this->returnValue($matchedParameters));
->willReturn($matchedParameters);

$router
->expects(self::once())
->method('getMatcher')
->willReturn($matcher);

$this->assertSame($matchedParameters, $router->matchRequest($request));
}

Expand All @@ -88,13 +98,20 @@ public function testMatchRequestRegularPathinfo()

$this->configResolver->expects($this->never())->method('getParameter');

/** @var \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Bundle\Core\Routing\DefaultRouter $router */
$router = $this->generateRouter(['match']);
$router
->expects($this->once())
/** @var \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter $router */
$router = $this->generateRouter(['getMatcher']);

$matcher = $this->createMock(UrlMatcherInterface::class);
$matcher->expects(self::once())
->method('match')
->with($pathinfo)
->will($this->returnValue($matchedParameters));
->willReturn($matchedParameters);

$router
->expects(self::once())
->method('getMatcher')
->willReturn($matcher);

$this->assertSame($matchedParameters, $router->matchRequest($request));
}

Expand All @@ -105,17 +122,17 @@ public function testGenerateNoSiteAccess($url)
{
$generator = $this->createMock(UrlGeneratorInterface::class);
$generator
->expects($this->once())
->expects(self::once())
->method('generate')
->with(__METHOD__)
->will($this->returnValue($url));
->willReturn($url);

/** @var \Ibexa\Bundle\Core\Routing\DefaultRouter|\PHPUnit\Framework\MockObject\MockObject $router */
/** @var \Ibexa\Bundle\Core\Routing\DefaultRouter&\PHPUnit\Framework\MockObject\MockObject $router */
$router = $this->generateRouter(['getGenerator']);
$router
->expects($this->any())
->expects(self::any())
->method('getGenerator')
->will($this->returnValue($generator));
->willReturn($generator);

$this->assertSame($url, $router->generate(__METHOD__));
}
Expand Down Expand Up @@ -147,28 +164,28 @@ public function testGenerateWithSiteAccess($urlGenerated, $relevantUri, $expecte
$nonSiteAccessAwareRoutes = ['_dontwantsiteaccess'];
$generator = $this->createMock(UrlGeneratorInterface::class);
$generator
->expects($this->once())
->expects(self::once())
->method('generate')
->with($routeName)
->will($this->returnValue($urlGenerated));
->willReturn($urlGenerated);

/** @var \Ibexa\Bundle\Core\Routing\DefaultRouter|\PHPUnit\Framework\MockObject\MockObject $router */
/** @var \Ibexa\Bundle\Core\Routing\DefaultRouter&\PHPUnit\Framework\MockObject\MockObject $router */
$router = $this->generateRouter(['getGenerator']);
$router
->expects($this->any())
->expects(self::any())
->method('getGenerator')
->will($this->returnValue($generator));
->willReturn($generator);

// If matcher is URILexer, we make it act as it's supposed to, prepending the siteaccess.
if ($isMatcherLexer) {
$matcher = $this->createMock(SiteAccess\URILexer::class);
// Route is siteaccess aware, we're expecting analyseLink() to be called
if (!in_array($routeName, $nonSiteAccessAwareRoutes)) {
$matcher
->expects($this->once())
->expects(self::once())
->method('analyseLink')
->with($relevantUri)
->will($this->returnValue("/$saName$relevantUri"));
->willReturn("/$saName$relevantUri");
} else {
// Non-siteaccess aware route, it's not supposed to be analysed
$matcher
Expand Down Expand Up @@ -235,27 +252,27 @@ public function testGenerateReverseSiteAccessMatch()
]
);
$versatileMatcher
->expects($this->once())
->expects(self::once())
->method('getRequest')
->will($this->returnValue($simplifiedRequest));
->willReturn($simplifiedRequest);
$siteAccessRouter
->expects($this->once())
->expects(self::once())
->method('matchByName')
->with($siteAccessName)
->will($this->returnValue(new SiteAccess($siteAccessName, 'foo', $versatileMatcher)));
->willReturn(new SiteAccess($siteAccessName, 'foo', $versatileMatcher));

$generator = $this->createMock(UrlGeneratorInterface::class);
$generator
->expects($this->at(0))
->expects(self::at(0))
->method('setContext')
->with($this->isInstanceOf(RequestContext::class));
->with(self::isInstanceOf(RequestContext::class));
$generator
->expects($this->at(1))
->expects(self::at(1))
->method('generate')
->with($routeName)
->will($this->returnValue($urlGenerated));
->willReturn($urlGenerated);
$generator
->expects($this->at(2))
->expects(self::at(2))
->method('setContext')
->with($this->requestContext);

Expand Down Expand Up @@ -296,7 +313,7 @@ public function testGetContextBySimplifiedRequest($uri)
*
* @see testGetContextBySimplifiedRequest
*
* @return array
* @phpstan-return array<array{string}>
*/
public function providerGetContextBySimplifiedRequest()
{
Expand Down
Loading