Thanks for the feedback, Toby.
As Toby said, this code is less than clear-cut, and I don't feel totally
comfortable about it. But I think the best decision is to mirror Jetty's
behavior as closely as possible, because it's likely to be the least
surprising. In other words, lots of people have used Jetty so their code
has been pretty well vetted, and people will have tailored their own
configurations to work with it.
On Wed, Mar 25, 2009 at 3:51 PM, Toby Reyelts <to...@google.com> wrote:
> Overall, the change to treat server classes vs system classes separately,
> such that system classes are "API classes" and server classes are
> "implementation classes" looks good.
>
> I'm concerned that there will still be places where things will clash -
> such as the sharing of log4j across both server and webapps. Given the
> complexity of the situation, I understand the desire to change behavior as
> little as possible in this area, though. There's also an argument that,
> since this is the way Jetty currently functions, you're just exposing Jetty
> behavior, as is.
>
> I think the situation would be simpler if GWT didn't treat the system
> classpath as a "user-level" classpath.
>
>
> On Wed, Mar 25, 2009 at 9:15 AM, <codesite-nore...@google.com> wrote:
>
>>
>> Author: sco...@google.com
>> Date: Wed Mar 25 05:43:19 2009
>> New Revision: 5078
>>
>> Modified:
>>
>>
>> releases/1.6/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java
>>
>> Log:
>> Amending r5077:
>> - Catch ClassNotFound in outside loader and allow internal load
>> - Allow "system" resources to be loaded internally if they're not found
>> externally
>> - jat suggestion to extract constant
>> - Fix comments
>>
>> Review by: tobyr (TBR)
>>
>> Modified:
>>
>> releases/1.6/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java
>>
>> ==============================================================================
>> ---
>>
>> releases/1.6/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java
>> (original)
>> +++
>>
>> releases/1.6/dev/core/src/com/google/gwt/dev/shell/jetty/JettyLauncher.java
>> Wed Mar 25 05:43:19 2009
>> @@ -249,6 +249,8 @@
>> */
>> private class WebAppClassLoaderExtension extends WebAppClassLoader {
>>
>> + private static final String META_INF_SERVICES =
>> "META-INF/services/";
>> +
>> public WebAppClassLoaderExtension() throws IOException {
>> super(bootStrapOnlyClassLoader, WebAppContextWithReload.this);
>> }
>> @@ -257,17 +259,21 @@
>> public URL findResource(String name) {
>> // Specifically for
>> META-INF/services/javax.xml.parsers.SAXParserFactory
>> String checkName = name;
>> - if (checkName.startsWith("META-INF/services/")) {
>> - checkName = checkName.substring("META-INF/services/".length());
>> + if (checkName.startsWith(META_INF_SERVICES)) {
>> + checkName = checkName.substring(META_INF_SERVICES.length());
>> }
>>
>> - // For system/server path, just try the outside world quietly.
>> + // For a system path, load from the outside world.
>> + URL found;
>> if (isSystemPath(checkName)) {
>> - return systemClassLoader.getResource(name);
>> + found = systemClassLoader.getResource(name);
>> + if (found != null) {
>> + return found;
>> + }
>> }
>>
>> // Always check this ClassLoader first.
>> - URL found = super.findResource(name);
>> + found = super.findResource(name);
>> if (found != null) {
>> return found;
>> }
>> @@ -303,14 +309,18 @@
>>
>> @Override
>> protected Class<?> findClass(String name) throws
>> ClassNotFoundException {
>> - // For system/server path, just try the outside world quietly.
>> + // For system path, always prefer the outside world.
>> if (isSystemPath(name)) {
>> - return systemClassLoader.loadClass(name);
>> + try {
>> + return systemClassLoader.loadClass(name);
>> + } catch (ClassNotFoundException e) {
>> + }
>> }
>>
>> try {
>> return super.findClass(name);
>> } catch (ClassNotFoundException e) {
>> + // Don't allow server classes to be loaded from the outside.
>> if (isServerPath(name)) {
>> throw e;
>> }
>>
>>
>>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
