IBX-5821: Fixed an issue where incomplete request object was passed o…

…ver to route Matcher (#319)
ibexa · Jan 30, 2024 · 9150cc6 · 9150cc6
1 parent dcb200b
commit 9150cc6
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 57 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -26395,11 +26395,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
@@ -26410,11 +26405,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
@@ -26425,11 +26415,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
@@ -26475,11 +26460,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

diff --git a/src/bundle/Core/Routing/DefaultRouter.php b/src/bundle/Core/Routing/DefaultRouter.php
@@ -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)

diff --git a/tests/bundle/Core/Routing/DefaultRouterTest.php b/tests/bundle/Core/Routing/DefaultRouterTest.php
@@ -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
@@ -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])
@@ -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));
     }
 
@@ -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));
     }
 
@@ -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__));
     }
@@ -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
@@ -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);
 
@@ -296,7 +313,7 @@ public function testGetContextBySimplifiedRequest($uri)
      *
      * @see testGetContextBySimplifiedRequest
      *
-     * @return array
+     * @phpstan-return array<array{string}>
      */
     public function providerGetContextBySimplifiedRequest()
     {