Skip to content

Commit c5218a2

Browse files
eddielaueddielau
committed
MAGETWO-34228: Session must not be cleared for POST request if layout is cacheable
- added request type check during depersonalization - changed unit tests
1 parent 3c25553 commit c5218a2

File tree

10 files changed

+105
-167
lines changed

10 files changed

+105
-167
lines changed

‎app/code/Magento/Catalog/Model/Layout/DepersonalizePlugin.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public function afterGenerateXml(\Magento\Framework\View\LayoutInterface $subjec
6464
if ($this->moduleManager->isEnabled('Magento_PageCache')
6565
&& $this->cacheConfig->isEnabled()
6666
&& !$this->request->isAjax()
67+
&& ($this->request->isGet() || $this->request->isHead())
6768
&& $subject->isCacheable()
6869
) {
6970
$this->catalogSession->clearStorage();

‎app/code/Magento/Checkout/Model/Layout/DepersonalizePlugin.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public function afterGenerateXml(\Magento\Framework\View\LayoutInterface $subjec
6161
if ($this->moduleManager->isEnabled('Magento_PageCache')
6262
&& $this->cacheConfig->isEnabled()
6363
&& !$this->request->isAjax()
64+
&& ($this->request->isGet() || $this->request->isHead())
6465
&& $subject->isCacheable()
6566
) {
6667
$this->checkoutSession->clearStorage();

‎app/code/Magento/Customer/Model/Layout/DepersonalizePlugin.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,11 @@ public function __construct(
9292
*/
9393
public function beforeGenerateXml(\Magento\Framework\View\LayoutInterface $subject)
9494
{
95-
if ($this->moduleManager->isEnabled(
96-
'Magento_PageCache'
97-
) && $this->cacheConfig->isEnabled() && !$this->request->isAjax() && $subject->isCacheable()
95+
if ($this->moduleManager->isEnabled('Magento_PageCache')
96+
&& $this->cacheConfig->isEnabled()
97+
&& !$this->request->isAjax()
98+
&& ($this->request->isGet() || $this->request->isHead())
99+
&& $subject->isCacheable()
98100
) {
99101
$this->customerGroupId = $this->customerSession->getCustomerGroupId();
100102
$this->formKey = $this->session->getData(\Magento\Framework\Data\Form\FormKey::FORM_KEY);
@@ -114,6 +116,7 @@ public function afterGenerateXml(\Magento\Framework\View\LayoutInterface $subjec
114116
if ($this->moduleManager->isEnabled('Magento_PageCache')
115117
&& $this->cacheConfig->isEnabled()
116118
&& !$this->request->isAjax()
119+
&& ($this->request->isGet() || $this->request->isHead())
117120
&& $subject->isCacheable()
118121
) {
119122
$this->visitor->setSkipRequestLogging(true);

‎app/code/Magento/PageCache/Model/Layout/DepersonalizePlugin.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public function afterGenerateXml(\Magento\Framework\View\LayoutInterface $subjec
6969
if ($this->moduleManager->isEnabled('Magento_PageCache')
7070
&& $this->cacheConfig->isEnabled()
7171
&& !$this->request->isAjax()
72+
&& ($this->request->isGet() || $this->request->isHead())
7273
&& $subject->isCacheable()
7374
) {
7475
$this->eventManager->dispatch('depersonalize_clear_session');

‎app/code/Magento/Persistent/Model/Layout/DepersonalizePlugin.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public function afterGenerateXml(
6464
if ($this->moduleManager->isEnabled('Magento_PageCache')
6565
&& $this->cacheConfig->isEnabled()
6666
&& !$this->request->isAjax()
67+
&& ($this->request->isGet() || $this->request->isHead())
6768
&& $subject->isCacheable()
6869
) {
6970
$this->persistentSession->setCustomerId(null);

‎dev/tests/unit/testsuite/Magento/Catalog/Model/Layout/DepersonalizePluginTest.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ public function testAfterGenerateXml()
7474
$this->moduleManagerMock->expects($this->once())->method('isEnabled')->with('Magento_PageCache')
7575
->willReturn(true);
7676
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->willReturn(true);
77-
$this->requestMock->expects($this->once($this->once()))->method('isAjax')->willReturn(false);
77+
$this->requestMock->expects($this->once())->method('isAjax')->willReturn(false);
78+
$this->requestMock->expects($this->once())->method('isGet')->willReturn(true);
7879
$this->layoutMock->expects($this->once())->method('isCacheable')->willReturn(true);
7980
$this->catalogSessionMock->expects($this->once())->method('clearStorage');
8081

@@ -114,7 +115,7 @@ public function testIsAjax()
114115
->with($this->equalTo('Magento_PageCache'))
115116
->willReturn(true);
116117
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->willReturn(true);
117-
$this->requestMock->expects($this->once($this->once()))->method('isAjax')->willReturn(true);
118+
$this->requestMock->expects($this->once())->method('isAjax')->willReturn(true);
118119
$this->catalogSessionMock->expects($this->never())->method('clearStorage');
119120

120121
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $this->resultLayout);
@@ -128,11 +129,28 @@ public function testLayoutIsNotCacheable()
128129
->with($this->equalTo('Magento_PageCache'))
129130
->willReturn(true);
130131
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->willReturn(true);
131-
$this->requestMock->expects($this->once($this->once()))->method('isAjax')->willReturn(false);
132+
$this->requestMock->expects($this->once())->method('isAjax')->willReturn(false);
133+
$this->requestMock->expects($this->once())->method('isGet')->willReturn(true);
132134
$this->layoutMock->expects($this->once())->method('isCacheable')->willReturn(false);
133135
$this->catalogSessionMock->expects($this->never())->method('clearStorage');
134136

135137
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $this->resultLayout);
136138
$this->assertEquals($this->resultLayout, $actualResult);
137139
}
140+
141+
public function testIsPost()
142+
{
143+
$this->moduleManagerMock->expects($this->once())
144+
->method('isEnabled')
145+
->with($this->equalTo('Magento_PageCache'))
146+
->willReturn(true);
147+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->willReturn(true);
148+
$this->requestMock->expects($this->once())->method('isAjax')->willReturn(false);
149+
$this->requestMock->expects($this->once())->method('isGet')->willReturn(false);
150+
$this->requestMock->expects($this->once())->method('isHead')->willReturn(false);
151+
$this->catalogSessionMock->expects($this->never())->method('clearStorage');
152+
153+
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $this->resultLayout);
154+
$this->assertEquals($this->resultLayout, $actualResult);
155+
}
138156
}

‎dev/tests/unit/testsuite/Magento/Checkout/Model/Layout/DepersonalizePluginTest.php

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,13 @@ public function testAfterGenerateXml()
8484
->method('isEnabled')
8585
->with($this->equalTo('Magento_PageCache'))
8686
->will($this->returnValue(true));
87-
$this->cacheConfigMock->expects($this->once())
88-
->method('isEnabled')
89-
->will($this->returnValue(true));
90-
$this->requestMock->expects($this->once($this->once()))
91-
->method('isAjax')
92-
->will($this->returnValue(false));
93-
$this->layoutMock->expects($this->once())
94-
->method('isCacheable')
95-
->will($this->returnValue(true));
87+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
88+
$this->requestMock->expects($this->once($this->once()))->method('isAjax')->will($this->returnValue(false));
89+
$this->requestMock->expects($this->once($this->once()))->method('isGet')->willReturn(true);
90+
$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue(true));
9691

97-
$this->checkoutSessionMock->expects($this->once())
92+
$this->checkoutSessionMock
93+
->expects($this->once())
9894
->method('clearStorage')
9995
->will($this->returnValue($expectedResult));
10096

‎dev/tests/unit/testsuite/Magento/Customer/Model/Layout/DepersonalizePluginTest.php

Lines changed: 51 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,13 @@ public function testBeforeGenerateXmlPageCacheEnabled()
124124
->method('isEnabled')
125125
->with('Magento_PageCache')
126126
->will($this->returnValue(true));
127-
$this->cacheConfigMock
128-
->expects($this->once())
129-
->method('isEnabled')
130-
->will($this->returnValue(true));
131-
$this->requestMock
132-
->expects($this->once())
133-
->method('isAjax')
134-
->will($this->returnValue(false));
135-
$this->layoutMock
127+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
128+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(false));
129+
$this->requestMock->expects($this->once())->method('isGet')->will($this->returnValue(true));
130+
$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue(true));
131+
$this->customerSessionMock->expects($this->once())->method('getCustomerGroupId');
132+
$this->sessionMock
136133
->expects($this->once())
137-
->method('isCacheable')
138-
->will($this->returnValue(true));
139-
$this->customerSessionMock->expects($this->once())
140-
->method('getCustomerGroupId');
141-
$this->sessionMock->expects($this->once())
142134
->method('getData')
143135
->with($this->equalTo(\Magento\Framework\Data\Form\FormKey::FORM_KEY));
144136
$output = $this->plugin->beforeGenerateXml($this->layoutMock);
@@ -155,9 +147,7 @@ public function testBeforeGenerateXmlPageCacheDisabled()
155147
->method('isEnabled')
156148
->with('Magento_PageCache')
157149
->will($this->returnValue(false));
158-
$this->requestMock
159-
->expects($this->never())
160-
->method('isAjax');
150+
$this->requestMock->expects($this->never())->method('isAjax');
161151
$output = $this->plugin->beforeGenerateXml($this->layoutMock);
162152
$this->assertEquals([], $output);
163153
}
@@ -172,16 +162,9 @@ public function testBeforeGenerateXmlRequestIsAjax()
172162
->method('isEnabled')
173163
->with('Magento_PageCache')
174164
->will($this->returnValue(true));
175-
$this->cacheConfigMock
176-
->expects($this->once())
177-
->method('isEnabled')
178-
->will($this->returnValue(true));
179-
$this->requestMock
180-
->expects($this->once())
181-
->method('isAjax')
182-
->will($this->returnValue(true));
183-
$this->layoutMock->expects($this->never())
184-
->method('isCacheable');
165+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
166+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(true));
167+
$this->layoutMock->expects($this->never())->method('isCacheable');
185168
$output = $this->plugin->beforeGenerateXml($this->layoutMock);
186169
$this->assertEquals([], $output);
187170
}
@@ -196,19 +179,11 @@ public function testBeforeGenerateXmlLayoutIsNotCacheable()
196179
->method('isEnabled')
197180
->with('Magento_PageCache')
198181
->will($this->returnValue(true));
199-
$this->cacheConfigMock
200-
->expects($this->once())
201-
->method('isEnabled')
202-
->will($this->returnValue(true));
203-
$this->requestMock
204-
->expects($this->once())
205-
->method('isAjax')
206-
->will($this->returnValue(false));
207-
$this->layoutMock->expects($this->once())
208-
->method('isCacheable')
209-
->will($this->returnValue(false));
210-
$this->customerSessionMock->expects($this->never())
211-
->method('getCustomerGroupId');
182+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
183+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(false));
184+
$this->requestMock->expects($this->once())->method('isGet')->will($this->returnValue(true));
185+
$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue(false));
186+
$this->customerSessionMock->expects($this->never())->method('getCustomerGroupId');
212187
$output = $this->plugin->beforeGenerateXml($this->layoutMock);
213188
$this->assertEquals([], $output);
214189
}
@@ -224,40 +199,16 @@ public function testAfterGenerateXmlPageCacheEnabled()
224199
->method('isEnabled')
225200
->with('Magento_PageCache')
226201
->will($this->returnValue(true));
227-
$this->cacheConfigMock
228-
->expects($this->once())
229-
->method('isEnabled')
230-
->will($this->returnValue(true));
231-
$this->requestMock
232-
->expects($this->once())
233-
->method('isAjax')
234-
->will($this->returnValue(false));
235-
$this->layoutMock
236-
->expects($this->once())
237-
->method('isCacheable')
238-
->will($this->returnValue(true));
239-
$this->visitorMock
240-
->expects($this->once())
241-
->method('setSkipRequestLogging')
242-
->with($this->equalTo(true));
243-
$this->visitorMock
244-
->expects($this->once())
245-
->method('unsetData');
246-
$this->sessionMock
247-
->expects($this->once())
248-
->method('clearStorage');
249-
$this->customerSessionMock
250-
->expects($this->once())
251-
->method('clearStorage');
252-
$this->customerSessionMock
253-
->expects($this->once())
254-
->method('setCustomerGroupId')
255-
->with($this->equalTo(null));
256-
$this->customerMock
257-
->expects($this->once())
258-
->method('setGroupId')
259-
->with($this->equalTo(null))
260-
->willReturnSelf();
202+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
203+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(false));
204+
$this->requestMock->expects($this->once())->method('isGet')->will($this->returnValue(true));
205+
$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue(true));
206+
$this->visitorMock->expects($this->once())->method('setSkipRequestLogging')->with($this->equalTo(true));
207+
$this->visitorMock->expects($this->once())->method('unsetData');
208+
$this->sessionMock->expects($this->once())->method('clearStorage');
209+
$this->customerSessionMock->expects($this->once())->method('clearStorage');
210+
$this->customerSessionMock->expects($this->once())->method('setCustomerGroupId')->with($this->equalTo(null));
211+
$this->customerMock->expects($this->once())->method('setGroupId')->with($this->equalTo(null))->willReturnSelf();
261212
$this->sessionMock
262213
->expects($this->once())
263214
->method('setData')
@@ -284,9 +235,7 @@ public function testAfterGenerateXmlPageCacheDisabled()
284235
->method('isEnabled')
285236
->with('Magento_PageCache')
286237
->will($this->returnValue(false));
287-
$this->requestMock
288-
->expects($this->never())
289-
->method('isAjax');
238+
$this->requestMock->expects($this->never())->method('isAjax');
290239
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $expectedResult);
291240
$this->assertSame($expectedResult, $actualResult);
292241
}
@@ -302,16 +251,29 @@ public function testAfterGenerateXmlRequestIsAjax()
302251
->method('isEnabled')
303252
->with($this->equalTo('Magento_PageCache'))
304253
->will($this->returnValue(true));
305-
$this->cacheConfigMock
254+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
255+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(true));
256+
$this->layoutMock->expects($this->never())->method('isCacheable');
257+
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $expectedResult);
258+
$this->assertSame($expectedResult, $actualResult);
259+
}
260+
261+
/**
262+
* Test afterGenerateXml method with enabled module PageCache and request is Post
263+
*/
264+
public function testAfterGenerateXmlRequestIsPost()
265+
{
266+
$expectedResult = $this->getMock('Magento\Framework\View\Layout', [], [], '', false);
267+
$this->moduleManagerMock
306268
->expects($this->once())
307269
->method('isEnabled')
270+
->with($this->equalTo('Magento_PageCache'))
308271
->will($this->returnValue(true));
309-
$this->requestMock
310-
->expects($this->once())
311-
->method('isAjax')
312-
->will($this->returnValue(true));
313-
$this->layoutMock->expects($this->never())
314-
->method('isCacheable');
272+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
273+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(false));
274+
$this->requestMock->expects($this->once())->method('isGet')->will($this->returnValue(false));
275+
$this->requestMock->expects($this->once())->method('isHead')->will($this->returnValue(false));
276+
$this->layoutMock->expects($this->never())->method('isCacheable');
315277
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $expectedResult);
316278
$this->assertSame($expectedResult, $actualResult);
317279
}
@@ -327,20 +289,11 @@ public function testAfterGenerateXmlLayoutIsNotCacheable()
327289
->method('isEnabled')
328290
->with($this->equalTo('Magento_PageCache'))
329291
->will($this->returnValue(true));
330-
$this->cacheConfigMock
331-
->expects($this->once())
332-
->method('isEnabled')
333-
->will($this->returnValue(true));
334-
$this->requestMock
335-
->expects($this->once())
336-
->method('isAjax')
337-
->will($this->returnValue(false));
338-
$this->layoutMock->expects($this->once())
339-
->method('isCacheable')
340-
->will($this->returnValue(false));
341-
$this->visitorMock
342-
->expects($this->never())
343-
->method('setSkipRequestLogging');
292+
$this->cacheConfigMock->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
293+
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(false));
294+
$this->requestMock->expects($this->once())->method('isGet')->will($this->returnValue(true));
295+
$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue(false));
296+
$this->visitorMock->expects($this->never())->method('setSkipRequestLogging');
344297
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $expectedResult);
345298
$this->assertSame($expectedResult, $actualResult);
346299
}

‎dev/tests/unit/testsuite/Magento/PageCache/Model/Layout/DepersonalizePluginTest.php

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,14 @@ public function testAfterGenerateXmlPageCacheEnabled()
8686
->method('isEnabled')
8787
->with($this->equalTo('Magento_PageCache'))
8888
->will($this->returnValue(true));
89-
$this->requestMock->expects($this->once($this->once()))
90-
->method('isAjax')
91-
->will($this->returnValue(false));
92-
$this->layoutMock->expects($this->once())
93-
->method('isCacheable')
94-
->will($this->returnValue(true));
89+
$this->requestMock->expects($this->once($this->once()))->method('isAjax')->will($this->returnValue(false));
90+
$this->requestMock->expects($this->once($this->once()))->method('isGet')->will($this->returnValue(true));
91+
$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue(true));
9592

9693
$this->eventManagerMock->expects($this->once())
9794
->method('dispatch')
9895
->with($this->equalTo('depersonalize_clear_session'));
99-
$this->messageSessionMock->expects($this->once())
100-
->method('clearStorage');
96+
$this->messageSessionMock->expects($this->once())->method('clearStorage');
10197

10298
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $expectedResult);
10399
$this->assertEquals($expectedResult, $actualResult);
@@ -113,9 +109,7 @@ public function testAfterGenerateXmlPageCacheDisabled()
113109
->method('isEnabled')
114110
->with($this->equalTo('Magento_PageCache'))
115111
->will($this->returnValue(false));
116-
$this->requestMock->expects($this->never())
117-
->method('isAjax')
118-
->will($this->returnValue(false));
112+
$this->requestMock->expects($this->never())->method('isAjax')->will($this->returnValue(false));
119113
$actualResult = $this->plugin->afterGenerateXml($this->layoutMock, $expectedResult);
120114
$this->assertEquals($expectedResult, $actualResult);
121115
}

0 commit comments

Comments
 (0)