#898520 follow-up by Damien Tournoud, chx, David Rothstein: Clean-up the upgrade path: comment.

12 files changed, 257 insertions, 126 deletions

diff --git a/includes/update.inc b/includes/update.inc
index 6b700700c..ff7fc696c 100644
--- a/includes/update.inc
+++ b/includes/update.inc
@@ -1315,3 +1315,16 @@ function update_retrieve_dependencies() {
   return $return;
 }
 
+/**
+ * @defgroup update-api-6.x-to-7.x Update versions of API functions.
+ * @{
+ * Functions similar to normal API function but not firing hooks.
+ *
+ * During update, it is impossible to judge the consequences of firing a hook
+ * as it might hit a module not yet updated. So simplified versions of some
+ * core APIs are provided.
+ */
+
+/**
+ * @} End of "defgroup update-api-6.x-to-7.x"
+ */
diff --git a/modules/comment/comment.install b/modules/comment/comment.install
index 412c8fc05..03b21860c 100644
--- a/modules/comment/comment.install
+++ b/modules/comment/comment.install
@@ -81,19 +81,9 @@ function comment_enable() {
  * Implements hook_update_dependencies().
  */
 function comment_update_dependencies() {
-  // Comment update 7005 creates comment Field API bundles and therefore must
-  // run after the Field module has been enabled, but before upgrading field
-  // data.
-  $dependencies['comment'][7005] = array(
-    'system' => 7049,
-  );
-  $dependencies['system'][7050] = array(
-    'comment' => 7005,
-  );
-
-  // Comment update 7012 creates the comment body field and therefore must run
+  // Comment update 7005 creates the comment body field and therefore must run
   // after text module has been enabled and entities have been updated.
-  $dependencies['comment'][7012] = array(
+  $dependencies['comment'][7005] = array(
     'system' => 7021,
   );
 
@@ -106,14 +96,36 @@ function comment_update_dependencies() {
  */
 
 /**
- * Remove comment settings for page ordering.
+ * Rename comment display setting variables.
  */
 function comment_update_7000() {
-  $types = node_type_get_types();
-  foreach ($types as $type => $object) {
+  $types = _update_7000_node_get_types();
+  foreach ($types as $type => $type_object) {
     variable_del('comment_default_order' . $type);
+
+    // Drupal 6 had four display modes:
+    // - COMMENT_MODE_FLAT_COLLAPSED = 1
+    // - COMMENT_MODE_FLAT_EXPANDED = 2
+    // - COMMENT_MODE_THREADED_COLLAPSED = 3
+    // - COMMENT_MODE_THREADED_EXPANDED = 4
+    //
+    // Drupal 7 doesn't support collapsed/expanded modes anymore, so we
+    // migrate all the flat modes to COMMENT_MODE_FLAT (0) and all the threaded
+    // modes to COMMENT_MODE_THREADED (1).
+    $setting = variable_get('comment_default_mode_' . $type, 4);
+    if ($setting == 3 || $setting == 4) {
+      variable_set('comment_default_mode_' . $type, 1);
+    }
+    else {
+      variable_set('comment_default_mode_' . $type, 0);
+    }
+
+    // There were only two comment modes in the past:
+    // - 1 was 'required' previously, convert into DRUPAL_REQUIRED (2).
+    // - 0 was 'optional' previously, convert into DRUPAL_OPTIONAL (1).
+    $preview = variable_get('comment_preview_' . $type, 1) ? 2 : 1;
+    variable_set('comment_preview_' . $type, $preview);
   }
-  return t('Comment order settings removed.');
 }
 
 /**
@@ -138,49 +150,29 @@ function comment_update_7001() {
 }
 
 /**
- * Rename {comments} table to {comment}.
+ * Rename {comments} table to {comment} and upgrade it.
  */
 function comment_update_7002() {
   db_rename_table('comments', 'comment');
-}
-
-/**
- * Rename comment display setting variables.
- */
-function comment_update_7004() {
-  $types = node_type_get_types();
-  foreach ($types as $type => $object) {
-    $setting = variable_get('comment_default_mode_' . $type, 4);
-    if ($setting == 3 || $setting == 4) {
-      variable_set('comment_default_mode_' . $type, 1);
-    }
-    else {
-      variable_set('comment_default_mode_' . $type, 0);
-    }
-  }
-}
 
-/**
- * Create comment Field API bundles.
- */
-function comment_update_7005() {
-  foreach (node_type_get_types() as $info) {
-    field_attach_create_bundle('comment', 'comment_node_' . $info->type);
-  }
-}
-
-/**
- * Create user related indexes.
- */
-function comment_update_7006() {
+  // Add user-related indexes.
   db_add_index('comment', 'comment_uid', array('uid'));
   db_add_index('node_comment_statistics', 'last_comment_uid', array('last_comment_uid'));
+
+  // Create a language column.
+  db_add_field('comment', 'language', array(
+    'type' => 'varchar',
+    'length' => 12,
+    'not null' => TRUE,
+    'default' => '',
+  ));
+  db_add_index('comment', 'comment_nid_language', array('nid', 'language'));
 }
 
 /**
  * Split {comment}.timestamp into 'created' and 'changed', improve indexing on {comment}.
  */
-function comment_update_7007() {
+function comment_update_7003() {
   // Drop the old indexes.
   db_drop_index('comment', 'status');
   db_drop_index('comment', 'pid');
@@ -211,45 +203,9 @@ function comment_update_7007() {
 }
 
 /**
- * Add language column to the {comment} table.
- */
-function comment_update_7008() {
-  // Create a language column.
-  db_add_field('comment', 'language', array(
-    'type' => 'varchar',
-    'length' => 12,
-    'not null' => TRUE,
-    'default' => '',
-  ));
-
-  // Create the index.
-  db_add_index('comment', 'comment_nid_language', array('nid', 'language'));
-}
-
-/**
- * Update preview setting variable to use new constants
+ * Upgrade the {node_comment_statistics} table.
  */
-function comment_update_7009() {
-  foreach (node_type_get_types() as $type => $object) {
-    // There were only two comment modes in the past:
-    // - 1 was 'required' previously, convert into DRUPAL_REQUIRED (2).
-    // - 0 was 'optional' previously, convert into DRUPAL_OPTIONAL (1).
-    $original_preview = variable_get('comment_preview_' . $type, 1);
-    if ($original_preview) {
-      $preview = DRUPAL_REQUIRED;
-    }
-    else {
-      $preview = DRUPAL_OPTIONAL;
-    }
-    variable_set('comment_preview_' . $type, $preview);
-  }
-  return array();
-}
-
-/**
- * Add {node_comment_statistics}.cid column.
- */
-function comment_update_7010() {
+function comment_update_7004() {
   db_add_field('node_comment_statistics', 'cid', array(
     'type' => 'int',
     'not null' => TRUE,
@@ -257,59 +213,75 @@ function comment_update_7010() {
     'description' => 'The {comment}.cid of the last comment.',
   ));
   db_add_index('node_comment_statistics', 'cid', array('cid'));
-}
 
-/**
- * Add an index to node_comment_statistics on comment_count.
- */
-function comment_update_7011() {
+  // Add an index on the comment_count.
   db_add_index('node_comment_statistics', 'comment_count', array('comment_count'));
 }
 
 /**
  * Create the comment_body field.
  */
-function comment_update_7012() {
+function comment_update_7005() {
   // Create comment body field.
   $field = array(
     'field_name' => 'comment_body',
     'type' => 'text_long',
-    'entity_types' => array('comment'),
+    'module' => 'text',
+    'entity_types' => array(
+      'comment',
+    ),
+    'settings' => array(),
+    'cardinality' => 1,
   );
-  field_create_field($field);
+  _update_7000_field_create_field($field);
 
   // Add the field to comments for all existing bundles.
-  $body_instance = array(
-    'field_name' => 'comment_body',
-    'label' => 'Comment',
+  $generic_instance = array(
     'entity_type' => 'comment',
-    'settings' => array('text_processing' => 1),
+    'label' => t('Comment'),
+    'settings' => array(
+      'text_processing' => 1,
+    ),
     'required' => TRUE,
     'display' => array(
       'default' => array(
         'label' => 'hidden',
         'type' => 'text_default',
         'weight' => 0,
+        'settings' => array(),
+        'module' => 'text',
       ),
     ),
+    'widget' => array(
+      'type' => 'text_textarea',
+      'settings' => array(
+        'rows' => 5,
+      ),
+      'weight' => 0,
+      'module' => 'text',
+    ),
+    'description' => '',
   );
-  foreach (node_type_get_types() as $info) {
-    $body_instance['bundle'] = 'comment_node_' . $info->type;
-    field_create_instance($body_instance);
+
+  $types = _update_7000_node_get_types();
+  foreach ($types as $type => $type_object) {
+    $instance = $generic_instance;
+    $instance['bundle'] = 'comment_node_' . $type;
+    _update_7000_field_create_instance($field, $instance);
   }
 }
 
 /**
  * Migrate data from the comment field to field storage.
  */
-function comment_update_7013(&$sandbox) {
+function comment_update_7006(&$sandbox) {
   // This is a multipass update. First set up some comment variables.
   if (empty($sandbox['total'])) {
     $comments = (bool) db_query_range('SELECT 1 FROM {comment}', 0, 1)->fetchField();
     $sandbox['types'] = array();
     if ($comments) {
       $sandbox['etid'] = _field_sql_storage_etid('comment');
-      $sandbox['types'] = node_type_get_types();
+      $sandbox['types'] = array_keys(_update_7000_node_get_types());
     }
     $sandbox['total'] = count($sandbox['types']);
   }
@@ -318,9 +290,9 @@ function comment_update_7013(&$sandbox) {
     $type = array_shift($sandbox['types']);
 
     $query = db_select('comment', 'c');
-    $query->innerJoin('node', 'n', 'c.nid = n.nid AND n.type = :type', array(':type' => $type->type));
+    $query->innerJoin('node', 'n', 'c.nid = n.nid AND n.type = :type', array(':type' => $type));
     $query->addField('c', 'cid', 'entity_id');
-    $query->addExpression("'comment_node_$type->type'", 'bundle');
+    $query->addExpression("'comment_node_$type'", 'bundle');
     $query->addExpression($sandbox['etid'], 'etid');
     $query->addExpression('0', 'deleted');
     $query->addExpression("'" . LANGUAGE_NONE . "'", 'language');
@@ -328,8 +300,7 @@ function comment_update_7013(&$sandbox) {
     $query->addField('c', 'comment', 'comment_body_value');
     $query->addField('c', 'format', 'comment_body_format');
 
-    $comment_body = field_info_field('comment_body');
-    $comment_body_table = _field_sql_storage_tablename($comment_body);
+    $comment_body_table = 'field_data_comment_body';
 
     db_insert($comment_body_table)
       ->from($query)
diff --git a/modules/field/field.install b/modules/field/field.install
index 8a0c5ba4e..4178f6da7 100644
--- a/modules/field/field.install
+++ b/modules/field/field.install
@@ -167,3 +167,127 @@ function field_schema() {
 
   return $schema;
 }
+
+/**
+ * Utility function: create a field by writing directly to the database.
+ *
+ * This function is valid for a database schema version 7000.
+ *
+ * @ingroup update-api-6.x-to-7.x
+ */
+function _update_7000_field_create_field(&$field) {
+  // Merge in default values.`
+  $field += array(
+    'entity_types' => array(),
+    'cardinality' => 1,
+    'translatable' => FALSE,
+    'locked' => FALSE,
+    'settings' => array(),
+    'indexes' => array(),
+    'deleted' => 0,
+    'active' => 1,
+  );
+
+  // Set storage.
+  $field['storage'] = array(
+    'type' => 'field_sql_storage',
+    'settings' => array(),
+    'module' => 'field_sql_storage',
+    'active' => 1,
+  );
+
+  // The serialized 'data' column contains everything from $field that does not
+  // have its own column and is not automatically populated when the field is
+  // read.
+  $data = $field;
+  unset($data['columns'], $data['field_name'], $data['type'], $data['active'], $data['module'], $data['storage_type'], $data['storage_active'], $data['storage_module'], $data['locked'], $data['cardinality'], $data['deleted']);
+  // Additionally, do not save the 'bundles' property populated by
+  // field_info_field().
+  unset($data['bundles']);
+
+  // Write the field to the database.
+  $record = array(
+    'field_name' => $field['field_name'],
+    'type' => $field['type'],
+    'module' => $field['module'],
+    'active' => (int) $field['active'],
+    'storage_type' => $field['storage']['type'],
+    'storage_module' => $field['storage']['module'],
+    'storage_active' => (int) $field['storage']['active'],
+    'locked' => (int) $field['locked'],
+    'data' => serialize($data),
+    'cardinality' => $field['cardinality'],
+    'translatable' => (int) $field['translatable'],
+    'deleted' => (int) $field['deleted'],
+  );
+  // We don't use drupal_write_record() here because it depends on the schema.
+  $field['id'] = db_insert('field_config')
+    ->fields($record)
+    ->execute();
+
+  // Create storage for this field, the field module is not guaranteed to be
+  // loaded at this point.
+  module_load_install($field['module']);
+  $schema = (array) module_invoke($field['module'], 'field_schema', $field);
+  $schema += array('columns' => array(), 'indexes' => array());
+  // 'columns' are hardcoded in the field type.
+  $field['columns'] = $schema['columns'];
+  // 'indexes' can be both hardcoded in the field type, and specified in the
+  // incoming $field definition.
+  $field['indexes'] += $schema['indexes'];
+
+  field_sql_storage_field_storage_create_field($field);
+}
+
+/**
+ * Utility function: create a field instance directly to the database.
+ *
+ * This function is valid for a database schema version 7000.
+ *
+ * @ingroup update-api-6.x-to-7.x
+ */
+function _update_7000_field_create_instance($field, &$instance) {
+  // Merge in defaults.
+  $instance += array(
+    'field_id' => $field['id'],
+    'field_name' => $field['field_name'],
+    'deleted' => 0,
+  );
+
+  // The serialized 'data' column contains everything from $instance that does
+  // not have its own column and is not automatically populated when the
+  // instance is read.
+  $data = $instance;
+  unset($data['id'], $data['field_id'], $data['field_name'], $data['entity_type'], $data['bundle'], $data['deleted']);
+
+  $record = array(
+    'field_id' => $instance['field_id'],
+    'field_name' => $instance['field_name'],
+    'entity_type' => $instance['entity_type'],
+    'bundle' => $instance['bundle'],
+    'data' => serialize($data),
+    'deleted' => (int) $instance['deleted'],
+  );
+  $instance['id'] = db_insert('field_config_instance')
+    ->fields($record)
+    ->execute();
+}
+
+/**
+ * @defgroup field-updates-6.x-to-7.x Field updates from 6.x to 7.x
+ * @{
+ */
+
+/**
+ * Field update version placeholder.
+ */
+function field_update_7000() {
+  // _update_7000_field_create_field() is supposed to create fields according
+  // to the structure of fields after running field_update_7000(). So this
+  // function needs to exist but as field is a new module in Drupal 7, it
+  // does not need to do anything.
+}
+
+/**
+ * @} End of "defgroup field-updates-6.x-to-7.x"
+ */
diff --git a/modules/filter/filter.install b/modules/filter/filter.install
index dcee618f5..52334a4fa 100644
--- a/modules/filter/filter.install
+++ b/modules/filter/filter.install
@@ -343,7 +343,7 @@ function filter_update_7005() {
     $format_roles = ($format->format == $default_format ? $all_roles : explode(',', $format->roles));
     foreach ($format_roles as $format_role) {
       if (in_array($format_role, $all_roles)) {
-        _update_user_role_grant_permissions($format_role, array('use text format ' . $format->format), 'filter');
+        _update_7000_user_role_grant_permissions($format_role, array('use text format ' . $format->format), 'filter');
       }
     }
   }
@@ -438,7 +438,7 @@ function filter_update_7008() {
   // that they do not or must not.
   if ($roles = user_roles(FALSE, 'administer filters')) {
     foreach ($roles as $rid => $name) {
-      _update_user_role_grant_permissions($rid, $permissions, 'filter');
+      _update_7000_user_role_grant_permissions($rid, $permissions, 'filter');
     }
   }
 }
diff --git a/modules/node/node.install b/modules/node/node.install
index e51e7f15c..c04f9b7ef 100644
--- a/modules/node/node.install
+++ b/modules/node/node.install
@@ -419,6 +419,17 @@ function node_update_dependencies() {
 }
 
 /**
+ * Utility function: fetch the node types directly from the database.
+ *
+ * This function is valid for a database schema version 7000.
+ *
+ * @ingroup update-api-6.x-to-7.x
+ */
+function _update_7000_node_get_types() {
+  return db_query('SELECT * FROM {node_type}')->fetchAllAssoc('type', PDO::FETCH_OBJ);
+}
+
+/**
  * @defgroup updates-6.x-to-7.x System updates from 6.x to 7.x
  * @{
  */
diff --git a/modules/simpletest/simpletest.info b/modules/simpletest/simpletest.info
index 94c1b8006..139e49cd0 100644
--- a/modules/simpletest/simpletest.info
+++ b/modules/simpletest/simpletest.info
@@ -39,5 +39,6 @@ files[] = tests/unicode.test
 files[] = tests/update.test
 files[] = tests/xmlrpc.test
 files[] = tests/upgrade/upgrade.test
+files[] = tests/upgrade/upgrade.comment.test
 files[] = tests/upgrade/upgrade.node.test
 files[] = tests/upgrade/upgrade.taxonomy.test
diff --git a/modules/simpletest/tests/upgrade/upgrade.node.test b/modules/simpletest/tests/upgrade/upgrade.node.test
index 0299ffa2a..f5cb3dffb 100644
--- a/modules/simpletest/tests/upgrade/upgrade.node.test
+++ b/modules/simpletest/tests/upgrade/upgrade.node.test
@@ -17,7 +17,9 @@ class NodeBodyUpgradePathTestCase extends UpgradePathTestCase {
 
   public function setUp() {
     // Path to the database dump.
-    $this->databaseDumpFile = drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php';
+    $this->databaseDumpFiles = array(
+      drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php',
+    );
     parent::setUp();
   }
 
@@ -51,7 +53,9 @@ class PollUpgradePathTestCase extends UpgradePathTestCase {
 
   public function setUp() {
     // Path to the database dump.
-    $this->databaseDumpFile = drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php';
+    $this->databaseDumpFiles = array(
+      drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php',
+    );
     parent::setUp();
 
     $this->uninstallModulesExcept(array('poll'));
diff --git a/modules/simpletest/tests/upgrade/upgrade.poll.test b/modules/simpletest/tests/upgrade/upgrade.poll.test
index b1df28283..12aff8bd8 100644
--- a/modules/simpletest/tests/upgrade/upgrade.poll.test
+++ b/modules/simpletest/tests/upgrade/upgrade.poll.test
@@ -20,8 +20,10 @@ class PollUpgradePathTestCase extends UpgradePathTestCase {
   }
 
   public function setUp() {
-    // Path to the database dump.
-    $this->databaseDumpFile = drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php';
+    // Path to the database dump files.
+    $this->databaseDumpFiles = array(
+      drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php',
+    );
     parent::setUp();
 
     $this->uninstallModulesExcept(array('poll'));
diff --git a/modules/simpletest/tests/upgrade/upgrade.taxonomy.test b/modules/simpletest/tests/upgrade/upgrade.taxonomy.test
index 616023532..70e006269 100644
--- a/modules/simpletest/tests/upgrade/upgrade.taxonomy.test
+++ b/modules/simpletest/tests/upgrade/upgrade.taxonomy.test
@@ -15,7 +15,9 @@ class UpgradePathTaxonomyTestCase extends UpgradePathTestCase {
 
   public function setUp() {
     // Path to the database dump.
-    $this->databaseDumpFile = drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php';
+    $this->databaseDumpFiles = array(
+     drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.filled.database.php',
+    );
     parent::setUp();
   }
 
diff --git a/modules/simpletest/tests/upgrade/upgrade.test b/modules/simpletest/tests/upgrade/upgrade.test
index 47d63a22c..481f7b90a 100644
--- a/modules/simpletest/tests/upgrade/upgrade.test
+++ b/modules/simpletest/tests/upgrade/upgrade.test
@@ -7,9 +7,11 @@
 abstract class UpgradePathTestCase extends DrupalWebTestCase {
 
   /**
-   * The file path to the dumped database to load into the child site.
+   * The file path(s) to the dumped database(s) to load into the child site.
+   *
+   * @var array
    */
-  var $databaseDumpFile = NULL;
+  var $databaseDumpFiles = array();
 
   /**
    * Flag that indicates whether the child site has been upgraded.
@@ -88,7 +90,9 @@ abstract class UpgradePathTestCase extends DrupalWebTestCase {
     $conf = array();
 
     // Load the database from the portable PHP dump.
-    require $this->databaseDumpFile;
+    foreach ($this->databaseDumpFiles as $file) {
+      require $file;
+    }
 
     // Set path variables.
     $this->variable_set('file_public_path', $public_files_directory);
@@ -328,8 +332,10 @@ class BasicUpgradePath extends UpgradePathTestCase {
   }
 
   public function setUp() {
-    // Path to the database dump.
-    $this->databaseDumpFile = drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.bare.database.php';
+    // Path to the database dump files.
+    $this->databaseDumpFiles = array(
+      drupal_get_path('module', 'simpletest') . '/tests/upgrade/drupal-6.bare.database.php',
+    );
     parent::setUp();
   }
 
diff --git a/modules/system/system.install b/modules/system/system.install
index e719e6bbb..858e576e5 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -2142,12 +2142,6 @@ function system_update_7029() {
 }
 
 /**
- * Moved to comment_update_7011().
- */
-function system_update_7030() {
-}
-
-/**
  * Removed in favour of Drupal 6 backport.
  *
  * @see system_update_6052()
diff --git a/modules/user/user.install b/modules/user/user.install
index 1bb03af7a..d3ec23740 100644
--- a/modules/user/user.install
+++ b/modules/user/user.install
@@ -362,14 +362,17 @@ function user_update_dependencies() {
 /**
  * Utility function: grant a set of permissions to a role during update.
  *
+ * This function is valid for a database schema version 7000.
+ *
  * @param $rid
  *   The role ID.
  * @param $permissions
  *   An array of permissions names.
  * @param $module
  *   The name of the module defining the permissions.
+ * @ingroup update-api-6.x-to-7.x
  */
-function _update_user_role_grant_permissions($rid, array $permissions, $module) {
+function _update_7000_user_role_grant_permissions($rid, array $permissions, $module) {
   // Grant new permissions for the role.
   foreach ($permissions as $name) {
     db_merge('role_permission')