Merge pull request #587 from magento-nord/NORD-PR

Fixed issues:
- MAGETWO-55605: Security Issue with referrer
- MAGETWO-58222: Cron separate process
- MAGETWO-56863: [GitHub] Saving CMS page does not create URL rewrite in Magento 2.1.0 with single-store mode #5923
- MAGETWO-60037: User is able to create empty URL key for category which leads to numerous errors

Author: MomotenkoNatalia

app/code/Magento/CatalogUrlRewrite/Observer/CategoryUrlPathAutogeneratorObserver.php

@@ -42,13 +42,18 @@ public function __construct(
     /**
      * @param \Magento\Framework\Event\Observer $observer
      * @return void
+     * @throws \Magento\Framework\Exception\LocalizedException
      */
     public function execute(\Magento\Framework\Event\Observer $observer)
     {
         /** @var Category $category */
         $category = $observer->getEvent()->getCategory();
         if ($category->getUrlKey() !== false) {
-            $category->setUrlKey($this->categoryUrlPathGenerator->getUrlKey($category))
+            $resultUrlKey = $this->categoryUrlPathGenerator->getUrlKey($category);
+            if (empty($resultUrlKey)) {
+                throw new \Magento\Framework\Exception\LocalizedException(__('Invalid URL key'));
+            }
+            $category->setUrlKey($resultUrlKey)
                 ->setUrlPath($this->categoryUrlPathGenerator->getUrlPath($category));
             if (!$category->isObjectNew()) {
                 $category->getResource()->saveAttribute($category, 'url_path');

app/code/Magento/Cms/Controller/Adminhtml/Page/Save.php

@@ -30,18 +30,38 @@ class Save extends \Magento\Backend\App\Action
      */
     protected $dataPersistor;
 
+    /**
+     * @var \Magento\Cms\Model\PageFactory
+     */
+    private $pageFactory;
+
+    /**
+     * @var \Magento\Cms\Api\PageRepositoryInterface
+     */
+    private $pageRepository;
+
     /**
      * @param Action\Context $context
      * @param PostDataProcessor $dataProcessor
      * @param DataPersistorInterface $dataPersistor
+     * @param \Magento\Cms\Model\PageFactory $pageFactory
+     * @param \Magento\Cms\Api\PageRepositoryInterface $pageRepository
+     *
      */
     public function __construct(
         Action\Context $context,
         PostDataProcessor $dataProcessor,
-        DataPersistorInterface $dataPersistor
+        DataPersistorInterface $dataPersistor,
+        \Magento\Cms\Model\PageFactory $pageFactory = null,
+        \Magento\Cms\Api\PageRepositoryInterface $pageRepository = null
     ) {
         $this->dataProcessor = $dataProcessor;
         $this->dataPersistor = $dataPersistor;
+        $this->pageFactory = $pageFactory
+            ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Cms\Model\PageFactory::class);
+        $this->pageRepository = $pageRepository
+            ?: \Magento\Framework\App\ObjectManager::getInstance()
+                ->get(\Magento\Cms\Api\PageRepositoryInterface::class);
         parent::__construct($context);
     }
 
@@ -66,7 +86,7 @@ public function execute()
             }
 
             /** @var \Magento\Cms\Model\Page $model */
-            $model = $this->_objectManager->create(\Magento\Cms\Model\Page::class);
+            $model = $this->pageFactory->create();
 
             $id = $this->getRequest()->getParam('page_id');
             if ($id) {
@@ -85,7 +105,7 @@ public function execute()
             }
 
             try {
-                $model->save();
+                $this->pageRepository->save($model);
                 $this->messageManager->addSuccess(__('You saved the page.'));
                 $this->dataPersistor->clear('cms_page');
                 if ($this->getRequest()->getParam('back')) {

app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Page/SaveTest.php

@@ -13,73 +13,61 @@ class SaveTest extends \PHPUnit_Framework_TestCase
     /**
      * @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $requestMock;
+    private $requestMock;
 
     /**
      * @var \Magento\Cms\Controller\Adminhtml\Page\PostDataProcessor|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $dataProcessorMock;
+    private $dataProcessorMock;
 
     /**
      * @var \Magento\Framework\App\Request\DataPersistorInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $dataPersistorMock;
+    private $dataPersistorMock;
 
     /**
      * @var \Magento\Backend\Model\View\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $resultRedirectFactory;
+    private $resultRedirectFactory;
 
     /**
      * @var \Magento\Backend\Model\View\Result\Redirect|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $resultRedirect;
+    private $resultRedirect;
 
     /**
-     * @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
-     */
-    protected $contextMock;
-
-    /**
-     * @var \Magento\Framework\ObjectManager\ObjectManager|\PHPUnit_Framework_MockObject_MockObject
-     */
-    protected $objectManagerMock;
-
-    /**
-     * @var \Magento\Cms\Model\Page|\PHPUnit_Framework_MockObject_MockObject $pageMock
+     * @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $pageMock;
+    private $messageManagerMock;
 
     /**
-     * @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
+     * @var \Magento\Framework\Event\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $messageManagerMock;
+    private $eventManagerMock;
 
     /**
-     * @var \Magento\Framework\Event\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
+     * @var \Magento\Cms\Model\PageFactory|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $eventManagerMock;
+    private $pageFactory;
 
     /**
-     * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
+     * @var \Magento\Cms\Api\PageRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $objectManager;
+    private $pageRepository;
 
     /**
      * @var \Magento\Cms\Controller\Adminhtml\Page\Save
      */
-    protected $saveController;
+    private $saveController;
 
     /**
      * @var int
      */
-    protected $pageId = 1;
+    private $pageId = 1;
 
     protected function setUp()
     {
-        $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
-
-        $this->contextMock = $this->getMock(\Magento\Backend\App\Action\Context::class, [], [], '', false);
+        $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
 
         $this->resultRedirectFactory = $this->getMockBuilder(\Magento\Backend\Model\View\Result\RedirectFactory::class)
             ->disableOriginalConstructor()
@@ -91,69 +79,37 @@ protected function setUp()
         $this->resultRedirectFactory->expects($this->atLeastOnce())
             ->method('create')
             ->willReturn($this->resultRedirect);
-
-        $this->dataProcessorMock = $this->getMock(
-            \Magento\Cms\Controller\Adminhtml\Page\PostDataProcessor::class,
-            ['filter'],
-            [],
-            '',
-            false
-        );
-
+        $this->dataProcessorMock = $this->getMockBuilder(
+            \Magento\Cms\Controller\Adminhtml\Page\PostDataProcessor::class
+        )->setMethods(['filter'])->disableOriginalConstructor()->getMock();
         $this->dataPersistorMock = $this->getMockBuilder(\Magento\Framework\App\Request\DataPersistorInterface::class)
             ->getMock();
-
-        $this->requestMock = $this->getMockForAbstractClass(
-            \Magento\Framework\App\RequestInterface::class,
-            [],
-            '',
-            false,
-            true,
-            true,
-            ['getParam', 'getPostValue']
-        );
-
-        $this->pageMock = $this->getMockBuilder(
-            \Magento\Cms\Model\Page::class
-        )->disableOriginalConstructor()->getMock();
-
-        $this->messageManagerMock = $this->getMock(
-            \Magento\Framework\Message\ManagerInterface::class,
-            [],
-            [],
-            '',
-            false
-        );
-
-        $this->eventManagerMock = $this->getMockForAbstractClass(
-            \Magento\Framework\Event\ManagerInterface::class,
-            [],
-            '',
-            false,
-            true,
-            true,
-            ['dispatch']
-        );
-
-        $this->objectManagerMock = $this->getMockBuilder(\Magento\Framework\ObjectManager\ObjectManager::class)
+        $this->requestMock = $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class)
+            ->setMethods(['getParam', 'getPostValue'])
+            ->getMockForAbstractClass();
+        $this->messageManagerMock = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class)
+            ->getMockForAbstractClass();
+        $this->eventManagerMock = $this->getMockBuilder(\Magento\Framework\Event\ManagerInterface::class)
+            ->setMethods(['dispatch'])
+            ->getMockForAbstractClass();
+        $this->pageFactory = $this->getMockBuilder(\Magento\Cms\Model\PageFactory::class)
             ->disableOriginalConstructor()
-            ->setMethods(['get', 'create'])
+            ->setMethods(['create'])
             ->getMock();
-
-        $this->contextMock->expects($this->any())->method('getRequest')->willReturn($this->requestMock);
-        $this->contextMock->expects($this->any())->method('getObjectManager')->willReturn($this->objectManagerMock);
-        $this->contextMock->expects($this->any())->method('getMessageManager')->willReturn($this->messageManagerMock);
-        $this->contextMock->expects($this->any())->method('getEventManager')->willReturn($this->eventManagerMock);
-        $this->contextMock->expects($this->any())
-            ->method('getResultRedirectFactory')
-            ->willReturn($this->resultRedirectFactory);
-
-        $this->saveController = $this->objectManager->getObject(
+        $this->pageRepository = $this->getMockBuilder(\Magento\Cms\Api\PageRepositoryInterface::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+        $this->saveController = $objectManager->getObject(
             \Magento\Cms\Controller\Adminhtml\Page\Save::class,
             [
-                'context' => $this->contextMock,
+                'request' => $this->requestMock,
+                'messageManager' => $this->messageManagerMock,
+                'eventManager' => $this->eventManagerMock,
+                'resultRedirectFactory' => $this->resultRedirectFactory,
                 'dataProcessor' => $this->dataProcessorMock,
                 'dataPersistor' => $this->dataPersistorMock,
+                'pageFactory' => $this->pageFactory,
+                'pageRepository' => $this->pageRepository
             ]
         );
     }
@@ -190,20 +146,21 @@ public function testSaveAction()
                     ['back', null, false],
                 ]
             );
-
-        $this->objectManagerMock->expects($this->atLeastOnce())
+        $page = $this->getMockBuilder(\Magento\Cms\Model\Page::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->pageFactory->expects($this->atLeastOnce())
             ->method('create')
-            ->with($this->equalTo(\Magento\Cms\Model\Page::class))
-            ->willReturn($this->pageMock);
+            ->willReturn($page);
 
-        $this->pageMock->expects($this->any())
+        $page->expects($this->any())
             ->method('load')
             ->willReturnSelf();
-        $this->pageMock->expects($this->any())
+        $page->expects($this->any())
             ->method('getId')
             ->willReturn(true);
-        $this->pageMock->expects($this->once())->method('setData');
-        $this->pageMock->expects($this->once())->method('save');
+        $page->expects($this->once())->method('setData');
+        $this->pageRepository->expects($this->once())->method('save')->with($page);
 
         $this->dataPersistorMock->expects($this->any())
             ->method('clear')
@@ -240,20 +197,21 @@ public function testSaveAndContinue()
         $this->dataProcessorMock->expects($this->any())
             ->method('filter')
             ->willReturnArgument(0);
-
-        $this->objectManagerMock->expects($this->atLeastOnce())
+        $page = $this->getMockBuilder(\Magento\Cms\Model\Page::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->pageFactory->expects($this->atLeastOnce())
             ->method('create')
-            ->with($this->equalTo(\Magento\Cms\Model\Page::class))
-            ->willReturn($this->pageMock);
+            ->willReturn($page);
 
-        $this->pageMock->expects($this->any())
+        $page->expects($this->any())
             ->method('load')
             ->willReturnSelf();
-        $this->pageMock->expects($this->any())
+        $page->expects($this->any())
             ->method('getId')
             ->willReturn(true);
-        $this->pageMock->expects($this->once())->method('setData');
-        $this->pageMock->expects($this->once())->method('save');
+        $page->expects($this->once())->method('setData');
+        $this->pageRepository->expects($this->once())->method('save')->with($page);
 
         $this->messageManagerMock->expects($this->once())
             ->method('addSuccess')
@@ -286,20 +244,22 @@ public function testSaveActionThrowsException()
         $this->dataProcessorMock->expects($this->any())
             ->method('filter')
             ->willReturnArgument(0);
-
-        $this->objectManagerMock->expects($this->atLeastOnce())
+        $page = $this->getMockBuilder(\Magento\Cms\Model\Page::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->pageFactory->expects($this->atLeastOnce())
             ->method('create')
-            ->with($this->equalTo(\Magento\Cms\Model\Page::class))
-            ->willReturn($this->pageMock);
+            ->willReturn($page);
 
-        $this->pageMock->expects($this->any())
+        $page->expects($this->any())
             ->method('load')
             ->willReturnSelf();
-        $this->pageMock->expects($this->any())
+        $page->expects($this->any())
             ->method('getId')
             ->willReturn(true);
-        $this->pageMock->expects($this->once())->method('setData');
-        $this->pageMock->expects($this->once())->method('save')->willThrowException(new \Exception('Error message.'));
+        $page->expects($this->once())->method('setData');
+        $this->pageRepository->expects($this->once())->method('save')->with($page)
+            ->willThrowException(new \Exception('Error message.'));
 
         $this->messageManagerMock->expects($this->never())
             ->method('addSuccess');