> > If the properties file was in the classpath, template path, or any
other
> > resource, it would have been found by Velocity, so it'd only have to
try
> > loading with the path as is and not worry about calculating any
template
> > path stuff.
> 
> That's not true - Velocity don't dig around anywhere other than where
the
> configured loaders look, and by default that's the file loader in the
> current directory.

Right, what I meant was, in the current PropertiesUtil, if templatePath
(Texen's templatePath property that maps to the
Velocity.FILE_RESOURCE_LOADER_PATH) was set, it tried to load the
property file off the file system by first prepending the templatePath.
If templatePath was not set, it'd tried to load it out of the classpath.


Given that these were the only two methods tried, the current mechanism
of using ContentResource would find if off the file system as Texen
inherently sets up a FileResourceLoader on the templatePath dir and
would also find it from the classpath as Texen will setup a
ClassPathResourceLoader if it's useClasspath property is set. And I
think it's safe to assume that if users want to load properties via the
classpath, they will already have Texen's useClasspath set (as they are
most likely already loading templates from the classpath).

> Just so you know where I'm coming from, my only concern is that this
doesn't
> break anyone, and second, it's not going to be a bear to maintain in
the
> future.  After that, if Texen users like it, I like it.
>
> Another approach is to try to find the properties file in the
classpath via
> using the classloader, or letting it be set in some system property.
Is
> that better/ more conventional/safer/ more elegant?

That was what the current PropertiesUtil tried to do, but they did some
guessing about whether to try the file system or classpath based on
whether Texen's templatePath was null or not. For Torque, we want to
have one Texen task seamlessly work both from the file system and the
classpath based on whether the user had an Ant ${useClasspath} property
set. So we were setting templatePath regardless of what useClasspath
was, hence PropertiesUtil always thought we wanted properties file from
the file system.

I had made a different patch that instead of assuming templatePath being
set means don't use the classpath, PropertiesUtil would just try the
file system first, then the classpath if that failed..

However, it just didn't sit very well with me, because it was
duplicating a lot of logic that to me the Velocity resource management
system could handle just fine.

After reviewing the current PropertiesUtil code, there are two cases
where the patched PropertiesUtil would break existing code, both of them
dealing with properties files outside of the resource loader's scope:

- a relative path is used, PropertiesUtil forcefully prepends
templatePath, but the relative path backs it way out of the resource
loader's scope via ..\..\..\.

- an abosulte path is used, PropertiesUtil forcesfully prepends
templatePath, but it's empty, so the absolute path would succeed.

Perhaps we could have a more experienced Texen developer, e.g. Jason van
Zyl, Stephane Bailliez, or Leon Messerschmidt (past people who've
committed/worked on PropertiesUtil), and see what they think about
losing the ability to load properties outside of the currently
configured resource loaders? If none of them use such functionality, I
think we could make a strong assumption that the patch is good.

Thanks,
Stephen


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to