LightController's ValidateInput method should be pure

[guilhermeluizsp]

This method should do what its name tells us: validate input data. Instead, it stores the validation result within a backing field:

https://github.com/Avanade/Liquid-Application-Framework/blob/8f03b94252db859c61992662bc7c84fced8fee64/src/Liquid.Activation/Controller/LightController.cs#L89-L115

This method is not idempotent as it has side effects.
Let's take this view model as an example:

public class TestVM : LightViewModel<TestVM>
{
	public string Id { get; set; }
	
	public override void Validate()
	{
		RuleFor(i => i.Id).NotEmpty().WithErrorCode("ERR_ID_REQUIRED");
	}
}

If we'd mistakenly call ValidateInput twice, we'd end up with THREE error messages. Why is that? The method calls the view model's Validate method:

https://github.com/Avanade/Liquid-Application-Framework/blob/8f03b94252db859c61992662bc7c84fced8fee64/src/Liquid.Activation/Controller/LightController.cs#L103-L106

which builds validators for the view model itself. By calling it multiple times, each RuleFor would also be applied multiple times. So, calling twice is the same as doing this:

public override void Validate()
{
	RuleFor(i => i.Id).NotEmpty().WithErrorCode("ERR_ID_REQUIRED");
	RuleFor(i => i.Id).NotEmpty().WithErrorCode("ERR_ID_REQUIRED");
}

Combine this with the fact that we reuse the list of errors, and now we have three error messages.

And this problem grows exponentially. ValidateInput should be a pure function.

Also, the method Factory<T> throws the InvalidInputException but, in my opinion, this is the responsability of the ValidateInput method.

[guilhermeluizsp]

I'd like to address another issues with this method. Well, Liquid has a mechanism to localize error messages via (by default) json files: We put the error code and its description inside a JSON object, just like this:

//localization.en-US.json
{
	"client_not_found": "Could not find a client with id '{0}'"
}

By using this feature, we can have both error codes and error messages within DomainResponses.

{
    "payLoad": {},
    "critics": [
        {
            "code": "client_not_found",
            "message": "Could not find a client with id '123'",
            "type": "Error"
        }
    ],
    "operationId": "6be5ede839c8ff449d3758320ee90e0b"
}

This method, however, only uses the error code, therefore making the Framework return inconsistent data (notice how it only uses the error code):

https://github.com/Avanade/Liquid-Application-Framework/blob/8b42d048fecbb379498274caa55d12fd351e8acd/src/Liquid.Activation/Controller/LightController.cs#L108-L115

{
    "payLoad": {},
    "critics": [
        {
            "code": "validation_id_empty",
            "message": "validation_id_empty", //should contain a human-readable message instead of the error code
            "type": "Error"
        }
    ],
    "operationId": "6be5ede839c8ff449d3758320ee90e0b"
}

I think this issue is more relevant to the validation middleware, but since this method is the "entry point" to that middleware, it is worth mentioning.

[bruno-brant]

One thing that I find worth mentioning and that is a corollary of the non-pure aspect of Validate(), is that the validators created by the call to Validate() aren't cached. This means that we are creating instances of Validators multiple times, on the hotpath, which can be a waste of resources (for instance, the implied reflection call to obtain the name of the property).