diff --git a/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraint.php b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraint.php index d982193750e9d85a39d43f11d6ff563886b34e18..bf8658aa6c08df381351b9d82d33ddf03e4a132a 100644 --- a/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraint.php +++ b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraint.php @@ -28,4 +28,11 @@ class ValidReferenceConstraint extends Constraint { */ public $message = 'The referenced entity (%type: %id) does not exist.'; + /** + * Validation message when the target_id is empty. + * + * @var string + */ + public $nullMessage = 'This value should not be null.'; + } diff --git a/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php index 6a8f7cde315677cdfd505e55b999efebd2eb2d1a..8f704c1c7b2de54cea7260fdb67331449e97debf 100644 --- a/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php +++ b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php @@ -19,10 +19,18 @@ class ValidReferenceConstraintValidator extends ConstraintValidator { * {@inheritdoc} */ public function validate($value, Constraint $constraint) { - /* @var \Drupal\Core\Field\FieldItemInterface $value */ + /** @var \Drupal\Core\Field\FieldItemInterface $value */ + /** @var ValidReferenceConstraint $constraint */ if (!isset($value)) { return; } + // We don't use a regular NotNull constraint for the target_id property as + // a NULL value is valid if the entity property contains an unsaved entity. + // @see \Drupal\Core\TypedData\DataReferenceTargetDefinition::getConstraints + if (!$value->isEmpty() && $value->target_id === NULL && !$value->entity->isNew()) { + $this->context->addViolation($constraint->nullMessage); + return; + } $id = $value->get('target_id')->getValue(); // '0' or NULL are considered valid empty references. if (empty($id)) { diff --git a/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php b/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php index 3d735dc367f7ed8505d973554ffffd8402ef7d36..4217b2ea9fb24922c96fc680bcf6fec2fd9b6e1c 100644 --- a/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php +++ b/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php @@ -27,12 +27,12 @@ public function referencedEntities() { // "autocreate" entities that are already populated in $item->entity. $target_entities = $ids = array(); foreach ($this->list as $delta => $item) { - if ($item->hasNewEntity()) { - $target_entities[$delta] = $item->entity; - } - elseif ($item->target_id !== NULL) { + if ($item->target_id !== NULL) { $ids[$delta] = $item->target_id; } + elseif ($item->hasNewEntity()) { + $target_entities[$delta] = $item->entity; + } } // Load and add the existing entities. diff --git a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php index bb5e762530ab6cc560f614b26b5afc9c9e4998df..18c050646f361ba84d70c3d05b2a306073e6cfd4 100644 --- a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php @@ -14,8 +14,8 @@ use Drupal\Core\Field\FieldItemBase; use Drupal\Core\Field\FieldStorageDefinitionInterface; use Drupal\Core\StringTranslation\TranslatableString; -use Drupal\Core\TypedData\DataDefinition; use Drupal\Core\TypedData\DataReferenceDefinition; +use Drupal\Core\TypedData\DataReferenceTargetDefinition; /** * Defines the 'entity_reference' entity field type. @@ -39,13 +39,6 @@ */ class EntityReferenceItem extends FieldItemBase { - /** - * Marker value to identify a newly created entity. - * - * @var int - */ - protected static $NEW_ENTITY_MARKER = -1; - /** * {@inheritdoc} */ @@ -82,12 +75,12 @@ public static function propertyDefinitions(FieldStorageDefinitionInterface $fiel } if ($target_id_data_type === 'integer') { - $target_id_definition = DataDefinition::create('integer') + $target_id_definition = DataReferenceTargetDefinition::create('integer') ->setLabel(new TranslatableString('@label ID', ['@label' => $target_type_info->getLabel()])) ->setSetting('unsigned', TRUE); } else { - $target_id_definition = DataDefinition::create('string') + $target_id_definition = DataReferenceTargetDefinition::create('string') ->setLabel(new TranslatableString('@label ID', ['@label' => $target_type_info->getLabel()])); } $target_id_definition->setRequired(TRUE); @@ -171,19 +164,23 @@ public function setValue($values, $notify = TRUE) { parent::setValue($values, FALSE); // Support setting the field item with only one property, but make sure // values stay in sync if only property is passed. - if (isset($values['target_id']) && !isset($values['entity'])) { + // NULL is a valid value, so we use array_key_exists(). + if (is_array($values) && array_key_exists('target_id', $values) && !isset($values['entity'])) { $this->onChange('target_id', FALSE); } - elseif (!isset($values['target_id']) && isset($values['entity'])) { + elseif (is_array($values) && !array_key_exists('target_id', $values) && isset($values['entity'])) { $this->onChange('entity', FALSE); } - elseif (isset($values['target_id']) && isset($values['entity'])) { + elseif (is_array($values) && array_key_exists('target_id', $values) && isset($values['entity'])) { // If both properties are passed, verify the passed values match. The // only exception we allow is when we have a new entity: in this case // its actual id and target_id will be different, due to the new entity // marker. $entity_id = $this->get('entity')->getTargetIdentifier(); - if ($entity_id != $values['target_id'] && ($values['target_id'] != static::$NEW_ENTITY_MARKER || !$this->entity->isNew())) { + // If the entity has been saved and we're trying to set both the + // target_id and the entity values with a non-null target ID, then the + // value for target_id should match the ID of the entity value. + if (!$this->entity->isNew() && $values['target_id'] !== NULL && ($entity_id !== $values['target_id'])) { throw new \InvalidArgumentException('The target id and entity passed to the entity reference item do not match.'); } } @@ -216,10 +213,10 @@ public function onChange($property_name, $notify = TRUE) { // Make sure that the target ID and the target property stay in sync. if ($property_name == 'entity') { $property = $this->get('entity'); - $target_id = $property->isTargetNew() ? static::$NEW_ENTITY_MARKER : $property->getTargetIdentifier(); + $target_id = $property->isTargetNew() ? NULL : $property->getTargetIdentifier(); $this->writePropertyValue('target_id', $target_id); } - elseif ($property_name == 'target_id' && $this->target_id != static::$NEW_ENTITY_MARKER) { + elseif ($property_name == 'target_id') { $this->writePropertyValue('entity', $this->target_id); } parent::onChange($property_name, $notify); @@ -252,6 +249,9 @@ public function preSave() { // react properly. $this->target_id = $this->entity->id(); } + if (!$this->isEmpty() && $this->target_id === NULL) { + $this->target_id = $this->entity->id(); + } } /** @@ -277,7 +277,7 @@ public static function generateSampleValue(FieldDefinitionInterface $field_defin * TRUE if the item holds an unsaved entity. */ public function hasNewEntity() { - return $this->target_id === static::$NEW_ENTITY_MARKER; + return !$this->isEmpty() && $this->target_id === NULL && $this->entity->isNew(); } /** diff --git a/core/lib/Drupal/Core/TypedData/DataReferenceTargetDefinition.php b/core/lib/Drupal/Core/TypedData/DataReferenceTargetDefinition.php new file mode 100644 index 0000000000000000000000000000000000000000..b3609861cb566cca85726b07512301e4eb3d3fee --- /dev/null +++ b/core/lib/Drupal/Core/TypedData/DataReferenceTargetDefinition.php @@ -0,0 +1,38 @@ +<?php +/** + * @file + * Contains \Drupal\Core\TypedData\DataReferenceTargetDefinition. + */ + +namespace Drupal\Core\TypedData; + +/** + * A typed data definition class for the entity reference target_id property. + * + * The target_id property differs from other data definitions in that it is + * required at the storage level, but not at the validation level. This is + * because its value can be set just-in-time using the preSave() method. + * + * Validation for the target_id property is provided by the 'ValidReference' + * validation constraint. + * + * @see \Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::preSave() + */ +class DataReferenceTargetDefinition extends DataDefinition { + + /** + * {@inheritdoc} + */ + public function getConstraints() { + $constraints = parent::getConstraints(); + // If this data definition is marked as required for the sake of schema + // definitions, we don't enforce it using the NotNull constraint. Instead + // \Drupal\Core\Field\EntityReferenceItem is validated by the + // 'ValidReference' constraint that operates at the field-item level. This + // constraint takes into consideration that the target_id property can + // be derived from the entity property. + unset($constraints['NotNull']); + return $constraints; + } + +} diff --git a/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php b/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php index a05c56d131c9b49a513ca5f76c717155fa8ca29b..8d492e5c98e1c1fa6916f7cb6557eb7d01e2e79c 100644 --- a/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php +++ b/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php @@ -12,8 +12,8 @@ use Drupal\Core\Field\FieldItemInterface; use Drupal\Core\StringTranslation\TranslatableString; use Drupal\Core\Language\LanguageInterface; -use Drupal\entity_reference\Tests\EntityReferenceTestTrait; use Drupal\entity_test\Entity\EntityTest; +use Drupal\entity_reference\Tests\EntityReferenceTestTrait; use Drupal\entity_test\Entity\EntityTestStringId; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; @@ -236,6 +236,30 @@ public function testConfigEntityReferenceItem() { $entity->save(); } + /** + * Tests entity auto create. + */ + public function testEntityAutoCreate() { + // The term entity is unsaved here. + $term = Term::create(array( + 'name' => $this->randomMachineName(), + 'vid' => $this->term->bundle(), + 'langcode' => LanguageInterface::LANGCODE_NOT_SPECIFIED, + )); + $entity = EntityTest::create(); + // Now assign the unsaved term to the field. + $entity->field_test_taxonomy_term->entity = $term; + $entity->name->value = $this->randomMachineName(); + // This is equal to storing an entity to tempstore or cache and retrieving + // it back. An example for this is node preview. + $entity = serialize($entity); + $entity = unserialize($entity); + // And then the entity. + $entity->save(); + $term = \Drupal::entityManager()->loadEntityByUuid($term->getEntityTypeId(), $term->uuid()); + $this->assertEqual($entity->field_test_taxonomy_term->entity->id(), $term->id()); + } + /** * Test saving order sequence doesn't matter. */ @@ -250,6 +274,12 @@ public function testEntitySaveOrder() { // Now assign the unsaved term to the field. $entity->field_test_taxonomy_term->entity = $term; $entity->name->value = $this->randomMachineName(); + // Now get the field value. + $value = $entity->get('field_test_taxonomy_term'); + $this->assertTrue(empty($value['target_id'])); + $this->assertNull($entity->field_test_taxonomy_term->target_id); + // And then set it. + $entity->field_test_taxonomy_term = $value; // Now save the term. $term->save(); // And then the entity. @@ -289,4 +319,41 @@ public function testSelectionHandlerSettings() { $this->assertTrue($field->getSetting('handler') == 'views'); } + /** + * Tests validation constraint. + */ + public function testValidation() { + // The term entity is unsaved here. + $term = Term::create(array( + 'name' => $this->randomMachineName(), + 'vid' => $this->term->bundle(), + 'langcode' => LanguageInterface::LANGCODE_NOT_SPECIFIED, + )); + $entity = EntityTest::create([ + 'field_test_taxonomy_term' => [ + 'entity' => $term, + 'target_id' => NULL, + ], + ]); + $errors = $entity->validate(); + // Using target_id of NULL is valid with an unsaved entity. + $this->assertEqual(0, count($errors)); + // Using target_id of NULL is not valid with a saved entity. + $term->save(); + $entity = EntityTest::create([ + 'field_test_taxonomy_term' => [ + 'entity' => $term, + 'target_id' => NULL, + ], + ]); + $errors = $entity->validate(); + $this->assertEqual(1, count($errors)); + $this->assertEqual($errors[0]->getMessage(), 'This value should not be null.'); + $this->assertEqual($errors[0]->getPropertyPath(), 'field_test_taxonomy_term.0'); + // This should rectify the issue, favoring the entity over the target_id. + $entity->save(); + $errors = $entity->validate(); + $this->assertEqual(0, count($errors)); + } + } diff --git a/core/modules/system/src/Tests/Entity/EntityReferenceFieldTest.php b/core/modules/system/src/Tests/Entity/EntityReferenceFieldTest.php index 4f66b4edd38b2189918e22e15e9306c350b5901d..f74410f68ddab37cbc8da74e4c2b71d59f55be7d 100644 --- a/core/modules/system/src/Tests/Entity/EntityReferenceFieldTest.php +++ b/core/modules/system/src/Tests/Entity/EntityReferenceFieldTest.php @@ -238,16 +238,15 @@ function testAutocreateApi() { }); $this->assertUserAutocreate($entity, function(EntityInterface $entity, UserInterface $user) { $entity->user_id[0]->get('entity')->setValue($user); - $entity->user_id[0]->get('target_id')->setValue(-1); }); $this->assertUserAutocreate($entity, function(EntityInterface $entity, UserInterface $user) { - $entity->user_id->setValue(array('entity' => $user, 'target_id' => -1)); + $entity->user_id->setValue(array('entity' => $user, 'target_id' => NULL)); }); try { $message = 'Setting both the entity and an invalid target_id property fails.'; $this->assertUserAutocreate($entity, function(EntityInterface $entity, UserInterface $user) { $user->save(); - $entity->user_id->setValue(array('entity' => $user, 'target_id' => -1)); + $entity->user_id->setValue(array('entity' => $user, 'target_id' => $this->generateRandomEntityId())); }); $this->fail($message); } @@ -273,16 +272,15 @@ function testAutocreateApi() { }); $this->assertUserRoleAutocreate($entity, function(EntityInterface $entity, RoleInterface $role) { $entity->user_role[0]->get('entity')->setValue($role); - $entity->user_role[0]->get('target_id')->setValue(-1); }); $this->assertUserRoleAutocreate($entity, function(EntityInterface $entity, RoleInterface $role) { - $entity->user_role->setValue(array('entity' => $role, 'target_id' => -1)); + $entity->user_role->setValue(array('entity' => $role, 'target_id' => NULL)); }); try { $message = 'Setting both the entity and an invalid target_id property fails.'; $this->assertUserRoleAutocreate($entity, function(EntityInterface $entity, RoleInterface $role) { $role->save(); - $entity->user_role->setValue(array('entity' => $role, 'target_id' => -1)); + $entity->user_role->setValue(array('entity' => $role, 'target_id' => $this->generateRandomEntityId(TRUE))); }); $this->fail($message); }