diff options
Diffstat (limited to 'modules/field')
| -rw-r--r-- | modules/field/field.api.php | 52 | ||||
| -rw-r--r-- | modules/field/field.attach.inc | 15 | ||||
| -rw-r--r-- | modules/field/field.crud.inc | 83 | ||||
| -rw-r--r-- | modules/field/field.test | 107 | ||||
| -rw-r--r-- | modules/field/modules/field_sql_storage/field_sql_storage.module | 60 | ||||
| -rw-r--r-- | modules/field/modules/field_sql_storage/field_sql_storage.test | 63 | ||||
| -rw-r--r-- | modules/field/modules/list/list.module | 8 | ||||
| -rw-r--r-- | modules/field/modules/number/number.module | 4 | ||||
| -rw-r--r-- | modules/field/modules/text/text.module | 7 |
9 files changed, 389 insertions, 10 deletions
diff --git a/modules/field/field.api.php b/modules/field/field.api.php index 5ed330f32..6fd2cef32 100644 --- a/modules/field/field.api.php +++ b/modules/field/field.api.php @@ -665,7 +665,7 @@ function hook_field_widget_error($element, $error) { * The name of the theme hook invoked when displaying the values is derived * from formatter type names, using the pattern field_formatter_FORMATTER_NAME. * field.module takes care of exposing the corresponding theme functions - * through hook_theme(). Specifically, field.module defines the theme + * through hook_theme(). Specifically, field.module defines the theme * hook: * * @code @@ -1274,6 +1274,56 @@ function hook_field_create_instance($instance) { } /** + * Forbid a field update from occurring. + * + * Any module may forbid any update for any reason. For example, the + * field's storage module might forbid an update if it would change + * the storage schema while data for the field exists. A field type + * module might forbid an update if it would change existing data's + * semantics, or if there are external dependencies on field settings + * that cannot be updated. + * + * @param $field + * The field as it will be post-update. + * @param $prior_field + * The field as it is pre-update. + * @param $has_data + * Whether any data already exists for this field. + * @return + * Throws a FieldException to prevent the update from occuring. + */ +function hook_field_update_field_forbid($field, $prior_field, $has_data) { + // A 'list' field stores integer keys mapped to display values. If + // the new field will have fewer values, and any data exists for the + // abandonded keys, the field will have no way to display them. So, + // forbid such an update. + if ($has_data && count($field['settings']['allowed_values']) < count($prior_field['settings']['allowed_values'])) { + // Identify the keys that will be lost. + $lost_keys = array_diff(array_keys($field['settings']['allowed_values']), array_keys($prior_field['settings']['allowed_values'])); + // If any data exist for those keys, forbid the update. + $count = field_attach_query($prior_field['id'], array('value', $lost_keys, 'IN'), 1); + if ($count > 0) { + throw new FieldException("Cannot update a list field not to include keys with existing data"); + } + } +} + +/** + * Act on a field being updated. + * + * This hook is invoked just after field is updated. + * + * @param $field + * The field as it is post-update. + * @param $prior_field + * The field as it was pre-update. + * @param $has_data + * Whether any data already exists for this field. + */ +function hook_field_update_field($field, $prior_field, $has_data) { +} + +/** * Act on a field being deleted. * * This hook is invoked just after field is deleted. diff --git a/modules/field/field.attach.inc b/modules/field/field.attach.inc index 5f2229376..7571ee4fc 100644 --- a/modules/field/field.attach.inc +++ b/modules/field/field.attach.inc @@ -153,7 +153,7 @@ define('FIELD_STORAGE_INSERT', 'insert'); * An associative array of additional options, with the following keys: * - 'field_name': The name of the field whose operation should be * invoked. By default, the operation is invoked on all the fields - * in the object's bundle. NOTE: This option is not compatible with + * in the object's bundle. NOTE: This option is not compatible with * the 'deleted' option; the 'field_id' option should be used * instead. * - 'field_id': The id of the field whose operation should be @@ -1012,6 +1012,19 @@ function field_attach_query_revisions($field_id, $conditions, $count, &$cursor = } /** + * Determine whether a field has any data. + * + * @param $field + * A field structure. + * @return + * TRUE if the field has data for any object; FALSE otherwise. + */ +function field_attach_field_has_data($field) { + $results = field_attach_query($field['id'], array(), 1); + return !empty($results); +} + +/** * Generate and return a structured content array tree suitable for * drupal_render() for all of the fields on an object. The format of * each field's rendered content depends on the display formatter and diff --git a/modules/field/field.crud.inc b/modules/field/field.crud.inc index 7f316c4db..210dad136 100644 --- a/modules/field/field.crud.inc +++ b/modules/field/field.crud.inc @@ -312,6 +312,89 @@ function field_create_field($field) { return $field; } +/* + * Update a field. + * + * Any module may forbid any update for any reason. For example, the + * field's storage module might forbid an update if it would change + * the storage schema while data for the field exists. A field type + * module might forbid an update if it would change existing data's + * semantics, or if there are external dependencies on field settings + * that cannot be updated. + * + * @param $field + * A field structure. $field['field_name'] must provided; it + * identifies the field that will be updated to match this + * structure. Any other properties of the field that are not + * specified in $field will be left unchanged, so it is not + * necessary to pass in a fully populated $field structure. + * @return + * Throws a FieldException if the update cannot be performed. + * @see field_create_field() + */ +function field_update_field($field) { + // Check that the specified field exists. + $prior_field = field_read_field($field['field_name']); + if (empty($prior_field)) { + throw new FieldException('Attempt to update a non-existent field.'); + } + + // Use the prior field values for anything not specifically set by the new + // field to be sure that all values are set. + $field += $prior_field; + $field['settings'] += $prior_field['settings']; + + // Field type cannot be changed. + if ($field['type'] != $prior_field['type']) { + throw new FieldException("Cannot change an existing field's type."); + } + + // Collect the new storage information, since what is in + // $prior_field may no longer be right. + $schema = (array) module_invoke($field['module'], 'field_schema', $field); + $schema += array('columns' => array(), 'indexes' => array()); + // 'columns' are hardcoded in the field type. + $field['columns'] = $schema['columns']; + // 'indexes' can be both hardcoded in the field type, and specified in the + // incoming $field definition. + $field += array( + 'indexes' => array(), + ); + $field['indexes'] += $schema['indexes']; + + $has_data = field_attach_field_has_data($field); + + // See if any module forbids the update by throwing an exception. + foreach (module_implements('field_update_forbid') as $module) { + $function = $module . '_field_update_forbid'; + $function($field, $prior_field, $has_data); + } + + // Tell the storage engine to update the field. Do this before + // saving the new definition since it still might fail. + module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_update_field', $field, $prior_field, $has_data); + + // Save the new field definition. @todo: refactor with + // field_create_field. + + // The serialized 'data' column contains everything from $field that does not + // have its own column and is not automatically populated when the field is + // read. + $data = $field; + unset($data['columns'], $data['field_name'], $data['type'], $data['locked'], $data['module'], $data['cardinality'], $data['active'], $data['deleted']); + $field['data'] = $data; + + // Store the field and create the id. + $primary_key = array('id'); + drupal_write_record('field_config', $field, $primary_key); + + // Clear caches + field_cache_clear(TRUE); + + // Invoke external hooks after the cache is cleared for API consistency. + module_invoke_all('field_update_field', $field, $prior_field, $has_data); +} + /** * Read a single field record directly from the database. Generally, * you should use the field_info_field() instead. diff --git a/modules/field/field.test b/modules/field/field.test index b530885ad..2523c8b72 100644 --- a/modules/field/field.test +++ b/modules/field/field.test @@ -1429,13 +1429,14 @@ class FieldCrudTestCase extends FieldTestCase { public static function getInfo() { return array( 'name' => 'Field CRUD tests', - 'description' => 'Create / read /update fields.', + 'description' => 'Test field create, read, update, and delete.', 'group' => 'Field', ); } function setUp() { - parent::setUp('field_test'); + // field_update_field() tests use number.module + parent::setUp('field_test', 'number'); } // TODO : test creation with @@ -1726,6 +1727,108 @@ class FieldCrudTestCase extends FieldTestCase { $this->assertEqual($entity->{$field['field_name']}[$langcode][$delta]['value'], $values[$delta]['value'], "Data in previously deleted field saves and loads correctly"); } } + + function testUpdateNonExistentField() { + $test_field = array('field_name' => 'does_not_exist', 'type' => 'number_decimal'); + try { + field_update_field($test_field); + $this->fail(t('Cannot update a field that does not exist.')); + } + catch (FieldException $e) { + $this->pass(t('Cannot update a field that does not exist.')); + } + } + + function testUpdateFieldType() { + $field = array('field_name' => 'field_type', 'type' => 'number_decimal'); + $field = field_create_field($field); + + $test_field = array('field_name' => 'field_type', 'type' => 'number_integer'); + try { + field_update_field($test_field); + $this->fail(t('Cannot update a field to a different type.')); + } + catch (FieldException $e) { + $this->pass(t('Cannot update a field to a different type.')); + } + } + + /** + * Test updating a field. + */ + function testUpdateField() { + // Create a decimal 5.2 field. + $field = array('field_name' => 'decimal53', 'type' => 'number_decimal', 'cardinality' => 3, 'settings' => array('precision' => 5, 'scale' => 2)); + $field = field_create_field($field); + $instance = array('field_name' => 'decimal53', 'bundle' => FIELD_TEST_BUNDLE); + $instance = field_create_instance($instance); + + // Update it to a deciaml 5.3 field. + $field['settings']['scale'] = 3; + field_update_field($field); + + // Save values with 2, 3, and 4 decimal places. + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $entity->decimal53[FIELD_LANGUAGE_NONE][0]['value'] = '1.23'; + $entity->decimal53[FIELD_LANGUAGE_NONE][1]['value'] = '1.235'; + $entity->decimal53[FIELD_LANGUAGE_NONE][2]['value'] = '1.2355'; + field_attach_insert('test_entity', $entity); + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + + // Verify that the updated 5.3 field rounds to 3 decimal places. + field_attach_load('test_entity', array(0 => $entity)); + $this->assertEqual($entity->decimal53[FIELD_LANGUAGE_NONE][0]['value'], '1.23', t('2 decimal places are left alone')); + $this->assertEqual($entity->decimal53[FIELD_LANGUAGE_NONE][1]['value'], '1.235', t('3 decimal places are left alone')); + $this->assertEqual($entity->decimal53[FIELD_LANGUAGE_NONE][2]['value'], '1.236', t('4 decimal places are rounded to 3')); + } + + /** + * Test updating a field with data. + */ + function testUpdateFieldSchemaWithData() { + // Create a decimal 5.2 field and add some data. + $field = array('field_name' => 'decimal52', 'type' => 'number_decimal', 'settings' => array('precision' => 5, 'scale' => 2)); + $field = field_create_field($field); + $instance = array('field_name' => 'decimal52', 'bundle' => FIELD_TEST_BUNDLE); + $instance = field_create_instance($instance); + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $entity->decimal52[FIELD_LANGUAGE_NONE][0]['value'] = '1.235'; + field_attach_insert('test_entity', $entity); + + // Attempt to update the field in a way that would work without data. + $field['settings']['scale'] = 3; + try { + field_update_field($field); + $this->fail(t('Cannot update field schema with data.')); + } + catch (FieldException $e) { + $this->pass(t('Cannot update field schema with data.')); + } + } + + /** + * Test field type modules forbidding an update. + */ + function testUpdateFieldForbid() { + $field = array('field_name' => 'forbidden', 'type' => 'test_field', 'settings' => array('changeable' => 0, 'unchangeable' => 0)); + $field = field_create_field($field); + $field['settings']['changeable']++; + try { + field_update_field($field); + $this->pass(t("A changeable setting can be updated.")); + } + catch (FieldException $e) { + $this->fail(t("An unchangeable setting cannot be updated.")); + } + $field['settings']['unchangeable']++; + try { + field_update_field($field); + $this->fail(t("An unchangeable setting can be updated.")); + } + catch (FieldException $e) { + $this->pass(t("An unchangeable setting cannot be updated.")); + } + } } class FieldInstanceCrudTestCase extends FieldTestCase { diff --git a/modules/field/modules/field_sql_storage/field_sql_storage.module b/modules/field/modules/field_sql_storage/field_sql_storage.module index ff096dfe6..97d372f3b 100644 --- a/modules/field/modules/field_sql_storage/field_sql_storage.module +++ b/modules/field/modules/field_sql_storage/field_sql_storage.module @@ -12,7 +12,7 @@ function field_sql_storage_help($path, $arg) { switch ($path) { case 'admin/help#field_sql_storage': - $output = '<p>' . t('The Field SQL Storage module stores Field API data in the database. It is the default field storage module, but other field storage modules may be available in the contributions repository.') . '</p>'; + $output = '<p>' . t('The Field SQL Storage module stores Field API data in the database. It is the default field storage module, but other field storage modules may be available in the contributions repository.') . '</p>'; return $output; } } @@ -204,6 +204,64 @@ function field_sql_storage_field_storage_create_field($field) { } /** + * Implement hook_field_update_field_forbid(). + * + * Forbid any field update that changes column definitions if there is + * any data. + */ +function field_sql_storage_field_update_forbid($field, $prior_field, $has_data) { + if ($has_data && $field['columns'] != $prior_field['columns']) { + throw new FieldException("field_sql_storage cannot change the schema for an existing field with data."); + } +} + +/** + * Implement hook_field_storage_update_field(). + */ +function field_sql_storage_field_storage_update_field($field, $prior_field, $has_data) { + $ret = array(); + + if (! $has_data) { + // There is no data. Re-create the tables completely. + $prior_schema = _field_sql_storage_schema($prior_field); + foreach ($prior_schema as $name => $table) { + db_drop_table($ret, $name, $table); + } + $schema = _field_sql_storage_schema($field); + foreach ($schema as $name => $table) { + db_create_table($ret, $name, $table); + } + } + else { + // There is data, so there are no column changes. Drop all the + // prior indexes and create all the new ones, except for all the + // priors that exist unchanged. + $table = _field_sql_storage_tablename($prior_field); + $revision_table = _field_sql_storage_revision_tablename($prior_field); + foreach ($prior_field['indexes'] as $name => $columns) { + if (!isset($field['indexes'][$name]) || $columns != $field['indexes'][$name]) { + $real_name = _field_sql_storage_indexname($field['field_name'], $name); + db_drop_index($ret, $table, $real_name); + db_drop_index($ret, $revision_table, $real_name); + } + } + $table = _field_sql_storage_tablename($field); + $revision_table = _field_sql_storage_revision_tablename($field); + foreach ($field['indexes'] as $name => $columns) { + if (!isset($prior_field['indexes'][$name]) || $columns != $prior_field['indexes'][$name]) { + $real_name = _field_sql_storage_indexname($field['field_name'], $name); + $real_columns = array(); + foreach ($columns as $column_name) { + $real_columns[] = _field_sql_storage_columnname($field['field_name'], $column_name); + } + db_add_index($ret, $table, $real_name, $real_columns); + db_add_index($ret, $revision_table, $real_name, $real_columns); + } + } + } +} + +/** * Implement hook_field_storage_delete_field(). */ function field_sql_storage_field_storage_delete_field($field_name) { diff --git a/modules/field/modules/field_sql_storage/field_sql_storage.test b/modules/field/modules/field_sql_storage/field_sql_storage.test index cb4aee045..5f8a30a42 100644 --- a/modules/field/modules/field_sql_storage/field_sql_storage.test +++ b/modules/field/modules/field_sql_storage/field_sql_storage.test @@ -22,7 +22,7 @@ class FieldSqlStorageTestCase extends DrupalWebTestCase { } function setUp() { - parent::setUp('field_sql_storage', 'field', 'field_test'); + parent::setUp('field_sql_storage', 'field', 'field_test', 'text'); $this->field_name = drupal_strtolower($this->randomName() . '_field_name'); $this->field = array('field_name' => $this->field_name, 'type' => 'test_field', 'cardinality' => 4); $this->field = field_create_field($this->field); @@ -294,4 +294,65 @@ class FieldSqlStorageTestCase extends DrupalWebTestCase { ->fetchField(); $this->assertEqual($count, 1, 'NULL field translation is wiped.'); } + + + /** + * Test adding and removing indexes while data is present. + */ + function testFieldUpdateIndexesWithData() { + // We do not have a db-agnostic inspection system in core yet, so + // for now we can only test this on mysql. + if (Database::getConnection()->databaseType() == 'mysql') { + // Create a decimal field. + $field_name = 'testfield'; + $field = array('field_name' => $field_name, 'type' => 'text'); + $field = field_create_field($field); + $instance = array('field_name' => $field_name, 'bundle' => FIELD_TEST_BUNDLE); + $instance = field_create_instance($instance); + $tables = array(_field_sql_storage_tablename($field), _field_sql_storage_revision_tablename($field)); + + // Verify the indexes we will create do not exist yet. + foreach ($tables as $table) { + $indexes = $this->getIndexes($table); + $this->assertTrue(empty($indexes['value']), t("No index named value exists in $table")); + $this->assertTrue(empty($indexes['value_format']), t("No index named value_format exists in $table")); + } + + // Add data so the table cannot be dropped. + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $entity->{$field_name}[FIELD_LANGUAGE_NONE][0]['value'] = 'field data'; + field_attach_insert('test_entity', $entity); + + // Add an index + $field = array('field_name' => $field_name, 'indexes' => array('value' => array('value'))); + field_update_field($field); + foreach ($tables as $table) { + $indexes = $this->getIndexes($table); + $this->assertTrue($indexes["{$field_name}_value"] == array(1 => "{$field_name}_value"), t("Index on value created in $table")); + } + + // Add a different index, removing the existing custom one. + $field = array('field_name' => $field_name, 'indexes' => array('value_format' => array('value', 'format'))); + field_update_field($field); + foreach ($tables as $table) { + $indexes = $this->getIndexes($table); + $this->assertTrue($indexes["{$field_name}_value_format"] == array(1 => "{$field_name}_value", 2 => "{$field_name}_format"), t("Index on value_format created in $table")); + $this->assertTrue(empty($indexes["{$field_name}_value"]), t("Index on value removed in $table")); + } + + // Verify that the tables were not dropped. + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + field_attach_load('test_entity', array(0 => $entity)); + $this->assertEqual($entity->{$field_name}[FIELD_LANGUAGE_NONE][0]['value'], 'field data', t("Index changes performed without dropping the tables")); + } + } + + function getIndexes($table) { + $indexes = array(); + $result = db_query("SHOW INDEXES FROM {" . $table . "}"); + foreach ($result as $row) { + $indexes[$row->key_name][$row->seq_in_index] = $row->column_name; + } + return $indexes; + } } diff --git a/modules/field/modules/list/list.module b/modules/field/modules/list/list.module index 9f29b2161..119cc9fea 100644 --- a/modules/field/modules/list/list.module +++ b/modules/field/modules/list/list.module @@ -85,8 +85,14 @@ function list_field_schema($field) { /** * Implement hook_field_settings_form(). + * + * @todo: If $has_data, add a form validate function to verify that the + * new allowed values do not exclude any keys for which data already + * exists in the databae (use field_attach_query()) to find out. + * Implement the validate function via hook_field_update_forbid() so + * list.module does not depend on form submission. */ -function list_field_settings_form($field, $instance) { +function list_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form['allowed_values'] = array( diff --git a/modules/field/modules/number/number.module b/modules/field/modules/number/number.module index 19fd5091f..594b0f1a5 100644 --- a/modules/field/modules/number/number.module +++ b/modules/field/modules/number/number.module @@ -96,7 +96,7 @@ function number_field_schema($field) { /** * Implement hook_field_settings_form(). */ -function number_field_settings_form($field, $instance) { +function number_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form = array(); @@ -107,6 +107,7 @@ function number_field_settings_form($field, $instance) { '#options' => drupal_map_assoc(range(10, 32)), '#default_value' => $settings['precision'], '#description' => t('The total number of digits to store in the database, including those to the right of the decimal.'), + '#disabled' => $has_data, ); $form['scale'] = array( '#type' => 'select', @@ -114,6 +115,7 @@ function number_field_settings_form($field, $instance) { '#options' => drupal_map_assoc(range(0, 10)), '#default_value' => $settings['scale'], '#description' => t('The number of digits to the right of the decimal.'), + '#disabled' => $has_data, ); $form['decimal'] = array( '#type' => 'select', diff --git a/modules/field/modules/text/text.module b/modules/field/modules/text/text.module index 978a0d33a..cc0245dee 100644 --- a/modules/field/modules/text/text.module +++ b/modules/field/modules/text/text.module @@ -116,7 +116,7 @@ function text_field_schema($field) { /** * Implement hook_field_settings_form(). */ -function text_field_settings_form($field, $instance) { +function text_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form['max_length'] = array( @@ -126,6 +126,9 @@ function text_field_settings_form($field, $instance) { '#required' => FALSE, '#description' => t('The maximum length of the field in characters. Leave blank for an unlimited size.'), '#element_validate' => array('_element_validate_integer_positive'), + // @todo: If $has_data, add a validate handler that only allows + // max_length to increase. + '#disabled' => $has_data, ); return $form; @@ -318,7 +321,7 @@ function theme_field_formatter_text_trimmed($element) { /** * Theme function for 'summary or trimmed' field formatter for - * text_with_summary fields. This formatter returns the summary + * text_with_summary fields. This formatter returns the summary * element of the field or, if the summary is empty, the trimmed * version of the full element of the field. */ |