Change the customer notification plugin from AbstractAction:dispatch to ActionInterface::execute

Vinai · Vinai · commit e0d905624045 · 2018-06-20T07:07:49.000-04:00
diff --git 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
@@ -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
@@ -333,7 +333,7 @@
     <type name="Magento\Directory\Model\AllowedCountries">
         <plugin name="customerAllowedCountries" type="Magento\Customer\Model\Plugin\AllowedCountries"/>
     </type>
-    <type name="Magento\Framework\App\Action\AbstractAction">
+    <type name="Magento\Framework\App\ActionInterface">
         <plugin name="customerNotification" type="Magento\Customer\Model\Plugin\CustomerNotification"/>
     </type>
     <type name="Magento\PageCache\Observer\FlushFormKey">
