Issue #5320 - fix the XmlHttpClientProvider for jetty 10, re-enable t…

…ests Signed-off-by: Lachlan Roberts <[email protected]>
jetty · Nov 3, 2020 · 8d21bb7 · 8d21bb7
1 parent 64655f7
commit 8d21bb7
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 20 deletions.
diff --git a/...core-client/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java b/...core-client/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java
@@ -20,23 +20,15 @@
 
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
-import org.slf4j.LoggerFactory;
 
 public interface HttpClientProvider
 {
     static HttpClient get()
     {
-        try
-        {
-            HttpClientProvider xmlProvider = new XmlHttpClientProvider();
-            HttpClient client = xmlProvider.newHttpClient();
-            if (client != null)
-                return client;
-        }
-        catch (Throwable x)
-        {
-            LoggerFactory.getLogger(HttpClientProvider.class).trace("IGNORED", x);
-        }
+        HttpClientProvider xmlProvider = new XmlHttpClientProvider();
+        HttpClient client = xmlProvider.newHttpClient();
+        if (client != null)
+            return client;
 
         return HttpClientProvider.newDefaultHttpClient();
     }

diff --git a/...e-client/src/main/java/org/eclipse/jetty/websocket/core/client/XmlHttpClientProvider.java b/...e-client/src/main/java/org/eclipse/jetty/websocket/core/client/XmlHttpClientProvider.java
@@ -33,20 +33,35 @@ class XmlHttpClientProvider implements HttpClientProvider
     @Override
     public HttpClient newHttpClient()
     {
-        URL resource = Thread.currentThread().getContextClassLoader().getResource("jetty-websocket-httpclient.xml");
+        ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+        if (contextClassLoader == null)
+            return null;
+
+        URL resource = contextClassLoader.getResource("jetty-websocket-httpclient.xml");
         if (resource == null)
-        {
             return null;
+
+        try
+        {
+            Thread.currentThread().setContextClassLoader(HttpClient.class.getClassLoader());
+            return newHttpClient(resource);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(contextClassLoader);
         }
+    }
 
+    private static HttpClient newHttpClient(URL resource)
+    {
         try
         {
             XmlConfiguration configuration = new XmlConfiguration(Resource.newResource(resource));
             return (HttpClient)configuration.configure();
         }
         catch (Throwable t)
         {
-            LOG.warn("Unable to load: {}", resource, t);
+            LOG.warn("Failure to load HttpClient from XML {}", resource, t);
         }
 
         return null;

diff --git a/...st-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/...st-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java
@@ -47,7 +47,6 @@
 import org.eclipse.jetty.websocket.api.StatusCode;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledOnJre;
 import org.junit.jupiter.api.condition.DisabledOnOs;
@@ -376,7 +375,6 @@ public void testLog4j2ModuleWithSimpleWebAppWithJSP() throws Exception
         }
     }
 
-    @Disabled
     @ParameterizedTest
     @ValueSource(strings = {"http", "https"})
     public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Exception
@@ -389,11 +387,12 @@ public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Ex
             .mavenLocalRepository(System.getProperty("mavenRepoPath"))
             .build();
 
+        String module = "https".equals(scheme) ? "test-keystore," + scheme : scheme;
         String[] args1 = {
             "--create-startd",
             "--approve-all-licenses",
-            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket,test-keystore," + scheme
-        };
+            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket," + module,
+            };
         try (JettyHomeTester.Run run1 = distribution.start(args1))
         {
             assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
@@ -406,6 +405,9 @@ public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Ex
             String[] args2 = {
                 "jetty.http.port=" + port,
                 "jetty.ssl.port=" + port,
+                // We need to expose the websocket client classes to the webapp for this to work.
+                "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.websocket.client.",
+                "jetty.webapp.addSystemClasses+=,+org.eclipse.jetty.websocket.client.",
                 // "jetty.server.dumpAfterStart=true",
             };
 
@@ -425,7 +427,6 @@ public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Ex
         }
     }
 
-    @Disabled
     @ParameterizedTest
     @ValueSource(strings = {"http", "https"})
     public void testWebsocketClientInWebapp(String scheme) throws Exception

diff --git a/tests/test-webapps/test-websocket-client-provided-webapp/pom.xml b/tests/test-webapps/test-websocket-client-provided-webapp/pom.xml
@@ -13,6 +13,15 @@
   <name>Test :: Jetty Websocket Simple Webapp with WebSocketClient</name>
 
   <dependencies>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-slf4j-impl</artifactId>
+      <scope>compile</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty.toolchain</groupId>
       <artifactId>jetty-servlet-api</artifactId>

diff --git a/tests/test-webapps/test-websocket-client-webapp/pom.xml b/tests/test-webapps/test-websocket-client-webapp/pom.xml
@@ -13,6 +13,15 @@
   <name>Test :: Jetty Websocket Simple Webapp with WebSocketClient</name>
 
   <dependencies>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-slf4j-impl</artifactId>
+      <scope>compile</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty.toolchain</groupId>
       <artifactId>jetty-servlet-api</artifactId>