You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Cyrille Le Clerc <cl...@apache.org> on 2010/03/29 20:32:45 UTC
Re: Proposal : port mod_expires in java as ExpiresFilter Servlet
Filter
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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Proposal : port mod_expires in java as ExpiresFilter Servlet
Filter
Posted by Cyrille Le Clerc <cl...@apache.org>.
Hello Christopher,
I imagine the project crew has been very busy these days. Do you
have feedbacks on the adaptations and explanations I did ? Is there
anything I could iterate on ?
To rephrase my previous email :
* I reduced the number of comparison against 'nulls' as you suggested.
I did this introducing a method
'isEligibleToExpirationHeaderGeneration(request)' with several
intermediate 'return' statements ; I found it easier to code than to
use nested if/else statements.
* The reason why I introduced the complexity of trapping the
'onStartWriteResponseBody' event is that I tried my best to implement
in ExpiresFilter the same behavior as in Apache Httpd mod_expires.
Cyrille
On Mon, Mar 29, 2010 at 8:32 PM, Cyrille Le Clerc <cl...@apache.org> wrote:
>
> 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-unsubscribe@tomcat.apache.org
> > For additional commands, e-mail: users-help@tomcat.apache.org
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org