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

FullObjectGraphTraversalStrategy.TraverseObject<TContext> makes its dictionary checks in (what appears to be) the wrong order #656

Closed
airbreather opened this issue Dec 8, 2021 · 1 comment · Fixed by #904
Labels

Comments

@airbreather
Copy link
Contributor

airbreather commented Dec 8, 2021

Shorter description: .EnsureRoundTrip() serializers fail to handle Dictionary<TKey, TValue> properties nested in other objects, because the check for IDictionary happens first and assigns the static type of object for both the key and value, before the check for IDictionary<TKey, TValue> that would assign the static types of TKey and TValue.

Longer description, with more specifics for why this happens, a minimal repro, and a workaround:


This bit here traverses an object that implements IDictionary<TKey, TValue>:

var genericDictionaryType = ReflectionUtility.GetImplementedGenericInterface(value.Type, typeof(IDictionary<,>));
if (genericDictionaryType != null)
{
var genericArguments = genericDictionaryType.GetGenericArguments();
var adaptedDictionary = Activator.CreateInstance(typeof(GenericDictionaryToNonGenericAdapter<,>).MakeGenericType(genericArguments), value.Value)!;
TraverseDictionary(new ObjectDescriptor(adaptedDictionary, value.Type, value.StaticType, value.ScalarStyle), visitor, genericArguments[0], genericArguments[1], context, path);
return;
}

And this bit here traverses an object that implements the non-generic IDictionary:

if (typeof(IDictionary).IsAssignableFrom(value.Type))
{
TraverseDictionary(value, visitor, typeof(object), typeof(object), context, path);
return;
}

Which looks like a fantastic idea to me... except that it actually checks the non-generic IDictionary first, and so it thinks that the key and value types for a Dictionary<TKey, TValue> are statically object instead of what they actually are, which causes problems for a .EnsureRoundTrip() serializer.

Here's a full repro, with a workaround below.

ConsoleApp0.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="YamlDotNet" Version="11.2.1" />
  </ItemGroup>

</Project>

Program.cs

using YamlDotNet.Serialization;

var serializer = new SerializerBuilder()
    .EnsureRoundtrip()
    .Build();

using StringWriter stringWriter = new();
serializer.Serialize(stringWriter, new MyOuterThing(), typeof(MyOuterThing));
Console.WriteLine(stringWriter);

public sealed class MyOuterThing
{
    public Dictionary<int, Payload> Lookups { get; set; } = new() { [1] = new() { I = 1 } };
}

public sealed class Payload
{
    public int I { get; set; }
}

Expected: should work
Actual: doesn't, exception:

YamlDotNet.Core.YamlException: (Line: 1, Col: 1, Idx: 0) - (Line: 1, Col: 1, Idx: 0): Cannot serialize type 'Payload' where a 'System.Object' was expected because no tag mapping has been registered for 'Payload', which means that it won't be possible to deserialize the document.
Register a tag mapping using the SerializerBuilder.WithTagMapping method.

E.g: builder.WithTagMapping("!Payload", typeof(Payload));
   at YamlDotNet.Serialization.EventEmitters.TypeAssigningEventEmitter.AssignTypeIfNeeded(ObjectEventInfo eventInfo)
   at YamlDotNet.Serialization.EventEmitters.TypeAssigningEventEmitter.Emit(MappingStartEventInfo eventInfo, IEmitter emitter)
   at YamlDotNet.Serialization.ObjectGraphVisitors.AnchorAssigningObjectGraphVisitor.VisitMappingStart(IObjectDescriptor mapping, Type keyType, Type valueType, IEmitter context)
   at YamlDotNet.Serialization.ObjectGraphVisitors.ChainedObjectGraphVisitor.VisitMappingStart(IObjectDescriptor mapping, Type keyType, Type valueType, IEmitter context)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.TraverseProperties[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.RoundtripObjectGraphTraversalStrategy.TraverseProperties[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.TraverseObject[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.Traverse[TContext](Object name, IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.TraverseDictionary[TContext](IObjectDescriptor dictionary, IObjectGraphVisitor`1 visitor, Type keyType, Type valueType, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.TraverseObject[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.Traverse[TContext](Object name, IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.TraverseProperties[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.RoundtripObjectGraphTraversalStrategy.TraverseProperties[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.TraverseObject[TContext](IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.Traverse[TContext](Object name, IObjectDescriptor value, IObjectGraphVisitor`1 visitor, TContext context, Stack`1 path)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.YamlDotNet.Serialization.IObjectGraphTraversalStrategy.Traverse[TContext](IObjectDescriptor graph, IObjectGraphVisitor`1 visitor, TContext context)
   at YamlDotNet.Serialization.SerializerBuilder.ValueSerializer.SerializeValue(IEmitter emitter, Object value, Type type)
   at YamlDotNet.Serialization.Serializer.EmitDocument(IEmitter emitter, Object graph, Type type)
   at YamlDotNet.Serialization.Serializer.Serialize(IEmitter emitter, Object graph, Type type)
   at YamlDotNet.Serialization.Serializer.Serialize(TextWriter writer, Object graph, Type type)
   at Program.<Main>$(String[] args) in C:\REDACTED\Program.cs:line 8

I'm able to work around it by using a subclass that does the generic type check first before falling back to the base, which looks like this:

Program.cs

using YamlDotNet.Serialization;
using YamlDotNet.Serialization.NamingConventions;
using YamlDotNet.Serialization.ObjectGraphTraversalStrategies;

var serializer = new SerializerBuilder()
    .EnsureRoundtrip()
    .WithObjectGraphTraversalStrategyFactory((typeInspector, typeResolver, typeConverters, maximumRecursion) => new CustomFullObjectGraphTraversalStrategy(typeInspector, typeResolver, maximumRecursion, CamelCaseNamingConvention.Instance))
    .Build();

using StringWriter stringWriter = new();
serializer.Serialize(stringWriter, new MyOuterThing(), typeof(MyOuterThing));
Console.WriteLine(stringWriter);

public sealed class MyOuterThing
{
    public Dictionary<int, Payload> Lookups { get; set; } = new() { [1] = new() { I = 1 } };
}

public sealed class Payload
{
    public int I { get; set; }
}

public sealed class CustomFullObjectGraphTraversalStrategy : FullObjectGraphTraversalStrategy
{
    public CustomFullObjectGraphTraversalStrategy(ITypeInspector typeDescriptor, ITypeResolver typeResolver, int maxRecursion, INamingConvention namingConvention)
        : base(typeDescriptor, typeResolver, maxRecursion, namingConvention)
    {
    }

    protected override void TraverseObject<TContext>(IObjectDescriptor value, IObjectGraphVisitor<TContext> visitor, TContext context, Stack<ObjectPathSegment> path)
    {
        if (value.Type.IsGenericType && value.Type.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IDictionary<,>)))
        {
            var genericArguments = value.Type.GetGenericArguments();
            TraverseDictionary(new ObjectDescriptor(value.Value, value.Type, value.StaticType, value.ScalarStyle), visitor, genericArguments[0], genericArguments[1], context, path);
        }
        else
        {
            base.TraverseObject(value, visitor, context, path);
        }
    }
}
EdwardCooke added a commit that referenced this issue May 11, 2024
Add a regression test for #656

+semver:fix
@aaubry
Copy link
Owner

aaubry commented May 11, 2024

A fix for this issue has been released in version 15.1.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants