Vadim Gritsenko wrote:

Unico Hommes wrote:

Vadim Gritsenko wrote:

Unico Hommes wrote:

Now only one release blocker remains: NPE in AbstractEnvironment.release()



No, not exactly. As I see it, ResourceReader should return Source's mimeType in this particular case, as per:


    public String getMimeType() {
        Context ctx = ObjectModelHelper.getContext(objectModel);
        if (ctx != null) {
            final String mimeType = ctx.getMimeType(source);
            if (mimeType != null) {
                return mimeType;
            }
        }

        return inputSource.getMimeType();
    }

But in this case, it was not able to get mime type of the SitemapSource. SitemapSource, in its turn, was retrieving mimeType from the environment:

344:                this.mimeType = this.environment.getContentType();

But for some reason it is not there. I think that is the problem and it was a valid test case for this problem. Unless I am mistaken... If I'm not, we should revert the removal of test case.




No don't say that! I thought we'd finally gotten rid it. :-(


Ooops. I'm sorry! :)



Never mind. It's not your fault ;-)

You are right though. My conclusion that the test case was no longer valid was premature. It used to be that SitemapSource would just return hard-coded text/xml as its mime type.

Anyway, the problem appears to be that the mime-type is only set on the environment by the processing pipeline during actual processing whereas at the time Reader.getMimeType is called the pipeline has only been setup and prepared.

Now I am wondering whether we can move setting the mime-type from the processing stage to the preparation stage.


Probably we should, if possible.


I should be possible if the issue Luigi raised doesn't hold valid. Should the data stream in the pipeline have any say over its own characteristics? I am not sure. It would be a whole lot easier if we would just mandate it. But then perhaps the Response.setHeader should not even be available to sitemap components, or FOM even. Hmm.


The reason it is currently being done during processing is that in the case of the caching pipeline the mime-type is held in the cached response and the retrieval of the cached response is delayed until the processing stage in order to optimize performance.


If mime-type is always set during pipeline setup, it will be not necessary to store it in the cached response at all. Moreover, current situation is flawed anyway: what about all the other headers? They are all lost on second request. If all headers are set during pipeline setup, we won't have the issue.

Is it really neccessary or desirable to cache the mime-type?


See above.


What happens if the effective mime-type is from the one from the reader definition or the read node declaration (<map:reader class=".." mime-type="text/xml"/> or <map:read src="somewhere" mime-type="text/xml" />) From these examples it follows that the mime-type must be part of the cache-key because if you were to change it in any of the attributes above the previously cached contents should not be served.


Not if it set as described above.


Btw. ResourceReader.getMimeType() method above looks a bit funny to me. The mime-type mapping from the context has priority over the mime-type of the individual source. Surely that should be the other way around?


There were some changes around mime type handling in 2.2...
http://issues.apache.org/bugzilla/show_bug.cgi?id=10277

ResourceReader implementation was not changed, though.


Yes, the conclusion at the end seems not to match the code in ResourceReader.

--
Unico




Reply via email to