What is the correct way to handle controller request model validation with nullable reference types?

Question

Say I have a request model object:

public class RequestModel
{
   public string? Id { get; set; }

   // other properties...
}

And I want to use that model for this example controller method:

public ResponseModel ExampleMethod(RequestModel request)
{
   // FluentValidation validator
   _validator.ValidateAndThrow(request);

   // This method does not accept a nullable type
   _dependency.DoSomething(request.Id); // Causes "Possible null reference argument for parameter" error

   return new ResponseModel();
}

In this case it's correct for the Id property to be marked as nullable (because in theory the request could not include it). The validator will ensure that the properties are not null. However if I want to use this property in DoSomething() then I will get compiler warnings due to the fact that the Id could be null. The only solution I can find is to map the external request object to some internal version where the properties are not nullable.

However this would require the mapping to essentially be performing validation (by throwing some kind of exception during mapping if a null is encountered) which feels like a violation of separation of concerns:

public ResponseModel ExampleMethod(RequestModel request)
{
   // FluentValidation validator
   _validator.ValidateAndThrow(request);

   // Map the request to an internal object - throw an exception if mapping fails due to null properties
   var internalModel = _mapper.Map<InternalModel>(request);

   // This method does not accept a nullable type
   _dependency.DoSomething(internalModel.Id); // No more error

   return new ResponseModel();
}

Not sure if I'm missing something here or if this is the only way to solve the problem. I can't make the property non-nullable as then it would require a default value (eg. empty string, or even worse - null! or default!) which would make it impossible to determine whether the property was missing in the request or was genuinely passed as an empty string. I believe something like this proposal may resolve the issue as then I would be able to indicate to the compiler that I'm expecting these non-nullable properties to be provided upon initialization (by model binding) rather than with a constructor. Am I missing some aspect of nullable reference types here that would make this any easier to deal with?

Answer 1

The only solution I can find is to map the external request object to some internal version where the properties are not nullable.

This sounds like a great approach to me. It's very common to separate request models from your core business models. The role of a controller action (which this appears to be) is largely to coordinate the translation of outside requests to and from the core business logic.

You might even want to have your dependency use your internal model rather than the Id, to avoid primitive obsession. If your dependency "knows" that the number it's being given should represent the Id of a specific type of model, it may be less error-prone to make it impossible for someone to give it a number that has nothing to do with that model type (or an ID directly from an input model that they forgot to validate).

   _dependency.DoSomething(internalModel);

However this would require the mapping to essentially be performing validation (by throwing some kind of exception during mapping if a null is encountered) which feels like a violation of separation of concerns

Validation of inputs is an implied part of any method's contract, including any method that returns a value. Does int.Parse() violate separation of concerns by throwing exceptions on bad inputs before returning an int?

If anything, you're violating separation of concerns by using a single model class to represent two different concepts (input versus domain model), which can change for different reasons.

There's only one "concern" involved with validating the input model and converting it into a known-valid domain model. That implies that you should probably separate that concern into its own method/class.

public ResponseModel ExampleMethod(RequestModel request)
{
   var internalModel = _requestValidator.Validate(request);

   _dependency.DoSomething(internalModel);

   return new ResponseModel();
}

The fact that your _requestValidator is using fluent model validation and automapper is an implementation detail that this level of code (a controller action, e.g.) shouldn't have to worry about. Maybe you'll change that some day to use explicit hand-coded mapping. You'd want your unit tests to test that validation/mapping independent of this class's logic.