-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix loss of page_cache cache_dir setting from di.xml #22228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
<?php declare(strict_types=1); | ||
/** | ||
* | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
|
||
namespace Magento\Catalog\Observer; | ||
|
||
use Magento\Catalog\Model\Category; | ||
use Magento\Framework\Event\Observer as Event; | ||
use Magento\Framework\Event\ObserverInterface; | ||
use Magento\PageCache\Model\Cache\Type as PageCache; | ||
use Magento\PageCache\Model\Config as CacheConfig; | ||
|
||
/** | ||
* Flush the built in page cache when a category is moved | ||
*/ | ||
class FlushCategoryPagesCache implements ObserverInterface | ||
{ | ||
|
||
/** | ||
* @var CacheConfig | ||
*/ | ||
private $cacheConfig; | ||
|
||
/** | ||
* | ||
* @var PageCache | ||
*/ | ||
private $pageCache; | ||
|
||
/** | ||
* FlushCategoryPagesCache constructor. | ||
* | ||
* @param CacheConfig $cacheConfig | ||
* @param PageCache $pageCache | ||
*/ | ||
public function __construct(CacheConfig $cacheConfig, PageCache $pageCache) | ||
{ | ||
$this->cacheConfig = $cacheConfig; | ||
$this->pageCache = $pageCache; | ||
} | ||
|
||
/** | ||
* Clean the category page cache if built in cache page cache is used. | ||
* | ||
* The built in cache requires cleaning all pages that contain the top category navigation menu when a | ||
* category is moved. This is because the built in cache does not support ESI blocks. | ||
* | ||
* @param Event $event | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function execute(Event $event) | ||
{ | ||
if ($this->cacheConfig->getType() == CacheConfig::BUILT_IN && $this->cacheConfig->isEnabled()) { | ||
$this->pageCache->clean(\Zend_Cache::CLEANING_MODE_MATCHING_ANY_TAG, [Category::CACHE_TAG]); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
<?php declare(strict_types=1); | ||
Vinai marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
|
||
namespace Magento\Framework\App\Cache\Frontend; | ||
|
||
use Magento\Framework\ObjectManager\ConfigInterface as ObjectManagerConfig; | ||
use Magento\TestFramework\ObjectManager; | ||
use PHPUnit\Framework\TestCase; | ||
|
||
Vinai marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/** | ||
* This superfluous comment can be removed as soon as the sniffs have been updated to match the coding guide lines. | ||
*/ | ||
class PoolTest extends TestCase | ||
{ | ||
public function testPageCacheNotSameAsDefaultCacheDirectory(): void | ||
{ | ||
/** @var ObjectManagerConfig $diConfig */ | ||
$diConfig = ObjectManager::getInstance()->get(ObjectManagerConfig::class); | ||
$argumentConfig = $diConfig->getArguments(\Magento\Framework\App\Cache\Frontend\Pool::class); | ||
|
||
$pageCacheDir = $argumentConfig['frontendSettings']['page_cache']['backend_options']['cache_dir'] ?? null; | ||
$defaultCacheDir = $argumentConfig['frontendSettings']['default']['backend_options']['cache_dir'] ?? null; | ||
|
||
$noPageCacheMessage = "No default page_cache directory set in di.xml: \n" . var_export($argumentConfig, true); | ||
$this->assertNotEmpty($pageCacheDir, $noPageCacheMessage); | ||
|
||
$sameCacheDirMessage = 'The page_cache and default cache storages share the same cache directory'; | ||
$this->assertNotSame($pageCacheDir, $defaultCacheDir, $sameCacheDirMessage); | ||
} | ||
|
||
/** | ||
* @covers \Magento\Framework\App\Cache\Frontend\Pool::_getCacheSettings | ||
* @depends testPageCacheNotSameAsDefaultCacheDirectory | ||
*/ | ||
public function testCleaningDefaultCachePreservesPageCache() | ||
{ | ||
$testData = 'test data'; | ||
$testKey = 'test-key'; | ||
|
||
/** @var \Magento\Framework\App\Cache\Frontend\Pool $cacheFrontendPool */ | ||
$cacheFrontendPool = ObjectManager::getInstance()->get(\Magento\Framework\App\Cache\Frontend\Pool::class); | ||
|
||
$pageCache = $cacheFrontendPool->get('page_cache'); | ||
$pageCache->save($testData, $testKey); | ||
|
||
$defaultCache = $cacheFrontendPool->get('default'); | ||
$defaultCache->clean(); | ||
|
||
$this->assertSame($testData, $pageCache->load($testKey)); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty Line missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. But I'm curious what is the reason for this rule? It seems very arbitrary and useless to me... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @larsroettig didn't get which line you talked about... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before I changed the commit line 59 was missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Vinai I don't think it's needed. Basically any missing line is not worth commenting - phpcs should catch all needed. |
Uh oh!
There was an error while loading. Please reload this page.