From 030a9cefc24451d67826b3fa3443e659d71bdf3e Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Thu, 31 Jan 2019 20:42:39 +1000 Subject: [PATCH] Issue #2937542 by alexpott, dawehner, Mile23, catch, mikelutz: Not setting the strict option of the Choice constraint to true is deprecated since Symfony 3.4 and will throw an exception in 4.0 --- .../Constraint/AllowedValuesConstraint.php | 1 + .../AllowedValuesConstraintValidator.php | 37 +++++++++++- .../AllowedValuesConstraintValidatorTest.php | 57 +++++++++++++++++++ .../Listeners/DeprecationListenerTrait.php | 1 - 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraint.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraint.php index d20f54a723e..798805e7def 100644 --- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraint.php +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraint.php @@ -16,6 +16,7 @@ use Symfony\Component\Validator\Constraints\Choice; */ class AllowedValuesConstraint extends Choice { + public $strict = TRUE; public $minMessage = 'You must select at least %limit choice.|You must select at least %limit choices.'; public $maxMessage = 'You must select at most %limit choice.|You must select at most %limit choices.'; diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraintValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraintValidator.php index 87a49674339..637d1e9196b 100644 --- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraintValidator.php +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraintValidator.php @@ -6,10 +6,12 @@ use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\TypedData\OptionsProviderInterface; use Drupal\Core\TypedData\ComplexDataInterface; +use Drupal\Core\TypedData\PrimitiveInterface; use Drupal\Core\TypedData\Validation\TypedDataAwareValidatorTrait; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\ChoiceValidator; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; /** * Validates the AllowedValues constraint. @@ -47,9 +49,24 @@ class AllowedValuesConstraintValidator extends ChoiceValidator implements Contai */ public function validate($value, Constraint $constraint) { $typed_data = $this->getTypedData(); - if ($typed_data instanceof OptionsProviderInterface) { + + // In a better world where typed data just returns typed values, we could + // set a constraint callback to use the OptionsProviderInterface. + // This is not possible right now though because we do the typecasting + // further down. + if ($constraint->callback) { + if (!\is_callable($choices = [$this->context->getObject(), $constraint->callback]) + && !\is_callable($choices = [$this->context->getClassName(), $constraint->callback]) + && !\is_callable($choices = $constraint->callback) + ) { + throw new ConstraintDefinitionException('The AllowedValuesConstraint constraint expects a valid callback'); + } + $allowed_values = \call_user_func($choices); + // parent::validate() does not need to invoke the callback again. + $constraint->callback = NULL; + } + elseif ($typed_data instanceof OptionsProviderInterface) { $allowed_values = $typed_data->getSettableValues($this->currentUser); - $constraint->choices = $allowed_values; // If the data is complex, we have to validate its main property. if ($typed_data instanceof ComplexDataInterface) { @@ -69,6 +86,22 @@ class AllowedValuesConstraintValidator extends ChoiceValidator implements Contai return; } + if (isset($allowed_values)) { + $constraint->choices = $allowed_values; + // Make the types match for strict checking. We can't use typed data here + // because types are not enforced everywhere. For example, + // \Drupal\Core\Field\Plugin\Field\FieldType\BooleanItem::getSettableValues() + // uses 0 and 1 to represent possible Boolean values whilst + // \Drupal\Core\TypedData\Plugin\DataType\BooleanData::getCastedValue() + // will return a proper typed Boolean. Therefore assume that + // $allowed_values contains values of the same type and cast $value to + // match. + settype($value, gettype(reset($allowed_values))); + } + elseif ($typed_data instanceof PrimitiveInterface) { + $value = $typed_data->getCastedValue(); + } + parent::validate($value, $constraint); } diff --git a/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php b/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php index 05a36ba2e96..6a6e85c3043 100644 --- a/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php +++ b/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php @@ -4,6 +4,7 @@ namespace Drupal\KernelTests\Core\TypedData; use Drupal\Core\TypedData\DataDefinition; use Drupal\KernelTests\KernelTestBase; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; /** * Tests AllowedValues validation constraint with both valid and invalid values. @@ -49,6 +50,62 @@ class AllowedValuesConstraintValidatorTest extends KernelTestBase { $this->assertEqual($violation->getMessage(), t('The value you selected is not a valid choice.'), 'The message for invalid value is correct.'); $this->assertEqual($violation->getRoot(), $typed_data, 'Violation root is correct.'); $this->assertEqual($violation->getInvalidValue(), 4, 'The invalid value is set correctly in the violation.'); + + // Test the validation when a value of an incorrect type is passed. + $typed_data = $this->typedData->create($definition, '1'); + $violations = $typed_data->validate(); + $this->assertEquals(0, $violations->count(), 'Value is coerced to the correct type and is valid.'); + } + + /** + * Tests the AllowedValuesConstraintValidator with callbacks. + */ + public function testValidationCallback() { + // Create a definition that specifies some AllowedValues and a callback. + // This tests that callbacks have a higher priority than a supplied list of + // values and can be used to coerce the value to the correct type. + $definition = DataDefinition::create('string') + ->addConstraint('AllowedValues', ['choices' => [1, 2, 3], 'callback' => [static::class, 'allowedValueCallback']]); + $typed_data = $this->typedData->create($definition, 'a'); + $violations = $typed_data->validate(); + $this->assertEquals(0, $violations->count(), 'Validation passed for correct value.'); + + $typed_data = $this->typedData->create($definition, 1); + $violations = $typed_data->validate(); + $this->assertEquals(0, $violations->count(), 'Validation passed for value that will be cast to the correct type.'); + + $typed_data = $this->typedData->create($definition, 2); + $violations = $typed_data->validate(); + $this->assertEquals(1, $violations->count(), 'Validation failed for incorrect value.'); + + $typed_data = $this->typedData->create($definition, 'd'); + $violations = $typed_data->validate(); + $this->assertEquals(1, $violations->count(), 'Validation failed for incorrect value.'); + } + + /** + * An AllowedValueConstraint callback. + * + * @return string[] + * A list of allowed values. + */ + public static function allowedValueCallback() { + return ['a', 'b', 'c', '1']; + } + + /** + * Tests the AllowedValuesConstraintValidator with an invalid callback. + */ + public function testValidationCallbackException() { + // Create a definition that specifies some AllowedValues and a callback. + // This tests that callbacks have a higher priority than a supplied list of + // values and can be used to coerce the value to the correct type. + $definition = DataDefinition::create('string') + ->addConstraint('AllowedValues', ['choices' => [1, 2, 3], 'callback' => [static::class, 'doesNotExist']]); + $typed_data = $this->typedData->create($definition, 1); + + $this->setExpectedException(ConstraintDefinitionException::class, 'The AllowedValuesConstraint constraint expects a valid callback'); + $typed_data->validate(); } } diff --git a/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php index cdec09340d3..c355f57ed61 100644 --- a/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php +++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php @@ -126,7 +126,6 @@ trait DeprecationListenerTrait { // is a Windows only deprecation. Remove when core no longer uses // WinCacheClassLoader in \Drupal\Core\DrupalKernel::initializeSettings(). 'The Symfony\Component\ClassLoader\WinCacheClassLoader class is deprecated since Symfony 3.3 and will be removed in 4.0. Use `composer install --apcu-autoloader` instead.', - 'Not setting the strict option of the Choice constraint to true is deprecated since Symfony 3.4 and will throw an exception in 4.0.', ]; }