Mailing-List: contact dev-help@cocoon.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cocoon.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
Message-ID: <46B68B94.5000003@reverycodes.com>
Date: Sun, 05 Aug 2007 22:46:44 -0400
From: Vadim Gritsenko <vadim@reverycodes.com>
User-Agent: Thunderbird 2.0.0.5 (Macintosh/20070716)
MIME-Version: 1.0
To: dev@cocoon.apache.org
Subject: Re: svn commit: r562199 - in
 /cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl:
 pom.xml src/changes/changes.xml
 src/main/java/org/apache/cocoon/components/treeprocessor/sitemap/SerializeNode.java
References: <20070802174506.3E1C11A981C@eris.apache.org>
 <46B2F1A0.7030104@apache.org> <46B53C77.6070102@reverycodes.com>
 <46B5AD51.4050104@apache.org>
In-Reply-To: <46B5AD51.4050104@apache.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Grzegorz Kossakowski wrote:
> Vadim Gritsenko pisze:
>> The problem is - was - two fold. First, BlockCallHttpServletResponse 
>> did not had a default status code. So when block call is complete and 
>> chosen servlet did not set any status code - which it can do - 
>> BlockCallHttpServletResponse was returning 0 instead of 200. Revision 
>> r562802 fixes this.
> 
> I think that Cocoon should *always* set status code

Not entirely true. It is perfectly fine for servlet application to not set any 
status code at all. Container will assume 200 and move one. And final HTTP 
response always will have some status set.

But since Cocoon here plays a role of container, it should perform container's 
functions - namely, BlockCallHttpServletResponse should now have a default code 
of 200, *not* 0 - to handle scenario above.


> and not assume that 
> default one is 200. However, I can understand that it would be harder to 
> fix.

Nothing's to fix anymore, see explanation above :)


>> Second problem is in ServletSource - it was tripping over 0 status 
>> code returned by BlockCallHttpServletResponse. It assumes that if 
>> status is not 200, it should be 304 - how naive :)
>>
>>   if (servletConnection.getResponseCode() != HttpServletResponse.SC_OK) {
>>     //most probably, servlet returned 304 (not modified) and we need 
>> to perform second request to get data
> 
> Oh, and it was me who invented this "clever" code, yes? :)

Sorry, I have no idea who wrote it.


>> Oh, and there is a third problem - I added a FIXME in revision r562800 
>> - it loses original request body once it decides it was a redirect.
> 
> You are of course right! Thanks for spotting this issue. What strikes me 
> is word "redirect". What do you mean by redirect in this case?

Sorry, above "redirect" should read "not modified". BTW, are you setting 
If-Modified-Since request header anywhere? ...... Yes I see that you do.


> I was going to fix COCOON-2096 so could also have a look at this. 
> However, I'm already busy so if want to fix them both (they are rather 
> related, so it makes sense to fix both at once) just let me know so we 
> do not duplicate the effort.

Honestly, I got no idea what this code does :) So I'm not sure what I should be 
fixing :) Having said that... *If* I get another chunk of time... I might look 
into it...


>> Actually I don't think we can allow it to perform POST second time at 
>> all. Just imagine it was somebody's credit card to be charged $999 - 
>> it won't be good at all [1] :)
>>
>>
>> [1] Yes in such cases operation should be idempotent - in perfect 
>> world, that is...
> 
> It was a design decision to assume that POST in this cases will be 
> idempotent. Moreover, this second request would be sent only if HTTP 
> HEAD returned 304, or at least, it was what the code supposed to do. If 
> you don't want this behaviour because you think it's unsafe just throw 
> whole caching away by choosing noncaching pipeline.

My first reaction was - "it's dangerous". I'd have to look deeper into it to say 
if it's really not as dangerous as my first thought :)


> Oh, and I can hardly imagine someone implementing charging somebody's 
> credit card and caching at the same time... :-)

Caching would be handy if your clientèle is a bunch of lemmings ;-)

Vadim


> [1] https://issues.apache.org/jira/browse/COCOON-2096