----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4874/#review7278 -----------------------------------------------------------
LGTM - BrianLillie On 2012-04-26 18:33:29, Dan Dumont wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4874/ > ----------------------------------------------------------- > > (Updated 2012-04-26 18:33:29) > > > Review request for shindig, Ryan Baxter, Stanton Sievers, and Brian Lillie. > > > Summary > ------- > > So I think this will clean up makeRequest from the client's perspective. > > The implications of this are that if we use the refreshInterval param we > should not return the original response but instead return the response that > has the clobbered cache headers. This is because we are processing the get > as-is and returned cache-control headers need to be valid for what we've > requested. > > There is currently an issue in > org.apache.shindig.gadgets.http.AbstractHttpCache where when building the > response to cache, we clobber the cache-control headers and do not return the > modified response. If these changes listed here for makeRequest make sense, > we can continue and fix this other problem. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > 1329959 > > Diff: https://reviews.apache.org/r/4874/diff > > > Testing > ------- > > Updated tests. All tests pass. > > > Thanks, > > Dan > >
