You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Jeremy Boynes <jb...@apache.org> on 2013/07/08 07:38:54 UTC

Re: Jasper improvements/XML handling

On Jun 25, 2013, at 10:44 PM, Jeremy Boynes <jb...@apache.org> wrote:

> I have been thinking about improvements to Jasper as well around better support for Servlet 3.0 concepts. One area would be decoupling it from Tomcat, bootstrapping using an SCI as hinted in ContextConfig. I'd also be interested in improving the Ant task as well, such as support for pre-compiling a separate package that would be treated as a web fragment (including web.xml-less pre-compilation, generating a web-fragment.xml rather than a web.xml snippet or potentially eliminating the XML entirely if the generated code can be annotated with @WebServlet).

I started on this and ended up with two patches that are converging. For 55166 (which started out somewhere different), I have a patch to update local entity handling in Catalina's XML parsing; for 53737, I started to have Jasper's translator rely only on the ServletContext and not need to parse merged web.xml XML passed as a ServletContext attribute. These are converging in a few areas.

Jasper's descriptor parser (used for web.xml and TLDs) can only validate DTD-based documents. I opened 55212 about adding it. Such a fix could benefit from using the same resolver and cached copies that Catalina does. This is less of an issue if Jasper uses the ServletContext as it could rely on the container to parse/validate web.xml. It would still be needed if we wanted JspC to validate files standalone but there is a separate ant task that can do that.

Another area of convergence is in parsing TLDs: currently both Catalina and Jasper do it, differently. I believe Catalina only reads them to detect the Listeners they define and if that's the case we could perhaps add a SCI to Jasper that handled all TLD functionality including registering the listeners programmatically (at least that's what Servlet 8.3 implies) . That would remove Catalina's dependency on web-jsptaglibrary*.xsd and on TLD-related classes. To support validation though we would need to add the XSD-based validation in Jasper.

Finally, Jasper parses these documents first into DOM and the into its own version based on TreeNode (this is separate from the parse tree used for actual JSPs). Jasper already depends on tomcat-util for the JarScanner so could potentially use Digester for this in the same way Catalina does. It would simplify the fragment merge logic in JspC if it could reuse what was in Catalina's WebXml (something I'm hacking around in 53737 right now).  Alternatively, we could still eliminate the intermediary DOM and TreeNode forms and use SAX or StAX to parse directly into Jasper's objects - especially for TLDs where it is using the spec's classes (TagLibraryInfo and TagInfo) as the model.

I'm planning on merging these two patches and pressing on with a converged solution, but probably won't be able to do much until next weekend.

Cheers
Jeremy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements/XML handling

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Jeremy,

A little late returning to the party...

On 7/8/13 1:38 AM, Jeremy Boynes wrote:
> Finally, Jasper parses these documents first into DOM and the into
> its own version based on TreeNode (this is separate from the parse
> tree used for actual JSPs). Jasper already depends on tomcat-util for
> the JarScanner so could potentially use Digester for this in the same
> way Catalina does. It would simplify the fragment merge logic in JspC
> if it could reuse what was in Catalina's WebXml (something I'm
> hacking around in 53737 right now).  Alternatively, we could still
> eliminate the intermediary DOM and TreeNode forms and use SAX or StAX
> to parse directly into Jasper's objects - especially for TLDs where
> it is using the spec's classes (TagLibraryInfo and TagInfo) as the
> model.

Any time you can use SAX instead of DOM without making the code too
complicated, you should.

-chris


Re: Jasper improvements - web resources and TLDs

Posted by Violeta Georgieva <mi...@gmail.com>.
2013/8/12 Mark Thomas wrote:
>
>
> I'm wondering about adding enableWebSockets() and enableJSPs() methods
> to Tomcat which would automatically add the relevant SCI to any Context
> created via one of the addContext(...) methods. Thoughts?
>

+1

Re: Jasper improvements - web resources and TLDs

Posted by Jeremy Boynes <jb...@apache.org>.
On Aug 12, 2013, at 8:40 AM, Mark Thomas <ma...@apache.org> wrote:

> On 12/08/2013 16:20, Jeremy Boynes wrote:
> 
> I'm wondering about adding enableWebSockets() and enableJSPs() methods
> to Tomcat which would automatically add the relevant SCI to any Context
> created via one of the addContext(...) methods. Thoughts?

I'm torn on this. On the up side, this would be a simple way to enable a specific feature. On the down side it would mean Catalina would need to know about features' SCIs which is adding coupling. An embedder would need to have added the JARs for the feature to the classpath at some point so how about just adding a loadServletContainerInitializers() method which would trigger the same discovery process ContextConfig was using?

--
Jeremy


Re: Jasper improvements - web resources and TLDs

Posted by Mark Thomas <ma...@apache.org>.
On 12/08/2013 16:20, Jeremy Boynes wrote:
> On Aug 12, 2013, at 7:46 AM, Mark Thomas <ma...@apache.org> wrote:
> 
>> On 10/08/2013 23:20, Jeremy Boynes wrote:
>>> On Jul 22, 2013, at 10:10 AM, Mark Thomas <ma...@apache.org>
>>> wrote:
>>> 
>>>> On 21/07/2013 18:36, Jeremy Boynes wrote: ...
>>>>> In the meantime, I'm going to look at refactoring the TLD
>>>>> support as described above.
>>>> 
>>>> Nice. See my note about TLD processing in the TOMCAT-NEXT.txt
>>>> file in the root on trunk.
>> 
>> Don't forget the above :)
> 
> Not forgotten, just a work in progress :)
> 
>> 
>>> I've taken the first step on this in 
>>> http://svn.apache.org/r1512826 which moves the TLD processing
>>> from TldConfig into Jasper's ServletContainerInitializer. There
>>> are a couple of implications for this: 1) Jasper's SCI needs to
>>> be activated to use TLDs This should cause no impact when
>>> ContextConfig is used to configure a web application in the
>>> normal way is Jasper is on the class path. When using Context
>>> directly in embedded mode, JasperInitializer now need to be added
>>> to enable JSP functionality which will automatically TLD
>>> processing to add listeners. Previously, TLDs would be scanned
>>> automatically unless the processTld property on StandardContext
>>> was explicitly set false. IOW, TLDs will only be processed if JSP
>>> functionality is added, and don't need to be turned off if it is
>>> not.
>> 
>> Isn't the SCI discovered if Jasper is on the classpath when running
>> in embedded mode? If not, do we know why not?
> 
> The SCI will be discovered if the embedder adds an application as
> that runs ContextConfig which does the scan. However, if they
> construct the context directly (without running ContextConfig) they
> will need to make sure the SCI is added to use JSP functionality.
> This is slightly different to now where TldConfig would scan for and
> add listeners from TLDs even if Jasper was not on the classpath
> (TldConfig would be added by StandardContext's initInternal method
> unless explicitly disabled by setting processTlds false). IMO, the
> new behaviour maintains a cleaner separation as it means embedders
> just using Servlet functionality are not impacted by JSP-related
> features.
> 
> There was one test case where we added a context directly but used
> JSPs. I had to add the SCI for that one. All other tests using the
> embedded server were fine.

Thanks for clarifying this.

I'm wondering about adding enableWebSockets() and enableJSPs() methods
to Tomcat which would automatically add the relevant SCI to any Context
created via one of the addContext(...) methods. Thoughts?


>>> 2) As part of the change for processTlds I deprecated the
>>> operations on Context to set/get TLD processing properties (for 
>>> validation and namespace) as they are can't be used by Jasper's
>>> SCI. If there are no objections to an API change at this stage I
>>> will go ahead and remove them completely. XML validation for TLDs
>>> can now be controlled via the 
>>> "org.apache.jasper.validateDescriptors" initParam for the
>>> ServletContext; namespace processing is always on as we require
>>> >3.0 (due to the SCI etc.) which uses namespaces anyway
>> 
>> Remove them. At this point in Tomcat 8 we should be removing
>> anything that is no longer used.
> 
> Will do. When I do that I'll also add the "removed in 8" deprecation
> warnings to tc7.0.x.

Great.

>> On a related note, I am now seeing a number of warnings about using
>> a deprecated API. It looks like this is because Jasper is still
>> using the old code. What is the timescale for switching Jasper to
>> use the SCI?
> 
> I would estimate the next week or so depending on real-world
> commitments (aka work). I can remove the deprecation tags in trunk if
> that would help.

No need to remove the deprecation warnings. I would like to get
8.0.0-RC2 tagged in about the same sort of timeframe (I have some
non-blocking IO issues to fix first) and it would be good although not
necessary to get this finished by then.

>>> 3) TldScanner parses TLDs as it goes creating a cache of parsed
>>> values keyed off TLD URI. It should probably create a second 
>>> mapping from "TLD resource path" to the parse result in order to
>>> handle taglib directives that specify paths. I'll look at that
>>> when refactoring the Jasper side. That will also allow the second
>>> scan to be removed.
>> 
>> This is touched upon in the TOMCAT-NEXT.txt file.
> 
> I managed to add more in this area after sending this, splitting the
> scan results and removing the second scan. The next step is to retain
> the TLD content loaded during the scan to pre-load a cache to avoid
> reloading each TLD every time a JSP is translated. That will need
> stronger dependency tracking to invalidate entries if TLDs or tag
> files change. It will take me a while though to figure out the code
> around TagLibraryInfoImpl as it's a bit convoluted.

If you think it is bad now... :)

I appreciate the problem. I spent a long time refactoring the Jasper and
Catalina code just to get them consistent. It is definitely worth taking
the time to figure out the code as that sometimes allows you to see a
way to simplify it.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements - web resources and TLDs

Posted by Jeremy Boynes <jb...@apache.org>.
On Aug 12, 2013, at 7:46 AM, Mark Thomas <ma...@apache.org> wrote:

> On 10/08/2013 23:20, Jeremy Boynes wrote:
>> On Jul 22, 2013, at 10:10 AM, Mark Thomas <ma...@apache.org> wrote:
>> 
>>> On 21/07/2013 18:36, Jeremy Boynes wrote:
>>> ...
>>>> In the meantime, I'm going to look at refactoring the TLD support as
>>>> described above.
>>> 
>>> Nice. See my note about TLD processing in the TOMCAT-NEXT.txt file in
>>> the root on trunk.
> 
> Don't forget the above :)

Not forgotten, just a work in progress :)

> 
>> I've taken the first step on this in
>>  http://svn.apache.org/r1512826
>> which moves the TLD processing from TldConfig into Jasper's ServletContainerInitializer. There are a couple of implications for this:
>> 1) Jasper's SCI needs to be activated to use TLDs
>>   This should cause no impact when ContextConfig is used to configure a web application in the normal way is Jasper is on the
>>   class path. When using Context directly in embedded mode, JasperInitializer now need to be added to enable JSP functionality 
>>   which will automatically TLD processing to add listeners. Previously, TLDs would be scanned automatically unless the processTld
>>   property on StandardContext was explicitly set false. IOW, TLDs will only be processed if JSP functionality is added, and
>>   don't need to be turned off if it is not.
> 
> Isn't the SCI discovered if Jasper is on the classpath when running in
> embedded mode? If not, do we know why not?

The SCI will be discovered if the embedder adds an application as that runs ContextConfig which does the scan. However, if they construct the context directly (without running ContextConfig) they will need to make sure the SCI is added to use JSP functionality. This is slightly different to now where TldConfig would scan for and add listeners from TLDs even if Jasper was not on the classpath (TldConfig would be added by StandardContext's initInternal method unless explicitly disabled by setting processTlds false). IMO, the new behaviour maintains a cleaner separation as it means embedders just using Servlet functionality are not impacted by JSP-related features.

There was one test case where we added a context directly but used JSPs. I had to add the SCI for that one. All other tests using the embedded server were fine.


> 
>> 2) As part of the change for processTlds I deprecated the operations on Context to set/get TLD processing properties (for
>>   validation and namespace) as they are can't be used by Jasper's SCI. If there are no objections to an API change
>>   at this stage I will go ahead and remove them completely. XML validation for TLDs can now be controlled via the
>>   "org.apache.jasper.validateDescriptors" initParam for the ServletContext; namespace processing is always on as we
>>   require >3.0 (due to the SCI etc.) which uses namespaces anyway
> 
> Remove them. At this point in Tomcat 8 we should be removing anything
> that is no longer used.

Will do. When I do that I'll also add the "removed in 8" deprecation warnings to tc7.0.x.

> 
> On a related note, I am now seeing a number of warnings about using a
> deprecated API. It looks like this is because Jasper is still using the
> old code. What is the timescale for switching Jasper to use the SCI?

I would estimate the next week or so depending on real-world commitments (aka work). I can remove the deprecation tags in trunk if that would help.

> 
>> 3) TldScanner parses TLDs as it goes creating a cache of parsed values keyed off TLD URI. It should probably create a second
>>   mapping from "TLD resource path" to the parse result in order to handle taglib directives that specify paths. I'll look at
>>   that when refactoring the Jasper side. That will also allow the second scan to be removed.
> 
> This is touched upon in the TOMCAT-NEXT.txt file.

I managed to add more in this area after sending this, splitting the scan results and removing the second scan. The next step is to retain the TLD content loaded during the scan to pre-load a cache to avoid reloading each TLD every time a JSP is translated. That will need stronger dependency tracking to invalidate entries if TLDs or tag files change. It will take me a while though to figure out the code around TagLibraryInfoImpl as it's a bit convoluted.

Cheers
Jeremy


Re: Jasper improvements - web resources and TLDs

Posted by Mark Thomas <ma...@apache.org>.
On 10/08/2013 23:20, Jeremy Boynes wrote:
> On Jul 22, 2013, at 10:10 AM, Mark Thomas <ma...@apache.org> wrote:
> 
>> On 21/07/2013 18:36, Jeremy Boynes wrote:
>> ...
>>> In the meantime, I'm going to look at refactoring the TLD support as
>>> described above.
>>
>> Nice. See my note about TLD processing in the TOMCAT-NEXT.txt file in
>> the root on trunk.

Don't forget the above :)

> I've taken the first step on this in
>   http://svn.apache.org/r1512826
> which moves the TLD processing from TldConfig into Jasper's ServletContainerInitializer. There are a couple of implications for this:
> 1) Jasper's SCI needs to be activated to use TLDs
>    This should cause no impact when ContextConfig is used to configure a web application in the normal way is Jasper is on the
>    class path. When using Context directly in embedded mode, JasperInitializer now need to be added to enable JSP functionality 
>    which will automatically TLD processing to add listeners. Previously, TLDs would be scanned automatically unless the processTld
>    property on StandardContext was explicitly set false. IOW, TLDs will only be processed if JSP functionality is added, and
>    don't need to be turned off if it is not.

Isn't the SCI discovered if Jasper is on the classpath when running in
embedded mode? If not, do we know why not?

> 2) As part of the change for processTlds I deprecated the operations on Context to set/get TLD processing properties (for
>    validation and namespace) as they are can't be used by Jasper's SCI. If there are no objections to an API change
>    at this stage I will go ahead and remove them completely. XML validation for TLDs can now be controlled via the
>    "org.apache.jasper.validateDescriptors" initParam for the ServletContext; namespace processing is always on as we
>    require >3.0 (due to the SCI etc.) which uses namespaces anyway

Remove them. At this point in Tomcat 8 we should be removing anything
that is no longer used.

On a related note, I am now seeing a number of warnings about using a
deprecated API. It looks like this is because Jasper is still using the
old code. What is the timescale for switching Jasper to use the SCI?

> 3) TldScanner parses TLDs as it goes creating a cache of parsed values keyed off TLD URI. It should probably create a second
>    mapping from "TLD resource path" to the parse result in order to handle taglib directives that specify paths. I'll look at
>    that when refactoring the Jasper side. That will also allow the second scan to be removed.

This is touched upon in the TOMCAT-NEXT.txt file.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements - web resources and TLDs

Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 22, 2013, at 10:10 AM, Mark Thomas <ma...@apache.org> wrote:

> On 21/07/2013 18:36, Jeremy Boynes wrote:
> ...
>> In the meantime, I'm going to look at refactoring the TLD support as
>> described above.
> 
> Nice. See my note about TLD processing in the TOMCAT-NEXT.txt file in
> the root on trunk.

I've taken the first step on this in
  http://svn.apache.org/r1512826
which moves the TLD processing from TldConfig into Jasper's ServletContainerInitializer. There are a couple of implications for this:
1) Jasper's SCI needs to be activated to use TLDs
   This should cause no impact when ContextConfig is used to configure a web application in the normal way is Jasper is on the
   class path. When using Context directly in embedded mode, JasperInitializer now need to be added to enable JSP functionality 
   which will automatically TLD processing to add listeners. Previously, TLDs would be scanned automatically unless the processTld
   property on StandardContext was explicitly set false. IOW, TLDs will only be processed if JSP functionality is added, and
   don't need to be turned off if it is not.

2) As part of the change for processTlds I deprecated the operations on Context to set/get TLD processing properties (for
   validation and namespace) as they are can't be used by Jasper's SCI. If there are no objections to an API change
   at this stage I will go ahead and remove them completely. XML validation for TLDs can now be controlled via the
   "org.apache.jasper.validateDescriptors" initParam for the ServletContext; namespace processing is always on as we
   require >3.0 (due to the SCI etc.) which uses namespaces anyway

3) TldScanner parses TLDs as it goes creating a cache of parsed values keyed off TLD URI. It should probably create a second
   mapping from "TLD resource path" to the parse result in order to handle taglib directives that specify paths. I'll look at
   that when refactoring the Jasper side. That will also allow the second scan to be removed.

Cheers
Jeremy


Re: Jasper improvements - web resources and TLDs

Posted by Mark Thomas <ma...@apache.org>.
On 21/07/2013 18:36, Jeremy Boynes wrote:

> With r1505235 JspC is now merging in <jsp-config> from web fragments.
> It is still not handling resources found in those jars e.g. if they
> are referenced by an include directive or from a prelude/coda
> configuration.

It also needs to compile any JSPs found as they could be accessed directly.

> This should be handled through the ServletContext so
> I'd suggest approaching it using the new webresource support.
> However, that has dependencies on Catalina (e.g. Lifecycle) that JspC
> would not need - how hard would that be to decouple?

There is a lot of code there that JspC just doesn't need. I wonder if,
at this stage, it would be simpler to just check each JAR for static
resources and process any JSPs found. That balance might change once
overlays are introduced but even then I suspect it won't.

> In the meantime, I'm going to look at refactoring the TLD support as
> described above.

Nice. See my note about TLD processing in the TOMCAT-NEXT.txt file in
the root on trunk.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements - web resources and TLDs

Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 8, 2013, at 12:23 AM, Mark Thomas <ma...@apache.org> wrote:
> On 08/07/2013 06:38, Jeremy Boynes wrote:
>> 
>> Another area of convergence is in parsing TLDs: currently both
>> Catalina and Jasper do it, differently.
> 
> A lot of the code is identical with large comments to keep the two
> implementations in sync.
> 
>> I believe Catalina only reads
>> them to detect the Listeners they define and if that's the case we
>> could perhaps add a SCI to Jasper that handled all TLD functionality
>> including registering the listeners programmatically (at least that's
>> what Servlet 8.3 implies).
> 
> Agreed.
> 
>> That would remove Catalina's dependency
>> on web-jsptaglibrary*.xsd and on TLD-related classes. To support
>> validation though we would need to add the XSD-based validation in
>> Jasper.
> 
> This is beginning to get far enough away from where the code is right
> now that I'd suggest just noting the issue and come back to it when the
> other changes are complete.

With r1505235 JspC is now merging in <jsp-config> from web fragments. It is still not handling resources found in those jars e.g. if they are referenced by an include directive or from a prelude/coda configuration. This should be handled through the ServletContext so I'd suggest approaching it using the new webresource support. However, that has dependencies on Catalina (e.g. Lifecycle) that JspC would not need - how hard would that be to decouple?

In the meantime, I'm going to look at refactoring the TLD support as described above.

Cheers
Jeremy


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements/XML handling

Posted by Mark Thomas <ma...@apache.org>.
On 08/07/2013 16:55, Violeta Georgieva wrote:
> 2013/7/8 Mark Thomas wrote:
>>
>> On 08/07/2013 06:38, Jeremy Boynes wrote:
>>> On Jun 25, 2013, at 10:44 PM, Jeremy Boynes <jb...@apache.org>
>>> wrote:
>>
>>> Jasper's descriptor parser (used for web.xml and TLDs) can only
>>> validate DTD-based documents. I opened 55212 about adding it. Such a
>>> fix could benefit from using the same resolver and cached copies that
>>> Catalina does. This is less of an issue if Jasper uses the
>>> ServletContext as it could rely on the container to parse/validate
>>> web.xml. It would still be needed if we wanted JspC to validate files
>>> standalone but there is a separate ant task that can do that.
>>
>> Jasper and Catalina need to share the same web.xml parsing and merging
>> code. Violetta started on this before she was a committer but the work
>> stalled primarily because of the size of patch which in turn was due to
>> the re-factoring required. The first step is to get the parsing and
>> merging into a package that can be shared by Jasper and Catalina. That
>> is far easier for a committer to do as they can use the refactoring
>> tools in an IDE rather than having to generate a huge patch and then
>> wait for a committer to find the time to review it.
>>
> 
> 
> Jeremy,
> 
> Here [1] is my work on the topic that I started several months ago.
> We can discuss which parts of it are still applicable and I can work on
> this also.
> 
> Regards
> Violeta
> 
> [1] https://github.com/violetagg/apache-tomcat-8.0/commits/master

Yep, that is where I was planning on starting from as well - at least
following the same steps anyway. There may well be differences in the
detail. One of the things I want to do is to take a look with Structure
101 to ensure that we get the interdependencies right.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements/XML handling

Posted by Violeta Georgieva <mi...@gmail.com>.
2013/7/8 Mark Thomas wrote:
>
> On 08/07/2013 06:38, Jeremy Boynes wrote:
> > On Jun 25, 2013, at 10:44 PM, Jeremy Boynes <jb...@apache.org>
> > wrote:
>
> > Jasper's descriptor parser (used for web.xml and TLDs) can only
> > validate DTD-based documents. I opened 55212 about adding it. Such a
> > fix could benefit from using the same resolver and cached copies that
> > Catalina does. This is less of an issue if Jasper uses the
> > ServletContext as it could rely on the container to parse/validate
> > web.xml. It would still be needed if we wanted JspC to validate files
> > standalone but there is a separate ant task that can do that.
>
> Jasper and Catalina need to share the same web.xml parsing and merging
> code. Violetta started on this before she was a committer but the work
> stalled primarily because of the size of patch which in turn was due to
> the re-factoring required. The first step is to get the parsing and
> merging into a package that can be shared by Jasper and Catalina. That
> is far easier for a committer to do as they can use the refactoring
> tools in an IDE rather than having to generate a huge patch and then
> wait for a committer to find the time to review it.
>


Jeremy,

Here [1] is my work on the topic that I started several months ago.
We can discuss which parts of it are still applicable and I can work on
this also.

Regards
Violeta

[1] https://github.com/violetagg/apache-tomcat-8.0/commits/master

Re: Jasper improvements/XML handling

Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 8, 2013, at 2:32 PM, Mark Thomas <ma...@apache.org> wrote:

> On 08/07/2013 16:02, Jeremy Boynes wrote:
>> On Jul 8, 2013, at 12:23 AM, Mark Thomas <ma...@apache.org> wrote:
>>> On 08/07/2013 06:38, Jeremy Boynes wrote:
>> ...
>>> 
>>>> I'm planning on merging these two patches and pressing on with a
>>>> converged solution, but probably won't be able to do much until next
>>>> weekend.
>>> 
>>> I very much doubt that a single patch that did everything above would
>>> ever get applied. Small incremental changes are the way to do this. I
>>> think you've identified the key issues and the right order in which to
>>> address them. Lets move forward but one small step at a time.
>> 
>> Thanks. It sounds like a couple of people have looked into this area already and found a similar ball of twine.
>> I'll start opening individual issues for the potential enhancements I found and break down the patches to be as fine grained as I can.
> 
> Great.
> 
> You've probably seen that I have started the work to move the web.xml
> processing code to a common package. I'm validating each step by
> comparing to Violetta's github version.
> 
> trunk is going to be somewhat of a moving target over the next few days.
> You'll probably see a few "It would be great if you could extract xyz
> from your patch" requests but check trunk before you start on them as
> they might already have been done by the time you read the message. For
> example, the changes to the schema resolver are likely to be needed
> fairly soon so that would be a good one to start with.

Thanks, I'll start with that once I've caught up. 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements/XML handling

Posted by Mark Thomas <ma...@apache.org>.
On 08/07/2013 16:02, Jeremy Boynes wrote:
> On Jul 8, 2013, at 12:23 AM, Mark Thomas <ma...@apache.org> wrote:
>> On 08/07/2013 06:38, Jeremy Boynes wrote:
> ...
>>
>>> I'm planning on merging these two patches and pressing on with a
>>> converged solution, but probably won't be able to do much until next
>>> weekend.
>>
>> I very much doubt that a single patch that did everything above would
>> ever get applied. Small incremental changes are the way to do this. I
>> think you've identified the key issues and the right order in which to
>> address them. Lets move forward but one small step at a time.
> 
> Thanks. It sounds like a couple of people have looked into this area already and found a similar ball of twine.
> I'll start opening individual issues for the potential enhancements I found and break down the patches to be as fine grained as I can.

Great.

You've probably seen that I have started the work to move the web.xml
processing code to a common package. I'm validating each step by
comparing to Violetta's github version.

trunk is going to be somewhat of a moving target over the next few days.
You'll probably see a few "It would be great if you could extract xyz
from your patch" requests but check trunk before you start on them as
they might already have been done by the time you read the message. For
example, the changes to the schema resolver are likely to be needed
fairly soon so that would be a good one to start with.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements/XML handling

Posted by Jeremy Boynes <jb...@apache.org>.
On Jul 8, 2013, at 12:23 AM, Mark Thomas <ma...@apache.org> wrote:
> On 08/07/2013 06:38, Jeremy Boynes wrote:
...
> 
>> I'm planning on merging these two patches and pressing on with a
>> converged solution, but probably won't be able to do much until next
>> weekend.
> 
> I very much doubt that a single patch that did everything above would
> ever get applied. Small incremental changes are the way to do this. I
> think you've identified the key issues and the right order in which to
> address them. Lets move forward but one small step at a time.

Thanks. It sounds like a couple of people have looked into this area already and found a similar ball of twine.
I'll start opening individual issues for the potential enhancements I found and break down the patches to be as fine grained as I can.

Cheers
Jeremy


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Jasper improvements/XML handling

Posted by Mark Thomas <ma...@apache.org>.
On 08/07/2013 06:38, Jeremy Boynes wrote:
> On Jun 25, 2013, at 10:44 PM, Jeremy Boynes <jb...@apache.org>
> wrote:
> 
>> I have been thinking about improvements to Jasper as well around
>> better support for Servlet 3.0 concepts. One area would be
>> decoupling it from Tomcat, bootstrapping using an SCI as hinted in
>> ContextConfig.

It was the intention of the Servlet EG that this should be possible.
There are a number of areas where changes are required to make this
work. I think you have identified the main ones.

>> I'd also be interested in improving the Ant task as
>> well, such as support for pre-compiling a separate package that
>> would be treated as a web fragment (including web.xml-less
>> pre-compilation, generating a web-fragment.xml rather than a
>> web.xml snippet or potentially eliminating the XML entirely if the
>> generated code can be annotated with @WebServlet).

My preference would be for the web-fragment.xml approach as I prefer
having just a single file to look at when debugging.

> I started on this and ended up with two patches that are converging.
> For 55166 (which started out somewhere different), I have a patch to
> update local entity handling in Catalina's XML parsing; for 53737, I
> started to have Jasper's translator rely only on the ServletContext
> and not need to parse merged web.xml XML passed as a ServletContext
> attribute. These are converging in a few areas.

Small, incremental changes are the way to go rather than one big patch.
The larger the patch the less likely it is to get applied.

I'd suggest getting the 55166 issues fixed first, then start looking at
the larger problem of making Jasper rely solely on the Servlet API.

> Jasper's descriptor parser (used for web.xml and TLDs) can only
> validate DTD-based documents. I opened 55212 about adding it. Such a
> fix could benefit from using the same resolver and cached copies that
> Catalina does. This is less of an issue if Jasper uses the
> ServletContext as it could rely on the container to parse/validate
> web.xml. It would still be needed if we wanted JspC to validate files
> standalone but there is a separate ant task that can do that.

Jasper and Catalina need to share the same web.xml parsing and merging
code. Violetta started on this before she was a committer but the work
stalled primarily because of the size of patch which in turn was due to
the re-factoring required. The first step is to get the parsing and
merging into a package that can be shared by Jasper and Catalina. That
is far easier for a committer to do as they can use the refactoring
tools in an IDE rather than having to generate a huge patch and then
wait for a committer to find the time to review it.

> Another area of convergence is in parsing TLDs: currently both
> Catalina and Jasper do it, differently.

A lot of the code is identical with large comments to keep the two
implementations in sync.

> I believe Catalina only reads
> them to detect the Listeners they define and if that's the case we
> could perhaps add a SCI to Jasper that handled all TLD functionality
> including registering the listeners programmatically (at least that's
> what Servlet 8.3 implies).

Agreed.

> That would remove Catalina's dependency
> on web-jsptaglibrary*.xsd and on TLD-related classes. To support
> validation though we would need to add the XSD-based validation in
> Jasper.

This is beginning to get far enough away from where the code is right
now that I'd suggest just noting the issue and come back to it when the
other changes are complete.

> Finally, Jasper parses these documents first into DOM and the into
> its own version based on TreeNode (this is separate from the parse
> tree used for actual JSPs). Jasper already depends on tomcat-util for
> the JarScanner so could potentially use Digester for this in the same
> way Catalina does. It would simplify the fragment merge logic in JspC
> if it could reuse what was in Catalina's WebXml (something I'm
> hacking around in 53737 right now).  Alternatively, we could still
> eliminate the intermediary DOM and TreeNode forms and use SAX or StAX
> to parse directly into Jasper's objects - especially for TLDs where
> it is using the spec's classes (TagLibraryInfo and TagInfo) as the
> model.

Again, I think this is looking too far ahead.

> I'm planning on merging these two patches and pressing on with a
> converged solution, but probably won't be able to do much until next
> weekend.

I very much doubt that a single patch that did everything above would
ever get applied. Small incremental changes are the way to do this. I
think you've identified the key issues and the right order in which to
address them. Lets move forward but one small step at a time.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org