Thanks for your fast feedbacks Christopher,
I updated the patch proposed on Bugzilla 48998 to include your advice
to limit the number of null checks. The implementation I found is
slightly different than your proposal but the idea remain the same.
Please note that I only modified the patch and not yet
ExpiresFilter.java on the google-code project as my priority is the
Tomcat proposal.
Regarding the simplification of generating expiration headers just
after 'response.setContentType()' is called rather than after the
first write in the response body, I didn't follow this way because
applications could add/set 'Cache-Control' and/or 'Expires' header
after invoking 'response.setContentType()'.
My understanding is that the filter must work after
'setContentType()', 'set/addHeader("Cache-Control", ...)' and
'set/addHeader("Expires", ...)' have been called but we don't know
which ones will be called and in which order they will be called.
This is the reason why I unfortunately added the complexity to trap
the 'onStartWriteResponseBody' event.
Cyrille
On Mon, Mar 29, 2010 at 3:20 PM, Christopher Schultz
<ch...@christopherschultz.net> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Cyrille,
>
> On 3/26/2010 12:43 PM, Cyrille Le Clerc wrote:
> > I have proposed with bugzilla 48998 a port of Apache mod_expires in
> > Java as ExpiresFilter Servlet Filter.
>
> Cool.
>
> > I detailed a standalone version of this filter on
> > http://code.google.com/p/xebia-france/wiki/ExpiresFilter . Moreover, I
> > tried my best to provide very detailed javadocs and docs (in filter.html).
> >
> > The proposed contribution is slightly different than the standalone
> > one because it uses Tomcat logging, few Servlet 3 enhancements and
> > Tomcat engine in the test cases.
>
> I read-through your code on code.google.com and I can see a couple areas
> where I think improvements might be made:
>
> - - In getExpirationDate, you check for the local 'configuration' being
> null several times in a row. In each case, the configuration may be set
> given a different 'level' of configuration. If the configuration gets
> set, several checks must still be made to see if it is null. You could
> mail out early and avoid some of these checks like this:
>
> if(configuration == null) {
> configuration = ...;
>
> if(configuration == null) {
> // try another strategy
> configuration = ...;
>
> if(configuration == null) {
> ...
> }
> }
> }
>
> I think can save a bit of CPU time without much in the way of code
> complexity.
>
> - - You might be able to wrap the Response class and check for the setting
> of the Content-Type header, instead of wrapping the response in order to
> intercept the first buffer flush to the client. Do you think that would
> work? It certainly would remove a lot of complexity from your code.
>
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkuwqQwACgkQ9CaO5/Lv0PDdwgCgrSHwmgUTDWybmk6/G1+AqNzY
> kCQAn0zVrQBARihaoQkfBJRtKYkjvsjs
> =kBWG
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
