Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-30562: Make it possible to ignore tags over configurable max length #116

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions docs/using_tags.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,21 @@ code involved to see if amount of tags can be reduced.

#### Limit tags header output by system

Typical case with too many tags would be when inline rendering some form of embed content object.
Typical case with too many tags would be when inline rendering a lot of embed content object.
Normally the system will add all the tags for this content, to handle every possible scenario of updates to them.

So if you embed hundreds of content on the same page _(in richtext, using relations, or using page builder)_, it will explode the tag usage.
So if you embed hundreds of content on the same page _(i.e. in richtext, using relations, or using page builder)_, it will explode the tag usage.

However if for instance you just display the content name, image attribute, and/or link, then it would be enough to:
- Just use `r<id>` tag, or preferably the abstractions for it.
- Optionally: Set reduced cache TTL for the given view in order to reduce remote risk of subtree operations affecting the cached page
without correctly purging the view.

If that is not an option, you can opt-in to set a max length parameter (in bytes) and corresponding ttl (in seconds):
```yaml
parameters:
# Warning, setting this means you risk losing tag information, risking stale cache. Here set below 8k:
ezplatform.http_cache.tags.header_max_length: 7900
# In order to reduce risk of stale cache issues, you should set a lower TTL here then globally (here set as 2h)
ezplatform.http_cache.tags.header_reduced_ttl: 7200
```
57 changes: 53 additions & 4 deletions spec/Handler/TagHandlerSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -21,12 +22,15 @@ 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);
$this->beConstructedWith($cacheManager, 'xkey', $purgeClient, $tagPrefix, $logger, 1000, 300);

$tagPrefix->getRepositoryPrefix()->willReturn('');
}

public function it_calls_purge_on_invalidate()
Expand Down Expand Up @@ -98,7 +102,7 @@ public function it_tags_all_tags_we_add(Response $response, ResponseHeaderBag $r
public function it_tags_all_tags_we_add_and_prefix_with_repo_id(Response $response, ResponseHeaderBag $responseHeaderBag, RepositoryTagPrefix $tagPrefix)
{
$tagPrefix->getRepositoryPrefix()->willReturn('0');
$responseHeaderBag->set('xkey', Argument::exact('0ez-all 0l4 0c4 0p2 ez-all'))->shouldBeCalled();
$responseHeaderBag->set('xkey', Argument::exact('ez-all 0ez-all 0l4 0c4 0p2'))->shouldBeCalled();

$this->addTags(['l4', 'c4']);
$this->addTags(['p2']);
Expand All @@ -110,10 +114,55 @@ public function it_tags_all_tags_we_add_and_prefix_with_repo_id_also_with_existi
$tagPrefix->getRepositoryPrefix()->willReturn('2');
$responseHeaderBag->has('xkey')->willReturn(true);
$responseHeaderBag->get('xkey', null, false)->willReturn(['tag1']);
$responseHeaderBag->set('xkey', Argument::exact('2tag1 2ez-all 2l4 2c4 2p2 ez-all'))->shouldBeCalled();
$responseHeaderBag->set('xkey', Argument::exact('ez-all 2tag1 2ez-all 2l4 2c4 2p2'))->shouldBeCalled();

$this->addTags(['l4', 'c4']);
$this->addTags(['p2']);
$this->tagResponse($response, false);
}

public function it_ignores_too_long_tag_header(Response $response, ResponseHeaderBag $responseHeaderBag, LoggerInterface $logger)
{
$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(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(['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);
}
}
51 changes: 47 additions & 4 deletions src/Handler/TagHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use EzSystems\PlatformHttpCacheBundle\PurgeClient\PurgeClientInterface;
use EzSystems\PlatformHttpCacheBundle\RepositoryTagPrefix;
use FOS\HttpCacheBundle\Handler\TagHandler as FOSTagHandler;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Response;
use FOS\HttpCacheBundle\CacheManager;

Expand All @@ -26,15 +27,30 @@ class TagHandler extends FOSTagHandler implements ContentTagInterface
private $prefixService;
private $tagsHeader;

/** @var \Psr\Log\LoggerInterface */
private $logger;

/** @var int|null */
private $tagsHeaderMaxLength;
andrerom marked this conversation as resolved.
Show resolved Hide resolved

/** @var int|null */
private $tagsHeaderReducedTTl;

public function __construct(
CacheManager $cacheManager,
$tagsHeader,
PurgeClientInterface $purgeClient,
RepositoryTagPrefix $prefixService
RepositoryTagPrefix $prefixService,
LoggerInterface $logger,
$maxTagsHeaderLength = null,
$tagsHeaderReducedTTl = null
) {
$this->tagsHeader = $tagsHeader;
$this->purgeClient = $purgeClient;
$this->prefixService = $prefixService;
$this->logger = $logger;
$this->tagsHeaderMaxLength = $maxTagsHeaderLength;
$this->tagsHeaderReducedTTl = $tagsHeaderReducedTTl;

parent::__construct($cacheManager, $tagsHeader);
$this->addTags(['ez-all']);
Expand Down Expand Up @@ -88,10 +104,37 @@ static function ($tag) use ($repoPrefix) {
},
$tags
);
// Also add a un-prefixed `ez-all` in order to be able to purge all across repos
$tags[] = 'ez-all';

$response->headers->set($this->tagsHeader, implode(' ', array_unique($tags)));
if ($repoPrefix !== '') {
// An un-prefixed `ez-all` for purging across repos, add to start of array to avoid being truncated
array_unshift($tags, 'ez-all');
}

$tagsString = implode(' ', array_unique($tags));
$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 (
$this->tagsHeaderReducedTTl &&
$responseSharedMaxAge &&
$this->tagsHeaderReducedTTl < $responseSharedMaxAge
) {
$response->setSharedMaxAge($this->tagsHeaderReducedTTl);
}

$this->logger->warning(
'HTTP Cache tags header max length reached and truncated to ' . $this->tagsHeaderMaxLength
andrerom marked this conversation as resolved.
Show resolved Hide resolved
);
}

$response->headers->set($this->tagsHeader, $tagsString);
}

return $this;
Expand Down
5 changes: 5 additions & 0 deletions src/Resources/config/services.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
parameters:
ezplatform.http_cache.controller.invalidatetoken.class: EzSystems\PlatformHttpCacheBundle\Controller\InvalidateTokenController
ezplatform.http_cache.listener.vary_header.class: EzSystems\PlatformHttpCacheBundle\EventListener\ConditionallyRemoveVaryHeaderListener
ezplatform.http_cache.tags.header_max_length: null
ezplatform.http_cache.tags.header_reduced_ttl: null

services:
ezplatform.http_cache.cache_manager:
Expand Down Expand Up @@ -50,6 +52,9 @@ services:
- '%ezplatform.http_cache.tags.header%'
- '@ezplatform.http_cache.purge_client'
- '@ezplatform.http_cache.repository_tag_prefix'
- '@logger'
- '%ezplatform.http_cache.tags.header_max_length%'
- '%ezplatform.http_cache.tags.header_reduced_ttl%'

ezplatform.http_cache.user_context_provider.role_identify:
class: EzSystems\PlatformHttpCacheBundle\ContextProvider\RoleIdentify
Expand Down