cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carsten Ziegeler" <cziege...@s-und-n.de>
Subject RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/sitemap SitemapComponentSelector.java
Date Thu, 15 Jul 2004 14:21:28 GMT
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


Mime
View raw message