commit fabc2ba6a252c356b7e082cbc47856e607fa42c2
parent 87acdce81b03bcf89f7d35a8177b074ddbdfa556
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 30 Mar 2016 01:16:41 -0400
Always start at MAX() + 1 for Zotero.ID.get(), and deasyncify
Instead of getting batches of unused primary key ids, even if they're lower
than other ids, which for some reason seemed like a good idea in 2008, just do
a `MAX()` on the table at startup and return the next available id on each call
to `Zotero.ID.get()`. This is much simpler, and not reusing ids allows them to
be used as a chronological sort field.
While SQLite's `SELECT last_insert_rowid()` could return auto-increment values,
it's unsafe with async DB access, since a second `INSERT` can come in before
the first `last_insert_rowid()` is called. This is true even in a transaction
unless a function that calls it is never called in parallel (e.g., with
`Zotero.Promise.all()`, which can be faster than sequential `yield`s).
Note that the next id is always initialized as MAX() + 1, so if an object is
added and then deleted, after a restart the same id will be given. (This is
equivalent to (though unrelated to) SQLite's `INTEGER PRIMARY KEY` behavior,
as opposed to its `INTEGER PRIMARY KEY AUTOINCREMENT` behavior.)
Closes #993, Feed items out of order
Diffstat:
11 files changed, 60 insertions(+), 351 deletions(-)
diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js
@@ -259,7 +259,7 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env)
var isNew = env.isNew;
var options = env.options;
- var collectionID = env.id = this._id = this.id ? this.id : yield Zotero.ID.get('collections');
+ var collectionID = this._id = this.id ? this.id : Zotero.ID.get('collections');
Zotero.debug("Saving collection " + this.id);
@@ -279,10 +279,7 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env)
let placeholders = env.sqlColumns.map(function () '?').join();
let sql = "INSERT INTO collections (" + env.sqlColumns.join(', ') + ") "
+ "VALUES (" + placeholders + ")";
- var insertID = yield Zotero.DB.queryAsync(sql, env.sqlValues);
- if (!collectionID) {
- collectionID = env.id = insertID;
- }
+ yield Zotero.DB.queryAsync(sql, env.sqlValues);
}
else {
let sql = 'UPDATE collections SET '
diff --git a/chrome/content/zotero/xpcom/data/creators.js b/chrome/content/zotero/xpcom/data/creators.js
@@ -90,7 +90,7 @@ Zotero.Creators = new function() {
sql, [data.firstName, data.lastName, data.fieldMode]
);
if (!id && create) {
- id = yield Zotero.ID.get('creators');
+ id = Zotero.ID.get('creators');
let sql = "INSERT INTO creators (creatorID, firstName, lastName, fieldMode) "
+ "VALUES (?, ?, ?, ?)";
yield Zotero.DB.queryAsync(
diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js
@@ -902,7 +902,7 @@ Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env)
// Undo registerObject() on failure
var func = function () {
- this.ObjectsClass.unload(env.id);
+ this.ObjectsClass.unload(this._id);
}.bind(this);
if (env.options.tx) {
env.transactionOptions.onRollback = func;
diff --git a/chrome/content/zotero/xpcom/data/feedItem.js b/chrome/content/zotero/xpcom/data/feedItem.js
@@ -153,7 +153,7 @@ Zotero.FeedItem.prototype._initSave = Zotero.Promise.coroutine(function* (env) {
var superOnCommit = env.transactionOptions.onCommit;
env.transactionOptions.onCommit = () => {
if (superOnCommit) superOnCommit();
- this.ObjectsClass._setGUIDMapping(this.guid, env.id);
+ this.ObjectsClass._setGUIDMapping(this.guid, this._id);
};
}
@@ -165,7 +165,15 @@ Zotero.FeedItem.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
if (this._changed.feedItemData || env.isNew) {
var sql = "REPLACE INTO feedItems VALUES (?,?,?,?)";
- yield Zotero.DB.queryAsync(sql, [env.id, this.guid, this._feedItemReadTime, this._feedItemTranslatedTime]);
+ yield Zotero.DB.queryAsync(
+ sql,
+ [
+ this.id,
+ this.guid,
+ this._feedItemReadTime,
+ this._feedItemTranslatedTime
+ ]
+ );
this._clearChanged('feedItemData');
}
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
@@ -1225,7 +1225,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
// Primary fields
//
// If available id value, use it -- otherwise we'll use autoincrement
- var itemID = env.id = this._id = this.id ? this.id : yield Zotero.ID.get('items');
+ var itemID = this._id = this.id ? this.id : Zotero.ID.get('items');
env.sqlColumns.push(
'itemTypeID',
@@ -1259,10 +1259,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
let sql = "INSERT INTO items (" + env.sqlColumns.join(", ") + ") "
+ "VALUES (" + env.sqlValues.map(function () "?").join() + ")";
- var insertID = yield Zotero.DB.queryAsync(sql, env.sqlValues);
- if (!itemID) {
- itemID = env.id = insertID;
- }
+ yield Zotero.DB.queryAsync(sql, env.sqlValues);
if (!env.options.skipNotifier) {
Zotero.Notifier.queue('add', 'item', itemID, env.notifierData);
@@ -1305,7 +1302,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
let valueID = yield Zotero.DB.valueQueryAsync(valueSQL, [value], { debug: true })
if (!valueID) {
- valueID = yield Zotero.ID.get('itemDataValues');
+ valueID = Zotero.ID.get('itemDataValues');
yield Zotero.DB.queryAsync(insertValueSQL, [valueID, value], { debug: false });
}
diff --git a/chrome/content/zotero/xpcom/data/library.js b/chrome/content/zotero/xpcom/data/library.js
@@ -444,7 +444,7 @@ Zotero.Library.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
}
if (env.isNew) {
- let id = yield Zotero.ID.get('libraries'); // Cannot retrieve auto-incremented ID with async queries
+ let id = Zotero.ID.get('libraries');
changedCols.unshift('libraryID');
params.unshift(id);
diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js
@@ -65,12 +65,9 @@ Zotero.Tags = new function() {
var sql = "SELECT tagID FROM tags WHERE name=?";
var id = yield Zotero.DB.valueQueryAsync(sql, data.tag);
if (!id && create) {
- id = yield Zotero.ID.get('tags');
+ id = Zotero.ID.get('tags');
let sql = "INSERT INTO tags (tagID, name) VALUES (?, ?)";
- let insertID = yield Zotero.DB.queryAsync(sql, [id, data.tag]);
- if (!id) {
- id = insertID;
- }
+ yield Zotero.DB.queryAsync(sql, [id, data.tag]);
}
return id;
});
diff --git a/chrome/content/zotero/xpcom/id.js b/chrome/content/zotero/xpcom/id.js
@@ -1,9 +1,9 @@
/*
***** BEGIN LICENSE BLOCK *****
- Copyright © 2009 Center for History and New Media
+ Copyright © 2016 Center for History and New Media
George Mason University, Fairfax, Virginia, USA
- http://zotero.org
+ https://www.zotero.org
This file is part of Zotero.
@@ -24,310 +24,38 @@
*/
Zotero.ID_Tracker = function () {
- this.getBigInt = getBigInt;
- this.skip = skip;
- this.getTableName = getTableName;
-
- // Number of ids to compare against at a time
- this.__defineGetter__('numIDs', function () 10000);
-
- // Number of times to try increasing the maxID if first range fails
- this.__defineGetter__('maxTries', function () 3);
-
- // Total number of ids to find
- this.__defineGetter__('maxToFind', function () 1000);
-
- var _available = {};
- var _min = {};
- var _skip = {};
-
-
- /*
- * Gets an unused primary key id for a DB table
- */
- this.get = Zotero.Promise.coroutine(function* (table, notNull) {
- table = this.getTableName(table);
-
- switch (table) {
- // Autoincrement tables
- //
- // Callers need to handle a potential NULL for these unless they
- // pass |notNull|
- case 'libraries':
- case 'items':
- case 'creators':
- case 'creatorData':
- case 'collections':
- case 'savedSearches':
- case 'tags':
- case 'customItemTypes':
- case 'customFields':
- var id = yield _getNextAvailable(table);
- if (!id && notNull) {
- return _getNext(table);
- }
- return id;
-
- // Non-autoincrement tables
- //
- // TODO: Use for everything
- case 'itemDataValues':
- case 'proxies':
- var id = yield _getNextAvailable(table);
- if (!id) {
- // If we can't find an empty id quickly, just use MAX() + 1
- return _getNext(table);
- }
- return id;
-
- default:
- throw ("Unsupported table '" + table + "' in Zotero.ID.get()");
+ var _tables = [
+ 'collections',
+ 'creators',
+ 'customFields',
+ 'customItemTypes',
+ 'itemDataValues',
+ 'items',
+ 'libraries',
+ 'proxies',
+ 'savedSearches',
+ 'tags'
+ ];
+ var _nextIDs = {};
+
+
+ this.init = Zotero.Promise.coroutine(function* () {
+ for (let table of _tables) {
+ _nextIDs[table] = yield _getNext(table);
}
});
- function getBigInt(max) {
- if (!max) {
- max = 9007199254740991;
- }
- return Math.floor(Math.random() * (max)) + 1;
- }
-
/**
- * Mark ids as used
- *
- * @param string table
- * @param int|array ids Item ids to skip
- */
- function skip(table, ids) {
- table = this.getTableName(table);
-
- switch (ids.constructor.name) {
- case 'Array':
- break;
-
- case 'Number':
- ids = [ids];
- break;
-
- default:
- throw ("ids must be an int or array of ints in Zotero.ID.skip()");
- }
-
- if (!ids.length) {
- return;
- }
-
- if (!_skip[table]) {
- _skip[table] = {};
- }
-
- for (var i=0, len=ids.length; i<len; i++) {
- _skip[table][ids[i]] = true;
- }
- }
-
-
- function getTableName(table) {
- // Used in sync.js
- if (table == 'searches') {
- table = 'savedSearches';
- }
-
- switch (table) {
- case 'collections':
- case 'creators':
- case 'creatorData':
- case 'itemDataValues':
- case 'items':
- case 'libraries':
- case 'savedSearches':
- case 'tags':
- case 'customItemTypes':
- case 'customFields':
- case 'proxies':
- return table;
-
- default:
- throw ("Invalid table '" + table + "' in Zotero.ID");
- }
- }
-
-
- /*
- * Returns the lowest available unused primary key id for table,
- * or NULL if none could be loaded in _loadAvailable()
- */
- var _getNextAvailable = Zotero.Promise.coroutine(function* (table) {
- if (!_available[table]) {
- yield _loadAvailable(table);
- }
-
- var arr = _available[table];
-
- while (arr[0]) {
- var id = arr[0][0];
-
- // End of range -- remove range
- if (id == arr[0][1]) {
- arr.shift();
-
- // Prepare table for refresh if all rows used
- if (arr.length == 0) {
- delete _available[table];
- }
- }
- // Within range -- increment
- else {
- arr[0][0]++;
- }
-
- if (_skip[table] && _skip[table][id]) {
- Zotero.debug("Skipping " + table + " id " + id);
- if (!_available[table]) {
- yield _loadAvailable(table);
- }
- continue;
- }
-
- _min[table] = id;
- return id;
- }
- return null;
- });
-
-
- /*
- * Get MAX(id) + 1 from table
- */
- var _getNext = Zotero.Promise.coroutine(function* (table) {
- var column = _getTableColumn(table);
-
- var sql = 'SELECT MAX(';
- if (_skip[table]) {
- var max = 0;
- for (var id in _skip[table]) {
- if (parseInt(id) > max) {
- max = parseInt(id);
- }
- }
- if (!max) {
- throw new Error("_skip['" + table + "'] must contain positive values");
- }
- sql += 'MAX(' + column + ', ' + max + ')';
- }
- else {
- sql += column;
- }
- sql += ')+1 FROM ' + table;
- return Zotero.DB.valueQueryAsync(sql);
- });
-
-
- /*
- * Loads available ids for table into memory
+ * Gets an unused primary key id for a DB table
*/
- var _loadAvailable = Zotero.Promise.coroutine(function* (table) {
- Zotero.debug("Loading available ids for table '" + table + "'");
-
- var minID = _min[table] ? _min[table] + 1 : 1;
- var numIDs = Zotero.ID.numIDs;
- var maxTries = Zotero.ID.maxTries;
- var maxToFind = Zotero.ID.maxToFind;
-
- var column = _getTableColumn(table);
-
- switch (table) {
- case 'creators':
- case 'creatorData':
- case 'items':
- case 'itemDataValues':
- case 'tags':
- break;
-
- case 'libraries':
- case 'collections':
- case 'savedSearches':
- case 'customItemTypes':
- case 'customFields':
- case 'proxies':
- var maxToFind = 100;
- break;
-
- default:
- throw ("Unsupported table '" + table + "' in Zotero.ID._loadAvailable()");
+ this.get = function (table) {
+ if (!_nextIDs[table]) {
+ throw new Error("IDs not loaded for table '" + table + "'");
}
- var maxID = minID + numIDs - 1;
- var sql = "SELECT " + column + " FROM " + table
- + " WHERE " + column + " BETWEEN ? AND ? ORDER BY " + column;
- var ids = yield Zotero.DB.columnQueryAsync(sql, [minID, maxID]);
- // If no ids found, we have numIDs unused ids
- if (!ids) {
- maxID = Math.min(maxID, minID + (maxToFind - 1));
- Zotero.debug("Found " + (maxID - minID + 1) + " available ids in table '" + table + "'");
- _available[table] = [[minID, maxID]];
- return;
- }
-
- // If we didn't find any unused ids, try increasing maxID a few times
- while (ids.length == numIDs && maxTries>0) {
- Zotero.debug('No available ids found between ' + minID + ' and ' + maxID + '; trying next ' + numIDs);
- minID = maxID + 1;
- maxID = minID + numIDs - 1;
- ids = yield Zotero.DB.columnQueryAsync(sql, [minID, maxID]);
- maxTries--;
- }
-
- // Didn't find any unused ids -- _getNextAvailable() will return NULL for
- // this table for rest of session
- if (ids.length == numIDs) {
- Zotero.debug("Found no available ids in table '" + table + "'");
- _available[table] = [];
- return;
- }
-
- var available = [], found = 0, j = 0, availableStart = null;
-
- for (var i=minID; i<=maxID && found<maxToFind; i++) {
- // We've gone past the found ids, so all remaining ids up to maxID
- // are available
- if (!ids[j]) {
- available.push([i, maxID]);
- found += (maxID - i) + 1;
- break;
- }
-
- // Skip ahead while ids are occupied
- if (ids[j] == i) {
- j++;
- continue;
- }
-
- // Advance counter while it's below the next used id
- while (ids[j] > i && i<=maxID) {
- if (!availableStart) {
- availableStart = i;
- }
- i++;
-
- if ((found + (i - availableStart) + 1) > maxToFind) {
- break;
- }
- }
- if (availableStart) {
- available.push([availableStart, i-1]);
- // Keep track of how many empties we've found
- found += ((i-1) - availableStart) + 1;
- availableStart = null;
- }
- j++;
- }
-
- Zotero.debug("Found " + found + " available ids in table '" + table + "'");
-
- _available[table] = available;
- });
+ return ++_nextIDs[table];
+ };
function _getTableColumn(table) {
@@ -351,33 +79,17 @@ Zotero.ID_Tracker = function () {
return table.substr(0, table.length - 1) + 'ID';
}
}
-}
-
-Zotero.ID = new Zotero.ID_Tracker;
-
-/**
- * Notifier observer to mark saved object ids as used
- */
-Zotero.ID.EventListener = new function () {
- this.init = init;
- this.notify = notify;
- function init() {
- Zotero.Notifier.registerObserver(this);
- }
-
- function notify(event, type, ids) {
- if (event == 'add') {
- try {
- var table = Zotero.ID.getTableName(type);
- }
- // Skip if not a table we handle
- catch (e) {
- return;
- }
- Zotero.ID.skip(table, ids);
- }
- }
+ /**
+ * Get MAX(id) + 1 from table
+ *
+ * @return {Promise<Integer>}
+ */
+ function _getNext(table) {
+ var sql = 'SELECT COALESCE(MAX(' + _getTableColumn(table) + ') + 1, 1) FROM ' + table;
+ return Zotero.DB.valueQueryAsync(sql);
+ };
}
+Zotero.ID = new Zotero.ID_Tracker;
diff --git a/chrome/content/zotero/xpcom/proxy.js b/chrome/content/zotero/xpcom/proxy.js
@@ -645,7 +645,7 @@ Zotero.Proxy.prototype.save = Zotero.Promise.coroutine(function* (transparent) {
);
yield Zotero.DB.queryAsync("DELETE FROM proxyHosts WHERE proxyID = ?", [this.proxyID]);
} else {
- let id = yield Zotero.ID.get('proxies');
+ let id = Zotero.ID.get('proxies');
yield Zotero.DB.queryAsync(
"INSERT INTO proxies (proxyID, multiHost, autoAssociate, scheme) VALUES (?, ?, ?, ?)",
[id, this.multiHost ? 1 : 0, this.autoAssociate ? 1 : 0, this.scheme]
diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js
@@ -148,7 +148,7 @@ Zotero.Search.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
var isNew = env.isNew;
var options = env.options;
- var searchID = env.id = this._id = this.id ? this.id : yield Zotero.ID.get('savedSearches');
+ var searchID = this._id = this.id ? this.id : Zotero.ID.get('savedSearches');
env.sqlColumns.push(
'savedSearchName'
@@ -164,10 +164,7 @@ Zotero.Search.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
let placeholders = env.sqlColumns.map(function () '?').join();
let sql = "INSERT INTO savedSearches (" + env.sqlColumns.join(', ') + ") "
+ "VALUES (" + placeholders + ")";
- var insertID = yield Zotero.DB.queryAsync(sql, env.sqlValues);
- if (!searchID) {
- searchID = env.id = insertID;
- }
+ yield Zotero.DB.queryAsync(sql, env.sqlValues);
}
else {
let sql = 'UPDATE savedSearches SET '
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
@@ -621,6 +621,7 @@ Components.utils.import("resource://gre/modules/osfile.jsm");
// Initialize Locate Manager
Zotero.LocateManager.init();
+ yield Zotero.ID.init();
yield Zotero.Collections.init();
yield Zotero.Items.init();
yield Zotero.Searches.init();
