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

Improve error message in scenaria where JsonIgnore is combined with required members. #92330

Open
grosch-intl opened this issue Sep 20, 2023 · 11 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@grosch-intl
Copy link

Description

It's often necessary to use the required flag on a property that you don't want serialized via JSON. Adding the JsonIgnore attribute to such a property results in an InvalidOperationException.

Reproduction Steps

void Main() {
	var dto = new DTO {
		NotRequiredNumber = 1,
		Number = 2
	};
	
	JsonSerializer.Serialize<DTO>(dto);
}

class DTO {
	[JsonIgnore]
	public required int Number { get; init; }
	public int NotRequiredNumber { get; init; }
}

Expected behavior

Should be able to have the JSON generated without the ignored property.

Actual behavior

JsonPropertyInfo 'Number' defined in type 'UserQuery+DTO' is marked required but does not specify a setter.

Regression?

No

Known Workarounds

No response

Configuration

  • .NET 7
  • Windows 10
  • x64
  • Not specific to this configuration.

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 20, 2023
@ghost
Copy link

ghost commented Sep 20, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

It's often necessary to use the required flag on a property that you don't want serialized via JSON. Adding the JsonIgnore attribute to such a property results in an InvalidOperationException.

Reproduction Steps

void Main() {
	var dto = new DTO {
		NotRequiredNumber = 1,
		Number = 2
	};
	
	JsonSerializer.Serialize<DTO>(dto);
}

class DTO {
	[JsonIgnore]
	public required int Number { get; init; }
	public int NotRequiredNumber { get; init; }
}

Expected behavior

Should be able to have the JSON generated without the ignored property.

Actual behavior

JsonPropertyInfo 'Number' defined in type 'UserQuery+DTO' is marked required but does not specify a setter.

Regression?

No

Known Workarounds

No response

Configuration

  • .NET 7
  • Windows 10
  • x64
  • Not specific to this configuration.

Other information

No response

Author: grosch-intl
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@gregsdennis
Copy link
Contributor

To be clear, are you only asking for this to be supported during serialization and not during deserialization?

@eiriktsarpalis
Copy link
Member

are you only asking for this to be supported during serialization and not during deserialization?

Annotating a property with [JsonIgnore] means ignoring in both serialization and deserialization. As such the JsonIgnore annotation violates the contract of the required keyword, so failing in that case is in my opinion by-design behavior.

What could be improved is the error message though, we should add better testing validating how the two features interact, specifically when the new ignore conditions are implemented in #66490.

@eiriktsarpalis eiriktsarpalis changed the title [System.Text.Json.Serialization.JsonIgnoreAttribute] with required property causes failure. Improve error message in scenaria where JsonIgnore is combined with required members. Sep 20, 2023
@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Sep 20, 2023
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Sep 20, 2023
@grosch-intl
Copy link
Author

To be clear, are you only asking for this to be supported during serialization and not during deserialization?

That's correct, just during deserialization.

@gregsdennis
Copy link
Contributor

During deserialization (going from JSON to C#)? (Your code shows serialization.)

I think you have a DTO design flaw then. You're saying that you want to ignore Number in the JSON but that it's required to make a C# object. I think this is just an invalid arrangement.

Why do you want Number to be required when creating this DTO from C#, but you don't want it populated when receiving it in JSON? That seems inconsistent.

I would split the DTO into two models: a domain model for populating in C# with required and a true DTO which is used solely for JSON serialization. Then you'll just need a mapper to go between them.

@grosch-intl
Copy link
Author

Sorry @gregsdennis, I meant serialization as you assumed. While you're technically 100% correct, I was hoping to not have to have another object. This is already a temporary object that's populated via AutoMapper and ProjectTo. While I don't want the caller to get the value, I do need it for the next query that's done in the same method.

I was hoping to avoid having to do yet another mapping to strip out that one property.

Think of something like this obviously non-compiling code. Need the internal ID for the next query but don't want to send it back.

var items = await _context.SomeTable.ProjectTo<SomeObj>(...).ToArrayAsync();
var ids = items.Select(x => x.InternalIdentifier).ToArrayAsync();
var otherStuff = await _context.SomeOtherTable
    .Where(x => ids.Contains(x.SomeTableIdentifier))
    .ToArrayAsync();

var mapping = otherStuff
    .ToLookup(x => x.InternalIdentifier)
    .ToDictionary(x => x.Key, x => x.Select(...));

foreach (var item in items) {
    item.Stuff = mapping[item.InternalIdentifier];

@gregsdennis
Copy link
Contributor

Sure, do what makes sense for you.

But I agree that this is by-design behavior.

@AArnott
Copy link
Contributor

AArnott commented Sep 26, 2023

I hit this. In my case, I have a custom deserializer, but wanted to utilize the JsonSerializer for serialization. That's why it made sense for [JsonIgnore] and required to appear on the same property. But it rejects serialization even though it could do that.

@TonyValenti
Copy link

@AArnott - How did you work around this?

@AArnott
Copy link
Contributor

AArnott commented Jan 28, 2025

@TonyValenti I don't recall, sorry. But I can only imagine I avoided System.Text.Json even for serialization since it didn't work. I probably would have switched to MessagePack, since that's my go-to for serialization anyway as it's faster and more compact.
Shameless plug: I have my own messagepack serializer library that is NativeAOT-ready now: Nerdbank.MessagePack.

@TonyValenti
Copy link

As a work around I set the property via a constructor parameter, removed required, and marked it as JsonIgnore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

5 participants