Skip to content

Commit d30390a

Browse files
Y-SindoY-Sindo
authored
Remove the Arguments requirement in a SignalR message (Azure#32590)
* Remove the Arguments requirement in a SignalR message. In isolated model, the arguments of a SignalR message are not required. However, it's required here. We cannot remove the `JsonRequired` attribute of the member as it's a breaking change and fails the build. We have to use a custom JSON converter to make it not required. * Hack to access internal member `ServiceManagerOptions.ObjectSerializer` # Contributing to the Azure SDK Please see our [CONTRIBUTING.md](https://github.com/Azure/azure-sdk-for-net/blob/main/CONTRIBUTING.md) if you are not familiar with contributing to this repository or have questions. For specific information about pull request etiquette and best practices, see [this section](https://github.com/Azure/azure-sdk-for-net/blob/main/CONTRIBUTING.md#pull-request-etiquette-and-best-practices).
1 parent af2ba27 commit d30390a

File tree

7 files changed

+109
-13
lines changed

7 files changed

+109
-13
lines changed

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/src/Bindings/InternalSignalRMessage.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Azure.SignalR;
5+
using Newtonsoft.Json;
6+
7+
namespace Microsoft.Azure.WebJobs.Extensions.SignalRService
8+
{
9+
[JsonObject]
10+
internal class InternalSignalRMessage
11+
{
12+
public string connectionId { get; set; }
13+
public string userId { get; set; }
14+
public string groupName { get; set; }
15+
[JsonRequired]
16+
public string target { get; set; }
17+
public object[] arguments { get; set; }
18+
public ServiceEndpoint[] endpoints { get; set; }
19+
}
20+
}

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/src/Bindings/JTokenExtensions.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using Newtonsoft.Json;
45
using Newtonsoft.Json.Linq;
56

67
namespace Microsoft.Azure.WebJobs.Extensions.SignalRService
78
{
89
internal static class JTokenExtensions
910
{
11+
public static readonly JsonSerializer JsonSerializers = JsonSerializer.Create(new JsonSerializerSettings
12+
{
13+
Converters = new JsonConverter[] { new ServiceEndpointJsonConverter(), new SignalRMessageConverter() }
14+
});
15+
1016
public static bool TryToObject<TOutput>(this JToken input, out TOutput output)
1117
{
1218
try
1319
{
14-
output = input.ToObject<TOutput>(ServiceEndpointJsonConverter.JsonSerializer);
20+
output = input.ToObject<TOutput>(JsonSerializers);
1521
}
1622
catch
1723
{

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/src/Bindings/ServiceEndpointJsonConverter.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ namespace Microsoft.Azure.WebJobs.Extensions.SignalRService
99
{
1010
internal class ServiceEndpointJsonConverter : JsonConverter<ServiceEndpoint>
1111
{
12-
public static readonly JsonSerializer JsonSerializer = JsonSerializer.Create(new JsonSerializerSettings
13-
{
14-
Converters = new JsonConverter[] { new ServiceEndpointJsonConverter() }
15-
});
1612
private const string FakeAccessKey = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
1713

1814
public override ServiceEndpoint ReadJson(JsonReader reader, Type objectType, ServiceEndpoint existingValue, bool hasExistingValue, JsonSerializer serializer)

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/src/Bindings/SignalRMessageConverter.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using Newtonsoft.Json;
6+
7+
namespace Microsoft.Azure.WebJobs.Extensions.SignalRService
8+
{
9+
/// <summary>
10+
/// The <see cref="SignalRMessage.Arguments"/> shouldn't be required, but it's a breaking change to remove the <see cref="JsonRequiredAttribute"/> of the member. As a workaround, we have to use a converter to override the behaviour.
11+
/// </summary>
12+
internal class SignalRMessageConverter : JsonConverter<SignalRMessage>
13+
{
14+
public override SignalRMessage ReadJson(JsonReader reader, Type objectType, SignalRMessage existingValue, bool hasExistingValue, JsonSerializer serializer)
15+
{
16+
var internalMessageObj = serializer.Deserialize<InternalSignalRMessage>(reader);
17+
return new()
18+
{
19+
ConnectionId = internalMessageObj.connectionId,
20+
UserId = internalMessageObj.userId,
21+
GroupName = internalMessageObj.groupName,
22+
Target = internalMessageObj.target,
23+
Arguments = internalMessageObj.arguments,
24+
Endpoints = internalMessageObj.endpoints,
25+
};
26+
}
27+
28+
public override void WriteJson(JsonWriter writer, SignalRMessage value, JsonSerializer serializer)
29+
{
30+
serializer.Serialize(writer, value);
31+
}
32+
}
33+
}

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/src/Config/SignalRConfigProvider.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public void Initialize(ExtensionConfigContext context)
8585
.BindToInput(new NegotiationContextAsyncConverter(_serviceManagerStore));
8686

8787
_ = context.AddBindingRule<SignalREndpointsAttribute>()
88-
.AddConverter<ServiceEndpoint[], JArray>(endpoints => JArray.FromObject(endpoints, ServiceEndpointJsonConverter.JsonSerializer))
88+
.AddConverter<ServiceEndpoint[], JArray>(endpoints => JArray.FromObject(endpoints, JTokenExtensions.JsonSerializers))
8989
.BindToInput(new SignalREndpointsAsyncConverter(_serviceManagerStore));
9090

9191
var signalRAttributeRule = context.AddBindingRule<SignalRAttribute>();

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/tests/Config/OptionsSetupFacts.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using System;
5+
using System.Reflection;
56
using Azure.Core.Serialization;
67
using Microsoft.Azure.SignalR.Management;
78
using Microsoft.Azure.WebJobs.Extensions.SignalRService;
@@ -81,7 +82,9 @@ public void TestHubProtocolConfiguration(string configKey, string configValue, T
8182
var options = new ServiceManagerOptions();
8283
var setup = new OptionsSetup(configuration, SingletonAzureComponentFactory.Instance, "key");
8384
setup.Configure(options);
84-
Assert.Equal(objectSerializerType, options.ObjectSerializer?.GetType());
85+
// hack to access internal member
86+
var serializer = typeof(ServiceManagerOptions).GetProperty("ObjectSerializer", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(options);
87+
Assert.Equal(objectSerializerType, serializer?.GetType());
8588
}
8689
}
8790
}

‎sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService/tests/SignalROutputConverterTests.cs‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using System;
5+
using Microsoft.Azure.SignalR;
56
using Microsoft.Azure.WebJobs.Extensions.SignalRService;
67
using Newtonsoft.Json.Linq;
78
using Xunit;
@@ -30,19 +31,56 @@ public void OutputConverter_SignalRMessage_JObjectReturnsValid()
3031
{
3132
var converter = new SignalROutputConverter();
3233

34+
var input = JObject.Parse(@"{
35+
'userId': 'user1',
36+
'target': 'newMessage',
37+
'arguments': [
38+
{
39+
'arg1': 'arg1',
40+
'agr2': 'arg2'
41+
}
42+
],
43+
'endpoints': [
44+
{
45+
'endpointType': 'primary',
46+
'name': 'endpoint1',
47+
'endpoint': 'https://endpoint1',
48+
'online': 'true'
49+
},
50+
{
51+
'endpointType': 'primary',
52+
'name': 'endpoint2',
53+
'endpoint': 'https://endpoint2',
54+
'online': 'true'
55+
}
56+
]
57+
}");
58+
59+
var converted = converter.ConvertToSignalROutput(input);
60+
Assert.Equal(typeof(SignalRMessage), converted.GetType());
61+
var message = (SignalRMessage)converted;
62+
Assert.Equal("newMessage", message.Target);
63+
Assert.Equal("user1", message.UserId);
64+
Assert.Equal(2, message.Endpoints.Length);
65+
Assert.Equal(new ServiceEndpoint("Endpoint=https://endpoint1;AccessKey=ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789;Version=1.0;", name: "endpoint1"), message.Endpoints[0]);
66+
Assert.Equal(new ServiceEndpoint("Endpoint=https://endpoint2;AccessKey=ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789;Version=1.0;", name: "endpoint2"), message.Endpoints[1]);
67+
}
68+
69+
[Fact]
70+
public void OutputConverter_SignalRMessage_ArgumentsNotRequired()
71+
{
72+
var converter = new SignalROutputConverter();
73+
3374
var input = JObject.Parse(@"{
3475
'userId': 'user1',
3576
'target': 'newMessage',
36-
'arguments': [
37-
{
38-
'arg1': 'arg1',
39-
'agr2': 'arg2'
40-
}
41-
]
4277
}");
4378

4479
var converted = converter.ConvertToSignalROutput(input);
4580
Assert.Equal(typeof(SignalRMessage), converted.GetType());
81+
var message = (SignalRMessage)converted;
82+
Assert.Equal("newMessage", message.Target);
83+
Assert.Equal("user1", message.UserId);
4684
}
4785

4886
[Fact]

0 commit comments

Comments
 (0)