From c7b2c84869ddf8c61aa5923c84517b0053637567 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 27 Dec 2008 05:42:52 +0000 Subject: [PATCH] - Add automatic merging of collection and tag metadata and associated items, with warning alerts (eventually to be converted to logged notifications) - Switch to using only keys for deleted items - Fix various tag-related problems - Probably other things --- .../content/zotero/xpcom/data/collection.js | 14 +- chrome/content/zotero/xpcom/data/tag.js | 57 ++- chrome/content/zotero/xpcom/schema.js | 10 +- chrome/content/zotero/xpcom/sync.js | 476 +++++++++++------- chrome/content/zotero/xpcom/utilities.js | 42 +- userdata.sql | 3 +- 6 files changed, 389 insertions(+), 213 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 963eebe367..cd40d025f8 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -398,7 +398,7 @@ Zotero.Collection.prototype.save = function () { currentIDs = []; } - if (this._previousData.childCollections) { + if (this._previousData) { for each(var id in this._previousData.childCollections) { if (currentIDs.indexOf(id) == -1) { removed.push(id); @@ -406,7 +406,7 @@ Zotero.Collection.prototype.save = function () { } } for each(var id in currentIDs) { - if (this._previousData.childCollections && + if (this._previousData && this._previousData.childCollections.indexOf(id) != -1) { continue; } @@ -442,7 +442,7 @@ Zotero.Collection.prototype.save = function () { currentIDs = []; } - if (this._previousData.childItems) { + if (this._previousData) { for each(var id in this._previousData.childItems) { if (currentIDs.indexOf(id) == -1) { removed.push(id); @@ -450,7 +450,7 @@ Zotero.Collection.prototype.save = function () { } } for each(var id in currentIDs) { - if (this._previousData.childItems && + if (this._previousData && this._previousData.childItems.indexOf(id) != -1) { continue; } @@ -801,6 +801,8 @@ Zotero.Collection.prototype.toArray = function() { Zotero.Collection.prototype.serialize = function(nested) { + var childCollections = this.getChildCollections(true); + var childItems = this.getChildItems(true); var obj = { primary: { collectionID: this.id, @@ -811,8 +813,8 @@ Zotero.Collection.prototype.serialize = function(nested) { name: this.name, parent: this.parent, }, - childCollections: this.getChildCollections(true), - childItems: this.getChildItems(true), + childCollections: childCollections ? childCollections : [], + childItems: childItems ? childItems : [], descendents: this.id ? this.getDescendents(nested) : [] }; return obj; diff --git a/chrome/content/zotero/xpcom/data/tag.js b/chrome/content/zotero/xpcom/data/tag.js index 3ee974cd46..8cc19f63b1 100644 --- a/chrome/content/zotero/xpcom/data/tag.js +++ b/chrome/content/zotero/xpcom/data/tag.js @@ -143,8 +143,8 @@ Zotero.Tag.prototype.loadFromRow = function (row) { /** * Returns items linked to this tag * - * @param bool asIDs Return as itemIDs - * @return array Array of Zotero.Item instances or itemIDs, + * @param {Boolean} asIDs Return as itemIDs + * @return {Array} Array of Zotero.Item instances or itemIDs, * or FALSE if none */ Zotero.Tag.prototype.getLinkedItems = function (asIDs) { @@ -211,7 +211,7 @@ Zotero.Tag.prototype.removeItem = function (itemID) { } -Zotero.Tag.prototype.save = function () { +Zotero.Tag.prototype.save = function (full) { // Default to manual tag if (!this.type) { this.type = 0; @@ -307,7 +307,7 @@ Zotero.Tag.prototype.save = function () { // Linked items - if (this._changed.linkedItems) { + if (full || this._changed.linkedItems) { var removed = []; var newids = []; var currentIDs = this.getLinkedItems(true); @@ -315,19 +315,31 @@ Zotero.Tag.prototype.save = function () { currentIDs = []; } - if (this._previousData.linkedItems) { - for each(var id in this._previousData.linkedItems) { - if (currentIDs.indexOf(id) == -1) { - removed.push(id); - } + // Use the database for comparison instead of relying on the cache + // This is necessary for a syncing edge case (described in sync.js). + if (full) { + var sql = "SELECT itemID FROM itemTags WHERE tagID=?"; + var dbItemIDs = Zotero.DB.columnQuery(sql, tagID); + if (dbItemIDs) { + removed = Zotero.Utilities.prototype.arrayDiff(currentIDs, dbItemIDs); + newids = Zotero.Utilities.prototype.arrayDiff(dbItemIDs, currentIDs); + } + else { + newids = currentIDs; } } - for each(var id in currentIDs) { - if (this._previousData.linkedItems && - this._previousData.linkedItems.indexOf(id) != -1) { - continue; + else { + if (this._previousData.linkedItems) { + removed = Zotero.Utilities.prototype.arrayDiff( + currentIDs, this._previousData.linkedItems + ); + newids = Zotero.Utilities.prototype.arrayDiff( + this._previousData.linkedItems, currentIDs + ); + } + else { + newids = currentIDs; } - newids.push(id); } if (removed.length) { @@ -414,8 +426,17 @@ Zotero.Tag.prototype.diff = function (tag, includeMatches, ignoreOnlyDateModifie var d2 = Zotero.Utilities.prototype.arrayDiff( otherData.linkedItems, thisData.linkedItems ); - numDiffs += d1.length; - numDiffs += d2.length; + numDiffs += d1.length + d2.length; + + if (d1.length || d2.length) { + numDiffs += d1.length + d2.length; + diff[0].linkedItems = d1; + diff[1].linkedItems = d2; + } + else { + diff[0].linkedItems = []; + diff[1].linkedItems = []; + } // DEBUG: ignoreOnlyDateModified wouldn't work if includeMatches was set? if (numDiffs == 0 || @@ -429,6 +450,8 @@ Zotero.Tag.prototype.diff = function (tag, includeMatches, ignoreOnlyDateModifie Zotero.Tag.prototype.serialize = function () { + var linkedItems = this.getLinkedItems(true); + var obj = { primary: { tagID: this.id, @@ -439,7 +462,7 @@ Zotero.Tag.prototype.serialize = function () { name: this.name, type: this.type, }, - linkedItems: this.getLinkedItems(true), + linkedItems: linkedItems ? linkedItems : [] }; return obj; } diff --git a/chrome/content/zotero/xpcom/schema.js b/chrome/content/zotero/xpcom/schema.js index 4c02d42381..8cb4f22ca5 100644 --- a/chrome/content/zotero/xpcom/schema.js +++ b/chrome/content/zotero/xpcom/schema.js @@ -2144,7 +2144,15 @@ Zotero.Schema = new function(){ } } - // + // // 1.5 Sync Preview 3.6 + if (i==47) { + Zotero.DB.query("ALTER TABLE syncDeleteLog RENAME TO syncDeleteLogOld"); + Zotero.DB.query("DROP INDEX syncDeleteLog_timestamp"); + Zotero.DB.query("CREATE TABLE syncDeleteLog (\n syncObjectTypeID INT NOT NULL,\n key TEXT NOT NULL UNIQUE,\n timestamp INT NOT NULL,\n FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID)\n);"); + Zotero.DB.query("CREATE INDEX syncDeleteLog_timestamp ON syncDeleteLog(timestamp);"); + Zotero.DB.query("INSERT INTO syncDeleteLog SELECT syncObjectTypeID, key, timestamp FROM syncDeleteLogOld"); + Zotero.DB.query("DROP TABLE syncDeleteLogOld"); + } } _updateDBVersion('userdata', toVersion); diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js index ca76655c0c..da1831111f 100644 --- a/chrome/content/zotero/xpcom/sync.js +++ b/chrome/content/zotero/xpcom/sync.js @@ -136,10 +136,10 @@ Zotero.Sync = new function() { /** * @param object lastSyncDate JS Date object - * @return mixed Returns object with deleted ids + * @return mixed * { - * items: [ { id: 123, key: ABCD1234 }, ... ] - * creators: [ { id: 123, key: ABCD1234 }, ... ], + * items: [ 'ABCD1234', 'BCDE2345', ... ] + * creators: [ 'ABCD1234', 'BCDE2345', ... ], * ... * } * or FALSE if none or -1 if last sync time is before start of log @@ -162,7 +162,7 @@ Zotero.Sync = new function() { } var param = false; - var sql = "SELECT syncObjectTypeID, objectID, key FROM syncDeleteLog"; + var sql = "SELECT syncObjectTypeID, key FROM syncDeleteLog"; if (lastSyncDate) { param = Zotero.Date.toUnixTimestamp(lastSyncDate); sql += " WHERE timestamp>?"; @@ -174,20 +174,17 @@ Zotero.Sync = new function() { return false; } - var deletedIDs = {}; + var deletedKeys = {}; for each(var syncObject in this.syncObjects) { - deletedIDs[syncObject.plural.toLowerCase()] = []; + deletedKeys[syncObject.plural.toLowerCase()] = []; } for each(var row in rows) { var type = this.getObjectTypeName(row.syncObjectTypeID); type = this.syncObjects[type].plural.toLowerCase() - deletedIDs[type].push({ - id: row.objectID, - key: row.key - }); + deletedKeys[type].push(row.key); } - return deletedIDs; + return deletedKeys; } @@ -297,7 +294,7 @@ Zotero.Sync.EventListener = new function () { Zotero.DB.beginTransaction(); if (event == 'delete') { - var sql = "INSERT INTO syncDeleteLog VALUES (?, ?, ?, ?)"; + var sql = "INSERT INTO syncDeleteLog VALUES (?, ?, ?)"; var syncStatement = Zotero.DB.getStatement(sql); if (isItem && Zotero.Sync.Storage.active) { @@ -326,9 +323,8 @@ Zotero.Sync.EventListener = new function () { } syncStatement.bindInt32Parameter(0, objectTypeID); - syncStatement.bindInt32Parameter(1, ids[i]); - syncStatement.bindStringParameter(2, key); - syncStatement.bindInt32Parameter(3, ts); + syncStatement.bindStringParameter(1, key); + syncStatement.bindInt32Parameter(2, ts); if (storageEnabled && oldItem.primary.itemType == 'attachment' && @@ -652,7 +648,7 @@ Zotero.Sync.Server = new function () { }); this.nextLocalSyncDate = false; - this.apiVersion = 2; + this.apiVersion = 3; default xml namespace = ''; @@ -715,7 +711,7 @@ Zotero.Sync.Server = new function () { } - Zotero.debug('Got session ID ' + _sessionID + ' from server'); + //Zotero.debug('Got session ID ' + _sessionID + ' from server'); if (callback) { callback(); @@ -1318,10 +1314,10 @@ Zotero.BufferedInputListener.prototype = { * }, * deleted: { * items: [ - * { id: 1234, key: ABCDEFGHIJKMNPQRSTUVWXYZ23456789 }, ... + * 'ABCD1234', 'BCDE2345', ... * ], * creators: [ - * { id: 1234, key: ABCDEFGHIJKMNPQRSTUVWXYZ23456789 }, ... + * 'ABCD1234', 'BCDE2345', ... * ] * } * }; @@ -1370,26 +1366,21 @@ Zotero.Sync.Server.Session.prototype.removeFromUpdated = function (syncObjectTyp } -Zotero.Sync.Server.Session.prototype.addToDeleted = function (syncObjectTypeName, id, key) { +Zotero.Sync.Server.Session.prototype.addToDeleted = function (syncObjectTypeName, key) { var pluralType = Zotero.Sync.syncObjects[syncObjectTypeName].plural.toLowerCase(); var deleted = this.uploadIDs.deleted[pluralType]; - - // DEBUG: inefficient - for each(var pair in deleted) { - if (pair.id == id) { - return; - } + if (deleted.indexOf(key) != -1) { + return; } - deleted.push({ id: id, key: key}); + deleted.push(key); } -Zotero.Sync.Server.Session.prototype.removeFromDeleted = function (syncObjectTypeName, id, key) { +Zotero.Sync.Server.Session.prototype.removeFromDeleted = function (syncObjectTypeName, key) { var pluralType = Zotero.Sync.syncObjects[syncObjectTypeName].plural.toLowerCase(); var deleted = this.uploadIDs.deleted[pluralType]; - for (var i=0; i'); - deletexml.@id = obj.id; - deletexml.@key = obj.key; + deletexml.@key = key; xml.deleted[types][type] += deletexml; } } @@ -2171,6 +2120,215 @@ Zotero.Sync.Server.Data = new function() { } + function _mergeCollection(localObj, remoteObj) { + var diff = localObj.diff(remoteObj, false, true); + if (!diff) { + return false; + } + + Zotero.debug("COLLECTION HAS CHANGED"); + Zotero.debug(diff); + + // Local is newer + if (diff[0].primary.dateModified > + diff[1].primary.dateModified) { + var remoteIsTarget = false; + var targetObj = localObj; + var targetDiff = diff[0]; + var otherDiff = diff[1]; + } + // Remote is newer + else { + var remoteIsTarget = true; + var targetObj = remoteObj; + var targetDiff = diff[1]; + var otherDiff = diff[0]; + } + + if (targetDiff.fields.name) { + var msg = _generateAutoChangeMessage( + 'collection', diff[0].fields.name, diff[1].fields.name, remoteIsTarget + ); + // TODO: log rather than alert + alert(msg); + } + + // Check for child collections in the other object + // that aren't in the target one + if (otherDiff.childCollections.length) { + // TODO: log + // TODO: add + throw ("Collection hierarchy conflict resolution is unimplemented"); + } + + // Add items in other object to target one + if (otherDiff.childItems.length) { + var childItems = targetObj.getChildItems(true); + targetObj.childItems = childItems.concat(otherDiff.childItems); + + var msg = _generateCollectionItemMergeMessage( + targetObj.name, + otherDiff.childItems, + remoteIsTarget + ); + // TODO: log rather than alert + alert(msg); + } + + targetObj.save(); + return true; + } + + + function _mergeTag(localObj, remoteObj) { + var diff = localObj.diff(remoteObj, false, true); + if (!diff) { + return false; + } + + Zotero.debug("TAG HAS CHANGED"); + Zotero.debug(diff); + + // Local is newer + if (diff[0].primary.dateModified > + diff[1].primary.dateModified) { + var remoteIsTarget = false; + var targetObj = localObj; + var targetDiff = diff[0]; + var otherDiff = diff[1]; + } + // Remote is newer + else { + var remoteIsTarget = true; + var targetObj = remoteObj; + var targetDiff = diff[1]; + var otherDiff = diff[0]; + } + + // TODO: log old name + if (targetDiff.fields.name) { + var msg = _generateAutoChangeMessage( + 'tag', diff[0].fields.name, diff[1].fields.name, remoteIsTarget + ); + alert(msg); + } + + // Add linked items in the other object to the target one + if (otherDiff.linkedItems.length) { + // need to handle changed items + + var linkedItems = targetObj.getLinkedItems(true); + targetObj.linkedItems = linkedItems.concat(otherDiff.linkedItems); + + var msg = _generateTagItemMergeMessage( + targetObj.name, + otherDiff.linkedItems, + remoteIsTarget + ); + // TODO: log rather than alert + alert(msg); + } + + targetObj.save(); + return true; + } + + + /** + * @param {String} itemType + * @param {String} localName + * @param {String} remoteName + * @param {Boolean} [remoteMoreRecent=false] + */ + function _generateAutoChangeMessage(itemType, localName, remoteName, remoteMoreRecent) { + if (localName === null) { + // TODO: localize + localName = "[deleted]"; + var localDelete = true; + } + + // TODO: localize + var msg = "A " + itemType + " has changed both locally and " + + "remotely since the last sync:"; + msg += "\n\n"; + msg += "Local version: " + localName + "\n"; + msg += "Remote version: " + remoteName + "\n"; + msg += "\n"; + if (localDelete) { + msg += "The remote version has been kept."; + } + else { + var moreRecent = remoteMoreRecent ? remoteName : localName; + msg += "The most recent version, '" + moreRecent + "', has been kept."; + } + return msg; + } + + + /** + * @param {String} collectionName + * @param {Integer[]} addedItemIDs + * @param {Boolean} remoteIsTarget + */ + function _generateCollectionItemMergeMessage(collectionName, addedItemIDs, remoteIsTarget) { + // TODO: localize + var introMsg = "Items in the collection '" + collectionName + "' have been " + + "added and/or removed in multiple locations." + + introMsg += " "; + if (remoteIsTarget) { + introMsg += "The following items have been added to the remote collection:"; + } + else { + introMsg += "The following items have been added to the local collection:"; + } + var itemText = []; + for each(var id in addedItemIDs) { + var item = Zotero.Items.get(id); + var title = item.getField('title'); + var text = " - " + title; + var firstCreator = item.getField('firstCreator'); + if (firstCreator) { + text += " (" + firstCreator + ")"; + } + itemText.push(text); + } + return introMsg + "\n\n" + itemText.join("\n"); + } + + + /** + * @param {String} tagName + * @param {Integer[]} addedItemIDs + * @param {Boolean} remoteIsTarget + */ + function _generateTagItemMergeMessage(tagName, addedItemIDs, remoteIsTarget) { + // TODO: localize + var introMsg = "The tag '" + tagName + "' has been " + + "added to and/or removed from items in multiple locations." + + introMsg += " "; + if (remoteIsTarget) { + introMsg += "It has been added to the following remote items:"; + } + else { + introMsg += "It has been added to the following local items:"; + } + var itemText = []; + for each(var id in addedItemIDs) { + var item = Zotero.Items.get(id); + var title = item.getField('title'); + var text = " - " + title; + var firstCreator = item.getField('firstCreator'); + if (firstCreator) { + text += " (" + firstCreator + ")"; + } + itemText.push(text); + } + return introMsg + "\n\n" + itemText.join("\n"); + } + + /** * Open a conflict resolution window and return the results * @@ -2223,7 +2381,7 @@ Zotero.Sync.Server.Data = new function() { delete relatedItems[obj.id]; } - syncSession.addToDeleted(type, obj.id, obj.left.key); + syncSession.addToDeleted(type, obj.left.key); } continue; } @@ -2236,7 +2394,7 @@ Zotero.Sync.Server.Data = new function() { // Item had been deleted locally, so remove from // deleted array if (obj.left == 'deleted') { - syncSession.removeFromDeleted(type, obj.id, obj.ref.key); + syncSession.removeFromDeleted(type, obj.ref.key); } // TODO: only upload if the local item was chosen @@ -2583,26 +2741,6 @@ Zotero.Sync.Server.Data = new function() { } - function _logCollectionItemMerge(collectionName, remoteItemIDs) { - // TODO: localize - var introMsg = "Items in the collection '" + collectionName + "' have been " - + "added and/or removed in multiple locations. The following remote " - + "items have been added to the local collection:"; - var itemText = []; - for each(var id in remoteItemIDs) { - var item = Zotero.Items.get(id); - var title = item.getField('title'); - var text = " - " + title; - var firstCreator = item.getField('firstCreator'); - if (firstCreator) { - text += " (" + firstCreator + ")"; - } - itemText.push(text); - } - return introMsg + "\n\n" + itemText.join("\n"); - } - - /** * Converts a Zotero.Creator object to an E4X object */ @@ -2871,16 +3009,16 @@ Zotero.Sync.Server.Data = new function() { function _deleteConflictingTag(syncSession, name, type) { var tagID = Zotero.Tags.getID(name, type); if (tagID) { + Zotero.debug("Deleting conflicting local tag " + tagID); var tag = Zotero.Tags.get(tagID); var linkedItems = tag.getLinkedItems(true); Zotero.Tags.erase(tagID); - // DEBUG: should purge() be called by Tags.erase() Zotero.Tags.purge(); syncSession.removeFromUpdated('tag', tagID); - syncSession.addToDeleted('tag', tagID, tag.key); + //syncSession.addToDeleted('tag', tag.key); - return linkedItems; + return linkedItems ? linkedItems : []; } return false; diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js index 8db8e26be4..13423eb9df 100644 --- a/chrome/content/zotero/xpcom/utilities.js +++ b/chrome/content/zotero/xpcom/utilities.js @@ -292,27 +292,33 @@ Zotero.Utilities.prototype.isInt = function(x) { /** - * Compares an array with another (comparator) and returns an array with - * the values from comparator that don't exist in vector + * Compares an array with another and returns an array with + * the values from array2 that don't exist in array1 * - * 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 + * @param {Array} array1 Array that will be checked + * @param {Array} array2 Array that will be compared + * @param {Boolean} useIndex If true, return an array containing just * the index of the comparator's elements; - * otherwise returns the values + * otherwise return 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; -}; - +Zotero.Utilities.prototype.arrayDiff = function(array1, array2, useIndex) { + if (array1.constructor.name != 'Array') { + throw ("array1 is not an array in Zotero.Utilities.arrayDiff() (" + array1 + ")"); + } + if (array2.constructor.name != 'Array') { + throw ("array2 is not an array in Zotero.Utilities.arrayDiff() (" + array2 + ")"); + } + + var val, pos, vals = []; + for (var i=0; i