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

Fix EZP-23057: viewContent does not set the X-Location-Id #903

Merged
merged 1 commit into from
Jun 30, 2014

Conversation

dpobel
Copy link
Contributor

@dpobel dpobel commented Jun 24, 2014

JIRA: https://jira.ez.no/browse/EZP-23057

Description

The view content action in the view controller does not set the X-Location-Id header preventing the cache from being purged!

(Bug reported by @pborreli)

Tests

manual test

@gggeek
Copy link
Contributor

gggeek commented Jun 24, 2014

A small update to phpdoc above?

@andrerom
Copy link
Contributor

Was it intended to set X-Content-Id perhaps when multi id support will be added?

@joaoinacio
Copy link

Q: What about secondary locations?

@andrerom
Copy link
Contributor

A: cache code should purge on all locations currently afaik.

@lolautruche
Copy link
Contributor

+1

1 similar comment
@andrerom
Copy link
Contributor

+1

@joaoinacio
Copy link

Untested, but wouldn't there be problems with custom requests, varnish etc?
Use case:

  • Content A, Locations 100 and 200
  • Render location 200, receive X-Location-Id: 100
  • Cannot purge location 200

@andrerom
Copy link
Contributor

This is viewContent, not viewLocation. But ideally this should set Content-id instead.

@gggeek
Copy link
Contributor

gggeek commented Jun 25, 2014

Right now when content is edited, it sends purge commands with all locations, so it will expire correctly viewContent as well.
Otoh custom code which only triggers an expiry using id of location B will not get the ViewContent cache to expire as well.

@andrerom are you suggesting we add a new header X-Content-Id and use it for cache expiration?
It might make sense, as we'd reduce the number of cached items tied to a single location-id (and Sf is already pushed to its limits by eZ5, according to implementors feedback).
Otoh it would mean that we have to send 1 more purge request on each content edit

@andrerom
Copy link
Contributor

I was implying multi id (Content, ContentType, Section...) support yes, and afaik this is planned to be solved in FOSHTTPCacheBundle which we intend to adopt over the summer.

@pborreli
Copy link
Contributor

@gggeek right now, the purge only invalidates page with an actual X-Location-Id header as it purges X-Location-Id: *

@andrerom
Copy link
Contributor

(and Sf is already pushed to its limits by eZ5, according to implementors feedback).

yes, but high traffic multi frontend is supposed to be handled with Varnish. A hammer works badly as a heavy duty breaker.. :)

@andrerom
Copy link
Contributor

Issue tagged to also be back ported to 5.3.2

@pborreli
Copy link
Contributor

@andrerom will it be included in next release 2014.05.01?

dpobel added a commit that referenced this pull request Jun 30, 2014
…on-id

Fix EZP-23057: viewContent does not set the X-Location-Id
@dpobel dpobel merged commit 54fecd2 into master Jun 30, 2014
@dpobel dpobel deleted the ezp-23057_viewcontent-x-location-id branch June 30, 2014 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants