Bug 54379 - Implement support for post-construct and pre-destroy elements in web.xml
Summary: Implement support for post-construct and pre-destroy elements in web.xml
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.34
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-07 04:07 UTC by Konstantin Kolinko
Modified: 2013-01-10 11:48 UTC (History)
0 users



Attachments
Patch proposal + tests (40.85 KB, patch)
2013-01-10 07:32 UTC, Violeta Georgieva
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2013-01-07 04:07:15 UTC
There appears to be a feature that web[-fragment].xml file can contain such elements as <post-construct> and <pre-destroy>, and they are treated as equivalents to @PostConstruct and @PreDestroy annotations being present on the classes mentioned in them.

This feature is
1. mentioned in the Java EE 6 Platform Specification, chapter "EE.5.2.5 Annotations and Injection" [1]
2. mentioned in chapter "8.2.3 Assembling the descriptor from web.xml, webfragment.xml and annotations",
see points "k." and "l." on page 81 (103/230) of servlet-3_0-mrel-spec.pdf

[1] javaee_platform-6_0-fr-spec.pdf
http://jcp.org/aboutJava/communityprocess/final/jsr316/index.html

An example can be found in Jetty wiki:
http://wiki.eclipse.org/Jetty/Feature/Annotations#Lifecycle_callbacks:_PostConstruct_PreDestroy

Searching by the tag names, I do not see any code in the current trunk that processes those XML elements.
Comment 1 Violeta Georgieva 2013-01-07 20:27:41 UTC
Hi,

I'm going to work on this.

Regards
Violeta
Comment 2 Mark Thomas 2013-01-09 12:02:53 UTC
Could you provide a progress update please. This is (currently) the only remaining issue that needs fixing before the 7.0.35 tag.
Comment 3 Violeta Georgieva 2013-01-09 12:10:02 UTC
I'm preparing the test cases just now.
I'll be ready till end of the day is that OK?
Comment 4 Mark Thomas 2013-01-09 12:16:49 UTC
That sounds great. Thanks.
Comment 5 Violeta Georgieva 2013-01-10 07:32:02 UTC
Created attachment 29840 [details]
Patch proposal + tests

Hi,

Please find attached a patch proposal and tests.

Important notes to the implementation:
1. Only one method per given event is allowed per class.
2. As per javaee_6.xsd if 'lifefycle-callback-class' element is not specified then the component that contains definitions for post-construct or pre-destroy should be used. For web applications we can specify this functionality only per web application and not per web component so in the implementation if the class is not specified then IllegalArgumentException will be thrown.
3. If method for a given event is specified in deployment descriptor and with annotation - the definition in the descriptor will be used.
4. If there are definitions in web.xml and in web-fragment.xml then web-fragment.xml will be ignored.
5. If there are no definitions in the web.xml and we have definitions in web fragments then they will be merged. However if there are conflicts - more than one method per given event per class - the merge operation will not succeed.
6. If the method specified in the deployment descriptor cannot be found in the class then IllegalArgumentException will be thrown.

I'm looking forward to your comment.

Regards
Violeta
Comment 6 Mark Thomas 2013-01-10 11:43:07 UTC
Many thanks for the patch. That was a much bigger job than I realised.

I have applied the patch to trunk with just a few minor tweaks:
- fixed various IDE / Checkstyle warnings (Java 7 <>, whitespace etc)
- removed unnecessary use of this (just a style thing)
- removed syncs from new maps in StandardContext (deployment is always single threaded for an app)
- renamed a test class for consistency
- correct a few indents
- Used Tomcat.enableNaming() rather than system property

I'll back-port to 7.0.x shortly.
Comment 7 Mark Thomas 2013-01-10 11:48:06 UTC
Fixed in 7.0.x and will be included in 7.0.35 onwards. Thanks again for the patch.