#356074 by chx and Damien Tournoud: Provide a sequences API. Gets rid of stupid tables that only contain an incrementing ID, and fixes database import problems due to user ID 0.

9 files changed, 169 insertions, 40 deletions

diff --git a/includes/actions.inc b/includes/actions.inc
index ce196cfe5..6921e2f44 100644
--- a/includes/actions.inc
+++ b/includes/actions.inc
@@ -324,7 +324,7 @@ function actions_save($function, $type, $params, $label, $aid = NULL) {
   // aid is the callback for singleton actions so we need to keep a separate
   // table for numeric aids.
   if (!$aid) {
-    $aid = db_insert('actions_aid')->useDefaults(array('aid'))->execute();
+    $aid = db_next_id();
   }
 
   db_merge('actions')
diff --git a/includes/database/database.inc b/includes/database/database.inc
index 82ac78bc7..81f04fb2e 100644
--- a/includes/database/database.inc
+++ b/includes/database/database.inc
@@ -1047,6 +1047,44 @@ abstract class DatabaseConnection extends PDO {
   public function commit() {
     throw new ExplicitTransactionsNotSupportedException();
   }
+
+  /**
+   * Retrieves an unique id from a given sequence.
+   *
+   * Use this function if for some reason you can't use a serial field. For
+   * example, MySQL has no ways of reading of the current value of a sequence
+   * and PostgreSQL can not advance the sequence to be larger than a given
+   * value. Or sometimes you just need a unique integer.
+   *
+   * @param $existing_id
+   *   After a database import, it might be that the sequences table is behind,
+   *   so by passing in the maximum existing id, it can be assured that we
+   *   never issue the same id.
+   * @return
+   *   An integer number larger than any number returned by earlier calls and
+   *   also larger than the $existing_id if one was passed in.
+   */
+  public function nextId($existing_id = 0) {
+    $transaction = $this->startTransaction();
+    // We can safely use literal queries here instead of the slower query
+    // builder because if a given database breaks here then it can simply
+    // override nextId. However, this is unlikely as we deal with short
+    // strings and integers and no known databases require special handling
+    // for those simple cases.
+    // If another transaction wants to write the same row, it will wait until
+    // this transaction commits.
+    $stmt = $this->query('UPDATE {sequences} SET value = GREATEST(value, :existing_id) + 1', array(
+      ':existing_id' => $existing_id,
+    ));
+    if (!$stmt->rowCount()) {
+      $this->query('INSERT INTO {sequences} (value) VALUES (:existing_id + 1)', array(
+        ':existing_id' => $existing_id,
+      ));
+    }
+    // The transaction gets committed when the transaction object gets
+    // destructed because it gets out of scope.
+    return $new_value;
+  }
 }
 
 /**
@@ -2079,6 +2117,24 @@ function db_close(array $options = array()) {
 }
 
 /**
+ * Retrieves a unique id.
+ *
+ * Use this function if for some reason you can't use a serial field,
+ * normally a serial field with db_last_insert_id is preferred.
+ *
+ * @param $existing_id
+ *   After a database import, it might be that the sequences table is behind,
+ *   so by passing in a minimum id, it can be assured that we never issue the
+ *   same id.
+ * @return
+ *   An integer number larger than any number returned before for this
+ *   sequence.
+ */
+function db_next_id($existing_id = 0) {
+  return Database::getConnection()->nextId($existing_id);
+}
+
+/**
  * @} End of "defgroup database".
  */
 
diff --git a/includes/database/mysql/database.inc b/includes/database/mysql/database.inc
index 3f143cfe2..bbf927655 100644
--- a/includes/database/mysql/database.inc
+++ b/includes/database/mysql/database.inc
@@ -68,6 +68,42 @@ class DatabaseConnection_mysql extends DatabaseConnection {
     // We don't want to override any of the defaults.
     return NULL;
   }
+
+  public function nextId($existing_id = 0) {
+    static $shutdown_registered = FALSE;
+    $new_id = $this->query('INSERT INTO {sequences} () VALUES ()', array(), array('return' => Database::RETURN_INSERT_ID));
+    // This should only happen after an import or similar event.
+    if ($existing_id >= $new_id) {
+      // If we INSERT a value manually into the sequences table, on the next
+      // INSERT, MySQL will generate a larger value. However, there is no way
+      // of knowing whether this value already exists in the table. MySQL
+      // provides an INSERT IGNORE which would work, but that can mask problems
+      // other than duplicate keys. Instead, we use INSERT ... ON DUPLICATE KEY
+      // UPDATE in such a way that the UPDATE does not do anything. This way,
+      // duplicate keys do not generate errors but everything else does.
+      $this->query('INSERT INTO {sequences} (value) VALUES (:value) ON DUPLICATE KEY UPDATE value = value', array(':value' => $existing_id));
+      $new_id = $this->query('INSERT INTO {sequences} () VALUES ()', array(), array('return' => Database::RETURN_INSERT_ID));
+    }
+    if (!$shutdown_registered) {
+      register_shutdown_function(array(get_class($this), 'nextIdDelete'));
+      $shutdown_registered = TRUE;
+    }
+    return $new_id;
+  }
+
+  public static function nextIdDelete() {
+    // While we want to clean up the table to keep it up from occupying too
+    // much storage and memory, we must keep the highest value in the table
+    // because InnoDB  uses an in-memory auto-increment counter as long as the
+    // server runs. When the server is stopped and restarted, InnoDB
+    // reinitializes the counter for each table for the first INSERT to the
+    // table based solely on values from the table so deleting all values would
+    // be a problem in this case. Also, TRUNCATE resets the auto increment
+    // counter.
+    $max_id = db_select('SELECT MAX(value) FROM {sequences}')->fetchField();
+    // We know we are using MySQL here, so need for the slower db_delete().
+    db_query('DELETE FROM {sequences} WHERE value < :value', array(':value' => $max_id));
+  }
 }
 
 
diff --git a/modules/simpletest/tests/actions.test b/modules/simpletest/tests/actions.test
index 12a97cc5f..7cc06b558 100644
--- a/modules/simpletest/tests/actions.test
+++ b/modules/simpletest/tests/actions.test
@@ -37,11 +37,13 @@ class ActionsConfigurationTestCase extends DrupalWebTestCase {
 
     // Make another POST request to the action edit page.
     $this->clickLink(t('configure'));
+    preg_match('|admin/config/system/actions/configure/(\d+)|', $this->getUrl(), $matches);
+    $aid = $matches[1];
     $edit = array();
     $new_action_label = $this->randomName();
     $edit['actions_label'] = $new_action_label;
     $edit['url'] = 'admin';
-    $this->drupalPost('admin/config/system/actions/configure/1', $edit, t('Save'));
+    $this->drupalPost(NULL, $edit, t('Save'));
 
     // Make sure that the action updated properly.
     $this->assertText(t('The action has been successfully saved.'), t("Make sure we get a confirmation that we've successfully updated the complex action."));
@@ -51,7 +53,7 @@ class ActionsConfigurationTestCase extends DrupalWebTestCase {
     // Make sure that deletions work properly.
     $this->clickLink(t('delete'));
     $edit = array();
-    $this->drupalPost('admin/config/system/actions/delete/1', $edit, t('Delete'));
+    $this->drupalPost("admin/config/system/actions/delete/$aid", $edit, t('Delete'));
 
     // Make sure that the action was actually deleted.
     $this->assertRaw(t('Action %action was deleted', array('%action' => $new_action_label)), t('Make sure that we get a delete confirmation message.'));
diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test
index 8d400e68c..d64ee9ed0 100644
--- a/modules/simpletest/tests/database_test.test
+++ b/modules/simpletest/tests/database_test.test
@@ -2981,3 +2981,30 @@ class DatabaseExtraTypesTestCase extends DrupalWebTestCase {
 
 }
 
+
+/**
+ * Check the sequences API.
+ */
+class DatabaseNextIdCase extends DrupalWebTestCase {
+  function getInfo() {
+    return array(
+      'name' => t('Sequences API'),
+      'description' => t('Test the secondary sequences API.'),
+      'group' => t('Database'),
+    );
+  }
+
+  /**
+   * Test that the sequences API work.
+   */
+  function testDbNextId() {
+    $first = db_next_id();
+    $second = db_next_id();
+    // We can test for exact increase in here because we know there is no
+    // other process operating on these tables -- normally we could only
+    // expect $second > $first.
+    $this->assertEqual($first + 1, $second, t('The second call from a sequence provides a number increased by one.'));
+    $result = db_next_id(1000);
+    $this->assertEqual($result, 1001, t('Sequence provides a larger number than the existing ID.'));
+  }
+}
diff --git a/modules/system/system.install b/modules/system/system.install
index 97f8e6724..4e5496f90 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -352,23 +352,19 @@ function system_install() {
   // Load system theme data appropriately.
   system_rebuild_theme_data();
 
-  // Inserting uid 0 here confuses MySQL -- the next user might be created as
-  // uid 2 which is not what we want. So we insert the first user here, the
-  // anonymous user. uid is 1 here for now, but very soon it will be changed
-  // to 0.
   db_insert('users')
     ->fields(array(
+      'uid' => 0,
       'name' => '',
       'mail' => '',
     ))
     ->execute();
   // We need some placeholders here as name and mail are uniques and data is
-  // presumed to be a serialized array. Install will change uid 1 immediately
-  // anyways. So we insert the superuser here, the uid is 2 here for now, but
-  // very soon it will be changed to 1.
-
+  // presumed to be a serialized array. This will be changed by the settings
+  // form.
   db_insert('users')
     ->fields(array(
+      'uid' => 1,
       'name' => 'placeholder-for-uid-1',
       'mail' => 'placeholder-for-uid-1',
       'created' => REQUEST_TIME,
@@ -376,19 +372,6 @@ function system_install() {
       'data' => serialize(array()),
     ))
     ->execute();
-  // This sets the above two users uid 0 (anonymous). We avoid an explicit 0
-  // otherwise MySQL might insert the next auto_increment value.
-  db_update('users')
-    ->expression('uid', 'uid - uid')
-    ->condition('name', '')
-    ->execute();
-
-  // This sets uid 1 (superuser). We skip uid 2 but that's not a big problem.
-  db_update('users')
-    ->fields(array('uid' => 1))
-    ->condition('name', 'placeholder-for-uid-1')
-    ->execute();
-
   // Built-in roles.
   $rid_anonymous = db_insert('role')
     ->fields(array('name' => 'anonymous user'))
@@ -615,19 +598,6 @@ function system_schema() {
     'primary key' => array('aid'),
   );
 
-  $schema['actions_aid'] = array(
-    'description' => 'Stores action IDs for non-default actions.',
-    'fields' => array(
-      'aid' => array(
-        'description' => 'Primary Key: Unique actions ID.',
-        'type' => 'serial',
-        'unsigned' => TRUE,
-        'not null' => TRUE,
-      ),
-    ),
-    'primary key' => array('aid'),
-  );
-
   $schema['batch'] = array(
     'description' => 'Stores details about batches (processes that run in multiple HTTP requests).',
     'fields' => array(
@@ -1451,6 +1421,19 @@ function system_schema() {
     'primary key' => array('name'),
   );
 
+  $schema['sequences'] = array(
+    'description' => 'Stores IDs.',
+    'fields' => array(
+      'value' => array(
+        'description' => 'The value of the sequence.',
+        'type' => 'serial',
+        'unsigned' => TRUE,
+        'not null' => TRUE,
+      ),
+     ),
+    'primary key' => array('value'),
+  );
+
   $schema['sessions'] = array(
     'description' => "Drupal's session handlers read and write into the sessions table. Each record represents a user session, either anonymous or authenticated.",
     'fields' => array(
@@ -2866,6 +2849,22 @@ function system_update_7043() {
 }
 
 /**
+ * Reuse the actions_aid table as sequences.
+ */
+function system_update_7043() {
+  db_drop_primary_key('actions_aid');
+  db_change_field('actions_aid', 'aid', 'value', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE), array('primary key' => array('value')));
+  db_rename_table('actions_aid', 'sequences');
+  $max = db_query('SELECT MAX(value) FROM {sequences}')->fetchField();
+  $max_uid = db_query('SELECT MAX(uid) FROM {users}')->fetchField();
+  if ($max_uid > $max) {
+    db_update('sequences')->fields(array('value' => $max_uid))->execute();
+  }
+  $max = db_query('SELECT MAX(value) FROM {sequences}')->fetchField();
+  db_delete('sequences')->condition('value', $max, '<');
+}
+
+/**
  * @} End of "defgroup updates-6.x-to-7.x"
  * The next series of updates should start at 8000.
  */
diff --git a/modules/trigger/trigger.test b/modules/trigger/trigger.test
index 7204dc395..d7732fe02 100644
--- a/modules/trigger/trigger.test
+++ b/modules/trigger/trigger.test
@@ -151,7 +151,10 @@ class TriggerCronTestCase extends DrupalWebTestCase {
       'subject' => $action_label,
     );
     $this->drupalPost('admin/config/system/actions/configure/' . $hash, $edit, t('Save'));
-    $edit = array('aid' => md5('1'));
+    $aid = db_query('SELECT aid FROM {actions} WHERE callback = :callback', array(':callback' => 'trigger_test_system_cron_conf_action'))->fetchField();
+    // $aid is likely 3 but if we add more uses for the sequences table in
+    // core it might break, so it is easier to get the value from the database.
+    $edit = array('aid' => md5($aid));
     $this->drupalPost('admin/structure/trigger/system', $edit, t('Assign'));
 
     // Add a second configurable action to the cron trigger.
@@ -161,7 +164,7 @@ class TriggerCronTestCase extends DrupalWebTestCase {
       'subject' => $action_label,
     );
     $this->drupalPost('admin/config/system/actions/configure/' . $hash, $edit, t('Save'));
-    $edit = array('aid' => md5('2'));
+    $edit = array('aid' => md5($aid + 1));
     $this->drupalPost('admin/structure/trigger/system', $edit, t('Assign'));
 
     // Force a cron run.
diff --git a/modules/user/user.install b/modules/user/user.install
index 610d3d566..2b93e4e68 100644
--- a/modules/user/user.install
+++ b/modules/user/user.install
@@ -102,10 +102,11 @@ function user_schema() {
     'description' => 'Stores user data.',
     'fields' => array(
       'uid' => array(
-        'type' => 'serial',
+        'type' => 'int',
         'unsigned' => TRUE,
         'not null' => TRUE,
         'description' => 'Primary Key: Unique user ID.',
+        'default' => 0,
       ),
       'name' => array(
         'type' => 'varchar',
diff --git a/modules/user/user.module b/modules/user/user.module
index fe75f1c7b..48140a862 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -448,6 +448,11 @@ function user_save($account, $edit = array(), $category = 'account') {
     user_module_invoke('update', $edit, $user, $category);
   }
   else {
+    // Allow 'uid' to be set by the caller. There is no danger of writing an
+    // existing user as drupal_write_record will do an INSERT.
+    if (empty($edit['uid'])) {
+      $edit['uid'] = db_next_id(db_query('SELECT MAX(uid) FROM {users}')->fetchField());
+    }
     // Allow 'created' to be set by the caller.
     if (!isset($edit['created'])) {
       $edit['created'] = REQUEST_TIME;