Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions src/Shared/RoslynUtils/WellKnownTypeData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ public enum WellKnownType
System_ComponentModel_DataAnnotations_ValidationAttribute,
System_ComponentModel_DataAnnotations_RequiredAttribute,
System_ComponentModel_DataAnnotations_CustomValidationAttribute,
System_ComponentModel_DataAnnotations_IValidatableObject,
Microsoft_Extensions_Validation_SkipValidationAttribute,
System_Type,
}
Expand Down Expand Up @@ -247,6 +248,7 @@ public enum WellKnownType
"System.ComponentModel.DataAnnotations.ValidationAttribute",
"System.ComponentModel.DataAnnotations.RequiredAttribute",
"System.ComponentModel.DataAnnotations.CustomValidationAttribute",
"System.ComponentModel.DataAnnotations.IValidatableObject",
"Microsoft.Extensions.Validation.SkipValidationAttribute",
"System.Type",
];
Expand Down
10 changes: 8 additions & 2 deletions src/Validation/gen/Parsers/ValidationsGenerator.TypesParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ internal bool TryExtractValidatableType(ITypeSymbol incomingTypeSymbol, WellKnow

visitedTypes.Add(typeSymbol);

var hasValidationAttributes = HasValidationAttributes(typeSymbol, wellKnownTypes);
var hasTypeLevelValidation = HasValidationAttributes(typeSymbol, wellKnownTypes) || HasIValidatableObjectInterface(typeSymbol, wellKnownTypes);
Copy link
Member

Choose a reason for hiding this comment

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

nit: the name is clearer as hasValidationAttributes. hasTypeLevelValidation makes it look like its not considering properties.

Copy link
Member Author

@oroztocil oroztocil Aug 25, 2025

Choose a reason for hiding this comment

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

This is a result of a partial check that gets ORed in a larger disjunction at the end of the method with other checks (the other checks need to be done because they also collect some data as out params so the logic cannot reasonably short-circuit). Property attributes are handled in one of the other checks.

hasValidationAttributes referred to class attributes, hasTypeLevelValidation now means the type either has class attributes or implements IValidatableObject.

Copy link
Member

Choose a reason for hiding this comment

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

Consider using the ImplementsInterface method here to do the check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, replaced


// Extract validatable types discovered in base types of this type and add them to the top-level list.
var current = typeSymbol.BaseType;
Expand All @@ -109,7 +109,7 @@ internal bool TryExtractValidatableType(ITypeSymbol incomingTypeSymbol, WellKnow
}

// No validatable members or derived types found, so we don't need to add this type.
if (members.IsDefaultOrEmpty && !hasValidationAttributes && !hasValidatableBaseType && !hasValidatableDerivedTypes)
if (members.IsDefaultOrEmpty && !hasTypeLevelValidation && !hasValidatableBaseType && !hasValidatableDerivedTypes)
{
return false;
}
Expand Down Expand Up @@ -301,4 +301,10 @@ internal static bool HasValidationAttributes(ISymbol symbol, WellKnownTypes well

return false;
}

internal static bool HasIValidatableObjectInterface(ITypeSymbol typeSymbol, WellKnownTypes wellKnownTypes)
{
var validatableObjectSymbol = wellKnownTypes.Get(WellKnownTypeData.WellKnownType.System_ComponentModel_DataAnnotations_IValidatableObject);

Choose a reason for hiding this comment

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

nit: explicit type would have been nice for readability

return typeSymbol.ImplementsInterface(validatableObjectSymbol);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,145 @@ async Task ValidateForTopLevelInvoked()
}
});
}

[Fact]
public async Task CanValidateIValidatableObject_WithoutPropertyValidations()
{
var source = """
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Validation;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;

Choose a reason for hiding this comment

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

This is a lot of usings for a simple app.

Shouldn't the following be enough,

using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;


WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

builder.Services.AddValidation();

WebApplication app = builder. Build();

app.MapPost("/base", (BaseClass model) => Results.Ok(model));
app.MapPost("/derived", (DerivedClass model) => Results.Ok(model));
Copy link
Member

Choose a reason for hiding this comment

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

We should add a case that validates that a class with a property that only implements IValidatableObject also works. I don't anticipate that this will require any code changes since the recursion already handles that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, added the test case

app.MapPost("/complex", (ComplexClass model) => Results.Ok(model));

app.Run();

public class BaseClass : IValidatableObject
{
public string? Value { get; set; }

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
if (string.IsNullOrEmpty(Value))
{
yield return new ValidationResult("Value cannot be null or empty.", [nameof(Value)]);
}
}
}

public class DerivedClass : BaseClass
{
}

public class ComplexClass
{
public NestedClass? NestedObject { get; set; }
}

public class NestedClass : IValidatableObject
{
public string? Value { get; set; }
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
if (string.IsNullOrEmpty(Value))
{
yield return new ValidationResult("Value cannot be null or empty.", [nameof(Value)]);
}
}
}
""";

await Verify(source, out var compilation);

await VerifyEndpoint(compilation, "/base", async (endpoint, serviceProvider) =>
{
await ValidateMethodCalled();

async Task ValidateMethodCalled()
{
var httpContext = CreateHttpContextWithPayload("""
{
"Value": ""
}
""", serviceProvider);

await endpoint.RequestDelegate(httpContext);

var problemDetails = await AssertBadRequest(httpContext);
Assert.Collection(problemDetails.Errors,
error =>
{
Assert.Equal("Value", error.Key);
Assert.Collection(error.Value,
msg => Assert.Equal("Value cannot be null or empty.", msg));
});
}
});

await VerifyEndpoint(compilation, "/derived", async (endpoint, serviceProvider) =>
{
await ValidateMethodCalled();

async Task ValidateMethodCalled()
{
var httpContext = CreateHttpContextWithPayload("""
{
"Value": ""
}
""", serviceProvider);

await endpoint.RequestDelegate(httpContext);

var problemDetails = await AssertBadRequest(httpContext);
Assert.Collection(problemDetails.Errors,
error =>
{
Assert.Equal("Value", error.Key);
Assert.Collection(error.Value,
msg => Assert.Equal("Value cannot be null or empty.", msg));
});
}
});

await VerifyEndpoint(compilation, "/complex", async (endpoint, serviceProvider) =>
{
await ValidateMethodCalled();

async Task ValidateMethodCalled()
{
var httpContext = CreateHttpContextWithPayload("""
{
"NestedObject": {
"Value": ""
}
}
""", serviceProvider);

await endpoint.RequestDelegate(httpContext);

var problemDetails = await AssertBadRequest(httpContext);
Assert.Collection(problemDetails.Errors,
error =>
{
Assert.Equal("NestedObject.Value", error.Key);
Assert.Collection(error.Value,
msg => Assert.Equal("Value cannot be null or empty.", msg));
});
}
});
}
}
Loading
Loading