MC-40663: \Error object not handled properly in numerous areas.

Author: engcom-Kilo

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

@@ -3,21 +3,28 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\Cms\Controller\Adminhtml\Page;
 
-use Magento\Framework\App\Action\HttpPostActionInterface;
 use Magento\Backend\App\Action;
+use Magento\Backend\Model\View\Result\Redirect;
+use Magento\Cms\Api\Data\PageInterface;
+use Magento\Cms\Api\PageRepositoryInterface;
 use Magento\Cms\Model\Page;
+use Magento\Cms\Model\PageFactory;
+use Magento\Framework\App\Action\HttpPostActionInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\Request\DataPersistorInterface;
+use Magento\Framework\Controller\ResultInterface;
 use Magento\Framework\Exception\LocalizedException;
 
 /**
  * Save CMS page action.
  *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-class Save extends \Magento\Backend\App\Action implements HttpPostActionInterface
+class Save extends Action implements HttpPostActionInterface
 {
     /**
      * Authorization level of a basic admin session
@@ -37,47 +44,46 @@ class Save extends \Magento\Backend\App\Action implements HttpPostActionInterfac
     protected $dataPersistor;
 
     /**
-     * @var \Magento\Cms\Model\PageFactory
+     * @var PageFactory
      */
     private $pageFactory;
 
     /**
-     * @var \Magento\Cms\Api\PageRepositoryInterface
+     * @var PageRepositoryInterface
      */
     private $pageRepository;
 
     /**
      * @param Action\Context $context
      * @param PostDataProcessor $dataProcessor
      * @param DataPersistorInterface $dataPersistor
-     * @param \Magento\Cms\Model\PageFactory|null $pageFactory
-     * @param \Magento\Cms\Api\PageRepositoryInterface|null $pageRepository
+     * @param PageFactory|null $pageFactory
+     * @param PageRepositoryInterface|null $pageRepository
      */
     public function __construct(
         Action\Context $context,
         PostDataProcessor $dataProcessor,
         DataPersistorInterface $dataPersistor,
-        \Magento\Cms\Model\PageFactory $pageFactory = null,
-        \Magento\Cms\Api\PageRepositoryInterface $pageRepository = null
+        PageFactory $pageFactory = null,
+        PageRepositoryInterface $pageRepository = null
     ) {
         $this->dataProcessor = $dataProcessor;
         $this->dataPersistor = $dataPersistor;
-        $this->pageFactory = $pageFactory ?: ObjectManager::getInstance()->get(\Magento\Cms\Model\PageFactory::class);
-        $this->pageRepository = $pageRepository
-            ?: ObjectManager::getInstance()->get(\Magento\Cms\Api\PageRepositoryInterface::class);
+        $this->pageFactory = $pageFactory ?: ObjectManager::getInstance()->get(PageFactory::class);
+        $this->pageRepository = $pageRepository ?: ObjectManager::getInstance()->get(PageRepositoryInterface::class);
         parent::__construct($context);
     }
 
     /**
      * Save action
      *
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
-     * @return \Magento\Framework\Controller\ResultInterface
+     * @return ResultInterface
      */
     public function execute()
     {
         $data = $this->getRequest()->getPostValue();
-        /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
+        /** @var Redirect $resultRedirect */
         $resultRedirect = $this->resultRedirectFactory->create();
         if ($data) {
             $data = $this->dataProcessor->filter($data);
@@ -88,7 +94,7 @@ public function execute()
                 $data['page_id'] = null;
             }
 
-            /** @var \Magento\Cms\Model\Page $model */
+            /** @var Page $model */
             $model = $this->pageFactory->create();
 
             $id = $this->getRequest()->getParam('page_id');
@@ -117,7 +123,7 @@ public function execute()
             } catch (LocalizedException $e) {
                 $this->messageManager->addExceptionMessage($e->getPrevious() ?: $e);
             } catch (\Throwable $e) {
-                $this->messageManager->addExceptionMessage($e, __('Something went wrong while saving the page.'));
+                $this->messageManager->addErrorMessage(__('Something went wrong while saving the page.'));
             }
 
             $this->dataPersistor->set('cms_page', $data);
@@ -129,10 +135,10 @@ public function execute()
     /**
      * Process result redirect
      *
-     * @param \Magento\Cms\Api\Data\PageInterface $model
-     * @param \Magento\Backend\Model\View\Result\Redirect $resultRedirect
+     * @param PageInterface $model
+     * @param Redirect $resultRedirect
      * @param array $data
-     * @return \Magento\Backend\Model\View\Result\Redirect
+     * @return Redirect
      * @throws LocalizedException
      */
     private function processResultRedirect($model, $resultRedirect, $data)
@@ -149,7 +155,7 @@ private function processResultRedirect($model, $resultRedirect, $data)
                 '*/*/edit',
                 [
                     'page_id' => $newPage->getId(),
-                    '_current' => true
+                    '_current' => true,
                 ]
             );
         }

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

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
@@ -7,6 +8,7 @@
 
 namespace Magento\Cms\Test\Unit\Controller\Adminhtml\Page;
 
+use Magento\Backend\App\Action\Context;
 use Magento\Backend\Model\View\Result\Redirect;
 use Magento\Backend\Model\View\Result\RedirectFactory;
 use Magento\Cms\Api\PageRepositoryInterface;
@@ -18,7 +20,6 @@
 use Magento\Framework\App\RequestInterface;
 use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\Message\ManagerInterface;
-use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -82,13 +83,14 @@ class SaveTest extends TestCase
      */
     private $pageId = 1;
 
+    /**
+     * @inheirtDoc
+     */
     protected function setUp(): void
     {
-        $objectManager = new ObjectManager($this);
-
         $this->resultRedirectFactory = $this->getMockBuilder(RedirectFactory::class)
             ->disableOriginalConstructor()
-            ->setMethods(['create'])
+            ->onlyMethods(['create'])
             ->getMock();
         $this->resultRedirect = $this->getMockBuilder(Redirect::class)
             ->disableOriginalConstructor()
@@ -98,7 +100,7 @@ protected function setUp(): void
             ->willReturn($this->resultRedirect);
         $this->dataProcessorMock = $this->getMockBuilder(
             PostDataProcessor::class
-        )->setMethods(['filter'])->disableOriginalConstructor()
+        )->onlyMethods(['filter'])->disableOriginalConstructor()
             ->getMock();
         $this->dataPersistorMock = $this->getMockBuilder(DataPersistorInterface::class)
             ->getMock();
@@ -108,27 +110,28 @@ protected function setUp(): void
         $this->messageManagerMock = $this->getMockBuilder(ManagerInterface::class)
             ->getMockForAbstractClass();
         $this->eventManagerMock = $this->getMockBuilder(\Magento\Framework\Event\ManagerInterface::class)
-            ->setMethods(['dispatch'])
+            ->onlyMethods(['dispatch'])
             ->getMockForAbstractClass();
         $this->pageFactory = $this->getMockBuilder(PageFactory::class)
             ->disableOriginalConstructor()
-            ->setMethods(['create'])
+            ->onlyMethods(['create'])
             ->getMock();
         $this->pageRepository = $this->getMockBuilder(PageRepositoryInterface::class)
             ->disableOriginalConstructor()
             ->getMockForAbstractClass();
-        $this->saveController = $objectManager->getObject(
-            Save::class,
-            [
-                'request' => $this->requestMock,
-                'messageManager' => $this->messageManagerMock,
-                'eventManager' => $this->eventManagerMock,
-                'resultRedirectFactory' => $this->resultRedirectFactory,
-                'dataProcessor' => $this->dataProcessorMock,
-                'dataPersistor' => $this->dataPersistorMock,
-                'pageFactory' => $this->pageFactory,
-                'pageRepository' => $this->pageRepository
-            ]
+        $context = $this->getMockBuilder(Context::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $context->method('getRequest')->willReturn($this->requestMock);
+        $context->method('getMessageManager')->willReturn($this->messageManagerMock);
+        $context->method('getEventManager')->willReturn($this->eventManagerMock);
+        $context->method('getResultRedirectFactory')->willReturn($this->resultRedirectFactory);
+        $this->saveController = new Save(
+            $context,
+            $this->dataProcessorMock,
+            $this->dataPersistorMock,
+            $this->pageFactory,
+            $this->pageRepository
         );
     }
 
@@ -140,7 +143,7 @@ public function testSaveAction()
             'stores' => ['0'],
             'is_active' => true,
             'content' => '"><script>alert("cookie: "+document.cookie)</script>',
-            'back' => 'close'
+            'back' => 'close',
         ];
 
         $filteredPostData = [
@@ -149,7 +152,7 @@ public function testSaveAction()
             'stores' => ['0'],
             'is_active' => true,
             'content' => '&quot;&gt;&lt;script&gt;alert(&quot;cookie: &quot;+document.cookie)&lt;/script&gt;',
-            'back' => 'close'
+            'back' => 'close',
         ];
 
         $this->dataProcessorMock->expects($this->any())
@@ -236,7 +239,7 @@ public function testSaveAndContinue()
             'stores' => ['0'],
             'is_active' => true,
             'content' => '"><script>alert("cookie: "+document.cookie)</script>',
-            'back' => 'continue'
+            'back' => 'continue',
         ];
         $this->requestMock->expects($this->any())->method('getPostValue')->willReturn($postData);
         $this->requestMock->expects($this->atLeastOnce())
@@ -304,12 +307,13 @@ public function testSaveActionThrowsException()
         $this->pageRepository->expects($this->once())->method('getById')->with($this->pageId)->willReturn($page);
         $page->expects($this->once())->method('setData');
         $this->pageRepository->expects($this->once())->method('save')->with($page)
-            ->willThrowException(new \Exception('Error message.'));
+            ->willThrowException(new \Error('Error message.'));
 
         $this->messageManagerMock->expects($this->never())
             ->method('addSuccessMessage');
         $this->messageManagerMock->expects($this->once())
-            ->method('addExceptionMessage');
+            ->method('addErrorMessage')
+            ->with('Something went wrong while saving the page.');
 
         $this->dataPersistorMock->expects($this->any())
             ->method('set')
@@ -318,7 +322,7 @@ public function testSaveActionThrowsException()
                 [
                     'page_id' => $this->pageId,
                     'layout_update_xml' => null,
-                    'custom_layout_update_xml' => null
+                    'custom_layout_update_xml' => null,
                 ]
             );