Mercurial > hg > release > thermostat-1.6

changeset 2010:c28824dce18c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix classloader setup of embedded jetty. The set thread context class loader will become the parent of the newly created WebAppClassLoader. This patch takes advantage of this fact which simplifies class loading for web-storage-service. Reviewed-by: neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-August/020460.html PR3125
author Severin Gehwolf <sgehwolf@redhat.com>
date Fri, 08 Jul 2016 19:31:34 +0200
parents 29f94fdee22b
children 53db26fe0673
files web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/DelegatingClassLoader.java web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/DelegatingWebappClassLoader.java web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/JettyContainerLauncher.java web/endpoint-plugin/web-service/src/test/java/com/redhat/thermostat/web/endpoint/internal/DelegatingClassLoaderTest.java web/endpoint-plugin/web-service/src/test/java/com/redhat/thermostat/web/endpoint/internal/DelegatingWebappClassLoaderTest.java
diffstat 5 files changed, 174 insertions(+), 188 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/DelegatingClassLoader.java	Fri Jul 08 19:31:34 2016 +0200
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2012-2016 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.web.endpoint.internal;
+
+/**
+ * Asks a delegating classloader to load classes, and if not found defer to the
+ * system class loader.
+ */
+class DelegatingClassLoader extends ClassLoader {
+    
+    private final ClassLoader delegate;
+
+    DelegatingClassLoader(ClassLoader delegate) {
+        this.delegate = delegate;
+    }
+    
+    public Class<?> loadClass(String className) throws ClassNotFoundException {
+        try {
+            return delegate.loadClass(className);
+        } catch (ClassNotFoundException e) {
+            return getSystemClassLoader().loadClass(className);
+        }
+    }
+}
--- a/web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/DelegatingWebappClassLoader.java	Tue Jul 05 10:32:42 2016 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,83 +0,0 @@
-/*
- * Copyright 2012-2016 Red Hat, Inc.
- *
- * This file is part of Thermostat.
- *
- * Thermostat is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published
- * by the Free Software Foundation; either version 2, or (at your
- * option) any later version.
- *
- * Thermostat is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with Thermostat; see the file COPYING.  If not see
- * <http://www.gnu.org/licenses/>.
- *
- * Linking this code with other modules is making a combined work
- * based on this code.  Thus, the terms and conditions of the GNU
- * General Public License cover the whole combination.
- *
- * As a special exception, the copyright holders of this code give
- * you permission to link this code with independent modules to
- * produce an executable, regardless of the license terms of these
- * independent modules, and to copy and distribute the resulting
- * executable under terms of your choice, provided that you also
- * meet, for each linked independent module, the terms and conditions
- * of the license of that module.  An independent module is a module
- * which is not derived from or based on this code.  If you modify
- * this code, you may extend this exception to your version of the
- * library, but you are not obligated to do so.  If you do not wish
- * to do so, delete this exception statement from your version.
- */
-
-package com.redhat.thermostat.web.endpoint.internal;
-
-import java.io.IOException;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-
-import org.eclipse.jetty.webapp.WebAppClassLoader;
-import org.eclipse.jetty.webapp.WebAppContext;
-
-import com.redhat.thermostat.common.utils.LoggingUtils;
-
-public class DelegatingWebappClassLoader extends WebAppClassLoader {
-
-    private static final boolean DEBUG = false;
-    private static final Logger logger = LoggingUtils.getLogger(DelegatingWebappClassLoader.class);
-    private final ClassLoader osgiDelegate;
-
-    public DelegatingWebappClassLoader(ClassLoader osgiDelegate, WebAppContext context)
-            throws IOException {
-        super(context);
-        this.osgiDelegate = osgiDelegate;
-    }
-
-    private void log(String s) {
-        if (DEBUG) {
-            logger.log(Level.FINEST, s);
-        }
-    }
-
-    @Override
-    public void addClassPath(String classPath) throws IOException {
-        log(String.format("Adding classpath: %s", classPath));
-        super.addClassPath(classPath);
-    }
-
-    @SuppressWarnings({ "rawtypes", "unchecked" })
-    @Override
-    public Class loadClass(String name) throws ClassNotFoundException {
-        try {
-            return super.loadClass(name);
-        } catch (ClassNotFoundException e) {
-            log(String.format("loading class using OSGi delegate: %s", name));
-            // try the osgi delegate
-            return osgiDelegate.loadClass(name);
-        }
-    }
-}
--- a/web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/JettyContainerLauncher.java	Tue Jul 05 10:32:42 2016 -0400
+++ b/web/endpoint-plugin/web-service/src/main/java/com/redhat/thermostat/web/endpoint/internal/JettyContainerLauncher.java	Fri Jul 08 19:31:34 2016 +0200
@@ -96,7 +96,26 @@
 
             @Override
             public void run() {
-                startContainerAndDeployWar(contextStartedLatch);
+                // Make the class loader aware of the osgi env. By default webapps use
+                // the set TCCL as parent. Note that a plain WebAppClassLoader is not sufficient. Mainly because of classes
+                // jetty itself is using (from the servlet API, other bundles etc). 
+                //
+                // It's a simple delegating class loader. If the OSGi loader (the loader of this
+                // class) doesn't find the class, we delegate to the system class loader.
+                // 
+                // Note that this also assumes proper wiring of the endpoint bundle. That should be
+                // the case by using explicit instructions for the maven-bundle-plugin
+                // and starting all required jetty bundles on boot.
+                //
+                // See also this bug for some discussion:
+                // https://github.com/eclipse/jetty.project/issues/705
+                ClassLoader osgiLoader = JettyContainerLauncher.class.getClassLoader();
+                Thread.currentThread().setContextClassLoader(new DelegatingClassLoader(osgiLoader));
+                try {
+                    startContainerAndDeployWar(contextStartedLatch);
+                } finally {
+                    Thread.currentThread().setContextClassLoader(null);
+                }
             }
             
         });
@@ -175,20 +194,6 @@
         writeWebDefaults(tempWebDefaults, uri);
         ctx.setDefaultsDescriptor(tempWebDefaults.getAbsolutePath());
         
-        // Make the class loader aware of the osgi env. By default it uses
-        // WebAppClassLoader which is not sufficient. Mainly because of classes
-        // jetty itself is using (from the servlet API, other bundles etc). 
-        // Thermostat itself should be happy with just WebAppClassLoader.
-        // 
-        // It's a simple delegating class loader. If WebappClassLoader doesn't
-        // find the class, we delegate to the class loader of this bundle (which
-        // in turn should delegate to the right loader).
-        // 
-        // Note that this also assumes proper wiring of bundles. That should be
-        // the case by using explicit instructions for the maven-bundle-plugin
-        // and starting all required jetty bundles on boot.
-        ctx.setClassLoader(new DelegatingWebappClassLoader(getClass().getClassLoader(), ctx));
-        
         // Make server startup fail if context cannot be deployed.
         // Please don't change this.
         ctx.setThrowUnavailableOnStartupException(true);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/web/endpoint-plugin/web-service/src/test/java/com/redhat/thermostat/web/endpoint/internal/DelegatingClassLoaderTest.java	Fri Jul 08 19:31:34 2016 +0200
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2012-2016 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.web.endpoint.internal;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import org.junit.Test;
+
+public class DelegatingClassLoaderTest {
+
+    @Test
+    public void verifyClassloaderDelegationFailsForNotExistingClass() throws Exception {
+        ClassLoader mockLoader = mock(ClassLoader.class);
+        DelegatingClassLoader delegateLoader = new DelegatingClassLoader(mockLoader);
+        String notExistingClassName = "com.redhat.thermostat.not.existing.FooClass";
+        doThrow(ClassNotFoundException.class).when(mockLoader).loadClass(eq(notExistingClassName));
+        try {
+            delegateLoader.loadClass(notExistingClassName);
+            fail("should not have found class " + notExistingClassName);
+        } catch (ClassNotFoundException e) {
+            // pass
+            verify(mockLoader).loadClass(eq(notExistingClassName));
+        }
+    }
+    
+    @Test
+    public void verifyClassloaderDelegationWorksForExistingClass() throws Exception {
+        ClassLoader mockLoader = mock(ClassLoader.class);
+        DelegatingClassLoader webappLoader = new DelegatingClassLoader(mockLoader);
+        // This class is not really there, but the fake class loader does not
+        // throw a CNFE
+        String className = "com.redhat.thermostat.not.existing.FooClass";
+        try {
+            Class<?> clazz = webappLoader.loadClass(className);
+            assertNull(clazz);
+        } catch (ClassNotFoundException e) {
+            fail("should have been able to load class since mock classloader does not throw CNFE");
+        }
+        verify(mockLoader).loadClass(eq(className));
+    }
+    
+    @Test
+    public void verifySystemClassIsResolvable() throws Exception {
+    	ClassLoader mockLoader = mock(ClassLoader.class);
+    	DelegatingClassLoader delegateLoader = new DelegatingClassLoader(mockLoader);
+    	String className = "javax.security.auth.login.LoginContext";
+    	doThrow(ClassNotFoundException.class).when(mockLoader).loadClass(eq(className));
+    	try {
+    		Class<?> clazz = delegateLoader.loadClass(className);
+    		assertNotNull(clazz);
+    	} catch (ClassNotFoundException e) {
+    		fail("should have been able to load system class " + className);
+    	}
+    }
+    
+}
--- a/web/endpoint-plugin/web-service/src/test/java/com/redhat/thermostat/web/endpoint/internal/DelegatingWebappClassLoaderTest.java	Tue Jul 05 10:32:42 2016 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,90 +0,0 @@
-/*
- * Copyright 2012-2016 Red Hat, Inc.
- *
- * This file is part of Thermostat.
- *
- * Thermostat is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published
- * by the Free Software Foundation; either version 2, or (at your
- * option) any later version.
- *
- * Thermostat is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with Thermostat; see the file COPYING.  If not see
- * <http://www.gnu.org/licenses/>.
- *
- * Linking this code with other modules is making a combined work
- * based on this code.  Thus, the terms and conditions of the GNU
- * General Public License cover the whole combination.
- *
- * As a special exception, the copyright holders of this code give
- * you permission to link this code with independent modules to
- * produce an executable, regardless of the license terms of these
- * independent modules, and to copy and distribute the resulting
- * executable under terms of your choice, provided that you also
- * meet, for each linked independent module, the terms and conditions
- * of the license of that module.  An independent module is a module
- * which is not derived from or based on this code.  If you modify
- * this code, you may extend this exception to your version of the
- * library, but you are not obligated to do so.  If you do not wish
- * to do so, delete this exception statement from your version.
- */
-
-package com.redhat.thermostat.web.endpoint.internal;
-
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-
-import org.eclipse.jetty.webapp.WebAppContext;
-import org.junit.Test;
-
-import com.redhat.thermostat.web.endpoint.internal.DelegatingWebappClassLoader;
-
-public class DelegatingWebappClassLoaderTest {
-
-    @Test
-    public void verifyClassloaderDelegationFailsForNotExistingClass() throws Exception {
-        ClassLoader mockLoader = mock(ClassLoader.class);
-        WebAppContext mockContext = mock(WebAppContext.class);
-        DelegatingWebappClassLoader webappLoader = new DelegatingWebappClassLoader(mockLoader, mockContext);
-        String notExistingClassName = "com.redhat.thermostat.not.existing.FooClass";
-        doThrow(ClassNotFoundException.class).when(mockLoader).loadClass(eq(notExistingClassName));
-        try {
-            webappLoader.loadClass(notExistingClassName);
-            fail("should not have found class " + notExistingClassName);
-        } catch (ClassNotFoundException e) {
-            // pass
-            verify(mockLoader).loadClass(eq(notExistingClassName));
-        } finally {
-            webappLoader.close();
-        }
-    }
-    
-    @Test
-    public void verifyClassloaderDelegationWorksForExistingClass() throws Exception {
-        ClassLoader mockLoader = mock(ClassLoader.class);
-        WebAppContext mockContext = mock(WebAppContext.class);
-        DelegatingWebappClassLoader webappLoader = new DelegatingWebappClassLoader(mockLoader, mockContext);
-        // This class is not really there, but the fake class loader does not
-        // throw a CNFE
-        String className = "com.redhat.thermostat.not.existing.FooClass";
-        try {
-            Class<?> clazz = webappLoader.loadClass(className);
-            assertNull(clazz);
-        } catch (ClassNotFoundException e) {
-            fail("should have been able to load class since mock classloader does not throw CNFE");
-        } finally {
-            webappLoader.close();
-        }
-        verify(mockLoader).loadClass(eq(className));
-    }
-    
-}