Re: [Springframework-developer] Classloading issue to resolve before M3 release!

[Colin S. <col...@ex...>]

I've modified ClassLoaderUtils.classPackageAsResourcePath to return a 
path value without automatically adding a leading slash, which it 
previously did. This change will allow the result to be fed to 
ClassLoader.getResource, or also Class.getResource by prepending a 
slash. I only added the original code a month and a half ago, with no 
internal usage in Spring, and I doubt very many people are using it 
externally yet. Even then, if they are feeding the result back to Spring 
things will still work, so hopefully nobody will be affected.


Colin Sampaleanu wrote:

> 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?
>>   
>