Skip to content

Commit b06b839

Browse files
magento-engcom-teammagento-engcom-team
authored
🔃 [Magento Community Engineering] Community Contributions - 2.4-develop latest changes
Accepted Community Pull Requests: - #29451: 💬 fix typo in php doc (by @brosenberger) - #29305: ScopeConfigInterface can be more than a string (by @fooman) - #29272: Unit Test / Deprecate / ObjectManager Helper (by @lbajsarowicz) - #28238: Prefix dropdown in checkout doesn't select empty value by default if required (by @engcom-Charlie) - #28480: Fix duplicate css when enable critical path (by @mrtuvn) - #26256: GH-26255: Refactor CacheInvalidate sendPurgeRequest to fix incorrect tag splitting (by @moloughlin) Fixed GitHub Issues: - #29470: [Issue] 💬 fix typo in php doc (reported by @m2-assistant[bot]) has been fixed in #29451 by @brosenberger in 2.4-develop branch Related commits: 1. 2f2ceb6 2. 200484d 3. 2841c9c - #29345: [Issue] ScopeConfigInterface can be more than a string (reported by @m2-assistant[bot]) has been fixed in #29305 by @fooman in 2.4-develop branch Related commits: 1. 8bb3b5e 2. 4cc85f3 - #29346: [Issue] Unit Test / Deprecate / ObjectManager Helper (reported by @m2-assistant[bot]) has been fixed in #29272 by @lbajsarowicz in 2.4-develop branch Related commits: 1. 03198de 2. d0218e1 3. 019646c - #18823: Prefix dropdown in checkout doesn't select empty value by default if required (reported by @DanieliMi) has been fixed in #28238 by @engcom-Charlie in 2.4-develop branch Related commits: 1. 7aa9a9c 2. daecbb4 3. f208eae 4. e5be53f 5. 0213d58 6. e322c58 - #26498: CSS shows up twice when Critical CSS Enabled (reported by @brightonmike) has been fixed in #28480 by @mrtuvn in 2.4-develop branch Related commits: 1. bacb817 - #8815: Varnish "Connection reset by peer" error when large catalog is reindexed on schedule (reported by @erikhansen) has been fixed in #26256 by @moloughlin in 2.4-develop branch Related commits: 1. e212301 2. 4c22303 3. fc2ce73 4. a9d376d 5. 5e69e82 6. 479c0cc 7. d0ef6ae 8. 85b3ffd 9. 116c191 10. 0926f5c 11. ffb1174 12. 99b26a4 13. a19274e - #26255: Magento_CacheInvalidate mis-handles very large tag patterns when doing a PURGE (reported by @moloughlin) has been fixed in #26256 by @moloughlin in 2.4-develop branch Related commits: 1. e212301 2. 4c22303 3. fc2ce73 4. a9d376d 5. 5e69e82 6. 479c0cc 7. d0ef6ae 8. 85b3ffd 9. 116c191 10. 0926f5c 11. ffb1174 12. 99b26a4 13. a19274e
2 parents cc6286a + 4c85ce7 commit b06b839

File tree

18 files changed

+484
-144
lines changed

18 files changed

+484
-144
lines changed

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

Lines changed: 36 additions & 26 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 = [];
@@ -145,14 +155,14 @@ private function sendPurgeRequestToServers($socketAdapter, $servers, $formattedT
145155
if ($errorCount == count($servers)) {
146156
$this->logger->critical(
147157
'No cache server(s) could be purged ' . $loggerMessage,
148-
compact('server', 'formattedTagsChunk')
158+
compact('servers', 'formattedTagsChunk')
149159
);
150160
return false;
151161
}
152162

153163
$this->logger->warning(
154164
'Unresponsive cache server(s) hit' . $loggerMessage,
155-
compact('server', 'formattedTagsChunk')
165+
compact('servers', 'formattedTagsChunk')
156166
);
157167
}
158168

‎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(

‎app/code/Magento/Customer/Model/Options.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,15 @@ private function prepareNamePrefixSuffixOptions($options, $isOptional = false)
9797
if (empty($options)) {
9898
return false;
9999
}
100+
100101
$result = [];
101-
$options = array_filter(explode(';', $options));
102+
$options = explode(';', $options);
102103
foreach ($options as $value) {
103-
$value = $this->escaper->escapeHtml(trim($value));
104-
$result[$value] = $value;
104+
$result[] = $this->escaper->escapeHtml(trim($value)) ?: ' ';
105105
}
106+
106107
if ($isOptional && trim(current($options))) {
107-
$result = array_merge([' ' => ' '], $result);
108+
$result = array_merge([' '], $result);
108109
}
109110

110111
return $result;

‎app/code/Magento/Downloadable/Block/Sales/Order/Email/Items/Downloadable.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
use Magento\Store\Model\ScopeInterface;
1313

1414
/**
15-
* Downlaodable Sales Order Email items renderer
15+
* Downloadable Sales Order Email items renderer
1616
*
1717
* @api
1818
* @since 100.0.2
@@ -81,6 +81,8 @@ public function getLinks()
8181
}
8282

8383
/**
84+
* Returns links title
85+
*
8486
* @return null|string
8587
*/
8688
public function getLinksTitle()
@@ -92,6 +94,8 @@ public function getLinksTitle()
9294
}
9395

9496
/**
97+
* Returns purchased link url
98+
*
9599
* @param Item $item
96100
* @return string
97101
*/

0 commit comments

Comments
 (0)