You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Grzegorz Kossakowski <gk...@apache.org> on 2007/08/03 11:13:04 UTC
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
vgritsenko@apache.org pisze:
> Author: vgritsenko
> Date: Thu Aug 2 10:45:05 2007
> New Revision: 562199
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=562199
> Log:
> Fix regression introduced in r530406, r532869.
> Update changes document: RC1 is long released.
What kind of regression you had in mind?
At this point I'm not sure how, but your commit breakes servletService components (generator, transformer and serializer) usage. You can
check it accessing this url:
http://localhost:8888/blocks/cocoon-forms-sample/
and you should get this error:
Message: Cannot create consumer source for request that is not POST.
--
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
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
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Grzegorz Kossakowski wrote:
> vgritsenko@apache.org pisze:
>> Author: vgritsenko
>> Date: Thu Aug 2 10:45:05 2007
>> New Revision: 562199
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=562199
>> Log:
>> Fix regression introduced in r530406, r532869.
>> Update changes document: RC1 is long released.
>
> What kind of regression you had in mind?
See [1]:
// Set status code *only* if there is one - do not override status
// code if it was set elsewhere.
It is an error to blindly override any status code which was set on a response
before.
Vadim
[1]
http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/sitemap/SerializeNode.java?r1=532869&r2=562199&pathrev=562199&diff_format=h
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
Posted by Felix Knecht <fe...@apache.org>.
> At this point I'm not sure how, but your commit breakes servletService
> components (generator, transformer and serializer) usage. You can
> check it accessing this url:
> http://localhost:8888/blocks/cocoon-forms-sample/
>
> and you should get this error:
> Message: Cannot create consumer source for request that is not POST.
>
+1
Same error appears also for other samples.
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
Posted by Grzegorz Kossakowski <gk...@apache.org>.
Vadim Gritsenko pisze:
> 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 :)
Ok, thanks for clarification.
>>
>> Oh, and it was me who invented this "clever" code, yes? :)
>
> Sorry, I have no idea who wrote it.
It was rhetoric question ;)
See r531498[1]
>>
>> 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.
Ok.
> 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...
I thought that the code was pretty well commented and annotated.
It's worth to take a look at COCOON-2046[2] because it contains further links that would allow you to understand goals behind this code.
If you have any suggestions about code documentation (something is not commented clearly enough) feel free to share them with me here.
>> 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 ;-)
;-)
[1] http://article.gmane.org/gmane.text.xml.cocoon.cvs/24126
[2] https://issues.apache.org/jira/browse/COCOON-2046
--
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
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
Posted by Vadim Gritsenko <va...@reverycodes.com>.
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
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
Posted by Grzegorz Kossakowski <gk...@apache.org>.
Vadim Gritsenko pisze:
> Grzegorz Kossakowski wrote:
>> vgritsenko@apache.org pisze:
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=562199
>>> Log:
>>> Fix regression introduced in r530406, r532869.
>>> Update changes document: RC1 is long released.
>>
>> At this point I'm not sure how, but your commit breakes servletService
>> components (generator, transformer and serializer) usage.
>
> 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 and not assume that default one is 200. However, I can understand that it would be
harder to fix.
> 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? :)
> Oh, and there is a third problem - I added a FIXME in revision r562800 -
> it looses 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?
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.
> 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] :)
>
> Vadim
>
>
> [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.
Oh, and I can hardly imagine someone implementing charging somebody's credit card and caching at the same time... :-)
[1] https://issues.apache.org/jira/browse/COCOON-2096
--
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
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
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Grzegorz Kossakowski wrote:
> vgritsenko@apache.org pisze:
>> URL: http://svn.apache.org/viewvc?view=rev&rev=562199
>> Log:
>> Fix regression introduced in r530406, r532869.
>> Update changes document: RC1 is long released.
>
> At this point I'm not sure how, but your commit breakes servletService
> components (generator, transformer and serializer) usage.
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.
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 there is a third problem - I added a FIXME in revision r562800 - it
looses original request body once it decides it was a redirect. 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] :)
Vadim
[1] Yes in such cases operation should be idempotent - in perfect world, that is...
Re: Running Samples, Re: svn commit: r562199
Posted by Grzegorz Kossakowski <gk...@apache.org>.
Jörg Heinicke pisze:
> Vadim Gritsenko wrote:
>
>> I got an exception [1]. Any ideas?
Vadim, if you want to see minimal version of Cocoon working *now* you can rebuild it omitting "-P allblocks" option. It should work, then.
> You can try to track that one down as well which actually was already supposed to be fixed [2] ;-)
I wanted to fix this issue but I found that I cannot reproduce it. Seems like XML parser in my configuration does not perform validation. I
have Java 6 on openSUSE 10.2 with standard configuration. I'm not sure how to enable this validation :-(
Vadim, since you are able to get this error it would be great if you could have a look at it. I'll post comment in COCOON-2079 explaining
how to track the actual problem.
--
Grzegorz Kossakowski
http://reflectingonthevicissitudes.wordpress.com/
Re: Running Samples, Re: svn commit: r562199
Posted by Jörg Heinicke <Jo...@gmx.de>.
Vadim Gritsenko wrote:
> I got an exception [1]. Any ideas?
You can try to track that one down as well which actually was already supposed to be fixed [2] ;-)
Joerg
[2] https://issues.apache.org/jira/browse/COCOON-2079
--
GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free SMS.
Alle Infos und kostenlose Anmeldung: http://www.gmx.net/de/go/freemail
Running Samples, Re: svn commit: r562199
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Grzegorz Kossakowski wrote:
> At this point I'm not sure how, but your commit breakes servletService
> components (generator, transformer and serializer) usage. You can check
> it accessing this url:
> http://localhost:8888/blocks/cocoon-forms-sample/
>
> and you should get this error:
> Message: Cannot create consumer source for request that is not POST.
Ok I finally managed to build trunk, can you give me a pointer on how to start
samples? I used:
cd core/cocoon-webapp && mvn -Dorg.apache.cocoon.mode=dev jetty:run
I got an exception [1]. Any ideas?
Vadim
[1]
ERROR [main] (ContextLoader.java:203) - Context initialization failed
org.springframework.beans.factory.BeanDefinitionStoreException: Unable to read
Avalon configuration from 'resource://org/apache/cocoon/cocoon.xconf'.; nested
exception is org.apache.avalon.framework.configuration.ConfigurationException:
Cannot load 'resource://org/apache/cocoon/cocoon.roles' at
jar:file:/Users/vgritsenko/Projects/Apache/cocoon/core/cocoon-webapp/target/cocoon-webapp/WEB-INF/lib/cocoon-core-2.2.0-RC2-SNAPSHOT.jar!/org/apache/cocoon/cocoon.xconf:30:61
Caused by:
org.apache.avalon.framework.configuration.ConfigurationException: Cannot load
'resource://org/apache/cocoon/cocoon.roles' at
jar:file:/Users/vgritsenko/Projects/Apache/cocoon/core/cocoon-webapp/target/cocoon-webapp/WEB-INF/lib/cocoon-core-2.2.0-RC2-SNAPSHOT.jar!/org/apache/cocoon/cocoon.xconf:30:61
at
org.apache.cocoon.core.container.spring.avalon.ConfigurationReader.handleInclude(ConfigurationReader.java:460)
at
org.apache.cocoon.core.container.spring.avalon.ConfigurationReader.parseConfiguration(ConfigurationReader.java:304)
at
org.apache.cocoon.core.container.spring.avalon.ConfigurationReader.convert(ConfigurationReader.java:253)
...
Caused by: org.apache.avalon.framework.configuration.ConfigurationException:
Cannot load
'jar:file:/Users/vgritsenko/Projects/Apache/cocoon/core/cocoon-webapp/target/cocoon-webapp/WEB-INF/lib/cocoon-core-2.2.0-RC2-SNAPSHOT.jar!/org/apache/cocoon/cocoon.roles'
at
jar:file:/Users/vgritsenko/Projects/Apache/cocoon/core/cocoon-webapp/target/cocoon-webapp/WEB-INF/lib/cocoon-core-2.2.0-RC2-SNAPSHOT.jar!/org/apache/cocoon/cocoon.xconf:30:61
at
org.apache.cocoon.core.container.spring.avalon.ConfigurationReader.loadURI(ConfigurationReader.java:508)
at
org.apache.cocoon.core.container.spring.avalon.ConfigurationReader.handleInclude(ConfigurationReader.java:458)
... 62 more
Caused by: org.gjt.xpp.XmlPullParserException: <![DOCTYPE declarations not
supported at line 33 and column 3 seen "...CDATA #REQUIRED\n class
CDATA #REQUIRED\n>\n]"... (parser state UNKNONW_EVENT (-1))
at org.gjt.xpp.sax2.Driver.parse(Driver.java:304)
at
org.apache.avalon.framework.configuration.DefaultConfigurationBuilder.build(DefaultConfigurationBuilder.java:255)
at
org.apache.avalon.framework.configuration.DefaultConfigurationBuilder.build(DefaultConfigurationBuilder.java:224)
at
org.apache.cocoon.core.container.spring.avalon.ConfigurationReader.loadURI(ConfigurationReader.java:506)