[
https://issues.apache.org/jira/browse/VELOCITY-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17878951#comment-17878951
]
Scott Cantor edited comment on VELOCITY-943 at 9/3/24 5:41 PM:
---------------------------------------------------------------
I kind of bum-rushed this, but here's a PR out of GitHub:
https://github.com/apache/velocity-engine/pull/51
I think this does what you want, but at a cost of some complication. I'm not
honestly sure it's warranted, the original behavior isn't really "a way of
doing something", it's just broken.
But I added a flag to the VelocityEngineFactory class for "supporting enhanced
classpath processing" that tries to fall back to the older/broken behavior.
Basically, the old code puts in either a Spring or File-based resource loader
depending on what sorts of paths it encounters.
The new code conditionally allows both types to be installed for the
appropriate path types based on how the original logic did that, but with a
callout for classpath: resource URLs.
Ultimately, and this is why I don't think this is really a "change", it comes
down to what paths you include. If you deliberately include both file and
classpath locations in your loader path, the old code isn't going to work. If
you don't include both, then this devolves to the original behavior anyway
since it's only going to install one or the other type of paths.
So if you're asking me as a Spring user, the right answer would be for me to
simplify the path back to "just do the right thing" based on what the paths
turn out to be.
was (Author: canto...@osu.edu):
I kind of bum-rushed this, but here's a PR out of GitHub:
https://github.com/apache/velocity-engine/pull/51
I think this does what you want, but at a cost of some complication. I'm not
honestly sure it's warranted, the original behavior isn't really "a way of
doing something", it's just broken.
But I added a flag to the VelocityEngineFactory class for "supporting enhanced
classpath processing" that tries to fall back to the older/broken behavior.
Basically, the old code puts in either a Spring or File-based resource loader
depending on what sorts of paths it encounters.
The new code conditionally allows both types to be installed for the
appropriate path types based on how the original logic did that, but with a
callout for classpath: resource URLs.
> 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.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
