ENGCOM-7073: GH-26255: Refactor CacheInvalidate sendPurgeRequest to f…

…ix incorrect tag splitting #26256

 - Merge Pull Request #26256 from moloughlin/magento2:fix/cache_invalidate_incorrect_tag_splitting
 - Merged 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

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

@@ -6,21 +6,24 @@
 namespace Magento\CacheInvalidate\Model;
 
 use Magento\Framework\Cache\InvalidateLogger;
+use Magento\PageCache\Model\Cache\Server;
+use Laminas\Http\Client\Adapter\Socket;
+use Laminas\Uri\Uri;
 
 /**
- * PurgeCache model
+ * Invalidate external HTTP cache(s) based on tag pattern
  */
 class PurgeCache
 {
     const HEADER_X_MAGENTO_TAGS_PATTERN = 'X-Magento-Tags-Pattern';
 
     /**
-     * @var \Magento\PageCache\Model\Cache\Server
+     * @var Server
      */
     protected $cacheServer;
 
     /**
-     * @var \Magento\CacheInvalidate\Model\SocketFactory
+     * @var SocketFactory
      */
     protected $socketAdapterFactory;
 
@@ -39,39 +42,46 @@ class PurgeCache
      *
      * @var int
      */
-    private $requestSize = 7680;
+    private $maxHeaderSize;
 
     /**
      * Constructor
      *
-     * @param \Magento\PageCache\Model\Cache\Server $cacheServer
-     * @param \Magento\CacheInvalidate\Model\SocketFactory $socketAdapterFactory
+     * @param Server $cacheServer
+     * @param SocketFactory $socketAdapterFactory
      * @param InvalidateLogger $logger
+     * @param int $maxHeaderSize
      */
     public function __construct(
-        \Magento\PageCache\Model\Cache\Server $cacheServer,
-        \Magento\CacheInvalidate\Model\SocketFactory $socketAdapterFactory,
-        InvalidateLogger $logger
+        Server $cacheServer,
+        SocketFactory $socketAdapterFactory,
+        InvalidateLogger $logger,
+        int $maxHeaderSize = 7680
     ) {
         $this->cacheServer = $cacheServer;
         $this->socketAdapterFactory = $socketAdapterFactory;
         $this->logger = $logger;
+        $this->maxHeaderSize = $maxHeaderSize;
     }
 
     /**
      * Send curl purge request to invalidate cache by tags pattern
      *
-     * @param string $tagsPattern
+     * @param array|string $tags
      * @return bool Return true if successful; otherwise return false
      */
-    public function sendPurgeRequest($tagsPattern)
+    public function sendPurgeRequest($tags)
     {
+        if (is_string($tags)) {
+            $tags = [$tags];
+        }
+
         $successful = true;
         $socketAdapter = $this->socketAdapterFactory->create();
         $servers = $this->cacheServer->getUris();
         $socketAdapter->setOptions(['timeout' => 10]);
 
-        $formattedTagsChunks = $this->splitTags($tagsPattern);
+        $formattedTagsChunks = $this->chunkTags($tags);
         foreach ($formattedTagsChunks as $formattedTagsChunk) {
             if (!$this->sendPurgeRequestToServers($socketAdapter, $servers, $formattedTagsChunk)) {
                 $successful = false;
@@ -82,24 +92,24 @@ public function sendPurgeRequest($tagsPattern)
     }
 
     /**
-     * Split tags by batches
+     * Split tags into batches to suit Varnish max. header size
      *
-     * @param string $tagsPattern
+     * @param array $tags
      * @return \Generator
      */
-    private function splitTags($tagsPattern)
+    private function chunkTags(array $tags): \Generator
     {
-        $tagsBatchSize = 0;
+        $currentBatchSize = 0;
         $formattedTagsChunk = [];
-        $formattedTags = explode('|', $tagsPattern);
-        foreach ($formattedTags as $formattedTag) {
-            if ($tagsBatchSize + strlen($formattedTag) > $this->requestSize - count($formattedTagsChunk) - 1) {
+        foreach ($tags as $formattedTag) {
+            // Check if (currentBatchSize + length of next tag + number of pipe delimiters) would exceed header size.
+            if ($currentBatchSize + strlen($formattedTag) + count($formattedTagsChunk) > $this->maxHeaderSize) {
                 yield implode('|', $formattedTagsChunk);
                 $formattedTagsChunk = [];
-                $tagsBatchSize = 0;
+                $currentBatchSize = 0;
             }
 
-            $tagsBatchSize += strlen($formattedTag);
+            $currentBatchSize += strlen($formattedTag);
             $formattedTagsChunk[] = $formattedTag;
         }
         if (!empty($formattedTagsChunk)) {
@@ -110,12 +120,12 @@ private function splitTags($tagsPattern)
     /**
      * Send curl purge request to servers to invalidate cache by tags pattern
      *
-     * @param \Laminas\Http\Client\Adapter\Socket $socketAdapter
-     * @param \Laminas\Uri\Uri[] $servers
+     * @param Socket $socketAdapter
+     * @param Uri[] $servers
      * @param string $formattedTagsChunk
      * @return bool Return true if successful; otherwise return false
      */
-    private function sendPurgeRequestToServers($socketAdapter, $servers, $formattedTagsChunk)
+    private function sendPurgeRequestToServers(Socket $socketAdapter, array $servers, string $formattedTagsChunk): bool
     {
         $headers = [self::HEADER_X_MAGENTO_TAGS_PATTERN => $formattedTagsChunk];
         $unresponsiveServerError = [];

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

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

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

@@ -77,7 +77,7 @@ public function execute(Observer $observer)
                 $tags[] = sprintf($pattern, $tag);
             }
             if (!empty($tags)) {
-                $this->purgeCache->sendPurgeRequest(implode('|', array_unique($tags)));
+                $this->purgeCache->sendPurgeRequest(array_unique($tags));
             }
         }
     }

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

@@ -53,6 +53,7 @@ protected function setUp(): void
                 'cacheServer' => $this->cacheServer,
                 'socketAdapterFactory' => $socketFactoryMock,
                 'logger' => $this->loggerMock,
+                'maxHeaderSize' => 256
             ]
         );
     }
@@ -64,6 +65,7 @@ protected function setUp(): void
     public function testSendPurgeRequest($hosts)
     {
         $uris = [];
+        /** @var array $host */
         foreach ($hosts as $host) {
             $port = isset($host['port']) ? $host['port'] : Server::DEFAULT_PORT;
             $uris[] = UriFactory::factory('')->setHost($host['host'])
@@ -92,7 +94,50 @@ public function testSendPurgeRequest($hosts)
         $this->loggerMock->expects($this->once())
             ->method('execute');
 
-        $this->assertTrue($this->model->sendPurgeRequest('tags'));
+        $this->assertTrue($this->model->sendPurgeRequest(['tags']));
+    }
+
+    public function testSendMultiPurgeRequest()
+    {
+        $tags = [
+            '(^|,)cat_p_95(,|$)', '(^|,)cat_p_96(,|$)', '(^|,)cat_p_97(,|$)', '(^|,)cat_p_98(,|$)',
+            '(^|,)cat_p_99(,|$)', '(^|,)cat_p_100(,|$)', '(^|,)cat_p_10038(,|$)', '(^|,)cat_p_142985(,|$)',
+            '(^|,)cat_p_199(,|$)', '(^|,)cat_p_300(,|$)', '(^|,)cat_p_12038(,|$)', '(^|,)cat_p_152985(,|$)',
+            '(^|,)cat_p_299(,|$)', '(^|,)cat_p_400(,|$)', '(^|,)cat_p_13038(,|$)', '(^|,)cat_p_162985(,|$)',
+        ];
+
+        $tagsSplitA = array_slice($tags, 0, 12);
+        $tagsSplitB = array_slice($tags, 12, 4);
+
+        $uri =  UriFactory::factory('')->setHost('localhost')
+            ->setPort(80)
+            ->setScheme('http');
+
+        $this->cacheServer->expects($this->once())
+            ->method('getUris')
+            ->willReturn([$uri]);
+
+        $this->socketAdapterMock->expects($this->exactly(2))
+            ->method('connect')
+            ->with($uri->getHost(), $uri->getPort());
+
+        $this->socketAdapterMock->expects($this->exactly(2))
+            ->method('write')
+            ->withConsecutive(
+                [
+                    'PURGE', $uri, '1.1',
+                    ['X-Magento-Tags-Pattern' => implode('|', $tagsSplitA), 'Host' => $uri->getHost()]
+                ],
+                [
+                    'PURGE', $uri, '1.1',
+                    ['X-Magento-Tags-Pattern' => implode('|', $tagsSplitB), 'Host' => $uri->getHost()]
+                ]
+            );
+
+        $this->socketAdapterMock->expects($this->exactly(2))
+            ->method('close');
+
+        $this->assertTrue($this->model->sendPurgeRequest($tags));
     }
 
     /**
@@ -130,6 +175,6 @@ public function testSendPurgeRequestWithException()
         $this->loggerMock->expects($this->once())
             ->method('critical');
 
-        $this->assertFalse($this->model->sendPurgeRequest('tags'));
+        $this->assertFalse($this->model->sendPurgeRequest(['tags']));
     }
 }

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

@@ -56,7 +56,7 @@ public function testFlushAllCache()
             Config::VARNISH
         );
 
-        $this->purgeCache->expects($this->once())->method('sendPurgeRequest')->with('.*');
+        $this->purgeCache->expects($this->once())->method('sendPurgeRequest')->with(['.*']);
         $this->model->execute($this->observerMock);
     }
 }

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

@@ -82,7 +82,7 @@ protected function setUp(): void
     public function testInvalidateVarnish()
     {
         $tags = ['cache_1', 'cache_group'];
-        $pattern = '((^|,)cache_1(,|$))|((^|,)cache_group(,|$))';
+        $pattern = ['((^|,)cache_1(,|$))', '((^|,)cache_group(,|$))'];
 
         $this->configMock->expects($this->once())->method('isEnabled')->willReturn(true);
         $this->configMock->expects(