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
Browser cache impossible to control for JSON output #19941
Comments
Why one minute? At that point you might as well not have a cache directive.
You can manually set an Expires header elsewhere, the code in the WebApplication setting the 15 minute header isn't passing the
Honestly, I'd rather this be a |
One minute is just an example. In our case we have a large busy site with dynamic AJAX resources which we would like the browser to cache between page loads, so 2-5 minutes can make a huge difference. I'll check out setting the expires header in this way, so I guess that this is not so much an issue as a feature request for a nice-to-have. In which case, having a DateTime sounds sensible and could be more usable than a count of seconds. |
Ahh, sorry. Read that at first as you expected the framework to default to one minute. Apparently I need my Monday morning nap before commenting on anymore GitHub issues 🤣 |
Here is how to do it
If you call JResponse::allowCache(true); The above will prevent 2 headers from being added by WebApplication::respond(): $this->setHeader('Cache-Control', 'no-store, no-cache, must-revalidate, post-check=0, pre-check=0', false);
// HTTP 1.0
$this->setHeader('Pragma', 'no-cache'); You still get the expires header added as you describe but that is ok, (see below) So the final code to use is // Prevent Joomla WebApplication::respond(): from setting 'Cache-Control' and 'Pragma'
JFactory::getApplication()->allowCache(true);
// CONTROL INTERMEDIARY CACHES (PROXY, ETC)
// Instruct them not cache (potentially) private resource (here you can add more code to decide)
$cacheControl = JFactory::getUser()->id ? 'private' : 'public';
// Set MAX-AGE, to allow modern browsers to cache the page, despite expires header in the past
$cacheControl .= ', max-age=300';
JResponse::setHeader('Cache-Control', $cacheControl );
// Make sure no legacy proxies do any caching (just in case)
// or just do not add this at all, Cache-Control + max-age take priority
// or as you said you cannot override it (if so just skip this)
JResponse::setHeader('Expires', 'Fri, 01 Jan 1990 00:00:00 GMT'); |
Except use |
I do not say that JFactory::getApplication()->allowCache(true); should not take more parameters to allow more control over what it does, maybe it should be made more flexible |
|
OK. It turns out the source of my woes is in fact \Joomla\CMS\Document\JsonDocument. It's overwriting the caching parameter in the application and I can't set it. It ignores the cache parameter passed in to render.
|
So the problems are:
When is Joomla 4 being released? ;-) |
I'm going to have to give up on solving this in my application, I need a fix to Joomla! I've been trying @ggppdk's suggestion but it doesn't work in this case. I set Cache-Control to max-age=300 in my JSON view (document->type() is json), by using the Application api, as suggested by @mbabker error_log in WebApplication::setHeader shows:
JsonDocument then sets $app->allowCache(false) as a hard-wired value shows:
Resulting headers are:
So, Cache-Control header is treated as a multi-value header and the combination of the values disables caching. Can we just take the $app->allCache(false) call out of JsonDocument::render()? It defaults to false in WebApplication, anyway. -David. |
Yes my suggestion will not work in case of JSON document as you said public function render($cache = false, $params = array())
{
$app = \JFactory::getApplication();
$app->allowCache(false); |
I don't see why it needs to be there (and if someone can give a valid reason then it can stay). |
I think the idea of adding it there was to guarantee that JSON responses are not cached, Removing it now, will cause a change in default behaviour that will break websites that have code / configuration to enable browser caching and do it without checking the document type but expect that JSON responses are not cached Only by extending API we could keep "B/C" behaviour
thus any call like the above is ugly ? probably yes, but i am talking about B/C and about breaking some websites ... |
Adding method parameters comes with the technical cost of new B/C constraints for the future. Please don't do that. Even if it's temporary it's bad. And some will argue adding optional parameters to existing methods is a B/C break too (actually on this platform we've gone so far as to say adding methods to classes breaks B/C, don't get me started). You do realize there is no place in core calling |
same as #19105 ? |
Yes, this is the same. |
I'm not keen on the additional parameter but it makes sense, I think, to remove the erroneous call in JsonDocument::render. I see the potential issue with plugins but it will pose a performance issue rather than a behaviour issue in that scenario, because the cache would then be disabled. If it was the other way round it would change the behaviour. |
I was going to suggest this as a work around
i mean as temporarily solution until the Joomla version that this addressed |
I really hate that workaround. I'd rather us fix the freakin' framework than keep saying "bypass and avoid it". All saying "use this workaround" is doing is basically saying "m'eh, we'll fix this eventually, if you're lucky". |
Set to "closed" on behalf of @jwaisner by The JTracker Application at issues.joomla.org/joomla-cms/19941 |
Duplicate of #19105 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19941. |
Steps to reproduce the issue
Try setting the expiry date to sometime in the future that isn't 15 minutes
$app = JFactory::getApplication();
$app->setHeader('Expires', gmdate('D, d M Y H:i:s', time() + 60) . ' GMT');
Optionally, also set the application to enable browser cache
$app->allowCache(true);
Expected result
We should get a HTTP header with Expires set to 1 minute in the future, somehow.
Actual result
If we don't set allowCache(true) then we get a date in 2005. This would be OK if allowCache allowed our value of Expires. Instead, it is hard-wired to set an expiry date of 15 minutes into the future (900s)
System information (as much as possible)
Joomla 3.8.6
Additional comments
I propose adding a function setExpiryPeriod to WebApplication which sets the number of seconds into the future the HTTP expiry field can be set.
The text was updated successfully, but these errors were encountered: