diff --git a/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php b/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php index ee45c7d..6779542 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. @@ -117,4 +117,15 @@ public function defaultValuesFormSubmit(array $element, array &$form, FormStateI return $default_value; } + /** + * {@inheritdoc} + */ + public function getConstraints() { + $constraints = parent::getConstraints(); + $constraints[] = \Drupal::typedDataManager() + ->getValidationConstraintManager() + ->create('EntityReferenceNotNull', []); + return $constraints; + } + } 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 fa0b4a0..e4485e0 100644 --- a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php @@ -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} */ public static function defaultStorageSettings() { @@ -170,19 +163,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.'); } } @@ -215,10 +212,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); @@ -229,10 +226,7 @@ public function onChange($property_name, $notify = TRUE) { */ public function isEmpty() { // Avoid loading the entity by first checking the 'target_id'. - if ($this->target_id !== NULL) { - return FALSE; - } - if ($this->entity && $this->entity instanceof EntityInterface) { + if ($this->target_id !== NULL || (($entity = $this->entity) && $entity instanceof EntityInterface)) { return FALSE; } return TRUE; @@ -251,6 +245,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(); + } } /** @@ -276,7 +273,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/Validation/Plugin/Validation/Constraint/EntityReferenceNotNullConstraint.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EntityReferenceNotNullConstraint.php new file mode 100644 index 0000000..38d3927 --- /dev/null +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EntityReferenceNotNullConstraint.php @@ -0,0 +1,19 @@ +isEmpty()) { + continue; + } + if ($item->target_id === NULL && !$item->entity->isNew()) { + $this->context->addViolation($constraint->message); + return; + } + } + } + +} diff --git a/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php b/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php index 4601c34..dbc9037 100644 --- a/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php +++ b/core/modules/field/src/Tests/EntityReference/EntityReferenceItemTest.php @@ -11,8 +11,8 @@ use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Field\FieldItemInterface; 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; @@ -228,6 +228,30 @@ public function testConfigEntityReferenceItem() { } /** + * 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. */ public function testEntitySaveOrder() { @@ -242,6 +266,12 @@ public function testEntitySaveOrder() { $entity->field_test_taxonomy_term->entity = $term; $entity->name->value = $this->randomMachineName(); // Now save the term. + // 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; $term->save(); // And then the entity. $entity->save(); @@ -280,4 +310,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'); + // 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 14a67ef..53de70a 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); }