diff --git a/src/HttpCache/VarnishPurger.php b/src/HttpCache/VarnishPurger.php index 33b7d225fb7..6daa217bef4 100644 --- a/src/HttpCache/VarnishPurger.php +++ b/src/HttpCache/VarnishPurger.php @@ -25,6 +25,7 @@ final class VarnishPurger implements PurgerInterface { private const DEFAULT_VARNISH_MAX_HEADER_LENGTH = 8000; + private const REGEXP_PATTERN = '(%s)($|\,)'; private $clients; private $maxHeaderLength; @@ -35,7 +36,7 @@ final class VarnishPurger implements PurgerInterface public function __construct(array $clients, int $maxHeaderLength = self::DEFAULT_VARNISH_MAX_HEADER_LENGTH) { $this->clients = $clients; - $this->maxHeaderLength = $maxHeaderLength; + $this->maxHeaderLength = $maxHeaderLength - mb_strlen(self::REGEXP_PATTERN) + 2; // 2 for %s } /** @@ -60,8 +61,9 @@ private function determineTagsPerHeader(array $escapedTags, string $glue): int * header length by the largest tag (minus the glue length) */ $tagsize = max(array_map('mb_strlen', $escapedTags)); + $gluesize = \strlen($glue); - return (int) floor($this->maxHeaderLength / ($tagsize + \strlen($glue))) ?: 1; + return (int) floor(($this->maxHeaderLength + $gluesize) / ($tagsize + $gluesize)) ?: 1; } /** @@ -85,10 +87,12 @@ private function purgeRequest(array $iris) { // Create the regex to purge all tags in just one request $parts = array_map(static function ($iri) { - return sprintf('(^|\,)%s($|\,)', preg_quote($iri)); + // here we should remove the prefix as it's not discriminent and cost a lot to compute + return preg_quote($iri); }, $iris); foreach ($this->chunkRegexParts($parts) as $regex) { + $regex = sprintf(self::REGEXP_PATTERN, $regex); $this->banRegex($regex); } } @@ -108,7 +112,7 @@ private function chunkRegexParts(array $parts): iterable return; } - $concatenatedParts = sprintf('(%s)', implode(")\n(", $parts)); + $concatenatedParts = implode("\n", $parts); if (\strlen($concatenatedParts) <= $this->maxHeaderLength) { yield str_replace("\n", '|', $concatenatedParts); diff --git a/tests/HttpCache/VarnishPurgerTest.php b/tests/HttpCache/VarnishPurgerTest.php index 4024bdfba96..d4827876f1d 100644 --- a/tests/HttpCache/VarnishPurgerTest.php +++ b/tests/HttpCache/VarnishPurgerTest.php @@ -33,26 +33,26 @@ class VarnishPurgerTest extends TestCase public function testPurge() { $clientProphecy1 = $this->prophesize(ClientInterface::class); - $clientProphecy1->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(^|\,)/foo($|\,)']])->willReturn(new Response())->shouldBeCalled(); - $clientProphecy1->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '((^|\,)/foo($|\,))|((^|\,)/bar($|\,))']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy1->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/foo)($|\,)']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy1->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/foo|/bar)($|\,)']])->willReturn(new Response())->shouldBeCalled(); $clientProphecy2 = $this->prophesize(ClientInterface::class); - $clientProphecy2->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(^|\,)/foo($|\,)']])->willReturn(new Response())->shouldBeCalled(); - $clientProphecy2->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '((^|\,)/foo($|\,))|((^|\,)/bar($|\,))']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy2->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/foo)($|\,)']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy2->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/foo|/bar)($|\,)']])->willReturn(new Response())->shouldBeCalled(); $clientProphecy3 = $this->prophesize(ClientInterface::class); - $clientProphecy3->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(^|\,)/foo($|\,)']])->willReturn(new Response())->shouldBeCalled(); - $clientProphecy3->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(^|\,)/bar($|\,)']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy3->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/foo)($|\,)']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy3->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/bar)($|\,)']])->willReturn(new Response())->shouldBeCalled(); $clientProphecy4 = $this->prophesize(ClientInterface::class); - $clientProphecy4->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(^|\,)/foo($|\,)']])->willReturn(new Response())->shouldBeCalled(); - $clientProphecy4->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(^|\,)/bar($|\,)']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy4->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/foo)($|\,)']])->willReturn(new Response())->shouldBeCalled(); + $clientProphecy4->request('BAN', '', ['headers' => ['ApiPlatform-Ban-Regex' => '(/bar)($|\,)']])->willReturn(new Response())->shouldBeCalled(); $purger = new VarnishPurger([$clientProphecy1->reveal(), $clientProphecy2->reveal()]); $purger->purge(['/foo']); $purger->purge(['/foo' => '/foo', '/bar' => '/bar']); - $purger = new VarnishPurger([$clientProphecy3->reveal(), $clientProphecy4->reveal()], 5); + $purger = new VarnishPurger([$clientProphecy3->reveal(), $clientProphecy4->reveal()], 12); $purger->purge(['/foo' => '/foo', '/bar' => '/bar']); } @@ -118,58 +118,57 @@ public function provideChunkHeaderCases() yield 'one iri' => [ 50, ['/foo'], - ['(^|\,)/foo($|\,)'], + ['(/foo)($|\,)'], ]; yield 'few iris' => [ 50, ['/foo', '/bar'], - ['((^|\,)/foo($|\,))|((^|\,)/bar($|\,))'], + ['(/foo|/bar)($|\,)'], ]; yield 'iris to generate a header with exactly the maximum length' => [ - 56, + 22, ['/foo', '/bar', '/baz'], - ['((^|\,)/foo($|\,))|((^|\,)/bar($|\,))|((^|\,)/baz($|\,))'], + ['(/foo|/bar|/baz)($|\,)'], ]; yield 'iris to generate a header with exactly the maximum length and a smaller one' => [ - 37, + 17, ['/foo', '/bar', '/baz'], [ - '((^|\,)/foo($|\,))|((^|\,)/bar($|\,))', - '(^|\,)/baz($|\,)', + '(/foo|/bar)($|\,)', + '(/baz)($|\,)', ], ]; yield 'with last iri too long to be part of the same header' => [ - 50, + 35, ['/foo', '/bar', '/some-longer-tag'], [ - '((^|\,)/foo($|\,))|((^|\,)/bar($|\,))', - '(^|\,)/some\-longer\-tag($|\,)', + '(/foo|/bar)($|\,)', + '(/some\-longer\-tag)($|\,)', ], ]; yield 'iris to have five headers' => [ - 50, + 25, ['/foo/1', '/foo/2', '/foo/3', '/foo/4', '/foo/5', '/foo/6', '/foo/7', '/foo/8', '/foo/9', '/foo/10'], [ - '((^|\,)/foo/1($|\,))|((^|\,)/foo/2($|\,))', - '((^|\,)/foo/3($|\,))|((^|\,)/foo/4($|\,))', - '((^|\,)/foo/5($|\,))|((^|\,)/foo/6($|\,))', - '((^|\,)/foo/7($|\,))|((^|\,)/foo/8($|\,))', - '((^|\,)/foo/9($|\,))|((^|\,)/foo/10($|\,))', + '(/foo/1|/foo/2)($|\,)', + '(/foo/3|/foo/4)($|\,)', + '(/foo/5|/foo/6)($|\,)', + '(/foo/7|/foo/8)($|\,)', + '(/foo/9|/foo/10)($|\,)', ], ]; yield 'with varnish default limit' => [ 8000, - array_fill(0, 1000, '/foo'), + array_fill(0, 3000, '/foo'), [ - implode('|', array_fill(0, 421, '((^|\,)/foo($|\,))')), - implode('|', array_fill(0, 421, '((^|\,)/foo($|\,))')), - implode('|', array_fill(0, 158, '((^|\,)/foo($|\,))')), + sprintf('(%s)($|\,)', implode('|', array_fill(0, 1598, '/foo'))), + sprintf('(%s)($|\,)', implode('|', array_fill(0, 1402, '/foo'))), ], ]; }