Microsoft.Extensions.Options.DataAnnotations: ValidateDataAnnotations() doesn't validate [Required] annotation for value types

[chappleg]

Description

When using the Options pattern, the ValidateDataAnnotations() function can be used to validate various DataAnnotations that are set as attributes in an options class. One of the most common DataAnnotations is the [Required] attribute. If [Required] is set for a given field, an exception should be thrown if that field is not present in the configuration.

This works fine for strings, and other reference types. But for value types, the [Required] DataAnnotation does nothing. If the value is not present in the configuration, the default value is used, and the [Required] DataAnnotation is ignored.

Ideally, the [Required] annotation should apply to all fields, both reference and value types.

This has been confusing for our team because many assumed that [Required] applied to all fields, and didn't realize that default values were being passed in for all bools, ints, doubles, TimeSpans, etc. instead of an exception being thrown.

Reproduction Steps

Here is a minimal class that should throw an OptionsValidationException:

using System.ComponentModel.DataAnnotations;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

var emptyConfiguration = new ConfigurationBuilder().Build();

var services = new ServiceCollection();
services
    .AddOptionsWithValidateOnStart<SampleOptions>()
    .Bind(emptyConfiguration)
    .ValidateDataAnnotations();

var sampleOptions = services
    .BuildServiceProvider()
    .GetRequiredService<IOptions<SampleOptions>>();

Console.WriteLine($"IsEnabled: {sampleOptions.Value.IsEnabled}");

public class SampleOptions
{
    [Required]
    public required bool IsEnabled { get; init; }
}

When I run it, I get this output:

IsEnabled: False

But I expect this output:

Unhandled exception. Microsoft.Extensions.Options.OptionsValidationException: DataAnnotation validation failed for 'SampleOptions' members: 'IsEnabled' with the error: 'The IsEnabled field is required.'.
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.UnnamedOptionsManager`1.get_Value()
   at Program.<Main>$(String[] args) in Q:\code\Other\OptionsValidationSample\Program.cs:line 18

That exception is what gets thrown if I change the type of IsEnabled from bool to string.

For reference, this is what my .csproj file looks like:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net9.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="9.0.5" />
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="9.0.5" />
    <PackageReference Include="Microsoft.Extensions.Options" Version="9.0.5" />
    <PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="9.0.5" />
    <PackageReference Include="Microsoft.Extensions.Options.DataAnnotations" Version="9.0.5" />
  </ItemGroup>

</Project>

Expected behavior

I expect an exception like this to be thrown if a boolean config value or other value type is missing from the configuration:

Unhandled exception. Microsoft.Extensions.Options.OptionsValidationException: DataAnnotation validation failed for 'SampleOptions' members: 'IsEnabled' with the error: 'The IsEnabled field is required.'.
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.UnnamedOptionsManager`1.get_Value()
   at Program.<Main>$(String[] args) in Q:\code\Other\OptionsValidationSample\Program.cs:line 18

Actual behavior

When a boolean or other value type is missing from the configuration, the default value is provided and no exception is thrown. For example, all bools are false, all ints are 0, etc.

Regression?

No

Known Workarounds

Right now I'm using a workaround where I directly check the configuration to see if the values exist. It allows me to make a call like this to do actual validation of the required value types:

configuration.ValidateRequiredProperties<SampleOptions>()

Here is the contents of the ValidateRequiredProperties() function:

public static class ConfigurationExtensions
{
    /// <summary>
    /// Validates that all properties of optionsType with an <see cref="RequiredAttribute"/> are present in the
    /// configuration. This function doesn't check for nested configuration values.
    ///
    /// The <see cref="OptionsBuilderDataAnnotationsExtensions.ValidateDataAnnotations"/> function does the same thing
    /// for object types, but never throws if a primitive type or struct is missing. Thus, if a property of type bool,
    /// int, long, double, TimeSpan, etc. is missing from the configuration, it will not throw. This function validates
    /// all of those property types.
    /// </summary>
    public static IConfiguration ValidateRequiredProperties<TOptions>(
        this IConfiguration configuration) where TOptions : class
    {
        var optionsType = typeof(TOptions);

        var missingProperties = optionsType
            .GetProperties()
            .Where(IsRequired)
            .Select(GetConfigurationKeyNameOrPropertyName)
            .Where(key => configuration[key] is null)
            .ToList();

        if (missingProperties.Count > 0)
        {
            throw new OptionsValidationException(
                optionsName: optionsType.Name,
                optionsType: optionsType,
                failureMessages: missingProperties.Select(p =>
                    $"DataAnnotation validation failed for '{optionsType.Name}' members: '{p}' with the error: 'The {p} field is required.'."));
        }

        return configuration;
    }

    /// <summary>
    /// Returns the Name from the <see cref="ConfigurationKeyNameAttribute"/> if one exists. Else, returns the property
    /// name.
    /// </summary>
    public static string GetConfigurationKeyNameOrPropertyName(this PropertyInfo property)
        => property.GetCustomAttribute<ConfigurationKeyNameAttribute>()?.Name ?? property.Name;

    /// <summary>
    /// Returns true if the <see cref="RequiredAttribute"/> is present on the property, else false.
    /// </summary>
    private static bool IsRequired(this PropertyInfo property)
        => property.GetCustomAttribute<RequiredAttribute>() is not null;
}

Configuration

Version: .net 8
OS: Windows and Linux
Architecture: x64

Other information

No response

[KalleOlaviNiemitalo]

If [Required] is set for a given field, an exception should be thrown if that field is not present in the configuration.

I disagree. Configuration binding is one way of setting properties of options but there are other ways too. ValidateDataAnnotations should not be changed to assume that all values come from configuration only.

[tarekgh]

All ValidationAttribute interfaces including RequiredAttribute support public virtual bool IsValid(object? value) which takes object? as a parameter. If you want to use RequiredAttribute on value type, you need to make the property nullable. i.e. in your code do:

        [Required]
        public required bool? IsEnabled { get; init; } // note it is a nullable bool 

runtime/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/ValidationAttribute.cs

Line 352 in 2411805

public virtual bool IsValid(object? value)

runtime/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/RequiredAttribute.cs

Line 38 in 2411805

public override bool IsValid(object? value)

Also, if you want to validate the property to be valid only when it has true value, you want to use something like AllowedValuesAttribute or DeniedValuesAttribute instead.

[chappleg]

Yes, but then I have to explicitly convert bool? to bool every time I use it. That kind of defeats the purpose of having a [Required] attribute that ensures that it is never null.

Also, while that may be a workaround, it doesn't help with the understandability of the [Required] attribute. Nowhere does it state it only applies to reference types and not value types.

[tarekgh]

That kind of defeats the purpose of having a [Required] attribute that ensures that it is never null.

I think there may be a bit of a contradiction here. 🙂 Could you clarify how you expect the Required attribute to validate a value type for null when the type itself isn't nullable? In other words, if this needs to be addressed, how would you propose fixing it?

Nowhere does it state it only applies to reference types and not value types.

It does work with value types, as I demonstrated. A possible improvement would be to add a note to the documentation indicating that nullable types should be used when working with value types.

[chappleg]

I think there may be a bit of a contradiction here. 🙂 Could you clarify how you expect the Required attribute to validate a value type for null when the type itself isn't nullable? In other words, if this needs to be addressed, how would you propose fixing it?

I guess I just had a different assumption of how options validation works. What I am understanding from your comments is that [Required] only means that the value is not null after binding configuration. I was under the impression that it meant that the value is required to be present in the configuration when we bind our options object.

That's fine. I understand how it works now.

It does work with value types, as I demonstrated. A possible improvement would be to add a note to the documentation indicating that nullable types should be used when working with value types.

Saying "it does work with value types" and "it does work with nullable value types" are two different things. A note in the documentation would be helpful because the only other way of understanding how this works is trial and error. Like I mentioned in the summary, many people on my team made the assumption that the [Required] annotation works on regular value types.

[tarekgh]

The docs already state the following https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.requiredattribute?view=net-9.0

Remarks

The RequiredAttribute attribute specifies that when a field on a form is validated, the field must contain a value. A validation exception is raised if the property is null, contains an empty string (""), or contains only white-space characters.

If you are interested in adding to the doc to clarify the value types, feel free to submit a PR on https://github.com/dotnet/dotnet-api-docs/blob/main/xml/System.ComponentModel.DataAnnotations/RequiredAttribute.xml and tag me there. Thanks!