Skip to content
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

Fix loss of page_cache cache_dir setting from di.xml #22228

Merged
merged 3 commits into from
Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions app/code/Magento/Catalog/Observer/FlushCategoryPagesCache.php
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]);
}
}
}
3 changes: 3 additions & 0 deletions app/code/Magento/Catalog/etc/adminhtml/events.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@
<event name="catalog_category_change_products">
<observer name="category_product_indexer" instance="Magento\Catalog\Observer\CategoryProductIndexer"/>
</event>
<event name="category_move">
<observer name="clean_cagegory_page_cache" instance="Magento\Catalog\Observer\FlushCategoryPagesCache" />
</event>
</config>
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
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));
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty Line missing

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larsroettig didn't get which line you talked about...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I changed the commit line 59 was missing.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@

namespace Magento\GraphQlCache\Controller;

use PHPUnit\Framework\TestCase;
use Magento\TestFramework\ObjectManager;
use Magento\Framework\App\Request\Http as HttpRequest;
use Magento\Framework\App\Response\HttpInterface as HttpResponse;
use Magento\Framework\Registry;
use Magento\GraphQl\Controller\GraphQl as GraphQlController;
use Magento\GraphQlCache\Model\CacheableQuery;
use Magento\PageCache\Model\Cache\Type as PageCache;
use Magento\TestFramework\Helper\Bootstrap;
use Magento\TestFramework\ObjectManager;
use PHPUnit\Framework\TestCase;

/**
* Abstract test class for Graphql cache tests
Expand All @@ -21,40 +27,114 @@ abstract class AbstractGraphqlCacheTest extends TestCase
*/
protected $objectManager;

/**
* @inheritdoc
*/
protected function setUp(): void
{
$this->objectManager = Bootstrap::getObjectManager();
$this->enablePageCachePlugin();
$this->enableCachebleQueryTestProxy();
}

protected function tearDown(): void
{
$this->disableCacheableQueryTestProxy();
$this->disablePageCachePlugin();
$this->flushPageCache();
}

protected function enablePageCachePlugin(): void
{
/** @var $registry Registry */
$registry = $this->objectManager->get(Registry::class);
$registry->register('use_page_cache_plugin', true, true);
}

protected function disablePageCachePlugin(): void
{
/** @var $registry Registry */
$registry = $this->objectManager->get(Registry::class);
$registry->unregister('use_page_cache_plugin');
}

protected function flushPageCache(): void
{
/** @var PageCache $fullPageCache */
$fullPageCache = $this->objectManager->get(PageCache::class);
$fullPageCache->clean();
}

/**
* Prepare a query and return a request to be used in the same test end to end
* Regarding the SuppressWarnings annotation below: the nested class below triggers a false rule match.
*
* @param string $query
* @return \Magento\Framework\App\Request\Http
* @SuppressWarnings(PHPMD.UnusedLocalVariable)
*/
protected function prepareRequest(string $query) : \Magento\Framework\App\Request\Http
{
$cacheableQuery = $this->objectManager->get(\Magento\GraphQlCache\Model\CacheableQuery::class);
$cacheableQueryReflection = new \ReflectionProperty(
$cacheableQuery,
'cacheTags'
);
$cacheableQueryReflection->setAccessible(true);
$cacheableQueryReflection->setValue($cacheableQuery, []);

/** @var \Magento\Framework\UrlInterface $urlInterface */
$urlInterface = $this->objectManager->create(\Magento\Framework\UrlInterface::class);
//set unique URL
$urlInterface->setQueryParam('query', $query);

$request = $this->objectManager->get(\Magento\Framework\App\Request\Http::class);
$request->setUri($urlInterface->getUrl('graphql'));
private function enableCachebleQueryTestProxy(): void
{
$cacheableQueryProxy = new class($this->objectManager) extends CacheableQuery {
/** @var CacheableQuery */
private $delegate;

public function __construct(ObjectManager $objectManager)
{
$this->reset($objectManager);
}

public function reset(ObjectManager $objectManager): void
{
$this->delegate = $objectManager->create(CacheableQuery::class);
}

public function getCacheTags(): array
{
return $this->delegate->getCacheTags();
}

public function addCacheTags(array $cacheTags): void
{
$this->delegate->addCacheTags($cacheTags);
}

public function isCacheable(): bool
{
return $this->delegate->isCacheable();
}

public function setCacheValidity(bool $cacheable): void
{
$this->delegate->setCacheValidity($cacheable);
}

public function shouldPopulateCacheHeadersWithTags(): bool
{
return $this->delegate->shouldPopulateCacheHeadersWithTags();
}
};
$this->objectManager->addSharedInstance($cacheableQueryProxy, CacheableQuery::class);
}

private function disableCacheableQueryTestProxy(): void
{
$this->resetQueryCacheTags();
$this->objectManager->removeSharedInstance(CacheableQuery::class);
}

protected function resetQueryCacheTags(): void
{
$this->objectManager->get(CacheableQuery::class)->reset($this->objectManager);
}

protected function dispatchGraphQlGETRequest(array $queryParams): HttpResponse
{
$this->resetQueryCacheTags();

/** @var HttpRequest $request */
$request = $this->objectManager->get(HttpRequest::class);
$request->setPathInfo('/graphql');
$request->setMethod('GET');
//set the actual GET query
$request->setQueryValue('query', $query);
return $request;
$request->setParams($queryParams);

// required for \Magento\Framework\App\PageCache\Identifier to generate the correct cache key
$request->setUri(implode('?', [$request->getPathInfo(), http_build_query($queryParams)]));

return $this->objectManager->create(GraphQlController::class)->dispatch($request);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

use Magento\Catalog\Api\Data\ProductInterface;
use Magento\Catalog\Api\ProductRepositoryInterface;
use Magento\Framework\App\Request\Http;
use Magento\GraphQl\Controller\GraphQl;
use Magento\GraphQlCache\Controller\AbstractGraphqlCacheTest;

/**
Expand All @@ -22,31 +20,12 @@
*/
class CategoriesWithProductsCacheTest extends AbstractGraphqlCacheTest
{
/**
* @var GraphQl
*/
private $graphqlController;

/**
* @var Http
*/
private $request;

/**
* @inheritdoc
*/
protected function setUp(): void
{
parent::setUp();
$this->graphqlController = $this->objectManager->get(\Magento\GraphQl\Controller\GraphQl::class);
$this->request = $this->objectManager->create(Http::class);
}
/**
* Test cache tags and debug header for category with products querying for products and category
*
* @magentoDataFixture Magento/Catalog/_files/category_product.php
*/
public function testToCheckRequestCacheTagsForCategoryWithProducts(): void
public function testRequestCacheTagsForCategoryWithProducts(): void
{
/** @var ProductRepositoryInterface $productRepository */
$productRepository = $this->objectManager->get(ProductRepositoryInterface::class);
Expand Down Expand Up @@ -91,17 +70,7 @@ public function testToCheckRequestCacheTagsForCategoryWithProducts(): void
'operationName' => 'GetCategoryWithProducts'
];

/** @var \Magento\Framework\UrlInterface $urlInterface */
$urlInterface = $this->objectManager->create(\Magento\Framework\UrlInterface::class);
//set unique URL
$urlInterface->setQueryParam('query', $queryParams['query']);
$urlInterface->setQueryParam('variables', $queryParams['variables']);
$urlInterface->setQueryParam('operationName', $queryParams['operationName']);
$this->request->setUri($urlInterface->getUrl('graphql'));
$this->request->setPathInfo('/graphql');
$this->request->setMethod('GET');
$this->request->setParams($queryParams);
$response = $this->graphqlController->dispatch($this->request);
$response = $this->dispatchGraphQlGETRequest($queryParams);
$this->assertEquals('MISS', $response->getHeader('X-Magento-Cache-Debug')->getFieldValue());
$expectedCacheTags = ['cat_c','cat_c_' . $categoryId,'cat_p','cat_p_' . $product->getId(),'FPC'];
$actualCacheTags = explode(',', $response->getHeader('X-Magento-Tags')->getFieldValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

namespace Magento\GraphQlCache\Controller\Catalog;

use Magento\Framework\App\Request\Http;
use Magento\GraphQl\Controller\GraphQl;
use Magento\GraphQlCache\Controller\AbstractGraphqlCacheTest;

/**
Expand All @@ -20,25 +18,12 @@
*/
class CategoryCacheTest extends AbstractGraphqlCacheTest
{
/**
* @var GraphQl
*/
private $graphqlController;

/**
* @inheritdoc
*/
protected function setUp(): void
{
parent::setUp();
$this->graphqlController = $this->objectManager->get(\Magento\GraphQl\Controller\GraphQl::class);
}
/**
* Test cache tags and debug header for category and querying only for category
*
* @magentoDataFixture Magento/Catalog/_files/category_product.php
*/
public function testToCheckRequestCacheTagsForForCategory(): void
public function testRequestCacheTagsForCategory(): void
{
$categoryId ='333';
$query
Expand All @@ -53,11 +38,10 @@ public function testToCheckRequestCacheTagsForForCategory(): void
}
}
QUERY;
$request = $this->prepareRequest($query);
$response = $this->graphqlController->dispatch($request);
$response = $this->dispatchGraphQlGETRequest(['query' => $query]);
$this->assertEquals('MISS', $response->getHeader('X-Magento-Cache-Debug')->getFieldValue());
$actualCacheTags = explode(',', $response->getHeader('X-Magento-Tags')->getFieldValue());
$expectedCacheTags = ['cat_c','cat_c_' . $categoryId,'FPC'];
$expectedCacheTags = ['cat_c','cat_c_' . $categoryId, 'FPC'];
$this->assertEquals($expectedCacheTags, $actualCacheTags);
}
}
Loading