commit e7d27ee0f319881cd06238481ccd4ff674015cc3
parent e2cbfbd0fecfa2afee127f9e85382a03f0edd45a
Author: Dan Stillman <dstillman@zotero.org>
Date: Thu, 21 Apr 2016 11:08:48 -0400
Close #931, [Async DB] Update long tag fixer
Diffstat:
7 files changed, 349 insertions(+), 162 deletions(-)
diff --git a/chrome/content/zotero/longTagFixer.js b/chrome/content/zotero/longTagFixer.js
@@ -132,7 +132,7 @@ var Zotero_Long_Tag_Fixer = new function () {
this.updateEditLength = function (len) {
document.getElementById('zotero-new-tag-character-count').value = len;
- var invalid = len == 0 || len > 255;
+ var invalid = len == 0 || len > Zotero.Tags.MAX_SYNC_LENGTH;
document.getElementById('zotero-new-tag-characters').setAttribute('invalid', invalid);
document.getElementById('zotero-long-tag-fixer').getButton('accept').disabled = invalid;
}
@@ -146,11 +146,9 @@ var Zotero_Long_Tag_Fixer = new function () {
this.save = function () {
try {
- var index = document.getElementById('zotero-new-tag-actions').selectedIndex;
+ var result = {};
- // Search for all matching tags across all libraries
- var sql = "SELECT tagID FROM tags WHERE name=?";
- var oldTagIDs = Zotero.DB.columnQuery(sql, _oldTag);
+ var index = document.getElementById('zotero-new-tag-actions').selectedIndex;
switch (index) {
// Split
@@ -166,48 +164,23 @@ var Zotero_Long_Tag_Fixer = new function () {
}
}
- Zotero.DB.beginTransaction();
-
- // Add new tags to all items linked to each matching old tag
- for (var i=0; i<oldTagIDs.length; i++) {
- var tag = Zotero.Tags.get(oldTagIDs[i]);
- var items = tag.getLinkedItems();
- if (items) {
- for (var j=0; j<items.length; j++) {
- items[j].addTags(newTags, tag.type);
- }
- }
- }
-
- // Remove old tags
- // TODO: Update
- Zotero.Tags.erase(oldTagIDs);
- Zotero.Tags.purge();
- Zotero.DB.commitTransaction();
+ result.op = 'split';
+ result.tags = newTags;
break;
// Edit
case 1:
- var value = document.getElementById('zotero-new-tag-editor').value;
- Zotero.DB.beginTransaction();
- for (var i=0; i<oldTagIDs.length; i++) {
- var tag = Zotero.Tags.get(oldTagIDs[i]);
- tag.name = value;
- tag.save();
- }
- Zotero.DB.commitTransaction();
+ result.op = 'edit';
+ result.tag = document.getElementById('zotero-new-tag-editor').value;
break;
// Delete
case 2:
- Zotero.DB.beginTransaction();
- Zotero.Tags.erase(oldTagIDs);
- Zotero.Tags.purge();
- Zotero.DB.commitTransaction();
+ result.op = 'delete';
break;
}
- _dataOut.result = true;
+ _dataOut.result = result;
}
catch (e) {
diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js
@@ -29,6 +29,7 @@
*/
Zotero.Tags = new function() {
this.MAX_COLORED_TAGS = 6;
+ this.MAX_SYNC_LENGTH = 255;
var _initialized = false;
var _tagsByID = new Map();
@@ -123,6 +124,15 @@ Zotero.Tags = new function() {
});
+ this.getLongTagsInLibrary = Zotero.Promise.coroutine(function* (libraryID) {
+ var sql = "SELECT DISTINCT tagID FROM tags "
+ + "JOIN itemTags USING (tagID) "
+ + "JOIN items USING (itemID) "
+ + "WHERE libraryID=? AND LENGTH(name)>?"
+ return yield Zotero.DB.columnQueryAsync(sql, [libraryID, this.MAX_SYNC_LENGTH]);
+ });
+
+
/**
* Get all tags in library
*
diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js
@@ -339,36 +339,6 @@ Zotero.Sync.Server = new function () {
}
break;
- case 'TAG_TOO_LONG':
- if (!Zotero.Sync.Runner.background) {
- var tag = xmlhttp.responseXML.firstChild.getElementsByTagName('tag');
- if (tag.length) {
- var tag = tag[0].firstChild.nodeValue;
- setTimeout(function () {
- var callback = function () {
- var sql = "SELECT DISTINCT name FROM tags WHERE LENGTH(name)>255 LIMIT 1";
- var tag = Zotero.DB.valueQuery(sql);
- if (tag) {
- var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
- .getService(Components.interfaces.nsIWindowMediator);
- var lastWin = wm.getMostRecentWindow("navigator:browser");
- var dataOut = { result: null };
- lastWin.openDialog('chrome://zotero/content/longTagFixer.xul', '', 'chrome,modal,centerscreen', tag, dataOut);
- if (dataOut.result) {
- callback();
- }
- }
- else {
- Zotero.Sync.Runner.sync();
- }
- };
-
- callback();
- }, 1);
- }
- }
- break;
-
// We can't reproduce it, but we can fix it
case 'WRONG_LIBRARY_TAG_ITEM':
var background = Zotero.Sync.Runner.background;
diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -905,10 +905,13 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func
// Handle failed objects
for (let index in json.results.failed) {
- let { code, message } = json.results.failed[index];
+ let { code, message, data } = json.results.failed[index];
let e = new Error(message);
- e.name = "ZoteroUploadObjectError";
+ e.name = "ZoteroObjectUploadError";
e.code = code;
+ if (data) {
+ e.data = data;
+ }
Zotero.logError("Error for " + objectType + " " + batch[index].key + " in "
+ this.library.name + ":\n\n" + e);
diff --git a/chrome/content/zotero/xpcom/sync/syncRunner.js b/chrome/content/zotero/xpcom/sync/syncRunner.js
@@ -73,7 +73,6 @@ Zotero.Sync.Runner_Module = function (options = {}) {
this.getAPIClient = function (options = {}) {
-
return new Zotero.Sync.APIClient({
baseURL: this.baseURL,
apiVersion: this.apiVersion,
@@ -141,7 +140,7 @@ Zotero.Sync.Runner_Module = function (options = {}) {
let emptyLibraryContinue = yield this.checkEmptyLibrary(keyInfo);
if (!emptyLibraryContinue) {
- this.end();
+ yield this.end(options);
Zotero.debug("Syncing cancelled because user library is empty");
return false;
}
@@ -168,7 +167,7 @@ Zotero.Sync.Runner_Module = function (options = {}) {
firstInSession: _firstInSession
};
- let librariesToSync = yield this.checkLibraries(
+ let librariesToSync = options.libraries = yield this.checkLibraries(
client,
options,
keyInfo,
@@ -217,11 +216,18 @@ Zotero.Sync.Runner_Module = function (options = {}) {
}
}
finally {
- this.end();
+ yield this.end(options);
+
+ if (options.restartSync) {
+ delete options.restartSync;
+ Zotero.debug("Restarting sync");
+ yield this.sync(options);
+ return;
+ }
+
+ Zotero.debug("Done syncing");
}
- Zotero.debug("Done syncing");
-
/*if (results.changesMade) {
Zotero.debug("Changes made during file sync "
+ "-- performing additional data sync");
@@ -682,33 +688,14 @@ Zotero.Sync.Runner_Module = function (options = {}) {
}
- this.end = function () {
- this.updateIcons(_errors);
+ this.end = Zotero.Promise.coroutine(function* (options) {
+ yield this.checkErrors(_errors, options);
+ if (!options.restartSync) {
+ this.updateIcons(_errors);
+ }
_errors = [];
_syncInProgress = false;
- }
-
-
- /**
- * Log a warning, but don't throw an error
- */
- this.warning = function (e) {
- Zotero.debug(e, 2);
- Components.utils.reportError(e);
- e.errorType = 'warning';
- _warning = e;
- }
-
-
- this.error = function (e) {
- if (typeof e == 'string') {
- e = new Error(e);
- e.errorType = 'error';
- }
- Zotero.debug(e, 1);
- this.updateIcons(e);
- throw (e);
- }
+ });
/**
@@ -846,7 +833,17 @@ Zotero.Sync.Runner_Module = function (options = {}) {
}
- this.checkError = function (e, background) {
+ this.checkErrors = Zotero.Promise.coroutine(function* (errors, options = {}) {
+ for (let e of errors) {
+ let handled = yield this.checkError(e, options);
+ if (handled) {
+ break;
+ }
+ }
+ });
+
+
+ this.checkError = Zotero.Promise.coroutine(function* (e, options = {}) {
if (e.name && e.name == 'Zotero Error') {
switch (e.error) {
case Zotero.Error.ERROR_SYNC_USERNAME_NOT_SET:
@@ -859,9 +856,9 @@ Zotero.Sync.Runner_Module = function (options = {}) {
var msg = Zotero.getString('sync.error.invalidLogin.text');
e.message = msg;
e.data = {};
- e.data.dialogText = msg;
- e.data.dialogButtonText = Zotero.getString('sync.openSyncPreferences');
- e.data.dialogButtonCallback = function () {
+ e.dialogText = msg;
+ e.dialogButtonText = Zotero.getString('sync.openSyncPreferences');
+ e.dialogButtonCallback = function () {
var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
.getService(Components.interfaces.nsIWindowMediator);
var win = wm.getMostRecentWindow("navigator:browser");
@@ -905,6 +902,83 @@ Zotero.Sync.Runner_Module = function (options = {}) {
break;
}
}
+ else if (e.name && e.name == 'ZoteroObjectUploadError') {
+ // Tag too long
+ if (e.code == 413 && e.data && e.data.tag !== undefined) {
+ // Show long tag fixer and handle result
+ e.dialogButtonText = Zotero.getString('general.fix');
+ e.dialogButtonCallback = Zotero.Promise.coroutine(function* () {
+ var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
+ .getService(Components.interfaces.nsIWindowMediator);
+ var lastWin = wm.getMostRecentWindow("navigator:browser");
+
+ // Open long tag fixer for every long tag in every editable library we're syncing
+ var editableLibraries = options.libraries
+ .filter(x => Zotero.Libraries.get(x).editable);
+ for (let libraryID of editableLibraries) {
+ let oldTagIDs = yield Zotero.Tags.getLongTagsInLibrary(libraryID);
+ for (let oldTagID of oldTagIDs) {
+ let oldTag = Zotero.Tags.getName(oldTagID);
+ let dataOut = { result: null };
+ lastWin.openDialog(
+ 'chrome://zotero/content/longTagFixer.xul',
+ '',
+ 'chrome,modal,centerscreen',
+ oldTag,
+ dataOut
+ );
+ // If dialog was cancelled, stop
+ if (!dataOut.result) {
+ return;
+ }
+ switch (dataOut.result.op) {
+ case 'split':
+ for (let libraryID of editableLibraries) {
+ let itemIDs = yield Zotero.Tags.getTagItems(libraryID, oldTagID);
+ yield Zotero.DB.executeTransaction(function* () {
+ for (let itemID of itemIDs) {
+ let item = yield Zotero.Items.getAsync(itemID);
+ for (let tag of dataOut.result.tags) {
+ item.addTag(tag);
+ }
+ item.removeTag(oldTag);
+ yield item.save();
+ }
+ yield Zotero.Tags.purge(oldTagID);
+ });
+ }
+ break;
+
+ case 'edit':
+ for (let libraryID of editableLibraries) {
+ let itemIDs = yield Zotero.Tags.getTagItems(libraryID, oldTagID);
+ yield Zotero.DB.executeTransaction(function* () {
+ for (let itemID of itemIDs) {
+ let item = yield Zotero.Items.getAsync(itemID);
+ item.replaceTag(oldTag, dataOut.result.tag);
+ yield item.save();
+ }
+ });
+ }
+ break;
+
+ case 'delete':
+ for (let libraryID of editableLibraries) {
+ yield Zotero.Tags.removeFromLibrary(libraryID, oldTagID);
+ }
+ break;
+ }
+ }
+ }
+
+ options.restartSync = true;
+ });
+ // If not a background sync, show fixer dialog immediately
+ if (!options.background) {
+ yield e.dialogButtonCallback();
+ }
+ }
+ }
// TEMP
return;
@@ -920,8 +994,7 @@ Zotero.Sync.Runner_Module = function (options = {}) {
if (!skipReload) {
Zotero.reloadDataObjects();
}
- Zotero.Sync.EventListener.resetIgnored();
- }
+ });
/**
@@ -1099,20 +1172,20 @@ Zotero.Sync.Runner_Module = function (options = {}) {
/*// If not an error and there's no explicit button text, don't show
// button to report errors
- if (e.errorType != 'error' && e.data.dialogButtonText === undefined) {
- e.data.dialogButtonText = null;
+ if (e.errorType != 'error' && e.dialogButtonText === undefined) {
+ e.dialogButtonText = null;
}*/
- if (e.data && e.data.dialogButtonText !== null) {
- if (e.data.dialogButtonText === undefined) {
+ if (e.data && e.dialogButtonText !== null) {
+ if (e.dialogButtonText === undefined) {
var buttonText = Zotero.getString('errorReport.reportError');
var buttonCallback = function () {
doc.defaultView.ZoteroPane.reportErrors();
};
}
else {
- var buttonText = e.data.dialogButtonText;
- var buttonCallback = e.data.dialogButtonCallback;
+ var buttonText = e.dialogButtonText;
+ var buttonCallback = e.dialogButtonCallback;
}
var button = doc.createElement('button');
diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties
@@ -56,6 +56,7 @@ general.openPreferences = Open Preferences
general.keys.ctrlShift = Ctrl+Shift+
general.keys.cmdShift = Cmd+Shift+
general.dontShowAgain = Don’t Show Again
+general.fix = Fix…
general.operationInProgress = A Zotero operation is currently in progress.
general.operationInProgress.waitUntilFinished = Please wait until it has finished.
diff --git a/test/tests/syncRunnerTest.js b/test/tests/syncRunnerTest.js
@@ -670,58 +670,9 @@ describe("Zotero.Sync.Runner", function () {
assert.isAbove(lastSyncTime, new Date().getTime() - 1000);
assert.isBelow(lastSyncTime, new Date().getTime());
})
-
-
- it("should show the sync error icon on error", function* () {
- let pubLib = Zotero.Libraries.get(publicationsLibraryID);
- pubLib.libraryVersion = 5;
- yield pubLib.save();
-
- setResponse('keyInfo.fullAccess');
- setResponse('userGroups.groupVersionsEmpty');
- // My Library
- setResponse({
- method: "GET",
- url: "users/1/settings",
- status: 200,
- headers: {
- "Last-Modified-Version": 5
- },
- json: {
- INVALID: true // TODO: Find a cleaner error
- }
- });
- // No publications changes
- setResponse({
- method: "GET",
- url: "users/1/publications/settings?since=5",
- status: 304,
- headers: {
- "Last-Modified-Version": 5
- },
- json: {}
- });
- setResponse({
- method: "GET",
- url: "users/1/publications/fulltext",
- status: 200,
- headers: {
- "Last-Modified-Version": 5
- },
- json: {}
- });
-
- spy = sinon.spy(runner, "updateIcons");
- yield runner.sync();
- assert.isTrue(spy.calledTwice);
- assert.isArray(spy.args[1][0]);
- assert.lengthOf(spy.args[1][0], 1);
- // Not an instance of Error for some reason
- var error = spy.args[1][0][0];
- assert.equal(Object.getPrototypeOf(error).constructor.name, "Error");
- });
})
+
describe("#createAPIKeyFromCredentials()", function() {
var data = {
name: "Automatic Zotero Client Key",
@@ -785,5 +736,211 @@ describe("Zotero.Sync.Runner", function () {
yield runner.deleteAPIKey();
});
- })
+ });
+
+
+ describe("Error Handling", function () {
+ var win;
+
+ afterEach(function () {
+ if (win) {
+ win.close();
+ }
+ });
+
+ it("should show the sync error icon on error", function* () {
+ let pubLib = Zotero.Libraries.get(publicationsLibraryID);
+ pubLib.libraryVersion = 5;
+ yield pubLib.save();
+
+ setResponse('keyInfo.fullAccess');
+ setResponse('userGroups.groupVersionsEmpty');
+ // My Library
+ setResponse({
+ method: "GET",
+ url: "users/1/settings",
+ status: 200,
+ headers: {
+ "Last-Modified-Version": 5
+ },
+ json: {
+ INVALID: true // TODO: Find a cleaner error
+ }
+ });
+ // No publications changes
+ setResponse({
+ method: "GET",
+ url: "users/1/publications/settings?since=5",
+ status: 304,
+ headers: {
+ "Last-Modified-Version": 5
+ },
+ json: {}
+ });
+ setResponse({
+ method: "GET",
+ url: "users/1/publications/fulltext",
+ status: 200,
+ headers: {
+ "Last-Modified-Version": 5
+ },
+ json: {}
+ });
+
+ spy = sinon.spy(runner, "updateIcons");
+ yield runner.sync();
+ assert.isTrue(spy.calledTwice);
+ assert.isArray(spy.args[1][0]);
+ assert.lengthOf(spy.args[1][0], 1);
+ // Not an instance of Error for some reason
+ var error = spy.args[1][0][0];
+ assert.equal(Object.getPrototypeOf(error).constructor.name, "Error");
+ });
+
+
+ // TODO: Test multiple long tags and tags across libraries
+ describe("Long Tag Fixer", function () {
+ it("should split a tag", function* () {
+ win = yield loadZoteroPane();
+
+ var item = yield createDataObject('item');
+ var tag = "title;feeling;matter;drum;treatment;caring;earthy;shrill;unit;obedient;hover;healthy;cheap;clever;wren;wicked;clip;shoe;jittery;shape;clear;dime;increase;complete;level;milk;false;infamous;lamentable;measure;cuddly;tasteless;peace;top;pencil;caption;unusual;depressed;frantic";
+ item.addTag(tag, 1);
+ yield item.saveTx();
+
+ setResponse('keyInfo.fullAccess');
+ setResponse('userGroups.groupVersions');
+ setResponse('groups.ownerGroup');
+ setResponse('groups.memberGroup');
+
+ server.respond(function (req) {
+ if (req.method == "POST" && req.url == baseURL + "users/1/items") {
+ var json = JSON.parse(req.requestBody);
+ if (json[0].tags.length == 1) {
+ req.respond(
+ 200,
+ {
+ "Last-Modified-Version": 5
+ },
+ JSON.stringify({
+ successful: {},
+ success: {},
+ unchanged: {},
+ failed: {
+ "0": {
+ code: 413,
+ message: "Tag 'title;feeling;matter;drum;treatment;caring;earthy;shrill;unit;obedient;hover…' is too long to sync",
+ data: {
+ tag
+ }
+ }
+ }
+ })
+ );
+ }
+ else {
+ let itemJSON = item.toResponseJSON();
+ itemJSON.version = 6;
+ itemJSON.data.version = 6;
+
+ req.respond(
+ 200,
+ {
+ "Last-Modified-Version": 6
+ },
+ JSON.stringify({
+ successful: {
+ "0": itemJSON
+ },
+ success: {
+ "0": json[0].key
+ },
+ unchanged: {},
+ failed: {}
+ })
+ );
+ }
+ }
+ });
+
+ waitForDialog(null, 'accept', 'chrome://zotero/content/longTagFixer.xul');
+ yield runner.sync({ libraries: [Zotero.Libraries.userLibraryID] });
+
+ assert.isFalse(Zotero.Tags.getID(tag));
+ assert.isNumber(Zotero.Tags.getID('feeling'));
+ });
+
+ it("should delete a tag", function* () {
+ win = yield loadZoteroPane();
+
+ var item = yield createDataObject('item');
+ var tag = "title;feeling;matter;drum;treatment;caring;earthy;shrill;unit;obedient;hover;healthy;cheap;clever;wren;wicked;clip;shoe;jittery;shape;clear;dime;increase;complete;level;milk;false;infamous;lamentable;measure;cuddly;tasteless;peace;top;pencil;caption;unusual;depressed;frantic";
+ item.addTag(tag, 1);
+ yield item.saveTx();
+
+ setResponse('keyInfo.fullAccess');
+ setResponse('userGroups.groupVersions');
+ setResponse('groups.ownerGroup');
+ setResponse('groups.memberGroup');
+
+ server.respond(function (req) {
+ if (req.method == "POST" && req.url == baseURL + "users/1/items") {
+ var json = JSON.parse(req.requestBody);
+ if (json[0].tags.length == 1) {
+ req.respond(
+ 200,
+ {
+ "Last-Modified-Version": 5
+ },
+ JSON.stringify({
+ successful: {},
+ success: {},
+ unchanged: {},
+ failed: {
+ "0": {
+ code: 413,
+ message: "Tag 'title;feeling;matter;drum;treatment;caring;earthy;shrill;unit;obedient;hover…' is too long to sync",
+ data: {
+ tag
+ }
+ }
+ }
+ })
+ );
+ }
+ else {
+ let itemJSON = item.toResponseJSON();
+ itemJSON.version = 6;
+ itemJSON.data.version = 6;
+
+ req.respond(
+ 200,
+ {
+ "Last-Modified-Version": 6
+ },
+ JSON.stringify({
+ successful: {
+ "0": itemJSON
+ },
+ success: {
+ "0": json[0].key
+ },
+ unchanged: {},
+ failed: {}
+ })
+ );
+ }
+ }
+ });
+
+ waitForDialog(function (dialog) {
+ dialog.Zotero_Long_Tag_Fixer.switchMode(2);
+ }, 'accept', 'chrome://zotero/content/longTagFixer.xul');
+ yield runner.sync({ libraries: [Zotero.Libraries.userLibraryID] });
+
+ assert.isFalse(Zotero.Tags.getID(tag));
+ assert.isFalse(Zotero.Tags.getID('feeling'));
+ });
+ });
+ });
})