Skip to content

Commit 12f3c45

Browse files
lachlan-robertslachlan-roberts
authored
Merge pull request #5374 from eclipse/jetty-9.4.x-5320-WebSocketHttpClient
Issue #5320 - using jetty-websocket-httpclient.xml within webapp
2 parents b77ee51 + 60c56d8 commit 12f3c45

File tree

18 files changed

+590
-87
lines changed

18 files changed

+590
-87
lines changed

‎jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,26 @@
3232
@Deprecated
3333
public abstract class ExtensionFactory implements Iterable<Class<? extends Extension>>
3434
{
35-
private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
36-
private Map<String, Class<? extends Extension>> availableExtensions;
35+
private final Map<String, Class<? extends Extension>> availableExtensions;
3736

3837
public ExtensionFactory()
3938
{
4039
availableExtensions = new HashMap<>();
41-
for (Extension ext : extensionLoader)
40+
Iterator<Extension> iterator = ServiceLoader.load(Extension.class).iterator();
41+
while (true)
4242
{
43-
if (ext != null)
43+
try
4444
{
45-
availableExtensions.put(ext.getName(), ext.getClass());
45+
if (!iterator.hasNext())
46+
break;
47+
48+
Extension ext = iterator.next();
49+
if (ext != null)
50+
availableExtensions.put(ext.getName(), ext.getClass());
51+
}
52+
catch (Throwable ignored)
53+
{
54+
// Ignored.
4655
}
4756
}
4857
}

‎jetty-websocket/websocket-client/pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
<groupId>org.eclipse.jetty</groupId>
2525
<artifactId>jetty-xml</artifactId>
2626
<version>${project.version}</version>
27+
<optional>true</optional>
2728
</dependency>
2829
<dependency>
2930
<groupId>org.eclipse.jetty</groupId>

‎jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/HttpClientProvider.java

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,33 +18,16 @@
1818

1919
package org.eclipse.jetty.websocket.client;
2020

21-
import java.lang.reflect.Method;
22-
2321
import org.eclipse.jetty.client.HttpClient;
24-
import org.eclipse.jetty.util.log.Log;
2522
import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
2623

2724
public final class HttpClientProvider
2825
{
2926
public static HttpClient get(WebSocketContainerScope scope)
3027
{
31-
try
32-
{
33-
if (Class.forName("org.eclipse.jetty.xml.XmlConfiguration") != null)
34-
{
35-
Class<?> xmlClazz = Class.forName("org.eclipse.jetty.websocket.client.XmlBasedHttpClientProvider");
36-
Method getMethod = xmlClazz.getMethod("get", WebSocketContainerScope.class);
37-
Object ret = getMethod.invoke(null, scope);
38-
if ((ret != null) && (ret instanceof HttpClient))
39-
{
40-
return (HttpClient)ret;
41-
}
42-
}
43-
}
44-
catch (Throwable ignore)
45-
{
46-
Log.getLogger(HttpClientProvider.class).ignore(ignore);
47-
}
28+
HttpClient httpClient = XmlBasedHttpClientProvider.get(scope);
29+
if (httpClient != null)
30+
return httpClient;
4831

4932
return DefaultHttpClientProvider.newHttpClient(scope);
5033
}

‎jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/XmlBasedHttpClientProvider.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,46 @@
2222

2323
import org.eclipse.jetty.client.HttpClient;
2424
import org.eclipse.jetty.util.log.Log;
25+
import org.eclipse.jetty.util.log.Logger;
26+
import org.eclipse.jetty.util.resource.Resource;
2527
import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
2628
import org.eclipse.jetty.xml.XmlConfiguration;
2729

2830
class XmlBasedHttpClientProvider
2931
{
32+
public static final Logger LOG = Log.getLogger(XmlBasedHttpClientProvider.class);
33+
3034
public static HttpClient get(@SuppressWarnings("unused") WebSocketContainerScope scope)
3135
{
32-
URL resource = Thread.currentThread().getContextClassLoader().getResource("jetty-websocket-httpclient.xml");
36+
ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
37+
if (contextClassLoader == null)
38+
return null;
39+
40+
URL resource = contextClassLoader.getResource("jetty-websocket-httpclient.xml");
3341
if (resource == null)
34-
{
3542
return null;
43+
44+
try
45+
{
46+
Thread.currentThread().setContextClassLoader(HttpClient.class.getClassLoader());
47+
return newHttpClient(resource);
48+
}
49+
finally
50+
{
51+
Thread.currentThread().setContextClassLoader(contextClassLoader);
3652
}
53+
}
3754

55+
private static HttpClient newHttpClient(URL resource)
56+
{
3857
try
3958
{
40-
XmlConfiguration configuration = new XmlConfiguration(resource);
59+
XmlConfiguration configuration = new XmlConfiguration(Resource.newResource(resource));
4160
return (HttpClient)configuration.configure();
4261
}
4362
catch (Throwable t)
4463
{
45-
Log.getLogger(XmlBasedHttpClientProvider.class).warn("Unable to load: " + resource, t);
64+
LOG.warn("Failure to load HttpClient from XML {}", resource, t);
4665
}
4766

4867
return null;

‎jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java

Lines changed: 2 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,6 @@
1919
package org.eclipse.jetty.websocket.common.extensions;
2020

2121
import java.io.IOException;
22-
import java.util.HashMap;
23-
import java.util.Iterator;
24-
import java.util.Map;
25-
import java.util.ServiceLoader;
26-
import java.util.Set;
2722
import java.util.zip.Deflater;
2823

2924
import org.eclipse.jetty.util.StringUtil;
@@ -42,10 +37,8 @@
4237

4338
public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle, Dumpable
4439
{
45-
private ContainerLifeCycle containerLifeCycle;
46-
private WebSocketContainerScope container;
47-
private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
48-
private Map<String, Class<? extends Extension>> availableExtensions;
40+
private final ContainerLifeCycle containerLifeCycle;
41+
private final WebSocketContainerScope container;
4942
private final InflaterPool inflaterPool = new InflaterPool(CompressionPool.INFINITE_CAPACITY, true);
5043
private final DeflaterPool deflaterPool = new DeflaterPool(CompressionPool.INFINITE_CAPACITY, Deflater.DEFAULT_COMPRESSION, true);
5144

@@ -59,42 +52,12 @@ public String toString()
5952
return String.format("%s@%x{%s}", WebSocketExtensionFactory.class.getSimpleName(), hashCode(), containerLifeCycle.getState());
6053
}
6154
};
62-
availableExtensions = new HashMap<>();
63-
for (Extension ext : extensionLoader)
64-
{
65-
if (ext != null)
66-
availableExtensions.put(ext.getName(), ext.getClass());
67-
}
6855

6956
this.container = container;
7057
containerLifeCycle.addBean(inflaterPool);
7158
containerLifeCycle.addBean(deflaterPool);
7259
}
7360

74-
@Override
75-
public Map<String, Class<? extends Extension>> getAvailableExtensions()
76-
{
77-
return availableExtensions;
78-
}
79-
80-
@Override
81-
public Class<? extends Extension> getExtension(String name)
82-
{
83-
return availableExtensions.get(name);
84-
}
85-
86-
@Override
87-
public Set<String> getExtensionNames()
88-
{
89-
return availableExtensions.keySet();
90-
}
91-
92-
@Override
93-
public boolean isAvailable(String name)
94-
{
95-
return availableExtensions.containsKey(name);
96-
}
97-
9861
@Override
9962
public Extension newInstance(ExtensionConfig config)
10063
{
@@ -139,24 +102,6 @@ public Extension newInstance(ExtensionConfig config)
139102
}
140103
}
141104

142-
@Override
143-
public void register(String name, Class<? extends Extension> extension)
144-
{
145-
availableExtensions.put(name, extension);
146-
}
147-
148-
@Override
149-
public void unregister(String name)
150-
{
151-
availableExtensions.remove(name);
152-
}
153-
154-
@Override
155-
public Iterator<Class<? extends Extension>> iterator()
156-
{
157-
return availableExtensions.values().iterator();
158-
}
159-
160105
/* --- All of the below ugliness due to not being able to break API compatibility with ExtensionFactory --- */
161106

162107
@Override

‎tests/test-distribution/pom.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,18 @@
117117
<version>${project.version}</version>
118118
<scope>test</scope>
119119
</dependency>
120+
<dependency>
121+
<groupId>org.eclipse.jetty.websocket</groupId>
122+
<artifactId>websocket-api</artifactId>
123+
<version>${project.version}</version>
124+
<scope>test</scope>
125+
</dependency>
126+
<dependency>
127+
<groupId>org.eclipse.jetty.websocket</groupId>
128+
<artifactId>websocket-client</artifactId>
129+
<version>${project.version}</version>
130+
<scope>test</scope>
131+
</dependency>
120132
<dependency>
121133
<groupId>org.eclipse.jetty.tests</groupId>
122134
<artifactId>test-felix-webapp</artifactId>

‎tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/AbstractDistributionTest.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.function.Supplier;
2222

2323
import org.eclipse.jetty.client.HttpClient;
24+
import org.eclipse.jetty.util.ssl.SslContextFactory;
2425
import org.junit.jupiter.api.AfterEach;
2526

2627
public class AbstractDistributionTest
@@ -29,7 +30,15 @@ public class AbstractDistributionTest
2930

3031
protected void startHttpClient() throws Exception
3132
{
32-
startHttpClient(HttpClient::new);
33+
startHttpClient(false);
34+
}
35+
36+
protected void startHttpClient(boolean secure) throws Exception
37+
{
38+
if (secure)
39+
startHttpClient(() -> new HttpClient(new SslContextFactory.Client(true)));
40+
else
41+
startHttpClient(HttpClient::new);
3342
}
3443

3544
protected void startHttpClient(Supplier<HttpClient> supplier) throws Exception

‎tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.eclipse.jetty.tests.distribution;
2020

2121
import java.io.File;
22+
import java.net.URI;
2223
import java.nio.file.Files;
2324
import java.nio.file.Path;
2425
import java.nio.file.Paths;
@@ -39,6 +40,8 @@
3940
import org.junit.jupiter.api.condition.DisabledOnOs;
4041
import org.junit.jupiter.api.condition.JRE;
4142
import org.junit.jupiter.api.condition.OS;
43+
import org.junit.jupiter.params.ParameterizedTest;
44+
import org.junit.jupiter.params.provider.ValueSource;
4245

4346
import static org.hamcrest.MatcherAssert.assertThat;
4447
import static org.hamcrest.Matchers.containsString;
@@ -304,4 +307,103 @@ public void testLog4j2ModuleWithSimpleWebAppWithJSP() throws Exception
304307
IO.delete(jettyBase.toFile());
305308
}
306309
}
310+
311+
@ParameterizedTest
312+
@ValueSource(strings = {"http", "https"})
313+
public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Exception
314+
{
315+
Path jettyBase = Files.createTempDirectory("jetty_base");
316+
String jettyVersion = System.getProperty("jettyVersion");
317+
DistributionTester distribution = DistributionTester.Builder.newInstance()
318+
.jettyVersion(jettyVersion)
319+
.jettyBase(jettyBase)
320+
.mavenLocalRepository(System.getProperty("mavenRepoPath"))
321+
.build();
322+
323+
String[] args1 = {
324+
"--create-startd",
325+
"--approve-all-licenses",
326+
"--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket," + scheme
327+
};
328+
try (DistributionTester.Run run1 = distribution.start(args1))
329+
{
330+
assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
331+
assertEquals(0, run1.getExitValue());
332+
333+
File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-provided-webapp:war:" + jettyVersion);
334+
distribution.installWarFile(webApp, "test");
335+
336+
int port = distribution.freePort();
337+
String[] args2 = {
338+
"jetty.http.port=" + port,
339+
"jetty.ssl.port=" + port,
340+
// "jetty.server.dumpAfterStart=true",
341+
};
342+
343+
try (DistributionTester.Run run2 = distribution.start(args2))
344+
{
345+
assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS));
346+
347+
// We should get the correct configuration from the jetty-websocket-httpclient.xml file.
348+
startHttpClient(scheme.equals("https"));
349+
URI serverUri = URI.create(scheme + "://localhost:" + port + "/test");
350+
ContentResponse response = client.GET(serverUri);
351+
assertEquals(HttpStatus.OK_200, response.getStatus());
352+
String content = response.getContentAsString();
353+
assertThat(content, containsString("WebSocketEcho: success"));
354+
assertThat(content, containsString("ConnectTimeout: 4999"));
355+
}
356+
}
357+
}
358+
359+
@ParameterizedTest
360+
@ValueSource(strings = {"http", "https"})
361+
public void testWebsocketClientInWebapp(String scheme) throws Exception
362+
{
363+
Path jettyBase = Files.createTempDirectory("jetty_base");
364+
String jettyVersion = System.getProperty("jettyVersion");
365+
DistributionTester distribution = DistributionTester.Builder.newInstance()
366+
.jettyVersion(jettyVersion)
367+
.jettyBase(jettyBase)
368+
.mavenLocalRepository(System.getProperty("mavenRepoPath"))
369+
.build();
370+
371+
String[] args1 = {
372+
"--create-startd",
373+
"--approve-all-licenses",
374+
"--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket," + scheme
375+
};
376+
try (DistributionTester.Run run1 = distribution.start(args1))
377+
{
378+
assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
379+
assertEquals(0, run1.getExitValue());
380+
381+
File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-webapp:war:" + jettyVersion);
382+
distribution.installWarFile(webApp, "test");
383+
384+
int port = distribution.freePort();
385+
String[] args2 = {
386+
"jetty.http.port=" + port,
387+
"jetty.ssl.port=" + port,
388+
// We must hide the websocket classes from the webapp if we are to include websocket client jars in WEB-INF/lib.
389+
"jetty.webapp.addServerClasses+=,+org.eclipse.jetty.websocket.",
390+
"jetty.webapp.addSystemClasses+=,-org.eclipse.jetty.websocket.",
391+
// "jetty.server.dumpAfterStart=true",
392+
};
393+
394+
try (DistributionTester.Run run2 = distribution.start(args2))
395+
{
396+
assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS));
397+
398+
// We should get the correct configuration from the jetty-websocket-httpclient.xml file.
399+
startHttpClient(scheme.equals("https"));
400+
URI serverUri = URI.create(scheme + "://localhost:" + port + "/test");
401+
ContentResponse response = client.GET(serverUri);
402+
assertEquals(HttpStatus.OK_200, response.getStatus());
403+
String content = response.getContentAsString();
404+
assertThat(content, containsString("WebSocketEcho: success"));
405+
assertThat(content, containsString("ConnectTimeout: 4999"));
406+
}
407+
}
408+
}
307409
}

‎tests/test-webapps/pom.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,7 @@
4343
<module>test-cdi-common-webapp</module>
4444
<module>test-weld-cdi-webapp</module>
4545
<module>test-owb-cdi-webapp</module>
46+
<module>test-websocket-client-webapp</module>
47+
<module>test-websocket-client-provided-webapp</module>
4648
</modules>
4749
</project>

0 commit comments

Comments
 (0)