From 586484850ff7fe6f791bb413bd44e2ebe8aa3cee Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 2 Aug 2018 09:16:34 -0500 Subject: [PATCH] Issue #2768 - JSR356 Session.getUserProperties() is null during @OnOpen Signed-off-by: Joakim Erdfelt --- .../websocket/jsr356/ConfiguratorTest.java | 5 ++++- .../websocket/jsr356/EchoCaptureHandler.java | 6 ++++-- .../websocket/jsr356/EndpointEchoClient.java | 1 + .../websocket/jsr356/EndpointEchoTest.java | 18 ++++++++++++------ .../jetty/websocket/jsr356/MessageQueue.java | 4 ++++ .../jsr356/samples/EchoStringEndpoint.java | 18 ++++++++++++++---- 6 files changed, 39 insertions(+), 13 deletions(-) diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/ConfiguratorTest.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/ConfiguratorTest.java index a2b01843f0f..fd4694c320d 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/ConfiguratorTest.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/ConfiguratorTest.java @@ -18,7 +18,9 @@ package org.eclipse.jetty.websocket.jsr356; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.is; import java.net.URI; import java.util.List; @@ -125,7 +127,8 @@ public class ConfiguratorTest session.getBasicRemote().sendText("Echo"); // Wait for echo - echoer.textCapture.messageQueue.awaitMessages(1,1000,TimeUnit.MILLISECONDS); + String echoed = echoer.textCapture.messages.poll(1,TimeUnit.SECONDS); + assertThat("Echoed", echoed, is("Echo")); // Validate client side configurator use Assert.assertThat("configurator.request",configurator.request,notNullValue()); diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EchoCaptureHandler.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EchoCaptureHandler.java index 8c7394cec17..d114b7f27f0 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EchoCaptureHandler.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EchoCaptureHandler.java @@ -20,13 +20,15 @@ package org.eclipse.jetty.websocket.jsr356; import javax.websocket.MessageHandler; +import org.eclipse.jetty.util.BlockingArrayQueue; + public class EchoCaptureHandler implements MessageHandler.Whole { - public MessageQueue messageQueue = new MessageQueue(); + public BlockingArrayQueue messages = new BlockingArrayQueue<>(); @Override public void onMessage(String message) { - messageQueue.offer(message); + messages.offer(message); } } diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoClient.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoClient.java index 0180f5f6ed1..23a00b71887 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoClient.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoClient.java @@ -52,6 +52,7 @@ public class EndpointEchoClient extends Endpoint if (LOG.isDebugEnabled()) LOG.debug("onOpen({}, {})",session,config); this.session = session; + this.session.getUserProperties().put("endpoint", this); Assert.assertThat("Session is required",session,notNullValue()); Assert.assertThat("EndpointConfig is required",config,notNullValue()); this.session.addMessageHandler(textCapture); diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoTest.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoTest.java index 3ec1d44c472..ac1a0be72dc 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoTest.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/EndpointEchoTest.java @@ -18,7 +18,9 @@ package org.eclipse.jetty.websocket.jsr356; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; import java.net.URI; import java.util.concurrent.TimeUnit; @@ -98,7 +100,8 @@ public class EndpointEchoTest session.getBasicRemote().sendText("Echo"); if (LOG.isDebugEnabled()) LOG.debug("Client Message Sent"); - echoer.textCapture.messageQueue.awaitMessages(1,1000,TimeUnit.MILLISECONDS); + String echoed = echoer.textCapture.messages.poll(1, TimeUnit.SECONDS); + assertThat("Echoed message", echoed, is("Echo")); } @Test @@ -113,8 +116,9 @@ public class EndpointEchoTest session.getBasicRemote().sendText("Echo"); if (LOG.isDebugEnabled()) LOG.debug("Client Message Sent"); - // TODO: figure out echo verification. - // echoer.textCapture.messageQueue.awaitMessages(1,1000,TimeUnit.MILLISECONDS); + EndpointEchoClient client = (EndpointEchoClient) session.getUserProperties().get("endpoint"); + String echoed = client.textCapture.messages.poll(1, TimeUnit.SECONDS); + assertThat("Echoed message", echoed, is("Echo")); } @Test @@ -131,7 +135,8 @@ public class EndpointEchoTest session.getBasicRemote().sendText("Echo"); if (LOG.isDebugEnabled()) LOG.debug("Client Message Sent"); - echoer.messageQueue.awaitMessages(1,1000,TimeUnit.MILLISECONDS); + String echoed = echoer.messages.poll(1, TimeUnit.SECONDS); + assertThat("Echoed message", echoed, is("Echo")); } @Test @@ -146,7 +151,8 @@ public class EndpointEchoTest session.getBasicRemote().sendText("Echo"); if (LOG.isDebugEnabled()) LOG.debug("Client Message Sent"); - // TODO: figure out echo verification. - // echoer.messageQueue.awaitMessages(1,1000,TimeUnit.MILLISECONDS); + EchoStringEndpoint client = (EchoStringEndpoint) session.getUserProperties().get("endpoint"); + String echoed = client.messages.poll(1, TimeUnit.SECONDS); + assertThat("Echoed message", echoed, is("Echo")); } } diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageQueue.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageQueue.java index c0add237272..a46f5fb4b20 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageQueue.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageQueue.java @@ -25,6 +25,10 @@ import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +/** + * @deprecated use {@code BlockingArrayQueue} instead + */ +@Deprecated public class MessageQueue extends BlockingArrayQueue { private static final Logger LOG = Log.getLogger(MessageQueue.class); diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/samples/EchoStringEndpoint.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/samples/EchoStringEndpoint.java index b0cdb0d1b2f..207e01c5ea1 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/samples/EchoStringEndpoint.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/samples/EchoStringEndpoint.java @@ -18,19 +18,29 @@ package org.eclipse.jetty.websocket.jsr356.samples; -import org.eclipse.jetty.websocket.jsr356.MessageQueue; +import javax.websocket.EndpointConfig; +import javax.websocket.Session; + +import org.eclipse.jetty.util.BlockingArrayQueue; /** * Legitimate structure for an Endpoint */ public class EchoStringEndpoint extends AbstractStringEndpoint { - public MessageQueue messageQueue = new MessageQueue(); + public BlockingArrayQueue messages = new BlockingArrayQueue<>(); + + @Override + public void onOpen(Session session, EndpointConfig config) + { + super.onOpen(session, config); + this.session.getUserProperties().put("endpoint", this); + } @Override public void onMessage(String message) { - messageQueue.offer(message); - session.getAsyncRemote().sendText(message); + this.messages.offer(message); + this.session.getAsyncRemote().sendText(message); } }