Commit 9f04e89b authored by alexpott's avatar alexpott

Issue #2137309 by fago: Typed data does not handle set() and onChange() consistently

parent b665ec3d
...@@ -88,7 +88,7 @@ public function set($property_name, $value, $notify = TRUE) { ...@@ -88,7 +88,7 @@ public function set($property_name, $value, $notify = TRUE) {
if ($notify && isset($this->parent)) { if ($notify && isset($this->parent)) {
$this->parent->onChange($this->name); $this->parent->onChange($this->name);
} }
return $property; return $this;
} }
/** /**
......
...@@ -385,9 +385,10 @@ protected function getTranslatedField($name, $langcode) { ...@@ -385,9 +385,10 @@ protected function getTranslatedField($name, $langcode) {
* {@inheritdoc} * {@inheritdoc}
*/ */
public function set($name, $value, $notify = TRUE) { public function set($name, $value, $notify = TRUE) {
// If default language or an entity key changes we need to react to that. // Assign the value on the child and overrule notify such that we get
$notify = $name == 'langcode' || in_array($name, $this->getEntityType()->getKeys()); // notified to handle changes afterwards. We can ignore notify as there is
$this->get($name)->setValue($value, $notify); // no parent to notify anyway.
$this->get($name)->setValue($value, TRUE);
} }
/** /**
......
...@@ -105,7 +105,8 @@ public function set($property_name, $value, $notify = TRUE) { ...@@ -105,7 +105,8 @@ public function set($property_name, $value, $notify = TRUE) {
throw new \InvalidArgumentException(String::format('Unable to set unknown property @name.', array('@name' => $property_name))); throw new \InvalidArgumentException(String::format('Unable to set unknown property @name.', array('@name' => $property_name)));
} }
// This will throw an exception for unknown fields. // This will throw an exception for unknown fields.
return $this->entity->set($property_name, $value, $notify); $this->entity->set($property_name, $value, $notify);
return $this;
} }
/** /**
......
...@@ -105,10 +105,7 @@ protected function getSetting($setting_name) { ...@@ -105,10 +105,7 @@ protected function getSetting($setting_name) {
} }
/** /**
* Overrides \Drupal\Core\TypedData\TypedData::setValue(). * {@inheritdoc}
*
* @param array|null $values
* An array of property values.
*/ */
public function setValue($values, $notify = TRUE) { public function setValue($values, $notify = TRUE) {
// Treat the values as property value of the first property, if no array is // Treat the values as property value of the first property, if no array is
...@@ -117,53 +114,39 @@ public function setValue($values, $notify = TRUE) { ...@@ -117,53 +114,39 @@ public function setValue($values, $notify = TRUE) {
$keys = array_keys($this->definition->getPropertyDefinitions()); $keys = array_keys($this->definition->getPropertyDefinitions());
$values = array($keys[0] => $values); $values = array($keys[0] => $values);
} }
$this->values = $values; parent::setValue($values, $notify);
// Update any existing property objects.
foreach ($this->properties as $name => $property) {
$value = NULL;
if (isset($values[$name])) {
$value = $values[$name];
}
$property->setValue($value, FALSE);
unset($this->values[$name]);
}
// Notify the parent of any changes.
if ($notify && isset($this->parent)) {
$this->parent->onChange($this->name);
}
}
/**
* {@inheritdoc}
*/
public function __get($name) {
// There is either a property object or a plain value - possibly for a
// not-defined property. If we have a plain value, directly return it.
if (isset($this->values[$name])) {
return $this->values[$name];
}
elseif (isset($this->properties[$name])) {
return $this->properties[$name]->getValue();
}
} }
/** /**
* {@inheritdoc} * {@inheritdoc}
*
* Different to the parent Map class, we avoid creating property objects as
* far as possible in order to optimize performance. Thus we just update
* $this->values if no property object has been created yet.
*/ */
public function set($property_name, $value, $notify = TRUE) { protected function writePropertyValue($property_name, $value) {
// For defined properties there is either a property object or a plain // For defined properties there is either a property object or a plain
// value that needs to be updated. // value that needs to be updated.
if (isset($this->properties[$property_name])) { if (isset($this->properties[$property_name])) {
$this->properties[$property_name]->setValue($value, FALSE); $this->properties[$property_name]->setValue($value, FALSE);
unset($this->values[$property_name]);
} }
// Allow setting plain values for not-defined properties also. // Allow setting plain values for not-defined properties also.
else { else {
$this->values[$property_name] = $value; $this->values[$property_name] = $value;
} }
// Directly notify ourselves. }
if ($notify) {
$this->onChange($property_name); /**
* {@inheritdoc}
*/
public function __get($name) {
// There is either a property object or a plain value - possibly for a
// not-defined property. If we have a plain value, directly return it.
if (isset($this->properties[$name])) {
return $this->properties[$name]->getValue();
}
elseif (isset($this->values[$name])) {
return $this->values[$name];
} }
} }
...@@ -183,29 +166,23 @@ public function __set($name, $value) { ...@@ -183,29 +166,23 @@ public function __set($name, $value) {
* {@inheritdoc} * {@inheritdoc}
*/ */
public function __isset($name) { public function __isset($name) {
return isset($this->values[$name]) || (isset($this->properties[$name]) && $this->properties[$name]->getValue() !== NULL); if (isset($this->properties[$name])) {
return $this->properties[$name]->getValue() !== NULL;
}
return isset($this->values[$name]);
} }
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
public function __unset($name) { public function __unset($name) {
$this->set($name, NULL); if ($this->definition->getPropertyDefinition($name)) {
unset($this->values[$name]); $this->set($name, NULL);
}
/**
* Overrides \Drupal\Core\TypedData\Map::onChange().
*/
public function onChange($property_name) {
// Notify the parent of changes.
if (isset($this->parent)) {
$this->parent->onChange($this->name);
} }
// Remove the plain value, such that any further __get() calls go via the else {
// updated property object. // Explicitly unset the property in $this->values if a non-defined
if (isset($this->properties[$property_name])) { // property is unset, such that its key is removed from $this->values.
unset($this->values[$property_name]); unset($this->values[$name]);
} }
} }
......
...@@ -141,13 +141,9 @@ public static function schema(FieldStorageDefinitionInterface $field_definition) ...@@ -141,13 +141,9 @@ public static function schema(FieldStorageDefinitionInterface $field_definition)
*/ */
public function setValue($values, $notify = TRUE) { public function setValue($values, $notify = TRUE) {
if (isset($values) && !is_array($values)) { if (isset($values) && !is_array($values)) {
// Directly update the property instead of invoking the parent, so it can // If either a scalar or an object was passed as the value for the item,
// handle objects and IDs. // assign it to the 'entity' property since that works for both cases.
$this->properties['entity']->setValue($values, $notify); $this->set('entity', $values, $notify);
// If notify was FALSE, ensure the target_id property gets synched.
if (!$notify) {
$this->set('target_id', $this->properties['entity']->getTargetIdentifier(), FALSE);
}
} }
else { else {
// Make sure that the 'entity' property gets set as 'target_id'. // Make sure that the 'entity' property gets set as 'target_id'.
...@@ -175,15 +171,15 @@ public function getValue() { ...@@ -175,15 +171,15 @@ public function getValue() {
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
public function onChange($property_name) { public function onChange($property_name, $notify = TRUE) {
// Make sure that the target ID and the target property stay in sync. // Make sure that the target ID and the target property stay in sync.
if ($property_name == 'target_id') { if ($property_name == 'target_id') {
$this->properties['entity']->setValue($this->target_id, FALSE); $this->writePropertyValue('entity', $this->target_id);
} }
elseif ($property_name == 'entity') { elseif ($property_name == 'entity') {
$this->set('target_id', $this->properties['entity']->getTargetIdentifier(), FALSE); $this->writePropertyValue('target_id', $this->get('entity')->getTargetIdentifier());
} }
parent::onChange($property_name); parent::onChange($property_name, $notify);
} }
/** /**
......
...@@ -69,14 +69,7 @@ public function setValue($values, $notify = TRUE) { ...@@ -69,14 +69,7 @@ public function setValue($values, $notify = TRUE) {
// Treat the values as property value of the language property, if no array // Treat the values as property value of the language property, if no array
// is given as this handles language codes and objects. // is given as this handles language codes and objects.
if (isset($values) && !is_array($values)) { if (isset($values) && !is_array($values)) {
// Directly update the property instead of invoking the parent, so that $this->set('language', $values, $notify);
// the language property can take care of updating the language code
// property.
$this->properties['language']->setValue($values, $notify);
// If notify was FALSE, ensure the value property gets synched.
if (!$notify) {
$this->set('value', $this->properties['language']->getTargetIdentifier(), FALSE);
}
} }
else { else {
// Make sure that the 'language' property gets set as 'value'. // Make sure that the 'language' property gets set as 'value'.
...@@ -100,14 +93,15 @@ public function applyDefaultValue($notify = TRUE) { ...@@ -100,14 +93,15 @@ public function applyDefaultValue($notify = TRUE) {
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
public function onChange($property_name) { public function onChange($property_name, $notify = TRUE) {
// Make sure that the value and the language property stay in sync. // Make sure that the value and the language property stay in sync.
if ($property_name == 'value') { if ($property_name == 'value') {
$this->properties['language']->setValue($this->value, FALSE); $this->writePropertyValue('language', $this->value);
} }
elseif ($property_name == 'language') { elseif ($property_name == 'language') {
$this->set('value', $this->properties['language']->getTargetIdentifier(), FALSE); $this->writePropertyValue('value', $this->get('language')->getTargetIdentifier());
} }
parent::onChange($property_name); parent::onChange($property_name, $notify);
} }
} }
...@@ -53,8 +53,7 @@ public function get($property_name); ...@@ -53,8 +53,7 @@ public function get($property_name);
* TRUE. If the update stems from a parent object, set it to FALSE to avoid * TRUE. If the update stems from a parent object, set it to FALSE to avoid
* being notified again. * being notified again.
* *
* @return \Drupal\Core\TypedData\TypedDataInterface * @return $this
* The property object.
* *
* @throws \InvalidArgumentException * @throws \InvalidArgumentException
* If the specified property does not exist. * If the specified property does not exist.
......
...@@ -94,11 +94,11 @@ public function setValue($values, $notify = TRUE) { ...@@ -94,11 +94,11 @@ public function setValue($values, $notify = TRUE) {
// Update any existing property objects. // Update any existing property objects.
foreach ($this->properties as $name => $property) { foreach ($this->properties as $name => $property) {
$value = NULL; $value = isset($values[$name]) ? $values[$name] : NULL;
if (isset($values[$name])) {
$value = $values[$name];
}
$property->setValue($value, FALSE); $property->setValue($value, FALSE);
// Remove the value from $this->values to ensure it does not contain any
// value for computed properties.
unset($this->values[$name]);
} }
// Notify the parent of any changes. // Notify the parent of any changes.
if ($notify && isset($this->parent)) { if ($notify && isset($this->parent)) {
...@@ -134,19 +134,34 @@ public function get($property_name) { ...@@ -134,19 +134,34 @@ public function get($property_name) {
} }
/** /**
* Implements \Drupal\Core\TypedData\ComplexDataInterface::set(). * {@inheritdoc}
*/ */
public function set($property_name, $value, $notify = TRUE) { public function set($property_name, $value, $notify = TRUE) {
// Separate the writing in a protected method, such that onChange
// implementations can make use of it.
$this->writePropertyValue($property_name, $value);
$this->onChange($property_name, $notify);
return $this;
}
/**
* Writes the value of a property without handling changes.
*
* Implementations of onChange() should use this method instead of set() in
* order to avoid onChange() being triggered again.
*
* @param string $property_name
* The name of the property to be written.
* @param $value
* The value to set.
*/
protected function writePropertyValue($property_name, $value) {
if ($this->definition->getPropertyDefinition($property_name)) { if ($this->definition->getPropertyDefinition($property_name)) {
$this->get($property_name)->setValue($value, $notify); $this->get($property_name)->setValue($value, FALSE);
} }
else { else {
// Just set the plain value, which allows adding a new entry to the map. // Just set the plain value, which allows adding a new entry to the map.
$this->values[$property_name] = $value; $this->values[$property_name] = $value;
// Directly notify ourselves.
if ($notify) {
$this->onChange($property_name, $value);
}
} }
} }
...@@ -212,11 +227,16 @@ public function __clone() { ...@@ -212,11 +227,16 @@ public function __clone() {
} }
/** /**
* Implements \Drupal\Core\TypedData\ComplexDataInterface::onChange(). * {@inheritdoc}
*
* @param bool $notify
* (optional) Whether to forward the notification to the parent. Defaults to
* TRUE. By passing FALSE, overrides of this method can re-use the logic
* of parent classes without triggering notification.
*/ */
public function onChange($property_name) { public function onChange($property_name, $notify = TRUE) {
// Notify the parent of changes. // Notify the parent of changes.
if (isset($this->parent)) { if ($notify && isset($this->parent)) {
$this->parent->onChange($this->name); $this->parent->onChange($this->name);
} }
} }
......
...@@ -131,13 +131,12 @@ public function isEmpty() { ...@@ -131,13 +131,12 @@ public function isEmpty() {
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
public function onChange($property_name) { public function onChange($property_name, $notify = TRUE) {
parent::onChange($property_name);
// Enforce that the computed date is recalculated. // Enforce that the computed date is recalculated.
if ($property_name == 'value') { if ($property_name == 'value') {
$this->date = NULL; $this->date = NULL;
} }
parent::onChange($property_name, $notify);
} }
} }
...@@ -130,16 +130,16 @@ protected function doTestReadWrite($entity_type) { ...@@ -130,16 +130,16 @@ protected function doTestReadWrite($entity_type) {
$this->assertEqual($this->entity_user->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: User name can be read.', array('%entity_type' => $entity_type))); $this->assertEqual($this->entity_user->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: User name can be read.', array('%entity_type' => $entity_type)));
// Change the assigned user by entity. // Change the assigned user by entity.
$new_user = $this->createUser(); $new_user1 = $this->createUser();
$entity->user_id->entity = $new_user; $entity->user_id->entity = $new_user1;
$this->assertEqual($new_user->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type))); $this->assertEqual($new_user1->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type)));
$this->assertEqual($new_user->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated username value can be read.', array('%entity_type' => $entity_type))); $this->assertEqual($new_user1->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated username value can be read.', array('%entity_type' => $entity_type)));
// Change the assigned user by id. // Change the assigned user by id.
$new_user = $this->createUser(); $new_user2 = $this->createUser();
$entity->user_id->target_id = $new_user->id(); $entity->user_id->target_id = $new_user2->id();
$this->assertEqual($new_user->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type))); $this->assertEqual($new_user2->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type)));
$this->assertEqual($new_user->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated username value can be read.', array('%entity_type' => $entity_type))); $this->assertEqual($new_user2->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated username value can be read.', array('%entity_type' => $entity_type)));
// Try unsetting a field. // Try unsetting a field.
$entity->name->value = NULL; $entity->name->value = NULL;
...@@ -148,6 +148,34 @@ protected function doTestReadWrite($entity_type) { ...@@ -148,6 +148,34 @@ protected function doTestReadWrite($entity_type) {
$this->assertNull($entity->user_id->target_id, format_string('%entity_type: User ID field is not set.', array('%entity_type' => $entity_type))); $this->assertNull($entity->user_id->target_id, format_string('%entity_type: User ID field is not set.', array('%entity_type' => $entity_type)));
$this->assertNull($entity->user_id->entity, format_string('%entity_type: User entity field is not set.', array('%entity_type' => $entity_type))); $this->assertNull($entity->user_id->entity, format_string('%entity_type: User entity field is not set.', array('%entity_type' => $entity_type)));
// Test setting the values via the typed data API works as well.
// Change the assigned user by entity.
$entity->user_id->first()->get('entity')->setValue($new_user2);
$this->assertEqual($new_user2->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type)));
$this->assertEqual($new_user2->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated user name value can be read.', array('%entity_type' => $entity_type)));
// Change the assigned user by id.
$entity->user_id->first()->get('target_id')->setValue($new_user2->id());
$this->assertEqual($new_user2->id(), $entity->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type)));
$this->assertEqual($new_user2->getUsername(), $entity->user_id->entity->name->value, format_string('%entity_type: Updated user name value can be read.', array('%entity_type' => $entity_type)));
// Try unsetting a field.
$entity->name->first()->get('value')->setValue(NULL);
$entity->user_id->first()->get('target_id')->setValue(NULL);
$this->assertNull($entity->name->value, format_string('%entity_type: Name field is not set.', array('%entity_type' => $entity_type)));
$this->assertNull($entity->user_id->target_id, format_string('%entity_type: User ID field is not set.', array('%entity_type' => $entity_type)));
$this->assertNull($entity->user_id->entity, format_string('%entity_type: User entity field is not set.', array('%entity_type' => $entity_type)));
// Create a fresh entity so target_id does not get its property object
// instantiated, then verify setting a new value via typed data API works.
$entity2 = entity_create($entity_type, array(
'user_id' => array('target_id' => $new_user1->id()),
));
// Access the property object, and set a value.
$entity2->user_id->first()->get('target_id')->setValue($new_user2->id());
$this->assertEqual($new_user2->id(), $entity2->user_id->target_id, format_string('%entity_type: Updated user id can be read.', array('%entity_type' => $entity_type)));
$this->assertEqual($new_user2->name->value, $entity2->user_id->entity->name->value, format_string('%entity_type: Updated user name value can be read.', array('%entity_type' => $entity_type)));
// Test using isset(), empty() and unset(). // Test using isset(), empty() and unset().
$entity->name->value = 'test unset'; $entity->name->value = 'test unset';
unset($entity->name->value); unset($entity->name->value);
......
...@@ -59,20 +59,16 @@ public function isEmpty() { ...@@ -59,20 +59,16 @@ public function isEmpty() {
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
public function onChange($property_name) { public function onChange($property_name, $notify = TRUE) {
// Notify the parent of changes.
if (isset($this->parent)) {
$this->parent->onChange($this->name);
}
// Unset processed properties that are affected by the change. // Unset processed properties that are affected by the change.
foreach ($this->definition->getPropertyDefinitions() as $property => $definition) { foreach ($this->definition->getPropertyDefinitions() as $property => $definition) {
if ($definition->getClass() == '\Drupal\text\TextProcessed') { if ($definition->getClass() == '\Drupal\text\TextProcessed') {
if ($property_name == 'format' || ($definition->getSetting('text source') == $property_name)) { if ($property_name == 'format' || ($definition->getSetting('text source') == $property_name)) {
$this->set($property, NULL, FALSE); $this->writePropertyValue($property, NULL);
} }
} }
} }
parent::onChange($property_name, $notify);
} }
/** /**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment