You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Carsten Ziegeler <cz...@s-und-n.de> on 2004/07/15 15:03:29 UTC
RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Great work, Sylvain.
Only one question: you also changed the mime-type setting (again):
http://cvs.apache.org/viewcvs/cocoon-2.1/src/java/org/apache/cocoon/componen
ts/pipeline/AbstractProcessingPipeline.java.diff?r1=1.25&r2=1.26
According to bug 10277 and to the votes we did, we only discussed the mime
type of
the reader, not the mime-type of the serializer.
In addition, it seems that you removed one case for the mime-type setting of
the
reader.
Carsten
> -----Original Message-----
> From: sylvain@apache.org [mailto:sylvain@apache.org]
> Sent: Thursday, July 15, 2004 2:50 PM
> To: cocoon-2.1-cvs@apache.org
> Subject: cvs commit:
> cocoon-2.1/src/java/org/apache/cocoon/sitemap
> SitemapComponentSelector.java
>
RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Sylvain Wallez wrote:
>
> _Silently_ whistling? You'll have to teach me this
> interesting technique ;-)
>
No problem, I can tell you how to do it after the fifth (or so) beer at
the next gt.
> Hey folks, look at who went in the way of my refactoring:
> http://marc.theaimsgroup.com/?l=xml-cocoon-cvs&m=108983395802371&w=2
>
Spoil sport :)
Carsten
Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Sylvain Wallez <sy...@apache.org>.
Carsten Ziegeler wrote:
>
>Sylvain Wallez wrote:
>
>
>>I still have some simplifications to do in the treebuilder,
>>such as simplifying role management (actually removing it!)
>>and nodebuilder creation, which will remove more dependencies
>>on the container.
>>
>>Once the first changes where finished and running, I
>>committed quickly because some CVS conflicts where appearing ;-)
>>
>>
>Sure, I will wait with the Fortress migration until you're finished.
>Hmm, CVS conflicts in those files. Who else is touching these?
>
>Carsten (silently whistling)
>
>
_Silently_ whistling? You'll have to teach me this interesting technique ;-)
Hey folks, look at who went in the way of my refactoring:
http://marc.theaimsgroup.com/?l=xml-cocoon-cvs&m=108983395802371&w=2
;-P
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Sylvain Wallez wrote:
>
> I still have some simplifications to do in the treebuilder,
> such as simplifying role management (actually removing it!)
> and nodebuilder creation, which will remove more dependencies
> on the container.
>
> Once the first changes where finished and running, I
> committed quickly because some CVS conflicts where appearing ;-)
>
Sure, I will wait with the Fortress migration until you're finished.
Hmm, CVS conflicts in those files. Who else is touching these?
Carsten (silently whistling)
Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Sylvain Wallez <sy...@apache.org>.
Carsten Ziegeler wrote:
>
>Sylvain Wallez wrote:
>
>
>>Actually, this is a side effect of the removal of OutputComponentSelector: the mimeType given to addReader and addSerializer is the one defined in the sitemap, be it on the particular statement of the default setting of the component declaration. This is implemented in SerializeNodeBuilder and ReadNodeBuilder.
>>
>>Previously, the pipeline had to do the additional job of getting the mimeType from the OutputComponentSelector it none was present on the statement, while conceptually this really was (and is now!) the sole responsibility of the sitemap engine.
>>
>>And if you look at the diff, the same if/else disappeared for the serializer ;-)
>>
>>
>>
>Cool! So actually everything is as it should be and I was just too blind :(
>
>
Hey not easy to track the changes in such a big number of files ;-)
>Thanks Sylvain!
>
>
I still have some simplifications to do in the treebuilder, such as
simplifying role management (actually removing it!) and nodebuilder
creation, which will remove more dependencies on the container.
Once the first changes where finished and running, I committed quickly
because some CVS conflicts where appearing ;-)
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Sylvain Wallez wrote:
>
> Actually, this is a side effect of the removal of
> OutputComponentSelector: the mimeType given to addReader and
> addSerializer is the one defined in the sitemap, be it on the
> particular
> statement of the default setting of the component
> declaration. This is
> implemented in SerializeNodeBuilder and ReadNodeBuilder.
>
> Previously, the pipeline had to do the additional job of getting the
> mimeType from the OutputComponentSelector it none was present on the
> statement, while conceptually this really was (and is now!) the sole
> responsibility of the sitemap engine.
>
> And if you look at the diff, the same if/else disappeared for the
> serializer ;-)
>
Cool! So actually everything is as it should be and I was just too blind :(
Thanks Sylvain!
Carsten
Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Sylvain Wallez <sy...@apache.org>.
Carsten Ziegeler wrote:
>Sylvain Wallez wrote:
>
>
>>Carsten Ziegeler wrote:
>>
>>
>>
>>>Great work, Sylvain.
>>>
>>>
>>>
>>>
>>Thanks. Hard job as there were explicit dependencies to
>>extensions of ComponentSelector (SitemapComponentSelector and
>>OutputComponentSelector) in many places.
>>
>>These extensions were actually used only to store information
>>internal to the sitemap (components mime-types and labels),
>>so I handled this internally (see ProcessorComponentInfo)
>>which allowed the removal of these extensions.
>>
>>
>Sounds good, will look at it.
>
>
>>There still is an ugly hack in ComponentsSelector to manage
>>the parent/child chaining since WrapperServiceSelector and
>>WrapperComponentSelector lack some public access to the
>>wrapped selector. I had no time to locate where is the actual
>>code (is it Avalon or Excalibur) nor start discussions about
>>the patch.
>>
>>
>I think we can leave it as it is. I plan to make the migration
>to Fortress sometime next week. Then this will be changed anyway.
>
>
>
>>>Only one question: you also changed the mime-type setting (again):
>>>
>>>http://cvs.apache.org/viewcvs/cocoon-2.1/src/java/org/apache/
>>>
>>>
>>cocoon/com
>>
>>
>>>ponen
>>>ts/pipeline/AbstractProcessingPipeline.java.diff?r1=1.25&r2=1.26
>>>
>>>According to bug 10277 and to the votes we did, we only
>>>
>>>
>>discussed the
>>
>>
>>>mime type of the reader, not the mime-type of the serializer.
>>>
>>>
>>>
>>>
>>Well, how could the mime-type handling be different for
>>reader and serializer?
>>
>>
>>
>Yes, I totally agree, but we never voted about it and this change
>is mentioned nowhere! So, at least it should be added to the
>updating documentation and in the status file.
>
>
Ok.
>>Additionally, this priorty order seems to me the only one that really makes sense:
>>- first check on the <map:read> or <map:serialize> statement (most specific setting)
>>- then check on the <map:reader> or <map:serializer> component declaration (sitemap-wide setting)
>>- then ask the serializer instance (builtin, system-wide setting)
>>
>>I also wondered why the pipeline raises a ProcessingException if no mime type exist on the serializer. Shouldn't we let the servlet engine decide?
>>
>>
>>
>>> <>In addition, it seems that you removed one case for the mime-type
>>> setting of the reader
>>
>>Sorry, what are you referring to?
>>
>>
>>
>To this change in AbstractProcessingPipeline:
>---------------
>@@ -557,8 +553,7 @@
>
>this.reader.setup(this.processor.getSourceResolver(),environment.getObjectMo
>del(),readerSource,readerParam);
> // Set the mime-type
> // the behaviour has changed from 2.1.x to 2.2 according to bug
>#10277:
>- // MIME type declared on the reader instance
>- // MIME type declared for the reader component
>+ // MIME type declared in the sitemap (instance or declaration,
>in this order)
> // Ask the Reader for a MIME type:
> // A *.doc reader could peek into the file
> // and return either text/plain or application/vnd.msword
>or
>@@ -566,8 +561,6 @@
> // by the server.
> if ( this.readerMimeType != null ) {
> environment.setContentType(this.readerMimeType);
>- } else if ( this.sitemapReaderMimeType != null ) {
>- environment.setContentType(this.sitemapReaderMimeType);
>
> } else {
> final String mimeType = this.reader.getMimeType();
> if (mimeType != null) {
>--------------
>
>
>
>You removed one else-if statement. I haven't looked at all your changes, perhaps you're doing the correct thing somewhere else. I just want to make sure that the implementation is still correct :)
>
>
Actually, this is a side effect of the removal of
OutputComponentSelector: the mimeType given to addReader and
addSerializer is the one defined in the sitemap, be it on the particular
statement of the default setting of the component declaration. This is
implemented in SerializeNodeBuilder and ReadNodeBuilder.
Previously, the pipeline had to do the additional job of getting the
mimeType from the OutputComponentSelector it none was present on the
statement, while conceptually this really was (and is now!) the sole
responsibility of the sitemap engine.
And if you look at the diff, the same if/else disappeared for the
serializer ;-)
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Sylvain Wallez wrote:
>
> Carsten Ziegeler wrote:
>
> > Great work, Sylvain.
> >
> >
>
> Thanks. Hard job as there were explicit dependencies to
> extensions of ComponentSelector (SitemapComponentSelector and
> OutputComponentSelector) in many places.
>
> These extensions were actually used only to store information
> internal to the sitemap (components mime-types and labels),
> so I handled this internally (see ProcessorComponentInfo)
> which allowed the removal of these extensions.
Sounds good, will look at it.
>
> There still is an ugly hack in ComponentsSelector to manage
> the parent/child chaining since WrapperServiceSelector and
> WrapperComponentSelector lack some public access to the
> wrapped selector. I had no time to locate where is the actual
> code (is it Avalon or Excalibur) nor start discussions about
> the patch.
I think we can leave it as it is. I plan to make the migration
to Fortress sometime next week. Then this will be changed anyway.
>
> >Only one question: you also changed the mime-type setting (again):
> >
> >http://cvs.apache.org/viewcvs/cocoon-2.1/src/java/org/apache/
> cocoon/com
> >ponen
> >ts/pipeline/AbstractProcessingPipeline.java.diff?r1=1.25&r2=1.26
> >
> >According to bug 10277 and to the votes we did, we only
> discussed the
> >mime type of the reader, not the mime-type of the serializer.
> >
> >
>
> Well, how could the mime-type handling be different for
> reader and serializer?
>
Yes, I totally agree, but we never voted about it and this change
is mentioned nowhere! So, at least it should be added to the
updating documentation and in the status file.
> Additionally, this priorty order seems to me the only one
> that really makes sense:
> - first check on the <map:read> or <map:serialize> statement
> (most specific setting)
> - then check on the <map:reader> or <map:serializer>
> component declaration (sitemap-wide setting)
> - then ask the serializer instance (builtin, system-wide setting)
>
> I also wondered why the pipeline raises a ProcessingException
> if no mime type exist on the serializer. Shouldn't we let the
> servlet engine decide?
>
> >In addition, it seems that you removed one case for the
> mime-type setting of the reader.
> >
> >
>
> Sorry, what are you referring to?
>
To this change in AbstractProcessingPipeline:
---------------
@@ -557,8 +553,7 @@
this.reader.setup(this.processor.getSourceResolver(),environment.getObjectMo
del(),readerSource,readerParam);
// Set the mime-type
// the behaviour has changed from 2.1.x to 2.2 according to bug
#10277:
- // MIME type declared on the reader instance
- // MIME type declared for the reader component
+ // MIME type declared in the sitemap (instance or declaration,
in this order)
// Ask the Reader for a MIME type:
// A *.doc reader could peek into the file
// and return either text/plain or application/vnd.msword
or
@@ -566,8 +561,6 @@
// by the server.
if ( this.readerMimeType != null ) {
environment.setContentType(this.readerMimeType);
- } else if ( this.sitemapReaderMimeType != null ) {
- environment.setContentType(this.sitemapReaderMimeType);
} else {
final String mimeType = this.reader.getMimeType();
if (mimeType != null) {
--------------
You removed one else-if statement. I haven't looked at all your changes,
perhaps you're doing
the correct thing somewhere else. I just want to make sure that the
implementation
is still correct :)
Carsten
Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Posted by Sylvain Wallez <sy...@apache.org>.
Carsten Ziegeler wrote:
> Great work, Sylvain.
>
>
Thanks. Hard job as there were explicit dependencies to extensions of
ComponentSelector (SitemapComponentSelector and OutputComponentSelector)
in many places.
These extensions were actually used only to store information internal
to the sitemap (components mime-types and labels), so I handled this
internally (see ProcessorComponentInfo) which allowed the removal of
these extensions.
There still is an ugly hack in ComponentsSelector to manage the
parent/child chaining since WrapperServiceSelector and
WrapperComponentSelector lack some public access to the wrapped
selector. I had no time to locate where is the actual code (is it Avalon
or Excalibur) nor start discussions about the patch.
>Only one question: you also changed the mime-type setting (again):
>
>http://cvs.apache.org/viewcvs/cocoon-2.1/src/java/org/apache/cocoon/componen
>ts/pipeline/AbstractProcessingPipeline.java.diff?r1=1.25&r2=1.26
>
>According to bug 10277 and to the votes we did, we only discussed the mime
>type of the reader, not the mime-type of the serializer.
>
>
Well, how could the mime-type handling be different for reader and
serializer?
Additionally, this priorty order seems to me the only one that really
makes sense:
- first check on the <map:read> or <map:serialize> statement (most
specific setting)
- then check on the <map:reader> or <map:serializer> component
declaration (sitemap-wide setting)
- then ask the serializer instance (builtin, system-wide setting)
I also wondered why the pipeline raises a ProcessingException if no mime
type exist on the serializer. Shouldn't we let the servlet engine decide?
>In addition, it seems that you removed one case for the mime-type setting of the reader.
>
>
Sorry, what are you referring to?
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }