Scott Cantor created VELOCITY-943:
-------------------------------------

             Summary: File vs. classpath issues with Spring 
VelocityEngineFactory
                 Key: VELOCITY-943
                 URL: https://issues.apache.org/jira/browse/VELOCITY-943
             Project: Velocity
          Issue Type: Improvement
    Affects Versions: 2.3
            Reporter: Scott Cantor


TL;DR, there's a nominal bug fix, and a possible improvement I can suggest, for 
the Spring support you copied in from Spring 4 based on the copy we're using in 
our project. Our copy differs in some slight ways so a straight patch diff 
wasn't very obvious and I'm just going to attach our copy of the file.

Full explanation: we ported the Spring support into our code for Spring 5 
before you had ported it in, and we were actually prepping a submission for 
that but you pulled it in before we had a chance. There were some slight 
improvements I made for our use, and today another issue in the same area of 
the code got reported and fixed, and it's nominally a bug, so I thought I'd try 
submitting that as a possible improvement along with my other change.

The issue is mainly around the way the Spring code combined use of the 
File-based Velocity loader with the Spring loader by checking for filesystem 
existence on the paths, in order to allow file-based usage to leverage 
Velocity's support for template reloading.

In Spring's code (and now your code), there's an issue because it processes 
each path via Spring ResourceLoader and then converts the Resource into a File 
for an exists() test. If that passes, it installs the file-based loader instead 
of the Spring loader. The problem/bug is that some containers unpack jars, but 
only partially. Some kind of weird optimization I guess. The result is that if 
some of the files get unpacked and not others, this code installs the file 
loader and then that fails later because not all the files are there.

The fix seems to be to check for classpath: leading off the path and skip the 
exists() check so that it will get deferred to the Spring loader. So that's a 
fix, nominally, though right now we've only seen this reported for Wildfly as a 
container.

The related enhancement I made was that I allowed for both File-based and 
Spring-based paths by walking the complete list and tracking each path set 
individually and allowing both loaders to get installed into the Velocity 
engine. That was needed for us to support both file-based templates along with 
system templates we ship in jars. So as it, we have to have that feature and 
can't use the original Spring code, so I'm hoping with the possible 
justification of a bug fix, you might take the other change also.

All of the differences that aren't cosmetic are in the 
initVelocityResourceLoader method, except that there's also a fix to 
initSpringResourceLoader that changes a setProperty to addProperty so that the 
Spring loader can get added to the set of loaders and not replace the file 
loader.

Apologies that a diff would be so messy but hopefully given that the class is 
simple and small, the differences will be clear enough with my explanation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org

Reply via email to