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

JSON support required properties #73063

Merged
merged 4 commits into from
Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// Specifies the property is required when deserializing.
/// Indicates that the annotated member must bind to a JSON property on deserialization.
/// </summary>
krwq marked this conversation as resolved.
Show resolved Hide resolved
/// <remarks>
/// <see langword="null"/> token in JSON will not trigger a validation error.
/// For contracts originating from <see cref="DefaultJsonTypeInfoResolver"/> or <see cref="JsonSerializerContext"/>,
/// this attribute will be mapped to <see cref="JsonPropertyInfo.IsRequired"/>.
/// </remarks>
[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)]
krwq marked this conversation as resolved.
Show resolved Hide resolved
public sealed class JsonRequiredAttribute : JsonAttribute
{
/// <summary>
/// Initializes a new instance of <see cref="JsonRequiredAttribute"/>.
/// </summary>
/// <remarks>
/// <see langword="null"/> token in JSON will not trigger a validation error.
/// </remarks>
public JsonRequiredAttribute()
{
}
public JsonRequiredAttribute() { }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the default constructor be removed here? Or is it required by ApiCompat?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not, I followed existing pattern with JsonInclude. We can put better xml doc comment I guess

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<BaseClassWithoutRequiredProperties>(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<JsonException>(() => JsonSerializer.Deserialize<BaseClassWithoutRequiredProperties>(json, options));
Assert.Contains(nameof(DerivedClassWithRequiredProperty.RequiredProperty), exception.Message);
Assert.DoesNotContain(nameof(BaseClassWithoutRequiredProperties.NonRequiredProperty), exception.Message);

json = """{"NonRequiredProperty":"non-required"}""";
deserialized = JsonSerializer.Deserialize<BaseClassWithoutRequiredProperties>(json, options);
Assert.Equal(obj.NonRequiredProperty, deserialized.NonRequiredProperty);
Assert.IsNotType<DerivedClassWithRequiredProperty>(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<BaseClassWithRequiredProperties>(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<JsonException>(() => JsonSerializer.Deserialize<BaseClassWithRequiredProperties>(json, options));
Assert.Contains(nameof(BaseClassWithRequiredProperties.RequiredProperty), exception.Message);
Assert.DoesNotContain(nameof(DerivedClassWithoutRequiredProperty.NonRequiredProperty), exception.Message);

json = """{"RequiredProperty":"required"}""";
deserialized = JsonSerializer.Deserialize<BaseClassWithRequiredProperties>(json, options);
Assert.Equal(obj.RequiredProperty, deserialized.RequiredProperty);
Assert.IsNotType<DerivedClassWithoutRequiredProperty>(deserialized);

json = "{}";
exception = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<BaseClassWithRequiredProperties>(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<BaseClassWithRequiredProperties>(json, options);
Assert.Null(deserialized.RequiredProperty);
Assert.Equal(((DerivedClassHidingRequiredPropertyWithNonRequired)obj).RequiredProperty, ((DerivedClassHidingRequiredPropertyWithNonRequired)deserialized).RequiredProperty);

json = """{"$type":"derived-hiding"}""";
deserialized = JsonSerializer.Deserialize<BaseClassWithRequiredProperties>(json, options);
Assert.Null(deserialized.RequiredProperty);
Assert.Null(((DerivedClassHidingRequiredPropertyWithNonRequired)deserialized).RequiredProperty);

json = """{"RequiredProperty":"base-required"}""";
deserialized = JsonSerializer.Deserialize<BaseClassWithRequiredProperties>(json, options);
Assert.Equal(obj.RequiredProperty, deserialized.RequiredProperty);
Assert.IsNotType<DerivedClassHidingRequiredPropertyWithNonRequired>(deserialized);

json = "{}";
JsonException exception = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<BaseClassWithRequiredProperties>(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<StructWithRequiredProperties>(json, options);
Assert.Equal(obj.PropertyA, deserialized.PropertyA);
Assert.Equal(obj.PropertyB, deserialized.PropertyB);

json = """{"PropertyA":123}""";
JsonException exception = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<StructWithRequiredProperties>(json, options));
Assert.DoesNotContain(nameof(StructWithRequiredProperties.PropertyA), exception.Message);
Assert.Contains(nameof(StructWithRequiredProperties.PropertyB), exception.Message);

json = "{}";
exception = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<StructWithRequiredProperties>(json, options));
Assert.Contains(nameof(StructWithRequiredProperties.PropertyA), exception.Message);
Assert.Contains(nameof(StructWithRequiredProperties.PropertyB), exception.Message);
}

public class ClassWithRequiredCustomAttributes
{
[JsonPropertyOrder(0)]
Expand Down Expand Up @@ -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
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,16 +430,18 @@ private static void TestTypeInfoImmutability<T>(JsonTypeInfo<T> typeInfo)
Assert.Null(property.ShouldSerialize);
Assert.Null(typeInfo.NumberHandling);

Assert.Throws<InvalidOperationException>(() => property.AttributeProvider = property.AttributeProvider);
Assert.Throws<InvalidOperationException>(() => property.CustomConverter = property.CustomConverter);
Assert.Throws<InvalidOperationException>(() => property.Name = property.Name);
Assert.Throws<InvalidOperationException>(() => property.Get = property.Get);
Assert.Throws<InvalidOperationException>(() => property.Set = property.Set);
Assert.Throws<InvalidOperationException>(() => property.ShouldSerialize = property.ShouldSerialize);
Assert.Throws<InvalidOperationException>(() => property.NumberHandling = property.NumberHandling);
Assert.Throws<InvalidOperationException>(() => property.Order = property.Order);
Assert.Throws<InvalidOperationException>(() => property.IsExtensionData = property.IsExtensionData);
Assert.Throws<InvalidOperationException>(() => 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<TargetInvocationException>(() => propertyInfo.SetValue(property, propertyInfo.GetValue(property)));
Assert.NotNull(exception.InnerException);
Assert.IsType<InvalidOperationException>(exception.InnerException);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,38 @@ private class ClassWithRequiredExtensionDataProperty
public required Dictionary<string, JsonElement>? TestExtensionData { get; set; }
}

[Fact]
public async void RequiredKeywordAndJsonRequiredCustomAttributeWorkCorrectlyTogether()
{
JsonSerializerOptions options = JsonSerializerOptions.Default;
JsonTypeInfo typeInfo = GetTypeInfo<ClassWithRequiredKeywordAndJsonRequiredCustomAttribute>(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<ClassWithRequiredKeywordAndJsonRequiredCustomAttribute>(json, options);
Assert.Equal(obj.SomeProperty, deserialized.SomeProperty);

json = "{}";
JsonException exception = await Assert.ThrowsAsync<JsonException>(
async () => await Serializer.DeserializeWrapper<ClassWithRequiredKeywordAndJsonRequiredCustomAttribute>(json, options));

Assert.Contains(nameof(ClassWithRequiredKeywordAndJsonRequiredCustomAttribute.SomeProperty), exception.Message);
}

private class ClassWithRequiredKeywordAndJsonRequiredCustomAttribute
{
[JsonRequired]
public required string SomeProperty { get; set; }
}

private static JsonTypeInfo GetTypeInfo<T>(JsonSerializerOptions options)
{
// For some variations of test (i.e. AsyncStreamWithSmallBuffer)
Expand Down