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

Reply via email to