#915906 by jbrown, yched: Fixed Deleting node type with only instance of a field leaves the field in a strange zombie state

3 files changed, 42 insertions, 30 deletions

diff --git a/modules/field/field.crud.inc b/modules/field/field.crud.inc
index 21e000cf2..5ff274826 100644
--- a/modules/field/field.crud.inc
+++ b/modules/field/field.crud.inc
@@ -588,7 +588,7 @@ function field_delete_field($field_name) {
     foreach ($field['bundles'] as $entity_type => $bundles) {
       foreach ($bundles as $bundle) {
         $instance = field_info_instance($entity_type, $field_name, $bundle);
-        field_delete_instance($instance);
+        field_delete_instance($instance, FALSE);
       }
     }
   }
@@ -923,8 +923,12 @@ function field_read_instances($params = array(), $include_additional = array())
  *
  * @param $instance
  *   An instance structure.
+ * @param $field_cleanup
+ *   If TRUE, the field will be deleted as well if its last instance is being
+ *   deleted. If FALSE, it is the caller's responsability to handle the case of
+ *   fields left without instances. Defaults to TRUE.
  */
-function field_delete_instance($instance) {
+function field_delete_instance($instance, $field_cleanup = TRUE) {
   // Mark the field instance for deletion.
   db_update('field_config_instance')
     ->fields(array('deleted' => 1))
@@ -933,14 +937,20 @@ function field_delete_instance($instance) {
     ->condition('bundle', $instance['bundle'])
     ->execute();
 
+  // Clear the cache.
+  field_cache_clear();
+
   // Mark instance data for deletion.
   $field = field_info_field($instance['field_name']);
   module_invoke($field['storage']['module'], 'field_storage_delete_instance', $instance);
 
-  // Clear the cache.
-  field_cache_clear();
-
+  // Let modules react to the deletion of the instance.
   module_invoke_all('field_delete_instance', $instance);
+
+  // Delete the field itself if we just deleted its last instance.
+  if ($field_cleanup && count($field['bundles']) == 0) {
+    field_delete_field($field['field_name']);
+  }
 }
 
 /**
@@ -1058,12 +1068,11 @@ function field_purge_batch($batch_size) {
     }
   }
 
-  // Retrieve all deleted fields. Any that have no bundles can be purged.
+  // Retrieve all deleted fields. Any that have no instances can be purged.
   $fields = field_read_fields(array('deleted' => 1), array('include_deleted' => 1));
   foreach ($fields as $field) {
-    // field_read_fields() does not return $field['bundles'] which we need.
-    $field = field_info_field_by_id($field['id']);
-    if (!isset($field['bundles']) || count($field['bundles']) == 0) {
+    $instances = field_read_instances(array('field_id' => $field['id']), array('include_deleted' => 1));
+    if (empty($instances)) {
       field_purge_field($field);
     }
   }
diff --git a/modules/field/tests/field.test b/modules/field/tests/field.test
index ccf8d2efe..05a6ede01 100644
--- a/modules/field/tests/field.test
+++ b/modules/field/tests/field.test
@@ -2545,6 +2545,11 @@ class FieldInstanceCrudTestCase extends FieldTestCase {
     // Make sure the other field instance is not deleted.
     $another_instance = field_read_instance('test_entity', $this->another_instance_definition['field_name'], $this->another_instance_definition['bundle']);
     $this->assertTrue(!empty($another_instance) && empty($another_instance['deleted']), t('A non-deleted field instance is not marked for deletion.'));
+
+    // Make sure the field is deleted when its last instance is deleted.
+    field_delete_instance($another_instance);
+    $field = field_read_field($another_instance['field_name'], array('include_deleted' => TRUE));
+    $this->assertTrue(!empty($field['deleted']), t('A deleted field is marked for deletion after all its instances have been marked for deletion.'));
   }
 }
 
@@ -3081,30 +3086,32 @@ class FieldBulkDeleteTestCase extends FieldTestCase {
   function testPurgeField() {
     $field = reset($this->fields);
 
-    foreach ($this->bundles as $bundle) {
-      // Delete the instance.
-      $instance = field_info_instance($this->entity_type, $field['field_name'], $bundle);
-      field_delete_instance($instance);
+    // Delete the first instance.
+    $instance = field_info_instance($this->entity_type, $field['field_name'], 'bb_1');
+    field_delete_instance($instance);
+
+    // Purge the data.
+    field_purge_batch(10);
 
-      // Purge the data.
-      field_purge_batch(10);
+    // Purge again to purge the instance.
+    field_purge_batch(0);
 
-      // Purge again to purge the instance.
-      field_purge_batch(0);
+    // The field still exists, not deleted.
+    $fields = field_read_fields(array('id' => $field['id']), array('include_deleted' => 1));
+    $this->assertTrue(isset($fields[$field['id']]) && !$fields[$field['id']]['deleted'], 'The field exists and is not deleted');
 
-      // The field still exists, not deleted, because it was never deleted.
-      $fields = field_read_fields(array('id' => $field['id']), array('include_deleted' => 1, 'include_inactive' => 1));
-      $this->assertTrue(isset($fields[$field['id']]), 'The field exists and is not deleted');
-    }
+    // Delete the second instance.
+    $instance = field_info_instance($this->entity_type, $field['field_name'], 'bb_2');
+    field_delete_instance($instance);
 
-    // Delete the field.
-    field_delete_field($field['field_name']);
+    // Purge the data.
+    field_purge_batch(10);
 
     // The field still exists, deleted.
-    $fields = field_read_fields(array('id' => $field['id']), array('include_deleted' => 1, 'include_inactive' => 1));
-    $this->assertEqual($fields[$field['id']]['deleted'], 1, 'The field exists and is deleted');
+    $fields = field_read_fields(array('id' => $field['id']), array('include_deleted' => 1));
+    $this->assertTrue(isset($fields[$field['id']]) && $fields[$field['id']]['deleted'], 'The field exists and is deleted');
 
-    // Purge the field.
+    // Purge again to purge the instance and the field.
     field_purge_batch(0);
 
     // The field is gone.
diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc
index fdddb018a..6274d5ab1 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -1706,10 +1706,6 @@ function field_ui_field_delete_form_submit($form, &$form_state) {
 
   if (!empty($bundle) && $field && !$field['locked'] && $form_values['confirm']) {
     field_delete_instance($instance);
-    // Delete the field if that was the last instance.
-    if (count($field['bundles']) == 1 && count(current($field['bundles'])) == 1) {
-      field_delete_field($field['field_name']);
-    }
     drupal_set_message(t('The field %field has been deleted from the %type content type.', array('%field' => $instance['label'], '%type' => $bundle_label)));
   }
   else {