From d1164548da45b9e2bb8ec22140061c1220748c23 Mon Sep 17 00:00:00 2001 From: Vasilii Burlacu Date: Thu, 21 Jun 2018 22:53:20 +0300 Subject: [PATCH 1/4] #16273: Fix bug in method getUrlInStore() of product model # Method $product->getUrlInStore() returning extremely long URLs, could be a bug (cherry picked from commit 7558ac0a478af8fc008228077f9e5380612d4138) --- .../Magento/Store/Url/Plugin/RouteParamsResolver.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php b/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php index c4f8a31430963..325e621c8a113 100644 --- a/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php +++ b/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php @@ -51,6 +51,8 @@ public function __construct( * @param \Magento\Framework\Url\RouteParamsResolver $subject * @param array $data * @param bool $unsetOldParams + * @throws \Magento\Framework\Exception\NoSuchEntityException + * * @return array */ public function beforeSetRouteParams( @@ -63,13 +65,19 @@ public function beforeSetRouteParams( unset($data['_scope']); } if (isset($data['_scope_to_url']) && (bool)$data['_scope_to_url'] === true) { - $storeCode = $subject->getScope() ?: $this->storeManager->getStore()->getCode(); + /** @var Store $currentScope */ + $currentScope = $subject->getScope(); + $storeCode = $currentScope && $currentScope instanceof Store ? + $currentScope->getCode() : + $this->storeManager->getStore()->getCode(); + $useStoreInUrl = $this->scopeConfig->getValue( Store::XML_PATH_STORE_IN_URL, StoreScopeInterface::SCOPE_STORE, $storeCode ); - if (!$useStoreInUrl && !$this->storeManager->hasSingleStore()) { + + if ($useStoreInUrl && !$this->storeManager->hasSingleStore()) { $this->queryParamsResolver->setQueryParam('___store', $storeCode); } } From 3a39a1674bf06d80d7df3969ecc78f84abd218c1 Mon Sep 17 00:00:00 2001 From: Vasilii Burlacu Date: Sat, 30 Jun 2018 15:04:59 +0300 Subject: [PATCH 2/4] Modified RouteParamsResolverTest according to latest bugfixes on tested class --- .../Url/Plugin/RouteParamsResolverTest.php | 48 +++++++++++++------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php b/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php index ed9ac7ebe438a..51e3a9dc32e13 100644 --- a/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php +++ b/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php @@ -22,6 +22,11 @@ class RouteParamsResolverTest extends \PHPUnit\Framework\TestCase */ protected $queryParamsResolverMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Store\Model\Store + */ + protected $storeMock; + /** * @var \Magento\Store\Url\Plugin\RouteParamsResolver */ @@ -30,7 +35,19 @@ class RouteParamsResolverTest extends \PHPUnit\Framework\TestCase protected function setUp() { $this->scopeConfigMock = $this->createMock(\Magento\Framework\App\Config\ScopeConfigInterface::class); + + $this->storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) + ->setMethods(['getCode']) + ->disableOriginalConstructor() + ->getMock(); + $this->storeMock->expects($this->any())->method('getCode')->willReturn('custom_store'); + $this->storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); + $this->storeManagerMock + ->expects($this->once()) + ->method('getStore') + ->willReturn($this->storeMock); + $this->queryParamsResolverMock = $this->createMock(\Magento\Framework\Url\QueryParamsResolverInterface::class); $this->model = new \Magento\Store\Url\Plugin\RouteParamsResolver( $this->scopeConfigMock, @@ -42,6 +59,8 @@ protected function setUp() public function testBeforeSetRouteParamsScopeInParams() { $storeCode = 'custom_store'; + $data = ['_scope' => $storeCode, '_scope_to_url' => true]; + $this->scopeConfigMock ->expects($this->once()) ->method('getValue') @@ -52,7 +71,7 @@ public function testBeforeSetRouteParamsScopeInParams() ) ->will($this->returnValue(false)); $this->storeManagerMock->expects($this->any())->method('hasSingleStore')->willReturn(false); - $data = ['_scope' => $storeCode, '_scope_to_url' => true]; + /** @var \PHPUnit_Framework_MockObject_MockObject $routeParamsResolverMock */ $routeParamsResolverMock = $this->getMockBuilder(\Magento\Framework\Url\RouteParamsResolver::class) ->setMethods(['setScope', 'getScope']) @@ -61,7 +80,8 @@ public function testBeforeSetRouteParamsScopeInParams() $routeParamsResolverMock->expects($this->once())->method('setScope')->with($storeCode); $routeParamsResolverMock->expects($this->once())->method('getScope')->willReturn($storeCode); - $this->queryParamsResolverMock->expects($this->once())->method('setQueryParam')->with('___store', $storeCode); + $this->queryParamsResolverMock->expects($this->never())->method('setQueryParam'); + $this->model->beforeSetRouteParams( $routeParamsResolverMock, @@ -72,6 +92,8 @@ public function testBeforeSetRouteParamsScopeInParams() public function testBeforeSetRouteParamsScopeUseStoreInUrl() { $storeCode = 'custom_store'; + $data = ['_scope' => $storeCode, '_scope_to_url' => true]; + $this->scopeConfigMock ->expects($this->once()) ->method('getValue') @@ -81,8 +103,9 @@ public function testBeforeSetRouteParamsScopeUseStoreInUrl() $storeCode ) ->will($this->returnValue(true)); + $this->storeManagerMock->expects($this->any())->method('hasSingleStore')->willReturn(false); - $data = ['_scope' => $storeCode, '_scope_to_url' => true]; + /** @var \PHPUnit_Framework_MockObject_MockObject $routeParamsResolverMock */ $routeParamsResolverMock = $this->getMockBuilder(\Magento\Framework\Url\RouteParamsResolver::class) ->setMethods(['setScope', 'getScope']) @@ -91,7 +114,7 @@ public function testBeforeSetRouteParamsScopeUseStoreInUrl() $routeParamsResolverMock->expects($this->once())->method('setScope')->with($storeCode); $routeParamsResolverMock->expects($this->once())->method('getScope')->willReturn($storeCode); - $this->queryParamsResolverMock->expects($this->never())->method('setQueryParam'); + $this->queryParamsResolverMock->expects($this->once())->method('setQueryParam')->with('___store', $storeCode); $this->model->beforeSetRouteParams( $routeParamsResolverMock, @@ -102,6 +125,8 @@ public function testBeforeSetRouteParamsScopeUseStoreInUrl() public function testBeforeSetRouteParamsSingleStore() { $storeCode = 'custom_store'; + $data = ['_scope' => $storeCode, '_scope_to_url' => true]; + $this->scopeConfigMock ->expects($this->once()) ->method('getValue') @@ -112,7 +137,7 @@ public function testBeforeSetRouteParamsSingleStore() ) ->will($this->returnValue(false)); $this->storeManagerMock->expects($this->any())->method('hasSingleStore')->willReturn(true); - $data = ['_scope' => $storeCode, '_scope_to_url' => true]; + /** @var \PHPUnit_Framework_MockObject_MockObject $routeParamsResolverMock */ $routeParamsResolverMock = $this->getMockBuilder(\Magento\Framework\Url\RouteParamsResolver::class) ->setMethods(['setScope', 'getScope']) @@ -132,6 +157,8 @@ public function testBeforeSetRouteParamsSingleStore() public function testBeforeSetRouteParamsNoScopeInParams() { $storeCode = 'custom_store'; + $data = ['_scope_to_url' => true]; + $this->scopeConfigMock ->expects($this->once()) ->method('getValue') @@ -140,17 +167,10 @@ public function testBeforeSetRouteParamsNoScopeInParams() \Magento\Store\Model\ScopeInterface::SCOPE_STORE, $storeCode ) - ->will($this->returnValue(false)); + ->will($this->returnValue(true)); + $this->storeManagerMock->expects($this->any())->method('hasSingleStore')->willReturn(false); - /** @var \PHPUnit_Framework_MockObject_MockObject| $routeParamsResolverMock */ - $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) - ->setMethods(['getCode']) - ->disableOriginalConstructor() - ->getMock(); - $storeMock->expects($this->any())->method('getCode')->willReturn($storeCode); - $this->storeManagerMock->expects($this->any())->method('getStore')->willReturn($storeMock); - $data = ['_scope_to_url' => true]; /** @var \PHPUnit_Framework_MockObject_MockObject $routeParamsResolverMock */ $routeParamsResolverMock = $this->getMockBuilder(\Magento\Framework\Url\RouteParamsResolver::class) ->setMethods(['setScope', 'getScope']) From 16ae409bbf5892b40b1a009eca76e2f1bbb73037 Mon Sep 17 00:00:00 2001 From: Vishal Gelani Date: Sun, 1 Jul 2018 11:33:38 +0530 Subject: [PATCH 3/4] Update RouteParamsResolverTest.php --- .../Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php b/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php index 51e3a9dc32e13..f31acd40a2e69 100644 --- a/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php +++ b/app/code/Magento/Store/Test/Unit/Url/Plugin/RouteParamsResolverTest.php @@ -82,7 +82,6 @@ public function testBeforeSetRouteParamsScopeInParams() $this->queryParamsResolverMock->expects($this->never())->method('setQueryParam'); - $this->model->beforeSetRouteParams( $routeParamsResolverMock, $data From 4dc6aba5daa877d5a39fcb8423c7f7a66875e8fd Mon Sep 17 00:00:00 2001 From: Vasilii Burlacu Date: Wed, 18 Jul 2018 09:15:18 +0300 Subject: [PATCH 4/4] Change checking of StoreInterface instead of Store in RouteParamsResolver.php --- app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php b/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php index 325e621c8a113..468352af78cbc 100644 --- a/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php +++ b/app/code/Magento/Store/Url/Plugin/RouteParamsResolver.php @@ -6,6 +6,7 @@ namespace Magento\Store\Url\Plugin; use \Magento\Store\Model\Store; +use \Magento\Store\Api\Data\StoreInterface; use \Magento\Store\Model\ScopeInterface as StoreScopeInterface; /** @@ -65,9 +66,9 @@ public function beforeSetRouteParams( unset($data['_scope']); } if (isset($data['_scope_to_url']) && (bool)$data['_scope_to_url'] === true) { - /** @var Store $currentScope */ + /** @var StoreInterface $currentScope */ $currentScope = $subject->getScope(); - $storeCode = $currentScope && $currentScope instanceof Store ? + $storeCode = $currentScope && $currentScope instanceof StoreInterface ? $currentScope->getCode() : $this->storeManager->getStore()->getCode();