NEW Validate DBFields

[emteknetnz]

Issue #11391

All DBFields will be automatically validated on save

Creates a new FieldsValidator class that can be used in both FormField's and DBField's

Fields validators are defined for those classes via config on those classes

private static array $field_validators = [
    [
        'class' => MyFieldValidator::class,
        'argCalls' => [null, 'getSomeValue'],
    ],
];

Where argCalls is an optional array of public methods to call on the $field to populate constructor args passed to the FieldValidator beyond the default $name + $value.

As they're a array defined in config, they are composable so subclasses can add additional FieldValidators

This is demonstrated with both:

• DBEmail extends DBVarchar extends DBField
• EmailField extends TextField extends FormField

Both DBEmail + Email field have an EmailValidator, and both DBVarchar and TextField have a StringLengthValidator

You can see this in action with private static $db = [ 'MyEmail' => 'Email(7)' ] which will create the new DBEmail field as a varchar(7) in the database.

If submit the form / make an API request, the email will be validated server side that it's both an email, and have a length less than or equal to 7 (JS validation will prevent entering a length greater than 7, though you can inspect the input element and change the value to bypass this).

This work is would go a long way to support future API work, where there's no form field validation and no JS to stop you from entering more than 7 characters. This new system would when making an API request would throw a ValidationException rather than allow you to save invalid emails that get truncated because they're too long. Within a CMS context however it does mean that there's double validation on both the scaffolded EmailField, and the DBEmail, though I think this will have essentially no real world downside.

Performance impact

As we are now validating all DBField's on DataObject::write(), most notably enforcing varchar limits, we need to be mindful of any performance impacts. Based on some quick testing it looks like the performance impact is tiny and not worth worrying about.

I tested on my laptop by putting a for loop of 1000 iterations around the DBField validation in DataObject::write() and timing it. I then created a DataObject with 10 varchars, and a model admin to manage it. I created and saved a new record, so while there were 10 new varchar fields, they had no data in them

0.1366 seconds / 14000 DBField validations

(4000 of those iterations will be from the ID/ClassName/LastEdited/Created fields

I then put in a value for the 10x varchar fields and saved again

0.2438 seconds / 14000 DBField validations

So even when bulk creating a large number of records, there isn't much performance impact. When saving a single record the performance impact won't be noticeable.

MySQL strict mode and data truncation

MySQL behaves differently depending if strict mode is enabled or not, which means either STRICT_TRANS_TABLES or STRICT_ALL_TABLES is enabled. This can be seen by running SELECT @@sql_mode;

When I connect to mysql via terminal it's in strict mode - STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION

When I go insert into MyDataObject (Email7) values ('123456789'); I get ERROR 1406 (22001): Data too long for column 'Email7' at row 1

When I run DB::query('SELECT @@sql_mode')->value(); It's not in strict mode REAL_AS_FLOAT,PIPES_AS_CONCAT,ANSI_QUOTES,IGNORE_SPACE,ANSI

When I go MyDataObject::create(['Email7' => '1234567890'])->write(); it's created the record with Email7 truncated to "1234567"

We can't give any guarantees around whether a websites particular setup will have strict mode enabled or not, so it seems like we should be enforcing varchar limits in the ORM

Config array

Re the config - I've gone with the kind of weird double array for maximum composability. Some other options:

private static array $field_validators = [
    // A) no argCalls, int index
    MyFieldValidator::class,
    // B) no argCalls, string index. string index means you cannot have two of the same class
    MyFieldValidator::class => [], 
    // C) argCalls, string index
    MyFieldValidator::class => [null, 'getSomeValue'],
    // D) string index, no change of collision, looks ugly though
    __CLASS__ . '.' . MyFieldValidator::class => [],
]

The chance of collisions is probably really low, so changing it so instead we have both A) and C) is probably fine, or B) and C) meaning an 'argCalls' array must always be present for consistency

Other notes

• I did not create the private static array $required = []; to replace RequiredFields in this PR, though I think that's worth doing separately