From c9dd655885e4be7e3354389bef8f9a102f94c349 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Fri, 8 Mar 2019 10:07:47 +1000
Subject: [PATCH] =?UTF-8?q?Issue=20#2937542=20by=20alexpott,=20mikelutz,?=
 =?UTF-8?q?=20larowlan,=20dawehner,=20Mile23,=20catch,=20G=C3=A1bor=20Hojt?=
 =?UTF-8?q?sy,=20Chi:=20Not=20setting=20the=20strict=20option=20of=20the?=
 =?UTF-8?q?=20Choice=20constraint=20to=20true=20is=20deprecated=20since=20?=
 =?UTF-8?q?Symfony=203.4=20and=20will=20throw=20an=20exception=20in=204.0?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../Constraint/AllowedValuesConstraint.php    |  1 +
 .../AllowedValuesConstraintValidator.php      | 36 +++++++++-
 .../src/Functional/OptionsWidgetsTest.php     | 70 +++++++++++++++++++
 .../AllowedValuesConstraintValidatorTest.php  | 61 ++++++++++++++++
 .../Listeners/DeprecationListenerTrait.php    |  1 -
 5 files changed, 167 insertions(+), 2 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 d20f54a723e9..798805e7def7 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 @@
  */
 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 87a496743395..21be12d0e0ee 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\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,6 +49,7 @@ public function __construct(AccountInterface $current_user) {
    */
   public function validate($value, Constraint $constraint) {
     $typed_data = $this->getTypedData();
+
     if ($typed_data instanceof OptionsProviderInterface) {
       $allowed_values = $typed_data->getSettableValues($this->currentUser);
       $constraint->choices = $allowed_values;
@@ -57,7 +60,8 @@ public function validate($value, Constraint $constraint) {
         if (!isset($name)) {
           throw new \LogicException('Cannot validate allowed values for complex data without a main property.');
         }
-        $value = $typed_data->get($name)->getValue();
+        $typed_data = $typed_data->get($name);
+        $value = $typed_data->getValue();
       }
     }
 
@@ -69,6 +73,36 @@ public function validate($value, Constraint $constraint) {
       return;
     }
 
+    // Get the value with the proper datatype in order to make strict
+    // comparisons using in_array().
+    if (!($typed_data instanceof PrimitiveInterface)) {
+      throw new \LogicException('The data type must be a PrimitiveInterface at this point.');
+    }
+    $value = $typed_data->getCastedValue();
+
+    // 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);
+      $constraint->choices = $allowed_values;
+      // parent::validate() does not need to invoke the callback again.
+      $constraint->callback = NULL;
+    }
+
+    // Force the choices to be the same type as the value.
+    $type = gettype($value);
+    foreach ($constraint->choices as &$choice) {
+      settype($choice, $type);
+    }
+
     parent::validate($value, $constraint);
   }
 
diff --git a/core/modules/options/tests/src/Functional/OptionsWidgetsTest.php b/core/modules/options/tests/src/Functional/OptionsWidgetsTest.php
index 5f15ac9cbee0..8363467c9787 100644
--- a/core/modules/options/tests/src/Functional/OptionsWidgetsTest.php
+++ b/core/modules/options/tests/src/Functional/OptionsWidgetsTest.php
@@ -35,6 +35,13 @@ class OptionsWidgetsTest extends FieldTestBase {
    */
   protected $card2;
 
+  /**
+   * A field storage with float values to use in this test class.
+   *
+   * @var \Drupal\field\Entity\FieldStorageConfig
+   */
+  protected $float;
+
   protected function setUp() {
     parent::setUp();
 
@@ -76,6 +83,22 @@ protected function setUp() {
     ]);
     $this->card2->save();
 
+    // Field storage with list of float values.
+    $this->float = FieldStorageConfig::create([
+      'field_name' => 'float',
+      'entity_type' => 'entity_test',
+      'type' => 'list_float',
+      'cardinality' => 1,
+      'settings' => [
+        'allowed_values' => [
+          '0.0' => '0.0',
+          '1.5' => '1.5',
+          '2.0' => '2.0',
+        ],
+      ],
+    ]);
+    $this->float->save();
+
     // Create a web user.
     $this->drupalLogin($this->drupalCreateUser(['view test entity', 'administer entity_test content']));
   }
@@ -447,6 +470,53 @@ public function testSelectListMultiple() {
     $this->assertFieldValues($entity_init, 'card_2', []);
   }
 
+  /**
+   * Tests the 'options_select' widget (float values).
+   */
+  public function testSelectListFloat() {
+
+    // Create an instance of the 'float value' field.
+    $field = FieldConfig::create([
+      'field_storage' => $this->float,
+      'bundle' => 'entity_test',
+      'required' => TRUE,
+    ]);
+    $field->save();
+
+    $this->container
+      ->get('entity_type.manager')
+      ->getStorage('entity_form_display')
+      ->load('entity_test.entity_test.default')
+      ->setComponent($this->float->getName(), ['type' => 'options_select'])
+      ->save();
+
+    // Create an entity.
+    $entity = EntityTest::create([
+      'user_id' => 1,
+      'name' => $this->randomMachineName(),
+    ]);
+    $entity->save();
+
+    // Display form.
+    $this->drupalGet('entity_test/manage/' . $entity->id() . '/edit');
+
+    // With no field data, nothing is selected.
+    $this->assertFalse($this->assertSession()->optionExists('float', 0)->isSelected());
+    $this->assertFalse($this->assertSession()->optionExists('float', 1.5)->isSelected());
+    $this->assertFalse($this->assertSession()->optionExists('float', 2)->isSelected());
+
+    // Submit form.
+    $edit = ['float' => 1.5];
+    $this->drupalPostForm(NULL, $edit, t('Save'));
+    $this->assertFieldValues($entity, 'float', [1.5]);
+
+    // Display form: check that the right options are selected.
+    $this->drupalGet('entity_test/manage/' . $entity->id() . '/edit');
+    $this->assertFalse($this->assertSession()->optionExists('float', 0)->isSelected());
+    $this->assertTrue($this->assertSession()->optionExists('float', 1.5)->isSelected());
+    $this->assertFalse($this->assertSession()->optionExists('float', 2)->isSelected());
+  }
+
   /**
    * Tests the 'options_select' and 'options_button' widget for empty value.
    */
diff --git a/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php b/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php
index 05a36ba2e96b..5dcbfccddcd9 100644
--- a/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php
+++ b/core/tests/Drupal/KernelTests/Core/TypedData/AllowedValuesConstraintValidatorTest.php
@@ -4,6 +4,7 @@
 
 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,66 @@ public function testValidation() {
     $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.');
+
+    $typed_data = $this->typedData->create($definition, 0);
+    $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 cdec09340d35..c355f57ed61e 100644
--- a/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -126,7 +126,6 @@ public static function getSkippedDeprecations() {
       // 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.',
     ];
   }
 
-- 
GitLab