Azure / azure-functions-signalrservice-extension

Azure Functions bindings for SignalR Service. Project moved to https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService .
MIT License
97 stars 46 forks source link

Support for JsonSerializerOptions when System.Text.Json is used for serialization #271

Closed ilya-git closed 2 years ago

ilya-git commented 2 years ago

I am using persistent connection to Azure SignalR Service ("AzureSignalRServiceTransportType": "Persistent"), this by default means that System.Text.Json is being used for serialization.

This also means that there is no way (not that I have found at least) to configure the serialization, like e.g. to make enum being serialized as strings and not numbers.

It would be very nice to have an ability to set up JsonSerializerOptions globally for the output binding somehow. Ideally both in config and in code, but code is probably much easier to achieve without the need to write to much configuration as it will provide all available options.

As far as I could figure out JsonHubProtocol is created here. Another constructor can accept IOptions<JsonHubProtocolOptions>, but it is not used. Perhaps switching to it and have a possibility to inject via DI would be enough?

I tried to do the same thing by switching to newtonsoft.json but did not find a way augment it's options either. So that means that the only way for me to achieve enums as strings is to connect to AzureSignalRService directly via a client library and not use output binding it seems which is quite unofrtunate.

Y-Sindo commented 2 years ago

Currently as a work around, you can add a config item Azure:SignalR:HubProtocol=NewtonsoftJson to switch to Newtonsoft.Json, then you can set JsonConvert.DefaultSettings to customize the JSON serialization behaviour.

For how to customize System.Text.Json, we need more consideration about the design. Thank you for your feedback.

ilya-git commented 2 years ago

Thanks @Y-Sindo, I have tried that, but I did not manage to change anything with JsonConvert.DefaultSettings. Should it work properly (officially supported) and I just did something wrong?

I have also tried to just serialize myself and add as a string, but then I get escaping from the library since it does not know this is a correct json, and I assume there is no way of telling it otherwise

jasmcc commented 2 years ago

Very closely related so not opening a new issue - adding Azure:SignalR:HubProtocol with a value of NewtonsoftJson does switch it to that parser for Clients.User....SendAsync. Like @ilya-git, however, I have been unable to get settings overrides to work.

I am also unable to get JsonConvert.DefaultSettings to have any effect; I suspect this is because here on line 35 the serializer settings are set to always resolve to default: https://github.com/Azure/azure-functions-signalrservice-extension/blob/16cff13c7b8f98bad8da488f91de49e4522c3e85/src/SignalRServiceExtension/Config/DependencyInjectionExtensions.cs

Or am I misreading this?

Y-Sindo commented 2 years ago

@jasmcc Thanks for your reminding. This line does prevent JsonConvert.DefaultSettings from taking effects. https://github.com/Azure/azure-functions-signalrservice-extension/blob/16cff13c7b8f98bad8da488f91de49e4522c3e85/src/SignalRServiceExtension/Config/DependencyInjectionExtensions.cs#L35

I think the systematic way to solve this problem is to provide a IFunctionsHostBuilder extension, so that we can do things like

    internal class Startup : FunctionsStartup
    {
        public override void Configure(IFunctionsHostBuilder builder)
        {
            builder.AddSignalR().UseSystemTextJson(Action<JsonSerializerOptions> ...)
        }
    }

Do you have any comments on this?

jasmcc commented 2 years ago

That sounds reasonable to me, I guess signatures for both Newtsonsoft and System.Text.Json.

ilya-git commented 2 years ago

Yes, this sounds good enough that we can programmatically supply any settings. Any ideas what can we do meanwhile?

I am wondering if we set camel case to false and this line gets called:

https://github.com/Azure/azure-functions-signalrservice-extension/blob/16cff13c7b8f98bad8da488f91de49e4522c3e85/src/SignalRServiceExtension/Config/DependencyInjectionExtensions.cs#L29

Maybe then we can manage to use JsonConvert.DefaultSettings, will try this approach later.

jasmcc commented 2 years ago

Good idea; I tried Azure:SignalR:HubProtocol:NewtonsoftJson:CamelCase as true or false and neither seemed to work. Let me know if you get different results, I may be doing the key value wrong.

Y-Sindo commented 2 years ago

That sounds reasonable to me, I guess signatures for both Newtsonsoft and System.Text.Json.

Yes, there will be two methods.

Azure:SignalR:HubProtocol:NewtonsoftJson:CamelCase won't work as there is no place to accept a JsonConvert.DefaultSettings.

ilya-git commented 2 years ago

Do we have any workaround now? I cannot see any way apart from a direct connection with client library to use custom json setting at the moment.

Or maybe it is not a lot of work and this fix will be released soon @Y-Sindo ?

Y-Sindo commented 2 years ago

@ilya-git You can use Transient mode without any HubProtocol settings and use JsonConvert.DefaultSettings to customize.

ilya-git commented 2 years ago

Thanks! that would probably be less efficient I assume but should do the trick until we get a better way of configuring it. I will try it out and report how it goes

ilya-git commented 2 years ago

I have by the a way a small comment for your proposed solution, what if this will be an extension method not for the host builder, but to the IServiceCollection, then it will probably be a bit more generic and look more like other places?

Y-Sindo commented 2 years ago

Supported in 1.8.0, see JSON object serialization