FluentValidation.AutoValidation icon indicating copy to clipboard operation
FluentValidation.AutoValidation copied to clipboard

Getting validation failures in AfterValidation interceptor method

Open nkamenar opened this issue 1 year ago • 5 comments

Previously we were using FluentValidation.AspNetCore to automatically validate parameter for controller actions. We then had an IAsyncActionFilter that was registered to detect if there are ModelState errors, and convert them into a custom error model. Since FluentValidation.AspNetCore is no longer supported we moved to SharpGrip.FluentValidation.AutoValidation however we noticed that SharpGrip seems to pre-empt our filter if there are errors and just return a plain ProblemDetails object.

After looking at the documentation I see you provide an AfterValidation interceptor, so I tried to convert our filter to be a IGlobalValidationInterceptor instead but I don't see how you get the list of failures in this method. The IValidationContext doesn't seem to have a way to get at the failures so I am wondering how is this used? Is the purpose of this method not to be able to take action based on the results of the validation? It's kind of hard to do that if I don't have access to the results of the validation in this method. Any help understanding would be greatly appreciated.

nkamenar avatar Dec 10 '24 20:12 nkamenar

Hi @nkamenar,

You are correct that the validation interceptors are not receiving the validation result. This was an oversight on my part when implementing the validation interceptors.

Since adding a new parameter to the interface would break existing code, I will set the milestone for this to v2. I haven’t conducted a deep dive into the code to determine if there is a backward-compatible fix that avoids introducing new interfaces. However, if you discover anything, please let me know!

See also: https://github.com/SharpGrip/FluentValidation.AutoValidation/issues/12

mvdgun avatar Dec 10 '24 21:12 mvdgun

I was able to replace an ActionFilterAttribute, which converts ModelState errors to ValidationProblemDetails, and use a custom IFluentValidationAutoValidationResultFactory instead.

services.AddFluentValidationAutoValidation(configuration =>
{
    // Replace the default result factory with a custom implementation.
    configuration.OverrideDefaultResultFactoryWith<CustomResultFactory>();
});

public class CustomResultFactory : IFluentValidationAutoValidationResultFactory
{
    public IActionResult CreateActionResult(ActionExecutingContext context, ValidationProblemDetails? validationProblemDetails)
    {
        // ModelState errors are available in context.ModelState
        return new BadRequestObjectResult(validationProblemDetails);
    }
}

icnocop avatar Dec 12 '24 19:12 icnocop

Hi @nkamenar,

You are correct that the validation interceptors are not receiving the validation result. This was an oversight on my part when implementing the validation interceptors.

Since adding a new parameter to the interface would break existing code, I will set the milestone for this to v2. I haven’t conducted a deep dive into the code to determine if there is a backward-compatible fix that avoids introducing new interfaces. However, if you discover anything, please let me know!

See also: #12

Hi!

A non-breaking change would be to create an additional "inspecting" interceptor e.g.

using FluentValidation;
using FluentValidation.Results;
using Microsoft.AspNetCore.Mvc.Filters;

namespace SharpGrip.FluentValidation.AutoValidation.Mvc.Interceptors
{
    /// <summary>
    /// Allows global intercepting and inspecting of the validation process by implementing this interface on a custom class and registering it with the service collection.
    ///
    /// The interceptor methods of instances of this interface will get called on each validation attempt.
    /// </summary>
    public interface IGlobalValidationInspectingInterceptor
    {
        public void Inspect(ActionExecutingContext actionExecutingContext, IValidationContext validationContext, ValidationResult validationResult);
    }
}

the main body of the filter then becomes:

                            var validatorInterceptor = validator as IValidatorInterceptor;
                            var globalValidationInterceptor = serviceProvider.GetService<IGlobalValidationInterceptor>();
// NEW LINE OF CODE
                            var globalValidationInspectingInterceptor = serviceProvider.GetService<IGlobalValidationInspectingInterceptor>();

                            IValidationContext validationContext = new ValidationContext<object>(subject);

                            if (validatorInterceptor != null)
                            {
                                validationContext = validatorInterceptor.BeforeValidation(actionExecutingContext, validationContext) ?? validationContext;
                            }

                            if (globalValidationInterceptor != null)
                            {
                                validationContext = globalValidationInterceptor.BeforeValidation(actionExecutingContext, validationContext) ?? validationContext;
                            }

                            var validationResult = await validator.ValidateAsync(validationContext, actionExecutingContext.HttpContext.RequestAborted);

// NEW LINE OF CODE
                            globalValidationInspectingInterceptor?.Inspect(actionExecutingContext, validationContext, validationResult);

                            if (validatorInterceptor != null)
                            {
                                validationResult = validatorInterceptor.AfterValidation(actionExecutingContext, validationContext) ?? validationResult;
                            }

                            if (globalValidationInterceptor != null)
                            {
                                validationResult = globalValidationInterceptor.AfterValidation(actionExecutingContext, validationContext) ?? validationResult;
                            }

In the test for this, I would add then following lines to the arrange section:

        var globalValidationInspectingInterceptor = Substitute.For<IGlobalValidationInspectingInterceptor>();
        serviceProvider.GetService(typeof(IGlobalValidationInspectingInterceptor)).Returns(globalValidationInspectingInterceptor);

and add the following to the assert section

        globalValidationInspectingInterceptor.Received(1).Inspect(actionExecutingContext, Arg.Any<IValidationContext>(), Arg.Any<ValidationResult>());

You may well want to create an IGlobalValidationInspectingAsyncInterceptor too.

Would you like me to create a PR for this?

JamesDriscoll avatar Jan 24 '25 16:01 JamesDriscoll

@JamesDriscoll thanks for your suggestions! I want to rework the interception logic in v2 to include the full validation scope. Fow now I don't want to support 2 variants of intercepting in v1.

I am interested in how you would go about reworking the interception logic so everything is included, perhaps you could take a look at that?

mvdgun avatar Jan 29 '25 16:01 mvdgun

Sorry for the slow response... have been quite hectic here. I think there are four use cases for interception here:

  1. Pre-validation in order to amend the validation context
  2. Post validation to observe the validation result and perform some action based on it
  3. Post validation to inspect and amend the validation result
  4. To produce a custom IActionResult in a custom IFluentValidationAutoValidationResultFactory

I think that there are situations where you want to perform synchronous operations here, and others where you want to be asynchronous.

I like that you have a global interface for all validation and that this can be also done on a per-validator basis.

I don't think that these use cases necessarily overlap. So I would probably look something like the following to cover the first three use cases, with a single method on each:

  • IGlobalBeforeValidationInterceptor
  • IGlobalObservingValidationInspector
  • IGlobalAfterValidationInterceptor
  • IGlobalBeforeValidationAsyncInterceptor
  • IGlobalObservingValidationAsyncInspector
  • IGlobalAfterValidationAsyncInterceptor
  • IBeforeValidationInterceptor
  • IObservingValidationInspector
  • IAfterValidationInterceptor
  • IBeforeValidationAsyncInterceptor
  • IObservingValidationAsyncInspector
  • IAfterValidationAsyncInterceptor

Then I would amend IFluentValidationAutoValidationResultFactory to also pass in the ValidationResult.

I think that would give consumers of the library the most flexibility, and also mean they only need to implement what they need and nothing else.

From my side, my use case is to observe in order to return a custom IActionResult.

I hope this helps!

I am happy to create a branch if you would like. If so should I branch from master?

JamesDriscoll avatar Feb 05 '25 11:02 JamesDriscoll