Azure / azure-functions-host

The host/runtime that powers Azure Functions
https://functions.azure.com
MIT License
1.94k stars 441 forks source link

ValidateDataAnnotations not working #6380

Closed mark-r-cullen closed 3 years ago

mark-r-cullen commented 4 years ago

I am trying to use AddOptions() along with ValidateDataAnnotations(), however the validation doesn't appear to be running at all.

I'm not sure if this is the correct place to raise this issue, so apologies if not, but I did find the last two comments on here are describing the same thing https://github.com/MicrosoftDocs/azure-docs/issues/30814, but the ticket got closed with no resolution (presumably as it was because this is only for documentation?)

Is this something that can be looked in to?

I am using...

<TargetFramework>netcoreapp2.2</TargetFramework>
<AzureFunctionsVersion>v2</AzureFunctionsVersion>

<PackageReference Include="Microsoft.NET.Sdk.Functions" Version="1.0.31" />
<PackageReference Include="Microsoft.Extensions.Options.DataAnnotations" Version="2.2.0" />
fabiocav commented 4 years ago

@mark-r-cullen to clarify, options should be working as documented here, correct?

Can you please share the code you're using for data annotations registration? Also, can you please clarify the expectation here? Are you expecting model validation in the context of a function invocation? If so, this is not something supported by Functions today.

mark-r-cullen commented 4 years ago

Well, if I leave "Test" out of local.settings.json with the following code I was expecting this to throw some kind of validation exception somewhere (searching suggests it should throw on the .Value), but it does nothing that I can see. To be clear, Options itself is working and if I fill in "Test" in local.settings.json it picks it up, it's just the ValidateDataAnnotations that seems to do nothing.

    public class Startup : FunctionsStartup
    {
        public override void Configure(IFunctionsHostBuilder builder)
        {
            builder.Services.AddLogging();

            builder.Services.AddSingleton<ITester, Tester>();

            builder.Services
                   .AddOptions<TesterSettings>()
                   .Configure<IConfiguration>((settings, config) =>
                                              {
                                                  config.Bind(settings);
                                              })
                   .ValidateDataAnnotations();
        }
    }

    public class TesterSettings
    {
        [Required]
        public string Test { get; set; }
    }

    public interface ITester
    {
        int DoThing();
    }

    public class Tester : ITester
    {
        private readonly TesterSettings _testerSettings;

        public Tester(IOptions<TesterSettings> options)
        {
            // Does not throw OptionsValidationException
            _testerSettings = options.Value;
        }

        public int DoThing()
        {
            // Throws because Test is null
            return _testerSettings.Test.Length;
        }
    }

    public class OptionsValidation
    {
        private readonly ITester _tester;

        public OptionsValidation(ITester tester)
        {
            _tester = tester;
        }

        [FunctionName(nameof(OptionsValidation))]
        public IActionResult Run(
            [HttpTrigger(AuthorizationLevel.Anonymous, "get", "post", Route = null)] 
            HttpRequest req,
            ILogger log)
        {
            var length = _tester.DoThing();

            return new OkObjectResult(length);
        }
    }

Are you expecting model validation in the context of a function invocation? If so, this is not something supported by Functions today.

Apologies if I misunderstand what you're saying here... you are basically saying that validation isn't supported with functions then? If not could you explain what ValidateDataAnnotations() is supposed to be doing?

andvas commented 4 years ago

Possibly same issue as https://github.com/Azure/azure-webjobs-sdk/issues/2522. Mentioned workaround seems to work locally, but you will lose logging on option creation so these lines will no longer be printed image

mark-r-cullen commented 4 years ago

@andvas What version of Functions are you using? I just tried that workaround and it didn't seem to do anything for me.

andvas commented 4 years ago

@mark-r-cullen I am using 3.0.3. Did you try making a request to the endpoint? As this exception will only be thrown on first request not function startup.

mark-r-cullen commented 4 years ago

@andvas Ok, maybe there is another issue with v2 functions then. This was with making a request to the function (as I didn't expect validation to happen on start up). The workaround didn't seem to do anything, it was exactly the same as before i.e there was no validation exception thrown and the property still just had a default value assigned (null).

ThejaChoudary commented 4 years ago

@fabiocav can you please look into this further.

fabiocav commented 4 years ago

Assigned the issue in the SDK the the upcoming milestone.

You can track the work by following: https://github.com/Azure/azure-webjobs-sdk/issues/2522

john-soklaski commented 3 years ago

We have run into this issue as well and it took me some time to find this github issue.

Since this has been fairly long standing does it make sense to add it to the docs so it can be worked around? https://github.com/MicrosoftDocs/azure-docs/blob/master/articles/azure-functions/functions-dotnet-dependency-injection.md#working-with-options-and-settings

Carl-Hugo commented 3 years ago

I ran into this issue as well and it also took me some time to understand it. I don't use data annotation but implemented IValidateOptions<TOptions>.

As a workaround, I manually run all IValidateOptions<TOptions> upon creation of the options object. Then I inject the option directly, not I[whatever]Options<TOptions> (that's a personal preference anyway).

This won't help with data annotations, but maybe it will help someone else.

Here is the workaround:

services.Configure<AdB2COptions>(configuration);
services.AddSingleton(sp =>
{
    var value = sp.GetService<IOptions<AdB2COptions>>().Value;

    // FIX: Running validation manually because Azure Functions 
    // does not seems to run it for some reasons.
    var validators = sp.GetServices<IValidateOptions<AdB2COptions>>();
    foreach (var validator in validators)
    {
        var result = validator.Validate("", value);
        if (result.Failed)
        {
            throw new OptionsValidationException("", typeof(AdB2COptions), new[] { result.FailureMessage });
        }
    }
    return value;
});
services.AddSingleton<IValidateOptions<AdB2COptions>, AdB2COptionsValidation>();

configuration is an IConfiguration.

daviur commented 3 years ago

Any update on this issue? I am having the same problem as @mark-r-cullen. ValidateDataAnnotations() does not appear to do anything.

services.AddOptions<AscOptions>()
                .Configure<IConfiguration>((ascOptions, configuration) =>
                {
                    configuration.GetSection(AscOptions.Asc).Bind(ascOptions);
                })
                .ValidateDataAnnotations();

I am using also in a Function:

<TargetFramework>netcoreapp3.1</TargetFramework>
<AzureFunctionsVersion>v3</AzureFunctionsVersion>

I can make it work by at function call doing this:

services.AddOptions<AscOptions>()
                .Configure<IConfiguration>((ascOptions, configuration) =>
                {
                    configuration.GetSection(AscOptions.Asc).Bind(ascOptions);
                    var context = new ValidationContext(ascOptions, null, null);
                    Validator.ValidateObject(ascOptions, context, true);
               }
fabiocav commented 3 years ago

@daviur thanks for the follow up. The issue has been triaged and is in our backlog, but it has not been prioritized/assigned to a sprint.

Flagging for further discussion in triage.

fabiocav commented 3 years ago

@daviur the fix for this is currently rolling out to prod (global deployment expected to finish by tomorrow, with core tools following in the coming days). The original fix was made as part of this: https://github.com/Azure/azure-webjobs-sdk/issues/2522

fabiocav commented 3 years ago

Closing this as resolved/pending deployment. If you continue to see issues on version 3.1.3 of the host, please let us know.