From bca4be77ed68382b11c61e2e646c08f61c4e8a19 Mon Sep 17 00:00:00 2001 From: cpenny <cpenny@silverstripe.com> Date: Thu, 28 May 2020 09:35:07 +1200 Subject: [PATCH] Update name to CompositeValidator. Add docblocks --- .../03_Forms/01_Validation.md | 2 +- ...lidatorList.php => CompositeValidator.php} | 87 ++++++++++++------ src/Forms/DefaultFormFactory.php | 8 +- src/Forms/GridField/GridFieldDetailForm.php | 2 +- src/ORM/DataExtension.php | 8 +- src/ORM/DataObject.php | 17 ++-- ...istTest.php => CompositeValidatorTest.php} | 92 +++++++++---------- ...torList.php => TestCompositeValidator.php} | 6 +- 8 files changed, 128 insertions(+), 94 deletions(-) rename src/Forms/{ValidatorList.php => CompositeValidator.php} (66%) rename tests/php/Forms/{ValidatorListTest.php => CompositeValidatorTest.php} (57%) rename tests/php/Forms/ValidatorTest/{TestValidatorList.php => TestCompositeValidator.php} (70%) diff --git a/docs/en/02_Developer_Guides/03_Forms/01_Validation.md b/docs/en/02_Developer_Guides/03_Forms/01_Validation.md index 4a5467c2f..191ddd2a7 100644 --- a/docs/en/02_Developer_Guides/03_Forms/01_Validation.md +++ b/docs/en/02_Developer_Guides/03_Forms/01_Validation.md @@ -275,7 +275,7 @@ class MyController extends Controller In the CMS, we're not creating the forms for editing CMS records. The `Form` instance is generated for us so we cannot call `setValidator` easily. However, a `DataObject` can provide its' own `Validator` instance/s through the -`getValidatorList()` method. The CMS interfaces such as [LeftAndMain](api:SilverStripe\Admin\LeftAndMain), +`getCompositeValidator()` method. The CMS interfaces such as [LeftAndMain](api:SilverStripe\Admin\LeftAndMain), [ModelAdmin](api:SilverStripe\Admin\ModelAdmin) and [GridField](api:SilverStripe\Forms\GridField\GridField) will respect the provided `Validator`/s and handle displaying error and success responses to the user. diff --git a/src/Forms/ValidatorList.php b/src/Forms/CompositeValidator.php similarity index 66% rename from src/Forms/ValidatorList.php rename to src/Forms/CompositeValidator.php index ae189c12c..f08dd9491 100644 --- a/src/Forms/ValidatorList.php +++ b/src/Forms/CompositeValidator.php @@ -6,11 +6,35 @@ use InvalidArgumentException; use SilverStripe\ORM\ValidationResult; /** - * Class ValidatorList + * CompositeValidator can contain between 0 and many different types of Validators. Each Validator is itself still + * responsible for Validating its form and generating its ValidationResult. + * + * Once each Validator has generated its ValidationResult, the CompositeValidator will combine these results into a + * single ValidationResult. This single ValidationResult is what will be returned by the CompositeValidator. + * + * You can add Validators to the CompositeValidator in any DataObject by extending the getCompositeValidator() method: + * + * public function getCompositeValidator(): CompositeValidator + * { + * $compositeValidator = parent::getCompositeValidator(); + * + * $compositeValidator->addValidator(RequiredFields::create(['MyRequiredField'])); + * + * return $compositeValidator + * } + * + * Or by implementing the updateCompositeValidator() method in a DataExtension: + * + * public function updateCompositeValidator(CompositeValidator $compositeValidator): void + * { + * $compositeValidator->addValidator(RequiredFields::create(['AdditionalContent'])); + * } + * + * Class CompositeValidator * * @package SilverStripe\Forms */ -class ValidatorList extends Validator +class CompositeValidator extends Validator { /** * @var array|Validator[] @@ -18,7 +42,7 @@ class ValidatorList extends Validator private $validators; /** - * ValidatorList constructor. + * CompositeValidator constructor. * * @param array|Validator[] $validators */ @@ -30,6 +54,8 @@ class ValidatorList extends Validator } /** + * Set the provided Form to the CompositeValidator and each Validator that has been added. + * * @param Form $form * @return Validator */ @@ -44,9 +70,9 @@ class ValidatorList extends Validator /** * @param Validator $validator - * @return ValidatorList + * @return CompositeValidator */ - public function addValidator(Validator $validator): ValidatorList + public function addValidator(Validator $validator): CompositeValidator { $this->validators[] = $validator; @@ -54,8 +80,8 @@ class ValidatorList extends Validator } /** - * Returns any errors there may be. This method considers the enabled status of the ValidatorList as a whole - * (exiting early if the List is disabled), as well as the enabled status of each individual Validator. + * Returns any errors there may be. This method considers the enabled status of the CompositeValidator as a whole + * (exiting early if the Composite is disabled), as well as the enabled status of each individual Validator. * * @return ValidationResult */ @@ -63,7 +89,7 @@ class ValidatorList extends Validator { $this->resetResult(); - // This ValidatorList has been disabled in full + // This CompositeValidator has been disabled in full if (!$this->getEnabled()) { return $this->result; } @@ -142,6 +168,8 @@ class ValidatorList extends Validator } /** + * Return all Validators that match a certain class name. EG: RequiredFields::class + * * @param string $className * @return array|Validator[] */ @@ -161,10 +189,12 @@ class ValidatorList extends Validator } /** + * Remove all Validators that match a certain class name. EG: RequiredFields::class + * * @param string $className - * @return ValidatorList + * @return CompositeValidator */ - public function removeValidatorsByType(string $className): ValidatorList + public function removeValidatorsByType(string $className): CompositeValidator { foreach ($this->getValidatorsByType($className) as $key => $validator) { $this->removeValidatorByKey($key); @@ -174,23 +204,9 @@ class ValidatorList extends Validator } /** - * @param int $key - * @return ValidatorList - */ - public function removeValidatorByKey(int $key): ValidatorList - { - if (!array_key_exists($key, $this->validators)) { - throw new InvalidArgumentException( - sprintf('Key "%s" does not exist in $validators array', $key) - ); - } - - unset($this->validators[$key]); - - return $this; - } - - /** + * Each Validator is aware of whether or not it can be cached. If even one Validator cannot be cached, then the + * CompositeValidator as a whole also cannot be cached. + * * @return bool */ public function canBeCached(): bool @@ -203,4 +219,21 @@ class ValidatorList extends Validator return true; } + + /** + * @param int $key + * @return CompositeValidator + */ + protected function removeValidatorByKey(int $key): CompositeValidator + { + if (!array_key_exists($key, $this->validators)) { + throw new InvalidArgumentException( + sprintf('Key "%s" does not exist in $validators array', $key) + ); + } + + unset($this->validators[$key]); + + return $this; + } } diff --git a/src/Forms/DefaultFormFactory.php b/src/Forms/DefaultFormFactory.php index a43fbb18c..9eea865ee 100644 --- a/src/Forms/DefaultFormFactory.php +++ b/src/Forms/DefaultFormFactory.php @@ -101,17 +101,17 @@ class DefaultFormFactory implements FormFactory return null; } - $validatorList = $context['Record']->getValidatorList(); + $compositeValidator = $context['Record']->getCompositeValidator(); // Extend validator - legacy support, will be removed in 5.0.0 - foreach ($validatorList->getValidators() as $validator) { + foreach ($compositeValidator->getValidators() as $validator) { $this->invokeWithExtensions('updateFormValidator', $validator, $controller, $name, $context); } // Extend validator - forward support, will be supported beyond 5.0.0 - $this->invokeWithExtensions('updateValidatorList', $validatorList); + $this->invokeWithExtensions('updateCompositeValidator', $compositeValidator); - return $validatorList; + return $compositeValidator; } /** diff --git a/src/Forms/GridField/GridFieldDetailForm.php b/src/Forms/GridField/GridFieldDetailForm.php index 47266f233..4e1a401bb 100644 --- a/src/Forms/GridField/GridFieldDetailForm.php +++ b/src/Forms/GridField/GridFieldDetailForm.php @@ -123,7 +123,7 @@ class GridFieldDetailForm implements GridField_URLHandler // if no validator has been set on the GridField then use the Validators from the record. if (!$this->getValidator()) { - $this->setValidator($record->getValidatorList()); + $this->setValidator($record->getCompositeValidator()); } return $handler->handleRequest($request); diff --git a/src/ORM/DataExtension.php b/src/ORM/DataExtension.php index 85814447d..3d29e7ea7 100644 --- a/src/ORM/DataExtension.php +++ b/src/ORM/DataExtension.php @@ -5,7 +5,7 @@ namespace SilverStripe\ORM; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Extension; use SilverStripe\Forms\FieldList; -use SilverStripe\Forms\ValidatorList; +use SilverStripe\Forms\CompositeValidator; use SilverStripe\ORM\Queries\SQLSelect; use Exception; @@ -137,11 +137,11 @@ abstract class DataExtension extends Extension /** * This function is used to provide modifications to the Validators used on a DataObject. * - * Caution: Use {@link ValidatorList->addValidator()} to add Validators. + * Caution: Use {@link CompositeValidator->addValidator()} to add Validators. * - * @param ValidatorList $validatorList + * @param CompositeValidator $compositeValidator */ - public function updateValidatorList(ValidatorList $validatorList): void + public function updateCompositeValidator(CompositeValidator $compositeValidator): void { } diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 12c6dc8da..0e7b25d20 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -15,7 +15,7 @@ use SilverStripe\Dev\Deprecation; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; -use SilverStripe\Forms\ValidatorList; +use SilverStripe\Forms\CompositeValidator; use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18nEntityProvider; use SilverStripe\ORM\Connect\MySQLSchemaManager; @@ -2449,25 +2449,26 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * @return ValidatorList + * @return CompositeValidator */ - public function getValidatorList(): ValidatorList + public function getCompositeValidator(): CompositeValidator { - $validatorList = new ValidatorList(); + $compositeValidator = new CompositeValidator(); // Support for the old method during the deprecation period if ($this->hasMethod('getCMSValidator')) { Deprecation::notice( '4.6', - 'getCMSValidator() is removed in 5.0 in favour of getValidatorList()' + 'getCMSValidator() is removed in 5.0 in favour of getCompositeValidator()' ); - $validatorList->addValidator($this->getCMSValidator()); + $compositeValidator->addValidator($this->getCMSValidator()); } - $this->invokeWithExtensions('updateValidatorList', $validatorList); + // Extend validator - forward support, will be supported beyond 5.0.0 + $this->invokeWithExtensions('updateCompositeValidator', $compositeValidator); - return $validatorList; + return $compositeValidator; } /** diff --git a/tests/php/Forms/ValidatorListTest.php b/tests/php/Forms/CompositeValidatorTest.php similarity index 57% rename from tests/php/Forms/ValidatorListTest.php rename to tests/php/Forms/CompositeValidatorTest.php index 839a797b6..c6a03e643 100644 --- a/tests/php/Forms/ValidatorListTest.php +++ b/tests/php/Forms/CompositeValidatorTest.php @@ -8,15 +8,15 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\RequiredFields; use SilverStripe\Forms\Tests\ValidatorTest\TestValidator; -use SilverStripe\Forms\Tests\ValidatorTest\TestValidatorList; +use SilverStripe\Forms\Tests\ValidatorTest\TestCompositeValidator; use SilverStripe\Forms\TextField; -use SilverStripe\Forms\ValidatorList; +use SilverStripe\Forms\CompositeValidator; /** * @package framework * @subpackage tests */ -class ValidatorListTest extends SapphireTest +class CompositeValidatorTest extends SapphireTest { /** * Common method for setting up form, since that will always be a dependency for the validator. @@ -38,28 +38,28 @@ class ValidatorListTest extends SapphireTest public function testAddValidator(): void { - $validatorList = new ValidatorList(); - $validatorList->addValidator(new RequiredFields()); - $validatorList->addValidator(new RequiredFields()); + $compositeValidator = new CompositeValidator(); + $compositeValidator->addValidator(new RequiredFields()); + $compositeValidator->addValidator(new RequiredFields()); - $this->assertCount(2, $validatorList->getValidators()); + $this->assertCount(2, $compositeValidator->getValidators()); } public function testSetForm(): void { $form = $this->getForm(); - $validatorList = new TestValidatorList(); + $compositeValidator = new TestCompositeValidator(); $validator = new TestValidator(); - $validatorList->addValidator($validator); + $compositeValidator->addValidator($validator); - $validatorList->setForm($form); + $compositeValidator->setForm($form); - $this->assertNotNull($validatorList->getForm()); - $this->assertCount(1, $validatorList->getValidators()); + $this->assertNotNull($compositeValidator->getForm()); + $this->assertCount(1, $compositeValidator->getValidators()); - foreach ($validatorList->getValidators() as $validator) { + foreach ($compositeValidator->getValidators() as $validator) { /** @var TestValidator $validator */ $this->assertNotNull($validator->getForm()); } @@ -67,46 +67,46 @@ class ValidatorListTest extends SapphireTest public function testGetValidatorsByType(): void { - $validatorList = new ValidatorList(); - $validatorList->addValidator(new RequiredFields()); - $validatorList->addValidator(new TestValidator()); - $validatorList->addValidator(new RequiredFields()); - $validatorList->addValidator(new TestValidator()); + $compositeValidator = new CompositeValidator(); + $compositeValidator->addValidator(new RequiredFields()); + $compositeValidator->addValidator(new TestValidator()); + $compositeValidator->addValidator(new RequiredFields()); + $compositeValidator->addValidator(new TestValidator()); - $this->assertCount(4, $validatorList->getValidators()); - $this->assertCount(2, $validatorList->getValidatorsByType(RequiredFields::class)); + $this->assertCount(4, $compositeValidator->getValidators()); + $this->assertCount(2, $compositeValidator->getValidatorsByType(RequiredFields::class)); } public function testRemoveValidatorsByType(): void { - $validatorList = new ValidatorList(); - $validatorList->addValidator(new RequiredFields()); - $validatorList->addValidator(new TestValidator()); - $validatorList->addValidator(new RequiredFields()); - $validatorList->addValidator(new TestValidator()); + $compositeValidator = new CompositeValidator(); + $compositeValidator->addValidator(new RequiredFields()); + $compositeValidator->addValidator(new TestValidator()); + $compositeValidator->addValidator(new RequiredFields()); + $compositeValidator->addValidator(new TestValidator()); - $this->assertCount(4, $validatorList->getValidators()); + $this->assertCount(4, $compositeValidator->getValidators()); - $validatorList->removeValidatorsByType(RequiredFields::class); - $this->assertCount(2, $validatorList->getValidators()); + $compositeValidator->removeValidatorsByType(RequiredFields::class); + $this->assertCount(2, $compositeValidator->getValidators()); } public function testCanBeCached(): void { - $validatorList = new ValidatorList(); - $validatorList->addValidator(new RequiredFields()); + $compositeValidator = new CompositeValidator(); + $compositeValidator->addValidator(new RequiredFields()); - $this->assertTrue($validatorList->canBeCached()); + $this->assertTrue($compositeValidator->canBeCached()); - $validatorList = new ValidatorList(); - $validatorList->addValidator(new RequiredFields(['Foor'])); + $compositeValidator = new CompositeValidator(); + $compositeValidator->addValidator(new RequiredFields(['Foor'])); - $this->assertFalse($validatorList->canBeCached()); + $this->assertFalse($compositeValidator->canBeCached()); } public function testFieldIsRequired(): void { - $validatorList = new ValidatorList(); + $compositeValidator = new CompositeValidator(); $fieldNames = [ 'Title', @@ -124,12 +124,12 @@ class ValidatorListTest extends SapphireTest ] ); - $validatorList->addValidator($requiredFieldsFirst); - $validatorList->addValidator($requiredFieldsSecond); + $compositeValidator->addValidator($requiredFieldsFirst); + $compositeValidator->addValidator($requiredFieldsSecond); foreach ($fieldNames as $field) { $this->assertTrue( - $validatorList->fieldIsRequired($field), + $compositeValidator->fieldIsRequired($field), sprintf('Failed to find "%s" field in required list', $field) ); } @@ -137,10 +137,10 @@ class ValidatorListTest extends SapphireTest public function testValidate(): void { - $validatorList = new ValidatorList(); + $compositeValidator = new CompositeValidator(); // Add two separate validators, each with one required field - $validatorList->addValidator(new RequiredFields(['Foo'])); - $validatorList->addValidator(new RequiredFields(['Bar'])); + $compositeValidator->addValidator(new RequiredFields(['Foo'])); + $compositeValidator->addValidator(new RequiredFields(['Bar'])); // Setup a form with the fields/data we're testing (a form is a dependency for validation right now) // We'll add three empty fields, but only two of them should be required @@ -153,7 +153,7 @@ class ValidatorListTest extends SapphireTest $form = $this->getForm(array_keys($data)); $form->disableSecurityToken(); // Setup validator now that we've got our form - $form->setValidator($validatorList); + $form->setValidator($compositeValidator); // Put data into the form so the validator can pull it back out again $form->loadDataFrom($data); @@ -164,8 +164,8 @@ class ValidatorListTest extends SapphireTest public function testRemoveValidation(): void { - $validatorList = new ValidatorList(); - $validatorList->addValidator(new TestValidator()); + $compositeValidator = new CompositeValidator(); + $compositeValidator->addValidator(new TestValidator()); // Setup a form with the fields/data we're testing (a form is a dependency for validation right now) $data = [ @@ -175,7 +175,7 @@ class ValidatorListTest extends SapphireTest $form = $this->getForm(array_keys($data)); $form->disableSecurityToken(); // Setup validator now that we've got our form - $form->setValidator($validatorList); + $form->setValidator($compositeValidator); // Put data into the form so the validator can pull it back out again $form->loadDataFrom($data); @@ -185,7 +185,7 @@ class ValidatorListTest extends SapphireTest // Make sure it doesn't fail after removing validation AND has no errors (since calling validate should reset // errors) - $validatorList->removeValidation(); + $compositeValidator->removeValidation(); $result = $form->validationResult(); $this->assertTrue($result->isValid()); $this->assertEmpty($result->getMessages()); diff --git a/tests/php/Forms/ValidatorTest/TestValidatorList.php b/tests/php/Forms/ValidatorTest/TestCompositeValidator.php similarity index 70% rename from tests/php/Forms/ValidatorTest/TestValidatorList.php rename to tests/php/Forms/ValidatorTest/TestCompositeValidator.php index 7cb91e046..44a92a0df 100644 --- a/tests/php/Forms/ValidatorTest/TestValidatorList.php +++ b/tests/php/Forms/ValidatorTest/TestCompositeValidator.php @@ -4,14 +4,14 @@ namespace SilverStripe\Forms\Tests\ValidatorTest; use SilverStripe\Dev\TestOnly; use SilverStripe\Forms\Form; -use SilverStripe\Forms\ValidatorList; +use SilverStripe\Forms\CompositeValidator; /** - * Class TestValidatorList + * Class TestCompositeValidator * * @package SilverStripe\Forms\Tests\ValidatorTest */ -class TestValidatorList extends ValidatorList implements TestOnly +class TestCompositeValidator extends CompositeValidator implements TestOnly { /** * Allow us to access the form for test purposes.