From 7fe1cef01ee8c81d633f42f0e807b89cbc96ba2d Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 13:59:44 +0530 Subject: [PATCH 01/39] Move predispatch and postdispatch events from abstract controller to action interface plugin See https://github.com/magento/community-features/issues/9 for further information. --- app/code/Magento/Store/etc/di.xml | 3 + .../Framework/App/ControllerActionTest.php | 174 ++++++++++++++++++ .../InheritanceBasedBackendAction.php | 25 +++ .../InheritanceBasedFrontendAction.php | 26 +++ .../TestStubs/InterfaceOnlyBackendAction.php | 24 +++ .../TestStubs/InterfaceOnlyFrontendAction.php | 42 +++++ .../Magento/Framework/App/Action/Action.php | 20 -- .../App/Action/Plugin/EventDispatchPlugin.php | 100 ++++++++++ .../App/Test/Unit/Action/ActionTest.php | 51 +---- 9 files changed, 402 insertions(+), 63 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php create mode 100644 dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php create mode 100644 dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php create mode 100644 dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php create mode 100644 dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php create mode 100644 lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php diff --git a/app/code/Magento/Store/etc/di.xml b/app/code/Magento/Store/etc/di.xml index 27133de270e2f..e36a3dbe09351 100644 --- a/app/code/Magento/Store/etc/di.xml +++ b/app/code/Magento/Store/etc/di.xml @@ -59,6 +59,9 @@ + + + diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php new file mode 100644 index 0000000000000..eb2d2de77f56a --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -0,0 +1,174 @@ +create(Event\ManagerInterface::class); + $eventManagerSpy = new class($originalEventManager) implements Event\ManagerInterface + { + /** + * @var Event\ManagerInterface + */ + private $delegate; + + /** + * @var array[]; + */ + private $dispatchedEvents = []; + + public function __construct(Event\ManagerInterface $delegate) + { + $this->delegate = $delegate; + } + + public function dispatch($eventName, array $data = []) + { + $this->dispatchedEvents[$eventName][] = [$eventName, $data]; + $this->delegate->dispatch($eventName, $data); + } + + public function spyOnDispatchedEvent(string $eventName): array + { + return $this->dispatchedEvents[$eventName] ?? []; + } + }; + + $objectManager->addSharedInstance($eventManagerSpy, Event\Manager\Proxy::class); + } + + private function assertEventDispatchCount($eventName, $expectedCount): void + { + $message = sprintf('Event %s was expected to be dispatched %d time(s).', $eventName, $expectedCount); + $this->assertCount($expectedCount, $this->getEventManager()->spyOnDispatchedEvent($eventName), $message); + } + + /** + * @return \Magento\Framework\App\Request\Http + */ + private function getRequest(): RequestInterface + { + return ObjectManager::getInstance()->get(\Magento\Framework\App\Request\Http::class); + } + + private function fakeBackendAuthentication() + { + $objectManager = ObjectManager::getInstance(); + $objectManager->get(BackendUrl::class)->turnOffSecretKey(); + + $auth = $objectManager->get(BackendAuth::class); + $auth->login(TestFramework::ADMIN_NAME, TestFramework::ADMIN_PASSWORD); + } + + private function configureRequestForAction(string $route, string $actionPath, string $actionName) + { + $request = $this->getRequest(); + + $request->setRouteName($route); + $request->setControllerName($actionPath); + $request->setActionName($actionName); + $request->setDispatched(); + } + + private function getEventManager(): Event\ManagerInterface + { + return ObjectManager::getInstance()->get(Event\ManagerInterface::class); + } + + private function assertPreAndPostDispatchEventsAreDispatched() + { + $this->assertEventDispatchCount('controller_action_predispatch', 1); + $this->assertEventDispatchCount('controller_action_predispatch_testroute', 1); + $this->assertEventDispatchCount('controller_action_predispatch_testroute_actionpath_actionname', 1); + $this->assertEventDispatchCount('controller_action_postdispatch_testroute_actionpath_actionname', 1); + $this->assertEventDispatchCount('controller_action_postdispatch_testroute', 1); + $this->assertEventDispatchCount('controller_action_postdispatch', 1); + } + + /** + * @magentoAppArea frontend + * @magentoAppIsolation enabled + */ + public function testInheritanceBasedFrontendActionDispatchesEvents() + { + $this->setupEventManagerSpy(); + + /** @var InheritanceBasedFrontendAction $action */ + $action = ObjectManager::getInstance()->create(InheritanceBasedFrontendAction::class); + $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); + + $action->dispatch($this->getRequest()); + + $this->assertPreAndPostDispatchEventsAreDispatched(); + } + + /** + * @magentoAppArea frontend + * @magentoAppIsolation enabled + */ + public function testInterfaceOnlyFrontendActionDispatchesEvents() + { + $this->setupEventManagerSpy(); + + /** @var InterfaceOnlyFrontendAction $action */ + $action = ObjectManager::getInstance()->create(InterfaceOnlyFrontendAction::class); + $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); + + $action->execute(); + + $this->assertPreAndPostDispatchEventsAreDispatched(); + } + + /** + * @magentoAppArea adminhtml + * @magentoAppIsolation enabled + */ + public function testInheritanceBasedAdminhtmlActionDispatchesEvents() + { + $this->fakeBackendAuthentication(); + + $this->setupEventManagerSpy(); + + /** @var InheritanceBasedBackendAction $action */ + $action = ObjectManager::getInstance()->create(InheritanceBasedBackendAction::class); + $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); + + $action->dispatch($this->getRequest()); + + $this->assertPreAndPostDispatchEventsAreDispatched(); + } + + /** + * @magentoAppArea adminhtml + * @magentoAppIsolation enabled + */ + public function testInterfaceOnlyAdminhtmlActionDispatchesEvents() + { + $this->setupEventManagerSpy(); + + /** @var InterfaceOnlyBackendAction $action */ + $action = ObjectManager::getInstance()->create(InterfaceOnlyBackendAction::class); + $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); + + $action->execute(); + + $this->assertPreAndPostDispatchEventsAreDispatched(); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php new file mode 100644 index 0000000000000..59cc9d2edccf7 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php @@ -0,0 +1,25 @@ +pageFactory = $pageFactory; + } + + public function execute() + { + return $this->pageFactory->create(); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php new file mode 100644 index 0000000000000..725cccc838a6b --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php @@ -0,0 +1,26 @@ +pageFactory = $pageFactory; + } + + public function execute() + { + return $this->pageFactory->create(); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php new file mode 100644 index 0000000000000..52362601d1965 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php @@ -0,0 +1,24 @@ +pageFactory = $pageFactory; + } + + public function execute() + { + return $this->pageFactory->create(); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php new file mode 100644 index 0000000000000..56eed45ea56bf --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php @@ -0,0 +1,42 @@ +pageFactory = $pageFactory; + $this->request = $request; + } + + public function execute() + { + return $this->pageFactory->create(); + } + + /** + * This method is a workaround for the interface violation where core code expects + * actions to extend the AbstractAction :((((( + * + * @return RequestInterface + */ + public function getRequest(): RequestInterface + { + return $this->request; + } +} diff --git a/lib/internal/Magento/Framework/App/Action/Action.php b/lib/internal/Magento/Framework/App/Action/Action.php index b68e7e873be6a..7412965be14cc 100644 --- a/lib/internal/Magento/Framework/App/Action/Action.php +++ b/lib/internal/Magento/Framework/App/Action/Action.php @@ -92,32 +92,12 @@ public function dispatch(RequestInterface $request) { $this->_request = $request; $profilerKey = 'CONTROLLER_ACTION:' . $request->getFullActionName(); - $eventParameters = ['controller_action' => $this, 'request' => $request]; - $this->_eventManager->dispatch('controller_action_predispatch', $eventParameters); - $this->_eventManager->dispatch('controller_action_predispatch_' . $request->getRouteName(), $eventParameters); - $this->_eventManager->dispatch( - 'controller_action_predispatch_' . $request->getFullActionName(), - $eventParameters - ); \Magento\Framework\Profiler::start($profilerKey); $result = null; if ($request->isDispatched() && !$this->_actionFlag->get('', self::FLAG_NO_DISPATCH)) { \Magento\Framework\Profiler::start('action_body'); $result = $this->execute(); - \Magento\Framework\Profiler::start('postdispatch'); - if (!$this->_actionFlag->get('', self::FLAG_NO_POST_DISPATCH)) { - $this->_eventManager->dispatch( - 'controller_action_postdispatch_' . $request->getFullActionName(), - $eventParameters - ); - $this->_eventManager->dispatch( - 'controller_action_postdispatch_' . $request->getRouteName(), - $eventParameters - ); - $this->_eventManager->dispatch('controller_action_postdispatch', $eventParameters); - } - \Magento\Framework\Profiler::stop('postdispatch'); \Magento\Framework\Profiler::stop('action_body'); } \Magento\Framework\Profiler::stop($profilerKey); diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php new file mode 100644 index 0000000000000..8c80aa1676a11 --- /dev/null +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -0,0 +1,100 @@ +request = $request; + $this->eventManager = $eventManager; + } + + public function beforeExecute(ActionInterface $subject) + { + $this->dispatchPreDispatchEvents($subject); + } + + /** + * @param ActionInterface $subject + * @return mixed[] + */ + private function getEventParameters(ActionInterface $subject): array + { + return ['controller_action' => $subject, 'request' => $this->request]; + } + + /** + * @param ActionInterface $subject + * @param ResultInterface|Response|null $result + * @return ResultInterface|Response|null + */ + public function afterExecute(ActionInterface $subject, $result) + { + if (! $this->isSetActionNoPostDispatchFlag($subject)) { + $this->dispatchPostDispatchEvents($subject); + } + + return $result; + } + + /** + * @param ActionInterface $subject + * @return bool + */ + private function isSetActionNoPostDispatchFlag(ActionInterface $subject): bool + { + return $subject instanceof Action && $subject->getActionFlag()->get('', Action::FLAG_NO_POST_DISPATCH); + } + + /** + * @param ActionInterface $action + */ + private function dispatchPreDispatchEvents(ActionInterface $action) + { + $this->eventManager->dispatch('controller_action_predispatch', $this->getEventParameters($action)); + $this->eventManager->dispatch( + 'controller_action_predispatch_' . $this->request->getRouteName(), + $this->getEventParameters($action) + ); + $this->eventManager->dispatch( + 'controller_action_predispatch_' . $this->request->getFullActionName(), + $this->getEventParameters($action) + ); + } + + /** + * @param ActionInterface $action + */ + private function dispatchPostDispatchEvents(ActionInterface $action) + { + $this->eventManager->dispatch( + 'controller_action_postdispatch_' . $this->request->getFullActionName(), + $this->getEventParameters($action) + ); + $this->eventManager->dispatch( + 'controller_action_postdispatch_' . $this->request->getRouteName(), + $this->getEventParameters($action) + ); + $this->eventManager->dispatch('controller_action_postdispatch', $this->getEventParameters($action)); + } +} diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php index ebd72f5badccf..cc0a43a703985 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php @@ -85,14 +85,13 @@ protected function setUp() $this->_eventManagerMock = $this->createMock(\Magento\Framework\Event\ManagerInterface::class); $this->_actionFlagMock = $this->createMock(\Magento\Framework\App\ActionFlag::class); $this->_redirectMock = $this->createMock(\Magento\Framework\App\Response\RedirectInterface::class); - $this->_requestMock = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class) - ->disableOriginalConstructor()->getMock(); + $this->_requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class); $this->_responseMock = $this->createMock(\Magento\Framework\App\ResponseInterface::class); $this->pageConfigMock = $this->createPartialMock(\Magento\Framework\View\Page\Config::class, ['getConfig']); $this->viewMock = $this->createMock(\Magento\Framework\App\ViewInterface::class); $this->viewMock->expects($this->any())->method('getPage')->will($this->returnValue($this->pageConfigMock)); - $this->pageConfigMock->expects($this->any())->method('getConfig')->will($this->returnValue(1)); + $this->pageConfigMock->expects($this->any())->method('getConfig')->willReturn(1); $this->objectManagerHelper = new ObjectManagerHelper($this); $this->action = $this->objectManagerHelper->getObject( @@ -111,29 +110,12 @@ protected function setUp() public function testDispatchPostDispatch() { - $this->_requestMock->expects($this->exactly(3))->method('getFullActionName')->will( - $this->returnValue(self::FULL_ACTION_NAME) - ); - $this->_requestMock->expects($this->exactly(2))->method('getRouteName')->will( - $this->returnValue(self::ROUTE_NAME) - ); - $expectedEventParameters = ['controller_action' => $this->action, 'request' => $this->_requestMock]; - $this->_eventManagerMock->expects($this->at(0))->method('dispatch')->with( - 'controller_action_predispatch', - $expectedEventParameters - ); - $this->_eventManagerMock->expects($this->at(1))->method('dispatch')->with( - 'controller_action_predispatch_' . self::ROUTE_NAME, - $expectedEventParameters - ); - $this->_eventManagerMock->expects($this->at(2))->method('dispatch')->with( - 'controller_action_predispatch_' . self::FULL_ACTION_NAME, - $expectedEventParameters - ); - - $this->_requestMock->expects($this->once())->method('isDispatched')->will($this->returnValue(true)); - $this->_actionFlagMock->expects($this->at(0))->method('get')->with('', Action::FLAG_NO_DISPATCH)->will( - $this->returnValue(false) + $this->_requestMock->method('getFullActionName')->willReturn(self::FULL_ACTION_NAME); + $this->_requestMock->method('getRouteName')->willReturn(self::ROUTE_NAME); + $this->_requestMock->method('isDispatched')->willReturn(true); + $this->_actionFlagMock->method('get')->willReturnMap( + ['', Action::FLAG_NO_DISPATCH, false], + ['', Action::FLAG_NO_POST_DISPATCH] ); // _forward expectations @@ -151,23 +133,6 @@ public function testDispatchPostDispatch() self::$actionParams ); - $this->_actionFlagMock->expects($this->at(1))->method('get')->with('', Action::FLAG_NO_POST_DISPATCH)->will( - $this->returnValue(false) - ); - - $this->_eventManagerMock->expects($this->at(3))->method('dispatch')->with( - 'controller_action_postdispatch_' . self::FULL_ACTION_NAME, - $expectedEventParameters - ); - $this->_eventManagerMock->expects($this->at(4))->method('dispatch')->with( - 'controller_action_postdispatch_' . self::ROUTE_NAME, - $expectedEventParameters - ); - $this->_eventManagerMock->expects($this->at(5))->method('dispatch')->with( - 'controller_action_postdispatch', - $expectedEventParameters - ); - $this->assertEquals($this->_responseMock, $this->action->dispatch($this->_requestMock)); } } From 12052d01e3dc90490360418705605da77154fcb9 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 14:42:46 +0530 Subject: [PATCH 02/39] Remove LoD violation which prohibits non inheritance based action controllers --- app/code/Magento/Customer/Model/Visitor.php | 15 +++++++++++++- .../Customer/Test/Unit/Model/VisitorTest.php | 14 +++++++++---- .../TestStubs/InterfaceOnlyFrontendAction.php | 20 +------------------ 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/app/code/Magento/Customer/Model/Visitor.php b/app/code/Magento/Customer/Model/Visitor.php index 4624dd8b6bcf5..abc4feb4a9ff5 100644 --- a/app/code/Magento/Customer/Model/Visitor.php +++ b/app/code/Magento/Customer/Model/Visitor.php @@ -200,7 +200,7 @@ public function saveByRequest($observer) public function isModuleIgnored($observer) { if (is_array($this->ignores) && $observer) { - $curModule = $observer->getEvent()->getControllerAction()->getRequest()->getRouteName(); + $curModule = $this->getRequest()->getRouteName(); if (isset($this->ignores[$curModule])) { return true; } @@ -315,4 +315,17 @@ public function getOnlineInterval() ); return $configValue ?: static::DEFAULT_ONLINE_MINUTES_INTERVAL; } + + /** + * @return \Magento\Framework\App\RequestInterface|\Magento\Framework\App\Request\Http + */ + private function getRequest() + { + if (null === $this->request) { + $this->request = \Magento\Framework\App\ObjectManager::getInstance()->create( + \Magento\Framework\App\RequestInterface::class + ); + } + return $this->request; + } } diff --git a/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php b/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php index ec787b9d3c873..debccda26ba75 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php @@ -39,6 +39,11 @@ class VisitorTest extends \PHPUnit\Framework\TestCase */ protected $session; + /** + * @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject + */ + private $request; + protected function setUp() { $this->registry = $this->createMock(\Magento\Framework\Registry::class); @@ -46,6 +51,7 @@ protected function setUp() ->disableOriginalConstructor() ->setMethods(['getSessionId', 'getVisitorData', 'setVisitorData']) ->getMock(); + $this->request = $this->createMock(\Magento\Framework\App\Request\Http::class); $this->objectManagerHelper = new ObjectManagerHelper($this); @@ -69,6 +75,7 @@ protected function setUp() 'registry' => $this->registry, 'session' => $this->session, 'resource' => $this->resource, + 'request' => $this->request, ] ); @@ -101,12 +108,11 @@ public function testIsModuleIgnored() 'session' => $this->session, 'resource' => $this->resource, 'ignores' => ['test_route_name' => true], + 'request' => $this->request, ] ); - $request = new \Magento\Framework\DataObject(['route_name' => 'test_route_name']); - $action = new \Magento\Framework\DataObject(['request' => $request]); - $event = new \Magento\Framework\DataObject(['controller_action' => $action]); - $observer = new \Magento\Framework\DataObject(['event' => $event]); + $this->request->method('getRouteName')->willReturn('test_route_name'); + $observer = new \Magento\Framework\DataObject(); $this->assertTrue($this->visitor->isModuleIgnored($observer)); } diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php index 56eed45ea56bf..c95b8e0091f30 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php @@ -3,7 +3,6 @@ namespace Magento\Framework\App\TestStubs; use Magento\Framework\App\ActionInterface; -use Magento\Framework\App\RequestInterface; use Magento\Framework\View\Result\PageFactory; class InterfaceOnlyFrontendAction implements ActionInterface @@ -13,30 +12,13 @@ class InterfaceOnlyFrontendAction implements ActionInterface */ private $pageFactory; - /** - * @var RequestInterface - */ - private $request; - - public function __construct(PageFactory $pageFactory, RequestInterface $request) + public function __construct(PageFactory $pageFactory) { $this->pageFactory = $pageFactory; - $this->request = $request; } public function execute() { return $this->pageFactory->create(); } - - /** - * This method is a workaround for the interface violation where core code expects - * actions to extend the AbstractAction :((((( - * - * @return RequestInterface - */ - public function getRequest(): RequestInterface - { - return $this->request; - } } From c39b1d2fd1b195562ac87f86a9e8024b81c2d694 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 14:48:48 +0530 Subject: [PATCH 03/39] Ignore CouplingBetweenObjects in controller action integration test case --- .../testsuite/Magento/Framework/App/ControllerActionTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php index eb2d2de77f56a..bbc2f7481cd53 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -14,6 +14,9 @@ use Magento\TestFramework\ObjectManager; use PHPUnit\Framework\TestCase; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class ControllerActionTest extends TestCase { public function setupEventManagerSpy(): void From a12b2d097ec04dfa59d2905b6a33ac0b42b88f51 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 15:08:48 +0530 Subject: [PATCH 04/39] Use ActionFlag as direct dependency instead of LoD violation --- .../App/Action/Plugin/EventDispatchPlugin.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index 8c80aa1676a11..d7738c6d9561b 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -3,6 +3,7 @@ namespace Magento\Framework\App\Action\Plugin; use Magento\Framework\App\Action\Action; +use Magento\Framework\App\ActionFlag; use Magento\Framework\App\ActionInterface; use Magento\Framework\App\Request\Http; use Magento\Framework\App\RequestInterface; @@ -22,11 +23,16 @@ class EventDispatchPlugin */ private $eventManager; - public function __construct(RequestInterface $request, ManagerInterface $eventManager) + /** + * @var ActionFlag + */ + private $actionFlag; + + public function __construct(RequestInterface $request, ManagerInterface $eventManager, ActionFlag $actionFlag) { - \assert($request instanceof Http, sprintf('The request has to be an instance of %s.', Http::class)); $this->request = $request; $this->eventManager = $eventManager; + $this->actionFlag = $actionFlag; } public function beforeExecute(ActionInterface $subject) @@ -63,7 +69,7 @@ public function afterExecute(ActionInterface $subject, $result) */ private function isSetActionNoPostDispatchFlag(ActionInterface $subject): bool { - return $subject instanceof Action && $subject->getActionFlag()->get('', Action::FLAG_NO_POST_DISPATCH); + return $this->actionFlag->get('', Action::FLAG_NO_POST_DISPATCH); } /** From 777311391bf2bd227500ab3c418f6b8e1573f20e Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 15:10:40 +0530 Subject: [PATCH 05/39] Remove void return type to keep code PHP 7.0 compatible --- .../testsuite/Magento/Framework/App/ControllerActionTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php index bbc2f7481cd53..6c9691576fb8d 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -19,7 +19,7 @@ */ class ControllerActionTest extends TestCase { - public function setupEventManagerSpy(): void + public function setupEventManagerSpy() { /** @var ObjectManager $objectManager */ $objectManager = ObjectManager::getInstance(); @@ -57,7 +57,7 @@ public function spyOnDispatchedEvent(string $eventName): array $objectManager->addSharedInstance($eventManagerSpy, Event\Manager\Proxy::class); } - private function assertEventDispatchCount($eventName, $expectedCount): void + private function assertEventDispatchCount($eventName, $expectedCount) { $message = sprintf('Event %s was expected to be dispatched %d time(s).', $eventName, $expectedCount); $this->assertCount($expectedCount, $this->getEventManager()->spyOnDispatchedEvent($eventName), $message); From 57bb6d390af5c34b4e81285133c7c3cf95b04f12 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 16:25:01 +0530 Subject: [PATCH 06/39] Prohibit calling controller execute if the NO_DISPATCHED flag was set --- app/code/Magento/Store/etc/di.xml | 1 + .../Framework/App/ControllerActionTest.php | 34 +++++++++++++++++-- .../InheritanceBasedFrontendAction.php | 11 ++++++ .../TestStubs/InterfaceOnlyFrontendAction.php | 11 ++++++ .../Plugin/ActionFlagNoDispatchPlugin.php | 31 +++++++++++++++++ .../App/Action/Plugin/EventDispatchPlugin.php | 3 +- 6 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php diff --git a/app/code/Magento/Store/etc/di.xml b/app/code/Magento/Store/etc/di.xml index e36a3dbe09351..611ae44221a48 100644 --- a/app/code/Magento/Store/etc/di.xml +++ b/app/code/Magento/Store/etc/di.xml @@ -61,6 +61,7 @@ + diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php index 6c9691576fb8d..5127d64c689de 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -16,6 +16,7 @@ /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.UnusedLocalVariable) */ class ControllerActionTest extends TestCase { @@ -68,10 +69,10 @@ private function assertEventDispatchCount($eventName, $expectedCount) */ private function getRequest(): RequestInterface { - return ObjectManager::getInstance()->get(\Magento\Framework\App\Request\Http::class); + return ObjectManager::getInstance()->get(\Magento\TestFramework\Request::class); } - private function fakeBackendAuthentication() + private function fakeAuthenticatedBackendRequest() { $objectManager = ObjectManager::getInstance(); $objectManager->get(BackendUrl::class)->turnOffSecretKey(); @@ -145,7 +146,7 @@ public function testInterfaceOnlyFrontendActionDispatchesEvents() */ public function testInheritanceBasedAdminhtmlActionDispatchesEvents() { - $this->fakeBackendAuthentication(); + $this->fakeAuthenticatedBackendRequest(); $this->setupEventManagerSpy(); @@ -174,4 +175,31 @@ public function testInterfaceOnlyAdminhtmlActionDispatchesEvents() $this->assertPreAndPostDispatchEventsAreDispatched(); } + + /** + * @magentoAppArea frontend + * @magentoAppIsolation enabled + */ + public function testSettingTheNoDispatchActionFlagProhibitsExecuteAndPostdispatchEvents() + { + $this->setupEventManagerSpy(); + + /** @var InterfaceOnlyFrontendAction $action */ + $action = ObjectManager::getInstance()->create(InterfaceOnlyFrontendAction::class); + $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); + + /** @var ActionFlag $actionFlag */ + $actionFlag = ObjectManager::getInstance()->get(ActionFlag::class); + $actionFlag->set('', ActionInterface::FLAG_NO_DISPATCH, true); + + $action->execute(); + + $this->assertFalse($action->isExecuted(), 'The controller execute() method was not expected to be called.'); + $this->assertEventDispatchCount('controller_action_predispatch', 1); + $this->assertEventDispatchCount('controller_action_predispatch_testroute', 1); + $this->assertEventDispatchCount('controller_action_predispatch_testroute_actionpath_actionname', 1); + $this->assertEventDispatchCount('controller_action_postdispatch_testroute_actionpath_actionname', 0); + $this->assertEventDispatchCount('controller_action_postdispatch_testroute', 0); + $this->assertEventDispatchCount('controller_action_postdispatch', 0); + } } diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php index 725cccc838a6b..654a0659aa77f 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php @@ -13,6 +13,11 @@ class InheritanceBasedFrontendAction extends Action */ private $pageFactory; + /** + * @var bool + */ + private $executeWasCalled = false; + public function __construct(Context $context, PageFactory $pageFactory) { parent::__construct($context); @@ -21,6 +26,12 @@ public function __construct(Context $context, PageFactory $pageFactory) public function execute() { + $this->executeWasCalled = true; return $this->pageFactory->create(); } + + public function isExecuted(): bool + { + return $this->executeWasCalled; + } } diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php index c95b8e0091f30..81e37d89ce332 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php @@ -12,6 +12,11 @@ class InterfaceOnlyFrontendAction implements ActionInterface */ private $pageFactory; + /** + * @var bool + */ + private $executeWasCalled = false; + public function __construct(PageFactory $pageFactory) { $this->pageFactory = $pageFactory; @@ -19,6 +24,12 @@ public function __construct(PageFactory $pageFactory) public function execute() { + $this->executeWasCalled = true; return $this->pageFactory->create(); } + + public function isExecuted(): bool + { + return $this->executeWasCalled; + } } diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php new file mode 100644 index 0000000000000..fa335a1e2fd84 --- /dev/null +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -0,0 +1,31 @@ +actionFlag = $actionFlag; + $this->response = $response; + } + + public function aroundExecute(ActionInterface $subject, callable $proceed) + { + return $this->actionFlag->get('', ActionInterface::FLAG_NO_DISPATCH) ? $this->response : $proceed(); + } +} diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index d7738c6d9561b..5409d8999c2dc 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -69,7 +69,8 @@ public function afterExecute(ActionInterface $subject, $result) */ private function isSetActionNoPostDispatchFlag(ActionInterface $subject): bool { - return $this->actionFlag->get('', Action::FLAG_NO_POST_DISPATCH); + return $this->actionFlag->get('', Action::FLAG_NO_DISPATCH) || + $this->actionFlag->get('', Action::FLAG_NO_POST_DISPATCH); } /** From f4c41c4dd56c67ac0d1a46c469ff8d6deedff7c3 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 16:42:02 +0530 Subject: [PATCH 07/39] Avoid duplicate annotations when it can be used on class --- .../Magento/Framework/App/ControllerActionTest.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php index 5127d64c689de..b74ed98d36864 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -9,12 +9,13 @@ use Magento\Framework\App\TestStubs\InterfaceOnlyBackendAction; use Magento\Framework\App\TestStubs\InterfaceOnlyFrontendAction; use Magento\Framework\Event; -use Magento\Security\Model\Plugin\Auth as SecurityAuth; use Magento\TestFramework\Bootstrap as TestFramework; use Magento\TestFramework\ObjectManager; +use Magento\TestFramework\Request as TestHttpRequest; use PHPUnit\Framework\TestCase; /** + * @magentoAppIsolation enabled * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.UnusedLocalVariable) */ @@ -65,11 +66,11 @@ private function assertEventDispatchCount($eventName, $expectedCount) } /** - * @return \Magento\Framework\App\Request\Http + * @return TestHttpRequest */ private function getRequest(): RequestInterface { - return ObjectManager::getInstance()->get(\Magento\TestFramework\Request::class); + return ObjectManager::getInstance()->get(TestHttpRequest::class); } private function fakeAuthenticatedBackendRequest() @@ -108,7 +109,6 @@ private function assertPreAndPostDispatchEventsAreDispatched() /** * @magentoAppArea frontend - * @magentoAppIsolation enabled */ public function testInheritanceBasedFrontendActionDispatchesEvents() { @@ -125,7 +125,6 @@ public function testInheritanceBasedFrontendActionDispatchesEvents() /** * @magentoAppArea frontend - * @magentoAppIsolation enabled */ public function testInterfaceOnlyFrontendActionDispatchesEvents() { @@ -142,7 +141,6 @@ public function testInterfaceOnlyFrontendActionDispatchesEvents() /** * @magentoAppArea adminhtml - * @magentoAppIsolation enabled */ public function testInheritanceBasedAdminhtmlActionDispatchesEvents() { @@ -161,7 +159,6 @@ public function testInheritanceBasedAdminhtmlActionDispatchesEvents() /** * @magentoAppArea adminhtml - * @magentoAppIsolation enabled */ public function testInterfaceOnlyAdminhtmlActionDispatchesEvents() { @@ -178,7 +175,6 @@ public function testInterfaceOnlyAdminhtmlActionDispatchesEvents() /** * @magentoAppArea frontend - * @magentoAppIsolation enabled */ public function testSettingTheNoDispatchActionFlagProhibitsExecuteAndPostdispatchEvents() { From ae4d87c009564bf933d8866c398f53352457ef83 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 16:42:30 +0530 Subject: [PATCH 08/39] Removed unused parameter --- .../Framework/App/Action/Plugin/EventDispatchPlugin.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index 5409d8999c2dc..202cc86952e79 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -38,6 +38,8 @@ public function __construct(RequestInterface $request, ManagerInterface $eventMa public function beforeExecute(ActionInterface $subject) { $this->dispatchPreDispatchEvents($subject); + + return []; } /** @@ -56,7 +58,7 @@ private function getEventParameters(ActionInterface $subject): array */ public function afterExecute(ActionInterface $subject, $result) { - if (! $this->isSetActionNoPostDispatchFlag($subject)) { + if (! $this->isSetActionNoPostDispatchFlag()) { $this->dispatchPostDispatchEvents($subject); } @@ -66,8 +68,9 @@ public function afterExecute(ActionInterface $subject, $result) /** * @param ActionInterface $subject * @return bool + * */ - private function isSetActionNoPostDispatchFlag(ActionInterface $subject): bool + private function isSetActionNoPostDispatchFlag(): bool { return $this->actionFlag->get('', Action::FLAG_NO_DISPATCH) || $this->actionFlag->get('', Action::FLAG_NO_POST_DISPATCH); From b2a035d93c98487b69bbcd6b719e79ddf8e39574 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 16:58:59 +0530 Subject: [PATCH 09/39] Move Action::dispatch() plugins to ActionInterface::execute() --- .../Magento/Store/App/Action/Plugin/StoreCheck.php | 14 +++++++------- .../Test/Unit/App/Action/Plugin/StoreCheckTest.php | 8 ++++---- app/code/Magento/Store/etc/di.xml | 6 ++---- .../Magento/Framework/App/Action/Plugin/Design.php | 8 +++----- .../App/Test/Unit/Action/Plugin/DesignTest.php | 5 ++--- 5 files changed, 18 insertions(+), 23 deletions(-) diff --git a/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php b/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php index d9cb146c9f12a..ce32498175ff4 100644 --- a/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php +++ b/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php @@ -4,8 +4,11 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Store\App\Action\Plugin; +use Magento\Framework\App\ActionInterface; + class StoreCheck { /** @@ -23,17 +26,14 @@ public function __construct( } /** - * @param \Magento\Framework\App\Action\AbstractAction $subject - * @param \Magento\Framework\App\RequestInterface $request + * @param ActionInterface $subject * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) * @throws \Magento\Framework\Exception\State\InitException */ - public function beforeDispatch( - \Magento\Framework\App\Action\AbstractAction $subject, - \Magento\Framework\App\RequestInterface $request - ) { - if (!$this->_storeManager->getStore()->isActive()) { + public function beforeExecute(ActionInterface $subject) + { + if (! $this->_storeManager->getStore()->isActive()) { throw new \Magento\Framework\Exception\State\InitException( __('Current store is not active.') ); diff --git a/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php b/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php index 93bd4f2ccfba4..f7ee5c4f9ae79 100644 --- a/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php +++ b/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php @@ -55,16 +55,16 @@ protected function setUp() * @expectedException \Magento\Framework\Exception\State\InitException * @expectedExceptionMessage Current store is not active. */ - public function testBeforeDispatchWhenStoreNotActive() + public function testBeforeExecuteWhenStoreNotActive() { $this->_storeMock->expects($this->any())->method('isActive')->will($this->returnValue(false)); - $this->_plugin->beforeDispatch($this->subjectMock, $this->requestMock); + $this->_plugin->beforeExecute($this->subjectMock, $this->requestMock); } - public function testBeforeDispatchWhenStoreIsActive() + public function testBeforeExecuteWhenStoreIsActive() { $this->_storeMock->expects($this->any())->method('isActive')->will($this->returnValue(true)); - $result = $this->_plugin->beforeDispatch($this->subjectMock, $this->requestMock); + $result = $this->_plugin->beforeExecute($this->subjectMock, $this->requestMock); $this->assertNull($result); } } diff --git a/app/code/Magento/Store/etc/di.xml b/app/code/Magento/Store/etc/di.xml index 611ae44221a48..f5370d8644982 100644 --- a/app/code/Magento/Store/etc/di.xml +++ b/app/code/Magento/Store/etc/di.xml @@ -55,11 +55,9 @@ - - - - + + diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/Design.php b/lib/internal/Magento/Framework/App/Action/Plugin/Design.php index 2893f1209dc98..babde4bcc883a 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/Design.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/Design.php @@ -3,6 +3,7 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Framework\App\Action\Plugin; use Magento\Framework\Message\MessageInterface; @@ -35,15 +36,12 @@ public function __construct( * Initialize design * * @param \Magento\Framework\App\ActionInterface $subject - * @param \Magento\Framework\App\RequestInterface $request * * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeDispatch( - \Magento\Framework\App\ActionInterface $subject, - \Magento\Framework\App\RequestInterface $request - ) { + public function beforeExecute(\Magento\Framework\App\ActionInterface $subject) + { try { $this->_designLoader->load(); } catch (\Magento\Framework\Exception\LocalizedException $e) { diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php index d5c7f4d0355ca..40d046f8b1678 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php @@ -7,14 +7,13 @@ class DesignTest extends \PHPUnit\Framework\TestCase { - public function testAroundDispatch() + public function testBeforeExecute() { $subjectMock = $this->createMock(\Magento\Framework\App\Action\Action::class); $designLoaderMock = $this->createMock(\Magento\Framework\View\DesignLoader::class); $messageManagerMock = $this->createMock(\Magento\Framework\Message\ManagerInterface::class); - $requestMock = $this->createMock(\Magento\Framework\App\RequestInterface::class); $plugin = new \Magento\Framework\App\Action\Plugin\Design($designLoaderMock, $messageManagerMock); $designLoaderMock->expects($this->once())->method('load'); - $plugin->beforeDispatch($subjectMock, $requestMock); + $plugin->beforeExecute($subjectMock); } } From 344c83cb360865c8b7da53abcc0f840af4fd5e0d Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 17:34:25 +0530 Subject: [PATCH 10/39] Move more Action::dispatch() plugins to ActionInterface::execute() --- .../Model/App/Action/ContextPlugin.php | 8 +++---- .../Model/App/Action/ContextPluginTest.php | 7 ++---- app/code/Magento/Customer/etc/frontend/di.xml | 4 ++-- .../Tax/Model/App/Action/ContextPlugin.php | 5 ++-- .../Unit/App/Action/ContextPluginTest.php | 9 ++++--- app/code/Magento/Tax/etc/frontend/di.xml | 2 +- .../Theme/Model/Theme/Plugin/Registration.php | 11 ++++----- .../Model/Theme/Plugin/RegistrationTest.php | 24 ++++++++----------- .../Weee/Model/App/Action/ContextPlugin.php | 7 ++---- .../Unit/App/Action/ContextPluginTest.php | 16 ++++++------- app/code/Magento/Weee/etc/frontend/di.xml | 2 +- 11 files changed, 39 insertions(+), 56 deletions(-) diff --git a/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php b/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php index b8c83551ee381..f221949051393 100644 --- a/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php +++ b/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php @@ -8,8 +8,7 @@ use Magento\Customer\Model\Context; use Magento\Customer\Model\GroupManagement; -use Magento\Framework\App\Action\AbstractAction; -use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\ActionInterface; use Magento\Customer\Model\Session; use Magento\Framework\App\Http\Context as HttpContext; @@ -41,12 +40,11 @@ public function __construct(Session $customerSession, HttpContext $httpContext) /** * Set customer group and customer session id to HTTP context * - * @param AbstractAction $subject - * @param RequestInterface $request + * @param ActionInterface $subject * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeDispatch(AbstractAction $subject, RequestInterface $request) + public function beforeExecute(ActionInterface $subject) { $this->httpContext->setValue( Context::CONTEXT_GROUP, diff --git a/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php b/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php index 4b89fda7c7051..8be32c52fe8b9 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php @@ -53,10 +53,7 @@ protected function setUp() ); } - /** - * Test aroundDispatch - */ - public function testBeforeDispatch() + public function testBeforeExecute() { $this->customerSessionMock->expects($this->once()) ->method('getCustomerGroupId') @@ -74,6 +71,6 @@ public function testBeforeDispatch() ] ) ); - $this->plugin->beforeDispatch($this->subjectMock, $this->requestMock); + $this->plugin->beforeExecute($this->subjectMock, $this->requestMock); } } diff --git a/app/code/Magento/Customer/etc/frontend/di.xml b/app/code/Magento/Customer/etc/frontend/di.xml index 4a45c4ad48d19..98a2e6f6d2773 100644 --- a/app/code/Magento/Customer/etc/frontend/di.xml +++ b/app/code/Magento/Customer/etc/frontend/di.xml @@ -20,8 +20,8 @@ - - + customerSession->isLoggedIn() || !$this->moduleManager->isEnabled('Magento_PageCache') || diff --git a/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php b/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php index 9a85ba0a9089b..8471e3b49fbb7 100644 --- a/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php @@ -111,9 +111,9 @@ protected function setUp() * @param bool $cache * @param bool $taxEnabled * @param bool $loggedIn - * @dataProvider beforeDispatchDataProvider + * @dataProvider beforeExecuteDataProvider */ - public function testBeforeDispatch($cache, $taxEnabled, $loggedIn) + public function testBeforeExecute($cache, $taxEnabled, $loggedIn) { $this->customerSessionMock->expects($this->any()) ->method('isLoggedIn') @@ -159,8 +159,7 @@ public function testBeforeDispatch($cache, $taxEnabled, $loggedIn) } $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); - $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $result = $this->contextPlugin->beforeDispatch($action, $request); + $result = $this->contextPlugin->beforeExecute($action); $this->assertNull($result); } else { $this->assertFalse($loggedIn); @@ -170,7 +169,7 @@ public function testBeforeDispatch($cache, $taxEnabled, $loggedIn) /** * @return array */ - public function beforeDispatchDataProvider() + public function beforeExecuteDataProvider() { return [ [false, false, false], diff --git a/app/code/Magento/Tax/etc/frontend/di.xml b/app/code/Magento/Tax/etc/frontend/di.xml index 4db3f54b0edd9..d1b8d93e96935 100644 --- a/app/code/Magento/Tax/etc/frontend/di.xml +++ b/app/code/Magento/Tax/etc/frontend/di.xml @@ -35,7 +35,7 @@ - + diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index 05030c56c2ee0..ae65b52f938af 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -5,8 +5,7 @@ */ namespace Magento\Theme\Model\Theme\Plugin; -use Magento\Backend\App\AbstractAction; -use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\ActionInterface; use Magento\Theme\Model\Theme\Registration as ThemeRegistration; use Magento\Framework\Exception\LocalizedException; use Psr\Log\LoggerInterface; @@ -69,15 +68,13 @@ public function __construct( /** * Add new theme from filesystem and update existing * - * @param AbstractAction $subject - * @param RequestInterface $request + * @param ActionInterface $subject * * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeDispatch( - AbstractAction $subject, - RequestInterface $request + public function beforeExecute( + ActionInterface $subject ) { try { if ($this->appState->getMode() != AppState::MODE_PRODUCTION) { diff --git a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php index 190a6edf55900..1bedfeea368d4 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php @@ -5,6 +5,7 @@ */ namespace Magento\Theme\Test\Unit\Model\Theme\Plugin; +use Magento\Framework\App\ActionInterface; use Magento\Theme\Model\Theme\Plugin\Registration; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Phrase; @@ -17,8 +18,8 @@ class RegistrationTest extends \PHPUnit\Framework\TestCase /** @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $logger; - /** @var \Magento\Backend\App\AbstractAction|\PHPUnit_Framework_MockObject_MockObject */ - protected $abstractAction; + /** @var ActionInterface|\PHPUnit_Framework_MockObject_MockObject */ + protected $action; /** @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $request; @@ -39,12 +40,7 @@ protected function setUp() { $this->themeRegistration = $this->createMock(\Magento\Theme\Model\Theme\Registration::class); $this->logger = $this->getMockForAbstractClass(\Psr\Log\LoggerInterface::class, [], '', false); - $this->abstractAction = $this->getMockForAbstractClass( - \Magento\Backend\App\AbstractAction::class, - [], - '', - false - ); + $this->action = $this->createMock(ActionInterface::class); $this->request = $this->getMockForAbstractClass(\Magento\Framework\App\RequestInterface::class, [], '', false); $this->appState = $this->createMock(\Magento\Framework\App\State::class); $this->themeCollection = $this->createMock(\Magento\Theme\Model\Theme\Collection::class); @@ -60,10 +56,10 @@ protected function setUp() /** * @param bool $hasParentTheme - * @dataProvider dataProviderBeforeDispatch + * @dataProvider dataProviderBeforeExecute * @SuppressWarnings(PHPMD.NPathComplexity) */ - public function testBeforeDispatch( + public function testBeforeExecute( $hasParentTheme ) { $themeId = 1; @@ -147,13 +143,13 @@ public function testBeforeDispatch( ->method('save') ->willReturnSelf(); - $this->plugin->beforeDispatch($this->abstractAction, $this->request); + $this->plugin->beforeExecute($this->action); } /** * @return array */ - public function dataProviderBeforeDispatch() + public function dataProviderBeforeExecute() { return [ [true], @@ -164,7 +160,7 @@ public function dataProviderBeforeDispatch() public function testBeforeDispatchWithProductionMode() { $this->appState->expects($this->once())->method('getMode')->willReturn('production'); - $this->plugin->beforeDispatch($this->abstractAction, $this->request); + $this->plugin->beforeExecute($this->action, $this->request); } public function testBeforeDispatchWithException() @@ -173,6 +169,6 @@ public function testBeforeDispatchWithException() $this->themeRegistration->expects($this->once())->method('register')->willThrowException($exception); $this->logger->expects($this->once())->method('critical'); - $this->plugin->beforeDispatch($this->abstractAction, $this->request); + $this->plugin->beforeExecute($this->action, $this->request); } } diff --git a/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php b/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php index 8363365372f63..58388401342ac 100644 --- a/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php +++ b/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php @@ -92,17 +92,14 @@ public function __construct( /** * @param \Magento\Framework\App\ActionInterface $subject - * @param \Magento\Framework\App\RequestInterface $request * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ - public function beforeDispatch( - \Magento\Framework\App\ActionInterface $subject, - \Magento\Framework\App\RequestInterface $request - ) { + public function beforeExecute(\Magento\Framework\App\ActionInterface $subject) + { if (!$this->weeeHelper->isEnabled() || !$this->customerSession->isLoggedIn() || !$this->moduleManager->isEnabled('Magento_PageCache') || diff --git a/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php b/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php index 5500350e243ad..ba8a9b57cfa6c 100644 --- a/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php @@ -123,7 +123,7 @@ protected function setUp() ); } - public function testBeforeDispatchBasedOnDefault() + public function testBeforeExecuteBasedOnDefault() { $this->customerSessionMock->expects($this->once()) ->method('isLoggedIn') @@ -188,10 +188,10 @@ public function testBeforeDispatchBasedOnDefault() $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeDispatch($action, $request); + $this->contextPlugin->beforeExecute($action, $request); } - public function testBeforeDispatchBasedOnOrigin() + public function testBeforeExecuteBasedOnOrigin() { $this->customerSessionMock->expects($this->once()) ->method('isLoggedIn') @@ -217,10 +217,10 @@ public function testBeforeDispatchBasedOnOrigin() $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeDispatch($action, $request); + $this->contextPlugin->beforeExecute($action, $request); } - public function testBeforeDispatchBasedOnBilling() + public function testBeforeExecuteBasedOnBilling() { $this->customerSessionMock->expects($this->once()) ->method('isLoggedIn') @@ -289,10 +289,10 @@ public function testBeforeDispatchBasedOnBilling() $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeDispatch($action, $request); + $this->contextPlugin->beforeExecute($action, $request); } - public function testBeforeDispatchBasedOnShipping() + public function testBeforeExecuterBasedOnShipping() { $this->customerSessionMock->expects($this->once()) ->method('isLoggedIn') @@ -361,6 +361,6 @@ public function testBeforeDispatchBasedOnShipping() $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeDispatch($action, $request); + $this->contextPlugin->beforeExecute($action, $request); } } diff --git a/app/code/Magento/Weee/etc/frontend/di.xml b/app/code/Magento/Weee/etc/frontend/di.xml index 1ce09205b6e77..f6417fd86a3e1 100644 --- a/app/code/Magento/Weee/etc/frontend/di.xml +++ b/app/code/Magento/Weee/etc/frontend/di.xml @@ -13,7 +13,7 @@ - + From 09bc573ad25e1631e2d1dfa9740178a6c641fca7 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 17:51:22 +0530 Subject: [PATCH 11/39] Set area for controller tests --- .../Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php | 2 ++ .../Magento/Paypal/Controller/Billing/AgreementTest.php | 1 + 2 files changed, 3 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php b/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php index 35892623ef7af..cb012d73bdfb0 100644 --- a/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php +++ b/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php @@ -14,6 +14,8 @@ class PlaceTest extends \Magento\TestFramework\TestCase\AbstractBackendControlle { /** * Test requestToAuthorizenetData returning + * + * @magentoAppArea adminhtml */ public function testExecuteAuthorizenetDataReturning() { diff --git a/dev/tests/integration/testsuite/Magento/Paypal/Controller/Billing/AgreementTest.php b/dev/tests/integration/testsuite/Magento/Paypal/Controller/Billing/AgreementTest.php index fd8b4fd2efad5..6f192b6479acd 100644 --- a/dev/tests/integration/testsuite/Magento/Paypal/Controller/Billing/AgreementTest.php +++ b/dev/tests/integration/testsuite/Magento/Paypal/Controller/Billing/AgreementTest.php @@ -19,6 +19,7 @@ class AgreementTest extends \Magento\TestFramework\TestCase\AbstractController * * @magentoDataFixture Magento/Customer/_files/customer.php * @magentoDbIsolation enabled + * @magentoAppArea frontend */ public function testReturnWizardAction() { From c9507a6de60f7e80627de38247027e7f85b26138 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 18:41:58 +0530 Subject: [PATCH 12/39] Add missing @SuppressWarnings(PHPMD.UnusedFormalParameter) to plugin method --- .../App/Action/Plugin/ActionFlagNoDispatchPlugin.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php index fa335a1e2fd84..98c51475a0687 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -24,6 +24,13 @@ public function __construct(ActionFlag $actionFlag, ResponseInterface $response) $this->response = $response; } + /** + * @param ActionInterface $subject + * @param callable $proceed + * @return ResponseInterface + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ public function aroundExecute(ActionInterface $subject, callable $proceed) { return $this->actionFlag->get('', ActionInterface::FLAG_NO_DISPATCH) ? $this->response : $proceed(); From 54ce141d3874f6c11a04806f20be0c13caaeddc1 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 7 Jan 2018 22:32:08 +0530 Subject: [PATCH 13/39] Remove whitespace at end of line --- .../Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php | 2 +- .../Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php | 2 +- .../Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php b/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php index cb012d73bdfb0..780c6b226ad70 100644 --- a/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php +++ b/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php @@ -14,7 +14,7 @@ class PlaceTest extends \Magento\TestFramework\TestCase\AbstractBackendControlle { /** * Test requestToAuthorizenetData returning - * + * * @magentoAppArea adminhtml */ public function testExecuteAuthorizenetDataReturning() diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php index 98c51475a0687..3c84203662073 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -28,7 +28,7 @@ public function __construct(ActionFlag $actionFlag, ResponseInterface $response) * @param ActionInterface $subject * @param callable $proceed * @return ResponseInterface - * + * * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function aroundExecute(ActionInterface $subject, callable $proceed) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index 202cc86952e79..c4bd407c2228c 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -68,7 +68,7 @@ public function afterExecute(ActionInterface $subject, $result) /** * @param ActionInterface $subject * @return bool - * + * */ private function isSetActionNoPostDispatchFlag(): bool { From 8d209c33261f2495b541f3817c1bc10a2d520b4f Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Tue, 9 Jan 2018 16:49:06 +0100 Subject: [PATCH 14/39] Ignore PHPMD StaticAccess for ObjectManager::getInstance() in test class --- .../testsuite/Magento/Framework/App/ControllerActionTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php index b74ed98d36864..ce2d92b62d55d 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -18,6 +18,7 @@ * @magentoAppIsolation enabled * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.UnusedLocalVariable) + * @SuppressWarnings(PHPMD.StaticAccess) */ class ControllerActionTest extends TestCase { From cd7a64ebcfe0bb0326780f5d2b2ff6d2f7e5b34f Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 10:18:31 +0100 Subject: [PATCH 15/39] Remove obsolete plugin method request parameter and clean up test class properties --- .../Model/App/Action/ContextPluginTest.php | 12 ++--- .../Unit/App/Action/Plugin/StoreCheckTest.php | 16 ++----- .../Model/Theme/Plugin/RegistrationTest.php | 8 +--- .../Unit/App/Action/ContextPluginTest.php | 48 +++++++++++-------- 4 files changed, 39 insertions(+), 45 deletions(-) diff --git a/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php b/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php index 8be32c52fe8b9..1b70f174c39fd 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php @@ -24,20 +24,15 @@ class ContextPluginTest extends \PHPUnit\Framework\TestCase protected $customerSessionMock; /** - * @var \Magento\Framework\App\Http\Context $httpContext|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\Http\Context|\PHPUnit_Framework_MockObject_MockObject */ protected $httpContextMock; /** - * @var \PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\Action\Action|\PHPUnit_Framework_MockObject_MockObject */ protected $subjectMock; - /** - * @var \PHPUnit_Framework_MockObject_MockObject - */ - protected $requestMock; - /** * Set up */ @@ -46,7 +41,6 @@ protected function setUp() $this->customerSessionMock = $this->createMock(\Magento\Customer\Model\Session::class); $this->httpContextMock = $this->createMock(\Magento\Framework\App\Http\Context::class); $this->subjectMock = $this->createMock(\Magento\Framework\App\Action\Action::class); - $this->requestMock = $this->createMock(\Magento\Framework\App\RequestInterface::class); $this->plugin = new \Magento\Customer\Model\App\Action\ContextPlugin( $this->customerSessionMock, $this->httpContextMock @@ -71,6 +65,6 @@ public function testBeforeExecute() ] ) ); - $this->plugin->beforeExecute($this->subjectMock, $this->requestMock); + $this->plugin->beforeExecute($this->subjectMock); } } diff --git a/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php b/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php index f7ee5c4f9ae79..b24db7aa4a269 100644 --- a/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php +++ b/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php @@ -13,12 +13,12 @@ class StoreCheckTest extends \PHPUnit\Framework\TestCase protected $_plugin; /** - * @var \PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $_storeManagerMock; /** - * @var \PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Store\Model\Store|\PHPUnit_Framework_MockObject_MockObject */ protected $_storeMock; @@ -26,12 +26,7 @@ class StoreCheckTest extends \PHPUnit\Framework\TestCase * @var \Magento\Framework\App\Action\AbstractAction|\PHPUnit_Framework_MockObject_MockObject */ protected $subjectMock; - - /** - * @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject - */ - protected $requestMock; - + protected function setUp() { $this->_storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); @@ -43,7 +38,6 @@ protected function setUp() )->will( $this->returnValue($this->_storeMock) ); - $this->requestMock = $this->createMock(\Magento\Framework\App\RequestInterface::class); $this->subjectMock = $this->getMockBuilder(\Magento\Framework\App\Action\AbstractAction::class) ->disableOriginalConstructor() ->getMockForAbstractClass(); @@ -58,13 +52,13 @@ protected function setUp() public function testBeforeExecuteWhenStoreNotActive() { $this->_storeMock->expects($this->any())->method('isActive')->will($this->returnValue(false)); - $this->_plugin->beforeExecute($this->subjectMock, $this->requestMock); + $this->_plugin->beforeExecute($this->subjectMock); } public function testBeforeExecuteWhenStoreIsActive() { $this->_storeMock->expects($this->any())->method('isActive')->will($this->returnValue(true)); - $result = $this->_plugin->beforeExecute($this->subjectMock, $this->requestMock); + $result = $this->_plugin->beforeExecute($this->subjectMock); $this->assertNull($result); } } diff --git a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php index 1bedfeea368d4..514964034b967 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php @@ -21,9 +21,6 @@ class RegistrationTest extends \PHPUnit\Framework\TestCase /** @var ActionInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $action; - /** @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $request; - /** @var \Magento\Framework\App\State|\PHPUnit_Framework_MockObject_MockObject */ protected $appState; @@ -41,7 +38,6 @@ protected function setUp() $this->themeRegistration = $this->createMock(\Magento\Theme\Model\Theme\Registration::class); $this->logger = $this->getMockForAbstractClass(\Psr\Log\LoggerInterface::class, [], '', false); $this->action = $this->createMock(ActionInterface::class); - $this->request = $this->getMockForAbstractClass(\Magento\Framework\App\RequestInterface::class, [], '', false); $this->appState = $this->createMock(\Magento\Framework\App\State::class); $this->themeCollection = $this->createMock(\Magento\Theme\Model\Theme\Collection::class); $this->themeLoader = $this->createMock(\Magento\Theme\Model\ResourceModel\Theme\Collection::class); @@ -160,7 +156,7 @@ public function dataProviderBeforeExecute() public function testBeforeDispatchWithProductionMode() { $this->appState->expects($this->once())->method('getMode')->willReturn('production'); - $this->plugin->beforeExecute($this->action, $this->request); + $this->plugin->beforeExecute($this->action); } public function testBeforeDispatchWithException() @@ -169,6 +165,6 @@ public function testBeforeDispatchWithException() $this->themeRegistration->expects($this->once())->method('register')->willThrowException($exception); $this->logger->expects($this->once())->method('critical'); - $this->plugin->beforeExecute($this->action, $this->request); + $this->plugin->beforeExecute($this->action); } } diff --git a/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php b/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php index ba8a9b57cfa6c..7ede0aaf7ca18 100644 --- a/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php @@ -14,50 +14,60 @@ class ContextPluginTest extends \PHPUnit\Framework\TestCase { /** - * @var \Magento\Tax\Helper\Data + * @var \Magento\Tax\Helper\Data|\PHPUnit_Framework_MockObject_MockObject */ protected $taxHelperMock; /** - * @var \Magento\Weee\Helper\Data + * @var \Magento\Weee\Helper\Data|\PHPUnit_Framework_MockObject_MockObject */ protected $weeeHelperMock; /** - * @var \Magento\Weee\Model\Tax + * @var \Magento\Weee\Model\Tax|\PHPUnit_Framework_MockObject_MockObject */ protected $weeeTaxMock; /** - * @var \Magento\Framework\App\Http\Context + * @var \Magento\Framework\App\Http\Context|\PHPUnit_Framework_MockObject_MockObject */ protected $httpContextMock; /** - * @var \Magento\Tax\Model\Calculation\Proxy + * @var \Magento\Tax\Model\Calculation\Proxy|\PHPUnit_Framework_MockObject_MockObject */ protected $taxCalculationMock; /** - * @var \Magento\Framework\Module\Manager + * @var \Magento\Framework\Module\Manager|\PHPUnit_Framework_MockObject_MockObject */ protected $moduleManagerMock; /** - * @var \Magento\PageCache\Model\Config + * @var \Magento\PageCache\Model\Config|\PHPUnit_Framework_MockObject_MockObject */ protected $cacheConfigMock; /** - * @var \Magento\Store\Model\StoreManager + * @var \Magento\Store\Model\StoreManager|\PHPUnit_Framework_MockObject_MockObject */ - protected $storeManageMock; + protected $storeManagerMock; /** - * @var \Magento\Framework\App\Config\ScopeConfig + * @var \Magento\Framework\App\Config|\PHPUnit_Framework_MockObject_MockObject */ protected $scopeConfigMock; + /** + * @var \Magento\Customer\Model\Session|\PHPUnit_Framework_MockObject_MockObject + */ + private $customerSessionMock; + + /** + * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager + */ + private $objectManager; + /** * @var \Magento\Tax\Model\App\Action\ContextPlugin */ @@ -98,7 +108,7 @@ protected function setUp() $this->cacheConfigMock = $this->getMockBuilder(\Magento\PageCache\Model\Config::class) ->disableOriginalConstructor() ->getMock(); - + $this->storeManagerMock = $this->getMockBuilder(\Magento\Store\Model\StoreManager::class) ->disableOriginalConstructor() ->getMock(); @@ -185,10 +195,10 @@ public function testBeforeExecuteBasedOnDefault() ->method('setValue') ->with('weee_tax_region', ['countryId' => 'US', 'regionId' => 0], 0); + /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); - $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeExecute($action, $request); + $this->contextPlugin->beforeExecute($action); } public function testBeforeExecuteBasedOnOrigin() @@ -214,10 +224,10 @@ public function testBeforeExecuteBasedOnOrigin() ->method('getTaxBasedOn') ->willReturn('origin'); + /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); - $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeExecute($action, $request); + $this->contextPlugin->beforeExecute($action); } public function testBeforeExecuteBasedOnBilling() @@ -286,10 +296,10 @@ public function testBeforeExecuteBasedOnBilling() ->method('setValue') ->with('weee_tax_region', ['countryId' => 'US', 'regionId' => 1], 0); + /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); - $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeExecute($action, $request); + $this->contextPlugin->beforeExecute($action); } public function testBeforeExecuterBasedOnShipping() @@ -358,9 +368,9 @@ public function testBeforeExecuterBasedOnShipping() ->method('setValue') ->with('weee_tax_region', ['countryId' => 'US', 'regionId' => 1], 0); + /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); - $request = $this->createPartialMock(\Magento\Framework\App\Request\Http::class, ['getActionName']); - $this->contextPlugin->beforeExecute($action, $request); + $this->contextPlugin->beforeExecute($action); } } From 64dcbeac40b9ddee5b8ed4c4af92c95a328a57e2 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 10:41:24 +0100 Subject: [PATCH 16/39] Return void instead of [] from before plugin if no arguments are modified --- .../App/Action/Plugin/ActionFlagNoDispatchPlugin.php | 5 ++++- .../Framework/App/Action/Plugin/EventDispatchPlugin.php | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php index 3c84203662073..ac1dfadb6a8cb 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -6,6 +6,9 @@ use Magento\Framework\App\ActionInterface; use Magento\Framework\App\ResponseInterface; +/** + * + */ class ActionFlagNoDispatchPlugin { /** @@ -28,7 +31,7 @@ public function __construct(ActionFlag $actionFlag, ResponseInterface $response) * @param ActionInterface $subject * @param callable $proceed * @return ResponseInterface - * + * * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function aroundExecute(ActionInterface $subject, callable $proceed) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index c4bd407c2228c..28074e8179bc8 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -11,6 +11,9 @@ use Magento\Framework\Event\ManagerInterface; use Magento\Framework\HTTP\PhpEnvironment\Response; +/** + * + */ class EventDispatchPlugin { /** @@ -38,8 +41,6 @@ public function __construct(RequestInterface $request, ManagerInterface $eventMa public function beforeExecute(ActionInterface $subject) { $this->dispatchPreDispatchEvents($subject); - - return []; } /** From e0d905624045b7c7934f4ac236e19b9ddafd4649 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 11:31:06 +0100 Subject: [PATCH 17/39] Change the customer notification plugin from AbstractAction:dispatch to ActionInterface::execute --- .../Model/Plugin/CustomerNotification.php | 62 ++++++++++--- .../Model/Plugin/CustomerNotificationTest.php | 89 ++++++++----------- app/code/Magento/Customer/etc/di.xml | 2 +- 3 files changed, 89 insertions(+), 64 deletions(-) diff --git a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php index 517aef5690ee6..230b1aa3fed4c 100644 --- a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php +++ b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php @@ -3,13 +3,15 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Customer\Model\Plugin; use Magento\Customer\Api\CustomerRepositoryInterface; use Magento\Customer\Model\Customer\NotificationStorage; use Magento\Customer\Model\Session; -use Magento\Framework\App\Action\AbstractAction; +use Magento\Framework\App\ActionInterface; use Magento\Framework\App\Area; +use Magento\Framework\App\ObjectManager; use Magento\Framework\App\RequestInterface; use Magento\Framework\App\State; use Magento\Framework\Exception\NoSuchEntityException; @@ -42,6 +44,11 @@ class CustomerNotification */ private $logger; + /** + * @var RequestInterface|\Magento\Framework\App\Request\Http + */ + private $request; + /** * Initialize dependencies. * @@ -50,37 +57,38 @@ class CustomerNotification * @param State $state * @param CustomerRepositoryInterface $customerRepository * @param LoggerInterface $logger + * @param RequestInterface|null $request */ public function __construct( Session $session, NotificationStorage $notificationStorage, State $state, CustomerRepositoryInterface $customerRepository, - LoggerInterface $logger + LoggerInterface $logger, + RequestInterface $request = null ) { $this->session = $session; $this->notificationStorage = $notificationStorage; $this->state = $state; $this->customerRepository = $customerRepository; $this->logger = $logger; + $this->request = $request ?? ObjectManager::getInstance()->get(RequestInterface::class); } /** - * @param AbstractAction $subject - * @param RequestInterface $request + * Refresh the customer session on frontend post requests if an update session notification is registered. + * + * @param ActionInterface $subject * @return void + * @throws \Magento\Framework\Exception\LocalizedException * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeDispatch(AbstractAction $subject, RequestInterface $request) + public function beforeExecute(ActionInterface $subject) { $customerId = $this->session->getCustomerId(); - if ($this->state->getAreaCode() == Area::AREA_FRONTEND && $request->isPost() - && $this->notificationStorage->isExists( - NotificationStorage::UPDATE_CUSTOMER_SESSION, - $customerId - ) - ) { + if ($this->isFrontendRequest() && $this->isPostRequest() && $this->isSessionUpdateRegisteredFor($customerId)) + { try { $customer = $this->customerRepository->getById($customerId); $this->session->setCustomerData($customer); @@ -92,4 +100,36 @@ public function beforeDispatch(AbstractAction $subject, RequestInterface $reques } } } + + /** + * Because RequestInterface has no isPost method the check is requied before calling it. + * + * @return bool + */ + private function isPostRequest(): bool + { + return method_exists($this->request, 'isPost') && $this->request->isPost(); + } + + /** + * Check if the current application area is frontend. + * + * @return bool + * @throws \Magento\Framework\Exception\LocalizedException + */ + private function isFrontendRequest(): bool + { + return $this->state->getAreaCode() == Area::AREA_FRONTEND; + } + + /** + * True if the session for the given customer ID needs to be refreshed. + * + * @param int $customerId + * @return bool + */ + private function isSessionUpdateRegisteredFor($customerId): bool + { + return $this->notificationStorage->isExists(NotificationStorage::UPDATE_CUSTOMER_SESSION, $customerId); + } } diff --git a/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php b/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php index c3c853bca1469..27999e903d900 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php @@ -5,12 +5,12 @@ */ namespace Magento\Customer\Test\Unit\Model\Plugin; -use Magento\Backend\App\AbstractAction; use Magento\Customer\Api\CustomerRepositoryInterface; use Magento\Customer\Api\Data\CustomerInterface; use Magento\Customer\Model\Customer\NotificationStorage; use Magento\Customer\Model\Plugin\CustomerNotification; use Magento\Customer\Model\Session; +use Magento\Framework\App\ActionInterface; use Magento\Framework\App\Area; use Magento\Framework\App\RequestInterface; use Magento\Framework\App\State; @@ -20,25 +20,25 @@ class CustomerNotificationTest extends \PHPUnit\Framework\TestCase { /** @var Session|\PHPUnit_Framework_MockObject_MockObject */ - private $sessionMock; + private $session; /** @var NotificationStorage|\PHPUnit_Framework_MockObject_MockObject */ - private $notificationStorageMock; + private $notificationStorage; /** @var CustomerRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $customerRepositoryMock; + private $customerRepository; /** @var State|\PHPUnit_Framework_MockObject_MockObject */ - private $appStateMock; + private $appState; /** @var RequestInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $requestMock; + private $request; - /** @var AbstractAction|\PHPUnit_Framework_MockObject_MockObject */ - private $abstractActionMock; - - /** @var LoggerInterface */ - private $loggerMock; + /** @var ActionInterface|\PHPUnit_Framework_MockObject_MockObject */ + private $actionInterfaceMock; + + /** @var LoggerInterface|\PHPUnit_Framework_MockObject_MockObject */ + private $logger; /** @var CustomerNotification */ private $plugin; @@ -48,76 +48,61 @@ class CustomerNotificationTest extends \PHPUnit\Framework\TestCase protected function setUp() { - $this->sessionMock = $this->getMockBuilder(Session::class) - ->disableOriginalConstructor() - ->setMethods(['getCustomerId', 'setCustomerData', 'setCustomerGroupId', 'regenerateId']) - ->getMock(); - $this->notificationStorageMock = $this->getMockBuilder(NotificationStorage::class) - ->disableOriginalConstructor() - ->setMethods(['isExists', 'remove']) - ->getMock(); - $this->customerRepositoryMock = $this->getMockBuilder(CustomerRepositoryInterface::class) - ->getMockForAbstractClass(); - $this->abstractActionMock = $this->getMockBuilder(AbstractAction::class) - ->disableOriginalConstructor() - ->getMockForAbstractClass(); - $this->requestMock = $this->getMockBuilder(RequestInterface::class) + $this->session = $this->createMock(Session::class); + $this->notificationStorage = $this->createMock(NotificationStorage::class); + $this->customerRepository = $this->createMock(CustomerRepositoryInterface::class); + $this->actionInterfaceMock = $this->createMock(ActionInterface::class); + $this->request = $this->getMockBuilder(RequestInterface::class) ->setMethods(['isPost']) ->getMockForAbstractClass(); - $this->appStateMock = $this->getMockBuilder(State::class) - ->disableOriginalConstructor() - ->setMethods(['getAreaCode']) - ->getMock(); - - $this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class); - $this->appStateMock->method('getAreaCode')->willReturn(Area::AREA_FRONTEND); - $this->requestMock->method('isPost')->willReturn(true); - $this->sessionMock->method('getCustomerId')->willReturn(self::$customerId); - $this->notificationStorageMock->expects($this->any()) + $this->appState = $this->createMock(State::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->appState->method('getAreaCode')->willReturn(Area::AREA_FRONTEND); + $this->request->method('isPost')->willReturn(true); + $this->session->method('getCustomerId')->willReturn(self::$customerId); + $this->notificationStorage->expects($this->any()) ->method('isExists') ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::$customerId) ->willReturn(true); $this->plugin = new CustomerNotification( - $this->sessionMock, - $this->notificationStorageMock, - $this->appStateMock, - $this->customerRepositoryMock, - $this->loggerMock + $this->session, + $this->notificationStorage, + $this->appState, + $this->customerRepository, + $this->logger, + $this->request ); } - public function testBeforeDispatch() + public function testBeforeExecute() { $customerGroupId =1; - $customerMock = $this->getMockForAbstractClass(CustomerInterface::class); + $customerMock = $this->createMock(CustomerInterface::class); $customerMock->method('getGroupId')->willReturn($customerGroupId); $customerMock->method('getId')->willReturn(self::$customerId); - $this->customerRepositoryMock->expects($this->once()) + $this->customerRepository->expects($this->once()) ->method('getById') ->with(self::$customerId) ->willReturn($customerMock); - $this->notificationStorageMock->expects($this->once()) + $this->notificationStorage->expects($this->once()) ->method('remove') ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::$customerId); - $this->sessionMock->expects($this->once())->method('setCustomerData')->with($customerMock); - $this->sessionMock->expects($this->once())->method('setCustomerGroupId')->with($customerGroupId); - $this->sessionMock->expects($this->once())->method('regenerateId'); - - $this->plugin->beforeDispatch($this->abstractActionMock, $this->requestMock); + $this->plugin->beforeExecute($this->actionInterfaceMock); } public function testBeforeDispatchWithNoCustomerFound() { - $this->customerRepositoryMock->method('getById') + $this->customerRepository->method('getById') ->with(self::$customerId) ->willThrowException(new NoSuchEntityException()); - $this->loggerMock->expects($this->once()) + $this->logger->expects($this->once()) ->method('error'); - $this->plugin->beforeDispatch($this->abstractActionMock, $this->requestMock); + $this->plugin->beforeExecute($this->actionInterfaceMock); } } diff --git a/app/code/Magento/Customer/etc/di.xml b/app/code/Magento/Customer/etc/di.xml index 0d99c1145e81b..082cea58766fd 100644 --- a/app/code/Magento/Customer/etc/di.xml +++ b/app/code/Magento/Customer/etc/di.xml @@ -333,7 +333,7 @@ - + From fd179eac77d12225b38e64f237b15b0403beb975 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 11:33:22 +0100 Subject: [PATCH 18/39] Remove training whitespace in empty PHPDoc lines --- .../App/Action/Plugin/ActionFlagNoDispatchPlugin.php | 4 ++-- .../Framework/App/Action/Plugin/EventDispatchPlugin.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php index ac1dfadb6a8cb..9256ab97ab7b7 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -7,7 +7,7 @@ use Magento\Framework\App\ResponseInterface; /** - * + * */ class ActionFlagNoDispatchPlugin { @@ -31,7 +31,7 @@ public function __construct(ActionFlag $actionFlag, ResponseInterface $response) * @param ActionInterface $subject * @param callable $proceed * @return ResponseInterface - * + * * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function aroundExecute(ActionInterface $subject, callable $proceed) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index 28074e8179bc8..2cc0bea8cf8e6 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -12,7 +12,7 @@ use Magento\Framework\HTTP\PhpEnvironment\Response; /** - * + * */ class EventDispatchPlugin { From 8e3bb74f377f839c8c0cb54e16deb9cfe3bf14cd Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 12:41:32 +0100 Subject: [PATCH 19/39] Remove more whitespace at end of line in PHPDoc blocks --- .../Customer/Model/Plugin/CustomerNotification.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php index 230b1aa3fed4c..60b5a9f572b8a 100644 --- a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php +++ b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php @@ -77,7 +77,7 @@ public function __construct( /** * Refresh the customer session on frontend post requests if an update session notification is registered. - * + * * @param ActionInterface $subject * @return void * @throws \Magento\Framework\Exception\LocalizedException @@ -87,8 +87,7 @@ public function beforeExecute(ActionInterface $subject) { $customerId = $this->session->getCustomerId(); - if ($this->isFrontendRequest() && $this->isPostRequest() && $this->isSessionUpdateRegisteredFor($customerId)) - { + if ($this->isFrontendRequest() && $this->isPostRequest() && $this->isSessionUpdateRegisteredFor($customerId)) { try { $customer = $this->customerRepository->getById($customerId); $this->session->setCustomerData($customer); @@ -103,7 +102,7 @@ public function beforeExecute(ActionInterface $subject) /** * Because RequestInterface has no isPost method the check is requied before calling it. - * + * * @return bool */ private function isPostRequest(): bool @@ -113,7 +112,7 @@ private function isPostRequest(): bool /** * Check if the current application area is frontend. - * + * * @return bool * @throws \Magento\Framework\Exception\LocalizedException */ @@ -124,7 +123,7 @@ private function isFrontendRequest(): bool /** * True if the session for the given customer ID needs to be refreshed. - * + * * @param int $customerId * @return bool */ From 5671bba3638cd95169a1dd2a6cd1dbb272e50d32 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 16:32:38 +0100 Subject: [PATCH 20/39] Add descriptions to the phpdoc blocks, even if they are superfluous --- .../Model/Plugin/CustomerNotification.php | 21 +++++++++++++++++-- app/code/Magento/Customer/Model/Visitor.php | 4 ++++ .../InheritanceBasedBackendAction.php | 3 +++ .../InheritanceBasedFrontendAction.php | 3 +++ .../TestStubs/InterfaceOnlyBackendAction.php | 3 +++ .../TestStubs/InterfaceOnlyFrontendAction.php | 3 +++ .../Plugin/ActionFlagNoDispatchPlugin.php | 4 +++- .../App/Action/Plugin/EventDispatchPlugin.php | 17 ++++++++++++++- 8 files changed, 54 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php index 60b5a9f572b8a..fe81509747357 100644 --- a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php +++ b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php @@ -72,7 +72,7 @@ public function __construct( $this->state = $state; $this->customerRepository = $customerRepository; $this->logger = $logger; - $this->request = $request ?? ObjectManager::getInstance()->get(RequestInterface::class); + $this->request = $request; } /** @@ -100,6 +100,21 @@ public function beforeExecute(ActionInterface $subject) } } + /** + * Return the shared request. + * If the request wasn't injected because of the backward compatible optional constructor dependency, + * create a new request instance. + * + * @return RequestInterface + */ + private function getRequest(): RequestInterface + { + if (null === $this->request) { + $this->request = ObjectManager::getInstance()->get(RequestInterface::class); + } + return $this->request; + } + /** * Because RequestInterface has no isPost method the check is requied before calling it. * @@ -107,7 +122,9 @@ public function beforeExecute(ActionInterface $subject) */ private function isPostRequest(): bool { - return method_exists($this->request, 'isPost') && $this->request->isPost(); + $request = $this->getRequest(); + + return method_exists($request, 'isPost') && $request->isPost(); } /** diff --git a/app/code/Magento/Customer/Model/Visitor.php b/app/code/Magento/Customer/Model/Visitor.php index abc4feb4a9ff5..3d0a3cda7a2e8 100644 --- a/app/code/Magento/Customer/Model/Visitor.php +++ b/app/code/Magento/Customer/Model/Visitor.php @@ -317,6 +317,10 @@ public function getOnlineInterval() } /** + * Return the shared request. + * If the request wasn't injected because of the backward compatible optional constructor dependency, + * create a new request instance. + * * @return \Magento\Framework\App\RequestInterface|\Magento\Framework\App\Request\Http */ private function getRequest() diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php index 59cc9d2edccf7..78bbc598baed0 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php @@ -5,6 +5,9 @@ use Magento\Backend\App\Action; use Magento\Framework\View\Result\PageFactory; +/** + * Stub inheritance based backend action controller for testing purposes. + */ class InheritanceBasedBackendAction extends Action { /** diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php index 654a0659aa77f..8d68014185e03 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php @@ -6,6 +6,9 @@ use Magento\Framework\App\Action\Context; use Magento\Framework\View\Result\PageFactory; +/** + * Stub inheritance based frontend action controller for testing purposes. + */ class InheritanceBasedFrontendAction extends Action { /** diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php index 52362601d1965..5fecd7bca0f80 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php @@ -5,6 +5,9 @@ use Magento\Framework\App\ActionInterface; use Magento\Framework\View\Result\PageFactory; +/** + * Stub interface action controller implementation for testing purposes. + */ class InterfaceOnlyBackendAction implements ActionInterface { /** diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php index 81e37d89ce332..36dd3bfd518f7 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php @@ -5,6 +5,9 @@ use Magento\Framework\App\ActionInterface; use Magento\Framework\View\Result\PageFactory; +/** + * Stub interface only based frontend action controller for testing purposes. + */ class InterfaceOnlyFrontendAction implements ActionInterface { /** diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php index 9256ab97ab7b7..98010656ff91d 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -7,7 +7,7 @@ use Magento\Framework\App\ResponseInterface; /** - * + * Do not call Action::execute() if the action flag FLAG_NO_DISPATCH is set. */ class ActionFlagNoDispatchPlugin { @@ -28,6 +28,8 @@ public function __construct(ActionFlag $actionFlag, ResponseInterface $response) } /** + * Do not call proceed if the no dispatch action flag is set. + * * @param ActionInterface $subject * @param callable $proceed * @return ResponseInterface diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index 2cc0bea8cf8e6..e7c4d610c230f 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -12,7 +12,7 @@ use Magento\Framework\HTTP\PhpEnvironment\Response; /** - * + * Dispatch the controller_action_predispatch and controller_action_post_dispatch events. */ class EventDispatchPlugin { @@ -38,12 +38,19 @@ public function __construct(RequestInterface $request, ManagerInterface $eventMa $this->actionFlag = $actionFlag; } + /** + * Trigger the controller_action_predispatch events + * + * @param ActionInterface $subject + */ public function beforeExecute(ActionInterface $subject) { $this->dispatchPreDispatchEvents($subject); } /** + * Build the event parameter array + * * @param ActionInterface $subject * @return mixed[] */ @@ -53,6 +60,8 @@ private function getEventParameters(ActionInterface $subject): array } /** + * Trigger the controller_action_postdispatch events if the suppressing action flag is not set + * * @param ActionInterface $subject * @param ResultInterface|Response|null $result * @return ResultInterface|Response|null @@ -67,6 +76,8 @@ public function afterExecute(ActionInterface $subject, $result) } /** + * Check if action flags are set that would suppress the post dispatch events. + * * @param ActionInterface $subject * @return bool * @@ -78,6 +89,8 @@ private function isSetActionNoPostDispatchFlag(): bool } /** + * Dispatch the controller_action_predispatch events. + * * @param ActionInterface $action */ private function dispatchPreDispatchEvents(ActionInterface $action) @@ -94,6 +107,8 @@ private function dispatchPreDispatchEvents(ActionInterface $action) } /** + * Dispatch the controller_action_postdispatch events. + * * @param ActionInterface $action */ private function dispatchPostDispatchEvents(ActionInterface $action) From cdac268c1d7645621951fb43cf54e4de31630a75 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 16:46:21 +0100 Subject: [PATCH 21/39] Remove comment promising to add the request as a future argument to execute because bc does not allow it --- lib/internal/Magento/Framework/App/ActionInterface.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/internal/Magento/Framework/App/ActionInterface.php b/lib/internal/Magento/Framework/App/ActionInterface.php index 921e1a2e16b1e..1501abe9eb956 100644 --- a/lib/internal/Magento/Framework/App/ActionInterface.php +++ b/lib/internal/Magento/Framework/App/ActionInterface.php @@ -25,8 +25,6 @@ interface ActionInterface /** * Execute action based on request and return result * - * Note: Request will be added as operation argument in future - * * @return \Magento\Framework\Controller\ResultInterface|ResponseInterface * @throws \Magento\Framework\Exception\NotFoundException */ From ee8da15bb05db3e00bbfb099e2f2182c0fa9f988 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 7 Feb 2018 17:34:30 +0100 Subject: [PATCH 22/39] Remove training whitespace from comment --- .../Framework/App/TestStubs/InterfaceOnlyBackendAction.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php index 5fecd7bca0f80..fd176e8163010 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php @@ -6,7 +6,7 @@ use Magento\Framework\View\Result\PageFactory; /** - * Stub interface action controller implementation for testing purposes. + * Stub interface action controller implementation for testing purposes. */ class InterfaceOnlyBackendAction implements ActionInterface { From 08466c5e53b5af8a1246b68c703d4b7c58708bf6 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 17 Jun 2018 06:35:24 +0200 Subject: [PATCH 23/39] Fix regression left over from rebase --- app/code/Magento/Customer/Model/Visitor.php | 15 ++++++++++----- .../Customer/Test/Unit/Model/VisitorTest.php | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Customer/Model/Visitor.php b/app/code/Magento/Customer/Model/Visitor.php index 3d0a3cda7a2e8..0b89aafb31cf7 100644 --- a/app/code/Magento/Customer/Model/Visitor.php +++ b/app/code/Magento/Customer/Model/Visitor.php @@ -158,6 +158,10 @@ public function initByRequest($observer) $this->setLastVisitAt((new \DateTime())->format(\Magento\Framework\Stdlib\DateTime::DATETIME_PHP_FORMAT)); + // prevent saving Visitor for safe methods, e.g. GET request + if ($this->getRequest()->isSafeMethod()) { + return $this; + } if (!$this->getId()) { $this->setSessionId($this->session->getSessionId()); $this->save(); @@ -177,7 +181,8 @@ public function initByRequest($observer) */ public function saveByRequest($observer) { - if ($this->skipRequestLogging || $this->isModuleIgnored($observer)) { + // prevent saving Visitor for safe methods, e.g. GET request + if ($this->skipRequestLogging || $this->getRequest()->isSafeMethod() || $this->isModuleIgnored($observer)) { return $this; } @@ -321,15 +326,15 @@ public function getOnlineInterval() * If the request wasn't injected because of the backward compatible optional constructor dependency, * create a new request instance. * - * @return \Magento\Framework\App\RequestInterface|\Magento\Framework\App\Request\Http + * @return \Magento\Framework\App\RequestSafetyInterface|\Magento\Framework\App\Request\Http */ private function getRequest() { - if (null === $this->request) { - $this->request = \Magento\Framework\App\ObjectManager::getInstance()->create( + if (null === $this->requestSafety) { + $this->requestSafety = \Magento\Framework\App\ObjectManager::getInstance()->create( \Magento\Framework\App\RequestInterface::class ); } - return $this->request; + return $this->requestSafety; } } diff --git a/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php b/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php index debccda26ba75..bdb2de2e99d9f 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php @@ -108,7 +108,7 @@ public function testIsModuleIgnored() 'session' => $this->session, 'resource' => $this->resource, 'ignores' => ['test_route_name' => true], - 'request' => $this->request, + 'requestSafety' => $this->request, ] ); $this->request->method('getRouteName')->willReturn('test_route_name'); From eaeafea871efa5aeba13e79c0a49fc7f8210bda6 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sun, 17 Jun 2018 08:44:08 +0200 Subject: [PATCH 24/39] Set area for test so the theme can be loaded --- .../Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php | 2 ++ .../Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php | 2 ++ .../Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php | 2 ++ .../Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php | 2 ++ 4 files changed, 8 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php index bbcbc40dc6640..cee8ebf4a0c15 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php @@ -10,6 +10,8 @@ /** * Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFiles class. + * + * @magentoAppArea adminhtml */ class DeleteFilesTest extends \PHPUnit\Framework\TestCase { diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php index 8e30e85541a42..05effba827c70 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php @@ -10,6 +10,8 @@ /** * Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFolder class. + * + * @magentoAppArea adminhtml */ class DeleteFolderTest extends \PHPUnit\Framework\TestCase { diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php index 0c74f18e9c44a..61d9a8cc42a82 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php @@ -10,6 +10,8 @@ /** * Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\NewFolder class. + * + * @magentoAppArea adminhtml */ class NewFolderTest extends \PHPUnit\Framework\TestCase { diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php index 534eb3db35b3f..b3fdcbc09f3ee 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php @@ -10,6 +10,8 @@ /** * Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\Upload class. + * + * @magentoAppArea adminhtml */ class UploadTest extends \PHPUnit\Framework\TestCase { From b6025fceaa5bfb9755e2f83366d1c152d577866b Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Wed, 20 Jun 2018 07:35:11 -0400 Subject: [PATCH 25/39] Make required changes after cherry-picking commits over from the 2.2-develop branch to the 2.3-develop branch --- app/code/Magento/Customer/Model/Visitor.php | 22 ++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Customer/Model/Visitor.php b/app/code/Magento/Customer/Model/Visitor.php index 0b89aafb31cf7..642dff9c579f6 100644 --- a/app/code/Magento/Customer/Model/Visitor.php +++ b/app/code/Magento/Customer/Model/Visitor.php @@ -6,8 +6,6 @@ namespace Magento\Customer\Model; -use Magento\Framework\Indexer\StateInterface; - /** * Class Visitor * @package Magento\Customer\Model @@ -67,6 +65,11 @@ class Visitor extends \Magento\Framework\Model\AbstractModel */ protected $indexerRegistry; + /** + * @var \Magento\Framework\App\RequestSafetyInterface + */ + private $requestSafety; + /** * @param \Magento\Framework\Model\Context $context * @param \Magento\Framework\Registry $registry @@ -80,6 +83,7 @@ class Visitor extends \Magento\Framework\Model\AbstractModel * @param array $ignoredUserAgents * @param array $ignores * @param array $data + * @param \Magento\Framework\App\RequestSafetyInterface|null $requestSafety * * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ @@ -95,7 +99,8 @@ public function __construct( \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null, array $ignoredUserAgents = [], array $ignores = [], - array $data = [] + array $data = [], + \Magento\Framework\App\RequestSafetyInterface $requestSafety = null ) { $this->session = $session; $this->httpHeader = $httpHeader; @@ -105,6 +110,7 @@ public function __construct( $this->scopeConfig = $scopeConfig; $this->dateTime = $dateTime; $this->indexerRegistry = $indexerRegistry; + $this->requestSafety = $requestSafety; } /** @@ -312,11 +318,9 @@ public function clean() */ public function getOnlineInterval() { - $configValue = intval( - $this->scopeConfig->getValue( - static::XML_PATH_ONLINE_INTERVAL, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) + $configValue = (int) $this->scopeConfig->getValue( + static::XML_PATH_ONLINE_INTERVAL, + \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); return $configValue ?: static::DEFAULT_ONLINE_MINUTES_INTERVAL; } @@ -332,7 +336,7 @@ private function getRequest() { if (null === $this->requestSafety) { $this->requestSafety = \Magento\Framework\App\ObjectManager::getInstance()->create( - \Magento\Framework\App\RequestInterface::class + \Magento\Framework\App\RequestSafetyInterface::class ); } return $this->requestSafety; From df2615a2891f2bfbe3da4ef2ef1da08e93670897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Sun, 19 Jan 2020 12:19:52 +0100 Subject: [PATCH 26/39] #16268 Amends to adopted branch --- .../Magento/Backend/App/AbstractAction.php | 252 ++++++++++-------- .../App/Action/Plugin/LoadDesignPlugin.php | 46 ++++ app/code/Magento/Backend/etc/adminhtml/di.xml | 5 +- ...CheckUserForgotPasswordBackendObserver.php | 63 +++-- .../Model/App/Action/ContextPlugin.php | 4 +- .../Model/Plugin/CustomerNotification.php | 27 +- app/code/Magento/Customer/Model/Visitor.php | 138 +++++----- .../Model/App/Action/ContextPluginTest.php | 32 ++- .../Model/Plugin/CustomerNotificationTest.php | 132 +++++---- .../Customer/Test/Unit/Model/VisitorTest.php | 101 +++---- .../Store/App/Action/Plugin/StoreCheck.php | 20 +- .../Unit/App/Action/Plugin/StoreCheckTest.php | 24 +- .../Tax/Model/App/Action/ContextPlugin.php | 56 ++-- .../Unit/App/Action/ContextPluginTest.php | 66 +++-- .../Theme/Model/Theme/Plugin/Registration.php | 12 +- .../Model/Theme/Plugin/RegistrationTest.php | 112 +++++--- .../Weee/Model/App/Action/ContextPlugin.php | 100 +++---- .../Unit/App/Action/ContextPluginTest.php | 126 +++++---- .../Directpost/Payment/PlaceTest.php | 73 +++-- .../Framework/App/ControllerActionTest.php | 29 +- .../InheritanceBasedBackendAction.php | 20 +- .../InheritanceBasedFrontendAction.php | 25 +- .../TestStubs/InterfaceOnlyBackendAction.php | 18 +- .../TestStubs/InterfaceOnlyFrontendAction.php | 23 +- .../Magento/Framework/App/Action/Action.php | 41 +-- .../Plugin/ActionFlagNoDispatchPlugin.php | 12 +- .../Framework/App/Action/Plugin/Design.php | 29 +- .../App/Action/Plugin/EventDispatchPlugin.php | 20 +- .../App/Test/Unit/Action/ActionTest.php | 77 +++--- 29 files changed, 1010 insertions(+), 673 deletions(-) create mode 100644 app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php diff --git a/app/code/Magento/Backend/App/AbstractAction.php b/app/code/Magento/Backend/App/AbstractAction.php index fb2daa283f111..583fc723cc38b 100644 --- a/app/code/Magento/Backend/App/AbstractAction.php +++ b/app/code/Magento/Backend/App/AbstractAction.php @@ -3,11 +3,24 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Backend\App; +use Magento\Backend\App\Action\Context; +use Magento\Backend\Helper\Data as BackendHelper; +use Magento\Backend\Model\Auth; +use Magento\Backend\Model\Session; +use Magento\Backend\Model\UrlInterface; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\AuthorizationInterface; +use Magento\Framework\Data\Form\FormKey\Validator as FormKeyValidator; +use Magento\Framework\Locale\ResolverInterface; +use Magento\Framework\View\Element\AbstractBlock; + /** * Generic backend controller * + * phpcs:disable Magento2.Classes.AbstractApi * @api * @SuppressWarnings(PHPMD.NumberOfChildren) * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -45,32 +58,32 @@ abstract class AbstractAction extends \Magento\Framework\App\Action\Action protected $_sessionNamespace = self::SESSION_NAMESPACE; /** - * @var \Magento\Backend\Helper\Data + * @var BackendHelper */ protected $_helper; /** - * @var \Magento\Backend\Model\Session + * @var Session */ protected $_session; /** - * @var \Magento\Framework\AuthorizationInterface + * @var AuthorizationInterface */ protected $_authorization; /** - * @var \Magento\Backend\Model\Auth + * @var Auth */ protected $_auth; /** - * @var \Magento\Backend\Model\UrlInterface + * @var UrlInterface */ protected $_backendUrl; /** - * @var \Magento\Framework\Locale\ResolverInterface + * @var ResolverInterface */ protected $_localeResolver; @@ -80,14 +93,14 @@ abstract class AbstractAction extends \Magento\Framework\App\Action\Action protected $_canUseBaseUrl; /** - * @var \Magento\Framework\Data\Form\FormKey\Validator + * @var FormKeyValidator */ protected $_formKeyValidator; /** - * @param \Magento\Backend\App\Action\Context $context + * @param Context $context */ - public function __construct(Action\Context $context) + public function __construct(Context $context) { parent::__construct($context); $this->_authorization = $context->getAuthorization(); @@ -101,6 +114,95 @@ public function __construct(Action\Context $context) } /** + * Dispatches the Action + * + * @param RequestInterface $request + * @return \Magento\Framework\App\ResponseInterface + */ + public function dispatch(RequestInterface $request) + { + if ($request->isDispatched() && $request->getActionName() !== 'denied' && !$this->_isAllowed()) { + $this->_response->setStatusHeader(403, '1.1', 'Forbidden'); + if (!$this->_auth->isLoggedIn()) { + return $this->_redirect('*/auth/login'); + } + + $this->_view->loadLayout(['default', 'adminhtml_denied'], true, true, false); + $this->_view->renderLayout(); + $this->_request->setDispatched(true); + + return $this->_response; + } + + if ($this->_isUrlChecked()) { + $this->_actionFlag->set('', self::FLAG_IS_URLS_CHECKED, true); + } + + $this->_processLocaleSettings(); + + // Need to preload isFirstPageAfterLogin (see https://github.com/magento/magento2/issues/15510) + if ($this->_auth->isLoggedIn()) { + $this->_auth->getAuthStorage()->isFirstPageAfterLogin(); + } + + return parent::dispatch($request); + } + + /** + * Check url keys. If non valid - redirect + * + * @return bool + * + * @see \Magento\Backend\App\Request\BackendValidator for default request validation. + */ + public function _processUrlKeys() + { + $_isValidFormKey = true; + $_isValidSecretKey = true; + $_keyErrorMsg = ''; + if ($this->_auth->isLoggedIn()) { + if ($this->getRequest()->isPost()) { + $_isValidFormKey = $this->_formKeyValidator->validate($this->getRequest()); + $_keyErrorMsg = __('Invalid Form Key. Please refresh the page.'); + } elseif ($this->_backendUrl->useSecretKey()) { + $_isValidSecretKey = $this->_validateSecretKey(); + $_keyErrorMsg = __('You entered an invalid Secret Key. Please refresh the page.'); + } + } + if (!$_isValidFormKey || !$_isValidSecretKey) { + $this->_actionFlag->set('', self::FLAG_NO_DISPATCH, true); + $this->_actionFlag->set('', self::FLAG_NO_POST_DISPATCH, true); + if ($this->getRequest()->getQuery('isAjax', false) || $this->getRequest()->getQuery('ajax', false)) { + $this->getResponse()->representJson( + $this->_objectManager->get( + \Magento\Framework\Json\Helper\Data::class + )->jsonEncode( + ['error' => true, 'message' => $_keyErrorMsg] + ) + ); + } else { + $this->_redirect($this->_backendUrl->getStartupPageUrl()); + } + return false; + } + return true; + } + + /** + * Generate url by route and parameters + * + * @param string $route + * @param array $params + * @return string + */ + public function getUrl($route = '', $params = []) + { + return $this->_helper->getUrl($route, $params); + } + + /** + * Determines whether current user is allowed to access Action + * * @return bool */ protected function _isAllowed() @@ -119,6 +221,8 @@ protected function _getSession() } /** + * Returns instantiated Message\ManagerInterface. + * * @return \Magento\Framework\Message\ManagerInterface */ protected function getMessageManager() @@ -146,6 +250,8 @@ protected function _setActiveMenu($itemId) } /** + * Adds element to Breadcrumbs block + * * @param string $label * @param string $title * @param string|null $link @@ -158,79 +264,51 @@ protected function _addBreadcrumb($label, $title, $link = null) } /** - * @param \Magento\Framework\View\Element\AbstractBlock $block + * Adds block to `content` block + * + * @param AbstractBlock $block * @return $this */ - protected function _addContent(\Magento\Framework\View\Element\AbstractBlock $block) + protected function _addContent(AbstractBlock $block) { return $this->_moveBlockToContainer($block, 'content'); } /** - * @param \Magento\Framework\View\Element\AbstractBlock $block + * Moves Block to `left` container + * + * @param AbstractBlock $block * @return $this */ - protected function _addLeft(\Magento\Framework\View\Element\AbstractBlock $block) + protected function _addLeft(AbstractBlock $block) { return $this->_moveBlockToContainer($block, 'left'); } /** - * @param \Magento\Framework\View\Element\AbstractBlock $block + * Adds Block to `js` container + * + * @param AbstractBlock $block * @return $this */ - protected function _addJs(\Magento\Framework\View\Element\AbstractBlock $block) + protected function _addJs(AbstractBlock $block) { return $this->_moveBlockToContainer($block, 'js'); } /** - * Set specified block as an anonymous child to specified container - * - * The block will be moved to the container from previous parent after all other elements + * Set specified block as an anonymous child to specified container. * - * @param \Magento\Framework\View\Element\AbstractBlock $block + * @param AbstractBlock $block * @param string $containerName * @return $this */ - private function _moveBlockToContainer(\Magento\Framework\View\Element\AbstractBlock $block, $containerName) + private function _moveBlockToContainer(AbstractBlock $block, $containerName) { $this->_view->getLayout()->setChild($containerName, $block->getNameInLayout(), ''); return $this; } - /** - * @param \Magento\Framework\App\RequestInterface $request - * @return \Magento\Framework\App\ResponseInterface - */ - public function dispatch(\Magento\Framework\App\RequestInterface $request) - { - if ($request->isDispatched() && $request->getActionName() !== 'denied' && !$this->_isAllowed()) { - $this->_response->setStatusHeader(403, '1.1', 'Forbidden'); - if (!$this->_auth->isLoggedIn()) { - return $this->_redirect('*/auth/login'); - } - $this->_view->loadLayout(['default', 'adminhtml_denied'], true, true, false); - $this->_view->renderLayout(); - $this->_request->setDispatched(true); - - return $this->_response; - } - - if ($this->_isUrlChecked()) { - $this->_actionFlag->set('', self::FLAG_IS_URLS_CHECKED, true); - } - - $this->_processLocaleSettings(); - - // Need to preload isFirstPageAfterLogin (see https://github.com/magento/magento2/issues/15510) - if ($this->_auth->isLoggedIn()) { - $this->_auth->getAuthStorage()->isFirstPageAfterLogin(); - } - - return parent::dispatch($request); - } - /** * Check whether url is checked * @@ -239,55 +317,13 @@ public function dispatch(\Magento\Framework\App\RequestInterface $request) protected function _isUrlChecked() { return !$this->_actionFlag->get('', self::FLAG_IS_URLS_CHECKED) - && !$this->getRequest()->isForwarded() - && !$this->_getSession()->getIsUrlNotice(true) - && !$this->_canUseBaseUrl; + && !$this->getRequest()->isForwarded() + && !$this->_getSession()->getIsUrlNotice(true) + && !$this->_canUseBaseUrl; } /** - * Check url keys. If non valid - redirect - * - * @return bool - * - * @see \Magento\Backend\App\Request\BackendValidator for default - * request validation. - */ - public function _processUrlKeys() - { - $_isValidFormKey = true; - $_isValidSecretKey = true; - $_keyErrorMsg = ''; - if ($this->_auth->isLoggedIn()) { - if ($this->getRequest()->isPost()) { - $_isValidFormKey = $this->_formKeyValidator->validate($this->getRequest()); - $_keyErrorMsg = __('Invalid Form Key. Please refresh the page.'); - } elseif ($this->_backendUrl->useSecretKey()) { - $_isValidSecretKey = $this->_validateSecretKey(); - $_keyErrorMsg = __('You entered an invalid Secret Key. Please refresh the page.'); - } - } - if (!$_isValidFormKey || !$_isValidSecretKey) { - $this->_actionFlag->set('', self::FLAG_NO_DISPATCH, true); - $this->_actionFlag->set('', self::FLAG_NO_POST_DISPATCH, true); - if ($this->getRequest()->getQuery('isAjax', false) || $this->getRequest()->getQuery('ajax', false)) { - $this->getResponse()->representJson( - $this->_objectManager->get( - \Magento\Framework\Json\Helper\Data::class - )->jsonEncode( - ['error' => true, 'message' => $_keyErrorMsg] - ) - ); - } else { - $this->_redirect($this->_backendUrl->getStartupPageUrl()); - } - return false; - } - return true; - } - - /** - * Set session locale, - * process force locale set through url params + * Set session locale, process force locale set through url params * * @return $this */ @@ -309,8 +345,8 @@ protected function _processLocaleSettings() * Set redirect into response * * @TODO MAGETWO-28356: Refactor controller actions to new ResultInterface - * @param string $path - * @param array $arguments + * @param string $path + * @param array $arguments * @return \Magento\Framework\App\ResponseInterface */ protected function _redirect($path, $arguments = []) @@ -333,19 +369,7 @@ protected function _redirect($path, $arguments = []) protected function _forward($action, $controller = null, $module = null, array $params = null) { $this->_getSession()->setIsUrlNotice($this->_actionFlag->get('', self::FLAG_IS_URLS_CHECKED)); - return parent::_forward($action, $controller, $module, $params); - } - - /** - * Generate url by route and parameters - * - * @param string $route - * @param array $params - * @return string - */ - public function getUrl($route = '', $params = []) - { - return $this->_helper->getUrl($route, $params); + parent::_forward($action, $controller, $module, $params); } /** @@ -359,7 +383,7 @@ protected function _validateSecretKey() return true; } - $secretKey = $this->getRequest()->getParam(\Magento\Backend\Model\UrlInterface::SECRET_KEY_PARAM_NAME, null); + $secretKey = $this->getRequest()->getParam(UrlInterface::SECRET_KEY_PARAM_NAME, null); if (!$secretKey || $secretKey != $this->_backendUrl->getSecretKey()) { return false; } diff --git a/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php b/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php new file mode 100644 index 0000000000000..1dee1e60b22cb --- /dev/null +++ b/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php @@ -0,0 +1,46 @@ +designLoader = $designLoader; + } + + /** + * Initiates design before dispatching Backend Actions. + * + * @param AbstractAction $backendAction + * @param array $args + * @return array + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function beforeDispatch(AbstractAction $backendAction, ...$args) + { + $this->designLoader->load(); + return $args; + } +} diff --git a/app/code/Magento/Backend/etc/adminhtml/di.xml b/app/code/Magento/Backend/etc/adminhtml/di.xml index 5f566396ab500..e39dbeb5f6680 100644 --- a/app/code/Magento/Backend/etc/adminhtml/di.xml +++ b/app/code/Magento/Backend/etc/adminhtml/di.xml @@ -60,8 +60,9 @@ - - + + + diff --git a/app/code/Magento/Captcha/Observer/CheckUserForgotPasswordBackendObserver.php b/app/code/Magento/Captcha/Observer/CheckUserForgotPasswordBackendObserver.php index e11e48a527169..21d7e2f1993b7 100644 --- a/app/code/Magento/Captcha/Observer/CheckUserForgotPasswordBackendObserver.php +++ b/app/code/Magento/Captcha/Observer/CheckUserForgotPasswordBackendObserver.php @@ -3,19 +3,28 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Captcha\Observer; +use Magento\Captcha\Helper\Data as CaptchaHelper; +use Magento\Framework\App\Action\Action; +use Magento\Framework\App\ActionFlag; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\Event\Observer as Event; use Magento\Framework\Event\ObserverInterface; +use Magento\Framework\Message\ManagerInterface; +use Magento\Framework\Session\SessionManagerInterface; /** - * Class CheckUserForgotPasswordBackendObserver + * Handle request for Forgot Password * * @SuppressWarnings(PHPMD.CookieAndSessionMisuse) */ class CheckUserForgotPasswordBackendObserver implements ObserverInterface { /** - * @var \Magento\Captcha\Helper\Data + * @var CaptchaHelper */ protected $_helper; @@ -25,62 +34,70 @@ class CheckUserForgotPasswordBackendObserver implements ObserverInterface protected $captchaStringResolver; /** - * @var \Magento\Framework\Session\SessionManagerInterface + * @var SessionManagerInterface */ protected $_session; /** - * @var \Magento\Framework\App\ActionFlag + * @var ActionFlag */ protected $_actionFlag; /** - * @var \Magento\Framework\Message\ManagerInterface + * @var ManagerInterface */ protected $messageManager; /** - * @param \Magento\Captcha\Helper\Data $helper + * @var RequestInterface + */ + private $request; + + /** + * @param CaptchaHelper $helper * @param CaptchaStringResolver $captchaStringResolver - * @param \Magento\Framework\Session\SessionManagerInterface $session - * @param \Magento\Framework\App\ActionFlag $actionFlag - * @param \Magento\Framework\Message\ManagerInterface $messageManager + * @param SessionManagerInterface $session + * @param ActionFlag $actionFlag + * @param ManagerInterface $messageManager + * @param RequestInterface|null $request */ public function __construct( - \Magento\Captcha\Helper\Data $helper, + CaptchaHelper $helper, CaptchaStringResolver $captchaStringResolver, - \Magento\Framework\Session\SessionManagerInterface $session, - \Magento\Framework\App\ActionFlag $actionFlag, - \Magento\Framework\Message\ManagerInterface $messageManager + SessionManagerInterface $session, + ActionFlag $actionFlag, + ManagerInterface $messageManager, + RequestInterface $request = null ) { $this->_helper = $helper; $this->captchaStringResolver = $captchaStringResolver; $this->_session = $session; $this->_actionFlag = $actionFlag; $this->messageManager = $messageManager; + $this->request = $request ?? ObjectManager::getInstance()->get(RequestInterface::class); } /** * Check Captcha On User Login Backend Page * - * @param \Magento\Framework\Event\Observer $observer - * @throws \Magento\Framework\Exception\Plugin\AuthenticationException + * @param Event $observer * @return $this + * @throws \Magento\Framework\Exception\Plugin\AuthenticationException */ - public function execute(\Magento\Framework\Event\Observer $observer) + public function execute(Event $observer) { $formId = 'backend_forgotpassword'; $captchaModel = $this->_helper->getCaptcha($formId); $controller = $observer->getControllerAction(); - $email = (string)$observer->getControllerAction()->getRequest()->getParam('email'); - $params = $observer->getControllerAction()->getRequest()->getParams(); - if (!empty($email) - && !empty($params) + $params = $this->request->getParams(); + $email = (string)$this->request->getParam('email'); + if (!empty($params) + && !empty($email) && $captchaModel->isRequired() - && !$captchaModel->isCorrect($this->captchaStringResolver->resolve($controller->getRequest(), $formId)) + && !$captchaModel->isCorrect($this->captchaStringResolver->resolve($this->request, $formId)) ) { - $this->_session->setEmail((string)$controller->getRequest()->getPost('email')); - $this->_actionFlag->set('', \Magento\Framework\App\Action\Action::FLAG_NO_DISPATCH, true); + $this->_session->setEmail($email); + $this->_actionFlag->set('', Action::FLAG_NO_DISPATCH, true); $this->messageManager->addErrorMessage(__('Incorrect CAPTCHA')); $controller->getResponse()->setRedirect( $controller->getUrl('*/*/forgotpassword', ['_nosecret' => true]) diff --git a/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php b/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php index f221949051393..5d926b47ca446 100644 --- a/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php +++ b/app/code/Magento/Customer/Model/App/Action/ContextPlugin.php @@ -8,12 +8,12 @@ use Magento\Customer\Model\Context; use Magento\Customer\Model\GroupManagement; -use Magento\Framework\App\ActionInterface; use Magento\Customer\Model\Session; +use Magento\Framework\App\ActionInterface; use Magento\Framework\App\Http\Context as HttpContext; /** - * Class ContextPlugin + * Introduces Context information for ActionInterface of Customer Action */ class ContextPlugin { diff --git a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php index bdf1a695589cf..aa821c9355777 100644 --- a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php +++ b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php @@ -11,12 +11,16 @@ use Magento\Customer\Model\Session; use Magento\Framework\App\ActionInterface; use Magento\Framework\App\Area; +use Magento\Framework\App\HttpRequestInterface; use Magento\Framework\App\ObjectManager; use Magento\Framework\App\RequestInterface; use Magento\Framework\App\State; use Magento\Framework\Exception\NoSuchEntityException; use Psr\Log\LoggerInterface; +/** + * Refresh the Customer session if `UpdateSession` notification registered + */ class CustomerNotification { /** @@ -72,7 +76,7 @@ public function __construct( $this->state = $state; $this->customerRepository = $customerRepository; $this->logger = $logger; - $this->request = $request; + $this->request = $request ?? ObjectManager::getInstance()->get(RequestInterface::class); } /** @@ -100,21 +104,6 @@ public function beforeExecute(ActionInterface $subject) } } - /** - * Return the shared request. - * If the request wasn't injected because of the backward compatible optional constructor dependency, - * create a new request instance. - * - * @return RequestInterface - */ - private function getRequest(): RequestInterface - { - if (null === $this->request) { - $this->request = ObjectManager::getInstance()->get(RequestInterface::class); - } - return $this->request; - } - /** * Because RequestInterface has no isPost method the check is requied before calling it. * @@ -122,9 +111,7 @@ private function getRequest(): RequestInterface */ private function isPostRequest(): bool { - $request = $this->getRequest(); - - return method_exists($request, 'isPost') && $request->isPost(); + return $this->request instanceof HttpRequestInterface && $this->request->isPost(); } /** @@ -135,7 +122,7 @@ private function isPostRequest(): bool */ private function isFrontendRequest(): bool { - return $this->state->getAreaCode() == Area::AREA_FRONTEND; + return $this->state->getAreaCode() === Area::AREA_FRONTEND; } /** diff --git a/app/code/Magento/Customer/Model/Visitor.php b/app/code/Magento/Customer/Model/Visitor.php index f00bb581509d2..2eb5520f7aa08 100644 --- a/app/code/Magento/Customer/Model/Visitor.php +++ b/app/code/Magento/Customer/Model/Visitor.php @@ -6,8 +6,23 @@ namespace Magento\Customer\Model; +use Magento\Customer\Api\Data\CustomerInterface; +use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\App\ObjectManager; +use Magento\Framework\App\Request; use Magento\Framework\App\RequestSafetyInterface; +use Magento\Framework\Data\Collection\AbstractDb; +use Magento\Framework\Event\Observer as EventObserver; +use Magento\Framework\HTTP\Header; +use Magento\Framework\Indexer\IndexerRegistry; +use Magento\Framework\Model\AbstractModel; +use Magento\Framework\Model\Context as ModelContext; +use Magento\Framework\Model\ResourceModel\AbstractResource; +use Magento\Framework\Registry; +use Magento\Framework\Session\Config as SessionConfig; +use Magento\Framework\Session\SessionManagerInterface; +use Magento\Framework\Stdlib\DateTime; +use Magento\Store\Model\ScopeInterface; /** * Class Visitor @@ -17,15 +32,13 @@ * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.CookieAndSessionMisuse) */ -class Visitor extends \Magento\Framework\Model\AbstractModel +class Visitor extends AbstractModel { const VISITOR_TYPE_CUSTOMER = 'c'; - const VISITOR_TYPE_VISITOR = 'v'; - const DEFAULT_ONLINE_MINUTES_INTERVAL = 15; - const XML_PATH_ONLINE_INTERVAL = 'customer/online_customers/online_minutes_interval'; + const SECONDS_24_HOURS = 86400; /** * @var string[] @@ -33,12 +46,12 @@ class Visitor extends \Magento\Framework\Model\AbstractModel protected $ignoredUserAgents; /** - * @var \Magento\Framework\Session\SessionManagerInterface + * @var SessionManagerInterface */ protected $session; /** - * @var \Magento\Framework\HTTP\Header + * @var Header */ protected $httpHeader; @@ -57,17 +70,17 @@ class Visitor extends \Magento\Framework\Model\AbstractModel /** * Core store config * - * @var \Magento\Framework\App\Config\ScopeConfigInterface + * @var ScopeConfigInterface */ protected $scopeConfig; /** - * @var \Magento\Framework\Stdlib\DateTime + * @var DateTime */ protected $dateTime; /** - * @var \Magento\Framework\Indexer\IndexerRegistry + * @var IndexerRegistry */ protected $indexerRegistry; @@ -77,15 +90,15 @@ class Visitor extends \Magento\Framework\Model\AbstractModel private $requestSafety; /** - * @param \Magento\Framework\Model\Context $context - * @param \Magento\Framework\Registry $registry - * @param \Magento\Framework\Session\SessionManagerInterface $session - * @param \Magento\Framework\HTTP\Header $httpHeader - * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig - * @param \Magento\Framework\Stdlib\DateTime $dateTime - * @param \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry - * @param \Magento\Framework\Model\ResourceModel\AbstractResource|null $resource - * @param \Magento\Framework\Data\Collection\AbstractDb|null $resourceCollection + * @param ModelContext $context + * @param Registry $registry + * @param SessionManagerInterface $session + * @param Header $httpHeader + * @param ScopeConfigInterface $scopeConfig + * @param DateTime $dateTime + * @param IndexerRegistry $indexerRegistry + * @param AbstractResource|null $resource + * @param AbstractDb|null $resourceCollection * @param array $ignoredUserAgents * @param array $ignores * @param array $data @@ -94,15 +107,15 @@ class Visitor extends \Magento\Framework\Model\AbstractModel * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( - \Magento\Framework\Model\Context $context, - \Magento\Framework\Registry $registry, - \Magento\Framework\Session\SessionManagerInterface $session, - \Magento\Framework\HTTP\Header $httpHeader, - \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, - \Magento\Framework\Stdlib\DateTime $dateTime, - \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry, - \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null, - \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null, + ModelContext $context, + Registry $registry, + SessionManagerInterface $session, + Header $httpHeader, + ScopeConfigInterface $scopeConfig, + DateTime $dateTime, + IndexerRegistry $indexerRegistry, + AbstractResource $resource = null, + AbstractDb $resourceCollection = null, array $ignoredUserAgents = [], array $ignores = [], array $data = [], @@ -116,7 +129,7 @@ public function __construct( $this->scopeConfig = $scopeConfig; $this->dateTime = $dateTime; $this->indexerRegistry = $indexerRegistry; - $this->requestSafety = $requestSafety ?? ObjectManager::getInstance()->get(RequestSafetyInterface::class); + $this->requestSafety = $requestSafety ?? ObjectManager::getInstance()->create(RequestSafetyInterface::class); } /** @@ -126,7 +139,7 @@ public function __construct( */ protected function _construct() { - $this->_init(\Magento\Customer\Model\ResourceModel\Visitor::class); + $this->_init(ResourceModel\Visitor::class); $userAgent = $this->httpHeader->getHttpUserAgent(); if ($this->ignoredUserAgents) { if (in_array($userAgent, $this->ignoredUserAgents)) { @@ -139,7 +152,7 @@ protected function _construct() * Skip request logging * * @param bool $skipRequestLogging - * @return \Magento\Customer\Model\Visitor + * @return Visitor */ public function setSkipRequestLogging($skipRequestLogging) { @@ -148,12 +161,10 @@ public function setSkipRequestLogging($skipRequestLogging) } /** - * Initialization visitor by request + * Initialization visitor by request. Used in event "controller_action_predispatch" * - * Used in event "controller_action_predispatch" - * - * @param \Magento\Framework\Event\Observer $observer - * @return \Magento\Customer\Model\Visitor + * @param EventObserver $observer + * @return Visitor */ public function initByRequest($observer) { @@ -168,7 +179,7 @@ public function initByRequest($observer) } } - $this->setLastVisitAt((new \DateTime())->format(\Magento\Framework\Stdlib\DateTime::DATETIME_PHP_FORMAT)); + $this->setLastVisitAt((new \DateTime())->format(DateTime::DATETIME_PHP_FORMAT)); // prevent saving Visitor for safe methods, e.g. GET request if ($this->requestSafety->isSafeMethod()) { @@ -189,13 +200,13 @@ public function initByRequest($observer) * * Used in event "controller_action_postdispatch" * - * @param \Magento\Framework\Event\Observer $observer - * @return \Magento\Customer\Model\Visitor + * @param EventObserver $observer + * @return Visitor */ public function saveByRequest($observer) { // prevent saving Visitor for safe methods, e.g. GET request - if ($this->skipRequestLogging || $this->getRequest()->isSafeMethod() || $this->isModuleIgnored($observer)) { + if ($this->skipRequestLogging || $this->requestSafety->isSafeMethod() || $this->isModuleIgnored($observer)) { return $this; } @@ -215,13 +226,13 @@ public function saveByRequest($observer) /** * Returns true if the module is required * - * @param \Magento\Framework\Event\Observer $observer + * @param EventObserver $observer * @return bool */ public function isModuleIgnored($observer) { if (is_array($this->ignores) && $observer) { - $curModule = $this->getRequest()->getRouteName(); + $curModule = $this->requestSafety->getRouteName(); if (isset($this->ignores[$curModule])) { return true; } @@ -234,12 +245,12 @@ public function isModuleIgnored($observer) * * Used in event "customer_login" * - * @param \Magento\Framework\Event\Observer $observer - * @return \Magento\Customer\Model\Visitor + * @param EventObserver $observer + * @return Visitor */ public function bindCustomerLogin($observer) { - /** @var \Magento\Customer\Api\Data\CustomerInterface $customer */ + /** @var CustomerInterface $customer */ $customer = $observer->getEvent()->getCustomer(); if (!$this->getCustomerId()) { $this->setDoCustomerLogin(true); @@ -253,8 +264,8 @@ public function bindCustomerLogin($observer) * * Used in event "customer_logout" * - * @param \Magento\Framework\Event\Observer $observer - * @return \Magento\Customer\Model\Visitor + * @param EventObserver $observer + * @return Visitor * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function bindCustomerLogout($observer) @@ -268,8 +279,8 @@ public function bindCustomerLogout($observer) /** * Create binding of checkout quote * - * @param \Magento\Framework\Event\Observer $observer - * @return \Magento\Customer\Model\Visitor + * @param EventObserver $observer + * @return Visitor */ public function bindQuoteCreate($observer) { @@ -286,8 +297,8 @@ public function bindQuoteCreate($observer) /** * Destroy binding of checkout quote * - * @param \Magento\Framework\Event\Observer $observer - * @return \Magento\Customer\Model\Visitor + * @param EventObserver $observer + * @return Visitor */ public function bindQuoteDestroy($observer) { @@ -305,10 +316,10 @@ public function bindQuoteDestroy($observer) */ public function getCleanTime() { - return $this->scopeConfig->getValue( - \Magento\Framework\Session\Config::XML_PATH_COOKIE_LIFETIME, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) + 86400; + return self::SECONDS_24_HOURS + $this->scopeConfig->getValue( + SessionConfig::XML_PATH_COOKIE_LIFETIME, + ScopeInterface::SCOPE_STORE + ); } /** @@ -331,25 +342,8 @@ public function getOnlineInterval() { $configValue = (int)$this->scopeConfig->getValue( static::XML_PATH_ONLINE_INTERVAL, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE + ScopeInterface::SCOPE_STORE ); return $configValue ?: static::DEFAULT_ONLINE_MINUTES_INTERVAL; } - - /** - * Return the shared request. - * If the request wasn't injected because of the backward compatible optional constructor dependency, - * create a new request instance. - * - * @return \Magento\Framework\App\RequestSafetyInterface|\Magento\Framework\App\Request\Http - */ - private function getRequest() - { - if (null === $this->requestSafety) { - $this->requestSafety = \Magento\Framework\App\ObjectManager::getInstance()->create( - \Magento\Framework\App\RequestSafetyInterface::class - ); - } - return $this->requestSafety; - } } diff --git a/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php b/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php index 1b70f174c39fd..4e722a36b57da 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/App/Action/ContextPluginTest.php @@ -6,30 +6,38 @@ namespace Magento\Customer\Test\Unit\Model\App\Action; +use Magento\Customer\Model\App\Action\ContextPlugin; use Magento\Customer\Model\Context; +use Magento\Customer\Model\Session; +use Magento\Framework\App\Action\Action; +use Magento\Framework\App\Http\Context as HttpContext; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; /** - * Class ContextPluginTest + * Unit Tests to cover ContextPlugin for Action Context */ -class ContextPluginTest extends \PHPUnit\Framework\TestCase +class ContextPluginTest extends TestCase { + const STUB_CUSTOMER_GROUP = 'UAH'; + const STUB_CUSTOMER_NOT_LOGGED_IN = 0; /** - * @var \Magento\Customer\Model\App\Action\ContextPlugin + * @var ContextPlugin */ protected $plugin; /** - * @var \Magento\Customer\Model\Session|\PHPUnit_Framework_MockObject_MockObject + * @var Session|MockObject */ protected $customerSessionMock; /** - * @var \Magento\Framework\App\Http\Context|\PHPUnit_Framework_MockObject_MockObject + * @var HttpContext|MockObject */ protected $httpContextMock; /** - * @var \Magento\Framework\App\Action\Action|\PHPUnit_Framework_MockObject_MockObject + * @var Action|MockObject */ protected $subjectMock; @@ -38,10 +46,10 @@ class ContextPluginTest extends \PHPUnit\Framework\TestCase */ protected function setUp() { - $this->customerSessionMock = $this->createMock(\Magento\Customer\Model\Session::class); - $this->httpContextMock = $this->createMock(\Magento\Framework\App\Http\Context::class); - $this->subjectMock = $this->createMock(\Magento\Framework\App\Action\Action::class); - $this->plugin = new \Magento\Customer\Model\App\Action\ContextPlugin( + $this->customerSessionMock = $this->createMock(Session::class); + $this->httpContextMock = $this->createMock(HttpContext::class); + $this->subjectMock = $this->createMock(Action::class); + $this->plugin = new ContextPlugin( $this->customerSessionMock, $this->httpContextMock ); @@ -60,8 +68,8 @@ public function testBeforeExecute() ->will( $this->returnValueMap( [ - [Context::CONTEXT_GROUP, 'UAH', $this->httpContextMock], - [Context::CONTEXT_AUTH, 0, $this->httpContextMock], + [Context::CONTEXT_GROUP, self::STUB_CUSTOMER_GROUP, $this->httpContextMock], + [Context::CONTEXT_AUTH, self::STUB_CUSTOMER_NOT_LOGGED_IN, $this->httpContextMock], ] ) ); diff --git a/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php b/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php index 27999e903d900..86bcf59bdd113 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php @@ -3,6 +3,7 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Customer\Test\Unit\Model\Plugin; use Magento\Customer\Api\CustomerRepositoryInterface; @@ -12,97 +13,116 @@ use Magento\Customer\Model\Session; use Magento\Framework\App\ActionInterface; use Magento\Framework\App\Area; +use Magento\Framework\App\HttpRequestInterface; use Magento\Framework\App\RequestInterface; use Magento\Framework\App\State; use Magento\Framework\Exception\NoSuchEntityException; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; class CustomerNotificationTest extends \PHPUnit\Framework\TestCase { - /** @var Session|\PHPUnit_Framework_MockObject_MockObject */ - private $session; - - /** @var NotificationStorage|\PHPUnit_Framework_MockObject_MockObject */ - private $notificationStorage; - - /** @var CustomerRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $customerRepository; - - /** @var State|\PHPUnit_Framework_MockObject_MockObject */ - private $appState; + private const STUB_CUSTOMER_ID = 1; + + /** + * @var Session|MockObject + */ + private $sessionMock; + + /** + * @var NotificationStorage|MockObject + */ + private $notificationStorageMock; + + /** + * @var CustomerRepositoryInterface|MockObject + */ + private $customerRepositoryMock; + + /** + * @var State|MockObject + */ + private $appStateMock; + + /** + * @var RequestInterface|MockObject + */ + private $requestMock; + + /** + * @var ActionInterface|MockObject + */ + private $actionMock; + + /** + * @var LoggerInterface|MockObject + */ + private $loggerMock; + + /** + * @var CustomerNotification + */ + private $plugin; - /** @var RequestInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $request; + protected function setUp() + { + $this->sessionMock = $this->createMock(Session::class); + $this->sessionMock->method('getCustomerId')->willReturn(self::STUB_CUSTOMER_ID); - /** @var ActionInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $actionInterfaceMock; - - /** @var LoggerInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $logger; + $this->customerRepositoryMock = $this->createMock(CustomerRepositoryInterface::class); + $this->actionMock = $this->createMock(ActionInterface::class); + $this->requestMock = $this->getMockBuilder([RequestInterface::class, HttpRequestInterface::class]) + ->getMock(); + $this->requestMock->method('isPost')->willReturn(true); - /** @var CustomerNotification */ - private $plugin; + $this->loggerMock = $this->createMock(LoggerInterface::class); - /** @var int */ - private static $customerId = 1; + $this->appStateMock = $this->createMock(State::class); + $this->appStateMock->method('getAreaCode')->willReturn(Area::AREA_FRONTEND); - protected function setUp() - { - $this->session = $this->createMock(Session::class); - $this->notificationStorage = $this->createMock(NotificationStorage::class); - $this->customerRepository = $this->createMock(CustomerRepositoryInterface::class); - $this->actionInterfaceMock = $this->createMock(ActionInterface::class); - $this->request = $this->getMockBuilder(RequestInterface::class) - ->setMethods(['isPost']) - ->getMockForAbstractClass(); - $this->appState = $this->createMock(State::class); - $this->logger = $this->createMock(LoggerInterface::class); - - $this->appState->method('getAreaCode')->willReturn(Area::AREA_FRONTEND); - $this->request->method('isPost')->willReturn(true); - $this->session->method('getCustomerId')->willReturn(self::$customerId); - $this->notificationStorage->expects($this->any()) + $this->notificationStorageMock = $this->createMock(NotificationStorage::class); + $this->notificationStorageMock->expects($this->any()) ->method('isExists') - ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::$customerId) + ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::STUB_CUSTOMER_ID) ->willReturn(true); $this->plugin = new CustomerNotification( - $this->session, - $this->notificationStorage, - $this->appState, - $this->customerRepository, - $this->logger, - $this->request + $this->sessionMock, + $this->notificationStorageMock, + $this->appStateMock, + $this->customerRepositoryMock, + $this->loggerMock, + $this->requestMock ); } public function testBeforeExecute() { - $customerGroupId =1; + $customerGroupId = 1; $customerMock = $this->createMock(CustomerInterface::class); $customerMock->method('getGroupId')->willReturn($customerGroupId); - $customerMock->method('getId')->willReturn(self::$customerId); + $customerMock->method('getId')->willReturn(self::STUB_CUSTOMER_ID); - $this->customerRepository->expects($this->once()) + $this->customerRepositoryMock->expects($this->once()) ->method('getById') - ->with(self::$customerId) + ->with(self::STUB_CUSTOMER_ID) ->willReturn($customerMock); - $this->notificationStorage->expects($this->once()) + $this->notificationStorageMock->expects($this->once()) ->method('remove') - ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::$customerId); + ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::STUB_CUSTOMER_ID); - $this->plugin->beforeExecute($this->actionInterfaceMock); + $this->plugin->beforeExecute($this->actionMock); } public function testBeforeDispatchWithNoCustomerFound() { - $this->customerRepository->method('getById') - ->with(self::$customerId) + $this->customerRepositoryMock->method('getById') + ->with(self::STUB_CUSTOMER_ID) ->willThrowException(new NoSuchEntityException()); - $this->logger->expects($this->once()) + $this->loggerMock->expects($this->once()) ->method('error'); - $this->plugin->beforeExecute($this->actionInterfaceMock); + $this->plugin->beforeExecute($this->actionMock); } } diff --git a/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php b/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php index bdb2de2e99d9f..c9cd525d71c49 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/VisitorTest.php @@ -6,16 +6,24 @@ namespace Magento\Customer\Test\Unit\Model; +use Magento\Customer\Model\ResourceModel\Visitor as VisitorResourceModel; +use Magento\Customer\Model\Session; +use Magento\Customer\Model\Visitor as VisitorModel; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Framework\DataObject; +use Magento\Framework\Registry; +use Magento\Framework\Session\SessionManagerInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; /** - * Class VisitorTest - * @package Magento\Customer\Model + * Unit Tests to cover Visitor Model */ -class VisitorTest extends \PHPUnit\Framework\TestCase +class VisitorTest extends TestCase { /** - * @var \Magento\Customer\Model\Visitor + * @var VisitorModel */ protected $visitor; @@ -25,37 +33,37 @@ class VisitorTest extends \PHPUnit\Framework\TestCase protected $objectManagerHelper; /** - * @var \Magento\Framework\Registry|\PHPUnit_Framework_MockObject_MockObject + * @var Registry|MockObject */ - protected $registry; + protected $registryMock; /** - * @var \Magento\Customer\Model\ResourceModel\Visitor|\PHPUnit_Framework_MockObject_MockObject + * @var VisitorResourceModel|MockObject */ - protected $resource; + protected $visitorResourceModelMock; /** - * @var \Magento\Framework\Session\SessionManagerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var SessionManagerInterface|MockObject */ - protected $session; + protected $sessionMock; /** - * @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject + * @var HttpRequest|MockObject */ - private $request; + private $httpRequestMock; protected function setUp() { - $this->registry = $this->createMock(\Magento\Framework\Registry::class); - $this->session = $this->getMockBuilder(\Magento\Customer\Model\Session::class) + $this->registryMock = $this->createMock(Registry::class); + $this->sessionMock = $this->getMockBuilder(Session::class) ->disableOriginalConstructor() ->setMethods(['getSessionId', 'getVisitorData', 'setVisitorData']) ->getMock(); - $this->request = $this->createMock(\Magento\Framework\App\Request\Http::class); + $this->httpRequestMock = $this->createMock(HttpRequest::class); $this->objectManagerHelper = new ObjectManagerHelper($this); - $this->resource = $this->getMockBuilder(\Magento\Customer\Model\ResourceModel\Visitor::class) + $this->visitorResourceModelMock = $this->getMockBuilder(VisitorResourceModel::class) ->setMethods([ 'beginTransaction', '__sleep', @@ -66,28 +74,28 @@ protected function setUp() 'commit', 'clean', ])->disableOriginalConstructor()->getMock(); - $this->resource->expects($this->any())->method('getIdFieldName')->willReturn('visitor_id'); - $this->resource->expects($this->any())->method('addCommitCallback')->willReturnSelf(); + $this->visitorResourceModelMock->expects($this->any())->method('getIdFieldName')->willReturn('visitor_id'); + $this->visitorResourceModelMock->expects($this->any())->method('addCommitCallback')->willReturnSelf(); $arguments = $this->objectManagerHelper->getConstructArguments( - \Magento\Customer\Model\Visitor::class, + VisitorModel::class, [ - 'registry' => $this->registry, - 'session' => $this->session, - 'resource' => $this->resource, - 'request' => $this->request, + 'registry' => $this->registryMock, + 'session' => $this->sessionMock, + 'resource' => $this->visitorResourceModelMock, + 'request' => $this->httpRequestMock, ] ); - $this->visitor = $this->objectManagerHelper->getObject(\Magento\Customer\Model\Visitor::class, $arguments); + $this->visitor = $this->objectManagerHelper->getObject(VisitorModel::class, $arguments); } public function testInitByRequest() { $oldSessionId = 'asdfhasdfjhkj2198sadf8sdf897'; $newSessionId = 'bsdfhasdfjhkj2198sadf8sdf897'; - $this->session->expects($this->any())->method('getSessionId')->willReturn($newSessionId); - $this->session->expects($this->atLeastOnce())->method('getVisitorData') + $this->sessionMock->expects($this->any())->method('getSessionId')->willReturn($newSessionId); + $this->sessionMock->expects($this->atLeastOnce())->method('getVisitorData') ->willReturn(['session_id' => $oldSessionId]); $this->visitor->initByRequest(null); $this->assertEquals($newSessionId, $this->visitor->getSessionId()); @@ -95,32 +103,32 @@ public function testInitByRequest() public function testSaveByRequest() { - $this->session->expects($this->once())->method('setVisitorData')->will($this->returnSelf()); + $this->sessionMock->expects($this->once())->method('setVisitorData')->will($this->returnSelf()); $this->assertSame($this->visitor, $this->visitor->saveByRequest(null)); } public function testIsModuleIgnored() { $this->visitor = $this->objectManagerHelper->getObject( - \Magento\Customer\Model\Visitor::class, + VisitorModel::class, [ - 'registry' => $this->registry, - 'session' => $this->session, - 'resource' => $this->resource, + 'registry' => $this->registryMock, + 'session' => $this->sessionMock, + 'resource' => $this->visitorResourceModelMock, 'ignores' => ['test_route_name' => true], - 'requestSafety' => $this->request, + 'requestSafety' => $this->httpRequestMock, ] ); - $this->request->method('getRouteName')->willReturn('test_route_name'); - $observer = new \Magento\Framework\DataObject(); + $this->httpRequestMock->method('getRouteName')->willReturn('test_route_name'); + $observer = new DataObject(); $this->assertTrue($this->visitor->isModuleIgnored($observer)); } public function testBindCustomerLogin() { - $customer = new \Magento\Framework\DataObject(['id' => '1']); - $observer = new \Magento\Framework\DataObject([ - 'event' => new \Magento\Framework\DataObject(['customer' => $customer]), + $customer = new DataObject(['id' => '1']); + $observer = new DataObject([ + 'event' => new DataObject(['customer' => $customer]), ]); $this->visitor->bindCustomerLogin($observer); @@ -136,7 +144,7 @@ public function testBindCustomerLogin() public function testBindCustomerLogout() { - $observer = new \Magento\Framework\DataObject(); + $observer = new DataObject(); $this->visitor->setCustomerId('1'); $this->visitor->bindCustomerLogout($observer); @@ -149,9 +157,9 @@ public function testBindCustomerLogout() public function testBindQuoteCreate() { - $quote = new \Magento\Framework\DataObject(['id' => '1', 'is_checkout_cart' => true]); - $observer = new \Magento\Framework\DataObject([ - 'event' => new \Magento\Framework\DataObject(['quote' => $quote]), + $quote = new DataObject(['id' => '1', 'is_checkout_cart' => true]); + $observer = new DataObject([ + 'event' => new DataObject(['quote' => $quote]), ]); $this->visitor->bindQuoteCreate($observer); $this->assertTrue($this->visitor->getDoQuoteCreate()); @@ -159,9 +167,9 @@ public function testBindQuoteCreate() public function testBindQuoteDestroy() { - $quote = new \Magento\Framework\DataObject(['id' => '1']); - $observer = new \Magento\Framework\DataObject([ - 'event' => new \Magento\Framework\DataObject(['quote' => $quote]), + $quote = new DataObject(['id' => '1']); + $observer = new DataObject([ + 'event' => new DataObject(['quote' => $quote]), ]); $this->visitor->bindQuoteDestroy($observer); $this->assertTrue($this->visitor->getDoQuoteDestroy()); @@ -169,7 +177,10 @@ public function testBindQuoteDestroy() public function testClean() { - $this->resource->expects($this->once())->method('clean')->with($this->visitor)->willReturnSelf(); + $this->visitorResourceModelMock->expects($this->once()) + ->method('clean') + ->with($this->visitor) + ->willReturnSelf(); $this->visitor->clean(); } } diff --git a/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php b/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php index ce32498175ff4..6843e0303edd9 100644 --- a/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php +++ b/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php @@ -1,6 +1,5 @@ _storeManager = $storeManager; } /** + * Verify before execute + * * @param ActionInterface $subject * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) - * @throws \Magento\Framework\Exception\State\InitException + * @throws \Magento\Framework\Exception\State\InitExceptionn */ public function beforeExecute(ActionInterface $subject) { - if (! $this->_storeManager->getStore()->isActive()) { - throw new \Magento\Framework\Exception\State\InitException( + if (!$this->_storeManager->getStore()->isActive()) { + throw new InitException( __('Current store is not active.') ); } diff --git a/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php b/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php index b24db7aa4a269..9ecd640b246af 100644 --- a/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php +++ b/app/code/Magento/Store/Test/Unit/App/Action/Plugin/StoreCheckTest.php @@ -5,32 +5,38 @@ */ namespace Magento\Store\Test\Unit\App\Action\Plugin; +use Magento\Framework\App\Action\AbstractAction; +use Magento\Store\App\Action\Plugin\StoreCheck; +use Magento\Store\Model\Store; +use Magento\Store\Model\StoreManagerInterface; +use PHPUnit\Framework\MockObject\MockObject; + class StoreCheckTest extends \PHPUnit\Framework\TestCase { /** - * @var \Magento\Store\App\Action\Plugin\StoreCheck + * @var StoreCheck */ protected $_plugin; /** - * @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var StoreManagerInterface|MockObject */ protected $_storeManagerMock; /** - * @var \Magento\Store\Model\Store|\PHPUnit_Framework_MockObject_MockObject + * @var Store|MockObject */ protected $_storeMock; /** - * @var \Magento\Framework\App\Action\AbstractAction|\PHPUnit_Framework_MockObject_MockObject + * @var AbstractAction|MockObject */ protected $subjectMock; - + protected function setUp() { - $this->_storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); - $this->_storeMock = $this->createMock(\Magento\Store\Model\Store::class); + $this->_storeManagerMock = $this->createMock(StoreManagerInterface::class); + $this->_storeMock = $this->createMock(Store::class); $this->_storeManagerMock->expects( $this->any() )->method( @@ -38,11 +44,11 @@ protected function setUp() )->will( $this->returnValue($this->_storeMock) ); - $this->subjectMock = $this->getMockBuilder(\Magento\Framework\App\Action\AbstractAction::class) + $this->subjectMock = $this->getMockBuilder(AbstractAction::class) ->disableOriginalConstructor() ->getMockForAbstractClass(); - $this->_plugin = new \Magento\Store\App\Action\Plugin\StoreCheck($this->_storeManagerMock); + $this->_plugin = new StoreCheck($this->_storeManagerMock); } /** diff --git a/app/code/Magento/Tax/Model/App/Action/ContextPlugin.php b/app/code/Magento/Tax/Model/App/Action/ContextPlugin.php index e8c0897e65c58..d768e2f6092a0 100644 --- a/app/code/Magento/Tax/Model/App/Action/ContextPlugin.php +++ b/app/code/Magento/Tax/Model/App/Action/ContextPlugin.php @@ -6,60 +6,68 @@ namespace Magento\Tax\Model\App\Action; +use Magento\Customer\Model\Session; +use Magento\Framework\App\ActionInterface; +use Magento\Framework\App\Http\Context as HttpContext; +use Magento\Framework\Module\Manager as ModuleManager; +use Magento\PageCache\Model\Config as PageCacheConfig; +use Magento\Tax\Helper\Data as TaxHelper; +use Magento\Tax\Model\Calculation; + /** - * Class ContextPlugin + * Provides Action Context on before executing Controller Action */ class ContextPlugin { /** - * @var \Magento\Customer\Model\Session + * @var Session */ protected $customerSession; /** - * @var \Magento\Framework\App\Http\Context + * @var HttpContext */ protected $httpContext; /** - * @var \Magento\Tax\Helper\Data + * @var TaxHelper */ protected $taxHelper; /** - * @var \Magento\Tax\Model\Calculation\Proxy + * @var Calculation */ protected $taxCalculation; /** * Module manager * - * @var \Magento\Framework\Module\Manager + * @var ModuleManager */ private $moduleManager; /** * Cache config * - * @var \Magento\PageCache\Model\Config + * @var PageCacheConfig */ private $cacheConfig; /** - * @param \Magento\Customer\Model\Session $customerSession - * @param \Magento\Framework\App\Http\Context $httpContext - * @param \Magento\Tax\Model\Calculation\Proxy $calculation - * @param \Magento\Tax\Helper\Data $taxHelper - * @param \Magento\Framework\Module\Manager $moduleManager - * @param \Magento\PageCache\Model\Config $cacheConfig + * @param Session $customerSession + * @param HttpContext $httpContext + * @param Calculation $calculation + * @param TaxHelper $taxHelper + * @param ModuleManager $moduleManager + * @param PageCacheConfig $cacheConfig */ public function __construct( - \Magento\Customer\Model\Session $customerSession, - \Magento\Framework\App\Http\Context $httpContext, - \Magento\Tax\Model\Calculation\Proxy $calculation, //phpcs:ignore Magento2.Classes.DiscouragedDependencies - \Magento\Tax\Helper\Data $taxHelper, - \Magento\Framework\Module\Manager $moduleManager, - \Magento\PageCache\Model\Config $cacheConfig + Session $customerSession, + HttpContext $httpContext, + Calculation $calculation, //phpcs:ignore Magento2.Classes.DiscouragedDependencies + TaxHelper $taxHelper, + ModuleManager $moduleManager, + PageCacheConfig $cacheConfig ) { $this->customerSession = $customerSession; $this->httpContext = $httpContext; @@ -72,14 +80,12 @@ public function __construct( /** * Before dispatch. * - * @param \Magento\Framework\App\ActionInterface $subject - * @param \Magento\Framework\App\RequestInterface $request - * @return mixed + * @param ActionInterface $subject + * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeExecute( - \Magento\Framework\App\ActionInterface $subject - ) { + public function beforeExecute(ActionInterface $subject) + { if (!$this->customerSession->isLoggedIn() || !$this->moduleManager->isEnabled('Magento_PageCache') || !$this->cacheConfig->isEnabled() || diff --git a/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php b/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php index bf31a421243a5..8601bdc175ae8 100644 --- a/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Tax/Test/Unit/App/Action/ContextPluginTest.php @@ -5,81 +5,105 @@ */ namespace Magento\Tax\Test\Unit\App\Action; +use Magento\Customer\Model\Session; +use Magento\Framework\App\Http\Context as HttpContext; +use Magento\Framework\App\Test\Unit\Action\Stub\ActionStub; +use Magento\Framework\Module\Manager as ModuleManager; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; +use Magento\PageCache\Model\Config as PageCacheConfig; +use Magento\Tax\Helper\Data as TaxHelper; +use Magento\Tax\Model\App\Action\ContextPlugin as TaxContextPlugin; +use Magento\Tax\Model\Calculation; +use Magento\Weee\Helper\Data as WeeeHelper; +use Magento\Weee\Model\Tax; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + /** - * Context plugin test + * Unit Tests to cover ContextPlugin */ -class ContextPluginTest extends \PHPUnit\Framework\TestCase +class ContextPluginTest extends TestCase { /** - * @var \Magento\Tax\Helper\Data + * @var ObjectManagerHelper + */ + private $objectManager; + + /** + * @var TaxHelper|MockObject */ protected $taxHelperMock; /** - * @var \Magento\Weee\Helper\Data + * @var WeeeHelper|MockObject */ protected $weeeHelperMock; /** - * @var \Magento\Weee\Model\Tax + * @var Tax|MockObject */ protected $weeeTaxMock; /** - * @var \Magento\Framework\App\Http\Context + * @var HttpContext|MockObject */ protected $httpContextMock; /** - * @var \Magento\Tax\Model\Calculation\Proxy + * @var Calculation|MockObject */ protected $taxCalculationMock; /** * Module manager * - * @var \Magento\Framework\Module\Manager + * @var ModuleManager|MockObject */ private $moduleManagerMock; /** * Cache config * - * @var \Magento\PageCache\Model\Config + * @var PageCacheConfig|MockObject */ private $cacheConfigMock; /** - * @var \Magento\Tax\Model\App\Action\ContextPlugin + * @var Session|MockObject + */ + private $customerSessionMock; + + /** + * @var TaxContextPlugin */ protected $contextPlugin; protected function setUp() { - $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->objectManager = new ObjectManagerHelper($this); - $this->taxHelperMock = $this->getMockBuilder(\Magento\Tax\Helper\Data::class) + $this->taxHelperMock = $this->getMockBuilder(TaxHelper::class) ->disableOriginalConstructor() ->getMock(); - $this->weeeHelperMock = $this->getMockBuilder(\Magento\Weee\Helper\Data::class) + $this->weeeHelperMock = $this->getMockBuilder(WeeeHelper::class) ->disableOriginalConstructor() ->getMock(); - $this->weeeTaxMock = $this->getMockBuilder(\Magento\Weee\Model\Tax::class) + $this->weeeTaxMock = $this->getMockBuilder(Tax::class) ->disableOriginalConstructor() ->getMock(); - $this->httpContextMock = $this->getMockBuilder(\Magento\Framework\App\Http\Context::class) + $this->httpContextMock = $this->getMockBuilder(HttpContext::class) ->disableOriginalConstructor() ->getMock(); - $this->taxCalculationMock = $this->getMockBuilder(\Magento\Tax\Model\Calculation\Proxy::class) + $this->taxCalculationMock = $this->getMockBuilder(Calculation::class) ->disableOriginalConstructor() ->setMethods(['getTaxRates']) ->getMock(); - $this->customerSessionMock = $this->getMockBuilder(\Magento\Customer\Model\Session::class) + $this->customerSessionMock = $this->getMockBuilder(Session::class) ->disableOriginalConstructor() ->setMethods( [ @@ -89,16 +113,16 @@ protected function setUp() ) ->getMock(); - $this->moduleManagerMock = $this->getMockBuilder(\Magento\Framework\Module\Manager::class) + $this->moduleManagerMock = $this->getMockBuilder(ModuleManager::class) ->disableOriginalConstructor() ->getMock(); - $this->cacheConfigMock = $this->getMockBuilder(\Magento\PageCache\Model\Config::class) + $this->cacheConfigMock = $this->getMockBuilder(PageCacheConfig::class) ->disableOriginalConstructor() ->getMock(); $this->contextPlugin = $this->objectManager->getObject( - \Magento\Tax\Model\App\Action\ContextPlugin::class, + TaxContextPlugin::class, [ 'customerSession' => $this->customerSessionMock, 'httpContext' => $this->httpContextMock, @@ -163,7 +187,7 @@ public function testBeforeExecute($cache, $taxEnabled, $loggedIn) ->with('tax_rates', [], 0); } - $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); + $action = $this->objectManager->getObject(ActionStub::class); $result = $this->contextPlugin->beforeExecute($action); $this->assertNull($result); } else { diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index ae65b52f938af..13102a722eddf 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -15,32 +15,34 @@ use Magento\Framework\Config\Theme; /** + * Plugin for Theme Registration + * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Registration { /** - * @var \Magento\Theme\Model\Theme\Registration + * @var ThemeRegistration */ protected $themeRegistration; /** - * @var \Magento\Theme\Model\Theme\Collection + * @var ThemeCollection */ protected $themeCollection; /** - * @var \Magento\Theme\Model\ResourceModel\Theme\Collection + * @var ThemeLoader */ protected $themeLoader; /** - * @var \Psr\Log\LoggerInterface + * @var LoggerInterface */ protected $logger; /** - * @var \Magento\Framework\App\State + * @var AppState */ protected $appState; diff --git a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php index 514964034b967..fb320de97dcc8 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php @@ -3,51 +3,76 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Theme\Test\Unit\Model\Theme\Plugin; use Magento\Framework\App\ActionInterface; -use Magento\Theme\Model\Theme\Plugin\Registration; +use Magento\Framework\App\State; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Theme\Model\ResourceModel\Theme\Collection as ThemeCollectionResourceModel; +use Magento\Theme\Model\Theme; +use Magento\Theme\Model\Theme\Collection as ThemeCollection; +use Magento\Theme\Model\Theme\Plugin\Registration as RegistrationPlugin; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Phrase; +use Magento\Theme\Model\Theme\Registration as ThemeRegistration; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; class RegistrationTest extends \PHPUnit\Framework\TestCase { - /** @var \Magento\Theme\Model\Theme\Registration|\PHPUnit_Framework_MockObject_MockObject */ - protected $themeRegistration; + /** + * @var ThemeRegistration|MockObject + */ + protected $themeRegistrationMock; - /** @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $logger; + /** + * @var LoggerInterface|MockObject + */ + protected $loggerMock; - /** @var ActionInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $action; + /** + * @var ActionInterface|MockObject + */ + protected $actionMock; - /** @var \Magento\Framework\App\State|\PHPUnit_Framework_MockObject_MockObject */ - protected $appState; + /** + * @var State|MockObject + */ + protected $appStateMock; - /** @var \Magento\Theme\Model\Theme\Collection|\PHPUnit_Framework_MockObject_MockObject */ - protected $themeCollection; + /** + * @var ThemeCollection|MockObject + */ + protected $themeCollectionMock; - /** @var \Magento\Theme\Model\ResourceModel\Theme\Collection|\PHPUnit_Framework_MockObject_MockObject */ - protected $themeLoader; + /** + * @var ThemeCollectionResourceModel|MockObject + */ + protected $themeLoaderMock; - /** @var Registration */ + /** + * @var RegistrationPlugin + */ protected $plugin; protected function setUp() { - $this->themeRegistration = $this->createMock(\Magento\Theme\Model\Theme\Registration::class); - $this->logger = $this->getMockForAbstractClass(\Psr\Log\LoggerInterface::class, [], '', false); - $this->action = $this->createMock(ActionInterface::class); - $this->appState = $this->createMock(\Magento\Framework\App\State::class); - $this->themeCollection = $this->createMock(\Magento\Theme\Model\Theme\Collection::class); - $this->themeLoader = $this->createMock(\Magento\Theme\Model\ResourceModel\Theme\Collection::class); - $this->plugin = new Registration( - $this->themeRegistration, - $this->themeCollection, - $this->themeLoader, - $this->logger, - $this->appState - ); + $this->themeRegistrationMock = $this->createMock(ThemeRegistration::class); + $this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class, [], '', false); + $this->actionMock = $this->createMock(ActionInterface::class); + $this->appStateMock = $this->createMock(State::class); + $this->themeCollectionMock = $this->createMock(ThemeCollection::class); + $this->themeLoaderMock = $this->createMock(ThemeCollectionResourceModel::class); + + $objectManager = new ObjectManager($this); + $this->plugin = $objectManager->getObject(RegistrationPlugin::class, [ + 'themeRegistration' => $this->themeRegistrationMock, + 'themeCollection' => $this->themeCollectionMock, + 'themeLoader' => $this->themeLoaderMock, + 'logger' => $this->loggerMock, + 'appState' => $this->appStateMock + ]); } /** @@ -55,13 +80,12 @@ protected function setUp() * @dataProvider dataProviderBeforeExecute * @SuppressWarnings(PHPMD.NPathComplexity) */ - public function testBeforeExecute( - $hasParentTheme - ) { + public function testBeforeExecute($hasParentTheme) + { $themeId = 1; $themeTitle = 'Theme title'; - $themeFromConfigMock = $this->getMockBuilder(\Magento\Theme\Model\Theme::class) + $themeFromConfigMock = $this->getMockBuilder(Theme::class) ->disableOriginalConstructor() ->setMethods([ 'getArea', @@ -71,7 +95,7 @@ public function testBeforeExecute( ]) ->getMock(); - $themeFromDbMock = $this->getMockBuilder(\Magento\Theme\Model\Theme::class) + $themeFromDbMock = $this->getMockBuilder(Theme::class) ->disableOriginalConstructor() ->setMethods([ 'setParentId', @@ -80,26 +104,26 @@ public function testBeforeExecute( ]) ->getMock(); - $parentThemeFromDbMock = $this->getMockBuilder(\Magento\Theme\Model\Theme::class) + $parentThemeFromDbMock = $this->getMockBuilder(Theme::class) ->disableOriginalConstructor() ->getMock(); - $parentThemeFromConfigMock = $this->getMockBuilder(\Magento\Theme\Model\Theme::class) + $parentThemeFromConfigMock = $this->getMockBuilder(Theme::class) ->disableOriginalConstructor() ->getMock(); - $this->appState->expects($this->once()) + $this->appStateMock->expects($this->once()) ->method('getMode') ->willReturn('default'); - $this->themeRegistration->expects($this->once()) + $this->themeRegistrationMock->expects($this->once()) ->method('register'); - $this->themeCollection->expects($this->once()) + $this->themeCollectionMock->expects($this->once()) ->method('loadData') ->willReturn([$themeFromConfigMock]); - $this->themeLoader->expects($hasParentTheme ? $this->exactly(2) : $this->once()) + $this->themeLoaderMock->expects($hasParentTheme ? $this->exactly(2) : $this->once()) ->method('getThemeByFullPath') ->willReturnMap([ ['frontend/Magento/blank', $parentThemeFromDbMock], @@ -139,7 +163,7 @@ public function testBeforeExecute( ->method('save') ->willReturnSelf(); - $this->plugin->beforeExecute($this->action); + $this->plugin->beforeExecute($this->actionMock); } /** @@ -155,16 +179,16 @@ public function dataProviderBeforeExecute() public function testBeforeDispatchWithProductionMode() { - $this->appState->expects($this->once())->method('getMode')->willReturn('production'); - $this->plugin->beforeExecute($this->action); + $this->appStateMock->expects($this->once())->method('getMode')->willReturn('production'); + $this->plugin->beforeExecute($this->actionMock); } public function testBeforeDispatchWithException() { $exception = new LocalizedException(new Phrase('Phrase')); - $this->themeRegistration->expects($this->once())->method('register')->willThrowException($exception); - $this->logger->expects($this->once())->method('critical'); + $this->themeRegistrationMock->expects($this->once())->method('register')->willThrowException($exception); + $this->loggerMock->expects($this->once())->method('critical'); - $this->plugin->beforeExecute($this->action); + $this->plugin->beforeExecute($this->actionMock); } } diff --git a/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php b/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php index 6113454d7ca27..66485d0410872 100644 --- a/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php +++ b/app/code/Magento/Weee/Model/App/Action/ContextPlugin.php @@ -6,78 +6,92 @@ namespace Magento\Weee\Model\App\Action; +use Magento\Customer\Model\Session as CustomerSession; +use Magento\Framework\App\ActionInterface; +use Magento\Framework\App\Config\ScopeConfigInterface; +use Magento\Framework\App\Http\Context as HttpContext; +use Magento\Framework\Module\Manager as ModuleManager; +use Magento\PageCache\Model\Config as PageCacheConfig; +use Magento\Store\Model\ScopeInterface; +use Magento\Store\Model\StoreManagerInterface; +use Magento\Tax\Helper\Data as TaxHelper; +use Magento\Tax\Model\Config as TaxConfig; +use Magento\Weee\Helper\Data as WeeeHelper; +use Magento\Weee\Model\Tax; + /** - * Class ContextPlugin + * Plugin to provide Context information to Weee Action + * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ContextPlugin { /** - * @var \Magento\Customer\Model\Session + * @var CustomerSession */ protected $customerSession; /** - * @var \Magento\Framework\App\Http\Context + * @var HttpContext */ protected $httpContext; /** - * @var \Magento\Tax\Helper\Data + * @var TaxHelper */ protected $taxHelper; /** - * @var \Magento\Weee\Helper\Data + * @var WeeeHelper */ protected $weeeHelper; /** - * @var \Magento\Framework\Module\Manager + * @var ModuleManager */ protected $moduleManager; /** - * @var \Magento\Weee\Model\Tax + * @var Tax */ protected $weeeTax; /** - * @var \Magento\PageCache\Model\Config + * @var PageCacheConfig */ protected $cacheConfig; /** - * @var \Magento\Store\Model\StoreManagerInterface + * @var StoreManagerInterface */ protected $storeManager; /** - * @var \Magento\Framework\App\Config\ScopeConfigInterface + * @var ScopeConfigInterface */ protected $scopeConfig; /** - * @param \Magento\Customer\Model\Session $customerSession - * @param \Magento\Framework\App\Http\Context $httpContext - * @param \Magento\Weee\Model\Tax $weeeTax - * @param \Magento\Tax\Helper\Data $taxHelper - * @param \Magento\Weee\Helper\Data $weeeHelper - * @param \Magento\Framework\Module\Manager $moduleManager - * @param \Magento\PageCache\Model\Config $cacheConfig - * @param \Magento\Store\Model\StoreManagerInterface $storeManager - * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig + * @param CustomerSession $customerSession + * @param HttpContext $httpContext + * @param Tax $weeeTax + * @param TaxHelper $taxHelper + * @param WeeeHelper $weeeHelper + * @param ModuleManager $moduleManager + * @param PageCacheConfig $cacheConfig + * @param StoreManagerInterface $storeManager + * @param ScopeConfigInterface $scopeConfig */ public function __construct( - \Magento\Customer\Model\Session $customerSession, - \Magento\Framework\App\Http\Context $httpContext, - \Magento\Weee\Model\Tax $weeeTax, - \Magento\Tax\Helper\Data $taxHelper, - \Magento\Weee\Helper\Data $weeeHelper, - \Magento\Framework\Module\Manager $moduleManager, - \Magento\PageCache\Model\Config $cacheConfig, - \Magento\Store\Model\StoreManagerInterface $storeManager, - \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig + CustomerSession $customerSession, + HttpContext $httpContext, + Tax $weeeTax, + TaxHelper $taxHelper, + WeeeHelper $weeeHelper, + ModuleManager $moduleManager, + PageCacheConfig $cacheConfig, + StoreManagerInterface $storeManager, + ScopeConfigInterface $scopeConfig ) { $this->customerSession = $customerSession; $this->httpContext = $httpContext; @@ -93,14 +107,14 @@ public function __construct( /** * Before dispatch. * - * @param \Magento\Framework\App\ActionInterface $subject + * @param ActionInterface $subject * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ - public function beforeExecute(\Magento\Framework\App\ActionInterface $subject) + public function beforeExecute(ActionInterface $subject) { if (!$this->weeeHelper->isEnabled() || !$this->customerSession->isLoggedIn() || @@ -125,26 +139,14 @@ public function beforeExecute(\Magento\Framework\App\ActionInterface $subject) } elseif ($countryId && !$regionId) { // country exist and region does not exist $regionId = 0; - $exist = $this->weeeTax->isWeeeInLocation( - $countryId, - $regionId, - $websiteId - ); + $exist = $this->weeeTax->isWeeeInLocation($countryId, $regionId, $websiteId); } else { // country and region exist - $exist = $this->weeeTax->isWeeeInLocation( - $countryId, - $regionId, - $websiteId - ); + $exist = $this->weeeTax->isWeeeInLocation($countryId, $regionId, $websiteId); if (!$exist) { // just check the country for weee $regionId = 0; - $exist = $this->weeeTax->isWeeeInLocation( - $countryId, - $regionId, - $websiteId - ); + $exist = $this->weeeTax->isWeeeInLocation($countryId, $regionId, $websiteId); } } @@ -168,13 +170,13 @@ protected function getWeeeTaxRegion($basedOn) $countryId = null; $regionId = null; $defaultCountryId = $this->scopeConfig->getValue( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_COUNTRY, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_COUNTRY, + ScopeInterface::SCOPE_STORE, null ); $defaultRegionId = $this->scopeConfig->getValue( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_REGION, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_REGION, + ScopeInterface::SCOPE_STORE, null ); diff --git a/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php b/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php index ed79892366526..b28a223ef2dce 100644 --- a/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php +++ b/app/code/Magento/Weee/Test/Unit/App/Action/ContextPluginTest.php @@ -3,124 +3,146 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Weee\Test\Unit\App\Action; +use Magento\Customer\Model\Session as CustomerSession; +use Magento\Framework\App\Config; +use Magento\Framework\App\Http\Context as HttpContext; +use Magento\Framework\App\Test\Unit\Action\Stub\ActionStub; +use Magento\Framework\Module\Manager as ModuleManager; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\PageCache\Model\Config as PageCacheConfig; +use Magento\Store\Model\ScopeInterface; +use Magento\Store\Model\Store; +use Magento\Store\Model\StoreManager; +use Magento\Tax\Helper\Data as TaxHelper; +use Magento\Tax\Model\Calculation\Proxy as TaxCalculation; +use Magento\Tax\Model\Config as TaxConfig; +use Magento\Weee\Helper\Data as WeeeHelper; +use Magento\Weee\Model\App\Action\ContextPlugin; +use Magento\Weee\Model\Tax; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + /** - * Class ContextPluginTest + * Unit Tests to cover Context Plugin * - * @package Magento\Weee\Test\Unit\App\Action * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ -class ContextPluginTest extends \PHPUnit\Framework\TestCase +class ContextPluginTest extends TestCase { /** - * @var \Magento\Tax\Helper\Data|\PHPUnit_Framework_MockObject_MockObject + * @var TaxHelper|MockObject */ protected $taxHelperMock; /** - * @var \Magento\Weee\Helper\Data|\PHPUnit_Framework_MockObject_MockObject + * @var WeeeHelper|MockObject */ protected $weeeHelperMock; /** - * @var \Magento\Weee\Model\Tax|\PHPUnit_Framework_MockObject_MockObject + * @var Tax|MockObject */ protected $weeeTaxMock; /** - * @var \Magento\Framework\App\Http\Context|\PHPUnit_Framework_MockObject_MockObject + * @var HttpContext|MockObject */ protected $httpContextMock; /** - * @var \Magento\Tax\Model\Calculation\Proxy|\PHPUnit_Framework_MockObject_MockObject + * @var TaxCalculation|MockObject */ protected $taxCalculationMock; /** - * @var \Magento\Framework\Module\Manager|\PHPUnit_Framework_MockObject_MockObject + * @var ModuleManager|MockObject */ protected $moduleManagerMock; /** - * @var \Magento\PageCache\Model\Config|\PHPUnit_Framework_MockObject_MockObject + * @var PageCacheConfig|MockObject */ protected $cacheConfigMock; /** - * @var \Magento\Store\Model\StoreManager|\PHPUnit_Framework_MockObject_MockObject + * @var StoreManager|MockObject */ protected $storeManagerMock; /** - * @var \Magento\Framework\App\Config|\PHPUnit_Framework_MockObject_MockObject + * @var Config|MockObject */ protected $scopeConfigMock; /** - * @var \Magento\Customer\Model\Session|\PHPUnit_Framework_MockObject_MockObject + * @var CustomerSession|MockObject */ private $customerSessionMock; /** - * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager + * @var ObjectManager */ private $objectManager; /** - * @var \Magento\Tax\Model\App\Action\ContextPlugin + * @var ContextPlugin */ protected $contextPlugin; protected function setUp() { - $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->objectManager = new ObjectManager($this); - $this->taxHelperMock = $this->getMockBuilder(\Magento\Tax\Helper\Data::class) + $this->taxHelperMock = $this->getMockBuilder(TaxHelper::class) ->disableOriginalConstructor() ->getMock(); - $this->weeeHelperMock = $this->getMockBuilder(\Magento\Weee\Helper\Data::class) + $this->weeeHelperMock = $this->getMockBuilder(WeeeHelper::class) ->disableOriginalConstructor() ->getMock(); - $this->weeeTaxMock = $this->getMockBuilder(\Magento\Weee\Model\Tax::class) + $this->weeeTaxMock = $this->getMockBuilder(Tax::class) ->disableOriginalConstructor() ->getMock(); - $this->httpContextMock = $this->getMockBuilder(\Magento\Framework\App\Http\Context::class) + $this->httpContextMock = $this->getMockBuilder(HttpContext::class) ->disableOriginalConstructor() ->getMock(); - $this->customerSessionMock = $this->getMockBuilder(\Magento\Customer\Model\Session::class) + $this->customerSessionMock = $this->getMockBuilder(CustomerSession::class) ->disableOriginalConstructor() ->setMethods( [ - 'getDefaultTaxBillingAddress', 'getDefaultTaxShippingAddress', 'getCustomerTaxClassId', - 'getWebsiteId', 'isLoggedIn' + 'getDefaultTaxBillingAddress', + 'getDefaultTaxShippingAddress', + 'getCustomerTaxClassId', + 'getWebsiteId', + 'isLoggedIn' ] ) ->getMock(); - $this->moduleManagerMock = $this->getMockBuilder(\Magento\Framework\Module\Manager::class) + $this->moduleManagerMock = $this->getMockBuilder(ModuleManager::class) ->disableOriginalConstructor() ->getMock(); - $this->cacheConfigMock = $this->getMockBuilder(\Magento\PageCache\Model\Config::class) + $this->cacheConfigMock = $this->getMockBuilder(PageCacheConfig::class) ->disableOriginalConstructor() ->getMock(); - - $this->storeManagerMock = $this->getMockBuilder(\Magento\Store\Model\StoreManager::class) + + $this->storeManagerMock = $this->getMockBuilder(StoreManager::class) ->disableOriginalConstructor() ->getMock(); - $this->scopeConfigMock = $this->getMockBuilder(\Magento\Framework\App\Config::class) + $this->scopeConfigMock = $this->getMockBuilder(Config::class) ->disableOriginalConstructor() ->getMock(); $this->contextPlugin = $this->objectManager->getObject( - \Magento\Weee\Model\App\Action\ContextPlugin::class, + ContextPlugin::class, [ 'customerSession' => $this->customerSessionMock, 'httpContext' => $this->httpContextMock, @@ -158,7 +180,7 @@ public function testBeforeExecuteBasedOnDefault() ->method('getTaxBasedOn') ->willReturn('billing'); - $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) + $storeMock = $this->getMockBuilder(Store::class) ->disableOriginalConstructor() ->getMock(); @@ -173,8 +195,8 @@ public function testBeforeExecuteBasedOnDefault() $this->scopeConfigMock->expects($this->at(0)) ->method('getValue') ->with( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_COUNTRY, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_COUNTRY, + ScopeInterface::SCOPE_STORE, null ) ->willReturn('US'); @@ -182,8 +204,8 @@ public function testBeforeExecuteBasedOnDefault() $this->scopeConfigMock->expects($this->at(1)) ->method('getValue') ->with( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_REGION, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_REGION, + ScopeInterface::SCOPE_STORE, null ) ->willReturn(0); @@ -197,8 +219,8 @@ public function testBeforeExecuteBasedOnDefault() ->method('setValue') ->with('weee_tax_region', ['countryId' => 'US', 'regionId' => 0], 0); - /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ - $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); + /** @var ActionStub $action */ + $action = $this->objectManager->getObject(ActionStub::class); $this->contextPlugin->beforeExecute($action); } @@ -226,8 +248,8 @@ public function testBeforeExecuteBasedOnOrigin() ->method('getTaxBasedOn') ->willReturn('origin'); - /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ - $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); + /** @var ActionStub $action */ + $action = $this->objectManager->getObject(ActionStub::class); $this->contextPlugin->beforeExecute($action); } @@ -255,7 +277,7 @@ public function testBeforeExecuteBasedOnBilling() ->method('getTaxBasedOn') ->willReturn('billing'); - $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) + $storeMock = $this->getMockBuilder(Store::class) ->disableOriginalConstructor() ->getMock(); @@ -270,8 +292,8 @@ public function testBeforeExecuteBasedOnBilling() $this->scopeConfigMock->expects($this->at(0)) ->method('getValue') ->with( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_COUNTRY, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_COUNTRY, + ScopeInterface::SCOPE_STORE, null ) ->willReturn('US'); @@ -279,8 +301,8 @@ public function testBeforeExecuteBasedOnBilling() $this->scopeConfigMock->expects($this->at(1)) ->method('getValue') ->with( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_REGION, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_REGION, + ScopeInterface::SCOPE_STORE, null ) ->willReturn(0); @@ -298,8 +320,8 @@ public function testBeforeExecuteBasedOnBilling() ->method('setValue') ->with('weee_tax_region', ['countryId' => 'US', 'regionId' => 1], 0); - /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ - $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); + /** @var ActionStub $action */ + $action = $this->objectManager->getObject(ActionStub::class); $this->contextPlugin->beforeExecute($action); } @@ -327,7 +349,7 @@ public function testBeforeExecuterBasedOnShipping() ->method('getTaxBasedOn') ->willReturn('shipping'); - $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) + $storeMock = $this->getMockBuilder(Store::class) ->disableOriginalConstructor() ->getMock(); @@ -342,8 +364,8 @@ public function testBeforeExecuterBasedOnShipping() $this->scopeConfigMock->expects($this->at(0)) ->method('getValue') ->with( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_COUNTRY, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_COUNTRY, + ScopeInterface::SCOPE_STORE, null ) ->willReturn('US'); @@ -351,8 +373,8 @@ public function testBeforeExecuterBasedOnShipping() $this->scopeConfigMock->expects($this->at(1)) ->method('getValue') ->with( - \Magento\Tax\Model\Config::CONFIG_XML_PATH_DEFAULT_REGION, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, + TaxConfig::CONFIG_XML_PATH_DEFAULT_REGION, + ScopeInterface::SCOPE_STORE, null ) ->willReturn(0); @@ -370,8 +392,8 @@ public function testBeforeExecuterBasedOnShipping() ->method('setValue') ->with('weee_tax_region', ['countryId' => 'US', 'regionId' => 1], 0); - /** @var \Magento\Framework\App\Test\Unit\Action\Stub\ActionStub $action */ - $action = $this->objectManager->getObject(\Magento\Framework\App\Test\Unit\Action\Stub\ActionStub::class); + /** @var ActionStub $action */ + $action = $this->objectManager->getObject(ActionStub::class); $this->contextPlugin->beforeExecute($action); } diff --git a/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php b/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php index 780c6b226ad70..1b8b1b716ce5f 100644 --- a/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php +++ b/dev/tests/integration/testsuite/Magento/Authorizenet/Controller/Adminhtml/Authorizenet/Directpost/Payment/PlaceTest.php @@ -3,18 +3,33 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Authorizenet\Controller\Adminhtml\Authorizenet\Directpost\Payment; +use Magento\Authorizenet\Model\Directpost; +use Magento\Backend\App\Action\Context as BackendActionContext; +use Magento\Backend\Model\Session\Quote as SessionQuote; +use Magento\Backend\Model\UrlInterface; +use Magento\Framework\Json\Helper\Data as JsonHelper; +use Magento\Framework\ObjectManagerInterface; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Quote\Model\Quote; +use Magento\Quote\Model\Quote\Payment; +use Magento\Sales\Model\AdminOrder\Create as AdminOrderCreate; +use Magento\Sales\Model\Order; +use Magento\TestFramework\Helper\Bootstrap; +use Magento\TestFramework\TestCase\AbstractBackendController; +use PHPUnit\Framework\MockObject\MockObject; + /** - * Class PlaceTest + * Verify AuthorizeNet Controller for PlaceOrder * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ -class PlaceTest extends \Magento\TestFramework\TestCase\AbstractBackendController +class PlaceTest extends AbstractBackendController { /** * Test requestToAuthorizenetData returning - * * @magentoAppArea adminhtml */ public function testExecuteAuthorizenetDataReturning() @@ -24,44 +39,44 @@ public function testExecuteAuthorizenetDataReturning() $this->getRequest()->setParam('payment', ['method' => 'authorizenet_directpost']); $this->getRequest()->setParam('controller', 'order_create'); $orderCreateMock = $this->getOrderCreateMock($requestToAuthorizenetData); - $directpostMock = $this->getMockBuilder(\Magento\Authorizenet\Model\Directpost::class) + $directpostMock = $this->getMockBuilder(Directpost::class) ->setMethods(['getCode']) ->disableOriginalConstructor() ->getMock(); $directpostMock->expects($this->once()) ->method('getCode') ->willReturn('authorizenet_directpost'); - $jsonHelper = $this->_objectManager->get(\Magento\Framework\Json\Helper\Data::class); - $objectManagerMock = $this->getMockBuilder(\Magento\Framework\ObjectManagerInterface::class) + $jsonHelper = $this->_objectManager->get(JsonHelper::class); + $objectManagerMock = $this->getMockBuilder(ObjectManagerInterface::class) ->setMethods(['create', 'get']) ->getMockForAbstractClass(); $objectManagerMock->expects($this->atLeastOnce()) ->method('create') - ->with(\Magento\Authorizenet\Model\Directpost::class) + ->with(Directpost::class) ->willReturn($directpostMock); - $authorizenetSessionMock = $this->getMockBuilder(\Magento\Authorizenet\Model\Directpost::class) + $authorizenetSessionMock = $this->getMockBuilder(Directpost::class) ->disableOriginalConstructor() ->getMock(); - $urlMock = $this->getMockBuilder(\Magento\Backend\Model\UrlInterface::class) + $urlMock = $this->getMockBuilder(UrlInterface::class) ->getMockForAbstractClass(); $objectManagerMock->expects($this->atLeastOnce()) ->method('get') ->willReturnMap([ - [\Magento\Sales\Model\AdminOrder\Create::class, $orderCreateMock], - [\Magento\Framework\Json\Helper\Data::class, $jsonHelper], - [\Magento\Authorizenet\Model\Directpost\Session::class, $authorizenetSessionMock], - [\Magento\Backend\Model\UrlInterface::class, $urlMock], + [AdminOrderCreate::class, $orderCreateMock], + [JsonHelper::class, $jsonHelper], + [Directpost\Session::class, $authorizenetSessionMock], + [UrlInterface::class, $urlMock], ]); - $context = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( - \Magento\Backend\App\Action\Context::class, + $context = $this->getObjectManager()->create( + BackendActionContext::class, [ 'objectManager' => $objectManagerMock ] ); - $controller = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( - \Magento\Authorizenet\Controller\Adminhtml\Authorizenet\Directpost\Payment\PlaceTesting::class, + $controller = $this->getObjectManager()->create( + PlaceTesting::class, ['context' => $context] ); $controller->execute(); @@ -70,14 +85,14 @@ public function testExecuteAuthorizenetDataReturning() /** * @param array $requestToAuthorizenetData - * @return \PHPUnit_Framework_MockObject_MockObject + * @return AdminOrderCreate|MockObject */ private function getOrderCreateMock($requestToAuthorizenetData) { - $methodInstanceMock = $this->getMockBuilder(\Magento\Authorizenet\Model\Directpost::class) + $methodInstanceMock = $this->getMockBuilder(Directpost::class) ->disableOriginalConstructor() ->getMock(); - $directpostRequestMock = $this->getMockBuilder(\Magento\Authorizenet\Model\Directpost\Request::class) + $directpostRequestMock = $this->getMockBuilder(Directpost\Request::class) ->setMethods(['getData']) ->disableOriginalConstructor() ->getMock(); @@ -87,7 +102,7 @@ private function getOrderCreateMock($requestToAuthorizenetData) $methodInstanceMock->expects($this->once()) ->method('generateRequestFromOrder') ->willReturn($directpostRequestMock); - $paymentMock = $this->getMockBuilder(\Magento\Quote\Model\Quote\Payment::class) + $paymentMock = $this->getMockBuilder(Payment::class) ->setMethods(['getMethod', 'getMethodInstance']) ->disableOriginalConstructor() ->getMock(); @@ -97,28 +112,28 @@ private function getOrderCreateMock($requestToAuthorizenetData) $paymentMock->expects($this->once()) ->method('getMethodInstance') ->willReturn($methodInstanceMock); - $quoteMock = $this->getMockBuilder(\Magento\Quote\Model\Quote::class) + $quoteMock = $this->getMockBuilder(Quote::class) ->setMethods(['getPayment', 'getStoreId']) ->disableOriginalConstructor() ->getMock(); $quoteMock->expects($this->any()) ->method('getPayment') ->willReturn($paymentMock); - $orderMock = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + $orderMock = $this->getMockBuilder(Order::class) ->setMethods(['getPayment']) ->disableOriginalConstructor() ->getMock(); $orderMock->expects($this->any()) ->method('getPayment') ->willReturn($paymentMock); - $sessionQuoteMock = $this->getMockBuilder(\Magento\Backend\Model\Session\Quote::class) + $sessionQuoteMock = $this->getMockBuilder(SessionQuote::class) ->setMethods(['getOrder']) ->disableOriginalConstructor() ->getMock(); $sessionQuoteMock->expects($this->once()) ->method('getOrder') ->willReturn($orderMock); - $orderCreateMock = $this->getMockBuilder(\Magento\Sales\Model\AdminOrder\Create::class) + $orderCreateMock = $this->getMockBuilder(AdminOrderCreate::class) ->setMethods(['getQuote', 'getSession', 'setIsValidate', 'importPostData', 'createOrder', 'setPaymentData']) ->disableOriginalConstructor() ->getMock(); @@ -140,4 +155,12 @@ private function getOrderCreateMock($requestToAuthorizenetData) return $orderCreateMock; } + + /** + * @return ObjectManagerInterface + */ + private function getObjectManager(): ObjectManagerInterface + { + return Bootstrap::getObjectManager(); + } } diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php index ce2d92b62d55d..12be7607e1872 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/ControllerActionTest.php @@ -1,4 +1,10 @@ -create(Event\ManagerInterface::class); - $eventManagerSpy = new class($originalEventManager) implements Event\ManagerInterface - { + $eventManagerSpy = new class($originalEventManager) implements Event\ManagerInterface { /** * @var Event\ManagerInterface */ @@ -57,7 +62,7 @@ public function spyOnDispatchedEvent(string $eventName): array } }; - $objectManager->addSharedInstance($eventManagerSpy, Event\Manager\Proxy::class); + $objectManager->addSharedInstance($eventManagerSpy, get_class($originalEventManager)); } private function assertEventDispatchCount($eventName, $expectedCount) @@ -86,7 +91,7 @@ private function fakeAuthenticatedBackendRequest() private function configureRequestForAction(string $route, string $actionPath, string $actionName) { $request = $this->getRequest(); - + $request->setRouteName($route); $request->setControllerName($actionPath); $request->setActionName($actionName); @@ -114,11 +119,11 @@ private function assertPreAndPostDispatchEventsAreDispatched() public function testInheritanceBasedFrontendActionDispatchesEvents() { $this->setupEventManagerSpy(); - + /** @var InheritanceBasedFrontendAction $action */ $action = ObjectManager::getInstance()->create(InheritanceBasedFrontendAction::class); $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); - + $action->dispatch($this->getRequest()); $this->assertPreAndPostDispatchEventsAreDispatched(); @@ -130,7 +135,7 @@ public function testInheritanceBasedFrontendActionDispatchesEvents() public function testInterfaceOnlyFrontendActionDispatchesEvents() { $this->setupEventManagerSpy(); - + /** @var InterfaceOnlyFrontendAction $action */ $action = ObjectManager::getInstance()->create(InterfaceOnlyFrontendAction::class); $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); @@ -146,7 +151,7 @@ public function testInterfaceOnlyFrontendActionDispatchesEvents() public function testInheritanceBasedAdminhtmlActionDispatchesEvents() { $this->fakeAuthenticatedBackendRequest(); - + $this->setupEventManagerSpy(); /** @var InheritanceBasedBackendAction $action */ @@ -184,13 +189,13 @@ public function testSettingTheNoDispatchActionFlagProhibitsExecuteAndPostdispatc /** @var InterfaceOnlyFrontendAction $action */ $action = ObjectManager::getInstance()->create(InterfaceOnlyFrontendAction::class); $this->configureRequestForAction('testroute', 'actionpath', 'actionname'); - + /** @var ActionFlag $actionFlag */ $actionFlag = ObjectManager::getInstance()->get(ActionFlag::class); $actionFlag->set('', ActionInterface::FLAG_NO_DISPATCH, true); - + $action->execute(); - + $this->assertFalse($action->isExecuted(), 'The controller execute() method was not expected to be called.'); $this->assertEventDispatchCount('controller_action_predispatch', 1); $this->assertEventDispatchCount('controller_action_predispatch_testroute', 1); diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php index 78bbc598baed0..b24cc2e30382c 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedBackendAction.php @@ -1,8 +1,17 @@ -pageFactory = $pageFactory; } + /** + * Creates Page + * + * @return ResponseInterface|ResultInterface|Page + */ public function execute() { return $this->pageFactory->create(); diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php index 8d68014185e03..315df9585c715 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InheritanceBasedFrontendAction.php @@ -1,9 +1,18 @@ -pageFactory = $pageFactory; } + /** + * Runs `execute()` method to create Page + * + * @return ResponseInterface|ResultInterface|Page + */ public function execute() { $this->executeWasCalled = true; return $this->pageFactory->create(); } + /** + * Determines whether execute method was called + * + * @return bool + */ public function isExecuted(): bool { return $this->executeWasCalled; diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php index fd176e8163010..86244fd67237b 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyBackendAction.php @@ -1,8 +1,16 @@ -pageFactory = $pageFactory; } + /** + * Creates Page object + * + * @return ResponseInterface|ResultInterface|Page + */ public function execute() { return $this->pageFactory->create(); diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php index 36dd3bfd518f7..123fe2ea87be1 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/TestStubs/InterfaceOnlyFrontendAction.php @@ -1,8 +1,16 @@ -pageFactory = $pageFactory; } + /** + * Creates Page object + * + * @return ResponseInterface|ResultInterface|Page + */ public function execute() { $this->executeWasCalled = true; return $this->pageFactory->create(); } + /** + * Returns whether `execute()` method was ran + * + * @return bool + */ public function isExecuted(): bool { return $this->executeWasCalled; diff --git a/lib/internal/Magento/Framework/App/Action/Action.php b/lib/internal/Magento/Framework/App/Action/Action.php index 7412965be14cc..3ae0bbac44f8a 100644 --- a/lib/internal/Magento/Framework/App/Action/Action.php +++ b/lib/internal/Magento/Framework/App/Action/Action.php @@ -3,12 +3,20 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Framework\App\Action; +use Magento\Framework\App\ActionFlag; use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\Response\RedirectInterface; use Magento\Framework\App\ResponseInterface; -use Magento\Framework\Controller\ResultInterface; +use Magento\Framework\App\ViewInterface; +use Magento\Framework\Event\ManagerInterface as EventManagerInterface; use Magento\Framework\Exception\NotFoundException; +use Magento\Framework\Message\ManagerInterface as MessageManagerInterface; +use Magento\Framework\ObjectManagerInterface; +use Magento\Framework\Profiler; +use Magento\Framework\UrlInterface; /** * Extend from this class to create actions controllers in frontend area of your application. @@ -17,6 +25,7 @@ * * TODO: Remove this class. Allow implementation of Action Controllers by just implementing Action Interface. * + * phpcs:disable Magento2.Classes.AbstractApi * @api * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.NumberOfChildren) @@ -24,7 +33,7 @@ abstract class Action extends AbstractAction { /** - * @var \Magento\Framework\ObjectManagerInterface + * @var ObjectManagerInterface */ protected $_objectManager; @@ -37,32 +46,32 @@ abstract class Action extends AbstractAction protected $_sessionNamespace; /** - * @var \Magento\Framework\Event\ManagerInterface + * @var EventManagerInterface */ protected $_eventManager; /** - * @var \Magento\Framework\App\ActionFlag + * @var ActionFlag */ protected $_actionFlag; /** - * @var \Magento\Framework\App\Response\RedirectInterface + * @var RedirectInterface */ protected $_redirect; /** - * @var \Magento\Framework\App\ViewInterface + * @var ViewInterface */ protected $_view; /** - * @var \Magento\Framework\UrlInterface + * @var UrlInterface */ protected $_url; /** - * @var \Magento\Framework\Message\ManagerInterface + * @var MessageManagerInterface */ protected $messageManager; @@ -92,15 +101,15 @@ public function dispatch(RequestInterface $request) { $this->_request = $request; $profilerKey = 'CONTROLLER_ACTION:' . $request->getFullActionName(); - \Magento\Framework\Profiler::start($profilerKey); + Profiler::start($profilerKey); $result = null; if ($request->isDispatched() && !$this->_actionFlag->get('', self::FLAG_NO_DISPATCH)) { - \Magento\Framework\Profiler::start('action_body'); + Profiler::start('action_body'); $result = $this->execute(); - \Magento\Framework\Profiler::stop('action_body'); + Profiler::stop('action_body'); } - \Magento\Framework\Profiler::stop($profilerKey); + Profiler::stop($profilerKey); return $result ?: $this->_response; } @@ -139,9 +148,9 @@ protected function _forward($action, $controller = null, $module = null, array $ /** * Set redirect into response * - * @param string $path - * @param array $arguments - * @return ResponseInterface + * @param string $path + * @param array $arguments + * @return ResponseInterface */ protected function _redirect($path, $arguments = []) { @@ -150,6 +159,8 @@ protected function _redirect($path, $arguments = []) } /** + * Returns ActionFlag value + * * @return \Magento\Framework\App\ActionFlag */ public function getActionFlag() diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php index 98010656ff91d..263e3663513d3 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/ActionFlagNoDispatchPlugin.php @@ -1,4 +1,10 @@ -actionFlag = $actionFlag; diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/Design.php b/lib/internal/Magento/Framework/App/Action/Plugin/Design.php index babde4bcc883a..a1979e88d8128 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/Design.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/Design.php @@ -6,27 +6,35 @@ namespace Magento\Framework\App\Action\Plugin; +use Magento\Framework\App\ActionInterface; +use Magento\Framework\Config\Dom\ValidationException; +use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Message\ManagerInterface as MessageManagerInterface; use Magento\Framework\Message\MessageInterface; +use Magento\Framework\View\DesignLoader; +/** + * Handling Exceptions on Design Loading + */ class Design { /** - * @var \Magento\Framework\View\DesignLoader + * @var DesignLoader */ protected $_designLoader; /** - * @var \Magento\Framework\Message\ManagerInterface + * @var MessageManagerInterface */ protected $messageManager; /** - * @param \Magento\Framework\View\DesignLoader $designLoader - * @param \Magento\Framework\Message\ManagerInterface $messageManager + * @param DesignLoader $designLoader + * @param MessageManagerInterface $messageManager */ public function __construct( - \Magento\Framework\View\DesignLoader $designLoader, - \Magento\Framework\Message\ManagerInterface $messageManager + DesignLoader $designLoader, + MessageManagerInterface $messageManager ) { $this->_designLoader = $designLoader; $this->messageManager = $messageManager; @@ -35,17 +43,16 @@ public function __construct( /** * Initialize design * - * @param \Magento\Framework\App\ActionInterface $subject - * + * @param ActionInterface $subject * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeExecute(\Magento\Framework\App\ActionInterface $subject) + public function beforeExecute(ActionInterface $subject) { try { $this->_designLoader->load(); - } catch (\Magento\Framework\Exception\LocalizedException $e) { - if ($e->getPrevious() instanceof \Magento\Framework\Config\Dom\ValidationException) { + } catch (LocalizedException $e) { + if ($e->getPrevious() instanceof ValidationException) { /** @var MessageInterface $message */ $message = $this->messageManager ->createMessage(MessageInterface::TYPE_ERROR) diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index e7c4d610c230f..34a8fa3cb1ea2 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -1,4 +1,10 @@ -request = $request; @@ -68,10 +79,10 @@ private function getEventParameters(ActionInterface $subject): array */ public function afterExecute(ActionInterface $subject, $result) { - if (! $this->isSetActionNoPostDispatchFlag()) { + if (!$this->isSetActionNoPostDispatchFlag()) { $this->dispatchPostDispatchEvents($subject); } - + return $result; } @@ -80,12 +91,11 @@ public function afterExecute(ActionInterface $subject, $result) * * @param ActionInterface $subject * @return bool - * */ private function isSetActionNoPostDispatchFlag(): bool { return $this->actionFlag->get('', Action::FLAG_NO_DISPATCH) || - $this->actionFlag->get('', Action::FLAG_NO_POST_DISPATCH); + $this->actionFlag->get('', Action::FLAG_NO_POST_DISPATCH); } /** diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php index cc0a43a703985..c2d30187845a4 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Action/ActionTest.php @@ -8,94 +8,87 @@ use \Magento\Framework\App\Action\Action; +use Magento\Framework\App\ActionFlag; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Framework\App\Response\RedirectInterface; +use Magento\Framework\App\ResponseInterface; +use Magento\Framework\App\ViewInterface; +use Magento\Framework\Event\ManagerInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; +use Magento\Framework\View\Page\Config as PageConfig; +use PHPUnit\Framework\MockObject\MockObject; class ActionTest extends \PHPUnit\Framework\TestCase { - /** @var \Magento\Framework\App\Test\Unit\Action\ActionFake */ + /** + * @var ActionFake + */ protected $action; - /** @var ObjectManagerHelper */ + /** + * @var ObjectManagerHelper + */ protected $objectManagerHelper; /** - * @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject + * @var HttpRequest|MockObject */ protected $_requestMock; /** - * @var \Magento\Framework\App\ResponseInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ResponseInterface|MockObject */ protected $_responseMock; /** - * @var \Magento\Framework\Event\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ManagerInterface|MockObject */ protected $_eventManagerMock; /** - * @var \Magento\Framework\App\ActionFlag|\PHPUnit_Framework_MockObject_MockObject + * @var ActionFlag|MockObject */ protected $_actionFlagMock; /** - * @var \Magento\Framework\App\Response\RedirectInterface|\PHPUnit_Framework_MockObject_MockObject + * @var RedirectInterface|MockObject */ protected $_redirectMock; /** - * @var \Magento\Framework\App\ViewInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ViewInterface|MockObject */ protected $viewMock; /** - * @var \Magento\Framework\View\Page\Config|\PHPUnit_Framework_MockObject_MockObject + * @var PageConfig|MockObject */ protected $pageConfigMock; - /** - * Full action name - */ - const FULL_ACTION_NAME = 'module/controller/someaction'; - - /** - * Route name - */ - const ROUTE_NAME = 'module/controller/actionroute'; - - /** - * Action name - */ - const ACTION_NAME = 'someaction'; - - /** - * Controller name - */ - const CONTROLLER_NAME = 'controller'; - - /** - * Module name - */ - const MODULE_NAME = 'module'; + public const FULL_ACTION_NAME = 'module/controller/someaction'; + public const ROUTE_NAME = 'module/controller/actionroute'; + public const ACTION_NAME = 'someaction'; + public const CONTROLLER_NAME = 'controller'; + public const MODULE_NAME = 'module'; public static $actionParams = ['param' => 'value']; protected function setUp() { - $this->_eventManagerMock = $this->createMock(\Magento\Framework\Event\ManagerInterface::class); - $this->_actionFlagMock = $this->createMock(\Magento\Framework\App\ActionFlag::class); - $this->_redirectMock = $this->createMock(\Magento\Framework\App\Response\RedirectInterface::class); - $this->_requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class); - $this->_responseMock = $this->createMock(\Magento\Framework\App\ResponseInterface::class); - - $this->pageConfigMock = $this->createPartialMock(\Magento\Framework\View\Page\Config::class, ['getConfig']); - $this->viewMock = $this->createMock(\Magento\Framework\App\ViewInterface::class); + $this->_eventManagerMock = $this->createMock(ManagerInterface::class); + $this->_actionFlagMock = $this->createMock(ActionFlag::class); + $this->_redirectMock = $this->createMock(RedirectInterface::class); + $this->_requestMock = $this->createMock(HttpRequest::class); + $this->_responseMock = $this->createMock(ResponseInterface::class); + + $this->pageConfigMock = $this->createPartialMock(PageConfig::class, ['getConfig']); + $this->viewMock = $this->createMock(ViewInterface::class); $this->viewMock->expects($this->any())->method('getPage')->will($this->returnValue($this->pageConfigMock)); $this->pageConfigMock->expects($this->any())->method('getConfig')->willReturn(1); $this->objectManagerHelper = new ObjectManagerHelper($this); $this->action = $this->objectManagerHelper->getObject( - \Magento\Framework\App\Test\Unit\Action\ActionFake::class, + ActionFake::class, [ 'request' => $this->_requestMock, 'response' => $this->_responseMock, From b00ed884fab246f8b5caceca31d539293df4d255 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Thu, 30 Jan 2020 14:22:02 +0100 Subject: [PATCH 27/39] :rofl: debugging on production --- .../Catalog/Controller/Adminhtml/Product/MassDelete.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php index 7fa9d36163502..f0135a15c223a 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php @@ -87,8 +87,11 @@ public function execute() $this->productRepository->delete($product); $productDeleted++; } catch (LocalizedException $exception) { + $this->messageManager->addErrorMessage((string)$exception); /** @FIXME Temporary for Debugging purposes */ $this->logger->error($exception->getLogMessage()); $productDeletedError++; + } catch (\Exception $e) { + $this->messageManager->addErrorMessage((string)$e); /** @FIXME Temporary for Debugging purposes */ } } From 71b7d1082deee4f01b029734bd46be6c3cdd0fae Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Fri, 31 Jan 2020 01:28:12 +0100 Subject: [PATCH 28/39] Remove temporary debug changes --- .../Controller/Adminhtml/Product/MassDelete.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php index f0135a15c223a..c779c01cd7d71 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/MassDelete.php @@ -8,15 +8,13 @@ namespace Magento\Catalog\Controller\Adminhtml\Product; -use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface; -use Magento\Framework\Controller\ResultFactory; use Magento\Backend\App\Action\Context; -use Magento\Ui\Component\MassAction\Filter; -use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; use Magento\Catalog\Api\ProductRepositoryInterface; -use Magento\Framework\Exception\CouldNotSaveException; -use Magento\Framework\Exception\StateException; +use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; +use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface; +use Magento\Framework\Controller\ResultFactory; use Magento\Framework\Exception\LocalizedException; +use Magento\Ui\Component\MassAction\Filter; use Psr\Log\LoggerInterface; /** @@ -87,11 +85,8 @@ public function execute() $this->productRepository->delete($product); $productDeleted++; } catch (LocalizedException $exception) { - $this->messageManager->addErrorMessage((string)$exception); /** @FIXME Temporary for Debugging purposes */ $this->logger->error($exception->getLogMessage()); $productDeletedError++; - } catch (\Exception $e) { - $this->messageManager->addErrorMessage((string)$e); /** @FIXME Temporary for Debugging purposes */ } } From 611ceb44e0b413fae1a61ca62017820d4f4b172c Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 5 Feb 2020 12:14:37 +0100 Subject: [PATCH 29/39] Code Review changes: Add sortOrder to plugin, avoid returning value from beforeDispatch --- .../Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php | 6 ++---- app/code/Magento/Backend/etc/adminhtml/di.xml | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php b/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php index 1dee1e60b22cb..1dfe055a1f06c 100644 --- a/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php +++ b/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php @@ -34,13 +34,11 @@ public function __construct(DesignLoader $designLoader) * Initiates design before dispatching Backend Actions. * * @param AbstractAction $backendAction - * @param array $args - * @return array + * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeDispatch(AbstractAction $backendAction, ...$args) + public function beforeDispatch(AbstractAction $backendAction) { $this->designLoader->load(); - return $args; } } diff --git a/app/code/Magento/Backend/etc/adminhtml/di.xml b/app/code/Magento/Backend/etc/adminhtml/di.xml index e39dbeb5f6680..1bfc504cf50e9 100644 --- a/app/code/Magento/Backend/etc/adminhtml/di.xml +++ b/app/code/Magento/Backend/etc/adminhtml/di.xml @@ -62,7 +62,7 @@ - + From 46a03804b28e4f3c3748c03efa4ff19b8e9977e1 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 5 Feb 2020 18:12:26 +0100 Subject: [PATCH 30/39] Fix static analysis --- .../Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php b/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php index 1dfe055a1f06c..7075e1b05e7db 100644 --- a/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php +++ b/app/code/Magento/Backend/App/Action/Plugin/LoadDesignPlugin.php @@ -8,6 +8,7 @@ namespace Magento\Backend\App\Action\Plugin; use Magento\Backend\App\AbstractAction; +use Magento\Framework\App\RequestInterface; use Magento\Framework\View\DesignLoader; /** @@ -34,10 +35,11 @@ public function __construct(DesignLoader $designLoader) * Initiates design before dispatching Backend Actions. * * @param AbstractAction $backendAction + * @param RequestInterface $request * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function beforeDispatch(AbstractAction $backendAction) + public function beforeDispatch(AbstractAction $backendAction, RequestInterface $request) { $this->designLoader->load(); } From 306048c9296aaa30b42ddaf87574c31ea2a88b07 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Tue, 11 Feb 2020 20:56:44 +0100 Subject: [PATCH 31/39] Code Review changes to Customer module --- .../Magento/Customer/Model/Plugin/CustomerNotification.php | 4 ++-- app/code/Magento/Customer/Model/Visitor.php | 4 ++-- .../Test/Unit/Model/Plugin/CustomerNotificationTest.php | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php index aa821c9355777..db694ad3295ce 100644 --- a/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php +++ b/app/code/Magento/Customer/Model/Plugin/CustomerNotification.php @@ -69,14 +69,14 @@ public function __construct( State $state, CustomerRepositoryInterface $customerRepository, LoggerInterface $logger, - RequestInterface $request = null + RequestInterface $request ) { $this->session = $session; $this->notificationStorage = $notificationStorage; $this->state = $state; $this->customerRepository = $customerRepository; $this->logger = $logger; - $this->request = $request ?? ObjectManager::getInstance()->get(RequestInterface::class); + $this->request = $request; } /** diff --git a/app/code/Magento/Customer/Model/Visitor.php b/app/code/Magento/Customer/Model/Visitor.php index 2eb5520f7aa08..53745aa7a30c6 100644 --- a/app/code/Magento/Customer/Model/Visitor.php +++ b/app/code/Magento/Customer/Model/Visitor.php @@ -38,7 +38,7 @@ class Visitor extends AbstractModel const VISITOR_TYPE_VISITOR = 'v'; const DEFAULT_ONLINE_MINUTES_INTERVAL = 15; const XML_PATH_ONLINE_INTERVAL = 'customer/online_customers/online_minutes_interval'; - const SECONDS_24_HOURS = 86400; + private const SECONDS_24_HOURS = 86400; /** * @var string[] @@ -129,7 +129,7 @@ public function __construct( $this->scopeConfig = $scopeConfig; $this->dateTime = $dateTime; $this->indexerRegistry = $indexerRegistry; - $this->requestSafety = $requestSafety ?? ObjectManager::getInstance()->create(RequestSafetyInterface::class); + $this->requestSafety = $requestSafety ?? ObjectManager::getInstance()->get(RequestSafetyInterface::class); } /** diff --git a/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php b/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php index 86bcf59bdd113..74a5a002c7845 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Plugin/CustomerNotificationTest.php @@ -112,6 +112,10 @@ public function testBeforeExecute() ->method('remove') ->with(NotificationStorage::UPDATE_CUSTOMER_SESSION, self::STUB_CUSTOMER_ID); + $this->sessionMock->expects($this->once())->method('setCustomerData')->with($customerMock); + $this->sessionMock->expects($this->once())->method('setCustomerGroupId')->with($customerGroupId); + $this->sessionMock->expects($this->once())->method('regenerateId'); + $this->plugin->beforeExecute($this->actionMock); } From 2f0d65ae0287974d0a1b2711156f6d8595a12a45 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 12 Feb 2020 13:00:20 +0100 Subject: [PATCH 32/39] Code Review changes --- app/code/Magento/Store/etc/di.xml | 3 -- app/etc/di.xml | 5 +++ .../{Design.php => LoadDesignPlugin.php} | 2 +- .../Test/Unit/Action/Plugin/DesignTest.php | 19 ----------- .../Action/Plugin/LoadDesignPluginTest.php | 34 +++++++++++++++++++ 5 files changed, 40 insertions(+), 23 deletions(-) rename lib/internal/Magento/Framework/App/Action/Plugin/{Design.php => LoadDesignPlugin.php} (98%) delete mode 100644 lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php create mode 100644 lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php diff --git a/app/code/Magento/Store/etc/di.xml b/app/code/Magento/Store/etc/di.xml index 45f79dc5fb5f8..2a138927b1cfa 100644 --- a/app/code/Magento/Store/etc/di.xml +++ b/app/code/Magento/Store/etc/di.xml @@ -65,9 +65,6 @@ - - - diff --git a/app/etc/di.xml b/app/etc/di.xml index 8120676e8dda5..2b65451e7eaa3 100644 --- a/app/etc/di.xml +++ b/app/etc/di.xml @@ -1774,6 +1774,11 @@ + + + + + diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/Design.php b/lib/internal/Magento/Framework/App/Action/Plugin/LoadDesignPlugin.php similarity index 98% rename from lib/internal/Magento/Framework/App/Action/Plugin/Design.php rename to lib/internal/Magento/Framework/App/Action/Plugin/LoadDesignPlugin.php index a1979e88d8128..2cda49c43c2ce 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/Design.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/LoadDesignPlugin.php @@ -16,7 +16,7 @@ /** * Handling Exceptions on Design Loading */ -class Design +class LoadDesignPlugin { /** * @var DesignLoader diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php deleted file mode 100644 index 40d046f8b1678..0000000000000 --- a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/DesignTest.php +++ /dev/null @@ -1,19 +0,0 @@ -createMock(\Magento\Framework\App\Action\Action::class); - $designLoaderMock = $this->createMock(\Magento\Framework\View\DesignLoader::class); - $messageManagerMock = $this->createMock(\Magento\Framework\Message\ManagerInterface::class); - $plugin = new \Magento\Framework\App\Action\Plugin\Design($designLoaderMock, $messageManagerMock); - $designLoaderMock->expects($this->once())->method('load'); - $plugin->beforeExecute($subjectMock); - } -} diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php new file mode 100644 index 0000000000000..f0c2790b62e7a --- /dev/null +++ b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php @@ -0,0 +1,34 @@ +createMock(Action::class); + + /** @var MockObject|DesignLoader $designLoaderMock */ + $designLoaderMock = $this->createMock(DesignLoader::class); + + /** @var MockObject|ManagerInterface $messageManagerMock */ + $messageManagerMock = $this->createMock(ManagerInterface::class); + + $plugin = new LoadDesignPlugin($designLoaderMock, $messageManagerMock); + + $designLoaderMock->expects($this->once())->method('load'); + $plugin->beforeExecute($actionMock); + } +} From 067e40d42a28e926ca4e4cc144295f4945166238 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 12 Feb 2020 14:06:54 +0100 Subject: [PATCH 33/39] Fix invalid filename --- .../App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php index f0c2790b62e7a..90acfde426931 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Action/Plugin/LoadDesignPluginTest.php @@ -13,7 +13,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -class DesignTest extends TestCase +class LoadDesignPluginTest extends TestCase { public function testBeforeExecute() { From c2ffbc5ad979d4928a36f4f33cee1ed600cc7270 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 12 Feb 2020 15:57:30 +0100 Subject: [PATCH 34/39] Rollback changes to di.xml entries --- app/code/Magento/Store/etc/di.xml | 3 +++ app/etc/di.xml | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Store/etc/di.xml b/app/code/Magento/Store/etc/di.xml index 2a138927b1cfa..5bd8f6e2349fc 100644 --- a/app/code/Magento/Store/etc/di.xml +++ b/app/code/Magento/Store/etc/di.xml @@ -65,6 +65,9 @@ + + + diff --git a/app/etc/di.xml b/app/etc/di.xml index 2b65451e7eaa3..8120676e8dda5 100644 --- a/app/etc/di.xml +++ b/app/etc/di.xml @@ -1774,11 +1774,6 @@ - - - - - From cb3ebb81257a24303720787c9042236c70212892 Mon Sep 17 00:00:00 2001 From: Ihor Sviziev Date: Mon, 17 Feb 2020 11:32:53 +0200 Subject: [PATCH 35/39] magento/magento2#26778 Eliminate the need for inheritance for action controllers Fix typo --- app/code/Magento/Store/App/Action/Plugin/StoreCheck.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php b/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php index 6843e0303edd9..d1171865275a8 100644 --- a/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php +++ b/app/code/Magento/Store/App/Action/Plugin/StoreCheck.php @@ -35,7 +35,7 @@ public function __construct( * @param ActionInterface $subject * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) - * @throws \Magento\Framework\Exception\State\InitExceptionn + * @throws \Magento\Framework\Exception\State\InitException */ public function beforeExecute(ActionInterface $subject) { From 4d46d794990e63f147a7253cf6d4a66b3e3a82fa Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Mon, 24 Feb 2020 19:48:39 +0100 Subject: [PATCH 36/39] Deprecate Action class --- lib/internal/Magento/Framework/App/Action/Action.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/App/Action/Action.php b/lib/internal/Magento/Framework/App/Action/Action.php index 3ae0bbac44f8a..a6c5bb5674ee8 100644 --- a/lib/internal/Magento/Framework/App/Action/Action.php +++ b/lib/internal/Magento/Framework/App/Action/Action.php @@ -23,7 +23,8 @@ * It contains standard action behavior (event dispatching, flag checks) * Action classes that do not extend from this class will lose this behavior and might not function correctly * - * TODO: Remove this class. Allow implementation of Action Controllers by just implementing Action Interface. + * @TODO: Remove this class. Allow implementation of Action Controllers by just implementing Action Interface. + * @deprecated Use \Magento\Framework\App\ActionInterface instead * * phpcs:disable Magento2.Classes.AbstractApi * @api From 472e808b54b1af1baed3b47955faa51379493f75 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Mon, 24 Feb 2020 23:42:03 +0100 Subject: [PATCH 37/39] Adjust Unit Tests to follow Request fetched on Constructor --- .../CheckUserForgotPasswordBackendObserverTest.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Captcha/Test/Unit/Observer/CheckUserForgotPasswordBackendObserverTest.php b/app/code/Magento/Captcha/Test/Unit/Observer/CheckUserForgotPasswordBackendObserverTest.php index 584e7eb2e215f..9d3aa88ed9be4 100644 --- a/app/code/Magento/Captcha/Test/Unit/Observer/CheckUserForgotPasswordBackendObserverTest.php +++ b/app/code/Magento/Captcha/Test/Unit/Observer/CheckUserForgotPasswordBackendObserverTest.php @@ -101,6 +101,7 @@ protected function setUp() ->getMockForAbstractClass(); $this->actionFlagMock = $this->createMock(ActionFlag::class); $this->messageManagerMock = $this->createMock(ManagerInterface::class); + $this->requestMock = $this->createMock(HttpRequest::class); $objectManager = new ObjectManagerHelper($this); $this->observer = $objectManager->getObject( @@ -110,7 +111,8 @@ protected function setUp() 'captchaStringResolver' => $this->captchaStringResolverMock, '_session' => $this->sessionMock, '_actionFlag' => $this->actionFlagMock, - 'messageManager' => $this->messageManagerMock + 'messageManager' => $this->messageManagerMock, + 'request' => $this->requestMock ] ); @@ -122,16 +124,12 @@ protected function setUp() ->with($formId) ->willReturn($this->captchaMock); - $this->requestMock = $this->createMock(HttpRequest::class); $this->httpResponseMock = $this->createMock(HttpResponse::class); $this->controllerMock = $this->getMockBuilder(Action::class) ->disableOriginalConstructor() - ->setMethods(['getUrl', 'getRequest', 'getResponse']) + ->setMethods(['getUrl', 'getResponse']) ->getMockForAbstractClass(); - $this->controllerMock->expects($this->any()) - ->method('getRequest') - ->willReturn($this->requestMock); $this->controllerMock->expects($this->any()) ->method('getResponse') ->willReturn($this->httpResponseMock); From 967aa6da34b6de2fb8320fe431ac47d8cf7cf162 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 26 Feb 2020 19:07:45 +0100 Subject: [PATCH 38/39] Restore Profiler for `postdispatch`, remove `@TODO` and change `@deprecated` message --- lib/internal/Magento/Framework/App/Action/Action.php | 3 +-- .../Framework/App/Action/Plugin/EventDispatchPlugin.php | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Action/Action.php b/lib/internal/Magento/Framework/App/Action/Action.php index a6c5bb5674ee8..2acf4d36af457 100644 --- a/lib/internal/Magento/Framework/App/Action/Action.php +++ b/lib/internal/Magento/Framework/App/Action/Action.php @@ -23,8 +23,7 @@ * It contains standard action behavior (event dispatching, flag checks) * Action classes that do not extend from this class will lose this behavior and might not function correctly * - * @TODO: Remove this class. Allow implementation of Action Controllers by just implementing Action Interface. - * @deprecated Use \Magento\Framework\App\ActionInterface instead + * @deprecated 2.4.0, use \Magento\Framework\App\ActionInterface * * phpcs:disable Magento2.Classes.AbstractApi * @api diff --git a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php index 34a8fa3cb1ea2..7d07d1f4cf457 100644 --- a/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php +++ b/lib/internal/Magento/Framework/App/Action/Plugin/EventDispatchPlugin.php @@ -16,6 +16,7 @@ use Magento\Framework\Controller\ResultInterface; use Magento\Framework\Event\ManagerInterface; use Magento\Framework\HTTP\PhpEnvironment\Response; +use Magento\Framework\Profiler; /** * Dispatch the controller_action_predispatch and controller_action_post_dispatch events. @@ -63,7 +64,7 @@ public function beforeExecute(ActionInterface $subject) * Build the event parameter array * * @param ActionInterface $subject - * @return mixed[] + * @return array */ private function getEventParameters(ActionInterface $subject): array { @@ -89,7 +90,6 @@ public function afterExecute(ActionInterface $subject, $result) /** * Check if action flags are set that would suppress the post dispatch events. * - * @param ActionInterface $subject * @return bool */ private function isSetActionNoPostDispatchFlag(): bool @@ -123,6 +123,7 @@ private function dispatchPreDispatchEvents(ActionInterface $action) */ private function dispatchPostDispatchEvents(ActionInterface $action) { + Profiler::start('postdispatch'); $this->eventManager->dispatch( 'controller_action_postdispatch_' . $this->request->getFullActionName(), $this->getEventParameters($action) @@ -132,5 +133,6 @@ private function dispatchPostDispatchEvents(ActionInterface $action) $this->getEventParameters($action) ); $this->eventManager->dispatch('controller_action_postdispatch', $this->getEventParameters($action)); + Profiler::stop('postdispatch'); } } From 909beac8a982bbeef2d2c54c0114f6786b04b604 Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Wed, 26 Feb 2020 16:28:25 -0600 Subject: [PATCH 39/39] Fixed deprecated messages --- app/code/Magento/Backend/App/AbstractAction.php | 2 ++ lib/internal/Magento/Framework/App/Action/Action.php | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Backend/App/AbstractAction.php b/app/code/Magento/Backend/App/AbstractAction.php index 583fc723cc38b..2f01700bdf51c 100644 --- a/app/code/Magento/Backend/App/AbstractAction.php +++ b/app/code/Magento/Backend/App/AbstractAction.php @@ -20,6 +20,8 @@ /** * Generic backend controller * + * @deprecated Use \Magento\Framework\App\ActionInterface + * * phpcs:disable Magento2.Classes.AbstractApi * @api * @SuppressWarnings(PHPMD.NumberOfChildren) diff --git a/lib/internal/Magento/Framework/App/Action/Action.php b/lib/internal/Magento/Framework/App/Action/Action.php index 2acf4d36af457..6a3b665c7d3ed 100644 --- a/lib/internal/Magento/Framework/App/Action/Action.php +++ b/lib/internal/Magento/Framework/App/Action/Action.php @@ -23,7 +23,7 @@ * It contains standard action behavior (event dispatching, flag checks) * Action classes that do not extend from this class will lose this behavior and might not function correctly * - * @deprecated 2.4.0, use \Magento\Framework\App\ActionInterface + * @deprecated Use \Magento\Framework\App\ActionInterface * * phpcs:disable Magento2.Classes.AbstractApi * @api