fix(reminders): adjust mysql procedures (#200) r=rfk

* fix(reminders): optimize mysql procedures for verification reminders

Fixes #200

Author: vladikoff

fxa-auth-db-server/test/backend/db_tests.js

@@ -1779,6 +1779,76 @@ module.exports = function(config, DB) {
           }
         )
 
+        test(
+          'reminders - multi fetch',
+          function (t) {
+            t.plan(1)
+            var fetchQuery = {
+              type: 'first',
+              reminderTime: 1,
+              reminderTimeOutdated: 100,
+              limit: 20
+            }
+            var account = createAccount()
+            var account2 = createAccount()
+            return db.createAccount(account.uid, account)
+              .then(
+                function () {
+                  return db.createAccount(account2.uid, account2)
+                }
+              )
+              .then(
+                function () {
+                  // create 'first' reminder for account one.
+                  return db.createVerificationReminder({
+                    uid: account.uid,
+                    type: 'first'
+                  })
+                }
+              )
+              .then(
+                function () {
+                  // create 'first' reminder for account two.
+                  return db.createVerificationReminder({
+                    uid: account2.uid,
+                    type: 'first'
+                  })
+                }
+              )
+              .then(
+                function () {
+                  return P.delay(20).then(function () {
+                    return P.all([
+                      // only one query should give results
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery),
+                      db.fetchReminders({}, fetchQuery)
+                    ])
+                  })
+                }
+              )
+              .then(
+                function (results) {
+                  var found = 0
+                  results.forEach((result) => {
+                    if (result.length === 2) {
+                      found++
+                    }
+                  })
+
+                  t.equal(found, 1, 'only one query has the result')
+                }
+              )
+          }
+        )
+
         test(
           'unblockCodes',
           function (t) {

lib/db/mysql.js

@@ -573,37 +573,38 @@ module.exports = function (log, error) {
   // VERIFICATION REMINDERS
 
   // INSERT : id, uid, email, emailCode, type, acceptLanguage, createdAt
-  var CREATE_REMINDER = 'CALL createVerificationReminder_1(?, ?, ?, ?)'
+  var CREATE_REMINDER = 'CALL createVerificationReminder_2(?, ?, ?)'
 
   MySql.prototype.createVerificationReminder = function (body) {
     if (! body || ! body.uid || ! body.type) {
       throw error.wrap(new Error('"uid", "type" are required'))
     }
 
     var reminderData = {
-      id: crypto.createHash('sha256').update(body.uid + body.type).digest(),
       uid: new Buffer(body.uid),
       type: body.type,
       createdAt: Date.now()
     }
 
     return this.write(CREATE_REMINDER, [
-      reminderData.id,
       reminderData.uid,
       reminderData.type,
       reminderData.createdAt
     ])
   }
 
   // SELECT:
-  var FETCH_REMINDERS = 'CALL fetchVerificationReminders_1(?, ?, ?, ?)'
+  var FETCH_REMINDERS = 'CALL fetchVerificationReminders_2(?, ?, ?, ?, ?)'
 
   MySql.prototype.fetchReminders = function (body, query) {
+    var now = Date.now()
+
     if (! query || ! query.reminderTime || ! query.reminderTimeOutdated || ! query.type || ! query.limit) {
       throw error.wrap(new Error('fetchReminders - reminderTime, reminderTimeOutdated, limit or type missing'))
     }
 
     return this.read(FETCH_REMINDERS, [
+      now,
       query.type,
       query.reminderTime,
       query.reminderTimeOutdated,

lib/db/patch.js

@@ -4,4 +4,4 @@
 
 // The expected patch level of the database. Update if you add a new
 // patch in the ./schema/ directory.
-module.exports.level = 39
+module.exports.level = 40

lib/db/schema/patch-039-040.sql

@@ -0,0 +1,97 @@
+-- Drop the old reminders table, it was not functioning correctly
+DROP TABLE IF EXISTS `verificationReminders`;
+
+
+CREATE TABLE verificationReminders (
+  uid BINARY(16) NOT NULL,
+  type VARCHAR(255) NOT NULL,
+  createdAt BIGINT SIGNED NOT NULL,
+  PRIMARY KEY(uid, type),
+  INDEX reminder_createdAt (createdAt)
+) ENGINE=InnoDB;
+
+
+CREATE PROCEDURE `createVerificationReminder_2` (
+    IN uid BINARY(16),
+    IN type VARCHAR(255),
+    IN createdAt BIGINT SIGNED
+)
+BEGIN
+    INSERT INTO verificationReminders(
+        uid,
+        type,
+        createdAt
+    )
+    VALUES(
+        uid,
+        type,
+        createdAt
+    );
+END;
+
+
+CREATE PROCEDURE `fetchVerificationReminders_2` (
+    IN currentTime BIGINT SIGNED,
+    IN reminderType VARCHAR(255),
+    IN reminderTime BIGINT SIGNED,
+    IN reminderTimeOutdated BIGINT SIGNED,
+    IN reminderLimit INTEGER
+)
+BEGIN
+  DECLARE EXIT HANDLER FOR SQLEXCEPTION
+  BEGIN
+    DO RELEASE_LOCK('fxa-auth-server.verification-reminders-lock');
+    ROLLBACK;
+    RESIGNAL;
+  END;
+
+  START TRANSACTION;
+
+  DO @lockAcquired:=GET_LOCK('fxa-auth-server.verification-reminders-lock', 1);
+
+  IF @lockAcquired THEN
+    DROP TEMPORARY TABLE IF EXISTS reminderResults;
+
+    -- Select these straight into the temporary table
+    -- and avoid using a cursor. Use ORDER BY to
+    -- ensure the query is deterministic.
+    CREATE TEMPORARY TABLE reminderResults AS
+      SELECT uid, type
+      FROM verificationReminders
+      -- Since we want to order by `createdAt`, we have to rearrange
+      -- the `WHERE` clauses so `createdAt` is positive rather than negated.
+      WHERE  createdAt < currentTime - reminderTime
+      AND createdAt > currentTime - reminderTimeOutdated
+      AND type = reminderType
+      ORDER BY createdAt, uid, type
+      LIMIT reminderLimit;
+
+    -- Because the query is deterministic we can delete
+    -- all the selected items at once, rather than calling
+    -- deleteVerificationReminder()
+    DELETE FROM verificationReminders
+      WHERE  createdAt < currentTime - reminderTime
+      AND createdAt > currentTime - reminderTimeOutdated
+      AND type = reminderType
+      ORDER BY createdAt, uid, type
+      LIMIT reminderLimit;
+
+    -- Clean up outdated reminders.
+    DELETE FROM
+      verificationReminders
+    WHERE
+      createdAt < currentTime - reminderTimeOutdated
+    AND
+      type = reminderType;
+
+    -- Return the result
+    SELECT * FROM reminderResults;
+
+    DO RELEASE_LOCK('fxa-auth-server.verification-reminders-lock');
+
+  END IF;
+  COMMIT;
+END;
+
+
+UPDATE dbMetadata SET value = '40' WHERE name = 'schema-patch-level';

lib/db/schema/patch-040-039.sql

@@ -0,0 +1,5 @@
+-- ALTER TABLE `verificationReminders` DROP PRIMARY KEY, ADD PRIMARY KEY (`id`);
+-- DROP PROCEDURE `fetchVerificationReminders_2`;
+-- DROP PROCEDURE `createVerificationReminder_2`;
+
+-- UPDATE dbMetadata SET value = '39' WHERE name = 'schema-patch-level';