Asp.net mvc web api model validation best practice

Question

I have an annoying code smell in my asp.net core api that I am passing around, and I can't come up with a way to fix.

In an MVC controller action, usually there is a very straight forward logic, at least in my case.

• Validate model (before calling the action)
• Query relevant data
• Do business logic
• Send response

Now in some actions, the logic needs to be a little bit more complex.

Sometimes, model validations requires a query to the database.

Using FluentValidation it's usually enough just to inject the required dependency, and use it to query the database - and then have the controller query other relevant data.

Now this is where it all gets messed up, I have many model validations that require the same data as the controller action.

so, what I do at the moment, in these cases seems like a code smell to me.

• Validate Model, querying only data that isn't required by the controller
• Query relevant data, and validate in the controller
• Do business logic
• Send response.

I don't like the idea of validating data inside the controller, since it often results in a 60 line action, most of which is model validation, and it breaks SOLID which might get confusing as the app grows.

The option of querying the data twice seems like an extremely bad option and frankly I see no reason to actually choose it.

So the question is basically, how do you suggest validating BindingModels against the database?

The ideal method to me seems like somehow injecting the queried data to the AbstractValidator implementation (or any other class responsible for validation), and to the controller, and that way I get to keep the original logic, without querying the same data twice. However, I am pretty sure actually implementing it is way worse.

Answer 1

I see two approaches which can be used in your case

1. You can change validation method to return data required for further actions. So validation method will look like below

public ValidationResult Validate(DataModel model, IDataService dataService)
{
    var requiredData = dataService.Load();
    var isModelValid = // do validation
    return new ValidationResult
    {
        IsValid = isModelValid,
        Model = dataModel,
        Data = requiredData
    };               
}

The responsibility of controller method will remain same - combine required actions and return result.

public IActionResult DoSomething(Model model)
{
    var result = _validationService.Validate(model, _dataService);

    var otherRelatedData = _dataService.LoadOtherData();
    var data = CombineData(result.Data, otherRelatedData);

    var actionResult = _businessLogic.Execute(model, data);

    return Ok(actionResult);
}

Code above is some kind of pseudo code - feel free refactor it, move responsibilities which satisfy your application design.

Main idea that by returning already loaded data as part of validation result will keep you from having some "hidden" cached state in data service instances. Having input - output functions makes code little bid easily to follow/understand.

2. Another approach will be load required data and pass it to the Validation method. Then you can re-use this data for business actions.

public IActionResult DoSomething(Model model)
{
    var dataForValidation = _dataService.LoadValidationData();
    var result = _validationService.Validate(model, dataForValidation);

    var otherRelatedData = _dataService.LoadOtherData();
    var data = CombineData(dataForValidation, otherRelatedData);

    var actionResult = _businessLogic.Execute(model, data);

    return Ok(actionResult);
}

Answer 2

What i would suggest is making a data layer.

Now you don't query against the database, but against your datalayer. That datalayer would be the same for the duration of your request (pick your favorite dependency injection framework to just return the same datalayer in both your validation logica and controller).

Once you know which data you are re-using, just simply add some caching logic to your data layer. If your controller requests the same data that you already queried, great it's already there and no second query is needed. If your controller & validation doesn't actually use the data in some code path you don't even have to query it at all (you don't have to know what the relevant data is ahead of time)

update: you can even gain some benefits between calls. For example if i read this stackexchange question then it's more likely that i create an answer to it. But the data for the question is already in the cache from reading it so i don't have to re-query it during the answering part.