Skip to content

Support cache tagging #2

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

Merged
merged 1 commit into from
Dec 30, 2013
Merged
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
89 changes: 76 additions & 13 deletions CacheManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
namespace FOS\HttpCacheBundle;

use FOS\HttpCacheBundle\HttpCache\HttpCacheInterface;
use FOS\HttpCacheBundle\Invalidation\CacheProxyInterface;
use FOS\HttpCacheBundle\Invalidation\Method\BanInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\RouterInterface;

/**
Expand All @@ -11,6 +14,11 @@
*/
class CacheManager
{
/**
* @var string
*/
protected $tagsHeader = 'X-Cache-Tags';

/**
* @var HttpCacheInterface
*/
Expand All @@ -31,15 +39,60 @@ class CacheManager
/**
* Constructor
*
* @param HttpCacheInterface $cache HTTP cache
* @param RouterInterface $router Symfony router
* @param CacheProxyInterface $cache HTTP cache
* @param RouterInterface $router Symfony router
*/
public function __construct(HttpCacheInterface $cache, RouterInterface $router)
public function __construct(CacheProxyInterface $cache, RouterInterface $router)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, now i get it. should BanInterface etc all extend CacheProxyInterface then? it means they are "a kind of cache proxy" which seems to be what we are saying.

{
$this->cache = $cache;
$this->router = $router;
}

/**
* Set the HTTP header name that will hold cache tags
*
* @param string $tagsHeader
*/
public function setTagsHeader($tagsHeader)
{
$this->tagsHeader = $tagsHeader;
}

/**
* Get the HTTP header name that will hold cache tags
*
* @return string
*/
public function getTagsHeader()
{
return $this->tagsHeader;
}

/**
* Assign cache tags to a response
*
* @param Response $response
* @param array $tags
* @param bool $replace Whether to replace the current tags on the
* response
*
* @return $this
*/
public function tagResponse(Response $response, array $tags, $replace = false)
{
if (!$replace) {
$tags = array_merge(
$response->headers->get($this->getTagsHeader(), array()),
$tags
);
}

$uniqueTags = array_unique($tags);
$response->headers->set($this->getTagsHeader(), implode(',', $uniqueTags));

return $this;
}

/**
* Invalidate a path (URL)
*
Expand Down Expand Up @@ -83,22 +136,32 @@ public function invalidateRegex($regex)
}

/**
* Flush all paths queued for invalidation
* Invalidate cache tags
*
* @param array $tags Cache tags
*
* @return array Paths that were flushed from the queue
* @return $this
* @throws \RuntimeException If HTTP cache does not support BAN requests
*/
public function flush()
public function invalidateTags(array $tags)
{
$queue = $this->getInvalidationQueue();

if (0 === count($queue)) {
return $queue;
if (!$this->cache instanceof BanInterface) {
throw new \RuntimeException('HTTP cache does not support BAN requests');
}

$this->cache->invalidateUrls($queue);
$this->invalidationQueue = array();
$headers = array($this->getTagsHeader() => '('.implode('|', $tags).')(,.+)?$');
$this->cache->ban($headers);

return $queue;
return $this;
}

/**
* Send all invalidation requests
*
*/
public function flush()
{
$this->cache->flush();
}

/**
Expand Down
68 changes: 68 additions & 0 deletions Configuration/Tag.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

namespace FOS\HttpCacheBundle\Configuration;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't the standard namespace for annotations Annotation? see for example https://github.com/schmittjoh/JMSDiExtraBundle/tree/master/Annotation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but the SensioFrameworkExtraBundle uses Configuration (e.g., Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache). As our Tag annotation extends the base configuration from the FrameworkExtraBundle, I decided to keep their namespace. Plus it makes more sense for people already using the Cache annotation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay


use FOS\HttpCacheBundle\Exception\InvalidTagException;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\ConfigurationAnnotation;

/**
* @Annotation
*/
class Tag extends ConfigurationAnnotation
{
protected $tags;
protected $expression;

public function setValue($data)
{
$this->setTags(is_array($data) ? $data: array($data));
}

/**
* @param mixed $expression
*/
public function setExpression($expression)
{
$this->expression = $expression;
}

/**
* @return mixed
*/
public function getExpression()
{
return $this->expression;
}

public function setTags(array $tags)
{
foreach ($tags as $tag) {
if (false !== \strpos($tag, ',')) {
throw new InvalidTagException($tag, ',');
}
}

$this->tags = $tags;
}

public function getTags()
{
return $this->tags;
}

/**
* {@inheritdoc}
*/
public function getAliasName()
{
return 'tag';
}

/**
* {@inheritdoc}
*/
public function allowArray()
{
return true;
}
}
90 changes: 90 additions & 0 deletions EventListener/TagListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<?php

namespace FOS\HttpCacheBundle\EventListener;

use FOS\HttpCacheBundle\CacheManager;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;

class TagListener implements EventSubscriberInterface
{
/**
* @var CacheManager
*/
protected $cacheManager;

/**
* Constructor
*
* @param CacheManager $cacheManager
* @param ExpressionLanguage $expressionLanguage
*/
public function __construct(
CacheManager $cacheManager,
ExpressionLanguage $expressionLanguage = null
) {
$this->cacheManager = $cacheManager;
$this->expressionLanguage = $expressionLanguage ?: new ExpressionLanguage();
}

/**
* Process the _tags request attribute, which is set when using the Tag
* annotation
*
* - For a safe (GET or HEAD) request, the tags are set on the response.
* - For a non-safe request, the tags will be invalidated.
*
* @param FilterResponseEvent $event
*/
public function onKernelResponse(FilterResponseEvent $event)
{
$request = $event->getRequest();

// Check for _tag request attribute that is set when using @Tag
// annotation
if (!$tagConfigurations = $request->attributes->get('_tag')) {
return;
}

$response = $event->getResponse();

// Only set cache tags or invalidate them if response is successful
if (!$response->isSuccessful()) {
return;
}

$tags = array();
foreach ($tagConfigurations as $tagConfiguration) {
if (null !== $tagConfiguration->getExpression()) {
$tags[] = $this->expressionLanguage->evaluate(
$tagConfiguration->getExpression(),
$request->attributes->all()
);
} else {
$tags = array_merge($tags, $tagConfiguration->getTags());
}
}

$uniqueTags = array_unique($tags);

if ($request->isMethodSafe()) {
// For safe requests (GET and HEAD), set cache tags on response
$this->cacheManager->tagResponse($response, $uniqueTags);
} else {
// For non-safe methods, invalidate the tags
$this->cacheManager->invalidateTags($uniqueTags);
}
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents()
{
return array(
KernelEvents::RESPONSE => 'onKernelResponse'
);
}
}
11 changes: 11 additions & 0 deletions Exception/InvalidTagException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace FOS\HttpCacheBundle\Exception;

class InvalidTagException extends \InvalidArgumentException
{
public function __construct($tag, $char)
{
parent:__construct(sprintf('Tag %s is invalid because it contains %s'));
}
}
8 changes: 8 additions & 0 deletions Invalidation/CacheProxyInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace FOS\HttpCacheBundle\Invalidation;

interface CacheProxyInterface
{

}
20 changes: 19 additions & 1 deletion Invalidation/Method/BanInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ interface BanInterface
const REGEX_MATCH_ALL = '.*';
const CONTENT_TYPE_ALL = self::REGEX_MATCH_ALL;

/**
* Ban cached objects matching HTTP headers
*
* Please make sure to configure your HTTP caching proxy to set the headers
* supplied here on the cached objects. So if you want to match objects by
* host name, configure your proxy to copy the host to a custom HTTP header
* such as X-Host.
*
* @param array $headers HTTP headers that path must match to be banned.
* Each header is either a:
* - regular string ('X-Host' => 'example.com')
* - or a POSIX regular expression
* ('X-Host' => '^(www\.)?(this|that)\.com$').
*
* @return $this
*/
public function ban(array $headers);

/**
* Ban paths matching a regular expression
*
Expand All @@ -30,5 +48,5 @@ interface BanInterface
*
* @return $this
*/
public function ban($path, $contentType = self::CONTENT_TYPE_ALL, array $hosts = null);
public function banPath($path, $contentType = self::CONTENT_TYPE_ALL, array $hosts = null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might comment that this is a convenience method and which headers it MUST use. i guess that should be standardized and not left to the implementation, or wdyt? or would the header names more be a SHOULD?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a tough one. On the one hand, the bundle's docs recommend a Varnish configuration with specified header names, and in order to work out of the box the implementation must use those header names. On the other, I think we should allow users to use the bundle with their own Varnish configuration (different header names etc.). Perhaps the best way would be to leave the interface free as it is, and do the right thing in our Varnish implementation? Users could always create their own implementation. To make this easier, we could allow overriding the header names in the Varnish class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. making the headers varnish is using configurable could be a nice solution to make things flexible if needed. everything else really is a documentation topic then.

}
Loading