Skip to content

Commit

Permalink
magento#24229: Don't disable FPC for maintenance, instead don't set p…
Browse files Browse the repository at this point in the history
…ublic cache headers

- Removes the observer for disabling and re-enabling the FPC during
  maintenande mode switch

- Disables setting public cache headers, if maintenance mode is enabled

- phpcs:ignore entries were added in places where no actual code was
  changed by this commit, but static tests failed
  • Loading branch information
theCapypara committed Nov 29, 2019
1 parent c7ca62e commit acd42b8
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 285 deletions.
16 changes: 13 additions & 3 deletions app/code/Magento/PageCache/Model/Layout/LayoutPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,31 @@ class LayoutPlugin
*/
protected $response;

/**
* @var \Magento\Framework\App\MaintenanceMode
*/
private $maintenanceMode;

/**
* Constructor
*
* @param \Magento\Framework\App\ResponseInterface $response
* @param \Magento\PageCache\Model\Config $config
* @param \Magento\PageCache\Model\Config $config
* @param \Magento\Framework\App\MaintenanceMode $maintenanceMode
*/
public function __construct(
\Magento\Framework\App\ResponseInterface $response,
\Magento\PageCache\Model\Config $config
\Magento\PageCache\Model\Config $config,
\Magento\Framework\App\MaintenanceMode $maintenanceMode
) {
$this->response = $response;
$this->config = $config;
$this->maintenanceMode = $maintenanceMode;
}

/**
* Set appropriate Cache-Control headers
*
* We have to set public headers in order to tell Varnish and Builtin app that page should be cached
*
* @param \Magento\Framework\View\Layout $subject
Expand All @@ -44,7 +53,7 @@ public function __construct(
*/
public function afterGenerateXml(\Magento\Framework\View\Layout $subject, $result)
{
if ($subject->isCacheable() && $this->config->isEnabled()) {
if ($subject->isCacheable() && !$this->maintenanceMode->isOn() && $this->config->isEnabled()) {
$this->response->setPublicHeaders($this->config->getTtl());
}
return $result;
Expand All @@ -68,6 +77,7 @@ public function afterGetOutput(\Magento\Framework\View\Layout $subject, $result)
if ($isVarnish && $isEsiBlock) {
continue;
}
// phpcs:ignore
$tags = array_merge($tags, $block->getIdentities());
}
}
Expand Down
108 changes: 0 additions & 108 deletions app/code/Magento/PageCache/Observer/SwitchPageCacheOnMaintenance.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

/**
* Page Cache state.
* @deprecated Originally used by now removed observer SwitchPageCacheOnMaintenance
*/
class PageCacheState
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class LayoutPluginTest extends \PHPUnit\Framework\TestCase
*/
protected $configMock;

/**
* @var \Magento\Framework\App\MaintenanceMode|\PHPUnit\Framework\MockObject\MockObject
*/
private $maintenanceModeMock;

protected function setUp()
{
$this->layoutMock = $this->getMockForAbstractClass(
Expand All @@ -40,27 +45,33 @@ protected function setUp()
);
$this->responseMock = $this->createMock(\Magento\Framework\App\Response\Http::class);
$this->configMock = $this->createMock(\Magento\PageCache\Model\Config::class);
$this->maintenanceModeMock = $this->createMock(\Magento\Framework\App\MaintenanceMode::class);

$this->model = new \Magento\PageCache\Model\Layout\LayoutPlugin(
$this->responseMock,
$this->configMock
$this->configMock,
$this->maintenanceModeMock
);
}

/**
* @param $cacheState
* @param $layoutIsCacheable
* @param $maintenanceModeIsEnabled
*
* @dataProvider afterGenerateXmlDataProvider
*/
public function testAfterGenerateXml($cacheState, $layoutIsCacheable)
public function testAfterGenerateXml($cacheState, $layoutIsCacheable, $maintenanceModeIsEnabled)
{
$maxAge = 180;
$result = 'test';

$this->layoutMock->expects($this->once())->method('isCacheable')->will($this->returnValue($layoutIsCacheable));
$this->configMock->expects($this->any())->method('isEnabled')->will($this->returnValue($cacheState));
$this->maintenanceModeMock->expects($this->any())->method('isOn')
->will($this->returnValue($maintenanceModeIsEnabled));

if ($layoutIsCacheable && $cacheState) {
if ($layoutIsCacheable && $cacheState && !$maintenanceModeIsEnabled) {
$this->configMock->expects($this->once())->method('getTtl')->will($this->returnValue($maxAge));
$this->responseMock->expects($this->once())->method('setPublicHeaders')->with($maxAge);
} else {
Expand All @@ -76,10 +87,11 @@ public function testAfterGenerateXml($cacheState, $layoutIsCacheable)
public function afterGenerateXmlDataProvider()
{
return [
'Full_cache state is true, Layout is cache-able' => [true, true],
'Full_cache state is true, Layout is not cache-able' => [true, false],
'Full_cache state is false, Layout is not cache-able' => [false, false],
'Full_cache state is false, Layout is cache-able' => [false, true]
'Full_cache state is true, Layout is cache-able' => [true, true, false],
'Full_cache state is true, Layout is not cache-able' => [true, false, false],
'Full_cache state is false, Layout is not cache-able' => [false, false, false],
'Full_cache state is false, Layout is cache-able' => [false, true, false],
'Full_cache state is true, Layout is cache-able, Maintenance mode is enabled' => [true, true, true],
];
}

Expand Down

This file was deleted.

Loading

0 comments on commit acd42b8

Please sign in to comment.