[ 
https://issues.apache.org/jira/browse/VELOCITY-943?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Scott Cantor updated VELOCITY-943:
----------------------------------
    Attachment: VelocityEngineFactory.java

> 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
>            Priority: Minor
>              Labels: spring
>         Attachments: VelocityEngineFactory.java
>
>
> 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