Add MessagePack support for Java SignalR Client

[wtgodbe]

Resolves #21644

Things to call out in docs:

1. Users should pass in a Type when calling HubConnection.on(), HubConnection.invoke(), or HubConnection.stream(). You can get a Type w/ something like new TypeReference<ArrayList<Integer>>() { }).getType(). The exception is primitives, for which you should use, e.g., int.class. The methods mentioned are generic, but have no generic parameter, so must be invoked with the syntax HubConnection.<ArrayList<Integer>>invoke(Type, ...)
2. Chars will be serialized as Strings. Chars are not defined by the messagePack spec, so its up to the library implementer to decide how to serialize them. The java library we use serializes them as Strings, which is different than the C# library we use, which serializes them as Shorts.

Todo:

1. Separate msgPack into its own package
2. Decide if we're making TypeReference public

[wtgodbe]

Should I even account for itemType being null? In theory it should never happen (would indicate that a function took an argument of type null, or returned something of type null, which shouldn't be possible) but the test InvocationBinder returns null:

aspnetcore/src/SignalR/clients/java/signalr/src/test/java/com/microsoft/signalr/JsonHubProtocolTest.java

Line 286 in 0d15648

private Class<?> returnType = null;

[halter73]

Could this happen given a protocol violation by the server?

If so, we should at the very least log an error and fail the corresponding client-to-server invocation or skip the corresponding server-to-client invocation. Closing the entire HubConnection with an error might be even better.

If this actually cannot happen short of memory corruption or similar, just assert.

[wtgodbe]

It could happen if the server provides an InvocationBinder that returns null from getReturnType(invocationId) when sending a StreamItem (or if the client does the same when sending an invocation)

[wtgodbe]

I don't account for the MessagePack data being malformed - if the user passes a Map when the function expects a String, this should just throw a MessagePackException. The C# client appears to do the same

[halter73]

How does this surface to the HubConnection user?

[wtgodbe]

A call to ParseMessages would throw a RuntimeException w/ "Error reading messagepack data" - JSONHubProtocol does the same:

aspnetcore/src/SignalR/clients/java/signalr/src/main/java/com/microsoft/signalr/JsonHubProtocol.java

Lines 175 to 177 in 0d15648

	} catch (IOException ex) {
	throw new RuntimeException("Error reading JSON.", ex);
	}

While the C# Client's method is TryParseMessages, so it just returns false if it runs into issues.

[davidfowl]

Should we change the interface HubProtocol to deal in byte[] rather than Strings? As it is, I have to convert between the two using StandardCharsets.ISO_8859_1, otherwise bytes like 0x90 and 0x93 will get translated incorrectly when going from byte[] -> String -> byte[]. Changing the interface would require changes to JsonHubProtocol, WebSocketTransport, and DefaultHttpClient.

Yes, otherwise it kinda defeats the purpose.

[Pilchie]

👀

[wtgodbe]

From today's meeting:

Next step is to convert the abstraction to use ByteBuffer instead of String (all the way down to

aspnetcore/src/SignalR/clients/java/signalr/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java

Lines 62 to 66 in e3501dd

	@Override
	public Completable send(String message) {
	websocketClient.send(message);
	return Completable.complete();
	}

). This should be fairly mechanical. Reason being that OkHTTP converts strings to UTF8 on Send, which will break MessagePack, so we need to invoke its ByteString method instead.

[wtgodbe]

Current state - I've converted everything to use ByteBuffer. Json is still working, but MsgPack fails immediately on parsing the first message - on inspection, the length header looks malformed. The payload is 36 bytes - first byte 0x1c (28), second byte 0xc2 (false), 3rd byte 0x96 (6-length array - this appears to be the first byte of the message). Will look into what's happening when I get back to this next week

[wtgodbe]

Change this

[wtgodbe]

@BrennanConroy @halter73 should this just be a param to the HubConnection constructor?

[BrennanConroy]

We'll want a method on the builder similar to

aspnetcore/src/SignalR/clients/ts/signalr/src/HubConnectionBuilder.ts

Line 147 in 50fb513

public withHubProtocol(protocol: IHubProtocol): HubConnectionBuilder {

and yes, the protocol will be passed to the constructor.

[wtgodbe]

a05868f

[BrennanConroy]

Need to default it to JsonHubProtocol :D

[wtgodbe]

Current issue: The ByteBuffer received here is ReadOnly:

aspnetcore/src/SignalR/clients/java/signalr/src/main/java/com/microsoft/signalr/HubConnection.java

Line 167 in c934d3e

this.callback = (payload) -> {

Figuring out why

[BrennanConroy]

Can we find the index from the original payload first, then make a string once with the known size?

[wtgodbe]

@BrennanConroy 35f2653

[wtgodbe]

/azp run

[wtgodbe]

@BrennanConroy @halter73 just pushed 6e53284, any other concerns?

[BrennanConroy]

File issues for followup work?

[wtgodbe]

#25086
#25087
#25088
#25089