[
https://issues.apache.org/jira/browse/VELOCITY-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17324576#comment-17324576
]
Tim Colson commented on VELOCITY-943:
-------------------------------------
I appreciate the detailed report and submission. Thx! That said, I'm just
interested in the Spring integration, not a committer like [~cbrisson] :)
> 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
