? BODY-AS-FIELD.patch ? DELETE-FIELD.patch ? Makefile ? constants.pl ? d6-50-nodes.sql.gz ? d7-50-nodes.sql.gz ? head.kpf ? patches ? modules/field/field.delete.inc ? modules/field/modules/combo ? scripts/generate-autoload.pl ? sites/modules ? sites/all/cck ? sites/all/modules/pbs ? sites/default/files ? sites/default/settings.php Index: modules/field/field.crud.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.crud.inc,v retrieving revision 1.10 diff -u -F^[fc] -r1.10 field.crud.inc --- modules/field/field.crud.inc 24 Apr 2009 03:44:17 -0000 1.10 +++ modules/field/field.crud.inc 27 Apr 2009 22:30:50 -0000 @@ -170,6 +170,8 @@ * @param $field * A field structure. The field_name and type properties are * required. + * @return + * The $field structure with the id property filled in. * @throw * FieldException */ @@ -195,9 +197,10 @@ function field_create_field($field) { throw new FieldException(t('Attempt to create a field of unknown type %type.', array('%type' => $field['type']))); } - // Ensure the field name is unique. We also check disabled or deleted fields. + // Ensure the field name is unique over active and disabled fields. + // We do not care about deleted fields. // TODO : do we want specific messages when clashing with a disabled or inactive field ? - $prior_field = field_read_field($field['field_name'], array('include_inactive' => TRUE, 'include_deleted' => TRUE)); + $prior_field = field_read_field($field['field_name'], array('include_inactive' => TRUE)); if (!empty($prior_field)) { throw new FieldException(t('Attempt to create field name %name which already exists.', array('%name' => $field['field_name']))); } @@ -217,26 +220,37 @@ function field_create_field($field) { // Create the data table. We need to populate the field columns, even though // we don't actually store them. $field['columns'] = (array) module_invoke($field['module'], 'field_columns', $field); - module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_create_field', $field); - + + // Store the field and create the id. drupal_write_record('field_config', $field); + // Invoke hook_field_storage_create_field after the field is + // complete (e.g. it has its id). + module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_create_field', $field); + // Clear caches field_cache_clear(TRUE); + + return $field; } /** * Read a single field record directly from the database. Generally, * you should use the field_info_field() instead. * + * This function will not return deleted fields. If multiple fields + * with the same name have ever existed, deleted or not, and not all + * of the deleted fields have been cleaned up, there is no way with + * this function to control which one will you get. Use + * field_read_fields() instead for this purpose. + * * @param $field_name * The field name to read. * @param array $include_additional * The default behavior of this function is to not return a field that - * is inactive or has been deleted. Setting - * $include_additional['include_inactive'] or - * $include_additional['include_deleted'] to TRUE will override this - * behavior. + * is inactive. Setting + * $include_additional['include_inactive'] to TRUE will override this + * behavior. * @return * A field structure, or FALSE. */ @@ -257,7 +271,9 @@ function field_read_field($field_name, $ * $include_additional['include_deleted'] to TRUE will override this * behavior. * @return - * An array of fields matching $params. + * An array of fields matching $params. If + * $include_additional['include_deletd'] is TRUE, the array is keyed + * by field id, otherwise it is keyed by field name. */ function field_read_fields($params = array(), $include_additional = array()) { $query = db_select('field_config', 'fc', array('fetch' => PDO::FETCH_ASSOC)); @@ -270,7 +286,8 @@ function field_read_fields($params = arr if (!isset($include_additional['include_inactive']) || !$include_additional['include_inactive']) { $query->condition('fc.active', 1); } - if (!isset($include_additional['include_deleted']) || !$include_additional['include_deleted']) { + $skip_deleted = (!isset($include_additional['include_deleted']) || !$include_additional['include_deleted']); + if ($skip_deleted) { $query->condition('fc.deleted', 0); } @@ -285,7 +302,11 @@ function field_read_fields($params = arr // Populate storage columns. $field['columns'] = (array) module_invoke($field['module'], 'field_columns', $field); - $fields[$field['field_name']] = $field; + $field_name = $field['field_name']; + if (!$skip_deleted) { + $field_name = $field['id']; + } + $fields[$field_name] = $field; } return $fields; } @@ -298,18 +319,21 @@ function field_read_fields($params = arr * The field name to delete. */ function field_delete_field($field_name) { - // Mark the field for deletion. - db_update('field_config') + // Mark field storage for deletion. + module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_delete_field', $field_name); + + // Mark any instances of the field for deletion. + db_update('field_config_instance') ->fields(array('deleted' => 1)) ->condition('field_name', $field_name) ->execute(); - // Mark any instances of the field for deletion. - db_update('field_config_instance') + // Mark the field for deletion. + db_update('field_config') ->fields(array('deleted' => 1)) ->condition('field_name', $field_name) ->execute(); - module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_delete_field', $field_name); + // Clear the cache. field_cache_clear(TRUE); } @@ -320,6 +344,8 @@ function field_delete_field($field_name) * @param $instance * A field instance structure. The field_name and bundle properties * are required. + * @return + * The $instance structure with the id property filled in. * @throw * FieldException */ @@ -343,7 +369,7 @@ function field_create_instance($instance // Ensure the field instance is unique. // TODO : do we want specific messages when clashing with a disabled or inactive instance ? - $prior_instance = field_read_instance($instance['field_name'], $instance['bundle'], array('include_inactive' => TRUE, 'include_deleted' => TRUE)); + $prior_instance = field_read_instance($instance['field_name'], $instance['bundle'], array('include_inactive' => TRUE)); if (!empty($prior_instance)) { throw new FieldException('Attempt to create a field instance which already exists.'); } @@ -356,7 +382,7 @@ function field_create_instance($instance // Invoke external hooks after the cache is cleared for API consistency. module_invoke_all('field_create_instance', $instance); - return FALSE; + return $instance; } /* @@ -488,15 +514,20 @@ function _field_write_instance($instance * Read a single instance record directly from the database. Generally, * you should use the field_info_instance() instead. * + * This function will not return deleted instances. If multiple + * instances with the same field name have ever existed, deleted or + * not, and not all of the deleted instances have been cleaned up, there + * is no way with this function to control which one will you get. + * Use field_read_instances() instead for this purpose. + * * @param $field_name * The field name to read. * @param $bundle * The bundle to which the field is bound. * @param array $include_additional * The default behavior of this function is to not return an instance that - * is inactive or has been deleted. Setting - * $include_additional['include_inactive'] or - * $include_additional['include_deleted'] to TRUE will override this + * is inactive. Setting + * $include_additional['include_inactive'] to TRUE will override this * behavior. * @return * An instance structure, or FALSE. Index: modules/field/field.install =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.install,v retrieving revision 1.4 diff -u -F^[fc] -r1.4 field.install --- modules/field/field.install 10 Mar 2009 09:45:31 -0000 1.4 +++ modules/field/field.install 27 Apr 2009 22:30:50 -0000 @@ -24,7 +24,7 @@ function field_schema() { 'type' => 'varchar', 'length' => 32, 'not null' => TRUE, - 'description' => 'The name of this field', + 'description' => 'The name of this field. Non-deleted field names are unique, but multiple deleted fields can have the same name.', ), 'type' => array( 'type' => 'varchar', @@ -56,13 +56,13 @@ function field_schema() { 'type' => 'int', 'size' => 'tiny', 'not null' => TRUE, - 'default' => 0, + 'default' => 0, ), 'active' => array( 'type' => 'int', 'size' => 'tiny', 'not null' => TRUE, - 'default' => 0, + 'default' => 0, ), 'deleted' => array( 'type' => 'int', @@ -72,8 +72,9 @@ function field_schema() { ), ), 'primary key' => array('id'), - 'unique keys' => array('field_name' => array('field_name')), 'indexes' => array( + // used by field_delete_field() among others + 'field_name' => array('field_name'), // used by field_read_fields 'active_deleted' => array('active', 'deleted'), // used by field_modules_disabled @@ -124,7 +125,7 @@ function field_schema() { ), 'primary key' => array('id'), 'unique keys' => array( - 'field_name_bundle' => array('field_name', 'bundle'), + 'field_id_bundle' => array('field_id', 'bundle'), ), 'indexes' => array( // used by field_read_instances Index: modules/field/field.test =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.test,v retrieving revision 1.11 diff -u -F^[fc] -r1.11 field.test --- modules/field/field.test 20 Apr 2009 02:46:22 -0000 1.11 +++ modules/field/field.test 27 Apr 2009 22:30:50 -0000 @@ -14,10 +14,10 @@ class FieldAttachTestCase extends Drupal parent::setUp('field_sql_storage', 'field', 'field_test'); $this->field_name = drupal_strtolower($this->randomName(). '_field_name'); - $this->table = _field_sql_storage_tablename($this->field_name); - $this->revision_table = _field_sql_storage_revision_tablename($this->field_name); $this->field = array('field_name' => $this->field_name, 'type' => 'test_field', 'cardinality' => 4); - field_create_field($this->field); + $this->field = field_create_field($this->field); + $this->table = _field_sql_storage_tablename($this->field); + $this->revision_table = _field_sql_storage_revision_tablename($this->field); $this->instance = array( 'field_name' => $this->field_name, 'bundle' => 'test_bundle', @@ -418,10 +418,10 @@ class FieldAttachTestCase extends Drupal // Create a second field for the test bundle $field_name = drupal_strtolower($this->randomName(). '_field_name'); - $table = _field_sql_storage_tablename($field_name); - $revision_table = _field_sql_storage_revision_tablename($field_name); $field = array('field_name' => $field_name, 'type' => 'test_field', 'cardinality' => 1); - field_create_field($field); + $field = field_create_field($field); + $table = _field_sql_storage_tablename($field); + $revision_table = _field_sql_storage_revision_tablename($field); $instance = array( 'field_name' => $field_name, 'bundle' => $this->instance['bundle'], @@ -916,10 +916,10 @@ class FieldFormTestCase extends DrupalWe } -class FieldTestCase extends DrupalWebTestCase { +class FieldCrudTestCase extends DrupalWebTestCase { public static function getInfo() { return array( - 'name' => t('Field tests'), + 'name' => t('Field CRUD tests'), 'description' => t("Create / read /update a field."), 'group' => t('Field') ); @@ -961,7 +961,7 @@ class FieldTestCase extends DrupalWebTes 'field_name' => drupal_strtolower($this->randomName()), 'type' => 'test_field', ); - field_create_field($field_definition); + $field_definition = field_create_field($field_definition); $field = field_read_field($field_definition['field_name']); @@ -978,7 +978,9 @@ class FieldTestCase extends DrupalWebTes $this->assertIdentical($settings, $field['settings'] , t('Default field settings have been written.')); // Check that a table has been created for the field. - $this->assertTrue(db_table_exists('field_data_' . $field_definition['field_name']), t('A table has been created for the field.')); + // TODO: Reference to _field_sql_storage_tablename will be removed + // by [#392706]. + $this->assertTrue(db_table_exists(_field_sql_storage_tablename($field_definition)), t('A table has been created for the field.')); // Guarantee that the name is unique. try { @@ -1036,7 +1038,8 @@ class FieldTestCase extends DrupalWebTes // Make sure that the field is marked as deleted when it is specifically // loaded. - $field = field_read_field($this->field['field_name'], array('include_deleted' => TRUE)); + $fields = field_read_fields(array(), array('include_deleted' => TRUE)); + $field = current($field); $this->assertTrue(!empty($field['deleted']), t('A deleted field is marked for deletion.')); // Make sure that this field's instance is marked as deleted when it is @@ -1057,6 +1060,30 @@ class FieldTestCase extends DrupalWebTes $this->assertTrue(!empty($another_field) && empty($another_field['deleted']), t('A non-deleted field is not marked for deletion.')); $another_instance = field_read_instance($this->another_instance_definition['field_name'], $this->another_instance_definition['bundle']); $this->assertTrue(!empty($another_instance) && empty($another_instance['deleted']), t('An instance of a non-deleted field is not marked for deletion.')); + + // Try to create a new field the same name as a deleted field and + // write data into it. + field_create_field($this->field); + field_create_instance($this->instance_definition); + $field = field_read_field($this->field['field_name']); + $this->assertTrue(!empty($field) && empty($field['deleted']), t('A new field with a previously used name is created.')); + $instance = field_read_instance($this->instance_definition['field_name'], $this->instance_definition['bundle']); + $this->assertTrue(!empty($instance) && empty($instance['deleted']), t('A new instance for a previously used field name is created.')); + + // Save an object with data for the field + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $values[0]['value'] = mt_rand(1, 127); + $entity->{$field['field_name']} = $values; + $entity_type = 'test_entity'; + field_attach_insert($entity_type, $entity); + + // Verify the field is present on load + $entity = field_test_create_stub_entity(0, 0, $this->instance_definition['bundle']); + field_attach_load($entity_type, array(0 => $entity)); + $this->assertIdentical(count($entity->{$field['field_name']}), count($values), "Data in previously deleted field saves and loads correctly"); + foreach ($values as $delta => $value) { + $this->assertEqual($entity->{$field['field_name']}[$delta]['value'], $values[$delta]['value'], "Data in previously deleted field saves and loads correctly"); + } } } Index: modules/field/modules/field_sql_storage/field_sql_storage.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/field_sql_storage/field_sql_storage.module,v retrieving revision 1.6 diff -u -F^[fc] -r1.6 field_sql_storage.module --- modules/field/modules/field_sql_storage/field_sql_storage.module 30 Mar 2009 03:44:55 -0000 1.6 +++ modules/field/modules/field_sql_storage/field_sql_storage.module 27 Apr 2009 22:30:50 -0000 @@ -20,25 +20,25 @@ function field_sql_storage_help($path, $ /** * Generate a table name for a field data table. * - * @param $name - * The name of the field + * @param $field + * The field structure. * @return * A string containing the generated name for the database table */ -function _field_sql_storage_tablename($name) { - return 'field_data_' . $name; +function _field_sql_storage_tablename($field) { + return "field_data_{$field['field_name']}_{$field['id']}"; } /** * Generate a table name for a field revision archive table. * * @param $name - * The name of the field + * The field structure. * @return * A string containing the generated name for the database table */ -function _field_sql_storage_revision_tablename($name) { - return 'field_data_revision_' . $name; +function _field_sql_storage_revision_tablename($field) { + return "field_revision_{$field['field_name']}_{$field['id']}"; } /** @@ -147,8 +147,8 @@ function _field_sql_storage_schema($fiel $revision['primary key'] = array('etid', 'revision_id', 'deleted', 'delta'); return array( - _field_sql_storage_tablename($field['field_name']) => $current, - _field_sql_storage_revision_tablename($field['field_name']) => $revision, + _field_sql_storage_tablename($field) => $current, + _field_sql_storage_revision_tablename($field) => $revision, ); } @@ -167,7 +167,8 @@ function field_sql_storage_field_storage */ function field_sql_storage_field_storage_delete_field($field_name) { // Mark all data associated with the field for deletion. - $table = _field_sql_storage_tablename($field_name); + $field = field_info_field($field_name); + $table = _field_sql_storage_tablename($field); db_update($table) ->fields(array('deleted' => 1)) ->execute(); @@ -198,7 +199,7 @@ function field_sql_storage_field_storage } $field = field_info_field($field_name); - $table = $load_current ? _field_sql_storage_tablename($field_name) : _field_sql_storage_revision_tablename($field_name); + $table = $load_current ? _field_sql_storage_tablename($field) : _field_sql_storage_revision_tablename($field); $results = db_select($table, 't') ->fields('t') @@ -239,10 +240,10 @@ function field_sql_storage_field_storage if (isset($skip_fields[$field_name])) { continue; } - - $table_name = _field_sql_storage_tablename($field_name); - $revision_name = _field_sql_storage_revision_tablename($field_name); + $field = field_read_field($field_name); + $table_name = _field_sql_storage_tablename($field); + $revision_name = _field_sql_storage_revision_tablename($field); // Leave the field untouched if $object comes with no $field_name property. // Empty the field if $object->$field_name is NULL or an empty array. @@ -313,8 +314,9 @@ function field_sql_storage_field_storage $instances = field_info_instances($bundle); foreach ($instances as $instance) { $field_name = $instance['field_name']; - $table_name = _field_sql_storage_tablename($field_name); - $revision_name = _field_sql_storage_revision_tablename($field_name); + $field = field_read_field($field_name); + $table_name = _field_sql_storage_tablename($field); + $revision_name = _field_sql_storage_revision_tablename($field); db_delete($table_name) ->condition('etid', $etid) ->condition('entity_id', $id) @@ -339,7 +341,8 @@ function field_sql_storage_field_storage $instances = field_info_instances($bundle); foreach ($instances as $instance) { $field_name = $instance['field_name']; - $revision_name = _field_sql_storage_revision_tablename($field_name); + $field = field_read_field($field_name); + $revision_name = _field_sql_storage_revision_tablename($field); db_delete($revision_name) ->condition('etid', $etid) ->condition('entity_id', $id) @@ -355,8 +358,9 @@ function field_sql_storage_field_storage * This function simply marks for deletion all data associated with the field. */ function field_sql_storage_field_storage_delete_instance($field_name, $bundle) { - $table_name = _field_sql_storage_tablename($field_name); - $revision_name = _field_sql_storage_revision_tablename($field_name); + $field = field_read_field($field_name); + $table_name = _field_sql_storage_tablename($field); + $revision_name = _field_sql_storage_revision_tablename($field); db_update($table_name) ->fields(array('deleted' => 1)) ->condition('bundle', $bundle) @@ -373,8 +377,9 @@ function field_sql_storage_field_storage function field_sql_storage_field_storage_rename_bundle($bundle_old, $bundle_new) { $instances = field_info_instances($bundle_old); foreach ($instances as $instance) { - $table_name = _field_sql_storage_tablename($instance['field_name']); - $revision_name = _field_sql_storage_revision_tablename($instance['field_name']); + $field = field_read_field($instance['field_name']); + $table_name = _field_sql_storage_tablename($field); + $revision_name = _field_sql_storage_revision_tablename($field); db_update($table_name) ->fields(array('bundle' => $bundle_new)) ->condition('bundle', $bundle_old)