From 0fa7ca4dd1e00ea3c747dcad91e754b007eee033 Mon Sep 17 00:00:00 2001 From: Mike Conley Date: Thu, 30 Nov 2017 18:09:54 -0500 Subject: [PATCH] Bug 888784 - Make FormHistory.update use Sqlite.jsm database backend. r=mak MozReview-Commit-ID: 7Ku1kFtTYZK --HG-- extra : rebase_source : e3679c4724ec8c8ba7dc5e99198152f23c754775 --- toolkit/components/satchel/FormHistory.jsm | 256 +++++++++++++-------- 1 file changed, 160 insertions(+), 96 deletions(-) diff --git a/toolkit/components/satchel/FormHistory.jsm b/toolkit/components/satchel/FormHistory.jsm index e4e120c519c2..fcf3d6b55bf2 100644 --- a/toolkit/components/satchel/FormHistory.jsm +++ b/toolkit/components/satchel/FormHistory.jsm @@ -621,14 +621,50 @@ function dbClose(aShutdown) { } } +/** + * @typedef {Object} InsertQueryData + * @property {Object} updatedChange + * A change requested by FormHistory. + * @property {String} query + * The insert query string. + */ + +/** + * Prepares a query and some default parameters when inserting an entry + * to the database. + * + * @param {Object} change + * The change requested by FormHistory. + * @param {number} now + * The current timestamp in microseconds. + * @returns {InsertQueryData} + * The query information needed to pass along to the database. + */ +function prepareInsertQuery(change, now) { + let updatedChange = Object.assign({}, change); + let query = "INSERT INTO moz_formhistory " + + "(fieldname, value, timesUsed, firstUsed, lastUsed, guid) " + + "VALUES (:fieldname, :value, :timesUsed, :firstUsed, :lastUsed, :guid)"; + updatedChange.timesUsed = updatedChange.timesUsed || 1; + updatedChange.firstUsed = updatedChange.firstUsed || now; + updatedChange.lastUsed = updatedChange.lastUsed || now; + + return { + updatedChange, + query, + }; +} + /** * Constructs and executes database statements from a pre-processed list of * inputted changes. * * @param {Array.} aChanges changes to form history - * @param {Object} aCallbacks + * @param {Object} aPreparedHandlers */ -async function updateFormHistoryWrite(aChanges, aCallbacks) { +// XXX This should be split up and the complexity reduced. +// eslint-disable-next-line complexity +async function updateFormHistoryWrite(aChanges, aPreparedHandlers) { log("updateFormHistoryWrite " + aChanges.length); // pass 'now' down so that every entry in the batch has the same timestamp @@ -638,51 +674,65 @@ async function updateFormHistoryWrite(aChanges, aCallbacks) { // stmts or bind a new set of parameters to an existing storage statement. // stmts and bindingArrays are updated when makeXXXStatement eventually // calls dbCreateAsyncStatement. - let stmts = []; + let queries = []; let notifications = []; - let bindingArrays = new Map(); + let conn = await FormHistory.db; for (let change of aChanges) { let operation = change.op; delete change.op; - let stmt; switch (operation) { - case "remove": + case "remove": { log("Remove from form history " + change); - let delStmt = makeMoveToDeletedStatement(change.guid, now, change, bindingArrays); - if (delStmt && !stmts.includes(delStmt)) { - stmts.push(delStmt); - } - if ("timeDeleted" in change) { - delete change.timeDeleted; - } - stmt = makeRemoveStatement(change, bindingArrays); + let queryTerms = makeQueryPredicates(change); // Fetch the GUIDs we are going to delete. try { - await new Promise((res, rej) => { - let selectStmt = makeSearchStatement(change, ["guid"]); - let selectHandlers = { - handleCompletion() { - res(); - }, - handleError() { - log("remove select guids failure"); - }, - handleResult(aResultSet) { - for (let row = aResultSet.getNextRow(); row; row = aResultSet.getNextRow()) { - notifications.push(["formhistory-remove", row.getResultByName("guid")]); - } - }, - }; - dbConnection.executeAsync([selectStmt], 1, selectHandlers); + let query = "SELECT guid FROM moz_formhistory"; + if (queryTerms) { + query += " WHERE " + queryTerms; + } + + await conn.executeCached(query, change, row => { + notifications.push(["formhistory-remove", row.getResultByName("guid")]); }); } catch (e) { - log("Error in select statement: " + e); + log("Error getting guids from moz_formhistory: " + e); } + if (supportsDeletedTable) { + log("Moving to deleted table " + change); + let query = "INSERT INTO moz_deleted_formhistory (guid, timeDeleted)"; + + // TODO: Add these items to the deleted items table once we've sorted + // out the issues from bug 756701 + if (change.guid || queryTerms) { + query += + change.guid ? " VALUES (:guid, :timeDeleted)" + : " SELECT guid, :timeDeleted FROM moz_formhistory WHERE " + queryTerms; + change.timeDeleted = now; + queries.push({ query, params: Object.assign({}, change) }); + } + + if ("timeDeleted" in change) { + delete change.timeDeleted; + } + } + + let query = "DELETE FROM moz_formhistory"; + if (queryTerms) { + log("removeEntries"); + query += " WHERE " + queryTerms; + } else { + log("removeAllEntries"); + // Not specifying any fields means we should remove all entries. We + // won't need to modify the query in this case. + } + + queries.push({ query, params: change }); break; - case "update": + } + case "update": { log("Update form history " + change); let guid = change.guid; delete change.guid; @@ -692,70 +742,90 @@ async function updateFormHistoryWrite(aChanges, aCallbacks) { change.guid = change.newGuid; delete change.newGuid; } - stmt = makeUpdateStatement(guid, change, bindingArrays); + + let query = "UPDATE moz_formhistory SET "; + let queryTerms = makeQueryPredicates(change, ", "); + if (!queryTerms) { + throw Components.Exception("Update query must define fields to modify.", + Cr.NS_ERROR_ILLEGAL_VALUE); + } + query += queryTerms + " WHERE guid = :existing_guid"; + change.existing_guid = guid; + queries.push({ query, params: change }); notifications.push(["formhistory-update", guid]); break; - case "bump": + } + case "bump": { log("Bump form history " + change); if (change.guid) { - stmt = makeBumpStatement(change.guid, now, bindingArrays); + let query = "UPDATE moz_formhistory " + + "SET timesUsed = timesUsed + 1, lastUsed = :lastUsed WHERE guid = :guid"; + let queryParams = { + lastUsed: now, + guid: change.guid, + }; + + queries.push({ query, params: queryParams }); notifications.push(["formhistory-update", change.guid]); } else { change.guid = generateGUID(); - stmt = makeAddStatement(change, now, bindingArrays); - notifications.push(["formhistory-add", change.guid]); + let { query, updatedChange } = prepareInsertQuery(change, now); + queries.push({ query, params: updatedChange }); + notifications.push(["formhistory-add", updatedChange.guid]); } break; - case "add": + } + case "add": { log("Add to form history " + change); if (!change.guid) { change.guid = generateGUID(); } - stmt = makeAddStatement(change, now, bindingArrays); - notifications.push(["formhistory-add", change.guid]); + + let { query, updatedChange } = prepareInsertQuery(change, now); + queries.push({ query, params: updatedChange }); + notifications.push(["formhistory-add", updatedChange.guid]); break; - default: + } + default: { // We should've already guaranteed that change.op is one of the above throw Components.Exception("Invalid operation " + operation, Cr.NS_ERROR_ILLEGAL_VALUE); - } - - // As identical statements are reused, only add statements if they aren't already present. - if (stmt && !stmts.includes(stmt)) { - stmts.push(stmt); + } } } - for (let stmt of stmts) { - stmt.bindParameters(bindingArrays.get(stmt)); + try { + await runUpdateQueries(conn, queries); + } catch (e) { + aPreparedHandlers.handleError(e); + aPreparedHandlers.handleCompletion(1); + return; } - let handlers = { - handleCompletion(aReason) { - if (aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED) { - for (let [notification, param] of notifications) { - // We're either sending a GUID or nothing at all. - sendNotification(notification, param); - } - } + for (let [notification, param] of notifications) { + // We're either sending a GUID or nothing at all. + sendNotification(notification, param); + } - if (aCallbacks && aCallbacks.handleCompletion) { - aCallbacks.handleCompletion( - aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED ? - 0 : - 1 - ); - } - }, - handleError(aError) { - if (aCallbacks && aCallbacks.handleError) { - aCallbacks.handleError(aError); - } - }, - handleResult: NOOP, - }; + aPreparedHandlers.handleCompletion(0); +} - dbConnection.executeAsync(stmts, stmts.length, handlers); +/** + * Runs queries for an update operation to the database. This + * is separated out from updateFormHistoryWrite to avoid shutdown + * leaks where the handlers passed to updateFormHistoryWrite would + * leak from the closure around the executeTransaction function. + * + * @param {SqliteConnection} conn the database connection + * @param {Object} queries query string and param pairs generated + * by updateFormHistoryWrite + */ +async function runUpdateQueries(conn, queries) { + await conn.executeTransaction(async () => { + for (let { query, params } of queries) { + await conn.executeCached(query, params); + } + }); } /** @@ -1170,7 +1240,7 @@ this.FormHistory = { }); }, - update(aChanges, aCallbacks) { + update(aChanges, aHandlers) { // Used to keep track of how many searches have been started. When that number // are finished, updateFormHistoryWrite can be called. let numSearches = 0; @@ -1187,17 +1257,15 @@ this.FormHistory = { aChanges = [aChanges]; } + let handlers = this._prepareHandlers(aHandlers); + let isRemoveOperation = aChanges.every(change => change && change.op && change.op == "remove"); if (!Prefs.enabled && !isRemoveOperation) { - if (aCallbacks && aCallbacks.handleError) { - aCallbacks.handleError({ - message: "Form history is disabled, only remove operations are allowed", - result: Ci.mozIStorageError.MISUSE, - }); - } - if (aCallbacks && aCallbacks.handleCompletion) { - aCallbacks.handleCompletion(1); - } + handlers.handleError({ + message: "Form history is disabled, only remove operations are allowed", + result: Ci.mozIStorageError.MISUSE, + }); + handlers.handleCompletion(1); return; } @@ -1257,13 +1325,11 @@ this.FormHistory = { handleResult(aResult) { if (this.foundResult) { log("Database contains multiple entries with the same fieldname/value pair."); - if (aCallbacks && aCallbacks.handleError) { - aCallbacks.handleError({ - message: - "Database contains multiple entries with the same fieldname/value pair.", - result: 19, // Constraint violation - }); - } + handlers.handleError({ + message: + "Database contains multiple entries with the same fieldname/value pair.", + result: 19, // Constraint violation + }); searchFailed = true; return; @@ -1274,18 +1340,16 @@ this.FormHistory = { }, handleError(aError) { - if (aCallbacks && aCallbacks.handleError) { - aCallbacks.handleError(aError); - } + handlers.handleError(aError); }, handleCompletion(aReason) { completedSearches++; if (completedSearches == numSearches) { if (!aReason && !searchFailed) { - updateFormHistoryWrite(aChanges, aCallbacks); - } else if (aCallbacks && aCallbacks.handleCompletion) { - aCallbacks.handleCompletion(1); + updateFormHistoryWrite(aChanges, handlers); + } else { + handlers.handleCompletion(1); } } }, @@ -1294,7 +1358,7 @@ this.FormHistory = { if (numSearches == 0) { // We don't have to wait for any statements to return. - updateFormHistoryWrite(aChanges, aCallbacks); + updateFormHistoryWrite(aChanges, handlers); } },