- Patch #367013 by bjaspan, yched: clean ups for field_update_field().

8 files changed, 47 insertions, 43 deletions

diff --git a/modules/field/field.api.php b/modules/field/field.api.php
index 380444af1..86c8da924 100644
--- a/modules/field/field.api.php
+++ b/modules/field/field.api.php
@@ -1315,7 +1315,7 @@ function hook_field_create_instance($instance) {
  * @param $has_data
  *   Whether any data already exists for this field.
  * @return
- *   Throws a FieldException to prevent the update from occuring.
+ *   Throws a FieldUpdateForbiddenException 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
@@ -1328,7 +1328,7 @@ function hook_field_update_field_forbid($field, $prior_field, $has_data) {
     // 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");
+      throw new FieldUpdateForbiddenException("Cannot update a list field not to include keys with existing data");
     }
   }
 }
diff --git a/modules/field/field.attach.inc b/modules/field/field.attach.inc
index 5b5d31ed4..4b5441904 100644
--- a/modules/field/field.attach.inc
+++ b/modules/field/field.attach.inc
@@ -1110,19 +1110,6 @@ 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 89d0b2d37..fe5bb282d 100644
--- a/modules/field/field.crud.inc
+++ b/modules/field/field.crud.inc
@@ -410,7 +410,7 @@ function field_update_field($field) {
   );
   $field['indexes'] += $schema['indexes'];
 
-  $has_data = field_attach_field_has_data($field);
+  $has_data = field_has_data($field);
 
   // See if any module forbids the update by throwing an exception.
   foreach (module_implements('field_update_forbid') as $module) {
diff --git a/modules/field/field.module b/modules/field/field.module
index 4d031e935..a4e9e4d12 100644
--- a/modules/field/field.module
+++ b/modules/field/field.module
@@ -131,6 +131,11 @@ define('FIELD_QUERY_COMPLETE', 'FIELD_QUERY_COMPLETE');
 class FieldException extends Exception {}
 
 /**
+ * Exception class thrown by hook_field_update_forbid().
+ */
+class FieldUpdateForbiddenException extends FieldException {}
+
+/**
  * Implement hook_flush_caches.
  */
 function field_flush_caches() {
@@ -611,6 +616,19 @@ function field_view_field($obj_type, $object, $field, $instance, $build_mode = '
 }
 
 /**
+ * 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_has_data($field) {
+  $results = field_attach_query($field['id'], array(), 1);
+  return !empty($results);
+}
+
+/**
  * Determine whether the user has access to a given field.
  *
  * @param $op
diff --git a/modules/field/field.test b/modules/field/field.test
index 2c1b74bf4..7c1d2da6b 100644
--- a/modules/field/field.test
+++ b/modules/field/field.test
@@ -1874,30 +1874,6 @@ class FieldCrudTestCase extends FieldTestCase {
   }
 
   /**
-   * 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() {
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 fd06e2ea1..5cb49875e 100644
--- a/modules/field/modules/field_sql_storage/field_sql_storage.module
+++ b/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -223,7 +223,7 @@ function field_sql_storage_field_storage_create_field($field) {
  */
 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.");
+    throw new FieldUpdateForbiddenException("field_sql_storage cannot change the schema for an existing field with data.");
   }
 }
 
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 7108913da..f5a98cbf9 100644
--- a/modules/field/modules/field_sql_storage/field_sql_storage.test
+++ b/modules/field/modules/field_sql_storage/field_sql_storage.test
@@ -295,6 +295,29 @@ class FieldSqlStorageTestCase extends DrupalWebTestCase {
     $this->assertEqual($count, 1, 'NULL field translation is wiped.');
   }
 
+  /**
+   * Test trying to update 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 adding and removing indexes while data is present.
diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc
index 3096eb602..ba2734ac4 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -1115,7 +1115,7 @@ function field_ui_field_edit_form($form, &$form_state, $obj_type, $bundle, $inst
   $description .= $info['description'] . '</p>';
   $form['#prefix'] = '<div class="description">' . $description . '</div>';
 
-  $has_data = field_attach_field_has_data($field);
+  $has_data = field_has_data($field);
   if ($has_data) {
     $description = '<p>' . t('These settings apply to the %field field everywhere it is used. Because the field already has data, some settings can no longer be changed.', array('%field' => $instance['label'])) . '</p>';
   }
@@ -1144,7 +1144,7 @@ function field_ui_field_edit_form($form, &$form_state, $obj_type, $bundle, $inst
     '#description' => $description,
   );
 
-  // Add additional field settings from the field module. The field module is
+  // Add additional field type settings. The field type module is
   // responsible for not returning settings that cannot be changed if
   // the field already has data.
   $additions = module_invoke($field['module'], 'field_settings_form', $field, $instance, $has_data);