Issue #1115106 by Damien Tournoud: Fixed Failures to update a field are not properly handed in SQL Storage and Field UI.

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 695561624..2ed783507 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 f94344fa2..773de3d07 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 ce84d83e5..f4fd3d7ba 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -1597,10 +1597,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);
   }
 }
 
@@ -1671,7 +1669,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'])));
   }
 
@@ -1999,7 +1997,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'])) {