Skip to content

Commit 53bda4d

Browse files
committed
Issue 104700: Clarify error message for collection property with no getter
- Added verification to distinguish between different error types. - Renamed existing test to more accurately reflect that it tests required properties without a setter. - Added a new test to ensure that a collection property without a getter throws an InvalidOperationException.
1 parent e733c2f commit 53bda4d

File tree

4 files changed

+53
-14
lines changed

4 files changed

+53
-14
lines changed

src/libraries/System.Text.Json/src/Resources/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,4 +758,7 @@
758758
<data name="JsonSchemaExporter_DepthTooLarge" xml:space="preserve">
759759
<value>The depth of the generated JSON schema exceeds the JsonSerializerOptions.MaxDepth setting.</value>
760760
</data>
761+
<data name="JsonPropertyRequiredAndCollectionPropertyMissingGetter" xml:space="preserve">
762+
<value>JsonPropertyInfo '{0}' defined in type '{1}' is marked required but is a collection property and does not specify a getter.</value>
763+
</data>
761764
</root>

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -448,19 +448,7 @@ internal void Configure()
448448

449449
if (IsRequired)
450450
{
451-
if (!CanDeserialize &&
452-
!(AssociatedParameter?.IsRequiredParameter is true &&
453-
Options.RespectRequiredConstructorParameters))
454-
{
455-
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndNotDeserializable(this);
456-
}
457-
458-
if (IsExtensionData)
459-
{
460-
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndExtensionData(this);
461-
}
462-
463-
Debug.Assert(!IgnoreNullTokensOnRead);
451+
EnsureRequiredPropertyIsDeserializable();
464452
}
465453

466454
IsConfigured = true;
@@ -472,6 +460,31 @@ internal void Configure()
472460
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
473461
internal abstract void DetermineReflectionPropertyAccessors(MemberInfo memberInfo, bool useNonPublicAccessors);
474462

463+
private void EnsureRequiredPropertyIsDeserializable()
464+
{
465+
if (!CanDeserialize)
466+
{
467+
bool isCollectionProperty = (EffectiveConverter.ConverterStrategy & (ConverterStrategy.Enumerable | ConverterStrategy.Dictionary)) != 0;
468+
bool isRequiredParameterRespected = AssociatedParameter?.IsRequiredParameter is true && Options.RespectRequiredConstructorParameters;
469+
470+
if (isCollectionProperty)
471+
{
472+
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndCollectionPropertyMissingGetter(this);
473+
}
474+
else if (!isRequiredParameterRespected)
475+
{
476+
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndNotDeserializable(this);
477+
}
478+
}
479+
480+
if (IsExtensionData)
481+
{
482+
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndExtensionData(this);
483+
}
484+
485+
Debug.Assert(!IgnoreNullTokensOnRead);
486+
}
487+
475488
private void CacheNameAsUtf8BytesAndEscapedNameSection()
476489
{
477490
Debug.Assert(Name != null);

src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,12 @@ public static void ThrowInvalidOperationException_SerializerPropertyNameNull(Jso
275275
throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNameNull, jsonPropertyInfo.DeclaringType, jsonPropertyInfo.MemberName));
276276
}
277277

278+
[DoesNotReturn]
279+
public static void ThrowInvalidOperationException_JsonPropertyRequiredAndCollectionPropertyMissingGetter(JsonPropertyInfo jsonPropertyInfo)
280+
{
281+
throw new InvalidOperationException(SR.Format(SR.JsonPropertyRequiredAndCollectionPropertyMissingGetter, jsonPropertyInfo.Name, jsonPropertyInfo.DeclaringType));
282+
}
283+
278284
[DoesNotReturn]
279285
public static void ThrowInvalidOperationException_JsonPropertyRequiredAndNotDeserializable(JsonPropertyInfo jsonPropertyInfo)
280286
{

src/libraries/System.Text.Json/tests/Common/RequiredKeywordTests.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ public async Task ChangingPropertiesWithRequiredKeywordToNotBeRequiredAllowsDese
564564
}
565565

566566
[Fact]
567-
public async Task RequiredNonDeserializablePropertyThrows()
567+
public async Task RequiredPropertyWithoutSetterThrows()
568568
{
569569
JsonSerializerOptions options = Serializer.GetDefaultOptionsWithMetadataModifier(static ti =>
570570
{
@@ -610,6 +610,23 @@ public class ClassWithRequiredExtensionDataProperty
610610
public required Dictionary<string, JsonElement>? TestExtensionData { get; set; }
611611
}
612612

613+
[Fact]
614+
public async Task RequiredCollectionPropertyWithoutGetterThrows()
615+
{
616+
string json = """{"Foo":"foo","Bar":"bar"}""";
617+
InvalidOperationException exception = await Assert.ThrowsAsync<InvalidOperationException>(
618+
async () => await Serializer.DeserializeWrapper<ClassWithRequiredCollectionPropertyWithoutGetter>(json));
619+
Assert.Contains(nameof(ClassWithRequiredCollectionPropertyWithoutGetter.TestCollectionPropertyWithoutGetter), exception.Message);
620+
}
621+
622+
public class ClassWithRequiredCollectionPropertyWithoutGetter
623+
{
624+
public required Dictionary<string, JsonElement>? TestCollectionPropertyWithoutGetter
625+
{
626+
set => TestCollectionPropertyWithoutGetter = value;
627+
}
628+
}
629+
613630
[Fact]
614631
public async Task RequiredKeywordAndJsonRequiredCustomAttributeWorkCorrectlyTogether()
615632
{

0 commit comments

Comments
 (0)