changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Nov 2, 2020 · 60c56d8 · 60c56d8
1 parent 458d67e
commit 60c56d8
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 13 deletions.
diff --git a/...t-client/src/main/java/org/eclipse/jetty/websocket/client/XmlBasedHttpClientProvider.java b/...t-client/src/main/java/org/eclipse/jetty/websocket/client/XmlBasedHttpClientProvider.java
@@ -46,16 +46,10 @@ public static HttpClient get(@SuppressWarnings("unused") WebSocketContainerScope
             Thread.currentThread().setContextClassLoader(HttpClient.class.getClassLoader());
             return newHttpClient(resource);
         }
-        catch (Throwable t)
-        {
-            LOG.warn("Failure to load HttpClient from XML", t);
-        }
         finally
         {
             Thread.currentThread().setContextClassLoader(contextClassLoader);
         }
-
-        return null;
     }
 
     private static HttpClient newHttpClient(URL resource)
@@ -67,7 +61,7 @@ private static HttpClient newHttpClient(URL resource)
         }
         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
@@ -351,9 +351,7 @@ public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Ex
                 assertEquals(HttpStatus.OK_200, response.getStatus());
                 String content = response.getContentAsString();
                 assertThat(content, containsString("WebSocketEcho: success"));
-
-                // We cannot test the HttpClient timeout because it is a server class not exposed to the webapp.
-                // assertThat(content, containsString("ConnectTimeout: 4999"));
+                assertThat(content, containsString("ConnectTimeout: 4999"));
             }
         }
     }
@@ -387,6 +385,7 @@ public void testWebsocketClientInWebapp(String scheme) throws Exception
             String[] args2 = {
                 "jetty.http.port=" + port,
                 "jetty.ssl.port=" + port,
+                // We must hide the websocket classes from the webapp if we are to include websocket client jars in WEB-INF/lib.
                 "jetty.webapp.addServerClasses+=,+org.eclipse.jetty.websocket.",
                 "jetty.webapp.addSystemClasses+=,-org.eclipse.jetty.websocket.",
                 // "jetty.server.dumpAfterStart=true",

diff --git a/tests/test-distribution/src/test/resources/keystore b/tests/test-distribution/src/test/resources/keystore
diff --git a/...webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/WebSocketClientServlet.java b/...webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/WebSocketClientServlet.java
@@ -19,6 +19,7 @@
 package org.eclipse.jetty.tests.webapp.websocket;
 
 import java.io.PrintWriter;
+import java.lang.reflect.Method;
 import java.net.URI;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.CountDownLatch;
@@ -45,7 +46,7 @@ public class WebSocketClientServlet extends HttpServlet
     @Override
     public void init() throws ServletException
     {
-        // We must rely on jetty-websocket-httpclient.xml as we do not have access to server classes like HttpClient.
+        // Cannot instantiate an HttpClient here because it's a server class, and therefore must rely on jetty-websocket-httpclient.xml
         client = new WebSocketClient();
 
         try
@@ -90,8 +91,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
             PrintWriter writer = resp.getWriter();
             writer.println("WebSocketEcho: " + ("test message".equals(response) ? "success" : "failure"));
             writer.println("WebSocketEcho: success");
-            // We cannot test the HttpClient timeout because it is a server class not exposed to the webapp.
-            // writer.println("ConnectTimeout: " + client.getHttpClient().getConnectTimeout());
+
+            // We need to test HttpClient timeout with reflection because it is a server class not exposed to the webapp.
+            Object httpClient = client.getHttpClient();
+            Method getConnectTimeout = httpClient.getClass().getMethod("getConnectTimeout");
+            writer.println("ConnectTimeout: " + getConnectTimeout.invoke(httpClient));
         }
         catch (Exception e)
         {