In MVC should a model handle validation?

Question

I am trying to re-architect a web application I developed to use the MVC pattern, but I'm not sure if validation should be handled in the model or not. For example, I'm setting up one of my models like this:

class AM_Products extends AM_Object 
{
    public function save( $new_data = array() ) 
    {
        // Save code
    }
}

First Question: So I'm wondering if my save method should call a validation function on $new_data or assume that the data has already been validated?

Also, if it were to offer validation, I'm thinking some of the model code to define data types would look like this:

class AM_Products extends AM_Object
{
    protected function init() // Called by __construct in AM_Object
    {
        // This would match up to the database column `age`
        register_property( 'age', 'Age', array( 'type' => 'int', 'min' => 10, 'max' => 30 ) ); 
    }
}

Second Question: Every child class of AM_Object would run register_property for each column in the database of that specific object. I'm not sure if this is a good way of doing it or not.

Third Question: If validation should be handled by the model, should it return an error message or an error code and have the view use the code to display an appropriate message?

Answer 1

First Answer: A key role of the model is to maintain integrity. However processing user input is a responsibility of a controller.

That is, the controller must translate user data (which most of the time is just strings) into something meaningful. This requires parsing (and may depend on such things as the locale, given that for example, there are different decimal operators etc.).
So the actual validation, as in "is the data well formed?", should be performed by the controller. However the verification, as in "does the data make sense?" should be performed within the model.

To clarify this with an example:
Assume your application allows you to add some entities, with a date (an issue with a dead-line for example). You might have an API, where dates might be represented as mere Unix time stamps, while when coming from a HTML page, it will be a set of different values or a string in the format of MM/DD/YYYY. You don't want this information in the model. You want each controller to individually try to figure out the date. However, when the date is then passed to the model, the model must maintain integrity. For example, it might make sense to not allow dates in the past, or dates, that are on holidays/sundays, etc.

Your controller contains input (processing) rules. Your model contains business rules. You want your business rules to always be enforced, no matter what happens. Assuming you had business rules in the controller, then you'd have to duplicate them, should you ever create a different controller.

Second Answer: The approach does make sense, however the method could be made more powerful. Instead of the last parameter being an array, it should be an instance of IContstraint which is defined as:

interface IConstraint {
     function test($value);//returns bool
}

And for numbers you could have something as

class NumConstraint {
    var $grain;
    var $min;
    var $max;
    function __construct($grain = 1, $min = NULL, $max = NULL) {
         if ($min === NULL) $min = INT_MIN;
         if ($max === NULL) $max = INT_MAX;
         $this->min = $min;
         $this->max = $max;
         $this->grain = $grain;
    }
    function test($value) {
         return ($value % $this->grain == 0 && $value >= $min && $value <= $max);
    }
}

Also I don't see what 'Age' is meant to represent, to be honest. Is it the actual property name? Assuming there's a convention by default, the parameter could simple go to the end of the function and be optional. If not set, it would default to the to_camel_case of the DB column name.

Thus the example call would look like:

register_property('age', new NumConstraint(1, 10, 30));

The point of using interfaces is that you can add more and more constraints as you go and they can be as complicated as you want. For a string to match a regular expression. For a date to be at least 7 days ahead. And so on.

Third Answer: Every Model entity should have a method like Result checkValue(string property, mixed value). The controller should call it prior to setting data. The Result should have all the information about whether the check failed, and in case it did, give reasons, so the controller can propagate those to the view accordingly.
If a wrong value is passed to the model, the model should simply respond by raising an exception.

Answer 2

I do not completely agree with "back2dos": My recommendation is to always use a separate form/validation layer, that the controller can use to validate input data before it's sent to the model.

From a theoretical standpoint, model validation operates on trusted data (internal system state) and should ideally be repeatable at any point in time while input validation explicitly operates once on data that comes from untrusted sources (depending on the use case and user privileges).

This separation makes it possible to build reusable models, controllers and forms that can be loosely coupled through dependency injection. Think of input validation as whitelist validation (“accept known good”) and model validation as blacklist validation (“reject known bad”). Whitelist validation is more secure while blacklist validation prevents your model layer from being overly constrained to very specific use cases.

Invalid model data should always cause an exception to be thrown (otherwise the application can continue running without noticing the mistake) while invalid input values coming from external sources are not unexpected, but rather common (unless you got users that never make mistakes).

See also: https://lastzero.net/2015/11/why-im-using-a-separate-layer-for-input-data-validation/

Answer 3

Yes, the model should perform validation. The UI should validate the input, too.

It's clearly the responsibility of the model to determine valid values and states. Sometimes such rules change often. In that case I'd feed the model from metadata and/or decorate it.

Answer 4

Great question!

In terms of world wide web development, what if you asked the following, also.

"If bad user input is supplied to a controller from a user interface, should the controller update the View in a kind of cyclic loop, forcing commands and input data to be accurate before processing them? How? How does the view get updated under normal conditions? Is a view tightly coupled to a model? Is user input validation core business logic of the model, or is it preliminary to it and thus should occur inside the controller (because user input data is part of the request)?

(In effect, can, and should, one delay instantiating a model until good input is acquired?)

My opinion is that models should manage a pure and pristine circumstance (as much as possible), unencumbered by basic HTTP request input validation that should occur before model instantiation (and definitely before the model gets input data). Since managing state data (persistent, or otherwise) and API relationships is the world of the model, let basic HTTP request input validation occur in the controller.

Summarizing.

1) Validate your route (parsed from the URL), as the controller and method must exist before anything else can go forward. This should definitely happen in the front-controller realm (Router class), before getting to the true controller. Duh. :-)

2) A model may have many sources of input data: an HTTP request, a database, a file, an API, and yes, a network. If you are going to place all of your input validation into the model, then you consider HTTP request input validation part of the business requirements for the program. Case closed.

3) Yet, it is myopic to go through the expense of instantiating lots of objects if the HTTP request input is no good! You can know if ** HTTP request input** is good (that came in with the request) by validating it before instantiating the model and all its complexities (yes, perhaps even more validators for API and DB input/output data).

Test the following:

a) The HTTP request method (GET, POST, PUT, PATCH, DELETE ...)

b) Minimum HTML controls (do you have enough?).

c) Maximum HTML controls (do you have too many?).

d) Correct HTML controls (do you have the right ones?).

e) Input encoding (typically, is the encoding UTF-8?).

f) Max input size (is any of the input wildly out of bounds?).

Remember, you may get strings and files, so waiting for the model to instantiate could get very expensive as requests hit your server.

What I have described here hits at the intent of the request coming in through the controller. Omitting the verification of intent leaves your application more vulnerable. Intent can only be good (playing by your fundamental rules) or bad (going outside your fundamental rules).

Intent for an HTTP request is an all or nothing proposition. Everything passes, or the request is invalid. No need to send anything to the model.

This basic level of HTTP request intent has nothing to do with regular user input errors and validation. In my applications, an HTTP request must be valid in the five ways above for me to honor it. In a defense-in-depth way of speaking, you never get to user input validation on the server-side if any these five things fail.

Yes, this means even file input must conform to your front-end attempts to verify and tell the user the max file size accepted. Only HTML? No JavaScript? Fine, but the user must be informed of the consequences of uploading files that are too big (chiefly, that they will lose all form data and be kicked out of the system).

4) Does this mean that HTTP request input data is not part of the business logic of the application? No, it just means computers are finite devices and resources must be used wisely. It makes sense to stop malicious activity sooner, not later. You pay more in compute resources for waiting to stop it later.

5) If the HTTP request input is bad, the entire request is bad. That is how I look at it. The definition of good HTTP request input is derived from the business requirements of the model, but there must be some point of resource demarcation. How long will you let a bad request live before killing it and saying, "Oh, hey, never mind. Bad request."

The judgement is not simply that the user has made a reasonable input mistake, but that an HTTP request is so out-of-bounds that it must be declared malicious and stopped immediately.

6) So, for my money, the HTTP request (METHOD, URL/route, and data) is either ALL good, or NOTHING else can proceed. A robust model already has validation tasks to concern itself with, but a good resource shepherd says "My way, or the high way. Come correct, or do not come at all."

It is your program, though. "There is more than one way to do it." Some ways cost more in time and money than others. Validating HTTP request data later (in the model) should cost more over the lifetime of an application (especially if scaling up or out).

If your validators are modular, validating basic *HTTP request input** in the controller should not be a problem. Just use a strategized Validator class, where validators are sometimes composed of specialized validators, too (e-mail, phone, form token, captcha, ...).

Some see this as completely wrong headed, but HTTP was in its infancy when the Gang of Four wrote Design Patterns: Elements of Re-usable Object-Oriented Software.

==========================================================================

Now, as it pertains to normal user input validation (after the HTTP request has been deemed valid), it is updating the view when the user messes up that you need to think about! This kind of user input validation should occur in the model.

You have no guarantee of JavaScript on the front-end. This means you have no way to guarantee asynchronous updating of your application's UI with error statuses. True progressive enhancement would cover the synchronous use case, too.

Accounting for the synchronous use case is an art that is being lost more and more because some people do not want to go through the time, and hassle, of tracking the state of all their UI tricks (show/hide controls, disable/enable controls, error indications, error messages) on the back-end (usually by tracking state in arrays).

Update: In the diagram, I say that the View should reference the Model. No. You should pass data to the View from the Model to preserve loose coupling.