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 source gen support for unspeakable types #82457

Closed
Tracked by #79122
brunolins16 opened this issue Feb 22, 2023 · 9 comments · Fixed by #83631 or #84768
Closed
Tracked by #79122

Add source gen support for unspeakable types #82457

brunolins16 opened this issue Feb 22, 2023 · 9 comments · Fixed by #83631 or #84768
Assignees
Labels
area-System.Text.Json partner-impact This issue impacts a partner who needs to be kept updated source-generator Indicates an issue with a source generator feature
Milestone

Comments

@brunolins16
Copy link
Member

Description

Minimal APIs uses the runtime type to call the serializer in most of the scenarios, that means once I have a compiler generated type (unspeakable type) returning from my API, this type will be used for serialization.

While it works correctly when the reflection-based resolver is turned on, I could not find a way to add this compiler generated type to my source-generated JsonSerializerContext. Even if I try to specify the interface implemented by type, eg.: IAsyncEnumerable and still getting the following error:

Metadata for type 'Program+<<<Main>$>g__Foo|0_5>d' was not provided by TypeInfoResolver of type 'MyContext'. If using source generation, ensure that all root types passed to the serializer have been indicated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.

Reproduction Steps

var builder = WebApplication.CreateBuilder(args);
builder.Services.ConfigureHttpJsonOptions(options => options.SerializerOptions.TypeInfoResolver = MyContext.Default);
var app = builder.Build();

app.MapGet("/foo", Foo);
async IAsyncEnumerable<int> Foo()
{
    foreach (var i in Enumerable.Range(0, 10))
    {
        await Task.Delay(1000);
        yield return i;
    }
}

[JsonSerializable(typeof(IAsyncEnumerable<int>))]
internal partial class MyContext : JsonSerializerContext
{}

Expected behavior

While is possible to have some additional logic on ASP.NET Core side, to check some sort of known types, the expected behavior is to have the metadata resolution be able to detect, as a fallback in the source generator context, if the requested type implements one of the interfaces registered in the context.

Eg.:

Current source-gen`ed context

    public override global::System.Text.Json.Serialization.Metadata.JsonTypeInfo? GetTypeInfo(global::System.Type type)
    {
        if (type == typeof(global::System.Collections.Generic.IAsyncEnumerable<global::System.Int32>))
        {
            return this.IAsyncEnumerableInt32;
        }
        return null;
   }

Expected

    public override global::System.Text.Json.Serialization.Metadata.JsonTypeInfo? GetTypeInfo(global::System.Type type)
    {
        if (type == typeof(global::System.Collections.Generic.IAsyncEnumerable<global::System.Int32>))
        {
            return this.IAsyncEnumerableInt32;
        }

        if (type.IsAssignableFrom(typeof(global::System.Collections.Generic.IAsyncEnumerable<global::System.Int32>)))
        {
            return this.IAsyncEnumerableInt32;
        }

        return null;
   }

Actual behavior

System.NotSupportedException: Metadata for type 'Program+<<<Main>$>g__Foo|0_5>d' was not provided by TypeInfoResolver of type 'MyContext'. If using source generation, ensure that all root types passed to the serializer have been indicated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException_NoMetadataForType(Type type, IJsonTypeInfoResolver resolver)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Boolean resolveIfMutable)
   at Microsoft.AspNetCore.Internal.ExecuteHandlerHelper.WriteJsonResponseAsync[T](HttpResponse response, T value, JsonSerializerOptions options, JsonTypeInfo`1 jsonTypeInfo)
   at lambda_method4(Closure, Object, HttpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 22, 2023
@ghost
Copy link

ghost commented Feb 22, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Minimal APIs uses the runtime type to call the serializer in most of the scenarios, that means once I have a compiler generated type (unspeakable type) returning from my API, this type will be used for serialization.

While it works correctly when the reflection-based resolver is turned on, I could not find a way to add this compiler generated type to my source-generated JsonSerializerContext. Even if I try to specify the interface implemented by type, eg.: IAsyncEnumerable and still getting the following error:

Metadata for type 'Program+<<<Main>$>g__Foo|0_5>d' was not provided by TypeInfoResolver of type 'MyContext'. If using source generation, ensure that all root types passed to the serializer have been indicated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.

Reproduction Steps

var builder = WebApplication.CreateBuilder(args);
builder.Services.ConfigureHttpJsonOptions(options => options.SerializerOptions.TypeInfoResolver = MyContext.Default);
var app = builder.Build();

app.MapGet("/foo", Foo);
async IAsyncEnumerable<int> Foo()
{
    foreach (var i in Enumerable.Range(0, 10))
    {
        await Task.Delay(1000);
        yield return i;
    }
}

[JsonSerializable(typeof(IAsyncEnumerable<int>))]
internal partial class MyContext : JsonSerializerContext
{}

Expected behavior

While is possible to have some additional logic on ASP.NET Core side, to check some sort of known types, the expected behavior is to have the metadata resolution be able to detect, as a fallback in the source generator context, if the requested type implements one of the interfaces registered in the context.

Eg.:

Current source-gen`ed context

    public override global::System.Text.Json.Serialization.Metadata.JsonTypeInfo? GetTypeInfo(global::System.Type type)
    {
        if (type == typeof(global::System.Collections.Generic.IAsyncEnumerable<global::System.Int32>))
        {
            return this.IAsyncEnumerableInt32;
        }
        return null;
   }

Expected

    public override global::System.Text.Json.Serialization.Metadata.JsonTypeInfo? GetTypeInfo(global::System.Type type)
    {
        if (type == typeof(global::System.Collections.Generic.IAsyncEnumerable<global::System.Int32>))
        {
            return this.IAsyncEnumerableInt32;
        }

        if (type.IsAssignableFrom(typeof(global::System.Collections.Generic.IAsyncEnumerable<global::System.Int32>)))
        {
            return this.IAsyncEnumerableInt32;
        }

        return null;
   }

Actual behavior

System.NotSupportedException: Metadata for type 'Program+<<<Main>$>g__Foo|0_5>d' was not provided by TypeInfoResolver of type 'MyContext'. If using source generation, ensure that all root types passed to the serializer have been indicated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException_NoMetadataForType(Type type, IJsonTypeInfoResolver resolver)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Boolean resolveIfMutable)
   at Microsoft.AspNetCore.Internal.ExecuteHandlerHelper.WriteJsonResponseAsync[T](HttpResponse response, T value, JsonSerializerOptions options, JsonTypeInfo`1 jsonTypeInfo)
   at lambda_method4(Closure, Object, HttpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: brunolins16
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis eiriktsarpalis added partner-impact This issue impacts a partner who needs to be kept updated and removed untriaged New issue has not been triaged by the area owner labels Feb 22, 2023
@eiriktsarpalis eiriktsarpalis added this to the 8.0.0 milestone Feb 22, 2023
@eiriktsarpalis eiriktsarpalis added the source-generator Indicates an issue with a source generator feature label Feb 22, 2023
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Feb 22, 2023

For contrast, this is how the source generator produces metadata for IAsyncEnumerable's implementations that are visible to the source generator:

[JsonSerializable(typeof(MyAsyncEnumerable))]
public partial class MyContext : JsonSerializerContext
{
}

public class MyAsyncEnumerable : IAsyncEnumerable<int>
{
    IAsyncEnumerator<int> IAsyncEnumerable<int>.GetAsyncEnumerator(CancellationToken cancellationToken)
        => throw new NotImplementedException();
}

Produces

jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateIAsyncEnumerableInfo<global::MyAsyncEnumerable, global::System.Int32>(options, info);

It's important to note that we wouldn't be able produce equivalent metadata for private implementations without resorting to Type.MakeGenericType calls, which are out of the question for NativeAOT scenaria. This is how the reflection converter factory works today:

Type converterType = typeof(IAsyncEnumerableOfTConverter<,>).MakeGenericType(typeToConvert, elementType);
return (JsonConverter)Activator.CreateInstance(converterType)!;

One possible solution is that we use nearest ancestor resolution logic and fall back to the closest available supertype. This fallback should only happen in serialization as generally speaking it could only be supported in contravariant methods. We should never be returning the JsonTypeInfo of a supertype in any of the GetTypeInfo methods as that would violate the following invariants:

  1. options.GetTypeInfo(type).Type == type and
  2. options.GetTypeInfo(typeof(T)) is JsonTypeInfo<T>

We also need to be careful about leaving out fallback to trivial supertypes such as JsonTypeInfo<object> as that would always just produce {} in JSON.

@eiriktsarpalis eiriktsarpalis self-assigned this Mar 9, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 18, 2023
@krwq
Copy link
Member

krwq commented Mar 20, 2023

Why is ASP.NET looking at the runtime type when calling JSON? It should be using return type directly and then the metadata for unspeakable type wouldn't be needed... Am I missing something here?

@eiriktsarpalis
Copy link
Member

Why is ASP.NET looking at the runtime type when calling JSON? It should be using return type directly and then the metadata for unspeakable type wouldn't be needed... Am I missing something here?

Short answer is filters. They can return object so any static type information is lost

@krwq
Copy link
Member

krwq commented Mar 20, 2023

should they be special casing that on their side then?

@eiriktsarpalis
Copy link
Member

They could, although it would be replicating the logic that the PR is introducing. And, as mentioned in #83631 (comment) the issue is not specific to boxing, it also addresses the issue of private interface implementation support.

cc @eerhardt

@eerhardt
Copy link
Member

Note, ASP.NET also looks at the runtime type (today) in order to get the intended polymorphism serialization behavior.

See:

That should hopefully no longer be needed once #77532 is addressed.

should they be special casing that on their side then?

How would ASP.NET special case it? It only has a single type - the runtime type. There is no "declared type" in filters. The "declared type" is object.

@eiriktsarpalis
Copy link
Member

And it's not just impacting ASP.NET obviously, we've had our own share of issues filed about this: #79661

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 20, 2023
@eiriktsarpalis
Copy link
Member

Reopening to track potential changes in the semantics of the feature (enabling via feature flags, etc.)

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 13, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 19, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json partner-impact This issue impacts a partner who needs to be kept updated source-generator Indicates an issue with a source generator feature
Projects
None yet
4 participants