microsoft / semantic-kernel

Integrate cutting-edge LLM technology quickly and easily into your apps
https://aka.ms/semantic-kernel
MIT License
21.91k stars 3.27k forks source link

.Net: Support of JsonSerializerSettings in SK #5867

Open SergeyMenshykh opened 7 months ago

SergeyMenshykh commented 7 months ago

Today, SK has several content classes that are registered for polymorphic {de}serialization using the JsonDerivedType attribute on the base KernelContent class.

There are two issues with the approach identified so far:

  1. It is not really scalable because each SK component must copy component-specific content classes to the SK abstractions project/package, only to be able to register them for polymorphic usage. (We must not reference the specific component project from the abstraction one.)
  2. It is not possible to register custom content classes, which SK consumers may create and use, for polymorphic {de}serialization.

Scope:

  1. Design a solution that would deal with the above issues.
  2. Make sure the solution can support/extensible enough to support AOT - https://github.com/microsoft/semantic-kernel/issues/5411

Additional context: https://github.com/microsoft/semantic-kernel/pull/5639

SergeyMenshykh commented 7 months ago

CC: @stephentoub , @eiriktsarpalis

eiriktsarpalis commented 7 months ago

For a bit of context, STJ doesn't support polymorphic serialization in open class hierarchies because under certain circumstances it creates the conditions for remote code execution using maliciously crafted payloads. Derived types must therefore be explicitly opted in. Additionally, they need to be opted in at the declaration of the base type, because it helps with compile-time discovery for the case of the source generator.

Nevertheless, derived types can be configured programmatically at runtime via the contract customization APIs (specifically using the JsonTypeInfo.PolymorphismOptions). For the case of AOT the user would additionally be responsible to provide source generated contexts, which might look as follows:

// Types derived in SK
[JsonDerivedType(typeof(DerivedKernelContent))]
class KernelContent;

class DerivedKernelContent;

[JsonSerializable(typeof(KernelContent))]
partial class SKContext : JsonSerializerContext;

// Types defined in user assemblies
class UserDerivedType : KernelContent;

[JsonSerializable(typeof(UserDerivedType))]
partial class UserContext : JsonSerializerContext
{
    public static void AppendContext(JsonSerializerOptions options)
    {
        // Append this context to the resolve chain
        // and modify the contract for `KernelContent` to include the derived type.
        options.TypeInfoResolver = JsonTypeInfoResolver
            .Combine(options.TypeInfoResolver, UserContext.Default)
            .WithAddedModifier(typeInfo =>
            {
                if (typeInfo.Type == typeof(KernelContent))
                {
                    typeInfo.PolymorphismOptions?.DerivedTypes.Add(new(typeof(UserDerivedType), "userDerived"));
                }
            });
    }
}

That approach forces a lot of boilerplate on the user though, so you might instead want to consider a registration approach. This could either happen via an extension method:

public static class KernelContentExtensions
{
    public static void WithDerivedKernelContentType(this JsonSerializerOptions options, Type derivedType, string? identifier = null, JsonSerializerContext? context = null)
    {
        IJsonTypeInfoResolver? resolver = options.TypeInfoResolver;
        if (context != null)
        {
            resolver = JsonTypeInfoResolver.Combine(resolver, context);
        }

        resolver = resolver?.WithAddedModifier(typeInfo =>
        {
            if (typeInfo.Type == typeof(KernelContent))
            {
                JsonDerivedType derivedTypeInfo = identifier is null ? new(derivedType) : new(derivedType, identifier);
                typeInfo.PolymorphismOptions?.DerivedTypes.Add(derivedTypeInfo);
            }
        });

        options.TypeInfoResolver = resolver;
    }
}

which users might be able to configure as follows:

options.WithDerivedKernelContentType(typeof(UserDerivedType), identifier: "userDerived", UserContext.Default);

class UserDerivedType : KernelContent;

[JsonSerializable(typeof(UserDerivedType))]
partial class UserContext : JsonSerializerContext;

Assuming you don't care about AOT support, or have concerns about, say, delayed assembly loading things becomes easier. For example, it might be possible to define this custom attribute:

var options = new JsonSerializerOptions { TypeInfoResolver = new DefaultJsonTypeInfoResolver().WithAddedModifier(DerivingFromAttribute.ConfigureDerivingFromAttribute) };
var content = JsonSerializer.Deserialize<KernelContent>("""{"$type":"derived"}""", options);
Console.WriteLine(content is DerivedContent); // True

class KernelContent;

[DerivingFrom(typeof(KernelContent), "derived")]
class DerivedContent : KernelContent;

public class DerivingFromAttribute(Type baseType, string? typeDiscriminator = null) : Attribute
{
    public Type BaseType { get; } = baseType;
    public string? TypeDiscriminator { get; } = typeDiscriminator;

    public static void ConfigureDerivingFromAttribute(JsonTypeInfo typeInfo)
    {
        if (typeInfo.Type.IsSealed)
        {
            return;
        }

        // !!! run time search for all derived types !!!
        var derivedTypes = AppDomain.CurrentDomain.GetAssemblies()
            .SelectMany(asm => asm.GetTypes())
            .Where(t => typeInfo.Type.IsAssignableFrom(t));

        foreach (Type derivedType in derivedTypes)
        {
            if (derivedType.GetCustomAttribute<DerivingFromAttribute>() is DerivingFromAttribute attr && attr.BaseType == typeInfo.Type)
            {
                JsonDerivedType jsonDerivedType = attr.TypeDiscriminator is null ? new(derivedType) : new(derivedType, attr.TypeDiscriminator);
                (typeInfo.PolymorphismOptions ??= new()).DerivedTypes.Add(jsonDerivedType);
            }
        }
    }
}
github-actions[bot] commented 3 months ago

This issue is stale because it has been open for 90 days with no activity.