Dan, Definitely will be good to not have a default value for the refresh setting ... did you just update it just for makeRequest or for the full set of APIs?
My personal sense is that the http caching spec has gone through a lot of review and tweaking and forms a foundational part of what users expect for the behavior in their browser. Both ignoring the origin server's caching directives and reinventing rather than reusing in this area seem to me to be the wrong overall semantic/approach. Rich Thompson From: Dan Dumont/Westford/IBM@Lotus To: [email protected], Date: 04/26/2012 03:10 PM Subject: Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0 Rich, I have a review out which impacts some of what you're talking about here. https://reviews.apache.org/r/4874/ After this patch, makeRequest will never send a refresh setting unless the gadget explicitly provides one. At this point, is it really a problem if a gadget knowingly wants to cache some data it knows will be stale for performance reasons? From: Rich Thompson/Watson/IBM@IBMUS To: [email protected], Date: 04/26/2012 02:38 PM Subject: Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0 Yes, but http caching does encourage servers to sync their time with the internet time servers while also being tolerant of skewed clocks (i.e. cached content might get served for slightly longer than the origin server said was valid). I think there are two significant issues needing to be addressed on caching: 1. If the origin server said the response can't be cached, no cached response should ever get served back on later requests (this JIRA/patch addresses this point) 2. Section 6.2 of the Gadget-core spec says that the request's refresh setting overrides the origin server's cache controls. I think this is invalid ... the refresh param should indicate the default for caching and potentially reduce the time a response will be cached, but not increase the time a cached response is considered valid. Rich Thompson Brian Lillie---04/26/2012 10:57:28 AM-------------------------------------------------------------- This is an automatically generated e-mai From: Brian Lillie/Austin/IBM@IBMUS To: Dan Dumont/Westford/IBM@Lotus, "Stanton Sievers" <[email protected]>, Brian Lillie/Austin/IBM@IBMUS, "Ryan Baxter" <[email protected]>, Cc: Brian Lillie/Austin/IBM@IBMUS, "Xiao Feng Yu" <[email protected]>, "shindig" <[email protected]> Date: 04/26/2012 10:57 AM Subject: Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0 ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4877/#review7263 ----------------------------------------------------------- As noted in the JIRA, the issue with the no-cache being returned on the first request, and the (incorrect) cached response on the second request occurs due to the modification of the cached response in the AbstractHttpCache.addResponse. This code slams the request's refresh interval onto the cache-control headers, and puts that into the cache, while the original request with no-cache is returned to the user. It seems like this fix would address only one specific issue, that of an incoming request with refresh interval = 0. If the refresh interval specified by the client is sufficiently smaller than the time skew between the two servers, wouldn't the same issue occur ? - BrianLillie On 2012-04-26 02:15:03, Xiao Feng Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4877/ > ----------------------------------------------------------- > > (Updated 2012-04-26 02:15:03) > > > Review request for shindig, Ryan Baxter, Dan Dumont, Stanton Sievers, and Brian Lillie. > > > Summary > ------- > > Since max-age=0 means the response expires immediately when the response is generated, and there is no need to compute TTL with possibly incorrect server time. > We can update the isStale method in HttpResponse to return true if max-age equals 0 instead of relying on computed TTL. > > > This addresses bug SHINDIG-1759. > https://issues.apache.org/jira/browse/SHINDIG-1759 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java 1330087 > > Diff: https://reviews.apache.org/r/4877/diff > > > Testing > ------- > > > Thanks, > > Xiao Feng > >
