Fix for shindig bug 1346- do not cache 304 responses from origin server

Do not cache server response if the response is a 304.

[Paul Lindner, 2010-06-08 12:17:40]

seems fine for a first cut.  I assume the etag etal support comes later...

[johnfargo, 2010-06-08 13:05:30]

Yes... but it's also orthogonal IMO.

The point here is that, regardless the fetcher's behavior, we shouldn't
cache a 304 -- there's no guarantee that the Shindig fetcher's behavior is
tied to the original request (esp. request headers such as If-None-Match et
al). Stated another way, this is Shindig-as-fetcher-to-site rather than
client-as-fetcher-to-Shindig.

Etag support lives more on the "front end" of the proxying pipeline,
utilizing state that's under the control of the shindig back-end.

--j

On Tue, Jun 8, 2010 at 5:47 PM, <lindner@inuus.com> wrote:

> seems fine for a first cut.  I assume the etag etal support comes
> later...
>
>
>
> http://codereview.appspot.com/1605041/show
>

[Paul Lindner, 2010-06-08 13:15:48]

/me nods

We'll still need to fix the processing so we don't respond with 304s to the
client and deal with the situation where we get a 304 and the cache entry is
invalidated below us -- again that's follow on work.

So let's go ahead and commit this then..

On Tue, Jun 8, 2010 at 6:05 AM, John Hjelmstad <johnfargo@gmail.com> wrote:

> Yes... but it's also orthogonal IMO.
>
> The point here is that, regardless the fetcher's behavior, we shouldn't
> cache a 304 -- there's no guarantee that the Shindig fetcher's behavior is
> tied to the original request (esp. request headers such as If-None-Match et
> al). Stated another way, this is Shindig-as-fetcher-to-site rather than
> client-as-fetcher-to-Shindig.
>
> Etag support lives more on the "front end" of the proxying pipeline,
> utilizing state that's under the control of the shindig back-end.
>
> --j
>
> On Tue, Jun 8, 2010 at 5:47 PM, <lindner@inuus.com> wrote:
>
>> seems fine for a first cut.  I assume the etag etal support comes
>> later...
>>
>>
>>
>> http://codereview.appspot.com/1605041/show
>>
>
>

[johnfargo, 2010-06-10 15:31:17]

Sounds good re: committing this.

Re: the remainder, I believe I agree but if I'm not mistaken the underlying
issue is whether we want to support being a "full" HTTP client in the
ETags/If-Modified-Since/etc sense. The issue that gave rise to this behavior
is that an HttpFetcher was modified to include such behavior. Without them,
we won't get 304s in the first place. Should we support this in a more
explicit way?

Let me know if I'm missing the motivation behind the use cases you describe.

Cheers,
--j

On Tue, Jun 8, 2010 at 5:15 PM, Paul Lindner <lindner@inuus.com> wrote:

> /me nods
>
> We'll still need to fix the processing so we don't respond with 304s to the
> client and deal with the situation where we get a 304 and the cache entry is
> invalidated below us -- again that's follow on work.
>
> So let's go ahead and commit this then..
>
>
> On Tue, Jun 8, 2010 at 6:05 AM, John Hjelmstad <johnfargo@gmail.com>wrote:
>
>> Yes... but it's also orthogonal IMO.
>>
>> The point here is that, regardless the fetcher's behavior, we shouldn't
>> cache a 304 -- there's no guarantee that the Shindig fetcher's behavior is
>> tied to the original request (esp. request headers such as If-None-Match et
>> al). Stated another way, this is Shindig-as-fetcher-to-site rather than
>> client-as-fetcher-to-Shindig.
>>
>> Etag support lives more on the "front end" of the proxying pipeline,
>> utilizing state that's under the control of the shindig back-end.
>>
>> --j
>>
>> On Tue, Jun 8, 2010 at 5:47 PM, <lindner@inuus.com> wrote:
>>
>>> seems fine for a first cut.  I assume the etag etal support comes
>>> later...
>>>
>>>
>>>
>>> http://codereview.appspot.com/1605041/show
>>>
>>
>>
>

[johnfargo, 2010-06-10 15:43:43]

(patch committed as r953355, thanks!)

On Thu, Jun 10, 2010 at 7:30 PM, John Hjelmstad <johnfargo@gmail.com> wrote:

> Sounds good re: committing this.
>
> Re: the remainder, I believe I agree but if I'm not mistaken the underlying
> issue is whether we want to support being a "full" HTTP client in the
> ETags/If-Modified-Since/etc sense. The issue that gave rise to this behavior
> is that an HttpFetcher was modified to include such behavior. Without them,
> we won't get 304s in the first place. Should we support this in a more
> explicit way?
>
> Let me know if I'm missing the motivation behind the use cases you
> describe.
>
> Cheers,
> --j
>
>
> On Tue, Jun 8, 2010 at 5:15 PM, Paul Lindner <lindner@inuus.com> wrote:
>
>> /me nods
>>
>> We'll still need to fix the processing so we don't respond with 304s to
>> the client and deal with the situation where we get a 304 and the cache
>> entry is invalidated below us -- again that's follow on work.
>>
>> So let's go ahead and commit this then..
>>
>>
>> On Tue, Jun 8, 2010 at 6:05 AM, John Hjelmstad <johnfargo@gmail.com>wrote:
>>
>>> Yes... but it's also orthogonal IMO.
>>>
>>> The point here is that, regardless the fetcher's behavior, we shouldn't
>>> cache a 304 -- there's no guarantee that the Shindig fetcher's behavior is
>>> tied to the original request (esp. request headers such as If-None-Match et
>>> al). Stated another way, this is Shindig-as-fetcher-to-site rather than
>>> client-as-fetcher-to-Shindig.
>>>
>>> Etag support lives more on the "front end" of the proxying pipeline,
>>> utilizing state that's under the control of the shindig back-end.
>>>
>>> --j
>>>
>>> On Tue, Jun 8, 2010 at 5:47 PM, <lindner@inuus.com> wrote:
>>>
>>>> seems fine for a first cut.  I assume the etag etal support comes
>>>> later...
>>>>
>>>>
>>>>
>>>> http://codereview.appspot.com/1605041/show
>>>>
>>>
>>>
>>
>