diff --git a/app/code/Magento/UrlRewrite/Controller/Router.php b/app/code/Magento/UrlRewrite/Controller/Router.php index 9c1e184cbc506..9f0278d5655f4 100644 --- a/app/code/Magento/UrlRewrite/Controller/Router.php +++ b/app/code/Magento/UrlRewrite/Controller/Router.php @@ -77,7 +77,7 @@ public function __construct( public function match(RequestInterface $request) { $rewrite = $this->getRewrite( - $request->getPathInfo(), + $this->getNormalisedRequestPath($request), $this->storeManager->getStore()->getId() ); @@ -153,4 +153,30 @@ protected function getRewrite($requestPath, $storeId) ] ); } + + /** + * Get normalized request path + * + * @param RequestInterface|HttpRequest $request + * @return string + */ + private function getNormalisedRequestPath(RequestInterface $request) + { + $path = $request->getPathInfo(); + /** + * If request contains query params then we need to trim a slash in end of the path. + * For example: + * the original request is: http://my-host.com/category-url-key.html/?color=black + * where the original path is: category-url-key.html/ + * and the result path will be: category-url-key.html + * + * It need to except a redirect like this: + * http://my-host.com/category-url-key.html/?color=black => http://my-host.com/category-url-key.html + */ + if (!empty($path) && $request->getQuery()->count()) { + $path = rtrim($path, '/'); + } + + return $path; + } } diff --git a/app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php b/app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php index b22fc55938468..6eea8b962bf9f 100644 --- a/app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php +++ b/app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php @@ -3,61 +3,88 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento\UrlRewrite\Test\Unit\Controller; use Magento\Framework\App\Action\Forward; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Framework\UrlInterface; use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; use Magento\Store\Model\Store; +use PHPUnit\Framework\MockObject\MockObject; +use Zend\Stdlib\ParametersInterface; /** + * Test class for UrlRewrite Controller Router + * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class RouterTest extends \PHPUnit\Framework\TestCase { - /** @var \Magento\UrlRewrite\Controller\Router */ - protected $router; + /** + * @var \Magento\UrlRewrite\Controller\Router + */ + private $router; + + /** + * @var \Magento\Framework\App\ActionFactory|MockObject + */ + private $actionFactory; - /** @var \Magento\Framework\App\ActionFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $actionFactory; + /** + * @var UrlInterface|MockObject + */ + private $url; - /** @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $url; + /** + * @var \Magento\Store\Model\StoreManagerInterface|MockObject + */ + private $storeManager; - /** @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $storeManager; + /** + * @var Store|MockObject + */ + private $store; - /** @var \Magento\Store\Model\Store|\PHPUnit_Framework_MockObject_MockObject */ - protected $store; + /** + * @var \Magento\Framework\App\ResponseInterface|MockObject + */ + private $response; - /** @var \Magento\Framework\App\ResponseInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $response; + /** + * @var \Magento\Framework\App\RequestInterface|MockObject + */ + private $request; - /** @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $request; + /** + * @var ParametersInterface|MockObject + */ + private $requestQuery; - /** @var \Magento\UrlRewrite\Model\UrlFinderInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $urlFinder; + /** + * @var \Magento\UrlRewrite\Model\UrlFinderInterface|MockObject + */ + private $urlFinder; /** - * @return void + * @inheritDoc */ protected function setUp() { $objectManager = new ObjectManager($this); $this->actionFactory = $this->createMock(\Magento\Framework\App\ActionFactory::class); - $this->url = $this->createMock(\Magento\Framework\UrlInterface::class); + $this->url = $this->createMock(UrlInterface::class); $this->storeManager = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); $this->response = $this->createPartialMock( \Magento\Framework\App\ResponseInterface::class, ['setRedirect', 'sendResponse'] ); + $this->requestQuery = $this->createMock(ParametersInterface::class); $this->request = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class) ->disableOriginalConstructor()->getMock(); + $this->request->method('getQuery')->willReturn($this->requestQuery); $this->urlFinder = $this->createMock(\Magento\UrlRewrite\Model\UrlFinderInterface::class); $this->store = $this->getMockBuilder( - \Magento\Store\Model\Store::class + Store::class )->disableOriginalConstructor()->getMock(); $this->router = $objectManager->getObject( @@ -166,17 +193,17 @@ public function testNoRewriteAfterStoreSwitcherWhenNoOldRewrite() $this->request->expects($this->any())->method('getPathInfo')->will($this->returnValue('request-path')); $this->request->expects($this->any())->method('getParam')->with('___from_store') ->will($this->returnValue('old-store')); - $oldStore = $this->getMockBuilder(\Magento\Store\Model\Store::class)->disableOriginalConstructor()->getMock(); + $oldStore = $this->getMockBuilder(Store::class)->disableOriginalConstructor()->getMock(); $this->storeManager->expects($this->any())->method('getStore') ->will($this->returnValueMap([['old-store', $oldStore], [null, $this->store]])); $oldStore->expects($this->any())->method('getId')->will($this->returnValue('old-store-id')); $this->store->expects($this->any())->method('getId')->will($this->returnValue('current-store-id')); - $oldUrlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $oldUrlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $oldUrlRewrite->expects($this->any())->method('getEntityType')->will($this->returnValue('entity-type')); $oldUrlRewrite->expects($this->any())->method('getEntityId')->will($this->returnValue('entity-id')); $oldUrlRewrite->expects($this->any())->method('getRequestPath')->will($this->returnValue('request-path')); - $urlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $urlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $urlRewrite->expects($this->any())->method('getRequestPath')->will($this->returnValue('request-path')); @@ -191,17 +218,17 @@ public function testNoRewriteAfterStoreSwitcherWhenOldRewriteEqualsToNewOne() $this->request->expects($this->any())->method('getPathInfo')->will($this->returnValue('request-path')); $this->request->expects($this->any())->method('getParam')->with('___from_store') ->will($this->returnValue('old-store')); - $oldStore = $this->getMockBuilder(\Magento\Store\Model\Store::class)->disableOriginalConstructor()->getMock(); + $oldStore = $this->getMockBuilder(Store::class)->disableOriginalConstructor()->getMock(); $this->storeManager->expects($this->any())->method('getStore') ->will($this->returnValueMap([['old-store', $oldStore], [null, $this->store]])); $oldStore->expects($this->any())->method('getId')->will($this->returnValue('old-store-id')); $this->store->expects($this->any())->method('getId')->will($this->returnValue('current-store-id')); - $oldUrlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $oldUrlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $oldUrlRewrite->expects($this->any())->method('getEntityType')->will($this->returnValue('entity-type')); $oldUrlRewrite->expects($this->any())->method('getEntityId')->will($this->returnValue('entity-id')); $oldUrlRewrite->expects($this->any())->method('getRequestPath')->will($this->returnValue('old-request-path')); - $urlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $urlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $urlRewrite->expects($this->any())->method('getRequestPath')->will($this->returnValue('old-request-path')); @@ -234,7 +261,7 @@ public function testNoRewriteAfterStoreSwitcherWhenOldRewriteEqualsToNewOne() public function testMatchWithRedirect() { $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($this->store)); - $urlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $urlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $urlRewrite->expects($this->any())->method('getRedirectType')->will($this->returnValue('redirect-code')); $urlRewrite->expects($this->any())->method('getTargetPath')->will($this->returnValue('target-path')); @@ -256,7 +283,7 @@ public function testMatchWithRedirect() public function testMatchWithCustomInternalRedirect() { $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($this->store)); - $urlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $urlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $urlRewrite->expects($this->any())->method('getEntityType')->will($this->returnValue('custom')); $urlRewrite->expects($this->any())->method('getRedirectType')->will($this->returnValue('redirect-code')); @@ -278,7 +305,7 @@ public function testMatchWithCustomInternalRedirect() public function testMatchWithCustomExternalRedirect($targetPath) { $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($this->store)); - $urlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $urlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $urlRewrite->expects($this->any())->method('getEntityType')->will($this->returnValue('custom')); $urlRewrite->expects($this->any())->method('getRedirectType')->will($this->returnValue('redirect-code')); @@ -310,7 +337,7 @@ public function externalRedirectTargetPathDataProvider() public function testMatch() { $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($this->store)); - $urlRewrite = $this->getMockBuilder(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class) + $urlRewrite = $this->getMockBuilder(UrlRewrite::class) ->disableOriginalConstructor()->getMock(); $urlRewrite->expects($this->any())->method('getRedirectType')->will($this->returnValue(0)); $urlRewrite->expects($this->any())->method('getTargetPath')->will($this->returnValue('target-path')); @@ -318,10 +345,60 @@ public function testMatch() $this->urlFinder->expects($this->any())->method('findOneByData')->will($this->returnValue($urlRewrite)); $this->request->expects($this->once())->method('setPathInfo')->with('/target-path'); $this->request->expects($this->once())->method('setAlias') - ->with(\Magento\Framework\UrlInterface::REWRITE_REQUEST_PATH_ALIAS, 'request-path'); + ->with(UrlInterface::REWRITE_REQUEST_PATH_ALIAS, 'request-path'); $this->actionFactory->expects($this->once())->method('create') ->with(\Magento\Framework\App\Action\Forward::class); $this->router->match($this->request); } + + /** + * Test to match corresponding URL Rewrite on request with query params + * + * @param string $originalRequestPath + * @param string $requestPath + * @param int $countOfQueryParams + * @dataProvider matchWithQueryParamsDataProvider + */ + public function testMatchWithQueryParams(string $originalRequestPath, string $requestPath, int $countOfQueryParams) + { + $targetPath = 'target-path'; + + $this->storeManager->method('getStore')->willReturn($this->store); + $urlRewrite = $this->createMock(UrlRewrite::class); + $urlRewrite->method('getRedirectType')->willReturn(0); + $urlRewrite->method('getTargetPath')->willReturn($targetPath); + $urlRewrite->method('getRequestPath')->willReturn($requestPath); + $this->urlFinder->method('findOneByData') + ->with([UrlRewrite::REQUEST_PATH => $requestPath, UrlRewrite::STORE_ID => $this->store->getId()]) + ->willReturn($urlRewrite); + + $this->requestQuery->method('count')->willReturn($countOfQueryParams); + $this->request->method('getPathInfo') + ->willReturn($originalRequestPath); + $this->request->expects($this->once()) + ->method('setPathInfo') + ->with('/' . $targetPath); + $this->request->expects($this->once()) + ->method('setAlias') + ->with(UrlInterface::REWRITE_REQUEST_PATH_ALIAS, $requestPath); + $this->actionFactory->expects($this->once()) + ->method('create') + ->with(Forward::class); + + $this->router->match($this->request); + } + + /** + * Data provider for Test to match corresponding URL Rewrite on request with query params + * + * @return array + */ + public function matchWithQueryParamsDataProvider(): array + { + return [ + ['/category.html/', 'category.html/', 0], + ['/category.html/', 'category.html', 1], + ]; + } }