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

Call constructor on empty #443

Open
Vraiment opened this issue Oct 21, 2019 · 16 comments
Open

Call constructor on empty #443

Vraiment opened this issue Oct 21, 2019 · 16 comments
Labels

Comments

@Vraiment
Copy link

Let's imagine I have the following class:

class MyClass
{
    IList<int> Values { get; set; }
}

Given the following yaml file:

Values:

When I deserialize that class the resulting instance of MyClass will have a null Values, I'd like to have an empty List<int> instead. Is there a way to do this with minimal configuration (for both serializing and deserializing)?

@aaubry
Copy link
Owner

aaubry commented Oct 21, 2019

It is not too hard to achieve.

On the serialization side, you need to add an implementation of IEventEmitter that will recognize this case. Something like this:

public sealed class ForceEmptySequencesOnSerialization : ChainedEventEmitter
{
    public ForceEmptySequencesOnSerialization(IEventEmitter nextEmitter) : base(nextEmitter)
    {
    }

    public override void Emit(ScalarEventInfo eventInfo, IEmitter emitter)
    {
        if (eventInfo.Tag == "tag:yaml.org,2002:null" && IsList(eventInfo.Source.Type))
        {
            emitter.Emit(new SequenceStart(null, null, false, SequenceStyle.Flow));
            emitter.Emit(new SequenceEnd());
        }
        else
        {
            base.Emit(eventInfo, emitter);
        }
    }

    private bool IsList(Type type)
    {
        return typeof(IList).IsAssignableFrom(type)
            || (type.IsInterface && type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IList<>))
            || type.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IList<>));
    }
}

and register it in the SerializerBuilder:

var serializer = new SerializerBuilder()
    .WithEventEmitter(inner => new ForceEmptySequencesOnSerialization(inner), s => s.After<TypeAssigningEventEmitter>())
    .Build();

I have registered it after TypeAssigningEventEmitter so that we can test that the tag is "tag:yaml.org,2002:null".

On the deserialization side, you need an INodeDeserializer similar to this:

public sealed class ForceEmptyListsOnDeserialization : INodeDeserializer
{
    private readonly IObjectFactory objectFactory = new DefaultObjectFactory();

    public bool Deserialize(IParser parser, Type expectedType, Func<IParser, Type, object> nestedObjectDeserializer, out object value)
    {
        value = null;

        if (IsList(expectedType) && parser.Accept<NodeEvent>(out var evt))
        {
            if (NodeIsNull(evt))
            {
                parser.SkipThisAndNestedEvents();
                value = objectFactory.Create(expectedType);
                return true;
            }
        }

        return false;
    }

    private bool NodeIsNull(NodeEvent nodeEvent)
    {
        // http://yaml.org/type/null.html

        if (nodeEvent.Tag == "tag:yaml.org,2002:null")
        {
            return true;
        }

        if (nodeEvent is Scalar scalar && scalar.Style == ScalarStyle.Plain)
        {
            var value = scalar.Value;
            return value == "" || value == "~" || value == "null" || value == "Null" || value == "NULL";
        }

        return false;
    }

    private bool IsList(Type type)
    {
        return typeof(IList).IsAssignableFrom(type)
            || (type.IsInterface && type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IList<>))
            || type.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IList<>));
    }
}

and register it like this:

var deserializer = new DeserializerBuilder()
    .WithNodeDeserializer(new ForceEmptyListsOnDeserialization())
    .Build();

@Vraiment
Copy link
Author

So for deserialization it worked perfectly, for serialization didn't. I didn't mention it but I will warranty that there are no null values in my classes so the eventInfo.Tag == "tag:yaml.org,2002:null" line will always be false.

But the output of serializing an empty list is Values: [] instead of Values:, how do I accomplish this?

@aaubry
Copy link
Owner

aaubry commented Oct 27, 2019

Ok, I misunderstood what you wanted to achieve. However the same logic applies, but instead of testing eventInfo.Tag == "tag:yaml.org,2002:null" you need to check if the list is empty, and if it is, you need to emit a single Scalar with an empty value.

@Vraiment
Copy link
Author

The problem is ScalarEventInfo.Source has a string with the alias of the property at that point instead of the list.

After some debugging I figured out that the methods for SequenceStartEventInfo and SequenceEndEventInfo have access to the the list via the Source property.

So I modified my custom event emitter with the following:

public override void Emit(SequenceStartEventInfo eventInfo, IEmitter emitter)
{
    if (ImplementsIList(eventInfo.Source.Type) && ListIsEmpty(eventInfo.Source.Type, eventInfo.Source.Value))
    {
        IObjectDescriptor objectDescriptor = new ObjectDescriptor(string.Empty, typeof(string), typeof(string), ScalarStyle.Any);
        ScalarEventInfo scalarEventInfo = new ScalarEventInfo(objectDescriptor);

        Emit(scalarEventInfo, emitter);

        return;
    }

    base.Emit(eventInfo, emitter);
}

public override void Emit(SequenceEndEventInfo eventInfo, IEmitter emitter)
{
    if (ImplementsIList(eventInfo.Source.Type) && ListIsEmpty(eventInfo.Source.Type, eventInfo.Source.Value))
    {
        return;
    }

    base.Emit(eventInfo, emitter);
}

The problem with this approach is an exception is thrown:

YamlDotNet.Core.YamlException
  HResult=0x80131500
  Message=(Line: 1, Col: 1, Idx: 0) - (Line: 1, Col: 1, Idx: 0): Neither tag nor isImplicit flags are specified.
  Source=YamlDotNet
  StackTrace:
   at YamlDotNet.Core.Emitter.SelectScalarStyle(ParsingEvent evt)
   at YamlDotNet.Core.Emitter.EmitScalar(ParsingEvent evt)
   at YamlDotNet.Core.Emitter.EmitNode(ParsingEvent evt, Boolean isRoot, Boolean isMapping, Boolean isSimpleKey)
   at YamlDotNet.Core.Emitter.EmitBlockMappingValue(ParsingEvent evt, Boolean isSimple)
   at YamlDotNet.Core.Emitter.StateMachine(ParsingEvent evt)
   at YamlDotNet.Core.Emitter.Emit(ParsingEvent event)
   at YamlDotNet.Serialization.EventEmitters.WriterEventEmitter.YamlDotNet.Serialization.IEventEmitter.Emit(MappingEndEventInfo eventInfo, IEmitter emitter)
   at YamlDotNet.Serialization.EventEmitters.ChainedEventEmitter.Emit(MappingEndEventInfo eventInfo, IEmitter emitter)
   at YamlDotNet.Serialization.EventEmitters.ChainedEventEmitter.Emit(MappingEndEventInfo eventInfo, IEmitter emitter)
   at YamlDotNet.Serialization.ObjectGraphVisitors.EmittingObjectGraphVisitor.YamlDotNet.Serialization.IObjectGraphVisitor<YamlDotNet.Core.IEmitter>.VisitMappingEnd(IObjectDescriptor mapping, IEmitter context)
   at YamlDotNet.Serialization.ObjectGraphVisitors.ChainedObjectGraphVisitor.VisitMappingEnd(IObjectDescriptor mapping, IEmitter context)
   at YamlDotNet.Serialization.ObjectGraphVisitors.ChainedObjectGraphVisitor.VisitMappingEnd(IObjectDescriptor mapping, IEmitter context)
   at YamlDotNet.Serialization.ObjectGraphVisitors.ChainedObjectGraphVisitor.VisitMappingEnd(IObjectDescriptor mapping, IEmitter context)
   at YamlDotNet.Serialization.ObjectGraphTraversalStrategies.FullObjectGraphTraversalStrategy.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)
   at YamlDotNet.Serialization.Serializer.Serialize(Object graph)
   at SAGESharp.SLB.Level.SerializationTests.Test_Writing_A_Yaml_Conversation_File_Successfully(SerializationTestCaseData`1 testCaseData)

@aaubry
Copy link
Owner

aaubry commented Oct 28, 2019

I think you need to set the IsPlainImplicit property of ScalarEventInfo to true because the tag is not set. I can't confirm right now, but I will check later.

@Vraiment
Copy link
Author

Seeting IsPlainImplicit to true helped but it generated the following file:

Values: ''

I've decided that this is more complex that I expected and I will just not deviate from the yaml specification, clients should write [] to specify an empty list.

Feel free to solve this issue, thanks!

@DerAlbertCom
Copy link

DerAlbertCom commented Mar 12, 2021

Sorry, to interrupt this discussion ;)

{
   MyClass() { 
     Values = new List<int>();
  }
    IList<int> Values { get; set; }
}

This is good coding practices, never allow null for Collections. You don't need to null check, less errors, less problems.

And with that YamlDotnet and everything else works as you desires.

@mpelesh
Copy link

mpelesh commented Mar 19, 2021

Nope. Deserializer will still set it to null.

@DerAlbertCom
Copy link

DerAlbertCom commented Mar 21, 2021

@mpelesh maybe you have customized it. Can't reproduced that problem.

  public class EmptyListTest 
    {
        private readonly ITestOutputHelper _outputHelper;

        public EmptyListTest(ITestOutputHelper outputHelper)
        {
            _outputHelper = outputHelper;
        }

        public class NullList
        {
            public NullList()
            {
                Strings = new List<string>() { };
            }
            public IList<string> Strings { get; private  set; }
        }

        [Fact]
        public void Should_not_have_null()
        {
            var originalList = new NullList();
            var serializer = new SerializerBuilder().Build();

            var yaml = serializer.Serialize(originalList);
            _outputHelper.WriteLine(yaml);

            var deserializer = new DeserializerBuilder().Build();

            var list = deserializer.Deserialize<NullList>(yaml);

            list.Strings.Should().NotBeNull();
            list.Strings.Should().HaveCount(0);
        }
        
        [Fact]
        public void Should_restore_list()
        {
            var originalList = new NullList();
            originalList.Strings.Add("FooBar");
            var serializer = new SerializerBuilder().Build();

            var yaml = serializer.Serialize(originalList);
            _outputHelper.WriteLine(yaml);

            var deserializer = new DeserializerBuilder().Build();

            var list = deserializer.Deserialize<NullList>(yaml);

            list.Strings.Should().HaveCount(1);
            list.Strings[0].Should().Be("FooBar");
        }
    }

Result of Should_not_have_null
image

Result of Should_restore_list()
image

@AltarnRain
Copy link

I also ran into the this issue. Particularly nasty since I've fully embraced 'Nullable' as a project option.

While I'd love to see YamlDotNet add support for initializing a collection as 'empty' when there are no elements to add to it I found a solution.

@DerAlbertCom solution did not work for me. I tried initializing my collections in a constructor and via property initialization. Neither worked, my collections, arrays in this case, were still set to null but I adjusted your solution a little. I did not investigate why, I did find a solution. Use a full property.

public class MyClass
{
	private string[]? myProp;
        public string[] MyProperty { get => this.myProp ?? Array.Empty<string>(); set => this.myProp = value }
}

I like this solution because it places the responsibility of delivering an empty collection on the class and not the deserializer.

Implementing this is a breeze. Visual Studio can convert an auto property to a full property and vice versa. All you need to do is return 'empty' in case of null whatever 'empty' is.

Hope this helps.

@seanamosw
Copy link

So I recently bumped into this. What is interesting, is using a property initializer at some point in the past used to work.

Code like this breaks now:

public class Test
{
    public List<X> Things { get; set; } = new List<X>();
}

Things will now be deserialized as null on an empty Things property, eg. things: . We had code like this that was in a not often hit codepath and it used to work. Today someone found it was blowing up as a nullref.

I unfortunately cannot say when this change happened, again it was in a very seldom hit codepath.

@ahaleiii
Copy link

ahaleiii commented Oct 8, 2021

Given objects:

public class Root {
  public MyObjectA ObjA { get; set; } = new MyObjectA();
  public MyObjectB ObjB { get; set; } = new MyObjectB();
}

public class MyObjectA {
  public string ItemA1 { get; set; }
  public string ItemA2 { get; set; }
}

public class MyObjectB {
  public string ItemB1 { get; set; }
  public string ItemB2 { get; set; }
}

And given yaml:

# yaml1
---
obj-a:
  item-a1: "foo"
  item-a2: "bar"
...
# yaml2
---
obj-a:
  item-a1: "foo"
  item-a2: "bar"
obj-b:
...

I would expect the deserializer (using HyphenatedNamingConvention) to produce the same results. (YamlDotNet NuGet package v11.2.1.)

Instead, yaml2 will produce a root.ObjB value of null. Why?

@AltarnRain
Copy link

Given objects:

public class Root {
  public MyObjectA ObjA { get; set; } = new MyObjectA();
  public MyObjectB ObjB { get; set; } = new MyObjectB();
}

public class MyObjectA {
  public string ItemA1 { get; set; }
  public string ItemA2 { get; set; }
}

public class MyObjectB {
  public string ItemB1 { get; set; }
  public string ItemB2 { get; set; }
}

And given yaml:

# yaml1
---
obj-a:
  item-a1: "foo"
  item-a2: "bar"
...
# yaml2
---
obj-a:
  item-a1: "foo"
  item-a2: "bar"
obj-b:
...

I would expect the deserializer (using HyphenatedNamingConvention) to produce the same results. (YamlDotNet NuGet package v11.2.1.)

Instead, yaml2 will produce a root.ObjB value of null. Why?

I would too and judging from seanamosw reply to my comment this used to work in the past and now it does not. Perhaps message the authors of the package?

@rcdailey
Copy link

rcdailey commented Jun 16, 2022

@aaubry Why not a simpler IsList condition?

private static bool IsList(Type type)
{
    return typeof(IEnumerable).IsAssignableFrom(type);
}

The solution you posted checks IList which doesn't allow ICollection to work.

rcdailey added a commit to recyclarr/recyclarr that referenced this issue Jun 16, 2022
When you specify an empty object in YAML, like:

```
quality_profiles:
```

This causes that respective object/collection to be assigned `null`.
YamlDotNet feature request covering this behavior can be found
[here][1].

Fixes #89.

[1]: aaubry/YamlDotNet#443
@aaubry
Copy link
Owner

aaubry commented Jun 17, 2022

@aaubry Why not a simpler IsList condition?

private static bool IsList(Type type)
{
    return typeof(IEnumerable).IsAssignableFrom(type);
}

The solution you posted checks IList which doesn't allow ICollection to work.

My implementation checks for both IList and IList<T>. Some classes may implement only the generic one. You could probably use ICollection but I don't remember if there's a reason for wanting IList specifically.
Note that just testing for IEnumerable will match types that you don't consider collections, such as string.

@rcdailey
Copy link

Due to the nature of how YAML is structured, is it even possible for string to be processed as a collection here? Legitimate question. I can't think of a YAML example that would cause this.

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

No branches or pull requests

8 participants