Skip to content

Commit 54a16aa

Browse files
kevin-carrollkevin-carroll
authored
Fix for Subscription Message Serialziation (#169)
* Fixed a bug where some client libraries would reject subscription messages due to erroneous id values.
1 parent 56d66fd commit 54a16aa

File tree

10 files changed

+75
-25
lines changed

10 files changed

+75
-25
lines changed

‎.github/workflows/ci-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ env:
2424
jobs:
2525
build:
2626
name: Sanity Build
27-
runs-on: ubuntu-20.04
27+
runs-on: ubuntu-24.04
2828

2929
steps:
3030
- uses: actions/checkout@v3

‎.github/workflows/nuget-deployment.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ env:
2626
jobs:
2727
deployment:
2828
name: Pack & Deploy
29-
runs-on: ubuntu-20.04
29+
runs-on: ubuntu-24.04
3030

3131
steps:
3232
- uses: actions/checkout@v3

‎src/ancillary-projects/starwars/starwars-api90/Properties/launchSettings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"$schema": "http://json.schemastore.org/launchsettings.json",
33
"profiles": {
4-
"StarWars: .NET 8": {
4+
"StarWars: .NET 9": {
55
"commandName": "Project",
66
"applicationUrl": "http://localhost:5000",
77
"environmentVariables": {

‎src/graphql-aspnet-subscriptions/SubscriptionServer/Protocols/GraphqlTransportWs/GqltwsClientProxy.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ public GqltwsClientProxy(
9494
_serializerOptions.Converters.Add(new GqltwsServerDataMessageConverter(schema, responseWriter));
9595
_serializerOptions.Converters.Add(new GqltwsServerCompleteMessageConverter());
9696
_serializerOptions.Converters.Add(new GqltwsServerErrorMessageConverter(schema));
97+
_serializerOptions.Converters.Add(new GqltwsMessageConverter());
9798
}
9899

99100
/// <summary>

‎src/graphql-aspnet-subscriptions/SubscriptionServer/Protocols/GraphqlWsLegacy/GraphqlWsLegacyClientProxy.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ public GraphqlWsLegacyClientProxy(
9393
_serializeOptions.Converters.Add(new GraphqlWsLegacyServerDataMessageConverter(schema, responseWriter));
9494
_serializeOptions.Converters.Add(new GraphqlWsLegacyServerCompleteMessageConverter());
9595
_serializeOptions.Converters.Add(new GraphqlWsLegacyServerErrorMessageConverter(schema));
96+
_serializeOptions.Converters.Add(new GraphqlWsLegacyMessageConverter());
9697
}
9798

9899
/// <inheritdoc />
@@ -305,7 +306,7 @@ private async Task AcknowledgeNewConnectionAsync()
305306
/// <inheritdoc />
306307
protected override async Task ExecuteKeepAliveAsync(CancellationToken cancelToken = default)
307308
{
308-
await this.SendMessageAsync(new GraphqlWsLegacyKeepAliveOperationMessage());
309+
await this.SendMessageAsync(new GraphqlWsLegacyKeepAliveOperationMessage(), cancelToken);
309310
}
310311

311312
/// <inheritdoc />

‎src/graphql-aspnet/Common/TimerAsync.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ public void Start()
9090
}
9191
finally
9292
{
93-
_semaphore.Release();
93+
if (!_disposed)
94+
_semaphore.Release();
9495
}
9596
}
9697

@@ -121,7 +122,8 @@ public async Task StopAsync()
121122
finally
122123
{
123124
this.IsRunning = false;
124-
_semaphore.Release();
125+
if (!_disposed)
126+
_semaphore.Release();
125127
}
126128
}
127129

@@ -195,4 +197,4 @@ public void Dispose()
195197
/// <value><c>true</c> if the timer is running; otherwise, <c>false</c>.</value>
196198
public bool IsRunning { get; private set; }
197199
}
198-
}
200+
}

‎src/unit-tests/graphql-aspnet-subscriptions-tests/SubscriptionServer/Protocols/GraphqlTransportWs/GqltwsClientAsserts.cs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@ public static class GqltwsClientAsserts
2525
/// <param name="connection">The connection.</param>
2626
/// <param name="type">The type of message to check for.</param>
2727
/// <param name="dequeue">if true, the message is removed from the queue.</param>
28-
internal static void AssertGqltwsResponse(
28+
/// <returns>The raw string data (usually json formatted) of the message data recevied by the connection proxy </returns>
29+
internal static string AssertGqltwsResponse(
2930
this MockClientConnection connection,
3031
GqltwsMessageType type,
3132
bool dequeue = true)
3233
{
33-
connection.AssertGqltwsResponse(type, null, false, null, false, dequeue);
34+
return connection.AssertGqltwsResponse(type, null, false, null, false, dequeue);
3435
}
3536

3637
/// <summary>
@@ -40,13 +41,14 @@ internal static void AssertGqltwsResponse(
4041
/// <param name="type">The type of message to check for.</param>
4142
/// <param name="id">The id returned by the server, if supplied.</param>
4243
/// <param name="dequeue">if true, the message is removed from the queue.</param>
43-
internal static void AssertGqltwsResponse(
44+
/// <returns>The raw string data (usually json formatted) of the message data recevied by the connection proxy </returns>
45+
internal static string AssertGqltwsResponse(
4446
this MockClientConnection connection,
4547
GqltwsMessageType type,
4648
string id,
4749
bool dequeue = true)
4850
{
49-
connection.AssertGqltwsResponse(type, id, true, null, false, dequeue);
51+
return connection.AssertGqltwsResponse(type, id, true, null, false, dequeue);
5052
}
5153

5254
/// <summary>
@@ -58,17 +60,18 @@ internal static void AssertGqltwsResponse(
5860
/// <param name="id">The expected identifier of the subscription that rendered the data.</param>
5961
/// <param name="expectedPayloadJson">The expected payload of the message, converted to a json string.</param>
6062
/// <param name="dequeue">if set to <c>true</c> if the message should be removed from the queue.</param>
61-
internal static void AssertGqltwsResponse(
63+
/// <returns>The raw string data (usually json formatted) of the message data recevied by the connection proxy </returns>
64+
internal static string AssertGqltwsResponse(
6265
this MockClientConnection connection,
6366
GqltwsMessageType type,
6467
string id,
6568
string expectedPayloadJson,
6669
bool dequeue = true)
6770
{
68-
connection.AssertGqltwsResponse(type, id, true, expectedPayloadJson, true, dequeue);
71+
return connection.AssertGqltwsResponse(type, id, true, expectedPayloadJson, true, dequeue);
6972
}
7073

71-
private static void AssertGqltwsResponse(
74+
private static string AssertGqltwsResponse(
7275
this MockClientConnection connection,
7376
GqltwsMessageType type,
7477
string id,
@@ -81,14 +84,14 @@ private static void AssertGqltwsResponse(
8184
Assert.Fail("No messages queued.");
8285

8386
var message = dequeue ? connection.DequeueNextReceivedMessage() : connection.PeekNextReceivedMessage();
84-
var str = Encoding.UTF8.GetString(message.Data);
87+
var rawMessageData = Encoding.UTF8.GetString(message.Data);
8588

8689
var options = new JsonSerializerOptions();
8790
options.PropertyNameCaseInsensitive = true;
8891
options.AllowTrailingCommas = true;
8992
options.Converters.Add(new GqltwsResponseMessageConverter());
9093

91-
var convertedMessage = JsonSerializer.Deserialize<GqltwsResponseMessage>(str, options);
94+
var convertedMessage = JsonSerializer.Deserialize<GqltwsResponseMessage>(rawMessageData, options);
9295

9396
Assert.IsNotNull(convertedMessage, "Could not deserialize response message");
9497
Assert.AreEqual(type, convertedMessage.Type, $"Expected message type of {type.ToString()} but got {convertedMessage.Type.ToString()}");
@@ -103,6 +106,8 @@ private static void AssertGqltwsResponse(
103106

104107
if (compareId)
105108
Assert.AreEqual(id, convertedMessage.Id);
109+
110+
return rawMessageData;
106111
}
107112
}
108113
}

‎src/unit-tests/graphql-aspnet-subscriptions-tests/SubscriptionServer/Protocols/GraphqlTransportWs/GqltwsClientProxyTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,5 +746,23 @@ public async Task Deserialize_InvalidMessage_ProcessesUnknownMessage()
746746
connection.AssertServerClosedConnection((ConnectionCloseStatus)GqltwsConstants.CustomCloseEventIds.InvalidMessageType);
747747
graphqlWsClient.Dispose();
748748
}
749+
750+
[Test]
751+
public async Task SendConnectionInitMessage_RespondsWithCorrectAckMessage()
752+
{
753+
using var restorePoint = new GraphQLGlobalSubscriptionRestorePoint();
754+
(var socketClient, var client, var router) = this.CreateConnection();
755+
756+
socketClient.QueueClientMessage(new GqltwsClientConnectionInitMessage());
757+
socketClient.QueueConnectionClosedByClient();
758+
759+
await client.StartConnectionAsync();
760+
761+
var rawMessageData = socketClient.AssertGqltwsResponse(GqltwsMessageType.CONNECTION_ACK);
762+
socketClient.AssertClientClosedConnection();
763+
764+
var expectedData = "{ \"type\": \"connection_ack\" }";
765+
CommonAssertions.AreEqualJsonStrings(expectedData, rawMessageData);
766+
}
749767
}
750768
}

‎src/unit-tests/graphql-aspnet-subscriptions-tests/SubscriptionServer/Protocols/GraphqlWsLegacy/GraphqlWsLegacyClientAsserts.cs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@ public static class GraphqlWsLegacyClientAsserts
2525
/// <param name="connection">The connection.</param>
2626
/// <param name="type">The type of message to check for.</param>
2727
/// <param name="dequeue">if true, the message is removed from the queue.</param>
28-
internal static void AssertGraphqlWsLegacyResponse(
28+
/// <returns>The raw string data (usually json formatted) of the message data recevied by the connection proxy </returns>
29+
internal static string AssertGraphqlWsLegacyResponse(
2930
this MockClientConnection connection,
3031
GraphqlWsLegacyMessageType type,
3132
bool dequeue = true)
3233
{
33-
connection.AssertGraphqlWsLegacyResponse(type, null, false, null, false, dequeue);
34+
return connection.AssertGraphqlWsLegacyResponse(type, null, false, null, false, dequeue);
3435
}
3536

3637
/// <summary>
@@ -40,13 +41,14 @@ internal static void AssertGraphqlWsLegacyResponse(
4041
/// <param name="type">The type of message to check for.</param>
4142
/// <param name="id">The id returned by the server, if supplied.</param>
4243
/// <param name="dequeue">if true, the message is removed from the queue.</param>
43-
internal static void AssertGraphqlWsLegacyResponse(
44+
/// <returns>The raw string data (usually json formatted) of the message data recevied by the connection proxy </returns>
45+
internal static string AssertGraphqlWsLegacyResponse(
4446
this MockClientConnection connection,
4547
GraphqlWsLegacyMessageType type,
4648
string id,
4749
bool dequeue = true)
4850
{
49-
connection.AssertGraphqlWsLegacyResponse(type, id, true, null, false, dequeue);
51+
return connection.AssertGraphqlWsLegacyResponse(type, id, true, null, false, dequeue);
5052
}
5153

5254
/// <summary>
@@ -58,17 +60,18 @@ internal static void AssertGraphqlWsLegacyResponse(
5860
/// <param name="id">The expected identifier of the subscription that rendered the data.</param>
5961
/// <param name="expectedPayloadJson">The expected payload of the message, converted to a json string.</param>
6062
/// <param name="dequeue">if set to <c>true</c> if the message should be removed from the queue.</param>
61-
internal static void AssertGraphqlWsLegacyResponse(
63+
/// <returns>The raw string data (usually json formatted) of the message data recevied by the connection proxy </returns>
64+
internal static string AssertGraphqlWsLegacyResponse(
6265
this MockClientConnection connection,
6366
GraphqlWsLegacyMessageType type,
6467
string id,
6568
string expectedPayloadJson,
6669
bool dequeue = true)
6770
{
68-
connection.AssertGraphqlWsLegacyResponse(type, id, true, expectedPayloadJson, true, dequeue);
71+
return connection.AssertGraphqlWsLegacyResponse(type, id, true, expectedPayloadJson, true, dequeue);
6972
}
7073

71-
private static void AssertGraphqlWsLegacyResponse(
74+
private static string AssertGraphqlWsLegacyResponse(
7275
this MockClientConnection connection,
7376
GraphqlWsLegacyMessageType type,
7477
string id,
@@ -81,14 +84,14 @@ private static void AssertGraphqlWsLegacyResponse(
8184
Assert.Fail("No messages queued.");
8285

8386
var message = dequeue ? connection.DequeueNextReceivedMessage() : connection.PeekNextReceivedMessage();
84-
var str = Encoding.UTF8.GetString(message.Data);
87+
var rawData = Encoding.UTF8.GetString(message.Data);
8588

8689
var options = new JsonSerializerOptions();
8790
options.PropertyNameCaseInsensitive = true;
8891
options.AllowTrailingCommas = true;
8992
options.Converters.Add(new GraphqlWsLegacyResponseMessageConverter());
9093

91-
var convertedMessage = JsonSerializer.Deserialize<GraphqlWsLegacyResponseMessage>(str, options);
94+
var convertedMessage = JsonSerializer.Deserialize<GraphqlWsLegacyResponseMessage>(rawData, options);
9295

9396
Assert.IsNotNull(convertedMessage, "Could not deserialized response message");
9497
Assert.AreEqual(type, convertedMessage.Type, $"Expected message type of {type.ToString()} but got {convertedMessage.Type.ToString()}");
@@ -103,6 +106,8 @@ private static void AssertGraphqlWsLegacyResponse(
103106

104107
if (compareId)
105108
Assert.AreEqual(id, convertedMessage.Id);
109+
110+
return rawData;
106111
}
107112
}
108113
}

‎src/unit-tests/graphql-aspnet-subscriptions-tests/SubscriptionServer/Protocols/GraphqlWsLegacy/GraphqlWsLegacyClientProxyTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,5 +501,23 @@ public async Task Deserialize_InvalidMessage_ProcessesUnknownMessage()
501501
socketClient.AssertGraphqlWsLegacyResponse(GraphqlWsLegacyMessageType.ERROR);
502502
socketClient.AssertClientClosedConnection();
503503
}
504+
505+
[Test]
506+
public async Task SendConnectionInitMessage_RespondsWithCorrectAckMessage()
507+
{
508+
using var restorePoint = new GraphQLGlobalSubscriptionRestorePoint();
509+
(var socketClient, var client, var router) = this.CreateConnection();
510+
511+
socketClient.QueueClientMessage(new GraphqlWsLegacyClientConnectionInitMessage());
512+
socketClient.QueueConnectionClosedByClient();
513+
514+
await client.StartConnectionAsync();
515+
516+
var rawMessageData = socketClient.AssertGraphqlWsLegacyResponse(GraphqlWsLegacyMessageType.CONNECTION_ACK);
517+
socketClient.AssertClientClosedConnection();
518+
519+
var expectedData = "{ \"type\": \"connection_ack\" }";
520+
CommonAssertions.AreEqualJsonStrings(expectedData, rawMessageData);
521+
}
504522
}
505523
}

0 commit comments

Comments
 (0)