Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function overloads with Type as parameter for KiotaSerializer.Deserialize<T> #210

Open
dansmitt opened this issue Mar 21, 2024 · 4 comments
Labels
enhancement New feature or request Standard GitHub label Issue caused by core project dependency modules or library

Comments

@dansmitt
Copy link

We (@MichaMican and I) have implemented an TextFormatter extension to use Kiota as thge standard serializer for IParsable objects when working with ASP.NET Controllers.

See: microsoftgraph/msgraph-sdk-dotnet#2343 (comment)

In our TextInputFormatter we have to fallback to using Reflection because we work with an implementation of IParsable object that we have to set as the generic type of KiotaSerializer.Deserialize<T> during run time. As of now the function unfortunately has no overload to specify the Type as a parameter which would be super usefull in our case as we could then imrpove code readability by not using reflection.

Snippet from the TextInputFormatter:

public override Task<InputFormatterResult> ReadRequestBodyAsync(InputFormatterContext context, Encoding encoding)
{
    var httpContext = context.HttpContext;

    if (httpContext.Request.Body.CanRead)
    {
        var targetType = context.ModelType; // <- This is where we get the Type for Deserialize per runtime

        #region Reflection to define Generic type T of KiotaJsonSerializer.Deserialize<T> dynamically
        var methodInfo = typeof(KiotaJsonSerializer).GetMethod("Deserialize", BindingFlags.Public | BindingFlags.Static, [typeof(Stream)]);
        if (methodInfo == null)
        {
            throw new Exception("KiotaJsonSerializer.Deserialize changed. Please update implementation");
        }

        // This line types the KiotaJsonSerializer.Deserialize<T> to T equals to targetType
        var methodWithForcedTargetType = methodInfo.MakeGenericMethod(targetType);

        // obj is null because KiotaJsonSerializer.Deserialize is a static function
        // This function is analog to KiotaJsonSerializer.Deserialize<T>(httpContext.Request.Body) -> Where T = targetType
        var deserializedObject = methodWithForcedTargetType.Invoke(null, [httpContext.Request.Body]);

       // As we used reflection deserializedObject is of type object, hence we have to continue to use Reflection to access the BackingStore/InitializationCompleted property.
        var backingStoreProperty = (deserializedObject?.GetType().GetProperty("BackingStore"));
        var backingStoreValue = backingStoreProperty?.GetValue(deserializedObject);

        backingStoreValue?.GetType().GetProperty("InitializationCompleted")?.SetValue(backingStoreValue, false);

        #endregion

        return Task.FromResult(InputFormatterResult.Success(deserializedObject));
    }

    return Task.FromResult(InputFormatterResult.Failure());

}
@baywet
Copy link
Member

baywet commented Mar 21, 2024

Hi @dansmitt
Thanks for using kiota and for reaching out.
I'm not sure I'm following what's needed here?
Can you share and updated version of the snippet you've shared, removing the reflection and demonstrating the use of the missing method please?

@baywet baywet added question Further information is requested Needs: Author Feedback labels Mar 21, 2024
@MichaMican
Copy link

MichaMican commented Mar 22, 2024

Hi @baywet
Basically Kiota currently has a static function:

// from Microsoft.Kiota.Abstractions.dll (1.7.5)
public static T? Deserialize<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>(string serializedRepresentation) where T : IParsable```

Note that the generic Type T has to be explicitly defined here as it can't be implicitly derived from the methods parameters.
As @dansmitt mentioned we unfortunately don't have a static type but instead we get it during Runtime from the InputFormatterContext.

Hence if there would be a function where the type can be parsed as a parameter:

public static IParsable? Deserialize(string serializedRepresentation, Type targetTypeToSerializeTo)

we would not have to use reflection to set the generic type T of the KiotaJsonSerializer.Deserialize() function.

I'm not sure if there is a more specific type then IParsable - that this function could return - but that would already help a lot.

For reference System.Text.Json has also a function like that in their Serializer reperteure of JsonSerializer.Deserialize():

// from System.Text.Json.dll (8.0.3)
public static object? Deserialize([StringSyntax(StringSyntaxAttribute.Json)] ReadOnlySpan<char> json, Type returnType, JsonSerializerOptions? options = null)

@dansmitt
Copy link
Author

@baywet what @MichaMican says

@baywet
Copy link
Member

baywet commented Mar 22, 2024

Thanks for the detailed explanation here.
This type is ultimately used to get the factory method of the type

So assuming a bit of refactoring, it should be easy to implement an overload that accepts the type directly.

Is this something you'd be interested in submitting a pull request for?

@baywet baywet added enhancement New feature or request Standard GitHub label Issue caused by core project dependency modules or library and removed question Further information is requested Needs: Attention 👋 labels Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Standard GitHub label Issue caused by core project dependency modules or library
Projects
Status: Todo 📃
Development

No branches or pull requests

3 participants