Skip to content

Commit

Permalink
Issue #3428 - changes from review
Browse files Browse the repository at this point in the history
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
lachlan-roberts committed Jun 29, 2020
1 parent 35e82c8 commit 5da4d7e
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 84 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ public void removeMessageHandler(MessageHandler handler)
this.binarySink = null;
break;
default:
throw new IllegalStateException();
throw new IllegalStateException("Invalid MessageHandler type " + OpCode.name(key));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.lang.reflect.InvocationTargetException;
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.Map;
Expand All @@ -37,6 +38,7 @@
import javax.websocket.OnError;
import javax.websocket.OnMessage;
import javax.websocket.OnOpen;
import javax.websocket.PongMessage;
import javax.websocket.Session;

import org.eclipse.jetty.http.pathmap.UriTemplatePathSpec;
Expand All @@ -57,7 +59,6 @@
import org.eclipse.jetty.websocket.util.messages.PartialStringMessageSink;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jetty.websocket.javax.common.JavaxWebSocketCallingArgs.getArgsFor;

public abstract class JavaxWebSocketFrameHandlerFactory
{
Expand All @@ -76,6 +77,34 @@ public abstract class JavaxWebSocketFrameHandlerFactory
}
}

static InvokerUtils.Arg[] getArgsFor(Class<?> objectType)
{
return new InvokerUtils.Arg[]{new InvokerUtils.Arg(Session.class), new InvokerUtils.Arg(objectType).required()};
}

static final InvokerUtils.Arg[] textPartialCallingArgs = new InvokerUtils.Arg[]{
new InvokerUtils.Arg(Session.class),
new InvokerUtils.Arg(String.class).required(),
new InvokerUtils.Arg(boolean.class).required()
};

static final InvokerUtils.Arg[] binaryPartialBufferCallingArgs = new InvokerUtils.Arg[]{
new InvokerUtils.Arg(Session.class),
new InvokerUtils.Arg(ByteBuffer.class).required(),
new InvokerUtils.Arg(boolean.class).required()
};

static final InvokerUtils.Arg[] binaryPartialArrayCallingArgs = new InvokerUtils.Arg[]{
new InvokerUtils.Arg(Session.class),
new InvokerUtils.Arg(byte[].class).required(),
new InvokerUtils.Arg(boolean.class).required()
};

static final InvokerUtils.Arg[] pongCallingArgs = new InvokerUtils.Arg[]{
new InvokerUtils.Arg(Session.class),
new InvokerUtils.Arg(PongMessage.class).required()
};

protected final JavaxWebSocketContainer container;
protected final InvokerUtils.ParamIdentifier paramIdentifier;

Expand Down Expand Up @@ -327,7 +356,7 @@ private boolean matchOnMessage(Method onMsg, JavaxWebSocketFrameHandlerMetadata
Function<InvokerUtils.Arg[], MethodHandle> getMethodHandle)
{
// Partial Text Message.
MethodHandle methodHandle = getMethodHandle.apply(JavaxWebSocketCallingArgs.textPartialCallingArgs);
MethodHandle methodHandle = getMethodHandle.apply(textPartialCallingArgs);
if (methodHandle != null)
{
msgMetadata.setSinkClass(PartialStringMessageSink.class);
Expand All @@ -337,7 +366,7 @@ private boolean matchOnMessage(Method onMsg, JavaxWebSocketFrameHandlerMetadata
}

// Partial ByteBuffer Binary Message.
methodHandle = getMethodHandle.apply(JavaxWebSocketCallingArgs.binaryPartialBufferCallingArgs);
methodHandle = getMethodHandle.apply(binaryPartialBufferCallingArgs);
if (methodHandle != null)
{
msgMetadata.setSinkClass(PartialByteBufferMessageSink.class);
Expand All @@ -347,7 +376,7 @@ private boolean matchOnMessage(Method onMsg, JavaxWebSocketFrameHandlerMetadata
}

// Partial byte[] Binary Message.
methodHandle = getMethodHandle.apply(JavaxWebSocketCallingArgs.binaryPartialArrayCallingArgs);
methodHandle = getMethodHandle.apply(binaryPartialArrayCallingArgs);
if (methodHandle != null)
{
msgMetadata.setSinkClass(PartialByteArrayMessageSink.class);
Expand All @@ -357,7 +386,7 @@ private boolean matchOnMessage(Method onMsg, JavaxWebSocketFrameHandlerMetadata
}

// Pong Message.
MethodHandle pongHandle = getMethodHandle.apply(JavaxWebSocketCallingArgs.pongCallingArgs);
MethodHandle pongHandle = getMethodHandle.apply(pongCallingArgs);
if (pongHandle != null)
{
metadata.setPongHandle(pongHandle, onMsg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ private void add(Class<? extends Decoder> decoder, Class<? extends Decoder> inte
return;
}

// TODO: explain ordering of Decoders and why this is added at position 0.
registeredDecoders.add(0, new RegisteredDecoder(decoder, interfaceClass, objectType, config));
}

Expand All @@ -178,7 +179,8 @@ public List<RegisteredDecoder> getRegisteredDecoders(Class<?> returnType)
public List<RegisteredDecoder> getRegisteredDecoders(Class<? extends Decoder> interfaceType, Class<?> returnType)
{
return registeredDecoders.stream()
.filter(registered -> registered.interfaceType.equals(interfaceType) && registered.isType(returnType))
.filter(registered -> registered.interfaceType.equals(interfaceType))
.filter(registered -> registered.isType(returnType))
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@

public abstract class AbstractDecodedMessageSink implements MessageSink
{
protected final Logger _logger;
protected final CoreSession _coreSession;
protected final MethodHandle _methodHandle;
protected final MessageSink _messageSink;
private static final Logger LOG = LoggerFactory.getLogger(AbstractDecodedMessageSink.class);

private final CoreSession _coreSession;
private final MethodHandle _methodHandle;
private final MessageSink _messageSink;

public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHandle)
{
_logger = LoggerFactory.getLogger(getClass());
_coreSession = coreSession;
_methodHandle = methodHandle;

Expand All @@ -55,6 +55,16 @@ public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHa
}
}

public CoreSession getCoreSession()
{
return _coreSession;
}

public MethodHandle getMethodHandle()
{
return _methodHandle;
}

/**
* @return a message sink which will first decode the message then pass it to {@link #_methodHandle}.
* @throws Exception for any error in creating the message sink.
Expand All @@ -64,8 +74,8 @@ public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHa
@Override
public void accept(Frame frame, Callback callback)
{
if (_logger.isDebugEnabled())
_logger.debug("accepting frame {} for {}", frame, _messageSink);
if (LOG.isDebugEnabled())
LOG.debug("accepting frame {} for {}", frame, _messageSink);
_messageSink.accept(frame, callback);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@
import org.eclipse.jetty.websocket.javax.common.decoders.RegisteredDecoder;
import org.eclipse.jetty.websocket.util.messages.ByteBufferMessageSink;
import org.eclipse.jetty.websocket.util.messages.MessageSink;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class DecodedBinaryMessageSink<T> extends AbstractDecodedMessageSink.Basic<Decoder.Binary<T>>
{
private static final Logger LOG = LoggerFactory.getLogger(DecodedBinaryMessageSink.class);

public DecodedBinaryMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(session, methodHandle, decoders);
Expand All @@ -43,10 +47,10 @@ public DecodedBinaryMessageSink(CoreSession session, MethodHandle methodHandle,
@Override
MessageSink getMessageSink() throws Exception
{
MethodHandle methodHandle = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedBinaryMessageSink.class,
"onWholeMessage", MethodType.methodType(void.class, ByteBuffer.class))
MethodHandle methodHandle = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup()
.findVirtual(DecodedBinaryMessageSink.class, "onWholeMessage", MethodType.methodType(void.class, ByteBuffer.class))
.bindTo(this);
return new ByteBufferMessageSink(_coreSession, methodHandle);
return new ByteBufferMessageSink(getCoreSession(), methodHandle);
}

@SuppressWarnings("Duplicates")
Expand All @@ -59,7 +63,7 @@ public void onWholeMessage(ByteBuffer wholeMessage)
try
{
T obj = decoder.decode(wholeMessage);
_methodHandle.invoke(obj);
getMethodHandle().invoke(obj);
return;
}
catch (DecodeException e)
Expand All @@ -73,6 +77,6 @@ public void onWholeMessage(ByteBuffer wholeMessage)
}
}

_logger.warn("Message lost, willDecode() has returned false for all decoders in the decoder list.");
LOG.warn("Message lost, willDecode() has returned false for all decoders in the decoder list.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ MessageSink getMessageSink() throws Exception
MethodHandle methodHandle = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedBinaryStreamMessageSink.class,
"onStreamStart", MethodType.methodType(void.class, InputStream.class))
.bindTo(this);
return new InputStreamMessageSink(_coreSession, methodHandle);
return new InputStreamMessageSink(getCoreSession(), methodHandle);
}

@SuppressWarnings("Duplicates")
Expand All @@ -55,7 +55,7 @@ public void onStreamStart(InputStream stream)
try
{
T obj = _decoder.decode(stream);
_methodHandle.invoke(obj);
getMethodHandle().invoke(obj);
}
catch (DecodeException e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@
import org.eclipse.jetty.websocket.javax.common.decoders.RegisteredDecoder;
import org.eclipse.jetty.websocket.util.messages.MessageSink;
import org.eclipse.jetty.websocket.util.messages.StringMessageSink;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class DecodedTextMessageSink<T> extends AbstractDecodedMessageSink.Basic<Decoder.Text<T>>
{
private static final Logger LOG = LoggerFactory.getLogger(DecodedTextMessageSink.class);

public DecodedTextMessageSink(CoreSession session, MethodHandle methodHandle, List<RegisteredDecoder> decoders)
{
super(session, methodHandle, decoders);
Expand All @@ -45,7 +49,7 @@ MessageSink getMessageSink() throws NoSuchMethodException, IllegalAccessExceptio
MethodHandle methodHandle = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup()
.findVirtual(getClass(), "onMessage", MethodType.methodType(void.class, String.class))
.bindTo(this);
return new StringMessageSink(_coreSession, methodHandle);
return new StringMessageSink(getCoreSession(), methodHandle);
}

@SuppressWarnings("Duplicates")
Expand All @@ -58,7 +62,7 @@ public void onMessage(String wholeMessage)
try
{
T obj = decoder.decode(wholeMessage);
_methodHandle.invoke(obj);
getMethodHandle().invoke(obj);
return;
}
catch (DecodeException e)
Expand All @@ -72,6 +76,6 @@ public void onMessage(String wholeMessage)
}
}

_logger.warn("Message lost, willDecode() has returned false for all decoders in the decoder list.");
LOG.warn("Message lost, willDecode() has returned false for all decoders in the decoder list.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ MessageSink getMessageSink() throws Exception
MethodHandle methodHandle = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedTextStreamMessageSink.class,
"onStreamStart", MethodType.methodType(void.class, Reader.class))
.bindTo(this);
return new ReaderMessageSink(_coreSession, methodHandle);
return new ReaderMessageSink(getCoreSession(), methodHandle);
}

@SuppressWarnings("Duplicates")
Expand All @@ -55,7 +55,7 @@ public void onStreamStart(Reader reader)
try
{
T obj = _decoder.decode(reader);
_methodHandle.invoke(obj);
getMethodHandle().invoke(obj);
}
catch (DecodeException e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ public void accept(Calendar cal)
@SuppressWarnings("Duplicates")
public static class GmtDecoder implements Decoder.Binary<Calendar>
{

@Override
public Calendar decode(ByteBuffer buffer) throws DecodeException
{
Expand Down

0 comments on commit 5da4d7e

Please sign in to comment.