RickStrahl / Expando

Extensible dynamic types for .NET
108 stars 30 forks source link

JSON.NET [JsonRequired] is broken #14

Closed flakey-bit closed 6 years ago

flakey-bit commented 6 years ago

As demonstrated by the test case below, the behaviour of [JsonRequired] from Json.NET is broken.

    [TestFixture]
    public class JsonRequiredTests
    {
        [Test]
        public void TestJsonRequired()
        {
            // Passes
            Assert.That(() => JsonConvert.DeserializeObject<MyTestCase>("{}"), Throws.InstanceOf<JsonSerializationException>());

            // Fails
            Assert.That(() => JsonConvert.DeserializeObject<MyTestCase2>("{}"), Throws.InstanceOf<JsonSerializationException>());
        }

        public class MyTestCase
        {
            [JsonRequired]
            public string SomeProperty { get; set; }
        }

        public class MyTestCase2 : Westwind.Utilities.Expando
        {
            [JsonRequired]
            public string SomeProperty { get; set; }
        }
    }
RickStrahl commented 6 years ago

I think this would be more of an issue for JSON.NET not this class?

These are standard properties that you are adding and they should behave as such during serialization - Expando doesn't do anything special for JSON serialization to work.

flakey-bit commented 6 years ago

You're right - it looks like the attribute is there on the property:

        [Test]
        public void TestPropertyHasAttribute()
        {
            foreach (var type in new [] {typeof(MyTestCase), typeof(MyTestCase2) })
            {
                var prop = type.GetProperty("SomeProperty");
                var matchingAttr = prop.CustomAttributes.FirstOrDefault(a => a.AttributeType.Name == "JsonRequiredAttribute");
                Assert.That(matchingAttr, Is.Not.Null);
            }
        }

So a Json.NET issue most likely as you say - will close the ticket. Thanks for looking into it :)