If View Model property has value with certain length, replace it with query from database.Do it in Controller, Validation Attribute or somewhere else?

Question

My View Model looks like this:

public class ProductVM
{
       public string SerialNumber { get; set; }

       // Other properties left out for brevity
}

In the form that fills this model, users can supply the SerialNumber of the Product or they can supply the ProductId. The SerialNumber is always 100 characters long and the ProductId is always 50 characters long.

My current solution is to get the SerialNumber from database if ProductId is supplied, inside the Action method of the Controller, after checking if ModelState.IsValid. From my view of thinking, this is bad because of the following reasons:

1. All validation logic on the parameters data passed to the action method(same goes for View Models) should be done when calling ModelState.IsValid.
2. It makes the controller 'Fat' which is viewed as an anti-pattern, as it makes it harder to understand the flow of the process.

What I want to know is:

1. Should I leave the code as is?
2. Should I move this check/transformation outside the controller action method, and if so, where?
3. Should I make new Service Query to fetch the needed data, but instead of querying by SerialNumber, to query by ProductId? (This solution looks to me like repetition of code for most of the part)

Currently my Action method looks like this:

[HttpPost]
public ActionResult GenerateReport(ProductVM vm)
{
    if(ModelState.IsValid == false)
    {
        vm.Rehydrate();
        return View(vm);
    }

    // If Length is 100 then ProductId has been supplied, query database to find SerialNumber
    if(vm.SerialNumber.Length == 100)
    {
        vm.SerialNumber = _db.GetSerialNumber(vm.SerialNumber);'

        // This is the thing that I don't like.
        // Multiple places where data validation of View Model is being done inside Controller.
        if(string.IsNullOrEmpty(vm.SerialNumber))
        {
            ModelState.AddModelError("", "No Product has been found with the given ProductId!");
            return View(vm);
            
        }
    }

    var reportData = _productDomainService.GetReportData(vm.SerialNumber);
    var report = _infrastructure.GenerateReport(reportData);

    // Rest of code left out for brevity
    
}

Answer 1

My suggestion is to refactor the code to improve separation of concerns. While it's valid to use ModelState.IsValid for standard validations such as Required and MaxLength, more complex validation that involves database interaction, such as verifying the SerialNumber or ProductId, should be handled in a dedicated service.

Here’s how you can refactor the code:

1. Create a Service for Validation and Transformation

Create a service that handles the logic of transforming and validating the SerialNumber and ProductId. This will keep your controller focused on orchestrating actions rather than managing detailed validation.

Service Implementation Example:

public interface IProductService
{
    string TransformProductIdentifier(string identifier, out string errorMessage);
}

public class ProductService : IProductService
{
    private readonly YourDbContext _db;

    public ProductService(YourDbContext db)
    {
        _db = db;
    }

    public string TransformProductIdentifier(string identifier, out string errorMessage)
    {
        if (identifier.Length == 100) // Assuming this is a ProductId
        {
            var serialNumber = _db.GetSerialNumber(identifier);
            if (string.IsNullOrEmpty(serialNumber))
            {
                errorMessage = "No Product has been found with the given ProductId!";
                return null;
            }
            errorMessage = null;
            return serialNumber;
        }

        errorMessage = null;
        return identifier; // Assume it is already a valid SerialNumber
    }
}

2. Update the Controller to Use the Service

Refactor the controller to delegate validation and transformation to the service. This approach adheres to the principle of Single Responsibility and keeps the controller simpler.

Controller Implementation Example:

public class ReportController : Controller
{
    private readonly IProductService _productService;
    private readonly IProductDomainService _productDomainService;
    private readonly IInfrastructure _infrastructure;

    public ReportController(IProductService productService, 
                            IProductDomainService productDomainService, 
                            IInfrastructure infrastructure)
    {
        _productService = productService;
        _productDomainService = productDomainService;
        _infrastructure = infrastructure;
    }

    [HttpPost]
    public ActionResult GenerateReport(ProductVM vm)
    {
        if (!ModelState.IsValid)
        {
            vm.Rehydrate();
            return View(vm);
        }

        // Use the service to transform and validate the identifier
        var serialNumber = 
          _productService.TransformProductIdentifier(vm.SerialNumber, out var 
          errorMessage);

        if (!string.IsNullOrEmpty(errorMessage))
        {
            ModelState.AddModelError("", errorMessage);
            return View(vm);
        }

        vm.SerialNumber = serialNumber;

        var reportData = _productDomainService.GetReportData(vm.SerialNumber);
        var report = _infrastructure.GenerateReport(reportData);

        //more code
   }
}