microsoft/kiota-dotnet

Unexpected exception when using `KiotaJsonSerializer`

Closed this issue · 7 comments

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.

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);
    }

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.

ApiClientBuilder.RegisterDefaultSerializer<JsonSerializationWriterFactory>();

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?

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.

Those are the default serialization providers. The app developer can always pass a different set to the request adapter (at least with our implementations) which will be local to this request adapter.
In case the automatic registration is used from the client, first client wins for any given media type.
If we want those methods not to depend on the singleton, they'll need to accept a request adapter.

Would you be ok with me detaching the Iparsable extensions from the registry? To make sure they will work not matter the registry is filled?

For the Json extension methods defined in the Json package, yes. People won't have access to the method if they have not imported the dependency. So I think it's safe to assume they'll want to use this implementation.
For the format agnostic extension methods, I'm not sure we can do that without adding an additional required parameter.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.