Merge pull request #2270 from magento-qwerty/2.3-bugfixes-220318

Fixed issues: - MAGETWO-88209: [github] Pages are cached in browser and not updated - MAGETWO-88091: [GITHUB] [2.1] Store View Language switch leads to 404 on some cases #5416
magento · Mar 24, 2018 · c5f460e · c5f460e · bordeo · Apr 6, 2018
2 parents 068a97e + 2c3ff92
commit c5f460e
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 107 deletions.
diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Storage/DbStorage.php b/app/code/Magento/CatalogUrlRewrite/Model/Storage/DbStorage.php
@@ -12,25 +12,37 @@
 class DbStorage extends BaseDbStorage
 {
     /**
-     * @param array $data
-     * @return \Magento\Framework\DB\Select
+     * {@inheritDoc}
      */
     protected function prepareSelect(array $data)
     {
+        $metadata = [];
+        if (array_key_exists(UrlRewrite::METADATA, $data)) {
+            $metadata = $data[UrlRewrite::METADATA];
+            unset($data[UrlRewrite::METADATA]);
+        }
+
         $select = $this->connection->select();
-        $select->from(['url_rewrite' => $this->resource->getTableName('url_rewrite')])
-            ->joinLeft(
-                ['relation' => $this->resource->getTableName(Product::TABLE_NAME)],
-                'url_rewrite.url_rewrite_id = relation.url_rewrite_id'
-            )
-            ->where('url_rewrite.entity_id IN (?)', $data['entity_id'])
-            ->where('url_rewrite.entity_type = ?', $data['entity_type'])
-            ->where('url_rewrite.store_id IN (?)', $data['store_id']);
-        if (empty($data[UrlRewrite::METADATA]['category_id'])) {
+        $select->from([
+            'url_rewrite' => $this->resource->getTableName(self::TABLE_NAME)
+        ]);
+        $select->joinLeft(
+            ['relation' => $this->resource->getTableName(Product::TABLE_NAME)],
+            'url_rewrite.url_rewrite_id = relation.url_rewrite_id'
+        );
+
+        foreach ($data as $column => $value) {
+            $select->where('url_rewrite.' . $column . ' IN (?)', $value);
+        }
+        if (empty($metadata['category_id'])) {
             $select->where('relation.category_id IS NULL');
         } else {
-            $select->where('relation.category_id = ?', $data[UrlRewrite::METADATA]['category_id']);
+            $select->where(
+                'relation.category_id = ?',
+                $metadata['category_id']
+            );
         }
+
         return $select;
     }
 }
diff --git a/app/code/Magento/PageCache/etc/varnish4.vcl b/app/code/Magento/PageCache/etc/varnish4.vcl
@@ -187,6 +187,13 @@ sub vcl_deliver {
         unset resp.http.Age;
     }
 
+    # Not letting browser to cache non-static files.
+    if (resp.http.Cache-Control !~ "private" && req.url !~ "^/(pub/)?(media|static)/") {
+        set resp.http.Pragma = "no-cache";
+        set resp.http.Expires = "-1";
+        set resp.http.Cache-Control = "no-store, no-cache, must-revalidate, max-age=0";
+    }
+
     unset resp.http.X-Magento-Debug;
     unset resp.http.X-Magento-Tags;
     unset resp.http.X-Powered-By;

diff --git a/app/code/Magento/PageCache/etc/varnish5.vcl b/app/code/Magento/PageCache/etc/varnish5.vcl
@@ -188,6 +188,13 @@ sub vcl_deliver {
         unset resp.http.Age;
     }
 
+    # Not letting browser to cache non-static files.
+    if (resp.http.Cache-Control !~ "private" && req.url !~ "^/(pub/)?(media|static)/") {
+        set resp.http.Pragma = "no-cache";
+        set resp.http.Expires = "-1";
+        set resp.http.Cache-Control = "no-store, no-cache, must-revalidate, max-age=0";
+    }
+
     unset resp.http.X-Magento-Debug;
     unset resp.http.X-Magento-Tags;
     unset resp.http.X-Powered-By;

diff --git a/app/code/Magento/Store/Block/Switcher.php b/app/code/Magento/Store/Block/Switcher.php
@@ -10,7 +10,9 @@
 namespace Magento\Store\Block;
 
 use Magento\Directory\Helper\Data;
+use Magento\Store\Api\StoreResolverInterface;
 use Magento\Store\Model\Group;
+use Magento\Store\Model\Store;
 
 /**
  * @api
@@ -217,15 +219,18 @@ public function getStoreName()
     /**
      * Returns target store post data
      *
-     * @param \Magento\Store\Model\Store $store
+     * @param Store $store
      * @param array $data
      * @return string
      */
-    public function getTargetStorePostData(\Magento\Store\Model\Store $store, $data = [])
+    public function getTargetStorePostData(Store $store, $data = [])
     {
-        $data[\Magento\Store\Api\StoreResolverInterface::PARAM_NAME] = $store->getCode();
+        $data[StoreResolverInterface::PARAM_NAME] = $store->getCode();
+
+        //We need to set fromStore argument as true because
+        //it will enable proper URL rewriting during store switching.
         return $this->_postDataHelper->getPostData(
-            $store->getCurrentUrl(false),
+            $store->getCurrentUrl(true),
             $data
         );
     }

diff --git a/app/code/Magento/Store/Test/Unit/Block/SwitcherTest.php b/app/code/Magento/Store/Test/Unit/Block/SwitcherTest.php
@@ -53,7 +53,7 @@ public function testGetTargetStorePostData()
         $storeSwitchUrl = 'http://domain.com/stores/store/switch';
         $store->expects($this->atLeastOnce())
             ->method('getCurrentUrl')
-            ->with(false)
+            ->with(true)
             ->willReturn($storeSwitchUrl);
         $this->corePostDataHelper->expects($this->any())
             ->method('getPostData')

diff --git a/app/code/Magento/UrlRewrite/Controller/Router.php b/app/code/Magento/UrlRewrite/Controller/Router.php
@@ -5,10 +5,16 @@
  */
 namespace Magento\UrlRewrite\Controller;
 
+use Magento\Framework\App\RequestInterface;
 use Magento\UrlRewrite\Controller\Adminhtml\Url\Rewrite;
 use Magento\UrlRewrite\Model\OptionProvider;
 use Magento\UrlRewrite\Model\UrlFinderInterface;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
+use Magento\Framework\App\Request\Http as HttpRequest;
+use Magento\Framework\App\Response\Http as HttpResponse;
+use Magento\Framework\UrlInterface;
+use Magento\Framework\App\Action\Redirect;
+use Magento\Framework\App\ActionInterface;
 
 /**
  * UrlRewrite Controller Router
@@ -23,7 +29,7 @@ class Router implements \Magento\Framework\App\RouterInterface
     protected $actionFactory;
 
     /**
-     * @var \Magento\Framework\UrlInterface
+     * @var UrlInterface
      */
     protected $url;
 
@@ -33,7 +39,7 @@ class Router implements \Magento\Framework\App\RouterInterface
     protected $storeManager;
 
     /**
-     * @var \Magento\Framework\App\ResponseInterface
+     * @var HttpResponse
      */
     protected $response;
 
@@ -44,14 +50,14 @@ class Router implements \Magento\Framework\App\RouterInterface
 
     /**
      * @param \Magento\Framework\App\ActionFactory $actionFactory
-     * @param \Magento\Framework\UrlInterface $url
+     * @param UrlInterface $url
      * @param \Magento\Store\Model\StoreManagerInterface $storeManager
      * @param \Magento\Framework\App\ResponseInterface $response
      * @param UrlFinderInterface $urlFinder
      */
     public function __construct(
         \Magento\Framework\App\ActionFactory $actionFactory,
-        \Magento\Framework\UrlInterface $url,
+        UrlInterface $url,
         \Magento\Store\Model\StoreManagerInterface $storeManager,
         \Magento\Framework\App\ResponseInterface $response,
         UrlFinderInterface $urlFinder
@@ -64,48 +70,83 @@ public function __construct(
     }
 
     /**
-     * Match corresponding URL Rewrite and modify request
+     * Match corresponding URL Rewrite and modify request.
      *
-     * @param \Magento\Framework\App\RequestInterface $request
-     * @return \Magento\Framework\App\ActionInterface|null
+     * @param RequestInterface|HttpRequest $request
+     *
+     * @return ActionInterface|null
      */
-    public function match(\Magento\Framework\App\RequestInterface $request)
+    public function match(RequestInterface $request)
     {
         if ($fromStore = $request->getParam('___from_store')) {
+            //If we're in the process of switching stores then matching rewrite
+            //rule from previous store because the URL was not changed yet from
+            //old store's format.
             $oldStoreId = $this->storeManager->getStore($fromStore)->getId();
-            $oldRewrite = $this->getRewrite($request->getPathInfo(), $oldStoreId);
-            if ($oldRewrite) {
-                $rewrite = $this->urlFinder->findOneByData(
+            $oldRewrite = $this->getRewrite(
+                $request->getPathInfo(),
+                $oldStoreId
+            );
+            if ($oldRewrite && $oldRewrite->getRedirectType() === 0) {
+                //If there is a match and it's a correct URL then just
+                //redirecting to current store's URL equivalent,
+                //otherwise just continuing finding a rule within current store.
+                $currentRewrite = $this->urlFinder->findOneByData(
                     [
                         UrlRewrite::ENTITY_TYPE => $oldRewrite->getEntityType(),
                         UrlRewrite::ENTITY_ID => $oldRewrite->getEntityId(),
-                        UrlRewrite::STORE_ID => $this->storeManager->getStore()->getId(),
-                        UrlRewrite::IS_AUTOGENERATED => 1,
+                        UrlRewrite::STORE_ID =>
+                            $this->storeManager->getStore()->getId(),
+                        UrlRewrite::REDIRECT_TYPE => 0,
                     ]
                 );
-                if ($rewrite && $rewrite->getRequestPath() !== $oldRewrite->getRequestPath()) {
-                    return $this->redirect($request, $rewrite->getRequestPath(), OptionProvider::TEMPORARY);
+                if ($currentRewrite
+                    && $currentRewrite->getRequestPath()
+                    !== $oldRewrite->getRequestPath()
+                ) {
+                    return $this->redirect(
+                        $request,
+                        $this->url->getUrl(
+                            '',
+                            ['_direct' => $currentRewrite->getRequestPath()]
+                        ),
+                        OptionProvider::TEMPORARY
+                    );
                 }
             }
         }
-        $rewrite = $this->getRewrite($request->getPathInfo(), $this->storeManager->getStore()->getId());
+
+        $rewrite = $this->getRewrite(
+            $request->getPathInfo(),
+            $this->storeManager->getStore()->getId()
+        );
+
         if ($rewrite === null) {
+            //No rewrite rule matching current URl found, continuing with
+            //processing of this URL.
             return null;
         }
-
         if ($rewrite->getRedirectType()) {
+            //Rule requires the request to be redirected to another URL
+            //and cannot be processed further.
             return $this->processRedirect($request, $rewrite);
         }
-
-        $request->setAlias(\Magento\Framework\UrlInterface::REWRITE_REQUEST_PATH_ALIAS, $rewrite->getRequestPath());
+        //Rule provides actual URL that can be processed by a controller.
+        $request->setAlias(
+            UrlInterface::REWRITE_REQUEST_PATH_ALIAS,
+            $rewrite->getRequestPath()
+        );
         $request->setPathInfo('/' . $rewrite->getTargetPath());
-        return $this->actionFactory->create(\Magento\Framework\App\Action\Forward::class);
+        return $this->actionFactory->create(
+            \Magento\Framework\App\Action\Forward::class
+        );
     }
 
     /**
-     * @param \Magento\Framework\App\RequestInterface $request
+     * @param RequestInterface $request
      * @param UrlRewrite $rewrite
-     * @return \Magento\Framework\App\ActionInterface|null
+     *
+     * @return ActionInterface|null
      */
     protected function processRedirect($request, $rewrite)
     {
@@ -119,16 +160,17 @@ protected function processRedirect($request, $rewrite)
     }
 
     /**
-     * @param \Magento\Framework\App\RequestInterface $request
+     * @param RequestInterface|HttpRequest $request
      * @param string $url
      * @param int $code
-     * @return \Magento\Framework\App\ActionInterface
+     * @return ActionInterface
      */
     protected function redirect($request, $url, $code)
     {
         $this->response->setRedirect($url, $code);
         $request->setDispatched(true);
-        return $this->actionFactory->create(\Magento\Framework\App\Action\Redirect::class);
+
+        return $this->actionFactory->create(Redirect::class);
     }
 
     /**

diff --git a/app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php b/app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php
@@ -76,51 +76,6 @@ public function testNoRewriteExist()
         $this->assertNull($this->router->match($this->request));
     }
 
-    public function testRewriteAfterStoreSwitcher()
-    {
-        $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();
-        $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)
-            ->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)
-            ->disableOriginalConstructor()->getMock();
-        $urlRewrite->expects($this->any())->method('getRequestPath')->will($this->returnValue('new-request-path'));
-
-        $this->urlFinder->expects($this->any())->method('findOneByData')->will(
-            $this->returnValueMap([
-                [
-                    [UrlRewrite::REQUEST_PATH => 'request-path', UrlRewrite::STORE_ID => 'old-store-id'],
-                    $oldUrlRewrite,
-                ],
-                [
-                    [
-                        UrlRewrite::ENTITY_TYPE => 'entity-type',
-                        UrlRewrite::ENTITY_ID => 'entity-id',
-                        UrlRewrite::STORE_ID => 'current-store-id',
-                        UrlRewrite::IS_AUTOGENERATED => 1,
-                    ],
-                    $urlRewrite
-                ],
-            ])
-        );
-        $this->response->expects($this->once())->method('setRedirect')
-            ->with('new-request-path', OptionProvider::TEMPORARY);
-        $this->request->expects($this->once())->method('setDispatched')->with(true);
-        $this->actionFactory->expects($this->once())->method('create')
-            ->with(\Magento\Framework\App\Action\Redirect::class);
-
-        $this->router->match($this->request);
-    }
-
     public function testNoRewriteAfterStoreSwitcherWhenNoOldRewrite()
     {
         $this->request->expects($this->any())->method('getPathInfo')->will($this->returnValue('request-path'));