Merge remote-tracking branch 'origin/MC-35478' into 2.3-develop-pr53

Author: rostyslav-hymon

app/code/Magento/Cms/Model/PageRepository.php

@@ -7,21 +7,27 @@
 namespace Magento\Cms\Model;
 
 use Magento\Cms\Api\Data;
+use Magento\Cms\Api\Data\PageInterface;
+use Magento\Cms\Api\Data\PageInterfaceFactory;
+use Magento\Cms\Api\Data\PageSearchResultsInterface;
 use Magento\Cms\Api\PageRepositoryInterface;
+use Magento\Cms\Model\Api\SearchCriteria\PageCollectionProcessor;
 use Magento\Cms\Model\Page\IdentityMap;
+use Magento\Cms\Model\ResourceModel\Page as ResourcePage;
+use Magento\Cms\Model\ResourceModel\Page\CollectionFactory as PageCollectionFactory;
 use Magento\Framework\Api\DataObjectHelper;
 use Magento\Framework\Api\SearchCriteria\CollectionProcessorInterface;
+use Magento\Framework\Api\SearchCriteriaInterface;
 use Magento\Framework\App\ObjectManager;
+use Magento\Framework\App\Route\Config;
 use Magento\Framework\Exception\CouldNotDeleteException;
 use Magento\Framework\Exception\CouldNotSaveException;
 use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\Reflection\DataObjectProcessor;
-use Magento\Cms\Model\ResourceModel\Page as ResourcePage;
-use Magento\Cms\Model\ResourceModel\Page\CollectionFactory as PageCollectionFactory;
 use Magento\Store\Model\StoreManagerInterface;
 
 /**
- * Class PageRepository
+ * Cms page repository
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class PageRepository implements PageRepositoryInterface
@@ -57,12 +63,12 @@ class PageRepository implements PageRepositoryInterface
     protected $dataObjectProcessor;
 
     /**
-     * @var \Magento\Cms\Api\Data\PageInterfaceFactory
+     * @var PageInterfaceFactory
      */
     protected $dataPageFactory;
 
     /**
-     * @var \Magento\Store\Model\StoreManagerInterface
+     * @var StoreManagerInterface
      */
     private $storeManager;
 
@@ -76,30 +82,37 @@ class PageRepository implements PageRepositoryInterface
      */
     private $identityMap;
 
+    /**
+     * @var Config
+     */
+    private $routeConfig;
+
     /**
      * @param ResourcePage $resource
      * @param PageFactory $pageFactory
-     * @param Data\PageInterfaceFactory $dataPageFactory
+     * @param PageInterfaceFactory $dataPageFactory
      * @param PageCollectionFactory $pageCollectionFactory
      * @param Data\PageSearchResultsInterfaceFactory $searchResultsFactory
      * @param DataObjectHelper $dataObjectHelper
      * @param DataObjectProcessor $dataObjectProcessor
      * @param StoreManagerInterface $storeManager
      * @param CollectionProcessorInterface $collectionProcessor
      * @param IdentityMap|null $identityMap
+     * @param Config|null $routeConfig
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
         ResourcePage $resource,
         PageFactory $pageFactory,
-        Data\PageInterfaceFactory $dataPageFactory,
+        PageInterfaceFactory $dataPageFactory,
         PageCollectionFactory $pageCollectionFactory,
         Data\PageSearchResultsInterfaceFactory $searchResultsFactory,
         DataObjectHelper $dataObjectHelper,
         DataObjectProcessor $dataObjectProcessor,
         StoreManagerInterface $storeManager,
         CollectionProcessorInterface $collectionProcessor = null,
-        ?IdentityMap $identityMap = null
+        ?IdentityMap $identityMap = null,
+        ?Config $routeConfig = null
     ) {
         $this->resource = $resource;
         $this->pageFactory = $pageFactory;
@@ -110,17 +123,20 @@ public function __construct(
         $this->dataObjectProcessor = $dataObjectProcessor;
         $this->storeManager = $storeManager;
         $this->collectionProcessor = $collectionProcessor ?: $this->getCollectionProcessor();
-        $this->identityMap = $identityMap ?? ObjectManager::getInstance()->get(IdentityMap::class);
+        $this->identityMap = $identityMap ?? ObjectManager::getInstance()
+                ->get(IdentityMap::class);
+        $this->routeConfig = $routeConfig ?? ObjectManager::getInstance()
+                ->get(Config::class);
     }
 
     /**
      * Validate new layout update values.
      *
-     * @param Data\PageInterface $page
+     * @param PageInterface $page
      * @return void
      * @throws \InvalidArgumentException
      */
-    private function validateLayoutUpdate(Data\PageInterface $page): void
+    private function validateLayoutUpdate(PageInterface $page): void
     {
         //Persisted data
         $savedPage = $page->getId() ? $this->getById($page->getId()) : null;
@@ -140,18 +156,19 @@ private function validateLayoutUpdate(Data\PageInterface $page): void
     /**
      * Save Page data
      *
-     * @param \Magento\Cms\Api\Data\PageInterface|Page $page
+     * @param PageInterface|Page $page
      * @return Page
      * @throws CouldNotSaveException
      */
-    public function save(\Magento\Cms\Api\Data\PageInterface $page)
+    public function save(PageInterface $page)
     {
         if ($page->getStoreId() === null) {
             $storeId = $this->storeManager->getStore()->getId();
             $page->setStoreId($storeId);
         }
         try {
             $this->validateLayoutUpdate($page);
+            $this->validateRoutesDuplication($page);
             $this->resource->save($page);
             $this->identityMap->add($page);
         } catch (\Exception $exception) {
@@ -168,7 +185,7 @@ public function save(\Magento\Cms\Api\Data\PageInterface $page)
      *
      * @param string $pageId
      * @return Page
-     * @throws \Magento\Framework\Exception\NoSuchEntityException
+     * @throws NoSuchEntityException
      */
     public function getById($pageId)
     {
@@ -187,17 +204,15 @@ public function getById($pageId)
      *
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @SuppressWarnings(PHPMD.NPathComplexity)
-     * @param \Magento\Framework\Api\SearchCriteriaInterface $criteria
-     * @return \Magento\Cms\Api\Data\PageSearchResultsInterface
+     * @param SearchCriteriaInterface $criteria
+     * @return PageSearchResultsInterface
      */
-    public function getList(\Magento\Framework\Api\SearchCriteriaInterface $criteria)
+    public function getList(SearchCriteriaInterface $criteria)
     {
-        /** @var \Magento\Cms\Model\ResourceModel\Page\Collection $collection */
         $collection = $this->pageCollectionFactory->create();
 
         $this->collectionProcessor->process($criteria, $collection);
 
-        /** @var Data\PageSearchResultsInterface $searchResults */
         $searchResults = $this->searchResultsFactory->create();
         $searchResults->setSearchCriteria($criteria);
         $searchResults->setItems($collection->getItems());
@@ -208,11 +223,11 @@ public function getList(\Magento\Framework\Api\SearchCriteriaInterface $criteria
     /**
      * Delete Page
      *
-     * @param \Magento\Cms\Api\Data\PageInterface $page
+     * @param PageInterface $page
      * @return bool
      * @throws CouldNotDeleteException
      */
-    public function delete(\Magento\Cms\Api\Data\PageInterface $page)
+    public function delete(PageInterface $page)
     {
         try {
             $this->resource->delete($page);
@@ -247,10 +262,26 @@ public function deleteById($pageId)
     private function getCollectionProcessor()
     {
         if (!$this->collectionProcessor) {
-            $this->collectionProcessor = \Magento\Framework\App\ObjectManager::getInstance()->get(
-                \Magento\Cms\Model\Api\SearchCriteria\PageCollectionProcessor::class
-            );
+            // phpstan:ignore "Class Magento\Cms\Model\Api\SearchCriteria\PageCollectionProcessor not found."
+            $this->collectionProcessor = ObjectManager::getInstance()
+                ->get(PageCollectionProcessor::class);
         }
         return $this->collectionProcessor;
     }
+
+    /**
+     * Checks that page identifier doesn't duplicate existed routes
+     *
+     * @param PageInterface $page
+     * @return void
+     * @throws CouldNotSaveException
+     */
+    private function validateRoutesDuplication($page): void
+    {
+        if ($this->routeConfig->getRouteByFrontName($page->getIdentifier(), 'frontend')) {
+            throw new CouldNotSaveException(
+                __('The value specified in the URL Key field would generate a URL that already exists.')
+            );
+        }
+    }
 }

dev/tests/integration/testsuite/Magento/Cms/Model/PageRepositoryTest.php

@@ -32,10 +32,11 @@ class PageRepositoryTest extends TestCase
     /**
      * @inheritDoc
      */
-    protected function setUp()
+    protected function setUp(): void
     {
-        $this->repo = Bootstrap::getObjectManager()->get(PageRepositoryInterface::class);
-        $this->retriever = Bootstrap::getObjectManager()->get(GetPageByIdentifierInterface::class);
+        $objectManager = Bootstrap::getObjectManager();
+        $this->repo = $objectManager->get(PageRepositoryInterface::class);
+        $this->retriever = $objectManager->get(GetPageByIdentifierInterface::class);
     }
 
     /**
@@ -48,7 +49,7 @@ protected function setUp()
     public function testSaveUpdateXml(): void
     {
         $page = $this->retriever->execute('test_custom_layout_page_1', 0);
-        $page->setTitle($page->getTitle() .'TEST');
+        $page->setTitle($page->getTitle() . 'TEST');
 
         //Is successfully saved without changes to the custom layout xml.
         $page = $this->repo->save($page);
@@ -80,4 +81,19 @@ public function testSaveUpdateXml(): void
         $this->assertEmpty($page->getCustomLayoutUpdateXml());
         $this->assertEmpty($page->getLayoutUpdateXml());
     }
+
+    /**
+     * Verifies that cms page with identifier which duplicates existed route shouldn't be saved
+     *
+     * @return void
+     * @throws \Throwable
+     * @magentoDataFixture Magento/Cms/_files/pages.php
+     */
+    public function testSaveWithRouteDuplicate(): void
+    {
+        $page = $this->retriever->execute('page100', 0);
+        $page->setIdentifier('customer');
+        $this->expectException(CouldNotSaveException::class);
+        $this->repo->save($page);
+    }
 }