Issue #2346373 by fago, sidharrell, rteijeiro, arlinsandbulte: Data reference validation constraints are applied wrong

core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/BundleConstraintValidator.php

@@ -7,7 +7,6 @@
 
 namespace Drupal\Core\Entity\Plugin\Validation\Constraint;
 
-use Drupal\Core\TypedData\TypedDataInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 
@@ -19,22 +18,11 @@ class BundleConstraintValidator extends ConstraintValidator {
   /**
    * {@inheritdoc}
    */
-  public function validate($entity_adapter, Constraint $constraint) {
-    if (!isset($entity_adapter)) {
+  public function validate($entity, Constraint $constraint) {
+    if (!isset($entity)) {
       return;
     }
 
-    // @todo The $entity_adapter parameter passed to this function should always
-    //   be a typed data object, but due to a bug, the unwrapped entity is
-    //   passed for the computed entity property of entity reference fields.
-    //   Remove this after fixing that in https://www.drupal.org/node/2346373.
-    if (!$entity_adapter instanceof TypedDataInterface) {
-      $entity = $entity_adapter;
-    }
-    else {
-      $entity = $entity_adapter->getValue();
-    }
-
     if (!in_array($entity->bundle(), $constraint->getBundleOption())) {
       $this->context->addViolation($constraint->message, array('%bundle' => implode(', ', $constraint->getBundleOption())));
     }

core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/EntityTypeConstraintValidator.php

@@ -7,7 +7,6 @@
 
 namespace Drupal\Core\Entity\Plugin\Validation\Constraint;
 
-use Drupal\Core\TypedData\TypedDataInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 
@@ -17,24 +16,13 @@ use Symfony\Component\Validator\ConstraintValidator;
 class EntityTypeConstraintValidator extends ConstraintValidator {
 
   /**
-   * Implements \Symfony\Component\Validator\ConstraintValidatorInterface::validate().
+   * {@inheritdoc}
    */
-  public function validate($entity_adapter, Constraint $constraint) {
-    if (!isset($entity_adapter)) {
+  public function validate($entity, Constraint $constraint) {
+    if (!isset($entity)) {
       return;
     }
 
-    // @todo The $entity_adapter parameter passed to this function should always
-    //   be a typed data object, but due to a bug, the unwrapped entity is
-    //   passed for the computed entity property of entity reference fields.
-    //   Remove this after fixing that in https://www.drupal.org/node/2346373.
-    if (!$entity_adapter instanceof TypedDataInterface) {
-      $entity = $entity_adapter;
-    }
-    else {
-      $entity = $entity_adapter->getValue();
-    }
-
     /** @var $entity \Drupal\Core\Entity\EntityInterface */
     if ($entity->getEntityTypeId() != $constraint->type) {
       $this->context->addViolation($constraint->message, array('%type' => $constraint->type));

core/lib/Drupal/Core/Field/Plugin/DataType/Deriver/FieldItemDeriver.php

@@ -79,6 +79,7 @@ class FieldItemDeriver implements ContainerDeriverInterface {
     foreach ($this->fieldTypePluginManager->getDefinitions() as $plugin_id => $definition) {
       $definition['definition_class'] = '\Drupal\Core\Field\TypedData\FieldItemDataDefinition';
       $definition['list_definition_class'] = '\Drupal\Core\Field\BaseFieldDefinition';
+      $definition['unwrap_for_canonical_representation'] = FALSE;
       $this->derivatives[$plugin_id] = $definition;
     }
     return $this->derivatives;

core/lib/Drupal/Core/Plugin/Context/Context.php

@@ -46,7 +46,7 @@ class Context extends ComponentContext implements ContextInterface {
       }
       return NULL;
     }
-    return $this->contextData->getValue();
+    return $this->typedDataManager->getCanonicalRepresentation($this->contextData);
   }
 
   /**

core/lib/Drupal/Core/TypedData/Annotation/DataType.php

@@ -106,4 +106,13 @@ class DataType extends Plugin {
    */
   public $constraints;
 
+  /**
+   * Whether the typed object wraps the canonical representation of the data.
+   *
+   * @var bool
+   *
+   * @see \Drupal\Core\TypedData\TypedDataManager::getCanonicalRepresentation()
+   */
+  public $unwrap_for_canonical_representation = TRUE;
+
 }

core/lib/Drupal/Core/TypedData/TypedDataManager.php

@@ -332,7 +332,7 @@ class TypedDataManager extends DefaultPluginManager {
   public function getValidator() {
     if (!isset($this->validator)) {
       $this->validator = Validation::createValidatorBuilder()
-        ->setMetadataFactory(new MetadataFactory())
+        ->setMetadataFactory(new MetadataFactory($this))
         ->setTranslator(new DrupalTranslator())
         ->setConstraintValidatorFactory(new ConstraintValidatorFactory($this->classResolver))
         ->setApiVersion(Validation::API_VERSION_2_4)
@@ -415,4 +415,44 @@ class TypedDataManager extends DefaultPluginManager {
     $this->prototypes = array();
   }
 
+  /**
+   * Gets the canonical representation of a TypedData object.
+   *
+   * The canonical representation is typically used when data is passed on to
+   * other code components. In many use cases, the TypedData object is mostly
+   * unified adapter wrapping a primary value (e.g. a string, an entity...)
+   * which is the canonical representation that consuming code like constraint
+   * validators are really interested in. For some APIs, though, the domain
+   * object (e.g. Field API's FieldItem and FieldItemList) directly implements
+   * TypedDataInterface, and the canonical representation is thus the data
+   * object itself.
+   *
+   * When a TypedData object gets validated, for example, its canonical
+   * representation is passed on to constraint validators, which thus receive
+   * an Entity unwrapped, but a FieldItem as is.
+   *
+   * Data types specify whether their data objects need unwrapping by using the
+   * 'unwrap_for_canonical_representation' property in the data definition
+   * (defaults to TRUE).
+   *
+   * @param \Drupal\Core\TypedData\TypedDataInterface $data
+   *   The data.
+   *
+   * @return mixed
+   *   The canonical representation of the passed data.
+   */
+  public function getCanonicalRepresentation(TypedDataInterface $data) {
+    $data_definition = $data->getDataDefinition();
+    // In case a list is passed, respect the 'wrapped' key of its data type.
+    if ($data_definition instanceof ListDataDefinitionInterface) {
+      $data_definition = $data_definition->getItemDefinition();
+    }
+    // Get the plugin definition of the used data type.
+    $type_definition = $this->getDefinition($data_definition->getDataType());
+    if (!empty($type_definition['unwrap_for_canonical_representation'])) {
+      return $data->getValue();
+    }
+    return $data;
+  }
+
 }

core/lib/Drupal/Core/TypedData/Validation/Metadata.php

@@ -8,6 +8,7 @@
 namespace Drupal\Core\TypedData\Validation;
 
 use Drupal\Core\TypedData\TypedDataInterface;
+use Drupal\Core\TypedData\TypedDataManager;
 use Symfony\Component\Validator\ValidationVisitorInterface;
 use Symfony\Component\Validator\PropertyMetadataInterface;
 
@@ -37,6 +38,13 @@ class Metadata implements PropertyMetadataInterface {
    */
   protected $factory;
 
+  /**
+   * The typed data manager.
+   *
+   * @var \Drupal\Core\TypedData\TypedDataManager
+   */
+  protected $typedDataManager;
+
   /**
    * Constructs the object.
    *
@@ -47,11 +55,14 @@ class Metadata implements PropertyMetadataInterface {
    *   the data is the root of the typed data tree.
    * @param \Drupal\Core\TypedData\Validation\MetadataFactory $factory
    *   The factory to use for instantiating property metadata.
+   * @param \Drupal\Core\TypedData\TypedDataManager $typed_data_manager
+   *   The typed data manager.
    */
-  public function __construct(TypedDataInterface $typed_data, $name = '', MetadataFactory $factory) {
+  public function __construct(TypedDataInterface $typed_data, $name = '', MetadataFactory $factory, TypedDataManager $typed_data_manager) {
     $this->typedData = $typed_data;
     $this->name = $name;
     $this->factory = $factory;
+    $this->typedDataManager = $typed_data_manager;
   }
 
   /**
@@ -62,7 +73,7 @@ class Metadata implements PropertyMetadataInterface {
     // @todo: Do we have to care about groups? Symfony class metadata has
     // $propagatedGroup.
 
-    $visitor->visit($this, $typed_data->getValue(), $group, $propertyPath);
+    $visitor->visit($this, $this->typedDataManager->getCanonicalRepresentation($typed_data), $group, $propertyPath);
   }
 
   /**
@@ -89,7 +100,7 @@ class Metadata implements PropertyMetadataInterface {
    * @return mixed The value of the property.
    */
   public function getPropertyValue($container) {
-    return $this->typedData->getValue();
+    return $this->typedDataManager->getCanonicalRepresentation($this->typedData);
   }
 
   /**

core/lib/Drupal/Core/TypedData/Validation/MetadataFactory.php

@@ -10,6 +10,7 @@ namespace Drupal\Core\TypedData\Validation;
 use Drupal\Core\TypedData\ComplexDataInterface;
 use Drupal\Core\TypedData\ListInterface;
 use Drupal\Core\TypedData\TypedDataInterface;
+use Drupal\Core\TypedData\TypedDataManager;
 use Symfony\Component\Validator\MetadataFactoryInterface;
 
 /**
@@ -18,7 +19,24 @@ use Symfony\Component\Validator\MetadataFactoryInterface;
 class MetadataFactory implements MetadataFactoryInterface {
 
   /**
-   * Implements MetadataFactoryInterface::getMetadataFor().
+   * The typed data manager.
+   *
+   * @var \Drupal\Core\TypedData\TypedDataManager
+   */
+  protected $typedDataManager;
+
+  /**
+   * Constructs the object.
+   *
+   * @param \Drupal\Core\TypedData\TypedDataManager $typed_data_manager
+   *   The typed data manager.
+   */
+  public function __construct(TypedDataManager $typed_data_manager) {
+    $this->typedDataManager = $typed_data_manager;
+  }
+
+  /**
+   * {@inheritdoc}
    *
    * @param \Drupal\Core\TypedData\TypedDataInterface $typed_data
    *   Some typed data object containing the value to validate.
@@ -32,7 +50,7 @@ class MetadataFactory implements MetadataFactoryInterface {
     }
     $is_container = $typed_data instanceof ComplexDataInterface || $typed_data instanceof ListInterface;
     $class = '\Drupal\Core\TypedData\Validation\\' . ($is_container ? 'PropertyContainerMetadata' : 'Metadata');
-    return new $class($typed_data, $name, $this);
+    return new $class($typed_data, $name, $this, $this->typedDataManager);
   }
 
   /**

core/lib/Drupal/Core/TypedData/Validation/PropertyContainerMetadata.php

@@ -25,12 +25,16 @@ class PropertyContainerMetadata extends Metadata implements PropertyMetadataCont
     // if the data structure is empty. That way existing NotNull or NotBlank
     // constraints work as expected.
     if ($typed_data->isEmpty()) {
-      $typed_data = NULL;
+      $data = NULL;
     }
-    $visitor->visit($this, $typed_data, $group, $propertyPath);
+    else {
+      $data = $this->typedDataManager->getCanonicalRepresentation($typed_data);
+    }
+    $visitor->visit($this, $data, $group, $propertyPath);
     $pathPrefix = isset($propertyPath) && $propertyPath !== '' ? $propertyPath . '.' : '';
 
-    if ($typed_data) {
+    // Only continue validating if the data is not empty.
+    if ($data) {
       foreach ($typed_data as $name => $data) {
         $metadata = $this->factory->getMetadataFor($data, $name);
         $metadata->accept($visitor, $data, $group, $pathPrefix . $name);
@@ -56,10 +60,10 @@ class PropertyContainerMetadata extends Metadata implements PropertyMetadataCont
    */
   public function getPropertyMetadata($property_name) {
     if ($this->typedData instanceof ListInterface) {
-      return array(new Metadata($this->typedData[$property_name], $property_name));
+      return array(new Metadata($this->typedData[$property_name], $property_name, $this->factory, $this->typedDataManager));
     }
     elseif ($this->typedData instanceof ComplexDataInterface) {
-      return array(new Metadata($this->typedData->get($property_name), $property_name));
+      return array(new Metadata($this->typedData->get($property_name), $property_name, $this->factory, $this->typedDataManager));
     }
     else {
       throw new \LogicException("There are no known properties.");

core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ComplexDataConstraintValidator.php

@@ -8,6 +8,7 @@
 namespace Drupal\Core\Validation\Plugin\Validation\Constraint;
 
 use Drupal\Core\TypedData\ComplexDataInterface;
+use Drupal\Core\TypedData\TypedDataInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
@@ -25,6 +26,10 @@ class ComplexDataConstraintValidator extends ConstraintValidator {
       return;
     }
 
+    // If un-wrapped data has been passed, fetch the typed data object first.
+    if (!$value instanceof TypedDataInterface) {
+      $value = $this->context->getMetadata()->getTypedData();
+    }
     if (!$value instanceof ComplexDataInterface) {
       throw new UnexpectedTypeException($value, 'ComplexData');
     }