Commit a49d5430 authored by alexpott's avatar alexpott

Issue #2236903 by yched, tstoeckler, swentel, Jalandhar, nlisgo, andypost,...

Issue #2236903 by yched, tstoeckler, swentel, Jalandhar, nlisgo, andypost, fago, sun, amateescu: setSettings() on field definitions can unintentionally clear default values
parent ae70ed6f
......@@ -146,16 +146,36 @@ public function getSettings() {
}
/**
* Sets field settings.
* {@inheritdoc}
*
* @param array $settings
* The value to set.
* Note that the method does not unset existing settings not specified in the
* incoming $settings array.
*
* @return static
* The object itself for chaining.
* For example:
* @code
* // Given these are the default settings.
* $field_definition->getSettings() === [
* 'fruit' => 'apple',
* 'season' => 'summer',
* ];
* // Change only the 'fruit' setting.
* $field_definition->setSettings(['fruit' => 'banana']);
* // The 'season' setting persists unchanged.
* $field_definition->getSettings() === [
* 'fruit' => 'banana',
* 'season' => 'summer',
* ];
* @endcode
*
* For clarity, it is preferred to use setSetting() if not all available
* settings are supplied.
*/
public function setSettings(array $settings) {
$this->getItemDefinition()->setSettings($settings);
// Assign settings individiually, in order to keep the current values
// of settings not specified in $settings.
foreach ($settings as $setting_name => $setting) {
$this->getItemDefinition()->setSetting($setting_name, $setting);
}
return $this;
}
......@@ -167,15 +187,7 @@ public function getSetting($setting_name) {
}
/**
* Sets a field setting.
*
* @param string $setting_name
* The field setting to set.
* @param mixed $value
* The value to set.
*
* @return static
* The object itself for chaining.
* {@inheritdoc}
*/
public function setSetting($setting_name, $value) {
$this->getItemDefinition()->setSetting($setting_name, $value);
......
......@@ -350,7 +350,7 @@ public function getSettings() {
* {@inheritdoc}
*/
public function setSettings(array $settings) {
$this->settings = $settings;
$this->settings = $settings + $this->settings;
return $this;
}
......
......@@ -55,7 +55,29 @@ public function setDescription($description);
public function setTranslatable($translatable);
/**
* Sets field settings (overwrites existing settings).
* Sets field settings.
*
* Note that the method does not unset existing settings not specified in the
* incoming $settings array.
*
* For example:
* @code
* // Given these are the default settings.
* $field_definition->getSettings() === [
* 'fruit' => 'apple',
* 'season' => 'summer',
* ];
* // Change only the 'fruit' setting.
* $field_definition->setSettings(['fruit' => 'banana']);
* // The 'season' setting persists unchanged.
* $field_definition->getSettings() === [
* 'fruit' => 'banana',
* 'season' => 'summer',
* ];
* @endcode
*
* For clarity, it is preferred to use setSetting() if not all available
* settings are supplied.
*
* @param array $settings
* The array of field settings.
......
......@@ -57,19 +57,23 @@ public function getName();
public function getType();
/**
* Returns the field settings.
* Returns the storage settings.
*
* Each field type defines the settings that are meaningful for that type.
* For example, a text field can define a 'max_length' setting, and an image
* field can define a 'alt_field_required' setting.
*
* The method always returns an array of all available settings for this field
* type, possibly with the default values merged in if values have not been
* provided for all available settings.
*
* @return mixed[]
* An array of key/value pairs.
*/
public function getSettings();
/**
* Returns the value of a given field setting.
* Returns the value of a given storage setting.
*
* @param string $setting_name
* The setting name.
......
......@@ -565,7 +565,7 @@ public function setSetting($setting_name, $value) {
* {@inheritdoc}
*/
public function setSettings(array $settings) {
$this->settings = $settings;
$this->settings = $settings + $this->settings;
return $this;
}
......
......@@ -96,10 +96,32 @@ public function setCardinality($cardinality);
public function setSetting($setting_name, $value);
/**
* Sets field settings (overwrites existing settings).
* Sets field storage settings.
*
* Note that the method does not unset existing settings not specified in the
* incoming $settings array.
*
* For example:
* @code
* // Given these are the default settings.
* $storage_definition->getSettings() === [
* 'fruit' => 'apple',
* 'season' => 'summer',
* ];
* // Change only the 'fruit' setting.
* $storage_definition->setSettings(['fruit' => 'banana']);
* // The 'season' setting persists unchanged.
* $storage_definition->getSettings() === [
* 'fruit' => 'banana',
* 'season' => 'summer',
* ];
* @endcode
*
* For clarity, it is preferred to use setSetting() if not all available
* settings are supplied.
*
* @param array $settings
* The array of field settings.
* The array of storage settings.
*
* @return $this
*/
......
......@@ -261,9 +261,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
->setDescription(t('The text to be used for this link in the menu.'))
->setRequired(TRUE)
->setTranslatable(TRUE)
->setSettings(array(
'max_length' => 255,
))
->setSetting('max_length', 255)
->setDisplayOptions('view', array(
'label' => 'hidden',
'type' => 'string',
......@@ -279,9 +277,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
->setLabel(t('Description'))
->setDescription(t('Shown when hovering over the menu link.'))
->setTranslatable(TRUE)
->setSettings(array(
'max_length' => 255,
))
->setSetting('max_length', 255)
->setDisplayOptions('view', array(
'label' => 'hidden',
'type' => 'string',
......
......@@ -30,7 +30,7 @@ class EntityFieldTest extends EntityUnitTestBase {
*
* @var array
*/
public static $modules = array('filter', 'text', 'node', 'user');
public static $modules = array('filter', 'text', 'node', 'user', 'field_test');
/**
* @var string
......@@ -688,10 +688,8 @@ public function testEntityConstraintValidation() {
->save();
$definition = BaseFieldDefinition::create('entity_reference')
->setLabel('Test entity')
->setSettings(array(
'target_type' => 'node',
'target_bundle' => 'article',
));
->setSetting('target_type', 'node')
->setSetting('target_bundle', 'article');
$reference_field = \Drupal::TypedDataManager()->create($definition);
$reference = $reference_field->appendItem(array('entity' => $node))->get('entity');
$violations = $reference->validate();
......
<?php
/**
* @file
* Contains Drupal\system\Tests\Field\FieldSettingsTest.
*/
namespace Drupal\system\Tests\Field;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\system\Tests\Entity\EntityUnitTestBase;
/**
* Tests field settings methods on field definition structures.
*
* @group Field
*/
class FieldSettingsTest extends EntityUnitTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = array('field', 'field_test');
/**
* @covers \Drupal\Core\Field\BaseFieldDefinition::getSettings()
* @covers \Drupal\Core\Field\BaseFieldDefinition::setSettings()
*/
public function testBaseFieldSettings() {
$base_field = BaseFieldDefinition::create('test_field');
// Check that the default settings have been populated.
$expected_settings = [
'test_field_storage_setting' => 'dummy test string',
'changeable' => 'a changeable field storage setting',
'unchangeable' => 'an unchangeable field storage setting',
'translatable_storage_setting' => 'a translatable field storage setting',
'test_field_setting' => 'dummy test string',
'translatable_field_setting' => 'a translatable field setting',
];
$this->assertEqual($base_field->getSettings(), $expected_settings);
// Change one single setting using setSettings(), and check that the other
// expected settings are still present.
$expected_settings['test_field_setting'] = 'another test string';
$base_field->setSettings(['test_field_setting' => $expected_settings['test_field_setting']]);
$this->assertEqual($base_field->getSettings(), $expected_settings);
}
/**
* @covers \Drupal\field\Entity\FieldStorageConfig::getSettings()
* @covers \Drupal\field\Entity\FieldStorageConfig::setSettings()
*/
public function testConfigurableFieldStorageSettings() {
$field_storage = FieldStorageConfig::create([
'field_name' => 'test_field',
'entity_type' => 'entity_test',
'type' => 'test_field'
]);
// Check that the default settings have been populated.
$expected_settings = [
'test_field_storage_setting' => 'dummy test string',
'changeable' => 'a changeable field storage setting',
'unchangeable' => 'an unchangeable field storage setting',
'translatable_storage_setting' => 'a translatable field storage setting',
];
$this->assertEqual($field_storage->getSettings(), $expected_settings);
// Change one single setting using setSettings(), and check that the other
// expected settings are still present.
$expected_settings['test_field_storage_setting'] = 'another test string';
$field_storage->setSettings(['test_field_storage_setting' => $expected_settings['test_field_storage_setting']]);
$this->assertEqual($field_storage->getSettings(), $expected_settings);
}
/**
* @covers \Drupal\field\Entity\FieldStorageConfig::getSettings()
* @covers \Drupal\field\Entity\FieldStorageConfig::setSettings()
*/
public function testConfigurableFieldSettings() {
$field_storage = FieldStorageConfig::create([
'field_name' => 'test_field',
'entity_type' => 'entity_test',
'type' => 'test_field'
]);
$field = FieldConfig::create([
'field_storage' => $field_storage,
'bundle' => 'entity_test'
]);
// Note: FieldConfig does not populate default settings until the config
// is saved.
// @todo Remove once https://www.drupal.org/node/2327883 is fixed.
$field->save();
// Check that the default settings have been populated. Note: getSettings()
// returns both storage and field settings.
$expected_settings = [
'test_field_storage_setting' => 'dummy test string',
'changeable' => 'a changeable field storage setting',
'unchangeable' => 'an unchangeable field storage setting',
'translatable_storage_setting' => 'a translatable field storage setting',
'test_field_setting' => 'dummy test string',
'translatable_field_setting' => 'a translatable field setting',
'field_setting_from_config_data' => TRUE,
];
$this->assertEqual($field->getSettings(), $expected_settings);
// Change one single setting using setSettings(), and check that the other
// expected settings are still present.
$expected_settings['test_field_setting'] = 'another test string';
$field->setSettings(['test_field_setting' => $expected_settings['test_field_setting']]);
$this->assertEqual($field->getSettings(), $expected_settings);
}
}
......@@ -485,7 +485,7 @@ public function testDataTableFields() {
$base_field_definitions = $this->setupBaseFields(EntityTestMul::baseFieldDefinitions($this->baseEntityType));
$base_field_definitions['type'] = BaseFieldDefinition::create('entity_reference')
->setLabel('entity test type')
->setSettings(array('target_type' => 'entity_test_bundle'))
->setSetting('target_type', 'entity_test_bundle')
->setTranslatable(TRUE);
$base_field_definitions = $this->setupBaseFields($base_field_definitions);
$user_base_field_definitions = [
......
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