microsoft / kiota-dotnet

Abstractions library for the Kiota generated SDKs in dotnet
https://aka.ms/kiota/docs
MIT License
34 stars 30 forks source link

Unexpected exception when using `KiotaJsonSerializer` #397

Open svrooij opened 4 hours ago

svrooij commented 4 hours ago

Sample code:

var app = new Microsoft.Graph.Beta.Models.WinGetApp
{
    DisplayName = package?.Name,
    Description = package?.Description,
    ...
};
string json = await KiotaJsonSerializer.SerializeAsStringAsync(app, false, cancellationToken);

Results in:

Exception: System.InvalidOperationException: Content type application/json does not have a factory registered to be parsed
  at Microsoft.Kiota.Abstractions.Serialization.SerializationWriterFactoryRegistry.GetSerializationWriterFactory(String contentType, String& actualContentType)
  at Microsoft.Kiota.Abstractions.Serialization.SerializationWriterFactoryRegistry.GetSerializationWriter(String contentType, Boolean serializeOnlyChangedValues)
  at Microsoft.Kiota.Abstractions.Serialization.KiotaSerializer.GetSerializationWriter(String contentType, Object value, Boolean serializeOnlyChangedValues)
  at Microsoft.Kiota.Abstractions.Serialization.KiotaSerializer.SerializeAsStream[T](String contentType, T value, Boolean serializeOnlyChangedValues)
  at Microsoft.Kiota.Abstractions.Serialization.KiotaSerializer.SerializeAsStringAsync[T](String contentType, T value, Boolean serializeOnlyChangedValues, CancellationToken cancellationToken)
  at Microsoft.Kiota.Abstractions.Serialization.KiotaJsonSerializer.SerializeAsStringAsync[T](T value, Boolean serializeOnlyChangedValues, CancellationToken cancellationToken)

This makes me wonder where and how the factories are registered, and if this is even needed for the extension methods.

svrooij commented 3 hours ago

Maybe the new extensions should have used something like:

    internal static async Task<string> SerializeAsJsonString(this IParsable value, CancellationToken cancellationToken)
    {
        using var writer = new JsonSerializationWriter();
        writer.WriteObjectValue(null, value);
        using var stream = writer.GetSerializedContent();
        using var reader = new StreamReader(stream);
        return await reader.ReadToEndAsync(cancellationToken);
    }
baywet commented 1 hour ago

This is currently being registered here. https://github.com/microsoftgraph/msgraph-sdk-dotnet/blob/817308446f538133e6a7460166cdd8ef0334e964/src/Microsoft.Graph/Generated/BaseGraphServiceClient.cs#L469

And we're in the process of moving it there so the generated code is truely portable. https://github.com/microsoft/kiota-dotnet/blob/3fd334031cc1f03d9892fb6e2d3dba57c73ee2f8/src/bundle/DefaultRequestAdapter.cs#L38

Which mean that if you use the Json methods (static or extension) before the client, or in the future the request adapter, has been newed up, you'll run into this error.

Now, while the extension methods can register the serialization provider if it's missing because of where they are defined, the static methods can't. And there's really no reason why one should work but not the other. We could imagine moving the static KiotaJsonSerializer methods to the Json package. And implement a fallback rather than failing whenever the serialization provider is not present

Thoughts?

svrooij commented 55 minutes ago

What does this means if you would use 2 kiota clients in one application GitHub and Graph for instance? Do we get double registrations? Or will they be overridden? This might lead to issues if you setup custom factories?

I think the the KiotaJsonSerializer and the extension methods need to work indepently of the client. Maybe the registered factories should be set in the client and not in a singleton.