Consolidate validation systems

[emteknetnz]

Currently there are two main types of validation within the CMS, which is fairly confusing:

• Model validation which is defined in DataObject::validate() and is called as part of DataObject::write(). This will always be called unless DataObject.validation_enabled is set to false. It IS called when updating data via an API system.
• Form validation, which is combination FormField::validate() and somewhat confusingly DataObject::getCMSCompositeValidator(), which is in most cases only uses the RequiredFields validator. Form field validation is called when a POST submission for a Form is processed. It IS NOT called when updating data via an API system.

Ideally we'd get rid of Form validation entirely and have all validation fire whenever DataObject::write() is called. This would have two major benefits:

• Reduce confusion to new users about how to add validation within a Silverstripe project
• Facilitate Silverstripe being fully API driven, meaning that data can be safety written via central API. The can be both within the Silverstripe CMS GUI (GraphQL was at one point supposed to do this, a future API system may also wish to do this), and from any third party system integration. If we're serious about making the CMS fully API driven I think consolidating validation is a requirement

One idea that could help greatly here would be to move validation logic from FormFields and on to their DBField counterparts, for instance move EmailField::validate() to a new class DBEmail and put the validation logic there. Projects would then need to change 'Email' => 'Varchar', to 'Email' => 'Email'. All DataObject->write()'s to this field would now have the Email field validated. This would also have an additional benefit where form auto-scaffolding would now automatically give you the correct EmailField instead of the default TextField.

We could mitigate the risk that projects forget to change Varchar to Email by putting the validation logic in a trait and just leaving it on the Email field and keeping Form field validation functional (so things double validate), though update our documentation to only give examples of the new best practice.

We'd also probably want to rename getCMSCompositeValidator() to just getCompositeValidator() and get it to run on DataObject::write(), rather than on Form submission, as RequiredField's is probably the main way that validation is added within projects. We could also just deprecate it and add a new private $static array $required_fields = []; or just $required on DataObject.

Related issues

• CMS 6: Support multiple messages for Form and FormField instead of a single message #10967

Acceptance criteria

• Work out if it's feasible to move Form validation to Model validation in the CMS
• Work out mitigations to risks this would introduce to projects that are upgrading

PRs

• NEW Validate DBFields #11402
• API Rename method silverstripe-assets#646

[GuySartorelli]

Ideally we'd get rid of Form validation entirely

I assume you mean for data objects in the CMS specifically. We would obviously need to keep form validation for custom and front-end forms.

[emteknetnz]

Realistically we'd probably need to retain form validation as just removing it could cause significant upgrade pain. However I think it should be pushed to the background as much as possible and we ourselves ideally would upgrade things we manage to use model validation exclusively, and we encourage others to do the same.

We would obviously need to keep form validation for custom and front-end forms.

I'm not sure that's true, if the idea I raised in the original description of moving validation to DBField is viable, then projects could in theory create custom DBField's to correspond with any custom fields that have custom validation. That's probably annoying for people, though it doesn't mean it's not possible.

[GuySartorelli]

Not all forms are backed by a DataObject.
Not all form fields would have a suitable DbField analogue.

I would be very resistant to moving form validation to non-form constructs. We don't want to make it unnecessarily difficult to make front-end forms, nor to create new form field types.

[emteknetnz]

Yes agree, as I said we'd probably need to retain Form validation, though encourage the use of model validation instead.

For an example of how this would look, assuming it all worked as envisioned, linkfields EmailLink would go from this:

class EmailLink extends Link
{
    // ...
    private static array $db = [
        'Email' => 'Varchar(255)',
    ];

    public function getCMSFields(): FieldList
    {
        $this->beforeUpdateCMSFields(function (FieldList $fields) {
            $fields->replaceField('Email', EmailField::create(
                'Email',
                _t(__CLASS__ . '.EMAIL_FIELD', 'Email address'),
            ));
            $fields->removeByName('OpenInNew');
        });
        return parent::getCMSFields();
    }

    public function getCMSCompositeValidator(): CompositeValidator
    {
        $validator = parent::getCMSCompositeValidator();
        $validator->addValidator(RequiredFields::create(['Email']));
        return $validator;
    }  
    // ...

to this

class EmailLink extends Link
{
    // ...
    private static array $db = [
        'Email' => 'Email',
    ];
    
    private static array $required = [
        'Email',
    ];

    public function getCMSFields(): FieldList
    {
        $this->beforeUpdateCMSFields(function (FieldList $fields) {
            $fields->removeByName('OpenInNew');
        });
        return parent::getCMSFields();
    }   
    // ...

[GuySartorelli]

That's fine, so long as someone can still create a contact form on the front end with an email field, and it gets validated correctly without needing a dbfield or data object connected to it 👍

That doesn't necessarily mean the validation for email address has to live inside the email field, but there needs to be an intuitive way to validate it with minimal additional boilerplate.

[emteknetnz]

Work out if it's feasible to move Form validation to Model validation in the CMS

I think it's viable to add validation to the model, though keep the existing form validation where it is for backwards compatibility. I don't think having double validation will create any real-world issues.

I created a PR to framework that is a POC of:

• Creating new FieldValidator's that can be used on both FormFields and DBFields, which are added via config
• Adding subclasses of these FieldValidators to a handful of FormFields and DBFields
• Updating DataObject::write() to trigger DBField validation
• Creates a new DBEmail field, which is example of on new kind of field we can easily create. We could also do this for other sorts of fields such as DBUrl, DBIp, etc, which are essentially varchar fields that come with DBField validation.

The framework PR has a write-up which goes into details such as performance impacts and data truncation

Work out mitigations to risks this would introduce to projects that are upgrading

As we're retaining the existing form validation I don't think we're introducing much risk

The performance impacts of the added validation look to be insignficant

We'd start enforcing varchar limits so there's a risk that some existing projects that allowed truncated data will start throwing exceptions. I don't see this as a bad thing though. More strictness is almost always a good thing.

Guy pointed out that we should have a way to temporarily turn off DBField validation when importing huge amounts of data, e.g. DataObject->writeWithoutValidation() or DBField::withoutValidation($callable) {} - update there's already a $skipValidation param on DataObject::write() which takes care of this

[emteknetnz]

Presented and chatted through this POC with Guy, he's broadly happy with it, will create an implementation card