Skip to content

Add JsonIgnore attribute support to Minimal API validation generator #63075

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

Merged
merged 5 commits into from
Aug 4, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 3, 2025

This PR fixes an issue where Minimal API validation would throw a "Maximum validation depth exceeded" exception when processing types like JsonPatchDocument that contain properties with circular references marked with [JsonIgnore] attributes.

Problem

When using the new JsonPatchDocument<T> for System.Text.Json in .NET 10 with Minimal API validation enabled, the validation generator would traverse all properties of the type, including those marked with [JsonIgnore]. This caused infinite recursion through circular references in properties like SerializerOptions.Default.Default..., leading to:

System.InvalidOperationException: Maximum validation depth of 32 exceeded at 'SerializerOptions.Default.Default.Default...' in 'JsonSerializerOptions'. This is likely caused by a circular reference in the object graph.

Solution

The fix adds support for recognizing and skipping properties and parameters marked with [JsonIgnore] attributes during validation type discovery, similar to how [FromServices] attributes are already handled.

Key changes:

  • Added System.Text.Json.Serialization.JsonIgnoreAttribute to well-known types
  • Created IsJsonIgnoredProperty and IsJsonIgnoredParameter extension methods
  • Updated ExtractValidatableMembers to skip properties/parameters with [JsonIgnore] attributes
  • Added comprehensive test coverage for the JsonIgnore functionality

Example

With this fix, the following endpoint will work correctly:

app.MapPatch("/items/{id}", (int id, JsonPatchDocument<Item> patchDoc) => 
{
    // Validation will only process non-JsonIgnore properties
    return Results.Ok();
});

The validation generator will now properly exclude properties marked with [JsonIgnore] from validation traversal, preventing circular reference exceptions while still validating all other properties with validation attributes.

Fixes #63047.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Minimal API Validation throws exception for endpoint that accepts JsonPatchDocument Add JsonIgnore attribute support to Minimal API validation generator Aug 3, 2025
@Copilot Copilot AI requested a review from captainsafia August 3, 2025 01:52
Copilot finished work on behalf of captainsafia August 3, 2025 01:52
@captainsafia
Copy link
Member

@copilot Update the CanValidateComplexTypesWithJsonIgnore test with cases to cover record types. Use the existing test method and don't create a new one.

Copy link
Contributor Author

Copilot AI commented Aug 3, 2025

@copilot Update the CanValidateComplexTypesWithJsonIgnore test with cases to cover record types. Use the existing test method and don't create a new one.

Updated the CanValidateComplexTypesWithJsonIgnore test with record type test cases. Added RecordTypeWithJsonIgnore and CircularReferenceRecord types along with corresponding test validation for both the record endpoint and the existing class endpoint. The test now covers JsonIgnore functionality for both classes and records within the same test method as requested.

Commit: d038ce7

Copilot finished work on behalf of captainsafia August 3, 2025 02:28
- Remove IsJsonIgnoredParameter method since JsonIgnore can only be applied to properties and fields, not constructor parameters
- Update record parameter validation to check corresponding property for JsonIgnore instead
- Delete stale received snapshot file
- Update verified snapshot to include record type validation results

Co-authored-by: captainsafia <[email protected]>
@Copilot Copilot AI requested a review from captainsafia August 3, 2025 03:09
Copilot finished work on behalf of captainsafia August 3, 2025 03:09
@captainsafia
Copy link
Member

/azp run

@captainsafia captainsafia added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-validation Issues related to model validation in minimal and controller-based APIs and removed Attention: Shared Code Modified labels Aug 4, 2025
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@captainsafia captainsafia marked this pull request as ready for review August 4, 2025 15:59
@Copilot Copilot AI review requested due to automatic review settings August 4, 2025 15:59
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for respecting [JsonIgnore] attributes in the Minimal API validation generator to prevent "Maximum validation depth exceeded" exceptions when processing types with circular references. The primary purpose is to exclude properties marked with [JsonIgnore] from validation traversal, similar to how [FromServices] attributes are already handled.

Key changes:

  • Added JsonIgnoreAttribute to well-known types and created helper methods to detect it
  • Updated validation member extraction logic to skip properties with [JsonIgnore] attributes
  • Added comprehensive test coverage for both class and record types with JsonIgnore functionality

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
WellKnownTypeData.cs Added System_Text_Json_Serialization_JsonIgnoreAttribute to well-known types
RequiredSymbols.cs Added JsonIgnoreAttribute symbol to required symbols record
ITypeSymbolExtensions.cs Added IsJsonIgnoredProperty extension method for detecting JsonIgnore attributes
ValidationsGenerator.TypesParser.cs Updated member extraction logic to skip JsonIgnore properties
ValidationsGenerator.ComplexType.cs Added comprehensive test coverage for JsonIgnore functionality
ValidatableInfoResolver.g.verified.cs Generated test snapshot showing correct exclusion of JsonIgnore properties

@@ -148,6 +150,12 @@ internal ImmutableArray<ValidatableProperty> ExtractValidatableMembers(ITypeSymb
continue;
}

// Skip properties that have JsonIgnore attribute
if (correspondingProperty.IsJsonIgnoredProperty(jsonIgnoreAttributeSymbol))
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

Consider adding a null check for jsonIgnoreAttributeSymbol before calling the extension method to handle cases where the symbol might not be available.

Copilot uses AI. Check for mistakes.

@@ -186,6 +194,12 @@ internal ImmutableArray<ValidatableProperty> ExtractValidatableMembers(ITypeSymb
continue;
}

// Skip properties that have JsonIgnore attribute
if (member.IsJsonIgnoredProperty(jsonIgnoreAttributeSymbol))
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

Consider adding a null check for jsonIgnoreAttributeSymbol before calling the extension method to handle cases where the symbol might not be available.

Copilot uses AI. Check for mistakes.

Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

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

Philosophically, do we want this change? We're slowly adding more Json specific features to the general purpose validation feature.

We also just added [SkipValidation] which is what should be used if you don't want something to be validated.

{
return property.GetAttributes().Any(attr =>
attr.AttributeClass is not null &&
SymbolEqualityComparer.Default.Equals(attr.AttributeClass, jsonIgnoreAttributeSymbol));
Copy link
Member

@BrennanConroy BrennanConroy Aug 4, 2025

Choose a reason for hiding this comment

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

Not relevant for this change, but does this comparison work for derived types?
e.g. MyFromKeyedServicesAttribute : FromKeyedServicesAttribute

See code a few lines above

Copy link
Member

Choose a reason for hiding this comment

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

It won't, but JsonIgnoreAttribute is sealed so there's not likely to be derived types. FromKeyedServices is not.

Copy link
Member

Choose a reason for hiding this comment

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

So we should update the keyed service check in a separate PR

Copy link
Member

Choose a reason for hiding this comment

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

Yep -- we want to probably do an audit across the repo for anywhere we type check on FromKeyedServices and FromServices since both of those are unsealed.

@captainsafia
Copy link
Member

Philosophically, do we want this change? We're slowly adding more Json specific features to the general purpose validation feature.

The ship has sailed a bit on the source generator not having any STJ-specific logic in the generator since it already has logic for walking [JsonDerived] types as part of the discovery (ref).

I know we were hard core about the philosophical distinction for the property naming policy but this one feels more necessary since being incompatible with the serialziation rules by default could mean that we validate properties that are never serialized/set.

@captainsafia captainsafia merged commit 5723334 into main Aug 4, 2025
30 checks passed
@captainsafia captainsafia deleted the copilot/fix-63047 branch August 4, 2025 21:47
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-rc1 milestone Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-validation Issues related to model validation in minimal and controller-based APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimal API Validation throws exception for endpoint that accepts JsonPatchDocument
3 participants