Skip to content

Cleanup invalidation annotation #121

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 3 commits into from
Aug 2, 2014
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
22 changes: 22 additions & 0 deletions Configuration/InvalidateRoute.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace FOS\HttpCacheBundle\Configuration;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\ConfigurationAnnotation;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;

/**
* @Annotation
Expand Down Expand Up @@ -54,6 +55,27 @@ public function getName()
*/
public function setParams($params)
{
if (!is_array($params)) {
throw new \RuntimeException('InvalidateRoute params must be an array');
}
foreach ($params as $name => $value) {
if (is_array($value)) {
if (1 !== count($value) || !isset($value['expression'])) {
throw new \RuntimeException(sprintf(
'@InvalidateRoute param %s must be string or {"expression"="<expression>"}',
$name,
print_r($value, true)
));
}
if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
throw new InvalidConfigurationException(sprintf(
'@InvalidateRoute param %s uses an expression but the ExpressionLanguage is not available.',
$name
));
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

doing the validation early to notice problems as soon as possible


$this->params = $params;
}

Expand Down
4 changes: 4 additions & 0 deletions Configuration/Tag.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use FOS\HttpCacheBundle\Exception\InvalidTagException;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\ConfigurationAnnotation;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;

/**
* @Annotation
Expand All @@ -32,6 +33,9 @@ public function setValue($data)
*/
public function setExpression($expression)
{
if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
throw new InvalidConfigurationException('@Tag param %s uses an expression but the ExpressionLanguage is not available.');
}
$this->expression = $expression;
}

Expand Down
12 changes: 10 additions & 2 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ private function addMatch(NodeBuilder $rules)
->ifTrue(function ($v) {return !empty($v['additional_cacheable_status']) && !empty($v['match_response']);})
->thenInvalid('You may not set both additional_cacheable_status and match_response.')
->end()
->validate()
->ifTrue(function ($v) {return !empty($v['match_response']) && !class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage');})
->thenInvalid('Configured a match_response but ExpressionLanugage is not available')
Copy link
Member

Choose a reason for hiding this comment

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

typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, fixed in 8597598

->end()
->children()
->scalarNode('path')
->defaultNull()
Expand Down Expand Up @@ -324,7 +328,7 @@ private function addTagSection(ArrayNodeDefinition $rootNode)
->enumNode('enabled')
->values(array(true, false, 'auto'))
->defaultValue('auto')
->info('Allows to disable the listener for tag annotations when your project does not use the annotations. Enabled by default if you have expression language and the cache manager.')
->info('Allows to disable the event subscriber for tag configuration and annotations when your project does not use the annotations. Enabled by default if you configured the cache manager.')
->end()
->scalarNode('header')
->defaultValue('X-Cache-Tags')
Expand All @@ -334,6 +338,10 @@ private function addTagSection(ArrayNodeDefinition $rootNode)
->prototype('array')
->fixXmlConfig('tag')
->fixXmlConfig('tag_expression')
->validate()
->ifTrue(function ($v) {return !empty($v['tag_expressions']) && !class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage');})
->thenInvalid('Configured a tag_expression but ExpressionLanugage is not available')
->end()
->children();

$this->addMatch($rules);
Expand Down Expand Up @@ -361,7 +369,7 @@ private function addInvalidationSection(ArrayNodeDefinition $rootNode)
->enumNode('enabled')
->values(array(true, false, 'auto'))
->defaultValue('auto')
->info('Allows to disable the listener for invalidation annotations when your project does not use the annotations. Enabled by default if you have expression language and the cache manager.')
->info('Allows to disable the listener for invalidation. Enabled by default if the cache manager is configured. When disabled, the cache manager is no longer flushed automatically.')
->end()
->arrayNode('rules')
->info('Set what requests should invalidate which target routes.')
Expand Down
23 changes: 6 additions & 17 deletions DependencyInjection/FOSHttpCacheExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,9 @@ public function load(array $configs, ContainerBuilder $container)

if ($config['tags']['enabled']) {
// true or auto
if (class_exists('\Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
$loader->load('tag_listener.xml');
if (!empty($config['tags']['rules'])) {
$this->loadTagRules($container, $config['tags']['rules']);
}
} elseif (true === $config['tags']['enabled']) {
// silently skip if set to auto
throw new InvalidConfigurationException('The TagSubscriber requires symfony/expression-language and needs the cache_manager to be configured');
$loader->load('tag_listener.xml');
if (!empty($config['tags']['rules'])) {
$this->loadTagRules($container, $config['tags']['rules']);
}

$tagsHeader = $config['tags']['header'];
Expand All @@ -81,15 +76,9 @@ public function load(array $configs, ContainerBuilder $container)
}

if ($config['invalidation']['enabled']) {
// true or auto
if (class_exists('\Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
$loader->load('invalidation_listener.xml');
if (!empty($config['invalidation']['rules'])) {
$this->loadInvalidatorRules($container, $config['invalidation']['rules']);
}
} elseif (true === $config['invalidation']['enabled']) {
// silently skip if set to auto
throw new InvalidConfigurationException('The InvalidationSubscriber requires symfony/expression-language and needs the cache_manager to be configured');
$loader->load('invalidation_listener.xml');
if (!empty($config['invalidation']['rules'])) {
$this->loadInvalidatorRules($container, $config['invalidation']['rules']);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i noticed that the InvalidationSubscriber is the only one calling CacheManager::flush, so i guess we want that enabled whenver possible to reduce the WTF moments. now that we make expressions explicit, this should be fine - we only fail when people use expressions without having expression language.

note that includes/enabled.rst is now not correct anymore.

i suggest we also simplify the rules for the tag subscriber, it would also work without expressions now, right?

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think the TagSubscriber will: the constructor will fail without ExpressionLanguage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed. i copied the behaviour from InvalidationSubscriber now so it won't fail anymore.

}
}

Expand Down
8 changes: 2 additions & 6 deletions EventListener/InvalidationSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use FOS\HttpCacheBundle\CacheManager;
use FOS\HttpCacheBundle\Configuration\InvalidatePath;
use FOS\HttpCacheBundle\Configuration\InvalidateRoute;
use FOS\HttpCacheBundle\Invalidator\InvalidatorCollection;
use Symfony\Component\Console\ConsoleEvents;
use Symfony\Component\Console\Event\ConsoleEvent;
use Symfony\Component\Console\Output\OutputInterface;
Expand Down Expand Up @@ -210,11 +209,8 @@ private function invalidateRoutes(array $routes, Request $request)
if (null !== $route->getParams()) {
// Iterate over route params and try to evaluate their values
foreach ($route->getParams() as $key => $value) {
try {
$value = $this->getExpressionLanguage()->evaluate($value, $request->attributes->all());
} catch (SyntaxError $e) {
// If a syntax error occurred, we assume the param was
// no expression
if (is_array($value)) {
$value = $this->getExpressionLanguage()->evaluate($value['expression'], $request->attributes->all());
}

$params[$key] = $value;
Expand Down
18 changes: 16 additions & 2 deletions EventListener/TagSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function __construct(
ExpressionLanguage $expressionLanguage = null
) {
$this->cacheManager = $cacheManager;
$this->expressionLanguage = $expressionLanguage ?: new ExpressionLanguage();
$this->expressionLanguage = $expressionLanguage;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moving into a getter that instantiates if needed.

i validate the existence of expressionlanguage in the annoation class, so i think we never ever can have fatal errors anymore now, and can use everything without expressions properly.

}

/**
Expand Down Expand Up @@ -146,10 +146,24 @@ private function getAnnotationTags(Request $request)
*/
private function evaluateTag($expression, Request $request)
{
return $this->expressionLanguage->evaluate(
return $this->getExpressionLanguage()->evaluate(
$expression,
$request->attributes->all()
);
}

/**
* Delay instantiating the expression language instance until we need it,
* to support a setup with only symfony 2.3.
*
* @return ExpressionLanguage
*/
private function getExpressionLanguage()
{
if (!$this->expressionLanguage) {
$this->expressionLanguage = new ExpressionLanguage();
}

return $this->expressionLanguage;
}
}
2 changes: 1 addition & 1 deletion Exception/InvalidTagException.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ class InvalidTagException extends \InvalidArgumentException
{
public function __construct($tag, $char)
{
parent::__construct(sprintf('Tag %s is invalid because it contains %s'));
parent::__construct(sprintf('Tag %s is invalid because it contains %s', $tag, $char));
}
}
7 changes: 4 additions & 3 deletions Resources/doc/features/invalidation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,12 @@ invalidation from your controllers::
use FOS\HttpCacheBundle\Configuration\InvalidatePath;

/**
* @InvalidatePath("/posts")
* @InvalidatePath("/posts/latest")
* @InvalidatePath("/articles")
* @InvalidatePath("/articles/latest")
* @InvalidateRoute("overview", params={"type" = "latest"})")
* @InvalidateRoute("detail", params={"id" = {"expression"="id"}})")
*/
public function editAction()
public function editAction($id)
{
}

Expand Down
4 changes: 2 additions & 2 deletions Resources/doc/includes/enabled.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ enabled

**type**: ``enum``, **default**: ``auto``, **options**: ``true``, ``false``, ``auto``

Enabled by default if :ref:`ExpressionLanguage is installed <expression language requirement>`
and you have :doc:`configured a proxy client </reference/configuration/proxy-client>`.
Enabled by default if you have configured the cache manager with
:doc:`a proxy client </reference/configuration/proxy-client>`.
28 changes: 15 additions & 13 deletions Resources/doc/reference/annotations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ actions are executed.

.. note::

Annotations need the SensioFrameworkExtraBundle. Some features also need
the ExpressionLanguage. Make sure to
Annotations need the SensioFrameworkExtraBundle including registering the
Doctrine AnnotationsRegistry. Some features also need the
ExpressionLanguage. Make sure to
:ref:`installed the dependencies first <requirements>`.

.. _invalidatepath:
Expand All @@ -20,8 +21,8 @@ Invalidate a path::
use FOS\HttpCacheBundle\Configuration\InvalidatePath;

/**
* @InvalidatePath("/posts")
* @InvalidatePath("/posts/latest")
* @InvalidatePath("/articles")
* @InvalidatePath("/articles/latest")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed all examples to use /articles rather than /posts to avoid any confusion with the http POST method (i was confused while jumping around in the doc for a sec, so thought for somebody new to all this, its better to use this name)

Copy link
Member

Choose a reason for hiding this comment

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

Good one.

*/
public function editAction()
{
Expand All @@ -39,25 +40,25 @@ Invalidate a route with parameters::
use FOS\HttpCacheBundle\Configuration\InvalidateRoute;

/**
* @InvalidateRoute("posts")
* @InvalidateRoute("posts", params={"type" = "latest"})
* @InvalidateRoute("articles")
* @InvalidateRoute("articles", params={"type" = "latest"})
*/
public function editAction()
{
}

You can also use expressions_ in the route parameter values::
You can also use expressions_ in the route parameter values. This obviously
:ref:`requires the ExpressionLanguage component <requirements>`. To invalidate
route ``articles`` with the ``number`` parameter set to ``123``, do::

/**
* @InvalidateRoute("posts", params={"number" = "id"})
* @InvalidateRoute("articles", params={"number" = {"expression"="id"}})
*/
public function editAction(Request $request)
public function editAction(Request $request, $id)
{
// Assume $request->attributes->get('id') returns 123
}

Route ``posts`` will now be invalidated with value ``123`` for param ``number``.

See :doc:`/features/invalidation` for more information.

.. _tag:
Expand Down Expand Up @@ -106,8 +107,9 @@ If you prefer, you can combine tags in one annotation::
* @Tag({"news", "news-list"})
*/

You can also use expressions_ in tags. This will set tag ``news-123`` on the
response::
You can also use expressions_ in tags. This obviously
:ref:`requires the ExpressionLanguage component <requirements>`. The following
example sets the tag ``news-123`` on the Response::

/**
* @Tag(expression="'news-'~id")
Expand Down
69 changes: 69 additions & 0 deletions Tests/Functional/EventListener/InvalidationSubscriberTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php

/*
* This file is part of the FOSHttpCacheBundle package.
*
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace FOS\HttpCacheBundle\Tests\Functional\EventListener;

use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;

class InvalidationSubscriberTest extends WebTestCase
{
public function testInvalidateRoute()
{
$client = static::createClient();

$client->getContainer()->mock(
'fos_http_cache.cache_manager',
'\FOS\HttpCacheBundle\CacheManager'
)
->shouldReceive('invalidateRoute')->once()->with('test_noncached', array())
->shouldReceive('invalidateRoute')->once()->with('test_cached', array('id' => 'myhardcodedid'))
->shouldReceive('invalidateRoute')->once()->with('tag_one', array('id' => '42'))
->shouldReceive('flush')->once()
;

$client->request('POST', '/invalidate/route/42');
}

public function testInvalidatePath()
{
$client = static::createClient();

$client->getContainer()->mock(
'fos_http_cache.cache_manager',
'\FOS\HttpCacheBundle\CacheManager'
)
->shouldReceive('invalidatePath')->once()->with('/cached')
->shouldReceive('flush')->once()
;

$client->request('POST', '/invalidate/path');
}

public function testErrorIsNotInvalidated()
{
$client = static::createClient();

$client->getContainer()->mock(
'fos_http_cache.cache_manager',
'\FOS\HttpCacheBundle\CacheManager'
)
->shouldReceive('invalidatePath')->never()
->shouldReceive('flush')->once()
;

$client->request('POST', '/invalidate/error');
}

protected function tearDown()
{
static::createClient()->getContainer()->unmock('fos_http_cache.cache_manager');
}
}
8 changes: 4 additions & 4 deletions Tests/Functional/EventListener/TagSubscriberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ public function testAnnotationTagsAreSet()
{
$client = static::createClient();

$client->request('GET', '/test/list');
$client->request('GET', '/tag/list');
$response = $client->getResponse();
$this->assertEquals('all-items,item-123', $response->headers->get('X-Cache-Tags'));

$client->request('GET', '/test/123');
$client->request('GET', '/tag/123');
$response = $client->getResponse();
$this->assertEquals('item-123', $response->headers->get('X-Cache-Tags'));
}
Expand All @@ -41,7 +41,7 @@ public function testAnnotationTagsAreInvalidated()
->shouldReceive('flush')->once()
;

$client->request('POST', '/test/123');
$client->request('POST', '/tag/123');
}

public function testErrorIsNotInvalidated()
Expand All @@ -56,7 +56,7 @@ public function testErrorIsNotInvalidated()
->shouldReceive('flush')->once()
;

$client->request('POST', '/test/error');
$client->request('POST', '/tag/error');
}

public function testConfigurationTagsAreSet()
Expand Down
Loading