Improve testing coverage

ezsystems · Feb 7, 2020 · 7708fd5 · 7708fd5
1 parent 93e49d5
commit 7708fd5
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 11 deletions.
diff --git a/spec/Handler/TagHandlerSpec.php b/spec/Handler/TagHandlerSpec.php
@@ -11,6 +11,7 @@
 use FOS\HttpCacheBundle\CacheManager;
 use PhpSpec\ObjectBehavior;
 use Prophecy\Argument;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\ResponseHeaderBag;
 
@@ -21,12 +22,13 @@ public function let(
         PurgeClientInterface $purgeClient,
         Response $response,
         ResponseHeaderBag $responseHeaderBag,
-        RepositoryTagPrefix $tagPrefix
+        RepositoryTagPrefix $tagPrefix,
+        LoggerInterface $logger
     ) {
         $response->headers = $responseHeaderBag;
         $cacheManager->supports(CacheManager::INVALIDATE)->willReturn(true);
 
-        $this->beConstructedWith($cacheManager, 'xkey', $purgeClient, $tagPrefix, 1000);
+        $this->beConstructedWith($cacheManager, 'xkey', $purgeClient, $tagPrefix, $logger, 1000, 300);
     }
 
     public function it_calls_purge_on_invalidate()
@@ -117,23 +119,48 @@ public function it_tags_all_tags_we_add_and_prefix_with_repo_id_also_with_existi
         $this->tagResponse($response, false);
     }
 
-    public function it_ignores_too_long_tag_header(Response $response, ResponseHeaderBag $responseHeaderBag, RepositoryTagPrefix $tagPrefix)
+    public function it_ignores_too_long_tag_header(Response $response, ResponseHeaderBag $responseHeaderBag, LoggerInterface $logger)
     {
         $underLimitTags = 'ez-all';
         $length = 6;
         while(true) {
-            $tag = ' content-' . $length;
+            $tag = ' c' . $length;
             $tagLength = strlen($tag);
             if ($length + $tagLength  > 1000) {
                 break; // too long if we add more
             }
             $underLimitTags .= $tag;
             $length += $tagLength;
         }
+        $responseHeaderBag->getCacheControlDirective('s-maxage')->shouldBeCalled()->willReturn(200);
+        $response->setSharedMaxAge(300)->shouldNotBeCalled();
+        $logger->warning(Argument::containingString('HTTP Cache tags header max length reached and truncated to'))->shouldBeCalled();
         $responseHeaderBag->set('xkey', Argument::exact($underLimitTags))->shouldBeCalled();
 
         $this->addTags(explode(' ', $underLimitTags));
-        $this->addTags(['location-1111111', 'content-1111111']); // these tags are ignored
+        $this->addTags(['l1111111', 'c1111111']); // these tags are ignored
+        $this->tagResponse($response, true);
+    }
+
+    public function it_ignores_too_long_tag_header_and_reduces_ttl(Response $response, ResponseHeaderBag $responseHeaderBag)
+    {
+        $underLimitTags = 'ez-all';
+        $length = 6;
+        while(true) {
+            $tag = ' c' . $length;
+            $tagLength = strlen($tag);
+            if ($length + $tagLength  > 1000) {
+                break; // too long if we add more
+            }
+            $underLimitTags .= $tag;
+            $length += $tagLength;
+        }
+        $responseHeaderBag->getCacheControlDirective('s-maxage')->shouldBeCalled()->willReturn(500);
+        $response->setSharedMaxAge(300)->shouldBeCalled();
+        $responseHeaderBag->set('xkey', Argument::exact($underLimitTags))->shouldBeCalled();
+
+        $this->addTags(explode(' ', $underLimitTags));
+        $this->addTags(['l1111111', 'c1111111']); // these tags are ignored
         $this->tagResponse($response, true);
     }
 }
diff --git a/src/Handler/TagHandler.php b/src/Handler/TagHandler.php
@@ -26,12 +26,12 @@ class TagHandler extends FOSTagHandler implements ContentTagInterface
     private $purgeClient;
     private $prefixService;
     private $tagsHeader;
+    /** @var LoggerInterface */
+    private $logger;
     /** @var int|null */
     private $tagsHeaderMaxLength;
     /** @var int|null */
     private $tagsHeaderReducedTTl;
-    /** @var LoggerInterface */
-    private $logger;
 
     public function __construct(
         CacheManager $cacheManager,
@@ -106,10 +106,14 @@ static function ($tag) use ($repoPrefix) {
             $tags[] = 'ez-all';
 
             $tagsString = implode(' ', array_unique($tags));
-            if ($this->tagsHeaderMaxLength && strlen($tagsString) > $this->tagsHeaderMaxLength) {
-                $tagsString = trim(substr($tagsString, 0, strrpos(
-                    substr($tagsString, 0, $this->tagsHeaderMaxLength + 1), ' '
-                )));
+            $tagsLength = strlen($tagsString);
+            if ($this->tagsHeaderMaxLength && $tagsLength > $this->tagsHeaderMaxLength) {
+                $tagsString = substr(
+                    $tagsString,
+                    0,
+                    // Seek backwards from point of max length using negative offset
+                    strrpos($tagsString, ' ', $this->tagsHeaderMaxLength - $tagsLength)
+                );
 
                 $responseSharedMaxAge = $response->headers->getCacheControlDirective('s-maxage');
                 if (