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 <clecl...@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-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
