Skip to content

Commit f661224

Browse files
slavvkaslavvka
authored
ENGCOM-7107: Mark AbstractAccount as DEPRECATED for Magento_Customer controllers #27214 
2 parents 4b9960c + 4f428fd commit f661224

File tree

5 files changed

+158
-107
lines changed

5 files changed

+158
-107
lines changed

‎app/code/Magento/Customer/Controller/AbstractAccount.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
use Magento\Framework\App\Action\Action;
1010

1111
/**
12-
* Class AbstractAccount
13-
* @package Magento\Customer\Controller
12+
* AbstractAccount class is deprecated, in favour of Composition approach to build Controllers
13+
*
1414
* @SuppressWarnings(PHPMD.NumberOfChildren)
15+
* @deprecated
16+
* @see \Magento\Customer\Controller\AccountInterface
1517
*/
1618
abstract class AbstractAccount extends Action implements AccountInterface
1719
{

‎app/code/Magento/Customer/Controller/Plugin/Account.php

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,72 +3,78 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Customer\Controller\Plugin;
79

10+
use Closure;
11+
use Magento\Customer\Controller\AccountInterface;
812
use Magento\Customer\Model\Session;
9-
use Magento\Framework\App\ActionInterface;
1013
use Magento\Framework\App\RequestInterface;
1114
use Magento\Framework\App\ResponseInterface;
12-
use Magento\Framework\App\Action\AbstractAction;
1315
use Magento\Framework\Controller\ResultInterface;
1416

17+
/**
18+
* Plugin verifies permissions using Action Name against injected (`fontend/di.xml`) rules
19+
*/
1520
class Account
1621
{
1722
/**
1823
* @var Session
1924
*/
20-
protected $session;
25+
private $session;
26+
27+
/**
28+
* @var RequestInterface
29+
*/
30+
private $request;
2131

2232
/**
2333
* @var array
2434
*/
2535
private $allowedActions = [];
2636

2737
/**
38+
* @param RequestInterface $request
2839
* @param Session $customerSession
2940
* @param array $allowedActions List of actions that are allowed for not authorized users
3041
*/
3142
public function __construct(
43+
RequestInterface $request,
3244
Session $customerSession,
3345
array $allowedActions = []
3446
) {
47+
$this->request = $request;
3548
$this->session = $customerSession;
3649
$this->allowedActions = $allowedActions;
3750
}
3851

3952
/**
40-
* Dispatch actions allowed for not authorized users
53+
* Executes original method if allowed, otherwise - redirects to log in
4154
*
42-
* @param AbstractAction $subject
43-
* @param RequestInterface $request
44-
* @return void
55+
* @param AccountInterface $controllerAction
56+
* @param Closure $proceed
57+
* @return ResultInterface|ResponseInterface|void
58+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
4559
*/
46-
public function beforeDispatch(AbstractAction $subject, RequestInterface $request)
60+
public function aroundExecute(AccountInterface $controllerAction, Closure $proceed)
4761
{
48-
$action = strtolower($request->getActionName());
49-
$pattern = '/^(' . implode('|', $this->allowedActions) . ')$/i';
50-
51-
if (!preg_match($pattern, $action)) {
52-
if (!$this->session->authenticate()) {
53-
$subject->getActionFlag()->set('', ActionInterface::FLAG_NO_DISPATCH, true);
54-
}
55-
} else {
56-
$this->session->setNoReferer(true);
62+
/** @FIXME Move Authentication and redirect out of Session model */
63+
if ($this->isActionAllowed() || $this->session->authenticate()) {
64+
return $proceed();
5765
}
5866
}
5967

6068
/**
61-
* Remove No-referer flag from customer session
69+
* Validates whether currently requested action is one of the allowed
6270
*
63-
* @param AbstractAction $subject
64-
* @param ResponseInterface|ResultInterface $result
65-
* @param RequestInterface $request
66-
* @return ResponseInterface|ResultInterface
67-
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
71+
* @return bool
6872
*/
69-
public function afterDispatch(AbstractAction $subject, $result, RequestInterface $request)
73+
private function isActionAllowed(): bool
7074
{
71-
$this->session->unsNoReferer(false);
72-
return $result;
75+
$action = strtolower($this->request->getActionName());
76+
$pattern = '/^(' . implode('|', $this->allowedActions) . ')$/i';
77+
78+
return (bool)preg_match($pattern, $action);
7379
}
7480
}

‎app/code/Magento/Customer/Test/Unit/Controller/Plugin/AccountTest.php

Lines changed: 51 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,23 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Customer\Test\Unit\Controller\Plugin;
78

9+
use Closure;
10+
use Magento\Customer\Controller\AccountInterface;
811
use Magento\Customer\Controller\Plugin\Account;
912
use Magento\Customer\Model\Session;
1013
use Magento\Framework\App\ActionFlag;
1114
use Magento\Framework\App\ActionInterface;
12-
use Magento\Framework\App\Action\AbstractAction;
1315
use Magento\Framework\App\Request\Http;
16+
use Magento\Framework\App\Request\Http as HttpRequest;
1417
use Magento\Framework\Controller\ResultInterface;
1518
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
19+
use PHPUnit\Framework\MockObject\MockObject;
20+
use PHPUnit\Framework\TestCase;
1621

17-
class AccountTest extends \PHPUnit\Framework\TestCase
22+
class AccountTest extends TestCase
1823
{
1924
/**
2025
* @var string
@@ -27,110 +32,98 @@ class AccountTest extends \PHPUnit\Framework\TestCase
2732
protected $plugin;
2833

2934
/**
30-
* @var Session | \PHPUnit_Framework_MockObject_MockObject
35+
* @var Session|MockObject
3136
*/
32-
protected $session;
37+
protected $sessionMock;
3338

3439
/**
35-
* @var AbstractAction | \PHPUnit_Framework_MockObject_MockObject
40+
* @var AccountInterface|MockObject
3641
*/
37-
protected $subject;
42+
protected $actionMock;
3843

3944
/**
40-
* @var Http | \PHPUnit_Framework_MockObject_MockObject
45+
* @var Http|MockObject
4146
*/
42-
protected $request;
47+
protected $requestMock;
4348

4449
/**
45-
* @var ActionFlag | \PHPUnit_Framework_MockObject_MockObject
50+
* @var ActionFlag|MockObject
4651
*/
47-
protected $actionFlag;
52+
protected $actionFlagMock;
4853

4954
/**
50-
* @var ResultInterface|\PHPUnit_Framework_MockObject_MockObject
55+
* @var ResultInterface|MockObject
5156
*/
52-
private $resultInterface;
57+
private $resultMock;
5358

5459
protected function setUp()
5560
{
56-
$this->session = $this->getMockBuilder(\Magento\Customer\Model\Session::class)
61+
$this->sessionMock = $this->getMockBuilder(Session::class)
5762
->disableOriginalConstructor()
58-
->setMethods([
59-
'setNoReferer',
60-
'unsNoReferer',
61-
'authenticate',
62-
])
63+
->setMethods(['setNoReferer', 'unsNoReferer', 'authenticate'])
6364
->getMock();
6465

65-
$this->subject = $this->getMockBuilder(AbstractAction::class)
66-
->setMethods([
67-
'getActionFlag',
68-
])
66+
$this->actionMock = $this->getMockBuilder(AccountInterface::class)
67+
->setMethods(['getActionFlag'])
6968
->disableOriginalConstructor()
7069
->getMockForAbstractClass();
7170

72-
$this->request = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class)
71+
$this->requestMock = $this->getMockBuilder(HttpRequest::class)
7372
->disableOriginalConstructor()
74-
->setMethods([
75-
'getActionName',
76-
])
73+
->setMethods(['getActionName'])
7774
->getMock();
7875

79-
$this->resultInterface = $this->getMockBuilder(ResultInterface::class)
76+
$this->resultMock = $this->getMockBuilder(ResultInterface::class)
8077
->getMockForAbstractClass();
8178

82-
$this->actionFlag = $this->getMockBuilder(\Magento\Framework\App\ActionFlag::class)
79+
$this->actionFlagMock = $this->getMockBuilder(ActionFlag::class)
8380
->disableOriginalConstructor()
8481
->getMock();
8582
}
8683

8784
/**
8885
* @param string $action
8986
* @param array $allowedActions
90-
* @param boolean $isActionAllowed
91-
* @param boolean $isAuthenticated
87+
* @param boolean $isAllowed
9288
*
93-
* @dataProvider beforeDispatchDataProvider
89+
* @dataProvider beforeExecuteDataProvider
9490
*/
95-
public function testBeforeDispatch(
96-
$action,
97-
$allowedActions,
98-
$isActionAllowed,
99-
$isAuthenticated
91+
public function testAroundExecuteInterruptsOriginalCallWhenNotAllowed(
92+
string $action,
93+
array $allowedActions,
94+
bool $isAllowed
10095
) {
101-
$this->request->expects($this->once())
96+
/** @var callable|MockObject $proceedMock */
97+
$proceedMock = $this->getMockBuilder(\stdClass::class)
98+
->setMethods(['__invoke'])
99+
->getMock();
100+
101+
$closureMock = Closure::fromCallable($proceedMock);
102+
103+
$this->requestMock->expects($this->once())
102104
->method('getActionName')
103105
->willReturn($action);
104106

105-
if ($isActionAllowed) {
106-
$this->session->expects($this->once())
107-
->method('setNoReferer')
108-
->with(true)
109-
->willReturnSelf();
107+
if ($isAllowed) {
108+
$proceedMock->expects($this->once())->method('__invoke')->willReturn($this->resultMock);
110109
} else {
111-
$this->session->expects($this->once())
112-
->method('authenticate')
113-
->willReturn($isAuthenticated);
114-
if (!$isAuthenticated) {
115-
$this->subject->expects($this->once())
116-
->method('getActionFlag')
117-
->willReturn($this->actionFlag);
118-
119-
$this->actionFlag->expects($this->once())
120-
->method('set')
121-
->with('', ActionInterface::FLAG_NO_DISPATCH, true)
122-
->willReturnSelf();
123-
}
110+
$proceedMock->expects($this->never())->method('__invoke');
124111
}
125112

126-
$plugin = new Account($this->session, $allowedActions);
127-
$plugin->beforeDispatch($this->subject, $this->request);
113+
$plugin = new Account($this->requestMock, $this->sessionMock, $allowedActions);
114+
$result = $plugin->aroundExecute($this->actionMock, $closureMock);
115+
116+
if ($isAllowed) {
117+
$this->assertSame($this->resultMock, $result);
118+
} else {
119+
$this->assertNull($result);
120+
}
128121
}
129122

130123
/**
131124
* @return array
132125
*/
133-
public function beforeDispatchDataProvider()
126+
public function beforeExecuteDataProvider()
134127
{
135128
return [
136129
[
@@ -165,24 +158,4 @@ public function beforeDispatchDataProvider()
165158
],
166159
];
167160
}
168-
169-
public function testAfterDispatch()
170-
{
171-
$this->session->expects($this->once())
172-
->method('unsNoReferer')
173-
->with(false)
174-
->willReturnSelf();
175-
176-
$plugin = (new ObjectManager($this))->getObject(
177-
Account::class,
178-
[
179-
'session' => $this->session,
180-
'allowedActions' => ['testaction']
181-
]
182-
);
183-
$this->assertSame(
184-
$this->resultInterface,
185-
$plugin->afterDispatch($this->subject, $this->resultInterface, $this->request)
186-
);
187-
}
188161
}

‎app/code/Magento/Customer/etc/frontend/di.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
</argument>
5252
</arguments>
5353
</type>
54-
<type name="Magento\Customer\Controller\AbstractAccount">
54+
<type name="Magento\Customer\Controller\AccountInterface">
5555
<plugin name="customer_account" type="Magento\Customer\Controller\Plugin\Account" />
5656
</type>
5757
<type name="Magento\Checkout\Block\Cart\Sidebar">

0 commit comments

Comments
 (0)