From c69825f46948125cb3aed03bb066d2b7ae87a286 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Fri, 29 Jul 2022 15:42:45 +0200 Subject: [PATCH 1/4] Support RequiredAttribute for reflection serialization --- .../System.Text.Json/ref/System.Text.Json.cs | 6 + .../src/System.Text.Json.csproj | 1 + .../Attributes/JsonRequiredAttribute.cs | 19 + .../Metadata/JsonPropertyInfo.cs | 19 +- .../JsonSerializerWrapper.Reflection.cs | 3 +- ...nTypeInfoResolverTests.JsonPropertyInfo.cs | 364 ++++++++++++++++++ ...tJsonTypeInfoResolverTests.JsonTypeInfo.cs | 5 +- .../Serialization/RequiredKeywordTests.cs | 112 +++++- 8 files changed, 516 insertions(+), 13 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs index 514dd8cd5852f..acf62902c450a 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -944,6 +944,11 @@ public sealed partial class JsonPropertyOrderAttribute : System.Text.Json.Serial public JsonPropertyOrderAttribute(int order) { } public int Order { get { throw null; } } } + [System.AttributeUsageAttribute(System.AttributeTargets.Field | System.AttributeTargets.Property, AllowMultiple = false)] + public sealed partial class JsonRequiredAttribute : System.Text.Json.Serialization.JsonAttribute + { + public JsonRequiredAttribute() { } + } [System.AttributeUsageAttribute(System.AttributeTargets.Class, AllowMultiple=true)] public sealed partial class JsonSerializableAttribute : System.Text.Json.Serialization.JsonAttribute { @@ -1152,6 +1157,7 @@ internal JsonPropertyInfo() { } public System.Text.Json.Serialization.JsonConverter? CustomConverter { get { throw null; } set { } } public System.Func? Get { get { throw null; } set { } } public bool IsExtensionData { get { throw null; } set { } } + public bool IsRequired { get { throw null; } set { } } public string Name { get { throw null; } set { } } public System.Text.Json.Serialization.JsonNumberHandling? NumberHandling { get { throw null; } set { } } public System.Text.Json.JsonSerializerOptions Options { get { throw null; } } diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 38d67f0bee544..7d111541bb1de 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -100,6 +100,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs new file mode 100644 index 0000000000000..0c7efa9280080 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Text.Json.Serialization +{ + /// + /// Specifies the property is required when deserializing. + /// + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)] + public sealed class JsonRequiredAttribute : JsonAttribute + { + /// + /// Initializes a new instance of . + /// + public JsonRequiredAttribute() + { + } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 7605421549e75..199b74580d595 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -203,7 +203,18 @@ public bool IsExtensionData private bool _isExtensionDataProperty; - internal bool IsRequired + /// + /// Specifies whether the current property is required during deserialization. + /// + /// + /// The instance has been locked for further modification. + /// + /// + /// For contracts originating from or , + /// the value of this property will be mapped from based on presence of annotation or presence of keyword. + /// If SetsRequiredMembersAttribute is specified on the constructor the keyword will not influence value of this property. + /// + public bool IsRequired { get => _isRequired; set @@ -506,10 +517,8 @@ private bool NumberHandingIsApplicable() private void DetermineIsRequired(MemberInfo memberInfo, bool shouldCheckForRequiredKeyword) { - if (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute()) - { - IsRequired = true; - } + IsRequired = memberInfo.GetCustomAttribute(inherit: false) != null + || (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute()); } internal abstract bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf8JsonWriter writer); diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/JsonSerializerWrapper.Reflection.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/JsonSerializerWrapper.Reflection.cs index a59be83de7e32..24636171cfc7d 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/JsonSerializerWrapper.Reflection.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/JsonSerializerWrapper.Reflection.cs @@ -646,10 +646,9 @@ public static JsonSerializerOptions ResolveOptionsInstanceWithSmallBuffer(JsonSe JsonSerializerOptions smallBufferCopy = new JsonSerializerOptions(options) { - // Copy the resolver explicitly until https://github.com/dotnet/aspnetcore/issues/38720 is resolved. - TypeInfoResolver = options.TypeInfoResolver, DefaultBufferSize = 1, }; + s_smallBufferMap.Add(options, smallBufferCopy); return smallBufferCopy; } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs index cb8c53ae64624..2f6ee7562b094 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs @@ -1690,6 +1690,338 @@ public static void IgnoreNullValues_Deserialization_CanBeInvalidatedByCustomCont Assert.Equal("NULL", value.Value); } + [Fact] + public static void RequiredAttributesGetDetectedAndFailDeserializationWhenValuesNotPresent() + { + JsonTypeInfo typeInfo = JsonSerializerOptions.Default.GetTypeInfo(typeof(ClassWithRequiredCustomAttributes)); + Assert.Equal(3, typeInfo.Properties.Count); + + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.NonRequired), typeInfo.Properties[0].Name); + Assert.False(typeInfo.Properties[0].IsRequired); + + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredA), typeInfo.Properties[1].Name); + Assert.True(typeInfo.Properties[1].IsRequired); + + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredB), typeInfo.Properties[2].Name); + Assert.True(typeInfo.Properties[2].IsRequired); + + ClassWithRequiredCustomAttributes obj = new(); + string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj)); + + var deserialized = JsonSerializer.Deserialize(json); + Assert.Null(deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Null(deserialized.RequiredB); + + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}")); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""")); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredB":"bar"}""")); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredA":null}""")); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + } + + [Fact] + public static void RequiredMemberCanBeModifiedToNonRequired() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributes)) + { + JsonPropertyInfo prop = ti.Properties[1]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredA), prop.Name); + Assert.True(prop.IsRequired); + prop.IsRequired = false; + Assert.False(prop.IsRequired); + } + } + } + } + }; + + ClassWithRequiredCustomAttributes obj = new(); + string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Null(deserialized.RequiredB); + + deserialized = JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredB":"bar"}""", options); + Assert.Equal("foo", deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Equal("bar", deserialized.RequiredB); + + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredA":null}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + } + + [Fact] + public static void NonRequiredMemberCanBeModifiedToRequired() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributes)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.NonRequired), prop.Name); + Assert.False(prop.IsRequired); + prop.IsRequired = true; + Assert.True(prop.IsRequired); + } + } + } + } + }; + + ClassWithRequiredCustomAttributes obj = new(); + string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Null(deserialized.RequiredB); + + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"RequiredA":null}""", options)); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":null,"RequiredA":null}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + } + + [Fact] + public static void RequiredExtensionDataPropertyThrows() + { + ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new(); + Assert.Throws(() => JsonSerializer.Serialize(obj)); + Assert.Throws(() => JsonSerializer.Deserialize("""{"Data":{}}""")); + } + + [Fact] + public static void RequiredExtensionDataPropertyCanBeFixedToNotBeRequiredWithResolver() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndDataExtensionProperty.Data), prop.Name); + Assert.True(prop.IsRequired); + prop.IsRequired = false; + Assert.False(prop.IsRequired); + } + } + } + } + }; + + ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new() + { + Data = new Dictionary() + { + ["foo"] = "bar" + } + }; + + string json = """{"foo":"bar"}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.NotNull(deserialized.Data); + Assert.Equal("bar", ((JsonElement)deserialized.Data["foo"]).GetString()); + } + + [Fact] + public static void RequiredExtensionDataPropertyCanBeFixedToNotBeExtensionDataWithResolver() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndDataExtensionProperty.Data), prop.Name); + Assert.True(prop.IsExtensionData); + prop.IsExtensionData = false; + Assert.False(prop.IsExtensionData); + } + } + } + } + }; + + ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new() + { + Data = new Dictionary() + { + ["foo"] = "bar" + } + }; + + string json = """{"Data":{"foo":"bar"}}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.NotNull(deserialized.Data); + Assert.Equal("bar", ((JsonElement)deserialized.Data["foo"]).GetString()); + Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + } + + [Fact] + public static void RequiredReadOnlyPropertyThrows() + { + ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); + Assert.Throws(() => JsonSerializer.Serialize(obj)); + Assert.Throws(() => JsonSerializer.Deserialize("""{"Data":{}}""")); + } + + [Fact] + public static void RequiredReadOnlyPropertyCanBeFixedToNotBeRequiredWithResolver() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndReadOnlyProperty.SomeProperty), prop.Name); + Assert.True(prop.IsRequired); + Assert.Null(prop.Set); + prop.IsRequired = false; + Assert.False(prop.IsRequired); + } + } + } + } + }; + + ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); + + string json = """{"SomeProperty":"SomePropertyInitialValue"}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + json = """{"SomeProperty":"SomeOtherValue"}"""; + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.Equal("SomePropertyInitialValue", deserialized.SomeProperty); + + json = "{}"; + deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.Equal("SomePropertyInitialValue", deserialized.SomeProperty); + } + + [Fact] + public static void RequiredReadOnlyPropertyCanBeFixedToBeWritableWithResolver() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndReadOnlyProperty.SomeProperty), prop.Name); + Assert.True(prop.IsRequired); + Assert.Null(prop.Set); + prop.Set = (obj, value) => ((ClassWithRequiredCustomAttributeAndReadOnlyProperty)obj).SetSomeProperty((string)value); + Assert.NotNull(prop.Set); + } + } + } + } + }; + + ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); + + string json = """{"SomeProperty":"SomePropertyInitialValue"}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + json = """{"SomeProperty":"SomeOtherValue"}"""; + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.Equal("SomeOtherValue", deserialized.SomeProperty); + + json = "{}"; + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains("SomeProperty", exception.Message); + } + public class ClassWithNullableProperty { private string? _value; @@ -1700,5 +2032,37 @@ public string? Value set => _value = value ?? "NULL"; } } + + public class ClassWithRequiredCustomAttributes + { + [JsonPropertyOrder(0)] + public string NonRequired { get; set; } + + [JsonPropertyOrder(1)] + [JsonRequired] + public string RequiredA { get; set; } + + [JsonPropertyOrder(2)] + [JsonRequired] + public string RequiredB { get; set; } + } + + public class ClassWithRequiredCustomAttributeAndDataExtensionProperty + { + [JsonRequired] + [JsonExtensionData] + public Dictionary? Data { get; set; } + } + + public class ClassWithRequiredCustomAttributeAndReadOnlyProperty + { + [JsonRequired] + public string SomeProperty { get; private set; } = "SomePropertyInitialValue"; + + public void SetSomeProperty(string value) + { + SomeProperty = value; + } + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs index 916001072c705..f3e13660f00c8 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs @@ -430,12 +430,16 @@ private static void TestTypeInfoImmutability(JsonTypeInfo typeInfo) Assert.Null(property.ShouldSerialize); Assert.Null(typeInfo.NumberHandling); + Assert.Throws(() => property.AttributeProvider = property.AttributeProvider); Assert.Throws(() => property.CustomConverter = property.CustomConverter); Assert.Throws(() => property.Name = property.Name); Assert.Throws(() => property.Get = property.Get); Assert.Throws(() => property.Set = property.Set); Assert.Throws(() => property.ShouldSerialize = property.ShouldSerialize); Assert.Throws(() => property.NumberHandling = property.NumberHandling); + Assert.Throws(() => property.Order = property.Order); + Assert.Throws(() => property.IsExtensionData = property.IsExtensionData); + Assert.Throws(() => property.IsRequired = property.IsRequired); } } @@ -943,7 +947,6 @@ public static void JsonConstructorAttributeIsOverriddenAndPropertiesAreSetWhenCr Assert.True(deserialized.E); } - [Theory] [InlineData(typeof(ICollection))] [InlineData(typeof(IList))] diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs index 9517d94c2ce1a..ac61deca7c069 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text.Json.Serialization.Metadata; +using Microsoft.Diagnostics.Runtime.ICorDebug; using Newtonsoft.Json; using Xunit; @@ -72,6 +73,10 @@ public async void ClassWithRequiredKeywordDeserialization(bool ignoreNullValues) IgnoreNullValues = ignoreNullValues }; + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(options), + nameof(PersonWithRequiredMembers.FirstName), + nameof(PersonWithRequiredMembers.LastName)); + var obj = new PersonWithRequiredMembers() { FirstName = "foo", @@ -132,6 +137,12 @@ public async void ClassWithRequiredKeywordAndSmallParametrizedCtorFailsDeseriali IgnoreNullValues = ignoreNullValues }; + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(options), + nameof(PersonWithRequiredMembersAndSmallParametrizedCtor.FirstName), + nameof(PersonWithRequiredMembersAndSmallParametrizedCtor.LastName), + nameof(PersonWithRequiredMembersAndSmallParametrizedCtor.Info1), + nameof(PersonWithRequiredMembersAndSmallParametrizedCtor.Info2)); + var obj = new PersonWithRequiredMembersAndSmallParametrizedCtor("badfoo", "badbar") { // note: these must be set during initialize or otherwise we get compiler errors @@ -209,6 +220,17 @@ public async void ClassWithRequiredKeywordAndLargeParametrizedCtorFailsDeseriali IgnoreNullValues = ignoreNullValues }; + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(options), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.AProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.BProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.CProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.DProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.EProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.FProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.GProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.HProp), + nameof(PersonWithRequiredMembersAndLargeParametrizedCtor.IProp)); + var obj = new PersonWithRequiredMembersAndLargeParametrizedCtor("bada", "badb", "badc", "badd", "bade", "badf", "badg") { // note: these must be set during initialize or otherwise we get compiler errors @@ -314,6 +336,9 @@ public PersonWithRequiredMembersAndLargeParametrizedCtor(string aprop, string bp [Fact] public async void ClassWithRequiredKeywordAndSetsRequiredMembersOnCtorWorks() { + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(JsonSerializerOptions.Default) + /* no required members */); + var obj = new PersonWithRequiredMembersAndSetsRequiredMembers() { FirstName = "foo", @@ -347,6 +372,9 @@ public PersonWithRequiredMembersAndSetsRequiredMembers() [Fact] public async void ClassWithRequiredKeywordSmallParametrizedCtorAndSetsRequiredMembersOnCtorWorks() { + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(JsonSerializerOptions.Default) + /* no required members */); + var obj = new PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers("foo", "bar"); string json = await Serializer.SerializeWrapper(obj); @@ -375,12 +403,15 @@ public PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers(s [Fact] public async void ClassWithRequiredKeywordLargeParametrizedCtorAndSetsRequiredMembersOnCtorWorks() { - var obj = new PersonWithRequiredMembersAndLargeParametrizedCtorndSetsRequiredMembers("a", "b", "c", "d", "e", "f", "g"); + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(JsonSerializerOptions.Default) + /* no required members */); + + var obj = new PersonWithRequiredMembersAndLargeParametrizedCtorAndSetsRequiredMembers("a", "b", "c", "d", "e", "f", "g"); string json = await Serializer.SerializeWrapper(obj); Assert.Equal("""{"A":"a","B":"b","C":"c","D":"d","E":"e","F":"f","G":"g"}""", json); - var deserialized = await Serializer.DeserializeWrapper(json); + var deserialized = await Serializer.DeserializeWrapper(json); Assert.Equal("a", deserialized.A); Assert.Equal("b", deserialized.B); Assert.Equal("c", deserialized.C); @@ -390,7 +421,7 @@ public async void ClassWithRequiredKeywordLargeParametrizedCtorAndSetsRequiredMe Assert.Equal("g", deserialized.G); } - private class PersonWithRequiredMembersAndLargeParametrizedCtorndSetsRequiredMembers + private class PersonWithRequiredMembersAndLargeParametrizedCtorAndSetsRequiredMembers { public required string A { get; set; } public required string B { get; set; } @@ -401,7 +432,7 @@ private class PersonWithRequiredMembersAndLargeParametrizedCtorndSetsRequiredMem public required string G { get; set; } [SetsRequiredMembers] - public PersonWithRequiredMembersAndLargeParametrizedCtorndSetsRequiredMembers(string a, string b, string c, string d, string e, string f, string g) + public PersonWithRequiredMembersAndLargeParametrizedCtorAndSetsRequiredMembers(string a, string b, string c, string d, string e, string f, string g) { A = a; B = b; @@ -414,7 +445,7 @@ public PersonWithRequiredMembersAndLargeParametrizedCtorndSetsRequiredMembers(st } [Fact] - public async void RemovingRequiredPropertiesAllowsDeserialization() + public async void RemovingPropertiesWithRequiredKeywordAllowsDeserialization() { JsonSerializerOptions options = new() { @@ -428,6 +459,7 @@ public async void RemovingRequiredPropertiesAllowsDeserialization() { if (ti.Properties[i].Name == nameof(PersonWithRequiredMembers.FirstName)) { + Assert.True(ti.Properties[i].IsRequired); JsonPropertyInfo property = ti.CreateJsonPropertyInfo(typeof(string), nameof(PersonWithRequiredMembers.FirstName)); property.Get = (obj) => ((PersonWithRequiredMembers)obj).FirstName; property.Set = (obj, val) => ((PersonWithRequiredMembers)obj).FirstName = (string)val; @@ -435,11 +467,52 @@ public async void RemovingRequiredPropertiesAllowsDeserialization() } else if (ti.Properties[i].Name == nameof(PersonWithRequiredMembers.LastName)) { + Assert.True(ti.Properties[i].IsRequired); JsonPropertyInfo property = ti.CreateJsonPropertyInfo(typeof(string), nameof(PersonWithRequiredMembers.LastName)); property.Get = (obj) => ((PersonWithRequiredMembers)obj).LastName; property.Set = (obj, val) => ((PersonWithRequiredMembers)obj).LastName = (string)val; ti.Properties[i] = property; } + else + { + Assert.False(ti.Properties[i].IsRequired); + } + } + } + } + } + }; + + var obj = new PersonWithRequiredMembers() + { + FirstName = "foo", + LastName = "bar" + }; + + string json = await Serializer.SerializeWrapper(obj, options); + Assert.Equal("""{"FirstName":"foo","MiddleName":"","LastName":"bar"}""", json); + + json = """{"LastName":"bar"}"""; + PersonWithRequiredMembers deserialized = await Serializer.DeserializeWrapper(json, options); + Assert.Null(deserialized.FirstName); + Assert.Equal("", deserialized.MiddleName); + Assert.Equal("bar", deserialized.LastName); + } + + [Fact] + public async void ChangingPropertiesWithRequiredKeywordToNotBeRequiredAllowsDeserialization() + { + JsonSerializerOptions options = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver() + { + Modifiers = + { + (ti) => + { + for (int i = 0; i < ti.Properties.Count; i++) + { + ti.Properties[i].IsRequired = false; } } } @@ -517,5 +590,34 @@ private class ClassWithRequiredExtensionDataProperty [JsonExtensionData] public required Dictionary? TestExtensionData { get; set; } } + + private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions options) + { + // For some variations of test (i.e. AsyncStreamWithSmallBuffer) + // we don't use options directly and use copy with changed settings. + // Because of that options.GetTypeInfo might throw even when Serializer.(De)SerializeWrapper was called.. + // We call into Serialize here to ensure that options are locked and options.GetTypeInfo queried. + JsonSerializer.Serialize(default(T), options); + return options.GetTypeInfo(typeof(T)); + } + + private static void AssertJsonTypeInfoHasRequiredProperties(JsonTypeInfo typeInfo, params string[] requiredProperties) + { + HashSet requiredPropertiesSet = new(requiredProperties); + + foreach (var property in typeInfo.Properties) + { + if (requiredPropertiesSet.Remove(property.Name)) + { + Assert.True(property.IsRequired); + } + else + { + Assert.False(property.IsRequired); + } + } + + Assert.Empty(requiredPropertiesSet); + } } } From 7852d75e49953cb845ecea1eb4de4f7dd2a7c554 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Mon, 1 Aug 2022 13:51:50 +0200 Subject: [PATCH 2/4] Re-use existing tests for source gen --- .../MetadataTests/DefaultJsonPropertyInfo.cs | 411 ++++++++++++++++++ ...nTypeInfoResolverTests.JsonPropertyInfo.cs | 364 ---------------- .../Serialization/RequiredKeywordTests.cs | 52 ++- .../System.Text.Json.Tests.csproj | 1 + 4 files changed, 445 insertions(+), 383 deletions(-) create mode 100644 src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs new file mode 100644 index 0000000000000..5befdc7e4671e --- /dev/null +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs @@ -0,0 +1,411 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Text.Json.Nodes; +using System.Text.Json.Nodes.Tests; +using System.Text.Json.Serialization.Metadata; +using System.Text.Json.Tests; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Text.Json.Serialization.Tests +{ + public class DefaultJsonPropertyInfoTests_DefaultJsonTypeInfoResolver : DefaultJsonPropertyInfoTests + { + protected override IJsonTypeInfoResolver CreateResolverWithModifiers(params Action[] modifiers) + { + var resolver = new DefaultJsonTypeInfoResolver(); + + foreach (var modifier in modifiers) + { + resolver.Modifiers.Add(modifier); + } + + return resolver; + } + } + + public class DefaultJsonPropertyInfoTests_SerializerContextNoWrapping : DefaultJsonPropertyInfoTests + { + protected override IJsonTypeInfoResolver CreateResolverWithModifiers(params Action[] modifiers) + { + if (modifiers.Length != 0) + { + throw new SkipTestException($"Testing non wrapped JsonSerializerContext but modifier is provided."); + } + + return Context.Default; + } + } + + public class DefaultJsonPropertyInfoTests_SerializerContextWrapped : DefaultJsonPropertyInfoTests + { + protected override IJsonTypeInfoResolver CreateResolverWithModifiers(params Action[] modifiers) + => new ContextWithModifiers(Context.Default, modifiers); + + private class ContextWithModifiers : IJsonTypeInfoResolver + { + private IJsonTypeInfoResolver _context; + private Action[] _modifiers; + + public ContextWithModifiers(JsonSerializerContext context, Action[] modifiers) + { + _context = context; + _modifiers = modifiers; + } + + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) + { + JsonTypeInfo? typeInfo = _context.GetTypeInfo(type, options); + Assert.NotNull(typeInfo); + + foreach (var modifier in _modifiers) + { + modifier(typeInfo); + } + + return typeInfo; + } + } + } + + public abstract partial class DefaultJsonPropertyInfoTests + { + protected abstract IJsonTypeInfoResolver CreateResolverWithModifiers(params Action[] modifiers); + + private JsonSerializerOptions CreateOptionsWithModifiers(params Action[] modifiers) + => new JsonSerializerOptions() + { + TypeInfoResolver = CreateResolverWithModifiers(modifiers) + }; + + private JsonSerializerOptions CreateOptions() => CreateOptionsWithModifiers(); + + [Fact] + public void RequiredAttributesGetDetectedAndFailDeserializationWhenValuesNotPresent() + { + JsonSerializerOptions options = CreateOptions(); + + JsonTypeInfo? typeInfo = options.GetTypeInfo(typeof(ClassWithRequiredCustomAttributes)); + Assert.NotNull(typeInfo); + + Assert.Equal(3, typeInfo.Properties.Count); + + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.NonRequired), typeInfo.Properties[0].Name); + Assert.False(typeInfo.Properties[0].IsRequired); + + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredA), typeInfo.Properties[1].Name); + Assert.True(typeInfo.Properties[1].IsRequired); + + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredB), typeInfo.Properties[2].Name); + Assert.True(typeInfo.Properties[2].IsRequired); + + ClassWithRequiredCustomAttributes obj = new(); + string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Null(deserialized.RequiredB); + + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredB":"bar"}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredA":null}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + } + + [ConditionalFact] + public void RequiredMemberCanBeModifiedToNonRequired() + { + JsonSerializerOptions options = CreateOptionsWithModifiers(ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributes)) + { + JsonPropertyInfo prop = ti.Properties[1]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredA), prop.Name); + Assert.True(prop.IsRequired); + prop.IsRequired = false; + Assert.False(prop.IsRequired); + } + }); + + ClassWithRequiredCustomAttributes obj = new(); + string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Null(deserialized.RequiredB); + + deserialized = JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredB":"bar"}""", options); + Assert.Equal("foo", deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Equal("bar", deserialized.RequiredB); + + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredA":null}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + } + + [ConditionalFact] + public void NonRequiredMemberCanBeModifiedToRequired() + { + JsonSerializerOptions options = CreateOptionsWithModifiers(ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributes)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributes.NonRequired), prop.Name); + Assert.False(prop.IsRequired); + prop.IsRequired = true; + Assert.True(prop.IsRequired); + } + }); + + ClassWithRequiredCustomAttributes obj = new(); + string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.NonRequired); + Assert.Null(deserialized.RequiredA); + Assert.Null(deserialized.RequiredB); + + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"RequiredA":null}""", options)); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + + exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":null,"RequiredA":null}""", options)); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); + Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); + Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); + } + + [Fact] + public void RequiredExtensionDataPropertyThrows() + { + JsonSerializerOptions options = CreateOptions(); + ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new(); + Assert.Throws(() => JsonSerializer.Serialize(obj, options)); + Assert.Throws(() => JsonSerializer.Deserialize("""{"Data":{}}""", options)); + } + + [ConditionalFact] + public void RequiredExtensionDataPropertyCanBeFixedToNotBeRequiredWithResolver() + { + JsonSerializerOptions options = CreateOptionsWithModifiers(ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndDataExtensionProperty.Data), prop.Name); + Assert.True(prop.IsRequired); + prop.IsRequired = false; + Assert.False(prop.IsRequired); + } + }); + + ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new() + { + Data = new Dictionary() + { + ["foo"] = "bar" + } + }; + + string json = """{"foo":"bar"}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.NotNull(deserialized.Data); + Assert.Equal("bar", ((JsonElement)deserialized.Data["foo"]).GetString()); + } + + [ConditionalFact] + public void RequiredExtensionDataPropertyCanBeFixedToNotBeExtensionDataWithResolver() + { + JsonSerializerOptions options = CreateOptionsWithModifiers(ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndDataExtensionProperty.Data), prop.Name); + Assert.True(prop.IsExtensionData); + prop.IsExtensionData = false; + Assert.False(prop.IsExtensionData); + } + }); + + ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new() + { + Data = new Dictionary() + { + ["foo"] = "bar" + } + }; + + string json = """{"Data":{"foo":"bar"}}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.NotNull(deserialized.Data); + Assert.Equal("bar", ((JsonElement)deserialized.Data["foo"]).GetString()); + Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); + } + + [Fact] + public void RequiredReadOnlyPropertyThrows() + { + JsonSerializerOptions options = CreateOptions(); + ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); + Assert.Throws(() => JsonSerializer.Serialize(obj, options)); + Assert.Throws(() => JsonSerializer.Deserialize("""{"Data":{}}""", options)); + } + + [ConditionalFact] + public void RequiredReadOnlyPropertyCanBeFixedToNotBeRequiredWithResolver() + { + JsonSerializerOptions options = CreateOptionsWithModifiers(ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndReadOnlyProperty.SomeProperty), prop.Name); + Assert.True(prop.IsRequired); + Assert.Null(prop.Set); + prop.IsRequired = false; + Assert.False(prop.IsRequired); + } + }); + + ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); + + string json = """{"SomeProperty":"SomePropertyInitialValue"}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + json = """{"SomeProperty":"SomeOtherValue"}"""; + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.Equal("SomePropertyInitialValue", deserialized.SomeProperty); + + json = "{}"; + deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.Equal("SomePropertyInitialValue", deserialized.SomeProperty); + } + + [ConditionalFact] + public void RequiredReadOnlyPropertyCanBeFixedToBeWritableWithResolver() + { + JsonSerializerOptions options = CreateOptionsWithModifiers(ti => + { + if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty)) + { + JsonPropertyInfo prop = ti.Properties[0]; + Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndReadOnlyProperty.SomeProperty), prop.Name); + Assert.True(prop.IsRequired); + Assert.Null(prop.Set); + prop.Set = (obj, value) => ((ClassWithRequiredCustomAttributeAndReadOnlyProperty)obj).SetSomeProperty((string)value); + Assert.NotNull(prop.Set); + } + }); + + ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); + + string json = """{"SomeProperty":"SomePropertyInitialValue"}"""; + Assert.Equal(json, JsonSerializer.Serialize(obj, options)); + + json = """{"SomeProperty":"SomeOtherValue"}"""; + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.NotNull(deserialized); + Assert.Equal("SomeOtherValue", deserialized.SomeProperty); + + json = "{}"; + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains("SomeProperty", exception.Message); + } + + public class ClassWithRequiredCustomAttributes + { + [JsonPropertyOrder(0)] + public string NonRequired { get; set; } + + [JsonPropertyOrder(1)] + [JsonRequired] + public string RequiredA { get; set; } + + [JsonPropertyOrder(2)] + [JsonRequired] + public string RequiredB { get; set; } + } + + public class ClassWithRequiredCustomAttributeAndDataExtensionProperty + { + [JsonRequired] + [JsonExtensionData] + public Dictionary? Data { get; set; } + } + + public class ClassWithRequiredCustomAttributeAndReadOnlyProperty + { + [JsonRequired] + public string SomeProperty { get; private set; } = "SomePropertyInitialValue"; + + public void SetSomeProperty(string value) + { + SomeProperty = value; + } + } + + [JsonSerializable(typeof(ClassWithRequiredCustomAttributes))] + [JsonSerializable(typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty))] + [JsonSerializable(typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty))] + internal partial class Context : JsonSerializerContext + { + } + } +} diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs index 2f6ee7562b094..cb8c53ae64624 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs @@ -1690,338 +1690,6 @@ public static void IgnoreNullValues_Deserialization_CanBeInvalidatedByCustomCont Assert.Equal("NULL", value.Value); } - [Fact] - public static void RequiredAttributesGetDetectedAndFailDeserializationWhenValuesNotPresent() - { - JsonTypeInfo typeInfo = JsonSerializerOptions.Default.GetTypeInfo(typeof(ClassWithRequiredCustomAttributes)); - Assert.Equal(3, typeInfo.Properties.Count); - - Assert.Equal(nameof(ClassWithRequiredCustomAttributes.NonRequired), typeInfo.Properties[0].Name); - Assert.False(typeInfo.Properties[0].IsRequired); - - Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredA), typeInfo.Properties[1].Name); - Assert.True(typeInfo.Properties[1].IsRequired); - - Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredB), typeInfo.Properties[2].Name); - Assert.True(typeInfo.Properties[2].IsRequired); - - ClassWithRequiredCustomAttributes obj = new(); - string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj)); - - var deserialized = JsonSerializer.Deserialize(json); - Assert.Null(deserialized.NonRequired); - Assert.Null(deserialized.RequiredA); - Assert.Null(deserialized.RequiredB); - - JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}")); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""")); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredB":"bar"}""")); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredA":null}""")); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - } - - [Fact] - public static void RequiredMemberCanBeModifiedToNonRequired() - { - JsonSerializerOptions options = new() - { - TypeInfoResolver = new DefaultJsonTypeInfoResolver() - { - Modifiers = - { - ti => - { - if (ti.Type == typeof(ClassWithRequiredCustomAttributes)) - { - JsonPropertyInfo prop = ti.Properties[1]; - Assert.Equal(nameof(ClassWithRequiredCustomAttributes.RequiredA), prop.Name); - Assert.True(prop.IsRequired); - prop.IsRequired = false; - Assert.False(prop.IsRequired); - } - } - } - } - }; - - ClassWithRequiredCustomAttributes obj = new(); - string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj, options)); - - var deserialized = JsonSerializer.Deserialize(json, options); - Assert.Null(deserialized.NonRequired); - Assert.Null(deserialized.RequiredA); - Assert.Null(deserialized.RequiredB); - - deserialized = JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredB":"bar"}""", options); - Assert.Equal("foo", deserialized.NonRequired); - Assert.Null(deserialized.RequiredA); - Assert.Equal("bar", deserialized.RequiredB); - - JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo", "RequiredA":null}""", options)); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - } - - [Fact] - public static void NonRequiredMemberCanBeModifiedToRequired() - { - JsonSerializerOptions options = new() - { - TypeInfoResolver = new DefaultJsonTypeInfoResolver() - { - Modifiers = - { - ti => - { - if (ti.Type == typeof(ClassWithRequiredCustomAttributes)) - { - JsonPropertyInfo prop = ti.Properties[0]; - Assert.Equal(nameof(ClassWithRequiredCustomAttributes.NonRequired), prop.Name); - Assert.False(prop.IsRequired); - prop.IsRequired = true; - Assert.True(prop.IsRequired); - } - } - } - } - }; - - ClassWithRequiredCustomAttributes obj = new(); - string json = """{"NonRequired":null,"RequiredA":null,"RequiredB":null}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj, options)); - - var deserialized = JsonSerializer.Deserialize(json, options); - Assert.Null(deserialized.NonRequired); - Assert.Null(deserialized.RequiredA); - Assert.Null(deserialized.RequiredB); - - JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":"foo"}""", options)); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"RequiredA":null}""", options)); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - - exception = Assert.Throws(() => JsonSerializer.Deserialize("""{"NonRequired":null,"RequiredA":null}""", options)); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.NonRequired), exception.Message); - Assert.DoesNotContain(nameof(ClassWithRequiredCustomAttributes.RequiredA), exception.Message); - Assert.Contains(nameof(ClassWithRequiredCustomAttributes.RequiredB), exception.Message); - } - - [Fact] - public static void RequiredExtensionDataPropertyThrows() - { - ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new(); - Assert.Throws(() => JsonSerializer.Serialize(obj)); - Assert.Throws(() => JsonSerializer.Deserialize("""{"Data":{}}""")); - } - - [Fact] - public static void RequiredExtensionDataPropertyCanBeFixedToNotBeRequiredWithResolver() - { - JsonSerializerOptions options = new() - { - TypeInfoResolver = new DefaultJsonTypeInfoResolver() - { - Modifiers = - { - ti => - { - if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty)) - { - JsonPropertyInfo prop = ti.Properties[0]; - Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndDataExtensionProperty.Data), prop.Name); - Assert.True(prop.IsRequired); - prop.IsRequired = false; - Assert.False(prop.IsRequired); - } - } - } - } - }; - - ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new() - { - Data = new Dictionary() - { - ["foo"] = "bar" - } - }; - - string json = """{"foo":"bar"}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj, options)); - var deserialized = JsonSerializer.Deserialize(json, options); - Assert.NotNull(deserialized); - Assert.NotNull(deserialized.Data); - Assert.Equal("bar", ((JsonElement)deserialized.Data["foo"]).GetString()); - } - - [Fact] - public static void RequiredExtensionDataPropertyCanBeFixedToNotBeExtensionDataWithResolver() - { - JsonSerializerOptions options = new() - { - TypeInfoResolver = new DefaultJsonTypeInfoResolver() - { - Modifiers = - { - ti => - { - if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty)) - { - JsonPropertyInfo prop = ti.Properties[0]; - Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndDataExtensionProperty.Data), prop.Name); - Assert.True(prop.IsExtensionData); - prop.IsExtensionData = false; - Assert.False(prop.IsExtensionData); - } - } - } - } - }; - - ClassWithRequiredCustomAttributeAndDataExtensionProperty obj = new() - { - Data = new Dictionary() - { - ["foo"] = "bar" - } - }; - - string json = """{"Data":{"foo":"bar"}}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj, options)); - var deserialized = JsonSerializer.Deserialize(json, options); - Assert.NotNull(deserialized); - Assert.NotNull(deserialized.Data); - Assert.Equal("bar", ((JsonElement)deserialized.Data["foo"]).GetString()); - Assert.Throws(() => JsonSerializer.Deserialize("{}", options)); - } - - [Fact] - public static void RequiredReadOnlyPropertyThrows() - { - ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); - Assert.Throws(() => JsonSerializer.Serialize(obj)); - Assert.Throws(() => JsonSerializer.Deserialize("""{"Data":{}}""")); - } - - [Fact] - public static void RequiredReadOnlyPropertyCanBeFixedToNotBeRequiredWithResolver() - { - JsonSerializerOptions options = new() - { - TypeInfoResolver = new DefaultJsonTypeInfoResolver() - { - Modifiers = - { - ti => - { - if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty)) - { - JsonPropertyInfo prop = ti.Properties[0]; - Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndReadOnlyProperty.SomeProperty), prop.Name); - Assert.True(prop.IsRequired); - Assert.Null(prop.Set); - prop.IsRequired = false; - Assert.False(prop.IsRequired); - } - } - } - } - }; - - ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); - - string json = """{"SomeProperty":"SomePropertyInitialValue"}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj, options)); - - json = """{"SomeProperty":"SomeOtherValue"}"""; - var deserialized = JsonSerializer.Deserialize(json, options); - Assert.NotNull(deserialized); - Assert.Equal("SomePropertyInitialValue", deserialized.SomeProperty); - - json = "{}"; - deserialized = JsonSerializer.Deserialize(json, options); - Assert.NotNull(deserialized); - Assert.Equal("SomePropertyInitialValue", deserialized.SomeProperty); - } - - [Fact] - public static void RequiredReadOnlyPropertyCanBeFixedToBeWritableWithResolver() - { - JsonSerializerOptions options = new() - { - TypeInfoResolver = new DefaultJsonTypeInfoResolver() - { - Modifiers = - { - ti => - { - if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty)) - { - JsonPropertyInfo prop = ti.Properties[0]; - Assert.Equal(nameof(ClassWithRequiredCustomAttributeAndReadOnlyProperty.SomeProperty), prop.Name); - Assert.True(prop.IsRequired); - Assert.Null(prop.Set); - prop.Set = (obj, value) => ((ClassWithRequiredCustomAttributeAndReadOnlyProperty)obj).SetSomeProperty((string)value); - Assert.NotNull(prop.Set); - } - } - } - } - }; - - ClassWithRequiredCustomAttributeAndReadOnlyProperty obj = new(); - - string json = """{"SomeProperty":"SomePropertyInitialValue"}"""; - Assert.Equal(json, JsonSerializer.Serialize(obj, options)); - - json = """{"SomeProperty":"SomeOtherValue"}"""; - var deserialized = JsonSerializer.Deserialize(json, options); - Assert.NotNull(deserialized); - Assert.Equal("SomeOtherValue", deserialized.SomeProperty); - - json = "{}"; - JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); - Assert.Contains("SomeProperty", exception.Message); - } - public class ClassWithNullableProperty { private string? _value; @@ -2032,37 +1700,5 @@ public string? Value set => _value = value ?? "NULL"; } } - - public class ClassWithRequiredCustomAttributes - { - [JsonPropertyOrder(0)] - public string NonRequired { get; set; } - - [JsonPropertyOrder(1)] - [JsonRequired] - public string RequiredA { get; set; } - - [JsonPropertyOrder(2)] - [JsonRequired] - public string RequiredB { get; set; } - } - - public class ClassWithRequiredCustomAttributeAndDataExtensionProperty - { - [JsonRequired] - [JsonExtensionData] - public Dictionary? Data { get; set; } - } - - public class ClassWithRequiredCustomAttributeAndReadOnlyProperty - { - [JsonRequired] - public string SomeProperty { get; private set; } = "SomePropertyInitialValue"; - - public void SetSomeProperty(string value) - { - SomeProperty = value; - } - } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs index ac61deca7c069..2a6af7ed539c9 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs @@ -57,7 +57,7 @@ public class RequiredKeywordTests_Node : RequiredKeywordTests public RequiredKeywordTests_Node() : base(JsonSerializerWrapper.NodeSerializer) { } } - public abstract class RequiredKeywordTests : SerializerTests + public abstract partial class RequiredKeywordTests : SerializerTests { public RequiredKeywordTests(JsonSerializerWrapper serializer) : base(serializer) { @@ -333,10 +333,13 @@ public PersonWithRequiredMembersAndLargeParametrizedCtor(string aprop, string bp } } - [Fact] - public async void ClassWithRequiredKeywordAndSetsRequiredMembersOnCtorWorks() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async void ClassWithRequiredKeywordAndSetsRequiredMembersOnCtorWorks(bool useContext) { - AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(JsonSerializerOptions.Default) + JsonSerializerOptions options = useContext ? SetsRequiredMembersTestsContext.Default.Options : JsonSerializerOptions.Default; + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(options) /* no required members */); var obj = new PersonWithRequiredMembersAndSetsRequiredMembers() @@ -345,11 +348,11 @@ public async void ClassWithRequiredKeywordAndSetsRequiredMembersOnCtorWorks() LastName = "bar" }; - string json = await Serializer.SerializeWrapper(obj); + string json = await Serializer.SerializeWrapper(obj, options); Assert.Equal("""{"FirstName":"foo","MiddleName":"","LastName":"bar"}""", json); json = """{"LastName":"bar"}"""; - var deserialized = await Serializer.DeserializeWrapper(json); + var deserialized = await Serializer.DeserializeWrapper(json, options); Assert.Equal("", deserialized.FirstName); Assert.Equal("", deserialized.MiddleName); Assert.Equal("bar", deserialized.LastName); @@ -369,18 +372,21 @@ public PersonWithRequiredMembersAndSetsRequiredMembers() } } - [Fact] - public async void ClassWithRequiredKeywordSmallParametrizedCtorAndSetsRequiredMembersOnCtorWorks() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async void ClassWithRequiredKeywordSmallParametrizedCtorAndSetsRequiredMembersOnCtorWorks(bool useContext) { - AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(JsonSerializerOptions.Default) + JsonSerializerOptions options = useContext ? SetsRequiredMembersTestsContext.Default.Options : JsonSerializerOptions.Default; + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(options) /* no required members */); var obj = new PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers("foo", "bar"); - string json = await Serializer.SerializeWrapper(obj); + string json = await Serializer.SerializeWrapper(obj, options); Assert.Equal("""{"FirstName":"foo","MiddleName":"","LastName":"bar"}""", json); - var deserialized = await Serializer.DeserializeWrapper(json); + var deserialized = await Serializer.DeserializeWrapper(json, options); Assert.Equal("foo", deserialized.FirstName); Assert.Equal("", deserialized.MiddleName); Assert.Equal("bar", deserialized.LastName); @@ -388,9 +394,9 @@ public async void ClassWithRequiredKeywordSmallParametrizedCtorAndSetsRequiredMe private class PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers { - public required string FirstName { get; init; } - public string MiddleName { get; init; } = ""; - public required string LastName { get; init; } + public required string FirstName { get; set; } + public string MiddleName { get; set; } = ""; + public required string LastName { get; set; } [SetsRequiredMembers] public PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers(string firstName, string lastName) @@ -400,18 +406,21 @@ public PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers(s } } - [Fact] - public async void ClassWithRequiredKeywordLargeParametrizedCtorAndSetsRequiredMembersOnCtorWorks() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async void ClassWithRequiredKeywordLargeParametrizedCtorAndSetsRequiredMembersOnCtorWorks(bool useContext) { - AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(JsonSerializerOptions.Default) + JsonSerializerOptions options = useContext ? SetsRequiredMembersTestsContext.Default.Options : JsonSerializerOptions.Default; + AssertJsonTypeInfoHasRequiredProperties(GetTypeInfo(options) /* no required members */); var obj = new PersonWithRequiredMembersAndLargeParametrizedCtorAndSetsRequiredMembers("a", "b", "c", "d", "e", "f", "g"); - string json = await Serializer.SerializeWrapper(obj); + string json = await Serializer.SerializeWrapper(obj, options); Assert.Equal("""{"A":"a","B":"b","C":"c","D":"d","E":"e","F":"f","G":"g"}""", json); - var deserialized = await Serializer.DeserializeWrapper(json); + var deserialized = await Serializer.DeserializeWrapper(json, options); Assert.Equal("a", deserialized.A); Assert.Equal("b", deserialized.B); Assert.Equal("c", deserialized.C); @@ -444,6 +453,11 @@ public PersonWithRequiredMembersAndLargeParametrizedCtorAndSetsRequiredMembers(s } } + [JsonSerializable(typeof(PersonWithRequiredMembersAndSetsRequiredMembers))] + [JsonSerializable(typeof(PersonWithRequiredMembersAndSmallParametrizedCtorAndSetsRequiredMembers))] + [JsonSerializable(typeof(PersonWithRequiredMembersAndLargeParametrizedCtorAndSetsRequiredMembers))] + private partial class SetsRequiredMembersTestsContext : JsonSerializerContext { } + [Fact] public async void RemovingPropertiesWithRequiredKeywordAllowsDeserialization() { diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj index 2bd76ae43071f..7d5176c752b5c 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj @@ -155,6 +155,7 @@ + From caa063cf0671a9cab5e8da8a3f044fabff02538e Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Mon, 1 Aug 2022 16:24:57 +0200 Subject: [PATCH 3/4] Source-gen support and address feedback --- .../gen/JsonSourceGenerator.Emitter.cs | 13 ++++++++++++- .../gen/JsonSourceGenerator.Parser.cs | 14 ++++++++++++-- .../System.Text.Json/gen/PropertyGenerationSpec.cs | 5 +++++ .../Attributes/JsonRequiredAttribute.cs | 3 +++ .../Serialization/Metadata/JsonPropertyInfo.cs | 9 ++++++--- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index b516bd13f51f9..d9698c71ce3f0 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -29,6 +29,7 @@ private sealed partial class Emitter private const string DefaultContextBackingStaticVarName = "s_defaultContext"; internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory"; private const string InfoVarName = "info"; + private const string PropertyInfoVarName = "propertyInfo"; internal const string JsonContextVarName = "jsonContext"; private const string NumberHandlingPropName = "NumberHandling"; private const string ObjectCreatorPropName = "ObjectCreator"; @@ -709,6 +710,7 @@ private static string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenera string memberTypeCompilableName = memberTypeMetadata.TypeRef; string infoVarName = $"{InfoVarName}{i}"; + string propertyInfoVarName = $"{PropertyInfoVarName}{i}"; sb.Append($@" {JsonPropertyInfoValuesTypeRef}<{memberTypeCompilableName}> {infoVarName} = new {JsonPropertyInfoValuesTypeRef}<{memberTypeCompilableName}>() @@ -728,7 +730,16 @@ private static string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenera JsonPropertyName = {jsonPropertyNameValue} }}; - {PropVarName}[{i}] = {JsonMetadataServicesTypeRef}.CreatePropertyInfo<{memberTypeCompilableName}>({OptionsLocalVariableName}, {infoVarName}); + {JsonPropertyInfoTypeRef} {propertyInfoVarName} = {JsonMetadataServicesTypeRef}.CreatePropertyInfo<{memberTypeCompilableName}>({OptionsLocalVariableName}, {infoVarName});"); + + if (memberMetadata.IsRequired) + { + sb.Append($@" + {propertyInfoVarName}.IsRequired = true;"); + } + + sb.Append($@" + {PropVarName}[{i}] = {propertyInfoVarName}; "); } diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs index ca04dff1fc839..b1fe30e7f4ace 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs @@ -40,6 +40,7 @@ private sealed class Parser private const string JsonNumberHandlingAttributeFullName = "System.Text.Json.Serialization.JsonNumberHandlingAttribute"; private const string JsonPropertyNameAttributeFullName = "System.Text.Json.Serialization.JsonPropertyNameAttribute"; private const string JsonPropertyOrderAttributeFullName = "System.Text.Json.Serialization.JsonPropertyOrderAttribute"; + private const string JsonRequiredAttributeFullName = "System.Text.Json.Serialization.JsonRequiredAttribute"; private const string JsonSerializerContextFullName = "System.Text.Json.Serialization.JsonSerializerContext"; private const string JsonSourceGenerationOptionsAttributeFullName = "System.Text.Json.Serialization.JsonSourceGenerationOptionsAttribute"; @@ -1197,7 +1198,8 @@ private PropertyGenerationSpec GetPropertyGenerationSpec( out string? converterInstantiationLogic, out int order, out bool hasFactoryConverter, - out bool isExtensionData); + out bool isExtensionData, + out bool isRequired); ProcessMember( memberInfo, @@ -1229,6 +1231,7 @@ private PropertyGenerationSpec GetPropertyGenerationSpec( IsProperty = memberInfo.MemberType == MemberTypes.Property, IsPublic = isPublic, IsVirtual = isVirtual, + IsRequired = isRequired, JsonPropertyName = jsonPropertyName, RuntimePropertyName = runtimePropertyName, PropertyNameVarName = propertyNameVarName, @@ -1275,7 +1278,8 @@ private void ProcessMemberCustomAttributes( out string? converterInstantiationLogic, out int order, out bool hasFactoryConverter, - out bool isExtensionData) + out bool isExtensionData, + out bool isRequired) { hasJsonInclude = false; jsonPropertyName = null; @@ -1284,6 +1288,7 @@ private void ProcessMemberCustomAttributes( converterInstantiationLogic = null; order = 0; isExtensionData = false; + isRequired = false; bool foundDesignTimeCustomConverter = false; hasFactoryConverter = false; @@ -1350,6 +1355,11 @@ private void ProcessMemberCustomAttributes( isExtensionData = true; } break; + case JsonRequiredAttributeFullName: + { + isRequired = true; + } + break; default: break; } diff --git a/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs b/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs index 62d6ff746c0fb..5417f25b6c649 100644 --- a/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs +++ b/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs @@ -30,6 +30,11 @@ internal sealed class PropertyGenerationSpec public bool IsVirtual { get; init; } + /// + /// The property has JsonRequiredAttribute. + /// + public bool IsRequired { get; init; } + /// /// The property name specified via JsonPropertyNameAttribute, if available. /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs index 0c7efa9280080..4152b0fcee0dc 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs @@ -12,6 +12,9 @@ public sealed class JsonRequiredAttribute : JsonAttribute /// /// Initializes a new instance of . /// + /// + /// token in JSON will not trigger a validation error. + /// public JsonRequiredAttribute() { } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 199b74580d595..9a49f91cff5ac 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -204,15 +204,18 @@ public bool IsExtensionData private bool _isExtensionDataProperty; /// - /// Specifies whether the current property is required during deserialization. + /// Specifies whether the current property is required for deserialization to be successful. /// /// /// The instance has been locked for further modification. /// /// /// For contracts originating from or , - /// the value of this property will be mapped from based on presence of annotation or presence of keyword. - /// If SetsRequiredMembersAttribute is specified on the constructor the keyword will not influence value of this property. + /// the value of this property will be mapped from annotations. + /// + /// For contracts using , properties using the keyword + /// will also map to this setting, unless deserialization uses a SetsRequiredMembersAttribute on a constructor that populates all required properties. + /// keyword is currently not supported in contracts. /// public bool IsRequired { From 44fe21b5dc0337c4b33e312c50d8b050291c40b3 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Tue, 2 Aug 2022 11:39:24 +0200 Subject: [PATCH 4/4] polymorphic cases testing, address other feedback --- .../Attributes/JsonRequiredAttribute.cs | 16 +- .../MetadataTests/DefaultJsonPropertyInfo.cs | 183 ++++++++++++++++++ ...tJsonTypeInfoResolverTests.JsonTypeInfo.cs | 22 ++- .../Serialization/RequiredKeywordTests.cs | 32 +++ 4 files changed, 236 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs index 4152b0fcee0dc..6ae67e650c9a7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs @@ -1,22 +1,24 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text.Json.Serialization.Metadata; + namespace System.Text.Json.Serialization { /// - /// Specifies the property is required when deserializing. + /// Indicates that the annotated member must bind to a JSON property on deserialization. /// + /// + /// token in JSON will not trigger a validation error. + /// For contracts originating from or , + /// this attribute will be mapped to . + /// [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)] public sealed class JsonRequiredAttribute : JsonAttribute { /// /// Initializes a new instance of . /// - /// - /// token in JSON will not trigger a validation error. - /// - public JsonRequiredAttribute() - { - } + public JsonRequiredAttribute() { } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs index 5befdc7e4671e..8b2dc8102b637 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs @@ -369,6 +369,147 @@ public void RequiredReadOnlyPropertyCanBeFixedToBeWritableWithResolver() Assert.Contains("SomeProperty", exception.Message); } + [Fact] + public void Polymorphic_BaseClassWithoutRequiredDerivedClassWithRequiredProperty() + { + JsonSerializerOptions options = CreateOptions(); + + BaseClassWithoutRequiredProperties obj = new DerivedClassWithRequiredProperty() + { + NonRequiredProperty = "non-required", + RequiredProperty = "required", + }; + + string json = JsonSerializer.Serialize(obj, options); + Assert.Equal("""{"$type":"derived","RequiredProperty":"required","NonRequiredProperty":"non-required"}""", json); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Equal(obj.NonRequiredProperty, deserialized.NonRequiredProperty); + Assert.Equal(((DerivedClassWithRequiredProperty)obj).RequiredProperty, ((DerivedClassWithRequiredProperty)deserialized).RequiredProperty); + + json = """{"$type":"derived","NonRequiredProperty":"non-required"}"""; + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains(nameof(DerivedClassWithRequiredProperty.RequiredProperty), exception.Message); + Assert.DoesNotContain(nameof(BaseClassWithoutRequiredProperties.NonRequiredProperty), exception.Message); + + json = """{"NonRequiredProperty":"non-required"}"""; + deserialized = JsonSerializer.Deserialize(json, options); + Assert.Equal(obj.NonRequiredProperty, deserialized.NonRequiredProperty); + Assert.IsNotType(deserialized); + } + + [Fact] + public void Polymorphic_BaseClassWithRequiredDerivedClassWithoutRequiredProperty() + { + JsonSerializerOptions options = CreateOptions(); + + BaseClassWithRequiredProperties obj = new DerivedClassWithoutRequiredProperty() + { + NonRequiredProperty = "non-required", + RequiredProperty = "required", + }; + + string json = JsonSerializer.Serialize(obj, options); + Assert.Equal("""{"$type":"derived","NonRequiredProperty":"non-required","RequiredProperty":"required"}""", json); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Equal(obj.RequiredProperty, deserialized.RequiredProperty); + Assert.Equal(((DerivedClassWithoutRequiredProperty)obj).NonRequiredProperty, ((DerivedClassWithoutRequiredProperty)deserialized).NonRequiredProperty); + + json = """{"$type":"derived","NonRequiredProperty":"non-required"}"""; + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains(nameof(BaseClassWithRequiredProperties.RequiredProperty), exception.Message); + Assert.DoesNotContain(nameof(DerivedClassWithoutRequiredProperty.NonRequiredProperty), exception.Message); + + json = """{"RequiredProperty":"required"}"""; + deserialized = JsonSerializer.Deserialize(json, options); + Assert.Equal(obj.RequiredProperty, deserialized.RequiredProperty); + Assert.IsNotType(deserialized); + + json = "{}"; + exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains(nameof(BaseClassWithRequiredProperties.RequiredProperty), exception.Message); + } + + [Fact] + public void Polymorphic_BaseClassWithRequiredDerivedHidingRequiredPropertyWithNonRequired() + { + JsonSerializerOptions options = CreateOptions(); + + JsonTypeInfo? typeInfo = options.GetTypeInfo(typeof(DerivedClassHidingRequiredPropertyWithNonRequired)); + Assert.NotNull(typeInfo); + Assert.Equal(1, typeInfo.Properties.Count); + + JsonPropertyInfo propertyInfo = typeInfo.Properties[0]; + Assert.Equal(nameof(DerivedClassHidingRequiredPropertyWithNonRequired.RequiredProperty), propertyInfo.Name); + Assert.False(propertyInfo.IsRequired); + + typeInfo = options.GetTypeInfo(typeof(BaseClassWithRequiredProperties)); + Assert.NotNull(typeInfo); + Assert.Equal(1, typeInfo.Properties.Count); + + propertyInfo = typeInfo.Properties[0]; + Assert.Equal(nameof(BaseClassWithRequiredProperties.RequiredProperty), propertyInfo.Name); + Assert.True(propertyInfo.IsRequired); + + BaseClassWithRequiredProperties obj = new DerivedClassHidingRequiredPropertyWithNonRequired() + { + RequiredProperty = "hidden-with-non-required", + }; + + obj.RequiredProperty = "base-required"; + + string json = JsonSerializer.Serialize(obj, options); + Assert.Equal("""{"$type":"derived-hiding","RequiredProperty":"hidden-with-non-required"}""", json); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.RequiredProperty); + Assert.Equal(((DerivedClassHidingRequiredPropertyWithNonRequired)obj).RequiredProperty, ((DerivedClassHidingRequiredPropertyWithNonRequired)deserialized).RequiredProperty); + + json = """{"$type":"derived-hiding"}"""; + deserialized = JsonSerializer.Deserialize(json, options); + Assert.Null(deserialized.RequiredProperty); + Assert.Null(((DerivedClassHidingRequiredPropertyWithNonRequired)deserialized).RequiredProperty); + + json = """{"RequiredProperty":"base-required"}"""; + deserialized = JsonSerializer.Deserialize(json, options); + Assert.Equal(obj.RequiredProperty, deserialized.RequiredProperty); + Assert.IsNotType(deserialized); + + json = "{}"; + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains(nameof(BaseClassWithRequiredProperties.RequiredProperty), exception.Message); + } + + [Fact] + public void StructWithRequiredPropertiesDoesWorkCorrectlyWithJsonRequiredCustomAttribute() + { + JsonSerializerOptions options = CreateOptions(); + + StructWithRequiredProperties obj = new() + { + PropertyA = 123, + PropertyB = 456, + }; + + string json = JsonSerializer.Serialize(obj, options); + Assert.Equal("""{"PropertyA":123,"PropertyB":456}""", json); + + var deserialized = JsonSerializer.Deserialize(json, options); + Assert.Equal(obj.PropertyA, deserialized.PropertyA); + Assert.Equal(obj.PropertyB, deserialized.PropertyB); + + json = """{"PropertyA":123}"""; + JsonException exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.DoesNotContain(nameof(StructWithRequiredProperties.PropertyA), exception.Message); + Assert.Contains(nameof(StructWithRequiredProperties.PropertyB), exception.Message); + + json = "{}"; + exception = Assert.Throws(() => JsonSerializer.Deserialize(json, options)); + Assert.Contains(nameof(StructWithRequiredProperties.PropertyA), exception.Message); + Assert.Contains(nameof(StructWithRequiredProperties.PropertyB), exception.Message); + } + public class ClassWithRequiredCustomAttributes { [JsonPropertyOrder(0)] @@ -401,9 +542,51 @@ public void SetSomeProperty(string value) } } + [JsonDerivedType(typeof(DerivedClassWithRequiredProperty), "derived")] + public class BaseClassWithoutRequiredProperties + { + public string NonRequiredProperty { get; set; } + } + + public class DerivedClassWithRequiredProperty : BaseClassWithoutRequiredProperties + { + [JsonRequired] + public string RequiredProperty { get; set; } + } + + [JsonDerivedType(typeof(DerivedClassWithoutRequiredProperty), "derived")] + [JsonDerivedType(typeof(DerivedClassHidingRequiredPropertyWithNonRequired), "derived-hiding")] + public class BaseClassWithRequiredProperties + { + [JsonRequired] + public string RequiredProperty { get; set; } + } + + public class DerivedClassWithoutRequiredProperty : BaseClassWithRequiredProperties + { + public string NonRequiredProperty { get; set; } + } + + public class DerivedClassHidingRequiredPropertyWithNonRequired : BaseClassWithRequiredProperties + { + public new string RequiredProperty { get; set; } + } + + public struct StructWithRequiredProperties + { + [JsonRequired] + public int PropertyA { get; set; } + + [JsonRequired] + public int PropertyB { get; set; } + } + [JsonSerializable(typeof(ClassWithRequiredCustomAttributes))] [JsonSerializable(typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty))] [JsonSerializable(typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty))] + [JsonSerializable(typeof(BaseClassWithoutRequiredProperties))] + [JsonSerializable(typeof(BaseClassWithRequiredProperties))] + [JsonSerializable(typeof(StructWithRequiredProperties))] internal partial class Context : JsonSerializerContext { } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs index f3e13660f00c8..695980b45232c 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs @@ -430,16 +430,18 @@ private static void TestTypeInfoImmutability(JsonTypeInfo typeInfo) Assert.Null(property.ShouldSerialize); Assert.Null(typeInfo.NumberHandling); - Assert.Throws(() => property.AttributeProvider = property.AttributeProvider); - Assert.Throws(() => property.CustomConverter = property.CustomConverter); - Assert.Throws(() => property.Name = property.Name); - Assert.Throws(() => property.Get = property.Get); - Assert.Throws(() => property.Set = property.Set); - Assert.Throws(() => property.ShouldSerialize = property.ShouldSerialize); - Assert.Throws(() => property.NumberHandling = property.NumberHandling); - Assert.Throws(() => property.Order = property.Order); - Assert.Throws(() => property.IsExtensionData = property.IsExtensionData); - Assert.Throws(() => property.IsRequired = property.IsRequired); + foreach (PropertyInfo propertyInfo in typeof(JsonPropertyInfo).GetProperties(BindingFlags.Instance | BindingFlags.Public)) + { + // We don't have any set only properties, if this ever changes we will need to update this code + if (propertyInfo.GetSetMethod() == null) + { + continue; + } + + TargetInvocationException exception = Assert.Throws(() => propertyInfo.SetValue(property, propertyInfo.GetValue(property))); + Assert.NotNull(exception.InnerException); + Assert.IsType(exception.InnerException); + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs index 2a6af7ed539c9..82c776cebf151 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/RequiredKeywordTests.cs @@ -605,6 +605,38 @@ private class ClassWithRequiredExtensionDataProperty public required Dictionary? TestExtensionData { get; set; } } + [Fact] + public async void RequiredKeywordAndJsonRequiredCustomAttributeWorkCorrectlyTogether() + { + JsonSerializerOptions options = JsonSerializerOptions.Default; + JsonTypeInfo typeInfo = GetTypeInfo(options); + AssertJsonTypeInfoHasRequiredProperties(typeInfo, + nameof(ClassWithRequiredKeywordAndJsonRequiredCustomAttribute.SomeProperty)); + + ClassWithRequiredKeywordAndJsonRequiredCustomAttribute obj = new() + { + SomeProperty = "foo" + }; + + string json = await Serializer.SerializeWrapper(obj, options); + Assert.Equal("""{"SomeProperty":"foo"}""", json); + + var deserialized = await Serializer.DeserializeWrapper(json, options); + Assert.Equal(obj.SomeProperty, deserialized.SomeProperty); + + json = "{}"; + JsonException exception = await Assert.ThrowsAsync( + async () => await Serializer.DeserializeWrapper(json, options)); + + Assert.Contains(nameof(ClassWithRequiredKeywordAndJsonRequiredCustomAttribute.SomeProperty), exception.Message); + } + + private class ClassWithRequiredKeywordAndJsonRequiredCustomAttribute + { + [JsonRequired] + public required string SomeProperty { get; set; } + } + private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions options) { // For some variations of test (i.e. AsyncStreamWithSmallBuffer)