Bug 54240 - Configurable system level tagPlugins.xml
Summary: Configurable system level tagPlugins.xml
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Jasper (show other bugs)
Version: trunk
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-04 07:31 UTC by Sheldon Shao
Modified: 2013-01-10 09:33 UTC (History)
0 users



Attachments
Patch for TagPluginManager.java (1.14 KB, patch)
2012-12-04 07:31 UTC, Sheldon Shao
Details | Diff
Patch for TagPluginManager.java (2.47 KB, patch)
2012-12-10 08:00 UTC, Sheldon Shao
Details | Diff
A TagPlugin for testing (1.18 KB, text/plain)
2012-12-10 08:01 UTC, Sheldon Shao
Details
A tag for testing (1.07 KB, text/plain)
2012-12-10 08:01 UTC, Sheldon Shao
Details
Test case fo TagPluginManager (2.69 KB, text/plain)
2012-12-10 08:02 UTC, Sheldon Shao
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sheldon Shao 2012-12-04 07:31:23 UTC
Created attachment 29687 [details]
Patch for TagPluginManager.java

Currently, tagPlugins.xml can only be put in a WAR.
However, if the configuration can be put into a web framework, so web application don't need to care about this.
Comment 1 Mark Thomas 2012-12-04 08:42:30 UTC
Comment on attachment 29687 [details]
Patch for TagPluginManager.java

No objections in principle. 

Some comments on the proposed solution:
1. I don't see a need for a system property here. Servlet context initialization parameters are a better choice.
2. No documentation.
3. Patch doesn't follow Tomcat coding standards.
Comment 2 Konstantin Kolinko 2012-12-04 13:03:35 UTC
For this implementation to work, one would need to put the configuration file into classpath. I think that is not a good place for a configuration file.


If it were implemented via context initialization parameters or parameters of JspServlet, I wonder how it will behave during offline generation of JSPs (aka the jspc tool of Jasper). Some testing is needed.
Comment 3 Sheldon Shao 2012-12-04 14:07:09 UTC
TagPlugin is good for making tag libraris more efficently. The target for tag libraries is to share between multiple web applications. Same as TagPlugin.

Imagine that one enterprise has many web applications. It's better to share same tagPlugins & tagPlugins.xml for all applications. So that all applications can benifit from it. 

Currently the solution is putting the tagPlugins.xml under WEB-INF.
It is not good for shareing this tagPlugins.xml with other applications.

Now the solution still supports tagPlugins.xml under WEB-INF. Instead, it also supports an system level tagPlugins.xml. This tagPlugins.xml might be packaged with those implementations of TagPlugin.  

I think TagPlugin implementations must provide such kind of configuration file. That's the idea of Zero-Configuration for web applications.
Comment 4 Konstantin Kolinko 2012-12-04 14:38:15 UTC
(In reply to comment #3)

Your way goes to multiple tag libraries. If each one is self-contained, it could configure itself, without a global tagPlugin file.

I do not know whether API is available in TagPluginManager, but there are already hooks to plug libraries in a container:
a) support for a Listener in Tag Library Descriptor (TLD) file
b) ServletContainerInitializer in Servlet 3.0 specification
Comment 5 Sheldon Shao 2012-12-05 07:22:30 UTC
Yes. Maybe it needs separated tagPlugins.xml for every tld file.

So the solution can be like this,
1. Check all resources in classpath with name "META-INF/jasper/tagPlugins.xml"
2. Parse them one by one and put all the tagPlugins into tagPluginManager.
3. Load tagPlugins configurations from "WEB-INF/tagPlugins.xml" if it existed.

With this new solution. TagPlugin declared in "META-INF/jasper/tagPlugins.xml" can be override by WebApp.  Tag library provider can also provide its own tagPlugins configurations for tag library.

What's your opinion ?

(In reply to comment #4)
> (In reply to comment #3)
Comment 6 Sheldon Shao 2012-12-10 08:00:42 UTC
Created attachment 29737 [details]
Patch for TagPluginManager.java
Comment 7 Sheldon Shao 2012-12-10 08:01:32 UTC
Created attachment 29738 [details]
A TagPlugin for testing
Comment 8 Sheldon Shao 2012-12-10 08:01:56 UTC
Created attachment 29739 [details]
A tag for testing
Comment 9 Sheldon Shao 2012-12-10 08:02:23 UTC
Created attachment 29740 [details]
Test case fo TagPluginManager
Comment 10 Sheldon Shao 2012-12-10 08:04:48 UTC
Hi Mark,

I made a little changes for Konstantin's concern. And refined the code and uploaded the test case.

Please take a look.

Thanks,

Sheldon
Comment 11 Mark Thomas 2013-01-10 08:46:15 UTC
I like the idea of using the existing mechanisms (such as ServletContainerInitializer) but that would require other changes since:
- TagPluginManager is not created at the point where the SCI runs
- There is no mechanism to expose the TagPluginManager to the SCI

Given that changes would have to be made and that Tag plug-ins are a Tomcat specific extension, I don't have an issue with a Tomcat specific mechanism to configure these plug-ins. I like the META-INF/japser approach as it means a plug-in JAR can be dropped into CATALINA_BASE/lib and be used by all web applications on that instance.

I'll be working on applying this patch for 7.0.35.
Comment 12 Mark Thomas 2013-01-10 09:33:36 UTC
Thanks for the patch and test case.

This has been applied to trunk and 7.0.x and will be included in 7.0.35 onwards.