Issue #5320 - also test WebSocketClient on server from WEB-INF/lib

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Oct 15, 2020 · 4cb475c · 4cb475c
1 parent 1b07c84
commit 4cb475c
Show file tree

Hide file tree

Showing 11 changed files with 270 additions and 110 deletions.
diff --git a/...socket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java b/...socket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java
@@ -32,17 +32,26 @@
 @Deprecated
 public abstract class ExtensionFactory implements Iterable<Class<? extends Extension>>
 {
-    private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
-    private Map<String, Class<? extends Extension>> availableExtensions;
+    private final Map<String, Class<? extends Extension>> availableExtensions;
 
     public ExtensionFactory()
     {
         availableExtensions = new HashMap<>();
-        for (Extension ext : extensionLoader)
+        Iterator<Extension> iterator = ServiceLoader.load(Extension.class).iterator();
+        while (true)
         {
-            if (ext != null)
+            try
             {
-                availableExtensions.put(ext.getName(), ext.getClass());
+                if (!iterator.hasNext())
+                    break;
+
+                Extension ext = iterator.next();
+                if (ext != null)
+                    availableExtensions.put(ext.getName(), ext.getClass());
+            }
+            catch (Throwable ignored)
+            {
+                // Ignored.
             }
         }
     }

diff --git a/...rc/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java b/...rc/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java
@@ -19,11 +19,6 @@
 package org.eclipse.jetty.websocket.common.extensions;
 
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.ServiceLoader;
-import java.util.Set;
 import java.util.zip.Deflater;
 
 import org.eclipse.jetty.util.StringUtil;
@@ -42,10 +37,8 @@
 
 public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle, Dumpable
 {
-    private ContainerLifeCycle containerLifeCycle;
-    private WebSocketContainerScope container;
-    private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
-    private Map<String, Class<? extends Extension>> availableExtensions;
+    private final ContainerLifeCycle containerLifeCycle;
+    private final WebSocketContainerScope container;
     private final InflaterPool inflaterPool = new InflaterPool(CompressionPool.INFINITE_CAPACITY, true);
     private final DeflaterPool deflaterPool = new DeflaterPool(CompressionPool.INFINITE_CAPACITY, Deflater.DEFAULT_COMPRESSION, true);
 
@@ -59,42 +52,12 @@ public String toString()
                 return String.format("%s@%x{%s}", WebSocketExtensionFactory.class.getSimpleName(), hashCode(), containerLifeCycle.getState());
             }
         };
-        availableExtensions = new HashMap<>();
-        for (Extension ext : extensionLoader)
-        {
-            if (ext != null)
-                availableExtensions.put(ext.getName(), ext.getClass());
-        }
 
         this.container = container;
         containerLifeCycle.addBean(inflaterPool);
         containerLifeCycle.addBean(deflaterPool);
     }
 
-    @Override
-    public Map<String, Class<? extends Extension>> getAvailableExtensions()
-    {
-        return availableExtensions;
-    }
-
-    @Override
-    public Class<? extends Extension> getExtension(String name)
-    {
-        return availableExtensions.get(name);
-    }
-
-    @Override
-    public Set<String> getExtensionNames()
-    {
-        return availableExtensions.keySet();
-    }
-
-    @Override
-    public boolean isAvailable(String name)
-    {
-        return availableExtensions.containsKey(name);
-    }
-
     @Override
     public Extension newInstance(ExtensionConfig config)
     {
@@ -139,24 +102,6 @@ public Extension newInstance(ExtensionConfig config)
         }
     }
 
-    @Override
-    public void register(String name, Class<? extends Extension> extension)
-    {
-        availableExtensions.put(name, extension);
-    }
-
-    @Override
-    public void unregister(String name)
-    {
-        availableExtensions.remove(name);
-    }
-
-    @Override
-    public Iterator<Class<? extends Extension>> iterator()
-    {
-        return availableExtensions.values().iterator();
-    }
-
     /* --- All of the below ugliness due to not being able to break API compatibility with ExtensionFactory --- */
 
     @Override

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
@@ -23,7 +23,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.client.HttpClient;
@@ -33,12 +32,9 @@
 import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2;
 import org.eclipse.jetty.unixsocket.UnixSocketConnector;
 import org.eclipse.jetty.unixsocket.client.HttpClientTransportOverUnixSockets;
-import org.eclipse.jetty.util.BlockingArrayQueue;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
-import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledOnJre;
 import org.junit.jupiter.api.condition.JRE;
@@ -311,7 +307,7 @@ public void testLog4j2ModuleWithSimpleWebAppWithJSP() throws Exception
 
     @ParameterizedTest
     @ValueSource(strings = {"http", "https"})
-    public void testWebsocketClientInWebapp(String scheme) throws Exception
+    public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Exception
     {
         Path jettyBase = Files.createTempDirectory("jetty_base");
         String jettyVersion = System.getProperty("jettyVersion");
@@ -331,20 +327,14 @@ public void testWebsocketClientInWebapp(String scheme) throws Exception
             assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
             assertEquals(0, run1.getExitValue());
 
-            File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-webapp:war:" + jettyVersion);
+            File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-provided-webapp:war:" + jettyVersion);
             distribution.installWarFile(webApp, "test");
 
             int port = distribution.freePort();
             String[] args2 = {
                 "jetty.http.port=" + port,
                 "jetty.ssl.port=" + port,
                 // "jetty.server.dumpAfterStart=true",
-                // "jetty.webapp.addSystemClasses+=,org.eclipse.jetty.client.",
-                // "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.client.",
-                // "jetty.webapp.addSystemClasses+=,org.eclipse.jetty.util.ssl.",
-                // "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.util.ssl.",
-                // "jetty.webapp.addSystemClasses+=,org.eclipse.jetty.util.component.",
-                // "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.util.component."
             };
 
             try (DistributionTester.Run run2 = distribution.start(args2))
@@ -365,38 +355,53 @@ public void testWebsocketClientInWebapp(String scheme) throws Exception
         }
     }
 
-    public static class WsListener implements WebSocketListener
+    @ParameterizedTest
+    @ValueSource(strings = {"http", "https"})
+    public void testWebsocketClientInWebapp(String scheme) throws Exception
     {
-        public BlockingArrayQueue<String> textMessages = new BlockingArrayQueue<>();
-        public final CountDownLatch closeLatch = new CountDownLatch(1);
-        public int closeCode;
+        Path jettyBase = Files.createTempDirectory("jetty_base");
+        String jettyVersion = System.getProperty("jettyVersion");
+        DistributionTester distribution = DistributionTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .jettyBase(jettyBase)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
 
-        @Override
-        public void onWebSocketClose(int statusCode, String reason)
+        String[] args1 = {
+            "--create-startd",
+            "--approve-all-licenses",
+            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket," + scheme
+        };
+        try (DistributionTester.Run run1 = distribution.start(args1))
         {
-            this.closeCode = statusCode;
-            closeLatch.countDown();
-        }
+            assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, run1.getExitValue());
 
-        @Override
-        public void onWebSocketConnect(Session session)
-        {
-        }
+            File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-webapp:war:" + jettyVersion);
+            distribution.installWarFile(webApp, "test");
 
-        @Override
-        public void onWebSocketError(Throwable cause)
-        {
-        }
+            int port = distribution.freePort();
+            String[] args2 = {
+                "jetty.http.port=" + port,
+                "jetty.ssl.port=" + port,
+                "jetty.webapp.addServerClasses+=,+org.eclipse.jetty.websocket.",
+                "jetty.webapp.addSystemClasses+=,-org.eclipse.jetty.websocket.",
+                // "jetty.server.dumpAfterStart=true",
+                };
 
-        @Override
-        public void onWebSocketBinary(byte[] payload, int offset, int len)
-        {
-        }
+            try (DistributionTester.Run run2 = distribution.start(args2))
+            {
+                assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS));
 
-        @Override
-        public void onWebSocketText(String message)
-        {
-            textMessages.add(message);
+                // We should get the correct configuration from the jetty-websocket-httpclient.xml file.
+                startHttpClient(scheme.equals("https"));
+                URI serverUri = URI.create(scheme + "://localhost:" + port + "/test");
+                ContentResponse response = client.GET(serverUri);
+                assertEquals(HttpStatus.OK_200, response.getStatus());
+                String content = response.getContentAsString();
+                assertThat(content, containsString("WebSocketEcho: success"));
+                assertThat(content, containsString("ConnectTimeout: 4999"));
+            }
         }
     }
 }
diff --git a/tests/test-webapps/pom.xml b/tests/test-webapps/pom.xml
@@ -44,5 +44,6 @@
     <module>test-weld-cdi-webapp</module>
     <module>test-owb-cdi-webapp</module>
     <module>test-websocket-client-webapp</module>
+    <module>test-websocket-client-provided-webapp</module>
   </modules>
 </project>
diff --git a/tests/test-webapps/test-websocket-client-provided-webapp/pom.xml b/tests/test-webapps/test-websocket-client-provided-webapp/pom.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <parent>
+    <groupId>org.eclipse.jetty.tests</groupId>
+    <artifactId>test-webapps-parent</artifactId>
+    <version>9.4.33-SNAPSHOT</version>
+  </parent>
+
+  <modelVersion>4.0.0</modelVersion>
+  <artifactId>test-websocket-client-provided-webapp</artifactId>
+  <packaging>war</packaging>
+
+  <name>Test :: Jetty Websocket Simple Webapp with WebSocketClient</name>
+
+  <dependencies>
+    <dependency>
+      <groupId>javax.servlet</groupId>
+      <artifactId>javax.servlet-api</artifactId>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>javax.websocket</groupId>
+      <artifactId>javax.websocket-api</artifactId>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.websocket</groupId>
+      <artifactId>websocket-client</artifactId>
+      <version>${project.version}</version>
+      <scope>provided</scope>
+    </dependency>
+  </dependencies>
+</project>
diff --git a/...-provided-webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/EchoEndpoint.java b/...-provided-webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/EchoEndpoint.java
@@ -0,0 +1,32 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.tests.webapp.websocket;
+
+import javax.websocket.OnMessage;
+import javax.websocket.server.ServerEndpoint;
+
+@ServerEndpoint(value = "/echo")
+public class EchoEndpoint
+{
+    @OnMessage
+    public String echo(String message)
+    {
+        return message;
+    }
+}