From 75c5793f38f115e41252e92d49fc34fcdbd91cc0 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 13 Oct 2016 15:58:34 -0700 Subject: [PATCH] Issue #207 - @PathParam support fixes + Arg.tag is now exposed for general use + JsrEndpointFunctions now decode (into primitives) the URI template exposed @PathParam static arguments + JsrEndpointFunctions simplified tracking of static args + ServerContainer.assertValidEndpoint() now validates added endpoints with @PathParam immediately (per spec) using a UriTemplate where each variable is an empty string --- .../jsr356/function/JsrEndpointFunctions.java | 76 ++------ ...intFunctions_OnMessage_TextStreamTest.java | 3 +- .../jsr356/server/ServerContainer.java | 8 + .../WebSocketServerContainerInitializer.java | 9 - ...intFunctions_OnMessage_TextStreamTest.java | 170 ++++++++++++++++++ .../jetty/websocket/common/reflect/Arg.java | 9 +- 6 files changed, 202 insertions(+), 73 deletions(-) create mode 100644 jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/JsrServerEndpointFunctions_OnMessage_TextStreamTest.java diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions.java index c33a780837a..c88495d1d29 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions.java @@ -23,14 +23,12 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.List; +import java.util.Collections; import java.util.Map; import java.util.NoSuchElementException; import java.util.concurrent.Executor; import java.util.function.BiFunction; import java.util.function.Function; -import java.util.stream.Collectors; import javax.websocket.ClientEndpoint; import javax.websocket.CloseReason; @@ -122,36 +120,10 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions } } - /** - * Represents a static value (as seen from a URI PathParam) - *

- * The decoding of the raw String to a object occurs later, - * when the callable/sink/function is created for a method - * that needs it converted to an object. - *

- */ - protected static class StaticArg implements Comparable - { - public final String name; - public final String value; - - public StaticArg(String name, String value) - { - this.name = name; - this.value = value; - } - - @Override - public int compareTo(StaticArg o) - { - return this.name.compareTo(o.name); - } - } - protected final AvailableEncoders encoders; protected final AvailableDecoders decoders; private final EndpointConfig endpointConfig; - private List staticArgs; + private Map staticArgs; public JsrEndpointFunctions(Object endpoint, WebSocketPolicy policy, Executor executor, AvailableEncoders encoders, AvailableDecoders decoders, @@ -164,11 +136,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions if (uriParams != null) { - this.staticArgs = new ArrayList<>(); - this.staticArgs.addAll(uriParams.entrySet().stream() - .map(entry -> new StaticArg(entry.getKey(), entry.getValue())) - .sorted() - .collect(Collectors.toList())); + this.staticArgs = Collections.unmodifiableMap(uriParams); } } @@ -606,7 +574,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions StringBuilder err = new StringBuilder(); err.append("@OnMessage.maxMessageSize=").append(annotation.maxMessageSize()); err.append(" not valid for PongMesssage types: "); - ReflectUtils.append(err,onMsg); + ReflectUtils.append(err, onMsg); LOG.warn(err.toString()); } return true; @@ -644,7 +612,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions StringBuilder err = new StringBuilder(); err.append("@OnMessage.maxMessageSize=").append(annotation.maxMessageSize()); err.append(" not valid for Streaming Binary types: "); - ReflectUtils.append(err,onMsg); + ReflectUtils.append(err, onMsg); LOG.warn(err.toString()); } return true; @@ -683,7 +651,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions StringBuilder err = new StringBuilder(); err.append("@OnMessage.maxMessageSize=").append(annotation.maxMessageSize()); err.append(" not valid for Streaming Text types: "); - ReflectUtils.append(err,onMsg); + ReflectUtils.append(err, onMsg); LOG.warn(err.toString()); } return true; @@ -721,7 +689,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions StringBuilder err = new StringBuilder(); err.append("@OnMessage.maxMessageSize=").append(annotation.maxMessageSize()); err.append(" not valid for Partial Binary Buffer types: "); - ReflectUtils.append(err,onMsg); + ReflectUtils.append(err, onMsg); LOG.warn(err.toString()); } return true; @@ -757,7 +725,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions StringBuilder err = new StringBuilder(); err.append("@OnMessage.maxMessageSize=").append(annotation.maxMessageSize()); err.append(" not valid for Partial Binary Array types: "); - ReflectUtils.append(err,onMsg); + ReflectUtils.append(err, onMsg); LOG.warn(err.toString()); } return true; @@ -794,7 +762,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions StringBuilder err = new StringBuilder(); err.append("@OnMessage.maxMessageSize=").append(annotation.maxMessageSize()); err.append(" not valid for Partial Text types: "); - ReflectUtils.append(err,onMsg); + ReflectUtils.append(err, onMsg); LOG.warn(err.toString()); } return true; @@ -914,6 +882,7 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions return; } + // Test specific return type to ensure we have a compatible encoder for it Class encoderClass = encoders.getEncoderFor(returnType); if (encoderClass == null) { @@ -930,26 +899,16 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions Object[] args = new Object[callArgs.length]; for (int i = 0; i < len; i++) { - Object staticValue = getDecodedStaticValue(callArgs[i].getName(), callArgs[i].getType()); - if (staticValue != null) - { - args[i] = staticValue; - } + String staticRawValue = staticArgs.get(callArgs[i].getTag()); + args[i] = AvailableDecoders.decodePrimitive(staticRawValue, callArgs[i].getType()); } return args; } private Object getDecodedStaticValue(String name, Class type) throws DecodeException { - for (StaticArg args : staticArgs) - { - if (args.name.equals(name)) - { - return AvailableDecoders.decodePrimitive(args.value, type); - } - } - - return null; + String value = staticArgs.get(name); + return AvailableDecoders.decodePrimitive(value, type); } private DynamicArgs.Builder createDynamicArgs(Arg... args) @@ -975,12 +934,9 @@ public class JsrEndpointFunctions extends CommonEndpointFunctions if (this.staticArgs != null) { - for (StaticArg staticArg : this.staticArgs) + for (Map.Entry entry : staticArgs.entrySet()) { - // TODO: translate from UriParam String to method param type? - // TODO: shouldn't this be the Arg seen in the method? - // TODO: use static decoder? - callArgs[idx++] = new Arg(staticArg.value.getClass()).setTag(staticArg.name); + callArgs[idx++] = new Arg(entry.getValue().getClass()).setTag(entry.getKey()); } } return callArgs; diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions_OnMessage_TextStreamTest.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions_OnMessage_TextStreamTest.java index a781b48edb9..f98eb27f11e 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions_OnMessage_TextStreamTest.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/function/JsrEndpointFunctions_OnMessage_TextStreamTest.java @@ -126,7 +126,7 @@ public class JsrEndpointFunctions_OnMessage_TextStreamTest } @Test - public void testInvokeMessageText() throws Exception + public void testInvokeMessageStream() throws Exception { TrackingSocket socket = performOnMessageInvocation(new MessageStreamSocket(), (endpoint) -> { @@ -135,5 +135,4 @@ public class JsrEndpointFunctions_OnMessage_TextStreamTest }); socket.assertEvent("onMessage(MessageReader) = \"Hello World\""); } - } diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java index aded955b9e6..fb4895922cd 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java +++ b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java @@ -176,6 +176,14 @@ public class ServerContainer extends ClientContainer implements javax.websocket. AvailableEncoders availableEncoders = new AvailableEncoders(config); AvailableDecoders availableDecoders = new AvailableDecoders(config); Map pathParameters = new HashMap<>(); + + // if any pathspec has a URI Template with variables, we should include them (as empty String value) + // in the test for validity of the declared @OnMessage methods that use @PathParam annotation + for (String variable : new UriTemplatePathSpec(config.getPath()).getVariables()) + { + pathParameters.put(variable, ""); + } + endpointFunctions = newJsrEndpointFunction(endpoint, availableEncoders, availableDecoders, pathParameters, config); endpointFunctions.start(); // this should trigger an exception if endpoint is invalid. } diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java index 10ff77af90f..bcca2b567f6 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java +++ b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/deploy/WebSocketServerContainerInitializer.java @@ -251,15 +251,6 @@ public class WebSocketServerContainerInitializer implements ServletContainerInit ServerContainer jettyContainer = configureContext(context,jettyContext); context.addListener(new ContextDestroyListener()); // make sure context is cleaned up when the context stops -// // Establish the DecoratedObjectFactory thread local -// // for various ServiceLoader initiated components to use. -// DecoratedObjectFactory instantiation = (DecoratedObjectFactory)context.getAttribute(DecoratedObjectFactory.ATTR); -// if (instantiation == null) -// { -// LOG.info("Using WebSocket local DecoratedObjectFactory - none found in ServletContext"); -// instantiation = new DecoratedObjectFactory(); -// } - if (LOG.isDebugEnabled()) { LOG.debug("Found {} classes",c.size()); diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/JsrServerEndpointFunctions_OnMessage_TextStreamTest.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/JsrServerEndpointFunctions_OnMessage_TextStreamTest.java new file mode 100644 index 00000000000..a731e554bb8 --- /dev/null +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/JsrServerEndpointFunctions_OnMessage_TextStreamTest.java @@ -0,0 +1,170 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// 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.websocket.jsr356.server; + +import java.io.IOException; +import java.io.Reader; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; + +import javax.websocket.ClientEndpointConfig; +import javax.websocket.EndpointConfig; +import javax.websocket.OnMessage; +import javax.websocket.server.PathParam; +import javax.websocket.server.ServerEndpoint; + +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.websocket.api.WebSocketPolicy; +import org.eclipse.jetty.websocket.common.function.EndpointFunctions; +import org.eclipse.jetty.websocket.common.test.DummyConnection; +import org.eclipse.jetty.websocket.jsr356.ClientContainer; +import org.eclipse.jetty.websocket.jsr356.ConfiguredEndpoint; +import org.eclipse.jetty.websocket.jsr356.JsrSession; +import org.eclipse.jetty.websocket.jsr356.client.EmptyClientEndpointConfig; +import org.eclipse.jetty.websocket.jsr356.decoders.AvailableDecoders; +import org.eclipse.jetty.websocket.jsr356.encoders.AvailableEncoders; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class JsrServerEndpointFunctions_OnMessage_TextStreamTest +{ + private static ClientContainer container; + + @BeforeClass + public static void initContainer() + { + container = new ClientContainer(); + } + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private AvailableEncoders encoders; + private AvailableDecoders decoders; + private Map uriParams = new HashMap<>(); + private EndpointConfig endpointConfig; + + public JsrServerEndpointFunctions_OnMessage_TextStreamTest() + { + endpointConfig = new EmptyClientEndpointConfig(); + encoders = new AvailableEncoders(endpointConfig); + decoders = new AvailableDecoders(endpointConfig); + uriParams = new HashMap<>(); + uriParams.put("param", "foo"); + } + + public JsrSession newSession(Object websocket) + { + String id = JsrServerEndpointFunctions_OnMessage_TextStreamTest.class.getSimpleName(); + URI requestURI = URI.create("ws://localhost/" + id); + WebSocketPolicy policy = WebSocketPolicy.newClientPolicy(); + DummyConnection connection = new DummyConnection(policy); + ClientEndpointConfig config = new EmptyClientEndpointConfig(); + ConfiguredEndpoint ei = new ConfiguredEndpoint(websocket, config); + return new JsrSession(container, id, requestURI, ei, policy, connection); + } + + @SuppressWarnings("Duplicates") + private TrackingSocket performOnMessageInvocation(TrackingSocket socket, Function func) throws Exception + { + // Establish endpoint function + JsrServerEndpointFunctions endpointFunctions = new JsrServerEndpointFunctions( + socket, container.getPolicy(), + container.getExecutor(), + encoders, + decoders, + uriParams, + endpointConfig + ); + endpointFunctions.start(); + + // This invocation is the same for all tests + endpointFunctions.onOpen(newSession(socket)); + + func.apply(endpointFunctions); + + return socket; + } + + @ServerEndpoint("/msg") + public static class MessageStreamSocket extends TrackingSocket + { + @OnMessage + public void onMessage(Reader stream) + { + try + { + String msg = IO.toString(stream); + addEvent("onMessage(%s) = \"%s\"", stream.getClass().getSimpleName(), msg); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + } + } + + @Test + public void testInvokeMessageStream() throws Exception + { + TrackingSocket socket = performOnMessageInvocation(new MessageStreamSocket(), (endpoint) -> + { + endpoint.onText(BufferUtil.toBuffer("Hello World", StandardCharsets.UTF_8), true); + return null; + }); + socket.assertEvent("onMessage(MessageReader) = \"Hello World\""); + } + + @ServerEndpoint("/msg/{param}") + public static class MessageStreamParamSocket extends TrackingSocket + { + @OnMessage + public String onMessage(Reader stream, @PathParam("param") String param) throws IOException + { + try + { + String msg = IO.toString(stream); + addEvent("onMessage(%s,%s) = \"%s\"", stream.getClass().getSimpleName(), param, msg); + return msg; + } + catch (IOException e) + { + throw new RuntimeException(e); + } + } + } + + @Test + public void testInvokeMessageStreamParam() throws Exception + { + TrackingSocket socket = performOnMessageInvocation(new MessageStreamParamSocket(), (endpoint) -> + { + endpoint.onText(BufferUtil.toBuffer("Hello World", StandardCharsets.UTF_8), true); + return null; + }); + socket.assertEvent("onMessage(MessageReader,foo) = \"Hello World\""); + } + +} diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/reflect/Arg.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/reflect/Arg.java index 4f54a22b39f..ba65e7cfe96 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/reflect/Arg.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/reflect/Arg.java @@ -29,7 +29,7 @@ public class Arg private final Class type; private Method method; private int index; - private Object tag; + private String tag; private boolean required = false; public Arg(Class type) @@ -87,7 +87,12 @@ public class Arg { return type.getName(); } - + + public String getTag() + { + return tag; + } + public Class getType() { return type;