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

Configuration or annotation invalidation not working in Symfony 2.7.1 #224

Closed
benr77 opened this issue Jun 17, 2015 · 23 comments
Closed

Configuration or annotation invalidation not working in Symfony 2.7.1 #224

benr77 opened this issue Jun 17, 2015 · 23 comments
Assignees

Comments

@benr77
Copy link
Contributor

benr77 commented Jun 17, 2015

Hi,

I'm setting up this bundle on Symfony 2.7.1 using the Symfony2 proxy client and of course the Symfony2 HttpCache. If I manually set up invalidation rules in the controller action methods, it works fine and I can see the PURGE requests appearing in the web server logs and the cache is cleared successfully.

However, I cannot get any invalidation to work with the invalidation rules specified either in the YML or as Annotations.

Here is my config

fos_http_cache:
  proxy_client:
    symfony:
      servers: 127.0.0.1
      base_url: dev.localhost
  cache_control:
    defaults:
      overwrite: true
    rules:
      -
        match:
          attributes:
            _route: ^booking$
        headers:
          cache_control: { public: true, max_age: 0, s_maxage: 64000 }
          etag: true
          vary: [Accept-Encoding]

      # match everything to set defaults
      -
        match:
          path: ^/
        headers:
          overwrite: false
          cache_control: { public: false, max_age: 0, s_maxage: 0 }
          etag: false
  invalidation:
    rules:
      -
        match:
          attributes: # When hitting these routes
            _route: "booking_update"
        routes:       # Invalidate cache for the following routes
          booking: ~

As you can see I'm asking it to invalidate the "booking" route when the "booking_update" route is successfully requested. This appears to do nothing.

I have also tried in the BookingUpdateAction() method the annotation

@InvalidateRoute("booking")

I have the SensioFrameworkExtra bundle installed as per the docs.

I have also modified app/AppCache.php as follows:

<?php

require_once __DIR__.'/AppKernel.php';

use Symfony\Component\HttpKernel\HttpKernelInterface;
use FOS\HttpCacheBundle\SymfonyCache\EventDispatchingHttpCache;

/**
 * Class AppCache
 */
class AppCache extends EventDispatchingHttpCache
{

}

Finally, here is my app_dev.php file

<?php

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Debug\Debug;

$loader = require_once __DIR__.'/../app/bootstrap.php.cache';
Debug::enable();

//require_once __DIR__.'/../app/AppKernel.php';
require_once __DIR__.'/../app/AppCache.php';

$kernel = new AppKernel('dev', true);
$kernel->loadClassCache();
$kernel = new AppCache($kernel);

Request::enableHttpMethodParameterOverride();
$request = Request::createFromGlobals();

$response = $kernel->handle($request);

$response->send();

$kernel->terminate($request, $response);

Can anyone shed any light on why the invalidation does not work unless I define it manually in the controller action method?

Alternatively, maybe this is a bug with Symfony 2.7.1

I'm running PHP 5.5.9 on Apache 2.4.7 on Linux Mint 17.1

Thanks

Ben

@ddeboer
Copy link
Member

ddeboer commented Jun 21, 2015

This is weird. All tests succeed on Symfony 2.7.1.

Can you show us how you invalidate bookingUpdate manually?

And when add a breakpoint in CacheManager::invalidateRoute(), is it hit when using annotations or configuration?

@benr77
Copy link
Contributor Author

benr77 commented Jun 23, 2015

I am manually invalidating the cache for a route called "booking" in the bookingUpdateAction() and bookingCreateAction() methods with:

$cacheManager = $this->get('fos_http_cache.cache_manager');
$cacheManager->invalidateRoute('booking')->flush();

@benr77
Copy link
Contributor Author

benr77 commented Jun 23, 2015

I can also confirm that CacheManager::invalidateRoute() is NOT hit when using configuration or annotations - only when I use the manual statements in my previous comment.

@dbu
Copy link
Contributor

dbu commented Jul 3, 2015

@benr77 can you try to reproduce the problem in a test and do a pull request that fails with symfony 2.7.1 please? i think the test is
https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/Tests/Functional/EventListener/InvalidationSubscriberTest.php which hits https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/Tests/Functional/Fixtures/Controller/InvalidationController.php

try to add your situation there so we see a failing test. that will be the best way to fix the problem.

@tgalopin
Copy link
Contributor

tgalopin commented Jul 3, 2015

I have the same problem. I tried to debug it a bit and Varnish is never contacted. I suspect a problem in the matching of a request (especially with attributes and _route). I'll try to debug it a bit more.

@ddeboer
Copy link
Member

ddeboer commented Jul 3, 2015

Thanks @tgalopin, keep us posted!

@tgalopin
Copy link
Contributor

tgalopin commented Jul 3, 2015

Okay, I found my problem, but I'm not sure it's the same than @benr77: FOSHttpCacheBundle does not match 204 No Content by default and I tried to invalidate on the deletion on an entity with an endpoint returning a 204. I think the 204 should be added to the list of default cached resources (it's a success after all :) ), espacially in https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/Http/RuleMatcher.php#L72.

I think it's not very clear in the documentation what status codes does match match.

For the moment I added the status in the configuration: additional_cacheable_status: [ 204 ]

@dbu
Copy link
Contributor

dbu commented Jul 4, 2015

hm, i think our reusing things tripps us here. for adding caching headers, it makes sense to ignore 204 because i can't think of a sane use case to return 204 to a GET request. but for active invalidation, i think all 2xx and 3xx status should be used. we want to invalidate whenever an operation worked out.

i guess the critical bit is here: https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/EventListener/InvalidationSubscriber.php#L161 - we should somehow make that match logic more flexible, have a separate isCacheableStatus method maybe

@dbu
Copy link
Contributor

dbu commented Jul 4, 2015

this should also be noted in the changelog. with master already being 2.0 we don't need to worry about BC breaks.

@ddeboer
Copy link
Member

ddeboer commented Jul 4, 2015

I agree on invalidating all 2xx/3xx responses.

But the RFC describes 204 as ‘cacheable by default’, too. So should we adhere to the RFC and put caching headers on that, too?

@dbu
Copy link
Contributor

dbu commented Jul 4, 2015

oh, indeed rfc2616 was more restrictive: http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.4 - lets fix the default list

but also fix the behaviour that invalidation happens on any status < 400.

@ddeboer
Copy link
Member

ddeboer commented Jul 5, 2015

Okay, the situation is as follows.

  1. When using annotations (@InvalidatePath or @InvalidateRoute), any 2xx response (including 204) correctly triggers invalidation.
  2. When using configuration (the invalidation: section in config.yml), 204 responses do not trigger invalidation. We should fix this in RuleMatcher for TagSubscriber, InvalidationSubscriber #69.

@tgalopin
Copy link
Contributor

tgalopin commented Jul 5, 2015

According to the first post of @benr77, @InvalidPath and @InvalidRoute do not works (which seems logical as it's the same system behind).

@ddeboer
Copy link
Member

ddeboer commented Jul 5, 2015

The systems are not completely identical:

  • for annotations, we only check whether the response is successful, so any 2xx, including 204 will work
  • for configuration, we additionally check whether the response is cacheable, currently excluding 204. This is a mistake, though (as I describe in RuleMatcher for TagSubscriber, InvalidationSubscriber #69 (comment)): when triggering invalidation we should not check for response cacheability but success instead.

@dbu
Copy link
Contributor

dbu commented Jul 6, 2015

do only 2xx invalidate or also 3xx? 3xx is also a successful state.

@tgalopin
Copy link
Contributor

tgalopin commented Jul 6, 2015

I think the bundle should follow the Symfony Response::isSuccessful()method (https://github.com/symfony/HttpFoundation/blob/master/Response.php#L1135): only 2xx status invalidate.

@dbu
Copy link
Contributor

dbu commented Jul 6, 2015

ok, i got the naming wrong. but we surely want to invalidate when a POST is answered with a 3xx status code. we can use isRedirect() or just compare ourselves.

@benr77
Copy link
Contributor Author

benr77 commented Jul 10, 2015

Hello all. I have been reading all your responses but not sure I follow everything.

Have you managed to find the problem that I reported? Is this something that is easily fixed? Do let me know if there's anything I can do to help.

@ddeboer
Copy link
Member

ddeboer commented Jul 10, 2015

@benr77 Which HTTP status code does your booking_update route return, that is, the route that should trigger the invalidation?

@benr77
Copy link
Contributor Author

benr77 commented Jul 10, 2015

@ddeboer It is a standard Symfony2 controller action, so in the updateAction() method, it performs a 302 redirect using the RedirectResponse() method

return $this->redirect($this->generateUrl('booking_show', ['id' => $id]));

@ddeboer
Copy link
Member

ddeboer commented Jul 10, 2015

Thanks, that explains it: when invalidating content, we call $response->isSuccessful(), which is true only for 2xx responses, and not for 3xx such as redirects. This makes your problem very similar to the one @tgalopin encountered. When using configuration, a workaround for now is to add additional_cacheable_status: [ 302 ].

This is a nasty problem though, and we’ll come up with a fix shortly.

ddeboer added a commit that referenced this issue Jul 10, 2015
Fix #224. Invalidation was not triggered when the controller response had
status code 204 or 302.
ddeboer added a commit that referenced this issue Jul 10, 2015
Fix #224. Invalidation was not triggered when the controller response had
status code 204 or 302.
@dbu dbu closed this as completed in #230 Jul 13, 2015
@lsmith77 lsmith77 removed the wip/poc label Jul 13, 2015
@ddeboer
Copy link
Member

ddeboer commented Jul 13, 2015

Fixed and tagged 1.3.2. @benr77 and @tgalopin Please update and let us know if your problems are resolved.

@benr77
Copy link
Contributor Author

benr77 commented Jul 14, 2015

@ddeboer Thank you! I've updated and as far as I can tell all is now working as expected. I have everything operating correctly via YML config.

Thanks everyone for helping resolve this so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants