|
From: <jue...@we...> - 2003-11-23 22:58:45
|
Colin,
=20
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.
=20
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.
=20
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...
=20
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.
=20
I've just committed that fix. Does that work for you too?
=20
Juergen
=20
________________________________
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 =3D =
Thread.currentThread().getContextClassLoader();
InputStream in =3D null;
if (ccl !=3D null) {
in =3D ccl.getResourceAsStream(name);
}
if (in =3D=3D null) {
in =3D 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?
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
|
|
From: <jue...@we...> - 2003-11-24 09:26:23
|
You don't need to add that slash for Hibernate's configuration file now: =
LocalSessionFactoryBean will automatically add the slash before passing =
it to Hibernate if you haven't specified it. Those checks for leading =
slashes were in there in all kinds of places: I've removed them =
everywhere besides in LocalSessionFactoryBean, as our own =
ClassLoaderUtils.getResourceAsStream does not need them anymore.
=20
We could go about other libs the same way: Prepend a leading slash or =
remove it before passing the path to the library, adapting to the way =
the library needs it, so that user-specified paths will work in any =
case. From the point of view of a Spring configuration, either style =
will work, no matter if interpreted by Spring or by the library. Note =
that we mainly load resources ourselves and feed them to the respective =
library anyway: For example, for JDO and Log4J.
=20
Regarding XmlWebApplicationContext: ServletContext.getResourceAsStream =
needs a leading slash according to the spec, that's why we add one if =
not there already. Most containers accept paths without leading slash =
too, though, for example Tomcat and Resin - but not Orion. It consider =
it generally a bit confusing that you need the slash for ServletContext, =
where the resource paths are interpreted as relative to the root in any =
case - while you must not use a leading slash for ClassLoader, where all =
resource paths are relative to the root too!
=20
Interpreting paths with leading slashes as ServletContext resources and =
without slashes as class path resources would be confusing, IMO. As =
mentioned above, many web app developers might be used to be able to =
load from the ServletContext with no leading slash, which wouldn't work =
with Spring then. We shouldn't rely on such ambiguous syntactic details =
to decide between two completely different ways of resource loading.
=20
Note that you can also specify URLs for =
ApplicationContext.getResourceAsStream, e.g. with a "file:" or "http:" =
prefix. So a cleaner way to support class path resources would probably =
be to define "classpath:" or the like as pseudo-prefix for URL-style =
paths. This way, there is only one meaning for non-URL paths in each =
context implementation, i.e. XmlWebApplicationContext always interprets =
them as ServletContext resource paths.
=20
Any other opinions on this? If yes, then please voice them today - we =
shouldn't delay M3 any further!
=20
Juergen
=20
________________________________
Von: spr...@li... im Auftrag =
von Colin Sampaleanu
Gesendet: Mo 24.11.2003 00:54
An: spr...@li...
Betreff: Re: [Springframework-developer] Classloading issue to resolve =
before M3 release!
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=FCrgen h=F6ller [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 =3D =
Thread.currentThread().getContextClassLoader();
> InputStream in =3D null;
> if (ccl !=3D null) {
> in =3D ccl.getResourceAsStream(name);
> }
> if (in =3D=3D null) {
> in =3D 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?
>=20
>
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
|
|
From: Colin S. <col...@ex...> - 2003-11-24 12:37:17
|
W/regards to doing something with XmlWebApplicationContext, I didn't realize that some containers accept paths in ServletContext.getResource without the leading slash. (I wish containers would just stick to the spec, this sort of looseness just ends confusing things and killing portability). It would still be nice to get it to be able to load from the classpath too though. I wonder what the implications would be of just hitting ServletContext.getResource as the code does now, then the ClassLoader.getResource if the resource is not found in the first call? Probably it could lead to some situations where people pick up stuff they don't want. As far as adding a pseudo prefix to indicate classpath, that's an ok idea. The only issue with it is that as there is no real url handler in the system for that prefix, if anybody in some Java code tried to manipulate this as a real URL they'd get a MalformedUrlException as soon as they tried to construct the URL. However, I don't think this is an issue since in fact this value is not always a URL in any case. jürgen höller [werk3AT] wrote: >You don't need to add that slash for Hibernate's configuration file now: LocalSessionFactoryBean will automatically add the slash before passing it to Hibernate if you haven't specified it. Those checks for leading slashes were in there in all kinds of places: I've removed them everywhere besides in LocalSessionFactoryBean, as our own ClassLoaderUtils.getResourceAsStream does not need them anymore. > >We could go about other libs the same way: Prepend a leading slash or remove it before passing the path to the library, adapting to the way the library needs it, so that user-specified paths will work in any case. From the point of view of a Spring configuration, either style will work, no matter if interpreted by Spring or by the library. Note that we mainly load resources ourselves and feed them to the respective library anyway: For example, for JDO and Log4J. > >Regarding XmlWebApplicationContext: ServletContext.getResourceAsStream needs a leading slash according to the spec, that's why we add one if not there already. Most containers accept paths without leading slash too, though, for example Tomcat and Resin - but not Orion. It consider it generally a bit confusing that you need the slash for ServletContext, where the resource paths are interpreted as relative to the root in any case - while you must not use a leading slash for ClassLoader, where all resource paths are relative to the root too! > >Interpreting paths with leading slashes as ServletContext resources and without slashes as class path resources would be confusing, IMO. As mentioned above, many web app developers might be used to be able to load from the ServletContext with no leading slash, which wouldn't work with Spring then. We shouldn't rely on such ambiguous syntactic details to decide between two completely different ways of resource loading. > >Note that you can also specify URLs for ApplicationContext.getResourceAsStream, e.g. with a "file:" or "http:" prefix. So a cleaner way to support class path resources would probably be to define "classpath:" or the like as pseudo-prefix for URL-style paths. This way, there is only one meaning for non-URL paths in each context implementation, i.e. XmlWebApplicationContext always interprets them as ServletContext resource paths. > >Any other opinions on this? If yes, then please voice them today - we shouldn't delay M3 any further! > >Juergen > > >________________________________ > >Von: spr...@li... im Auftrag von Colin Sampaleanu >Gesendet: Mo 24.11.2003 00:54 >An: spr...@li... >Betreff: Re: [Springframework-developer] Classloading issue to resolve before M3 release! > > > >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: > > |
|
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?
>
>
|
|
From: Colin S. <col...@ex...> - 2003-11-24 00:28:43
|
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?
>>
>
|