#1115106: failures to update a field are not properly handed in SQL Storage and Field UI.

From: Damien Tournoud <damien@commerceguys.com>


---
 .../field_sql_storage/field_sql_storage.module     |   36 +++++++++++++++++---
 .../field_sql_storage/field_sql_storage.test       |   25 ++++++++++++++
 modules/field_ui/field_ui.admin.inc                |   14 +++++---
 3 files changed, 64 insertions(+), 11 deletions(-)

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 6f49167..a4756dc 100644
--- a/modules/field/modules/field_sql_storage/field_sql_storage.module
+++ b/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -236,13 +236,37 @@ function field_sql_storage_field_update_forbid($field, $prior_field, $has_data)
 function field_sql_storage_field_storage_update_field($field, $prior_field, $has_data) {
   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($name, $table);
+
+    if (Database::getConnection()->supportsTransactionalDDL()) {
+      // If the database supports transactional DDL, we can go ahead and rely
+      // on it. If not, we will have to rollback manually if something fails.
+      $transaction = db_transaction();
+    }
+
+    try {
+      $prior_schema = _field_sql_storage_schema($prior_field);
+      foreach ($prior_schema as $name => $table) {
+        db_drop_table($name, $table);
+      }
+      $schema = _field_sql_storage_schema($field);
+      foreach ($schema as $name => $table) {
+        db_create_table($name, $table);
+      }
     }
-    $schema = _field_sql_storage_schema($field);
-    foreach ($schema as $name => $table) {
-      db_create_table($name, $table);
+    catch (Exception $e) {
+      if (Database::getConnection()->supportsTransactionalDDL()) {
+        $transaction->rollback();
+      }
+      else {
+        // Recreate tables.
+        $prior_schema = _field_sql_storage_schema($prior_field);
+        foreach ($prior_schema as $name => $table) {
+          if (!db_table_exists($name)) {
+            db_create_table($name, $table);
+          }
+        }
+      }
+      throw $e;
     }
   }
   else {
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 f94344f..773de3d 100644
--- a/modules/field/modules/field_sql_storage/field_sql_storage.test
+++ b/modules/field/modules/field_sql_storage/field_sql_storage.test
@@ -306,6 +306,31 @@ class FieldSqlStorageTestCase extends DrupalWebTestCase {
   }
 
   /**
+   * Test that failure to create fields is handled gracefully.
+   */
+  function testFieldUpdateFailure() {
+    // Create a text field.
+    $field = array('field_name' => 'test_text', 'type' => 'text', 'settings' => array('max_length' => 255));
+    $field = field_create_field($field);
+
+    // Attempt to update the field in a way that would break the storage.
+    $prior_field = $field;
+    $field['settings']['max_length'] = -1;
+    try {
+      field_update_field($field);
+      $this->fail(t('Update succeeded.'));
+    }
+    catch (Exception $e) {
+      $this->pass(t('Update properly failed.'));
+    }
+
+    // Ensure that the field tables are still there.
+    foreach (_field_sql_storage_schema($prior_field) as $table_name => $table_info) {
+      $this->assertTrue(db_table_exists($table_name), t('Table %table exists.', array('%table' => $table_name)));
+    }
+  }
+
+  /**
    * Test adding and removing indexes while data is present.
    */
   function testFieldUpdateIndexesWithData() {
diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc
index 96beb13..386c4d5 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -1576,10 +1576,8 @@ function field_ui_field_settings_form_submit($form, &$form_state) {
     drupal_set_message(t('Updated field %label field settings.', array('%label' => $instance['label'])));
     $form_state['redirect'] = field_ui_next_destination($entity_type, $bundle);
   }
-  catch (FieldException $e) {
+  catch (Exception $e) {
     drupal_set_message(t('Attempt to update field %label failed: %message.', array('%label' => $instance['label'], '%message' => $e->getMessage())), 'error');
-    // TODO: Where do we go from here?
-    $form_state['redirect'] = field_ui_next_destination($entity_type, $bundle);
   }
 }
 
@@ -1650,7 +1648,7 @@ function field_ui_widget_type_form_submit($form, &$form_state) {
     field_update_instance($instance);
     drupal_set_message(t('Changed the widget for field %label.', array('%label' => $instance['label'])));
   }
-  catch (FieldException $e) {
+  catch (Exception $e) {
     drupal_set_message(t('There was a problem changing the widget for field %label.', array('%label' => $instance['label'])));
   }
 
@@ -1970,7 +1968,13 @@ function field_ui_field_edit_form_submit($form, &$form_state) {
   // Update any field settings that have changed.
   $field_source = field_info_field($instance['field_name']);
   $field = array_merge($field_source, $field);
-  field_update_field($field);
+  try {
+    field_update_field($field);
+  }
+  catch (Exception $e) {
+    drupal_set_message(t('Attempt to update field %label failed: %message.', array('%label' => $instance['label'], '%message' => $e->getMessage())), 'error');
+    return;
+  }
 
   // Handle the default value.
   if (isset($form['instance']['default_value_widget'])) {