Skip to content

Commit be9fec5

Browse files
moozzykmoozzyk
committed
Adding result kind to completion message
Before we would rely on error being null to detect whether to read results and we had an additional 'hasResult' field. Now all this information is codified in a field.
1 parent 32ef3eb commit be9fec5

File tree

2 files changed

+64
-27
lines changed

2 files changed

+64
-27
lines changed

‎src/Microsoft.AspNetCore.SignalR.Common/Internal/Protocol/MessagePackHubProtocol.cs‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ public class MessagePackHubProtocol : IHubProtocol
1616
private const int StreamItemMessageType = 2;
1717
private const int CompletionMessageType = 3;
1818

19+
private const int ErrorResult = 1;
20+
private const int VoidResult = 2;
21+
private const int NonVoidResult = 3;
22+
1923
public string Name => "messagepack";
2024

2125
public ProtocolType Type => ProtocolType.Binary;
@@ -38,10 +42,7 @@ public bool TryParseMessages(ReadOnlyBuffer<byte> input, IInvocationBinder binde
3842
private static HubMessage ParseMessage(Stream input, IInvocationBinder binder)
3943
{
4044
var unpacker = Unpacker.Create(input);
41-
if (!unpacker.ReadInt32(out var messageType))
42-
{
43-
throw new FormatException("Message type is missing.");
44-
}
45+
var messageType = ReadInt32(unpacker, "messageType");
4546

4647
switch (messageType)
4748
{
@@ -90,18 +91,27 @@ private static StreamItemMessage CreateStreamItemMessage(Unpacker unpacker, IInv
9091
private static CompletionMessage CreateCompletionMessage(Unpacker unpacker, IInvocationBinder binder)
9192
{
9293
var invocationId = ReadInvocationId(unpacker);
93-
var error = ReadString(unpacker, "error");
94+
var resultKind = ReadInt32(unpacker, "resultKind");
9495

95-
var hasResult = false;
96+
string error = null;
9697
object result = null;
97-
if (error == null)
98+
var hasResult = false;
99+
100+
switch(resultKind)
98101
{
99-
hasResult = ReadBoolean(unpacker, "hasResult");
100-
if (hasResult)
101-
{
102+
case ErrorResult:
103+
error = ReadString(unpacker, "error");
104+
break;
105+
case NonVoidResult:
102106
var itemType = binder.GetReturnType(invocationId);
103107
result = DeserializeObject(unpacker, itemType, "argument");
104-
}
108+
hasResult = true;
109+
break;
110+
case VoidResult:
111+
hasResult = false;
112+
break;
113+
default:
114+
throw new FormatException("Invalid invocation result kind.");
105115
}
106116

107117
return new CompletionMessage(invocationId, error, result, hasResult);
@@ -153,16 +163,22 @@ private void WriteStremingItemMessage(StreamItemMessage streamItemMessage, Packe
153163

154164
private void WriteCompletionMessage(CompletionMessage completionMessage, Packer packer, Stream output)
155165
{
166+
var resultKind =
167+
completionMessage.Error != null ? ErrorResult :
168+
completionMessage.HasResult ? NonVoidResult :
169+
VoidResult;
170+
156171
packer.Pack(CompletionMessageType);
157172
packer.PackString(completionMessage.InvocationId);
158-
packer.PackString(completionMessage.Error);
159-
if (completionMessage.Error == null)
173+
packer.Pack(resultKind);
174+
switch (resultKind)
160175
{
161-
packer.Pack(completionMessage.HasResult);
162-
if (completionMessage.HasResult)
163-
{
176+
case ErrorResult:
177+
packer.PackString(completionMessage.Error);
178+
break;
179+
case NonVoidResult:
164180
packer.PackObject(completionMessage.Result);
165-
}
181+
break;
166182
}
167183
}
168184

@@ -171,6 +187,24 @@ private static string ReadInvocationId(Unpacker unpacker)
171187
return ReadString(unpacker, "invocationId");
172188
}
173189

190+
private static int ReadInt32(Unpacker unpacker, string field)
191+
{
192+
Exception msgPackException = null;
193+
try
194+
{
195+
if (unpacker.ReadInt32(out var value))
196+
{
197+
return value;
198+
}
199+
}
200+
catch (Exception e)
201+
{
202+
msgPackException = e;
203+
}
204+
205+
throw new FormatException($"Reading '{field}' as Int32 failed.", msgPackException);
206+
}
207+
174208
private static string ReadString(Unpacker unpacker, string field)
175209
{
176210
Exception msgPackException = null;

‎test/Microsoft.AspNetCore.SignalR.Common.Tests/Internal/Protocol/MessagePackHubProtocolTests.cs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ public void CanRoundTripInvocationMessage(HubMessage[] hubMessages)
7373

7474
public static IEnumerable<object[]> InvalidPayloads => new[]
7575
{
76-
new object[] { new byte[0], "Message type is missing." },
76+
new object[] { new byte[0], "Reading 'messageType' as Int32 failed." },
77+
new object[] { new byte[] { 0xc2 } , "Reading 'messageType' as Int32 failed." }, // message type is not int
7778
new object[] { new byte[] { 0x0a } , "Invalid message type: 10." },
7879

7980
// InvocationMessage
@@ -100,13 +101,15 @@ public void CanRoundTripInvocationMessage(HubMessage[] hubMessages)
100101
// CompletionMessage
101102
new object[] { new byte[] { 0x03 }, "Reading 'invocationId' as String failed." }, // 0xc2 is Bool false
102103
new object[] { new byte[] { 0x03, 0xc2 }, "Reading 'invocationId' as String failed." }, // 0xc2 is Bool false
103-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc2 }, "Reading 'error' as String failed." }, // 0xc2 is Bool false
104-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xa1 }, "Reading 'error' as String failed." }, // error is cut
105-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc0 }, "Reading 'hasResult' as Boolean failed." }, // hasResult missing
106-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc0, 0xa0 }, "Reading 'hasResult' as Boolean failed." }, // 0xa0 is string
107-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc0, 0xc3 }, "Deserializing object of the `String` type for 'argument' failed." }, // result missing
108-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc0, 0xc3, 0xa9 }, "Deserializing object of the `String` type for 'argument' failed." }, // result is cut
109-
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc0, 0xc3, 0x00 }, "Deserializing object of the `String` type for 'argument' failed." } // return type mismatch
104+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0xc2 }, "Reading 'resultKind' as Int32 failed." }, // result kind is not int
105+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0x0f }, "Invalid invocation result kind." }, // result kind is out of range
106+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0x01 }, "Reading 'error' as String failed." }, // error result but no error
107+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0x01, 0xa1 }, "Reading 'error' as String failed." }, // error is cut
108+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0x03 }, "Deserializing object of the `String` type for 'argument' failed." }, // non void result but result missing
109+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0x03, 0xa9 }, "Deserializing object of the `String` type for 'argument' failed." }, // result is cut
110+
new object[] { new byte[] { 0x03, 0xa3, 0x78, 0x79, 0x7a, 0x03, 0x00 }, "Deserializing object of the `String` type for 'argument' failed." }, // return type mismatch
111+
112+
// TODO: ReadAsInt32 and no int32 value
110113
};
111114

112115
[Theory]
@@ -132,8 +135,8 @@ public void ParserThrowsForInvalidMessages(byte[] payload, string expectedExcept
132135
[InlineData(new object[] {
133136
new byte[]
134137
{
135-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x03, 0xa1, 0x78, 0xa1, 0x45,
136-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x03, 0xa1, 0x78, 0xa1, 0x45,
138+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x03, 0xa1, 0x78, 0x02,
139+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x03, 0xa1, 0x78, 0x02,
137140
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x03, 0xa1
138141
}, 2 })]
139142
public void ParserDoesNotConsumePartialData(byte[] payload, int expectedMessagesCount)

0 commit comments

Comments
 (0)