From 108fd304aba2082be2a1bcb8b3d076ea24a3ec92 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 24 Oct 2008 11:10:05 +0000 Subject: [PATCH] Fixed lots of sync bugs - All string values are now trimmed going into the DB (with a migration step for existing values) -- this fixes erroneous conflicts due to leading/trailing whitespace in sync XML being ignored - Case disambiguation fixed on server - Added basic diff ability to collections and tags so that identical objects won't trigger conflicts - Fixed various other bugs that could cause erroneous conflicts - Moved string fields in serialize() objects into a 'fields' object for consistency with Zotero.Item Upshot of most of the above is that identical pre-upgrade libraries should now merge cleanly Also reorganized/simplified/modularized parts of the sync code --- chrome/content/zotero/overlay.js | 7 +- .../content/zotero/xpcom/data/collection.js | 53 +- chrome/content/zotero/xpcom/data/creator.js | 8 + .../content/zotero/xpcom/data/dataObjects.js | 66 ++ chrome/content/zotero/xpcom/data/item.js | 93 +-- chrome/content/zotero/xpcom/data/tag.js | 44 +- chrome/content/zotero/xpcom/report.js | 2 +- chrome/content/zotero/xpcom/schema.js | 51 ++ chrome/content/zotero/xpcom/search.js | 8 +- chrome/content/zotero/xpcom/sync.js | 585 ++++++++++-------- chrome/content/zotero/xpcom/utilities.js | 25 + userdata.sql | 2 +- 12 files changed, 608 insertions(+), 336 deletions(-) diff --git a/chrome/content/zotero/overlay.js b/chrome/content/zotero/overlay.js index 92386b0a2..fe709e49f 100644 --- a/chrome/content/zotero/overlay.js +++ b/chrome/content/zotero/overlay.js @@ -1856,11 +1856,10 @@ var ZoteroPane = new function() } if (!popup) { - try { - // trim - text = text.replace(/^[\xA0\r\n\s]*(.*)[\xA0\r\n\s]*$/m, "$1"); + if (!text) { + text = ''; } - catch (e){} + text = Zotero.Utilities.prototype.trim(text); var item = new Zotero.Item(false, 'note'); item.setNote(text); diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index f822f30b6..019be208c 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -72,6 +72,10 @@ Zotero.Collection.prototype._get = function (field) { Zotero.Collection.prototype._set = function (field, val) { + if (field == 'name') { + val = Zotero.Utilities.prototype.trim(val); + } + switch (field) { case 'id': // set using constructor //case 'collectionID': // set using constructor @@ -648,6 +652,49 @@ Zotero.Collection.prototype.hasDescendent = function(type, id) { } +/** + * Compares this collection to another + * + * Returns a two-element array containing two objects with the differing values, + * or FALSE if no differences + * + * @param {Zotero.Collection} collection Zotero.Collection to compare this item to + * @param {Boolean} includeMatches Include all fields, even those that aren't different + * @param {Boolean} ignoreOnlyDateModified If no fields other than dateModified + * are different, just return false + */ +Zotero.Collection.prototype.diff = function (collection, includeMatches, ignoreOnlyDateModified) { + var diff = []; + var thisData = this.serialize(); + var otherData = collection.serialize(); + var numDiffs = Zotero.Collections.diff(thisData, otherData, diff, includeMatches); + + // For the moment, just compare children and increase numDiffs if any differences + var d1 = Zotero.Utilities.prototype.arrayDiff( + thisData.childCollections, otherData.childCollections + ); + var d2 = Zotero.Utilities.prototype.arrayDiff( + thisData.childCollections, otherData.childCollections + ); + var d3 = Zotero.Utilities.prototype.arrayDiff( + thisData.childItems, otherData.childItems + ); + var d4 = Zotero.Utilities.prototype.arrayDiff( + thisData.childItems, otherData.childItems + ); + numDiffs += d1.length + d2.length + d3.length + d4.length; + + // DEBUG: ignoreOnlyDateModified wouldn't work if includeMatches was set? + if (numDiffs == 0 || + (ignoreOnlyDateModified && numDiffs == 1 + && diff[0].primary && diff[0].primary.dateModified)) { + return false; + } + + return diff; +} + + /** * Deletes collection and all descendent collections (and optionally items) **/ @@ -721,8 +768,10 @@ Zotero.Collection.prototype.serialize = function(nested) { dateModified: this.dateModified, key: this.key }, - name: this.name, - parent: this.parent, + fields: { + name: this.name, + parent: this.parent, + }, childCollections: this.getChildCollections(true), childItems: this.getChildItems(true), descendents: this.id ? this.getDescendents(nested) : [] diff --git a/chrome/content/zotero/xpcom/data/creator.js b/chrome/content/zotero/xpcom/data/creator.js index dcc70174f..a6d1311bd 100644 --- a/chrome/content/zotero/xpcom/data/creator.js +++ b/chrome/content/zotero/xpcom/data/creator.js @@ -73,6 +73,14 @@ Zotero.Creator.prototype._get = function (field) { Zotero.Creator.prototype._set = function (field, val) { + switch (field) { + case 'firstName': + case 'lastName': + case 'shortName': + val = value = Zotero.Utilities.prototype.trim(val); + break; + } + switch (field) { case 'id': // set using constructor //case 'creatorID': // set using constructor diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index 9cefac2d7..2f12b8f50 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -120,5 +120,71 @@ Zotero.DataObjects = function (object, objectPlural, id, table) { delete this._objectCache[ids[i]]; } } + + + /** + * @param {Object} data1 Serialized copy of first object + * @param {Object} data2 Serialized copy of second object + * @param {Array} diff Empty array to put diff data in + * @param {Boolean} [includeMatches=false] Include all fields, even those + * that aren't different + */ + this.diff = function (data1, data2, diff, includeMatches) { + diff.push({}, {}); + var numDiffs = 0; + + var subs = ['primary', 'fields']; + + for each(var sub in subs) { + diff[0][sub] = {}; + diff[1][sub] = {}; + for (var field in data1[sub]) { + if (!data1[sub][field] && !data2[sub][field]) { + continue; + } + + var changed = !data1[sub][field] || !data2[sub][field] || + data1[sub][field] != data2[sub][field]; + + if (includeMatches || changed) { + diff[0][sub][field] = data1[sub][field] ? + data1[sub][field] : ''; + diff[1][sub][field] = data2[sub][field] ? + data2[sub][field] : ''; + } + + if (changed) { + numDiffs++; + } + } + + // DEBUG: some of this is probably redundant + for (var field in data2[sub]) { + if (diff[0][sub][field] != undefined) { + continue; + } + + if (!data1[sub][field] && !data2[sub][field]) { + continue; + } + + var changed = !data1[sub][field] || !data2[sub][field] || + data1[sub][field] != data2[sub][field]; + + if (includeMatches || changed) { + diff[0][sub][field] = data1[sub][field] ? + data1[sub][field] : ''; + diff[1][sub][field] = data2[sub][field] ? + data2[sub][field] : ''; + } + + if (changed) { + numDiffs++; + } + } + } + + return numDiffs; + } } diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index dff8ed15c..e644bc293 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -541,6 +541,10 @@ Zotero.Item.prototype.inCollection = function(collectionID) { * Field can be passed as fieldID or fieldName */ Zotero.Item.prototype.setField = function(field, value, loadIn) { + if (typeof value == 'string') { + value = Zotero.Utilities.prototype.trim(value); + } + this._disabledCheck(); //Zotero.debug("Setting field '" + field + "' to '" + value + "' (loadIn: " + (loadIn ? 'true' : 'false') + ")"); @@ -580,8 +584,7 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) { } // If field value has changed - // dateModified is always marked as changed - if (this['_' + field] != value || field == 'dateModified') { + if (this['_' + field] != value) { Zotero.debug("Field '" + field + "' has changed from '" + this['_' + field] + "' to '" + value + "'", 4); // Save a copy of the object before modifying @@ -1816,14 +1819,6 @@ Zotero.Item.prototype.save = function() { } -Zotero.Item.prototype.updateDateModified = function() { - var sql = "UPDATE items SET dateModified=? WHERE itemID=?"; - Zotero.DB.query(sql, [Zotero.DB.transactionDateTime, this.id]); - sql = "SELECT dateModified FROM items WHERE itemID=?"; - this._dateModified = Zotero.DB.valueQuery(sql, this.id); -} - - Zotero.Item.prototype.isRegularItem = function() { return !(this.isNote() || this.isAttachment()); } @@ -1879,7 +1874,8 @@ Zotero.Item.prototype.setSource = function(sourceItemID) { throw ("setSource() can only be called on items of type 'note' or 'attachment'"); } - if (this._sourceItemID == sourceItemID) { + var oldSourceItemID = this.getSource(); + if (oldSourceItemID == sourceItemID) { Zotero.debug("Source item has not changed in Zotero.Item.setSource()"); return false; } @@ -2029,7 +2025,14 @@ Zotero.Item.prototype.setNote = function(text) { throw ("updateNote() can only be called on notes and attachments"); } - if (text == this._noteText) { + if (typeof text != 'string') { + throw ("text must be a string in Zotero.Item.setNote()"); + } + + text = Zotero.Utilities.prototype.trim(text); + + var oldText = this.getNote(); + if (text == oldText) { Zotero.debug("Note has not changed in Zotero.Item.setNote()"); return false; } @@ -2934,69 +2937,16 @@ Zotero.Item.prototype.getImageSrc = function() { * Returns a two-element array containing two objects with the differing values, * or FALSE if no differences * - * @param object item Zotero.Item to compare this item to - * @param bool includeMatches Include all fields, even those that aren't different - * @param bool ignoreOnlyDateModified If no fields other than dateModified - * are different, just return false + * @param {Zotero.Item} item Zotero.Item to compare this item to + * @param {Boolean} includeMatches Include all fields, even those that aren't different + * @param {Boolean} ignoreOnlyDateModified If no fields other than dateModified + * are different, just return false */ Zotero.Item.prototype.diff = function (item, includeMatches, ignoreOnlyDateModified) { + var diff = []; var thisData = this.serialize(); var otherData = item.serialize(); - - var diff = [{}, {}]; - var numDiffs = 0; - - var subs = ['primary', 'fields']; - - // TODO: base-mapped fields - for each(var sub in subs) { - diff[0][sub] = {}; - diff[1][sub] = {}; - for (var field in thisData[sub]) { - if (!thisData[sub][field] && !otherData[sub][field]) { - continue; - } - - var changed = !thisData[sub][field] || !otherData[sub][field] || - thisData[sub][field] != otherData[sub][field]; - - if (includeMatches || changed) { - diff[0][sub][field] = thisData[sub][field] ? - thisData[sub][field] : ''; - diff[1][sub][field] = otherData[sub][field] ? - otherData[sub][field] : ''; - } - - if (changed) { - numDiffs++; - } - } - - // DEBUG: some of this is probably redundant - for (var field in otherData[sub]) { - if (diff[0][sub][field] != undefined) { - continue; - } - - if (!thisData[sub][field] && !otherData[sub][field]) { - continue; - } - - var changed = !thisData[sub][field] || !otherData[sub][field] || - thisData[sub][field] != otherData[sub][field]; - - if (includeMatches || changed) { - diff[0][sub][field] = thisData[sub][field] ? - thisData[sub][field] : ''; - diff[1][sub][field] = otherData[sub][field] ? - otherData[sub][field] : ''; - } - - if (changed) { - numDiffs++; - } - } - } + var numDiffs = Zotero.Items.diff(thisData, otherData, diff, includeMatches); diff[0].creators = []; diff[1].creators = []; @@ -3012,6 +2962,7 @@ Zotero.Item.prototype.diff = function (item, includeMatches, ignoreOnlyDateModif // TODO: annotations + // DEBUG: ignoreOnlyDateModified wouldn't work if includeMatches was set? if (numDiffs == 0 || (ignoreOnlyDateModified && numDiffs == 1 && diff[0].primary && diff[0].primary.dateModified)) { diff --git a/chrome/content/zotero/xpcom/data/tag.js b/chrome/content/zotero/xpcom/data/tag.js index 5d92f341e..b85130144 100644 --- a/chrome/content/zotero/xpcom/data/tag.js +++ b/chrome/content/zotero/xpcom/data/tag.js @@ -66,6 +66,10 @@ Zotero.Tag.prototype._get = function (field) { Zotero.Tag.prototype._set = function (field, val) { + if (field == 'name') { + val = Zotero.Utilities.prototype.trim(val); + } + switch (field) { case 'id': // set using constructor //case 'tagID': // set using constructor @@ -386,6 +390,39 @@ Zotero.Tag.prototype.save = function () { } +/** + * Compares this tag to another + * + * Returns a two-element array containing two objects with the differing values, + * or FALSE if no differences + * + * @param {Zotero.Tag} tag Zotero.Tag to compare this item to + * @param {Boolean} includeMatches Include all fields, even those that aren't different + * @param {Boolean} ignoreOnlyDateModified If no fields other than dateModified + * are different, just return false + */ +Zotero.Tag.prototype.diff = function (tag, includeMatches, ignoreOnlyDateModified) { + var diff = []; + var thisData = this.serialize(); + var otherData = tag.serialize(); + var numDiffs = Zotero.Tags.diff(thisData, otherData, diff, includeMatches); + + // For the moment, just compare linked items and increase numDiffs if any differences + var d1 = Zotero.Utilities.prototype.arrayDiff( + thisData.linkedItems, otherData.linkedItems + ); + + // DEBUG: ignoreOnlyDateModified wouldn't work if includeMatches was set? + if (numDiffs == 0 || + (ignoreOnlyDateModified && numDiffs == 1 + && diff[0].primary && diff[0].primary.dateModified)) { + return false; + } + + return diff; +} + + Zotero.Tag.prototype.serialize = function () { var obj = { primary: { @@ -393,15 +430,16 @@ Zotero.Tag.prototype.serialize = function () { dateModified: this.dateModified, key: this.key }, - name: this.name, - type: this.type, + fields: { + name: this.name, + type: this.type, + }, linkedItems: this.getLinkedItems(true), }; return obj; } - /** * Remove tag from all linked items * diff --git a/chrome/content/zotero/xpcom/report.js b/chrome/content/zotero/xpcom/report.js index 7886930f7..dd23518bb 100644 --- a/chrome/content/zotero/xpcom/report.js +++ b/chrome/content/zotero/xpcom/report.js @@ -313,7 +313,7 @@ Zotero.Report = new function() { content += '

' + escapeXML(str) + '

\n'; content += '\n'; } diff --git a/chrome/content/zotero/xpcom/schema.js b/chrome/content/zotero/xpcom/schema.js index e1bfff249..588816930 100644 --- a/chrome/content/zotero/xpcom/schema.js +++ b/chrome/content/zotero/xpcom/schema.js @@ -2049,6 +2049,57 @@ Zotero.Schema = new function(){ } } } + + if (i==45) { + var rows = Zotero.DB.query("SELECT * FROM itemDataValues WHERE value REGEXP '(^\\s+|\\s+$)'"); + if (rows) { + for each(var row in rows) { + var trimmed = Zotero.Utilities.prototype.trim(row.value); + var valueID = Zotero.DB.valueQuery("SELECT valueID FROM itemDataValues WHERE value=?", trimmed); + if (valueID) { + Zotero.DB.query("UPDATE itemData SET valueID=? WHERE valueID=?", [valueID, row.valueID]); + Zotero.DB.query("DELETE FROM itemDataValues WHERE valueID=?", row.valueID); + } + else { + Zotero.DB.query("UPDATE itemDataValues SET value=? WHERE valueID=?", [trimmed, row.valueID]); + } + } + } + + Zotero.DB.query("UPDATE creatorData SET firstName=TRIM(firstName), lastName=TRIM(lastName)"); + var rows = Zotero.DB.query("SELECT * FROM creatorData ORDER BY lastName, firstName, creatorDataID"); + if (rows) { + for (var j=0; j remoteObj.dateModified) { - Zotero.Sync.addToUpdated(uploadIDs.updated.items, obj.id); + syncSession.addToUpdated(type, obj.id); continue; } @@ -1487,45 +1531,58 @@ Zotero.Sync.Server.Data = new function() { else { // Skip item if dateModified is the only modified // field (and no linked creators changed) - if (type == 'item') { - var diff = obj.diff(remoteObj, false, true); - if (!diff) { - // Check if creators changed - var creatorsChanged = false; + switch (type) { + // Will be handled by item CR for now + case 'creator': + remoteCreatorStore[remoteObj.id] = remoteObj; + syncSession.removeFromUpdated(type, obj.id); + continue; - var creators = obj.getCreators(); - var remoteCreators = remoteObj.getCreators(); - - if (creators.length != remoteCreators.length) { - creatorsChanged = true; - } - else { - creators = creators.concat(remoteCreators); - for each(var creator in creators) { - var r = remoteCreatorStore[creator.ref.id]; - // Doesn't include dateModified - if (r && !r.equals(creator.ref)) { - creatorsChanged = true; - break; + case 'item': + var diff = obj.diff(remoteObj, false, true); + if (!diff) { + // Check if creators changed + var creatorsChanged = false; + + var creators = obj.getCreators(); + var remoteCreators = remoteObj.getCreators(); + + if (creators.length != remoteCreators.length) { + creatorsChanged = true; + } + else { + creators = creators.concat(remoteCreators); + for each(var creator in creators) { + var r = remoteCreatorStore[creator.ref.id]; + // Doesn't include dateModified + if (r && !r.equals(creator.ref)) { + creatorsChanged = true; + break; + } } } + if (!creatorsChanged) { + syncSession.removeFromUpdated(type, obj.id); + continue; + } } - if (!creatorsChanged) { + break; + + case 'collection': + case 'tag': + var diff = obj.diff(remoteObj, false, true); + if (!diff) { + syncSession.removeFromUpdated(type, obj.id); continue; } - } - } - - // Will be handled by item CR for now - if (type == 'creator') { - remoteCreatorStore[remoteObj.id] = remoteObj; - continue; - } - - if (type != 'item') { - var msg = "Reconciliation unimplemented for " + types; - alert(msg); - throw(msg); + break; + + default: + Zotero.debug(obj); + Zotero.debug(remoteObj); + var msg = "Reconciliation unimplemented for " + types; + alert(msg); + throw(msg); } if (obj.isAttachment()) { @@ -1566,15 +1623,15 @@ Zotero.Sync.Server.Data = new function() { // // Object might not appear in local update array if server // data was cleared and synched from another client - var index = uploadIDs.updated[types].indexOf(oldID); + var index = syncSession.uploadIDs.updated[types].indexOf(oldID); if (index != -1) { - uploadIDs.updated[types][index] = newID; + syncSession.uploadIDs.updated[types][index] = newID; } // Update id in local deletions array - for (var i in uploadIDs.deleted[types]) { - if (uploadIDs.deleted[types][i].id == oldID) { - uploadIDs.deleted[types][i] = newID; + for (var i in syncSession.uploadIDs.deleted[types]) { + if (syncSession.uploadIDs.deleted[types][i].id == oldID) { + syncSession.uploadIDs.deleted[types][i] = newID; } } @@ -1587,11 +1644,11 @@ Zotero.Sync.Server.Data = new function() { if (type == 'creator') { var linkedItems = obj.getLinkedItems(); if (linkedItems) { - Zotero.Sync.addToUpdated(uploadIDs.updated.items, linkedItems); + syncSession.addToUpdated('item', linkedItems); } } - uploadIDs.changed[types][oldID] = { + syncSession.uploadIDs.changed[types][oldID] = { oldID: oldID, newID: newID }; @@ -1605,7 +1662,7 @@ Zotero.Sync.Server.Data = new function() { isNewObject = true; // Check if object has been deleted locally - for each(var pair in uploadIDs.deleted[types]) { + for each(var pair in syncSession.uploadIDs.deleted[types]) { if (pair.id != parseInt(xmlNode.@id) || pair.key != xmlNode.@key.toString()) { continue; @@ -1630,7 +1687,7 @@ Zotero.Sync.Server.Data = new function() { + " from '" + oldKey + "' to '" + newKey + "'", 2); keyObj.key = newKey; keyObj.save(); - Zotero.Sync.addToUpdated(uploadIDs.updated[types], keyObj.id); + syncSession.addToUpdated(type, keyObj.id); } } @@ -1652,13 +1709,13 @@ Zotero.Sync.Server.Data = new function() { var tagName = xmlNode.@name.toString(); var tagType = xmlNode.@type.toString() ? parseInt(xmlNode.@type) : 0; - var linkedItems = _deleteConflictingTag(tagName, tagType, uploadIDs); + var linkedItems = _deleteConflictingTag(syncSession, tagName, tagType); if (linkedItems) { obj.dateModified = Zotero.DB.transactionDateTime; for each(var id in linkedItems) { obj.addItem(id); } - Zotero.Sync.addToUpdated(uploadIDs.updated.tags, parseInt(xmlNode.@id)); + syncSession.addToUpdated('tag', parseInt(xmlNode.@id)); } } @@ -1670,12 +1727,8 @@ Zotero.Sync.Server.Data = new function() { obj ]); } - // Child items have to be saved after parent items - else if (type == 'item' && obj.getSource()) { - toSaveChildren.push(obj); - } else { - toSaveParents.push(obj); + toSave.push(obj); } // Don't use assigned-but-unsaved ids for new ids @@ -1689,11 +1742,10 @@ Zotero.Sync.Server.Data = new function() { if (obj.isRegularItem()) { var creators = obj.getCreators(); for each(var creator in creators) { - Zotero.Sync.removeFromDeleted( - uploadIDs.deleted.creators, + syncSession.removeFromDeleted( + 'creator', creator.ref.id, - creator.ref.key, - true + creator.ref.key ); } } @@ -1741,12 +1793,7 @@ Zotero.Sync.Server.Data = new function() { } // Local object hasn't been modified -- delete else { - if (type == 'item' && obj.getSource()) { - toDeleteChildren.push(id); - } - else { - toDeleteParents.push(id); - } + toDelete.push(id); } } } @@ -1755,80 +1802,20 @@ Zotero.Sync.Server.Data = new function() { // Reconcile objects that have changed locally and remotely // if (toReconcile.length) { - var io = { - dataIn: { - captions: [ - // TODO: localize - 'Local Item', - 'Remote Item', - 'Merged Item' - ], - objects: toReconcile - } - }; - - if (type == 'item') { - io.dataIn.changedCreators = remoteCreatorStore; - } - - var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"] - .getService(Components.interfaces.nsIWindowMediator); - var lastWin = wm.getMostRecentWindow("navigator:browser"); - lastWin.openDialog('chrome://zotero/content/merge.xul', '', 'chrome,modal,centerscreen', io); - - if (io.dataOut) { - for each(var obj in io.dataOut) { - // TODO: do we need to make sure item isn't already being saved? - - // Handle items deleted during merge - if (obj.ref == 'deleted') { - // Deleted item was remote - if (obj.left != 'deleted') { - if (type == 'item' && obj.left.getSource()) { - toDeleteParents.push(obj.id); - } - else { - toDeleteChildren.push(obj.id); - } - - if (relatedItemsStore[obj.id]) { - delete relatedItemsStore[obj.id]; - } - - uploadIDs.deleted[types].push({ - id: obj.id, - key: obj.left.key - }); - } - continue; - } - - if (type == 'item' && obj.ref.getSource()) { - toSaveParents.push(obj.ref); - } - else { - toSaveChildren.push(obj.ref); - } - - // Don't use assigned-but-unsaved ids for new ids - Zotero.ID.skip(types, obj.id); - - // Item had been deleted locally, so remove from - // deleted array - if (obj.left == 'deleted') { - Zotero.Sync.removeFromDeleted(uploadIDs.deleted[types], obj.id, obj.ref.key); - } - - // TODO: only upload if the local item was chosen - // or remote item was changed - - Zotero.Sync.addToUpdated(uploadIDs.updated[types], obj.id); - } - } - else { + var mergeData = _reconcile(type, toReconcile, remoteCreatorStore); + if (!mergeData) { + // TODO: throw? Zotero.DB.rollbackTransaction(); return false; } + _processMergeData( + syncSession, + type, + mergeData, + toSave, + toDelete, + relatedItemsStore + ); } /* @@ -1836,7 +1823,7 @@ Zotero.Sync.Server.Data = new function() { // Temporarily remove and store subcollections before saving // since referenced collections may not exist yet var collections = []; - for each(var obj in toSaveParents) { + for each(var obj in toSave) { var colIDs = obj.getChildCollections(true); // TODO: use exist(), like related items above obj.childCollections = []; @@ -1850,10 +1837,18 @@ Zotero.Sync.Server.Data = new function() { // Save objects Zotero.debug('Saving merged ' + types); - for each(var obj in toSaveParents) { - obj.save(); + // Save parent items first + if (type == 'item') { + for each(var obj in toSave) { + if (!obj.getSource()) { + obj.save(); + } + } } - for each(var obj in toSaveChildren) { + for each(var obj in toSave) { + if (type == 'item' && !obj.getSource()) { + continue; + } obj.save(); } @@ -1882,17 +1877,37 @@ Zotero.Sync.Server.Data = new function() { // Delete Zotero.debug('Deleting merged ' + types); - if (toDeleteChildren.length) { - Zotero.Sync.EventListener.ignoreDeletions(type, toDeleteChildren); - Zotero[Types].erase(toDeleteChildren); - Zotero.Sync.EventListener.unignoreDeletions(type, toDeleteChildren); + if (toDelete.length) { + // Items have to be deleted children-first + if (type == 'item') { + var parents = []; + var children = []; + for each(var id in toDelete) { + var item = Zotero.Items.get(id); + if (item.getSource()) { + children.push(item.id); + } + else { + parents.push(item.id); + } + } + if (children.length) { + Zotero.Sync.EventListener.ignoreDeletions('item', children); + Zotero.Items.erase(children); + Zotero.Sync.EventListener.unignoreDeletions('item', children); + } + if (parents.length) { + Zotero.Sync.EventListener.ignoreDeletions('item', parents); + Zotero.Items.erase(parents); + Zotero.Sync.EventListener.unignoreDeletions('item', parents); + } + } + else { + Zotero.Sync.EventListener.ignoreDeletions(type, toDelete); + Zotero[Types].erase(toDelete); + Zotero.Sync.EventListener.unignoreDeletions(type, toDelete); + } } - if (toDeleteParents.length) { - Zotero.Sync.EventListener.ignoreDeletions(type, toDeleteParents); - Zotero[Types].erase(toDeleteParents); - Zotero.Sync.EventListener.unignoreDeletions(type, toDeleteParents); - } - // Check mod times of updated items against stored time to see // if they've been updated elsewhere and mark for download if so @@ -1907,7 +1922,7 @@ Zotero.Sync.Server.Data = new function() { } } - var xmlstr = Zotero.Sync.Server.Data.buildUploadXML(uploadIDs); + var xmlstr = Zotero.Sync.Server.Data.buildUploadXML(syncSession); //Zotero.debug(xmlstr); //throw ('break'); @@ -1919,30 +1934,11 @@ Zotero.Sync.Server.Data = new function() { /** - * ids = { - * updated: { - * items: [123, 234, 345, 456], - * creators: [321, 432, 543, 654] - * }, - * changed: { - * items: { - * oldID: { oldID: 1234, newID: 5678 }, ... - * }, - * creators: { - * oldID: { oldID: 1234, newID: 5678 }, ... - * } - * }, - * deleted: { - * items: [ - * { id: 1234, key: ABCDEFGHIJKMNPQRSTUVWXYZ23456789 }, ... - * ], - * creators: [ - * { id: 1234, key: ABCDEFGHIJKMNPQRSTUVWXYZ23456789 }, ... - * ] - * } - * }; + * @param {Zotero.Sync.Server.Session} syncSession */ - function buildUploadXML(ids) { + this.buildUploadXML = function (syncSession) { + var ids = syncSession.uploadIDs; + var xml = // Add API version attribute @@ -1968,7 +1964,7 @@ Zotero.Sync.Server.Data = new function() { case 'item': var objs = Zotero[Types].get(ids.updated[types]); for each(var obj in objs) { - xml[types][type] += this[type + 'ToXML'](obj); + xml[types][type] += this[type + 'ToXML'](obj, syncSession); } break; @@ -2013,9 +2009,87 @@ Zotero.Sync.Server.Data = new function() { /** - * Converts a Zotero.Item object to an E4X object + * Open a conflict resolution window and return the results + * + * @param {String} type 'item', 'collection', etc. + * @param {Array[]} objectPairs Array of arrays of pairs of Item, Collection, etc. */ - function itemToXML(item) { + function _reconcile(type, objectPairs, changedCreators) { + var io = { + dataIn: { + captions: [ + // TODO: localize + 'Local Item', + 'Remote Item', + 'Merged Item' + ], + objects: objectPairs + } + }; + + if (type == 'item') { + io.dataIn.changedCreators = changedCreators; + } + + var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"] + .getService(Components.interfaces.nsIWindowMediator); + var lastWin = wm.getMostRecentWindow("navigator:browser"); + lastWin.openDialog('chrome://zotero/content/merge.xul', '', 'chrome,modal,centerscreen', io); + + return io.dataOut; + } + + + /** + * Process the results of conflict resolution + */ + function _processMergeData(syncSession, type, data, toSave, toDelete, relatedItems) { + var types = Zotero.Sync.syncObjects[type].plural.toLowerCase(); + + for each(var obj in data) { + // TODO: do we need to make sure item isn't already being saved? + + // Handle items deleted during merge + if (obj.ref == 'deleted') { + // Deleted item was remote + if (obj.left != 'deleted') { + toDelete.push(obj.id); + + if (relatedItems[obj.id]) { + delete relatedItems[obj.id]; + } + + syncSession.addToDeleted(type, obj.id, obj.left.key); + } + continue; + } + + toSave.push(obj.ref); + + // Don't use assigned-but-unsaved ids for new ids + Zotero.ID.skip(types, obj.id); + + // Item had been deleted locally, so remove from + // deleted array + if (obj.left == 'deleted') { + syncSession.removeFromDeleted(type, obj.id, obj.ref.key); + } + + // TODO: only upload if the local item was chosen + // or remote item was changed + + syncSession.addToUpdated(type, obj.id); + } + } + + + /** + * Converts a Zotero.Item object to an E4X object + * + * @param {Zotero.Item} item + * @param {Zotero.Sync.Server.Session} [syncSession] + */ + function itemToXML(item, syncSession) { var xml = ; var item = item.serialize(); @@ -2083,10 +2157,21 @@ Zotero.Sync.Server.Data = new function() { // Creators for (var index in item.creators) { var newCreator = ; - newCreator.@id = item.creators[index].creatorID; + var creatorID = item.creators[index].creatorID; + newCreator.@id = creatorID; newCreator.@creatorType = item.creators[index].creatorType; newCreator.@index = index; xml.creator += newCreator; + + /* + // Add creator XML as glue if not already included in sync session + if (syncSession && + syncSession.uploadIDs.updated.creators.indexOf(creatorID) == -1) { + var creator = Zotero.Creators.get(creatorID); + var creatorXML = Zotero.Sync.Server.Data.creatorToXML(creator); + xml.creator.creator = creatorXML; + } + */ } // Related items @@ -2315,7 +2400,7 @@ Zotero.Sync.Server.Data = new function() { collection.name = xmlCollection.@name.toString(); if (!skipPrimary) { collection.parent = xmlCollection.@parent.toString() ? - parseInt(xmlCollection.@parent) : false; + parseInt(xmlCollection.@parent) : null; collection.dateAdded = xmlCollection.@dateAdded.toString(); collection.dateModified = xmlCollection.@dateModified.toString(); collection.key = xmlCollection.@key.toString(); @@ -2600,7 +2685,7 @@ Zotero.Sync.Server.Data = new function() { * deleted tag, or FALSE if no * matching tag found */ - function _deleteConflictingTag(name, type, uploadIDs) { + function _deleteConflictingTag(syncSession, name, type) { var tagID = Zotero.Tags.getID(name, type); if (tagID) { var tag = Zotero.Tags.get(tagID); @@ -2609,14 +2694,8 @@ Zotero.Sync.Server.Data = new function() { // DEBUG: should purge() be called by Tags.erase() Zotero.Tags.purge(); - Zotero.Sync.removeFromUpdated( - uploadIDs.updated.tags, tagID - ); - - uploadIDs.deleted.tags.push({ - id: tagID, - key: tag.key - }); + syncSession.removeFromUpdated('tag', tagID); + syncSession.addToDeleted('tag', tagID, tag.key); return linkedItems; } diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js index 8bfbb3ad4..31101a644 100644 --- a/chrome/content/zotero/xpcom/utilities.js +++ b/chrome/content/zotero/xpcom/utilities.js @@ -290,6 +290,31 @@ Zotero.Utilities.prototype.isInt = function(x) { return false; } + +/** + * Compares an array with another (comparator) and returns an array with + * the values from comparator that don't exist in vector + * + * Code by Carlos R. L. Rodrigues + * From http://jsfromhell.com/array/diff [rev. #1] + * + * @param {Array} v Array that will be checked + * @param {Array} c Array that will be compared + * @param {Boolean} useIndex If true, returns an array containing just + * the index of the comparator's elements; + * otherwise returns the values + */ +Zotero.Utilities.prototype.arrayDiff = function(v, c, m) { + var d = [], e = -1, h, i, j, k; + for(i = c.length, k = v.length; i--;){ + for(j = k; j && (h = c[i] !== v[--j]);); + h && (d[++e] = m ? i : c[i]); + } + return d; +}; + + + /** * Generate a random integer between min and max inclusive * diff --git a/userdata.sql b/userdata.sql index c899c584e..b3bdb1fa9 100644 --- a/userdata.sql +++ b/userdata.sql @@ -1,4 +1,4 @@ --- 44 +-- 45 -- This file creates tables containing user-specific data -- any changes made -- here must be mirrored in transition steps in schema.js::_migrateSchema()