|
From: Colin S. <col...@ex...> - 2003-11-23 23:54:03
|
Unfortunately I don't agree that accepting both leading slashes and no
leading slashes is going to provide the least surprise. Hibernate is
Hibernate. If for some reason they decided to load via Class.getResource
instead of ClassLoader.getResource, I think they're wrong (because
they're not using the context classloader) but that's their business,
and at least they're consistent about it. In fact, they will fail if
they are given a path _without_ the leading slash.
If we accept both to mean the same, that's ok for Spring itself, but
it's not consistent with the fact that most other java code handles the
two variants differently. And if you give a path to Spring, you then
have to think, is this for Spring itself, which will handle both ways,
or is it going to end up being fed to some lib, which needs it in one
way, or fed to another lib, which needs it in another way, and is it
going to be doctored by Spring, or go straight through? i.e. right now
for Hibernate, the fact that Spring takes both formats elsewhere doesn't
help me at all, I still have to think of Hibernate and add that slash.
I'd rather be consistent and say, Spring needs resource paths in
standard ClassLoader.getResource format, and that's it. Then if you have
a case where a path is not for Spring, but is fed straight through to
some other lib (like Hibernate), which uses Class.getResource instead of
ClassLoader.getResource, you either strongly document this special case,
or fix it up yourself...
There is another consideration somewhat relating to this, and that is
the fact that currently XMLWebApplicationContext is hard-coded to add
(if not there already) a leading slash to all paths it is fed, and then
it uses ServletContext to load the defs, so the defs can only be loaded
off the web-context root. In this case, we could instead treat paths
which start with a leading slash as relating to ServletContext, and
paths which don't have a leading slash as relating to the context
classloader, with no interference. I've always thought it was an
unecessary limiation to not have XMLWebApplicationContext be able to
load contexts out of the classloader.
Regards,
Colin
jürgen höller [werk3AT] wrote:
>Colin,
>
>I've been messing around with that myself for a while, and have also modified that ClassLoaderUtils method. I didn't realize that ClassLoader.getResourceAsStream does not accept leading slashes! - I thought they would work as absolute paths with both Class.getResourceAsStream and ClassLoader.getResourceAsStream.
>
>As for the context class loader, you're right that we should be able to rely on it in any case anyway. ClassLoaderUtils.getResourceAsStream was intended as framework-internal class anyway: Nowhere within the framework does loading relative to a framework class make any sense.
>
>It may make sense though to accept leading slashes in paths, as many callers don't want to worry about whether the path gets interpreted by Class or ClassLoader. For example, Hibernate's Configuration object needs the slash, as it loads via Environment.class.getResourceAsStream...
>
>To stick to the principle of least surprise, I strongly vote for treating "/a/b/myresource" the same as "a/b/myresource". This way, callers don't need to worry about the actual loading strategy. They can stick to the latter pattern, which is correct for ClassLoader usage, but also the former, for example if used to it from Hibernate.
>
>I've just committed that fix. Does that work for you too?
>
>Juergen
>
>
>________________________________
>
>Von: spr...@li... im Auftrag von Colin Sampaleanu
>Gesendet: So 23.11.2003 21:37
>An: spr...@li...
>Betreff: [Springframework-developer] Classloading issue to resolve before M3 release!
>
>
>
>I just figured out that we have an issue with the existing code in
>ClassloaderUtils.getResourceAsStream, which is used in a number of places.
>
>Whoever wrote the original method:
>
> public static InputStream getResourceAsStream(Class clazz, String
>name) {
> ClassLoader ccl = Thread.currentThread().getContextClassLoader();
> InputStream in = null;
> if (ccl != null) {
> in = ccl.getResourceAsStream(name);
> }
> if (in == null) {
> in = clazz.getResourceAsStream(name);
> }
> return in;
> }
>
>did not realize that ClassLoader.getResourceAsStream and
>Class.getResourceAsStream do _not_ behave the same way w/regards to the
>path this is supplied to them.
>
>Class.getResource, expects to be given a path which is either relative
>or absolute to that class, as indicated by a leading /. If there is a
>leading slash, it strips off that slash, then passes on the request to
>its classloader (ie it calls ClassLoader.getResource). If there is no
>slash, it prepends the class package (without a leading slash) to the
>specified path, then calls getResource on its classloader.
>ClassLoader.getResource, on the other hand, expects that it is given
>only absolute paths, _without_ a leading slash. And it will fail if
>there is a leading slash.
>
>As such, if the code above is called with a path that is correct for the
>call to the context classloader (ie no leading slash), it will be
>incorrect for the call to class.getResource (as it will be considered
>relative). If it is called with code that is fully qualified/absolute
>and correct for the class.getResurce (ie it has a leading slash), it
>will always fail for the context classloader getResource call.
>
>Not good... Now this code is used in a number of places, usually with
>'/' prepended on the actual path. That is, the code assumes the resource
>lookup behaves in a similar fashion to Class.getResource, not
>ClassLoader.getResource. In that respect, the solution of 'least
>surpirse' is to treat the context classloader case the same. Even though
>it doesn't actually use the class package, if the path doesn't start
>with '/', it should be assumed to be relative and added to the specified
>resource path. Then this should be documented more explicitly (ie that
>it behaves like Class.getResource, not ClassLoader.getResource).
>
>Then, this could be deprecated, and we could make a prefered variation
>of getResource which doesn't take a class argument, just a resource
>path, and it would simply behave as the existing
>ClassLoader.getResource. From what I know, except in Java 1.1, the
>context classloader is always safe to use. (do we need Java 1.1
>compatibility here?)
>
>Then there is the question of classPackageAsResourcePath, which I
>originally wrote, but actually is only really correct when then used to
>call the current ClassLoaderUtils.getResourceAsStream or
>Class.getResourceAsStream. At a minimum, it should be documented, but I
>think it would be cleaner to not have it prepend a leading slash at all,
>so it could be fed to the regular ClassLoader.getResource.
>
>Finally, I think it possibly breaks the principal of least surprise that
>in most places (like ClassPathXmlApplicationContext or the web xml
>application context), we take resource paths and assume that
> a/b/c/d/123.abc
>is the same thing as
> /a/b/c/d/123.abc
>
>That is not how ClassLoader.getResource treats things, and that's not
>how Class.getResource treats things, so why are we doing it? At a
>minimum, it should be strongly documented, but I think it's simpler to
>just accept the first form only, to be like classloader resource
>specifications in other java apps/libs. This would obviously break some
>existing code. In fact, we even usually add the leading / right now
>automatically if it's not there.
>
>What do you guys think?
>
>
|