Skip to content
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

Getting validation failures in AfterValidation interceptor method #45

Open
nkamenar opened this issue Dec 10, 2024 · 5 comments
Open

Getting validation failures in AfterValidation interceptor method #45

nkamenar opened this issue Dec 10, 2024 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@nkamenar
Copy link

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.

@mvdgun
Copy link
Member

mvdgun commented Dec 10, 2024

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

@mvdgun mvdgun added the enhancement New feature or request label Dec 10, 2024
@mvdgun mvdgun added this to the v2.0 milestone Dec 10, 2024
@icnocop
Copy link
Contributor

icnocop commented Dec 12, 2024

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);
    }
}

@JamesDriscoll
Copy link

JamesDriscoll commented Jan 24, 2025

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?

@mvdgun
Copy link
Member

mvdgun commented Jan 29, 2025

@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?

@JamesDriscoll
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants