GH-26255: Refactor CacheInvalidate sendPurgeRequest to allow correct …

…purge batching, regardless of the size and content of provided tag pattern.
magento · Jan 3, 2020 · e212301 · e212301
1 parent 3e23510
commit e212301
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 19 deletions.
diff --git a/app/code/Magento/CacheInvalidate/Model/PurgeCache.php b/app/code/Magento/CacheInvalidate/Model/PurgeCache.php
@@ -39,39 +39,46 @@ class PurgeCache
      *
      * @var int
      */
-    private $requestSize = 7680;
+    private $requestSize;
 
     /**
      * Constructor
      *
      * @param \Magento\PageCache\Model\Cache\Server $cacheServer
      * @param \Magento\CacheInvalidate\Model\SocketFactory $socketAdapterFactory
      * @param InvalidateLogger $logger
+     * @param int $maxHeaderSize
      */
     public function __construct(
         \Magento\PageCache\Model\Cache\Server $cacheServer,
         \Magento\CacheInvalidate\Model\SocketFactory $socketAdapterFactory,
-        InvalidateLogger $logger
+        InvalidateLogger $logger,
+        $maxHeaderSize = 7680
     ) {
         $this->cacheServer = $cacheServer;
         $this->socketAdapterFactory = $socketAdapterFactory;
         $this->logger = $logger;
+        $this->requestSize = $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 +89,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($tags)
     {
-        $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->requestSize) {
                 yield implode('|', $formattedTagsChunk);
                 $formattedTagsChunk = [];
-                $tagsBatchSize = 0;
+                $currentBatchSize = 0;
             }
 
-            $tagsBatchSize += strlen($formattedTag);
+            $currentBatchSize += strlen($formattedTag);
             $formattedTagsChunk[] = $formattedTag;
         }
         if (!empty($formattedTagsChunk)) {

diff --git a/app/code/Magento/CacheInvalidate/Observer/FlushAllCacheObserver.php b/app/code/Magento/CacheInvalidate/Observer/FlushAllCacheObserver.php
@@ -43,7 +43,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(['.*']);
         }
     }
 }
diff --git a/app/code/Magento/CacheInvalidate/Observer/InvalidateVarnishObserver.php b/app/code/Magento/CacheInvalidate/Observer/InvalidateVarnishObserver.php
@@ -62,7 +62,7 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                 $tags[] = sprintf($pattern, $tag);
             }
             if (!empty($tags)) {
-                $this->purgeCache->sendPurgeRequest(implode('|', array_unique($tags)));
+                $this->purgeCache->sendPurgeRequest(array_unique($tags));
             }
         }
     }

diff --git a/app/code/Magento/CacheInvalidate/Test/Unit/Model/PurgeCacheTest.php b/app/code/Magento/CacheInvalidate/Test/Unit/Model/PurgeCacheTest.php
@@ -53,6 +53,7 @@ protected function setUp()
     public function testSendPurgeRequest($hosts)
     {
         $uris = [];
+        /** @var array $host */
         foreach ($hosts as $host) {
             $port = isset($host['port']) ? $host['port'] : \Magento\PageCache\Model\Cache\Server::DEFAULT_PORT;
             $uris[] = UriFactory::factory('')->setHost($host['host'])
@@ -81,7 +82,7 @@ public function testSendPurgeRequest($hosts)
         $this->loggerMock->expects($this->once())
             ->method('execute');
 
-        $this->assertTrue($this->model->sendPurgeRequest('tags'));
+        $this->assertTrue($this->model->sendPurgeRequest(['tags']));
     }
 
     /**
@@ -119,6 +120,6 @@ public function testSendPurgeRequestWithException()
         $this->loggerMock->expects($this->once())
             ->method('critical');
 
-        $this->assertFalse($this->model->sendPurgeRequest('tags'));
+        $this->assertFalse($this->model->sendPurgeRequest(['tags']));
     }
 }
diff --git a/app/code/Magento/CacheInvalidate/Test/Unit/Observer/FlushAllCacheObserverTest.php b/app/code/Magento/CacheInvalidate/Test/Unit/Observer/FlushAllCacheObserverTest.php
@@ -47,7 +47,7 @@ public function testFlushAllCache()
             $this->returnValue(\Magento\PageCache\Model\Config::VARNISH)
         );
 
-        $this->purgeCache->expects($this->once())->method('sendPurgeRequest')->with('.*');
+        $this->purgeCache->expects($this->once())->method('sendPurgeRequest')->with(['.*']);
         $this->model->execute($this->observerMock);
     }
 }