From 37a9929ad036fd8e8309111d87f8ce7f1e0e35cc Mon Sep 17 00:00:00 2001 From: Eden Date: Fri, 6 Dec 2019 08:19:27 +0700 Subject: [PATCH 1/3] Resolve A "500 (Internal Server Error)" appears in Developer Console if Delete the image that is added to Page Content issue25893 --- .../Adminhtml/Wysiwyg/Directive.php | 17 ++++++++--- .../Adminhtml/Wysiwyg/DirectiveTest.php | 29 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php index b21ea9fd7ef7b..db53c6a415ddd 100644 --- a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php +++ b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php @@ -4,6 +4,9 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + +declare(strict_types=1); + namespace Magento\Cms\Controller\Adminhtml\Wysiwyg; use Magento\Backend\App\Action; @@ -13,6 +16,8 @@ /** * Process template text for wysiwyg editor. + * + * Class Directive */ class Directive extends Action implements HttpGetActionInterface { @@ -73,10 +78,14 @@ public function execute() /** @var Config $config */ $config = $this->_objectManager->get(Config::class); $imagePath = $config->getSkinImagePlaceholderPath(); - $image->open($imagePath); - $resultRaw->setHeader('Content-Type', $image->getMimeType()); - $resultRaw->setContents($image->getImage()); - $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e); + try { + $image->open($imagePath); + $resultRaw->setHeader('Content-Type', $image->getMimeType()); + $resultRaw->setContents($image->getImage()); + $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e); + } catch (\Exception $e) { + $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e); + } } return $resultRaw; } diff --git a/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php b/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php index 16b218ebf6493..85c48e3fa3b7d 100644 --- a/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php +++ b/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php @@ -274,4 +274,33 @@ protected function prepareExecuteTest() ->method('create') ->willReturn($this->imageAdapterMock); } + + /** + * Test Execute With Deleted Image + * + * @covers \Magento\Cms\Controller\Adminhtml\Wysiwyg\Directive::execute + */ + public function testExecuteWithDeletedImage() + { + $exception = new \Exception('epic fail'); + $placeholderPath = 'pub/static/adminhtml/Magento/backend/en_US/Magento_Cms/images/wysiwyg_skin_image.png'; + $this->prepareExecuteTest(); + + $this->imageAdapterMock->expects($this->at(0)) + ->method('open') + ->with(self::IMAGE_PATH) + ->willThrowException($exception); + $this->wysiwygConfigMock->expects($this->once()) + ->method('getSkinImagePlaceholderPath') + ->willReturn($placeholderPath); + $this->imageAdapterMock->expects($this->at(1)) + ->method('open') + ->willThrowException($exception); + + $this->loggerMock->expects($this->once()) + ->method('critical') + ->with($exception); + + $this->wysiwygDirective->execute(); + } } From 66bf8d18bf6d5f88ffd5dc674106c52a8fd066a2 Mon Sep 17 00:00:00 2001 From: Eden Date: Fri, 6 Dec 2019 08:21:24 +0700 Subject: [PATCH 2/3] Unit Test for Directive --- .../Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php b/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php index 85c48e3fa3b7d..cede3a80cb98b 100644 --- a/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php +++ b/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php @@ -286,15 +286,18 @@ public function testExecuteWithDeletedImage() $placeholderPath = 'pub/static/adminhtml/Magento/backend/en_US/Magento_Cms/images/wysiwyg_skin_image.png'; $this->prepareExecuteTest(); - $this->imageAdapterMock->expects($this->at(0)) + $this->imageAdapterMock->expects($this->any()) ->method('open') ->with(self::IMAGE_PATH) ->willThrowException($exception); + $this->wysiwygConfigMock->expects($this->once()) ->method('getSkinImagePlaceholderPath') ->willReturn($placeholderPath); - $this->imageAdapterMock->expects($this->at(1)) + + $this->imageAdapterMock->expects($this->any()) ->method('open') + ->with($placeholderPath) ->willThrowException($exception); $this->loggerMock->expects($this->once()) From ab4fa339c24d0868fa7471d347ac98ad22f4d3fe Mon Sep 17 00:00:00 2001 From: Eden Date: Tue, 10 Dec 2019 20:28:28 +0700 Subject: [PATCH 3/3] Refactor and unit test for issue 25893 --- .../Adminhtml/Wysiwyg/Directive.php | 76 ++++++++++++++----- .../Adminhtml/Wysiwyg/DirectiveTest.php | 25 +++--- 2 files changed, 67 insertions(+), 34 deletions(-) diff --git a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php index db53c6a415ddd..97d0b35a2354f 100644 --- a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php +++ b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Directive.php @@ -13,6 +13,14 @@ use Magento\Cms\Model\Template\Filter; use Magento\Cms\Model\Wysiwyg\Config; use Magento\Framework\App\Action\HttpGetActionInterface; +use Magento\Framework\Image\Adapter\AdapterInterface; +use Magento\Framework\Image\AdapterFactory; +use Psr\Log\LoggerInterface; +use Magento\Framework\Url\DecoderInterface; +use Magento\Framework\Controller\Result\Raw; +use Magento\Framework\Controller\Result\RawFactory; +use Magento\Backend\App\Action\Context; +use Magento\Framework\App\ObjectManager; /** * Process template text for wysiwyg editor. @@ -30,34 +38,68 @@ class Directive extends Action implements HttpGetActionInterface const ADMIN_RESOURCE = 'Magento_Cms::media_gallery'; /** - * @var \Magento\Framework\Url\DecoderInterface + * @var DecoderInterface */ protected $urlDecoder; /** - * @var \Magento\Framework\Controller\Result\RawFactory + * @var RawFactory */ protected $resultRawFactory; /** - * @param Action\Context $context - * @param \Magento\Framework\Url\DecoderInterface $urlDecoder - * @param \Magento\Framework\Controller\Result\RawFactory $resultRawFactory + * @var LoggerInterface + */ + private $logger; + + /** + * @var AdapterFactory + */ + private $adapterFactory; + + /** + * @var Config + */ + private $config; + + /** + * @var Filter + */ + private $filter; + + /** + * Constructor + * + * @param Context $context + * @param DecoderInterface $urlDecoder + * @param RawFactory $resultRawFactory + * @param AdapterFactory|null $adapterFactory + * @param LoggerInterface|null $logger + * @param Config|null $config + * @param Filter|null $filter */ public function __construct( - Action\Context $context, - \Magento\Framework\Url\DecoderInterface $urlDecoder, - \Magento\Framework\Controller\Result\RawFactory $resultRawFactory + Context $context, + DecoderInterface $urlDecoder, + RawFactory $resultRawFactory, + AdapterFactory $adapterFactory = null, + LoggerInterface $logger = null, + Config $config = null, + Filter $filter = null ) { parent::__construct($context); $this->urlDecoder = $urlDecoder; $this->resultRawFactory = $resultRawFactory; + $this->adapterFactory = $adapterFactory ?: ObjectManager::getInstance()->get(AdapterFactory::class); + $this->logger = $logger ?: ObjectManager::getInstance()->get(LoggerInterface::class); + $this->config = $config ?: ObjectManager::getInstance()->get(Config::class); + $this->filter = $filter ?: ObjectManager::getInstance()->get(Filter::class); } /** * Template directives callback * - * @return \Magento\Framework\Controller\Result\Raw + * @return Raw */ public function execute() { @@ -65,26 +107,24 @@ public function execute() $directive = $this->urlDecoder->decode($directive); try { /** @var Filter $filter */ - $filter = $this->_objectManager->create(Filter::class); - $imagePath = $filter->filter($directive); - /** @var \Magento\Framework\Image\Adapter\AdapterInterface $image */ - $image = $this->_objectManager->get(\Magento\Framework\Image\AdapterFactory::class)->create(); - /** @var \Magento\Framework\Controller\Result\Raw $resultRaw */ + $imagePath = $this->filter->filter($directive); + /** @var AdapterInterface $image */ + $image = $this->adapterFactory->create(); + /** @var Raw $resultRaw */ $resultRaw = $this->resultRawFactory->create(); $image->open($imagePath); $resultRaw->setHeader('Content-Type', $image->getMimeType()); $resultRaw->setContents($image->getImage()); } catch (\Exception $e) { /** @var Config $config */ - $config = $this->_objectManager->get(Config::class); - $imagePath = $config->getSkinImagePlaceholderPath(); + $imagePath = $this->config->getSkinImagePlaceholderPath(); try { $image->open($imagePath); $resultRaw->setHeader('Content-Type', $image->getMimeType()); $resultRaw->setContents($image->getImage()); - $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e); + $this->logger->warning($e); } catch (\Exception $e) { - $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e); + $this->logger->warning($e); } } return $resultRaw; diff --git a/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php b/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php index cede3a80cb98b..5fea276225622 100644 --- a/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php +++ b/app/code/Magento/Cms/Test/Unit/Controller/Adminhtml/Wysiwyg/DirectiveTest.php @@ -151,7 +151,11 @@ protected function setUp() [ 'context' => $this->actionContextMock, 'urlDecoder' => $this->urlDecoderMock, - 'resultRawFactory' => $this->rawFactoryMock + 'resultRawFactory' => $this->rawFactoryMock, + 'adapterFactory' => $this->imageAdapterFactoryMock, + 'logger' => $this->loggerMock, + 'config' => $this->wysiwygConfigMock, + 'filter' => $this->templateFilterMock ] ); } @@ -228,7 +232,7 @@ public function testExecuteException() ->method('getImage') ->willReturn($imageBody); $this->loggerMock->expects($this->once()) - ->method('critical') + ->method('warning') ->with($exception); $this->rawFactoryMock->expects($this->any()) ->method('create') @@ -253,23 +257,12 @@ protected function prepareExecuteTest() ->method('decode') ->with($directiveParam) ->willReturn($directive); - $this->objectManagerMock->expects($this->once()) - ->method('create') - ->with(\Magento\Cms\Model\Template\Filter::class) - ->willReturn($this->templateFilterMock); + $this->templateFilterMock->expects($this->once()) ->method('filter') ->with($directive) ->willReturn(self::IMAGE_PATH); - $this->objectManagerMock->expects($this->any()) - ->method('get') - ->willReturnMap( - [ - [\Magento\Framework\Image\AdapterFactory::class, $this->imageAdapterFactoryMock], - [\Magento\Cms\Model\Wysiwyg\Config::class, $this->wysiwygConfigMock], - [\Psr\Log\LoggerInterface::class, $this->loggerMock] - ] - ); + $this->imageAdapterFactoryMock->expects($this->once()) ->method('create') ->willReturn($this->imageAdapterMock); @@ -301,7 +294,7 @@ public function testExecuteWithDeletedImage() ->willThrowException($exception); $this->loggerMock->expects($this->once()) - ->method('critical') + ->method('warning') ->with($exception); $this->wysiwygDirective->execute();