Skip to content

Commit 2d548ac

Browse files
slavvkaslavvka
authored
ENGCOM-7073: GH-26255: Refactor CacheInvalidate sendPurgeRequest to fix incorrect tag splitting #26256 
2 parents cf744a6 + 22c0827 commit 2d548ac

File tree

6 files changed

+88
-30
lines changed

6 files changed

+88
-30
lines changed

‎app/code/Magento/CacheInvalidate/Model/PurgeCache.php‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,24 @@
66
namespace Magento\CacheInvalidate\Model;
77

88
use Magento\Framework\Cache\InvalidateLogger;
9+
use Magento\PageCache\Model\Cache\Server;
10+
use Laminas\Http\Client\Adapter\Socket;
11+
use Laminas\Uri\Uri;
912

1013
/**
11-
* PurgeCache model
14+
* Invalidate external HTTP cache(s) based on tag pattern
1215
*/
1316
class PurgeCache
1417
{
1518
const HEADER_X_MAGENTO_TAGS_PATTERN = 'X-Magento-Tags-Pattern';
1619

1720
/**
18-
* @var \Magento\PageCache\Model\Cache\Server
21+
* @var Server
1922
*/
2023
protected $cacheServer;
2124

2225
/**
23-
* @var \Magento\CacheInvalidate\Model\SocketFactory
26+
* @var SocketFactory
2427
*/
2528
protected $socketAdapterFactory;
2629

@@ -39,39 +42,46 @@ class PurgeCache
3942
*
4043
* @var int
4144
*/
42-
private $requestSize = 7680;
45+
private $maxHeaderSize;
4346

4447
/**
4548
* Constructor
4649
*
47-
* @param \Magento\PageCache\Model\Cache\Server $cacheServer
48-
* @param \Magento\CacheInvalidate\Model\SocketFactory $socketAdapterFactory
50+
* @param Server $cacheServer
51+
* @param SocketFactory $socketAdapterFactory
4952
* @param InvalidateLogger $logger
53+
* @param int $maxHeaderSize
5054
*/
5155
public function __construct(
52-
\Magento\PageCache\Model\Cache\Server $cacheServer,
53-
\Magento\CacheInvalidate\Model\SocketFactory $socketAdapterFactory,
54-
InvalidateLogger $logger
56+
Server $cacheServer,
57+
SocketFactory $socketAdapterFactory,
58+
InvalidateLogger $logger,
59+
int $maxHeaderSize = 7680
5560
) {
5661
$this->cacheServer = $cacheServer;
5762
$this->socketAdapterFactory = $socketAdapterFactory;
5863
$this->logger = $logger;
64+
$this->maxHeaderSize = $maxHeaderSize;
5965
}
6066

6167
/**
6268
* Send curl purge request to invalidate cache by tags pattern
6369
*
64-
* @param string $tagsPattern
70+
* @param array|string $tags
6571
* @return bool Return true if successful; otherwise return false
6672
*/
67-
public function sendPurgeRequest($tagsPattern)
73+
public function sendPurgeRequest($tags)
6874
{
75+
if (is_string($tags)) {
76+
$tags = [$tags];
77+
}
78+
6979
$successful = true;
7080
$socketAdapter = $this->socketAdapterFactory->create();
7181
$servers = $this->cacheServer->getUris();
7282
$socketAdapter->setOptions(['timeout' => 10]);
7383

74-
$formattedTagsChunks = $this->splitTags($tagsPattern);
84+
$formattedTagsChunks = $this->chunkTags($tags);
7585
foreach ($formattedTagsChunks as $formattedTagsChunk) {
7686
if (!$this->sendPurgeRequestToServers($socketAdapter, $servers, $formattedTagsChunk)) {
7787
$successful = false;
@@ -82,24 +92,24 @@ public function sendPurgeRequest($tagsPattern)
8292
}
8393

8494
/**
85-
* Split tags by batches
95+
* Split tags into batches to suit Varnish max. header size
8696
*
87-
* @param string $tagsPattern
97+
* @param array $tags
8898
* @return \Generator
8999
*/
90-
private function splitTags($tagsPattern)
100+
private function chunkTags(array $tags): \Generator
91101
{
92-
$tagsBatchSize = 0;
102+
$currentBatchSize = 0;
93103
$formattedTagsChunk = [];
94-
$formattedTags = explode('|', $tagsPattern);
95-
foreach ($formattedTags as $formattedTag) {
96-
if ($tagsBatchSize + strlen($formattedTag) > $this->requestSize - count($formattedTagsChunk) - 1) {
104+
foreach ($tags as $formattedTag) {
105+
// Check if (currentBatchSize + length of next tag + number of pipe delimiters) would exceed header size.
106+
if ($currentBatchSize + strlen($formattedTag) + count($formattedTagsChunk) > $this->maxHeaderSize) {
97107
yield implode('|', $formattedTagsChunk);
98108
$formattedTagsChunk = [];
99-
$tagsBatchSize = 0;
109+
$currentBatchSize = 0;
100110
}
101111

102-
$tagsBatchSize += strlen($formattedTag);
112+
$currentBatchSize += strlen($formattedTag);
103113
$formattedTagsChunk[] = $formattedTag;
104114
}
105115
if (!empty($formattedTagsChunk)) {
@@ -110,12 +120,12 @@ private function splitTags($tagsPattern)
110120
/**
111121
* Send curl purge request to servers to invalidate cache by tags pattern
112122
*
113-
* @param \Laminas\Http\Client\Adapter\Socket $socketAdapter
114-
* @param \Laminas\Uri\Uri[] $servers
123+
* @param Socket $socketAdapter
124+
* @param Uri[] $servers
115125
* @param string $formattedTagsChunk
116126
* @return bool Return true if successful; otherwise return false
117127
*/
118-
private function sendPurgeRequestToServers($socketAdapter, $servers, $formattedTagsChunk)
128+
private function sendPurgeRequestToServers(Socket $socketAdapter, array $servers, string $formattedTagsChunk): bool
119129
{
120130
$headers = [self::HEADER_X_MAGENTO_TAGS_PATTERN => $formattedTagsChunk];
121131
$unresponsiveServerError = [];

‎app/code/Magento/CacheInvalidate/Observer/FlushAllCacheObserver.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
use Magento\Framework\Event\ObserverInterface;
99

10+
/**
11+
* Clear configured Varnish hosts when triggering a full cache flush (e.g. from the Cache Management admin dashboard)
12+
*/
1013
class FlushAllCacheObserver implements ObserverInterface
1114
{
1215
/**
@@ -43,7 +46,7 @@ public function __construct(
4346
public function execute(\Magento\Framework\Event\Observer $observer)
4447
{
4548
if ($this->config->getType() == \Magento\PageCache\Model\Config::VARNISH && $this->config->isEnabled()) {
46-
$this->purgeCache->sendPurgeRequest('.*');
49+
$this->purgeCache->sendPurgeRequest(['.*']);
4750
}
4851
}
4952
}

‎app/code/Magento/CacheInvalidate/Observer/InvalidateVarnishObserver.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public function execute(Observer $observer)
7777
$tags[] = sprintf($pattern, $tag);
7878
}
7979
if (!empty($tags)) {
80-
$this->purgeCache->sendPurgeRequest(implode('|', array_unique($tags)));
80+
$this->purgeCache->sendPurgeRequest(array_unique($tags));
8181
}
8282
}
8383
}

‎app/code/Magento/CacheInvalidate/Test/Unit/Model/PurgeCacheTest.php‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ protected function setUp(): void
5353
'cacheServer' => $this->cacheServer,
5454
'socketAdapterFactory' => $socketFactoryMock,
5555
'logger' => $this->loggerMock,
56+
'maxHeaderSize' => 256
5657
]
5758
);
5859
}
@@ -64,6 +65,7 @@ protected function setUp(): void
6465
public function testSendPurgeRequest($hosts)
6566
{
6667
$uris = [];
68+
/** @var array $host */
6769
foreach ($hosts as $host) {
6870
$port = isset($host['port']) ? $host['port'] : Server::DEFAULT_PORT;
6971
$uris[] = UriFactory::factory('')->setHost($host['host'])
@@ -92,7 +94,50 @@ public function testSendPurgeRequest($hosts)
9294
$this->loggerMock->expects($this->once())
9395
->method('execute');
9496

95-
$this->assertTrue($this->model->sendPurgeRequest('tags'));
97+
$this->assertTrue($this->model->sendPurgeRequest(['tags']));
98+
}
99+
100+
public function testSendMultiPurgeRequest()
101+
{
102+
$tags = [
103+
'(^|,)cat_p_95(,|$)', '(^|,)cat_p_96(,|$)', '(^|,)cat_p_97(,|$)', '(^|,)cat_p_98(,|$)',
104+
'(^|,)cat_p_99(,|$)', '(^|,)cat_p_100(,|$)', '(^|,)cat_p_10038(,|$)', '(^|,)cat_p_142985(,|$)',
105+
'(^|,)cat_p_199(,|$)', '(^|,)cat_p_300(,|$)', '(^|,)cat_p_12038(,|$)', '(^|,)cat_p_152985(,|$)',
106+
'(^|,)cat_p_299(,|$)', '(^|,)cat_p_400(,|$)', '(^|,)cat_p_13038(,|$)', '(^|,)cat_p_162985(,|$)',
107+
];
108+
109+
$tagsSplitA = array_slice($tags, 0, 12);
110+
$tagsSplitB = array_slice($tags, 12, 4);
111+
112+
$uri = UriFactory::factory('')->setHost('localhost')
113+
->setPort(80)
114+
->setScheme('http');
115+
116+
$this->cacheServer->expects($this->once())
117+
->method('getUris')
118+
->willReturn([$uri]);
119+
120+
$this->socketAdapterMock->expects($this->exactly(2))
121+
->method('connect')
122+
->with($uri->getHost(), $uri->getPort());
123+
124+
$this->socketAdapterMock->expects($this->exactly(2))
125+
->method('write')
126+
->withConsecutive(
127+
[
128+
'PURGE', $uri, '1.1',
129+
['X-Magento-Tags-Pattern' => implode('|', $tagsSplitA), 'Host' => $uri->getHost()]
130+
],
131+
[
132+
'PURGE', $uri, '1.1',
133+
['X-Magento-Tags-Pattern' => implode('|', $tagsSplitB), 'Host' => $uri->getHost()]
134+
]
135+
);
136+
137+
$this->socketAdapterMock->expects($this->exactly(2))
138+
->method('close');
139+
140+
$this->assertTrue($this->model->sendPurgeRequest($tags));
96141
}
97142

98143
/**
@@ -130,6 +175,6 @@ public function testSendPurgeRequestWithException()
130175
$this->loggerMock->expects($this->once())
131176
->method('critical');
132177

133-
$this->assertFalse($this->model->sendPurgeRequest('tags'));
178+
$this->assertFalse($this->model->sendPurgeRequest(['tags']));
134179
}
135180
}

‎app/code/Magento/CacheInvalidate/Test/Unit/Observer/FlushAllCacheObserverTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function testFlushAllCache()
5656
Config::VARNISH
5757
);
5858

59-
$this->purgeCache->expects($this->once())->method('sendPurgeRequest')->with('.*');
59+
$this->purgeCache->expects($this->once())->method('sendPurgeRequest')->with(['.*']);
6060
$this->model->execute($this->observerMock);
6161
}
6262
}

‎app/code/Magento/CacheInvalidate/Test/Unit/Observer/InvalidateVarnishObserverTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ protected function setUp(): void
8282
public function testInvalidateVarnish()
8383
{
8484
$tags = ['cache_1', 'cache_group'];
85-
$pattern = '((^|,)cache_1(,|$))|((^|,)cache_group(,|$))';
85+
$pattern = ['((^|,)cache_1(,|$))', '((^|,)cache_group(,|$))'];
8686

8787
$this->configMock->expects($this->once())->method('isEnabled')->willReturn(true);
8888
$this->configMock->expects(

0 commit comments

Comments
 (0)