You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Xiao Feng Yu <yu...@cn.ibm.com> on 2012/04/26 04:15:03 UTC

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/
-----------------------------------------------------------

(Updated 2012-04-26 02:15:03.180211)


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


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Xiao Feng Yu <yu...@cn.ibm.com>.

> On 2012-04-26 14:56:26, BrianLillie wrote:
> > 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 ?
> 
> Dan Dumont wrote:
>     Hmmm...  
>     
>     In my experience a max-age on a cache response indicates the maximum period of time before you should toss out the whole cached object and ask again.  Not that you shouldn't ask until you've hit the max-age (which i thought what expires was for).
>     
>     So should we treat all responses as stale if they come back with max-age so that we can ask with a last-modified query?  I think this is what browsers do.
> 
> BrianLillie wrote:
>     My interpretation of the HTTP 1.1 spec
>      - max-age - you are allowed to reuse without validating until the time expires
>      - expires - supposed to be deprecated via 1.1 spec, with cache-control taking precedence .. though of course browsers will do what they will
>      - no-cache - can't use the response for caching without re-validating with the server first
>     
>     Based on that, max-age shouldn't be treated as stale until the time expires

It is possible if the max-age value is sufficiently smaller than the time skew which is maximum to be 3 minutes by default (otherwise Shindig will take the time on Shindig server). But in rare case it would happen in real world - when the server specified max-age, the common value would be in hours or 0.  


- Xiao Feng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7263
-----------------------------------------------------------


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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by br...@us.ibm.com.

> On 2012-04-26 14:56:26, BrianLillie wrote:
> > 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 ?
> 
> Dan Dumont wrote:
>     Hmmm...  
>     
>     In my experience a max-age on a cache response indicates the maximum period of time before you should toss out the whole cached object and ask again.  Not that you shouldn't ask until you've hit the max-age (which i thought what expires was for).
>     
>     So should we treat all responses as stale if they come back with max-age so that we can ask with a last-modified query?  I think this is what browsers do.

My interpretation of the HTTP 1.1 spec
 - max-age - you are allowed to reuse without validating until the time expires
 - expires - supposed to be deprecated via 1.1 spec, with cache-control taking precedence .. though of course browsers will do what they will
 - no-cache - can't use the response for caching without re-validating with the server first

Based on that, max-age shouldn't be treated as stale until the time expires


- BrianLillie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7263
-----------------------------------------------------------


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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Dan Dumont <dd...@us.ibm.com>.

> On 2012-04-26 14:56:26, BrianLillie wrote:
> > 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 ?

Hmmm...  

In my experience a max-age on a cache response indicates the maximum period of time before you should toss out the whole cached object and ask again.  Not that you shouldn't ask until you've hit the max-age (which i thought what expires was for).

So should we treat all responses as stale if they come back with max-age so that we can ask with a last-modified query?  I think this is what browsers do.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7263
-----------------------------------------------------------


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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Rich Thompson <ri...@us.ibm.com>.
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:	dev@shindig.apache.org,
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:     dev@shindig.apache.org,
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"
<si...@gmail.com>, Brian Lillie/Austin/IBM@IBMUS, "Ryan Baxter"
<rb...@gmail.com>,
Cc: Brian Lillie/Austin/IBM@IBMUS, "Xiao Feng Yu" <yu...@cn.ibm.com>,
"shindig" <de...@shindig.apache.org>
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
>
>



Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Dan Dumont <dd...@us.ibm.com>.
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:     dev@shindig.apache.org, 
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" 
<si...@gmail.com>, Brian Lillie/Austin/IBM@IBMUS, "Ryan Baxter" 
<rb...@gmail.com>, 
Cc: Brian Lillie/Austin/IBM@IBMUS, "Xiao Feng Yu" <yu...@cn.ibm.com>, 
"shindig" <de...@shindig.apache.org>
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
> 
>



Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Rich Thompson <ri...@us.ibm.com>.
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:
   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)
   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



From:	Brian Lillie/Austin/IBM@IBMUS
To:	Dan Dumont/Westford/IBM@Lotus, "Stanton Sievers"
            <si...@gmail.com>, Brian Lillie/Austin/IBM@IBMUS, "Ryan
            Baxter" <rb...@gmail.com>,
Cc:	Brian Lillie/Austin/IBM@IBMUS, "Xiao Feng Yu"
            <yu...@cn.ibm.com>, "shindig" <de...@shindig.apache.org>
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
>
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by br...@us.ibm.com.
-----------------------------------------------------------
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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7313
-----------------------------------------------------------

Ship it!


LGTM

- Dan


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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Dan Dumont <dd...@us.ibm.com>.

> On 2012-04-27 18:08:34, Dan Dumont wrote:
> > Committed r1331528
> >

Please close this review.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7314
-----------------------------------------------------------


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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7314
-----------------------------------------------------------

Ship it!


Committed r1331528


- Dan


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
> 
>


Re: Review Request: Update isStale method in HttpResponse to return true when max-age equals 0

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4877/#review7259
-----------------------------------------------------------

Ship it!


LGTM

- Ryan


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
> 
>