From 6456b6f56143cc5091065352875421c5e9bcfdd1 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 30 Nov 2008 19:30:15 +0000 Subject: [PATCH] Adds partial collection conflict resolution support -- items are auto-merged, and an alert is displayed. Support for collection metadata and collection hierarchy conflicts is forthcoming, and the alert will be fed to a new notifications system. --- chrome/content/zotero/merge.js | 45 ++++--- .../content/zotero/xpcom/data/collection.js | 26 +++- .../content/zotero/xpcom/data/collections.js | 4 +- chrome/content/zotero/xpcom/sync.js | 127 ++++++++++++++++-- 4 files changed, 171 insertions(+), 31 deletions(-) diff --git a/chrome/content/zotero/merge.js b/chrome/content/zotero/merge.js index d1cac03a24..a7a1b2f194 100644 --- a/chrome/content/zotero/merge.js +++ b/chrome/content/zotero/merge.js @@ -38,19 +38,25 @@ var Zotero_Merge_Window = new function () { return; } - var firstObj = _objects[0][0] == 'deleted' ? _objects[0][1] : _objects[0][0]; + _mergeGroup.type = _io.dataIn.type; - if (firstObj instanceof Zotero.Item) { - if (firstObj.isNote()) { - _mergeGroup.type = 'note'; - } - else { - _mergeGroup.type = 'item'; - } - } - else { - throw ("Invalid merge object type '" + firstObj.constructor.name - + "' in Zotero_Merge_Window.init()"); + switch (_mergeGroup.type) { + case 'item': + var firstObj = _objects[0][0] == 'deleted' ? _objects[0][1] : _objects[0][0]; + if (firstObj.isNote()) { + _mergeGroup.type = 'note'; + } + else { + _mergeGroup.type = 'item'; + } + break; + + case 'collection': + break; + + default: + throw ("Unsupported merge object type '" + type + + "' in Zotero_Merge_Window.init()"); } _mergeGroup.leftCaption = _io.dataIn.captions[0]; @@ -83,7 +89,9 @@ var Zotero_Merge_Window = new function () { _mergeGroup.leftpane.removeAttribute("selected"); _mergeGroup.rightpane.removeAttribute("selected"); - _updateChangedCreators(); + if (_mergeGroup.type == 'item') { + _updateChangedCreators(); + } if (Zotero.isMac) { _wizard.getButton("next").setAttribute("hidden", "false"); @@ -127,7 +135,9 @@ var Zotero_Merge_Window = new function () { _mergeGroup.rightpane.removeAttribute("selected"); } - _updateChangedCreators(); + if (_mergeGroup.type == 'item') { + _updateChangedCreators(); + } // On Windows the buttons don't move when one is hidden if ((_pos + 1) != _objects.length) { @@ -190,7 +200,12 @@ var Zotero_Merge_Window = new function () { // Hack to support creator reconciliation via item view function _updateChangedCreators() { - if (_mergeGroup.type == 'item' && _io.dataIn.changedCreators) { + if (_mergeGroup.type != 'item') { + throw ("_updateChangedCreators called on non-item object in " + + "Zotero_Merge_Window._updateChangedCreators()"); + } + + if (_io.dataIn.changedCreators) { var originalCreators = _mergeGroup.rightpane.original.getCreators(); var clonedCreators = _mergeGroup.rightpane.ref.getCreators(); var refresh = false; diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 019be208c0..4194d325f0 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -671,18 +671,38 @@ Zotero.Collection.prototype.diff = function (collection, includeMatches, ignoreO // For the moment, just compare children and increase numDiffs if any differences var d1 = Zotero.Utilities.prototype.arrayDiff( - thisData.childCollections, otherData.childCollections + otherData.childCollections, thisData.childCollections ); var d2 = Zotero.Utilities.prototype.arrayDiff( thisData.childCollections, otherData.childCollections ); var d3 = Zotero.Utilities.prototype.arrayDiff( - thisData.childItems, otherData.childItems + otherData.childItems, thisData.childItems ); var d4 = Zotero.Utilities.prototype.arrayDiff( thisData.childItems, otherData.childItems ); - numDiffs += d1.length + d2.length + d3.length + d4.length; + numDiffs += d1.length + d2.length; + + if (d1.length || d2.length) { + numDiffs += d1.length + d2.length; + diff[0].childCollections = d1; + diff[1].childCollections = d2; + } + else { + diff[0].childCollections = []; + diff[1].childCollections = []; + } + + if (d3.length || d4.length) { + numDiffs += d3.length + d4.length; + diff[0].childItems = d3; + diff[1].childItems = d4; + } + else { + diff[0].childItems = []; + diff[1].childItems = []; + } // DEBUG: ignoreOnlyDateModified wouldn't work if includeMatches was set? if (numDiffs == 0 || diff --git a/chrome/content/zotero/xpcom/data/collections.js b/chrome/content/zotero/xpcom/data/collections.js index c7f92f51e6..3c5acd4d02 100644 --- a/chrome/content/zotero/xpcom/data/collections.js +++ b/chrome/content/zotero/xpcom/data/collections.js @@ -156,13 +156,13 @@ Zotero.Collections = new function() { var id = row.collectionID; ids.push(id); - // Creator doesn't exist -- create new object and stuff in array + // Collection doesn't exist -- create new object and stuff in array if (!this._objectCache[id]) { //this.get(id); this._objectCache[id] = new Zotero.Collection; this._objectCache[id].loadFromRow(row); } - // Existing creator -- reload in place + // Existing collection -- reload in place else { this._objectCache[id].loadFromRow(row); } diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js index 24ea97bbf2..eb12c48147 100644 --- a/chrome/content/zotero/xpcom/sync.js +++ b/chrome/content/zotero/xpcom/sync.js @@ -1519,6 +1519,7 @@ Zotero.Sync.Server.Data = new function() { typeloop: for each(var xmlNode in xml[types][type]) { + Zotero.debug("Processing remote " + type + " " + xmlNode.@id, 4); var isNewObject; var localDelete = false; @@ -1527,6 +1528,7 @@ Zotero.Sync.Server.Data = new function() { if (obj) { // Key match -- same item if (obj.key == xmlNode.@key.toString()) { + Zotero.debug("Matching local " + type + " exists", 4); isNewObject = false; var objDate = Zotero.Date.sqlToDate(obj.dateModified, true); @@ -1541,6 +1543,9 @@ Zotero.Sync.Server.Data = new function() { // linked to a creator whose id changed) || syncSession.uploadIDs.updated[types].indexOf(obj.id) != -1) { + Zotero.debug("Local " + type + " " + obj.id + + " has been modified since last sync", 4); + // Merge and store related items, since CR doesn't // affect related items if (type == 'item') { @@ -1563,6 +1568,7 @@ Zotero.Sync.Server.Data = new function() { // Some types we don't bother to reconcile if (_noMergeTypes.indexOf(type) != -1) { + // If local is newer, send to server if (obj.dateModified > remoteObj.dateModified) { syncSession.addToUpdated(type, obj.id); continue; @@ -1572,6 +1578,8 @@ Zotero.Sync.Server.Data = new function() { } // Mark other types for conflict resolution else { + var reconcile = false; + // Skip item if dateModified is the only modified // field (and no linked creators changed) switch (type) { @@ -1609,9 +1617,87 @@ Zotero.Sync.Server.Data = new function() { continue; } } + if (obj.isAttachment()) { + var msg = "Reconciliation unimplemented for attachment items"; + alert(msg); + throw(msg); + } + reconcile = true; break; case 'collection': + var diff = obj.diff(remoteObj, false, true); + if (diff) { + var fieldsChanged = false; + for (var field in diff[0].primary) { + if (field != 'dateModified') { + fieldsChanged = true; + break; + } + } + for (var field in diff[0].fields) { + fieldsChanged = true; + break; + } + + if (fieldsChanged) { + // Check for collection hierarchy change + if (diff[0].childCollections.length) { + // TODO + } + if (diff[1].childCollections.length) { + // TODO + } + // Check for item membership change + if (diff[0].childItems.length) { + var childItems = remoteObj.getChildItems(true); + remoteObj.childItems = childItems.concat(diff[0].childItems); + } + if (diff[1].childItems.length) { + var childItems = obj.getChildItems(true); + obj.childItems = childItems.concat(diff[1].childItems); + } + + // TODO: log + + // TEMP: uncomment once supported + //reconcile = true; + } + // No CR necessary + else { + var save = false; + // Check for child collections in the remote object + // that aren't in the local one + if (diff[1].childCollections.length) { + // TODO: log + // TODO: add + save = true; + } + // Check for items in the remote object + // that aren't in the local one + if (diff[1].childItems.length) { + var childItems = obj.getChildItems(true); + obj.childItems = childItems.concat(diff[1].childItems); + + var msg = _logCollectionItemMerge(obj.name, diff[1].childItems); + // TODO: log rather than alert + alert(msg); + + save = true; + } + + if (save) { + obj.save(); + } + continue; + } + } + else { + syncSession.removeFromUpdated(type, obj.id); + continue; + } + break; + case 'tag': var diff = obj.diff(remoteObj, false, true); if (!diff) { @@ -1621,14 +1707,7 @@ Zotero.Sync.Server.Data = new function() { break; } - if (type == 'item') { - if (obj.isAttachment()) { - var msg = "Reconciliation unimplemented for attachment items"; - alert(msg); - throw(msg); - } - } - else { + if (!reconcile) { Zotero.debug(obj); Zotero.debug(remoteObj); var msg = "Reconciliation unimplemented for " + types; @@ -1646,6 +1725,9 @@ Zotero.Sync.Server.Data = new function() { continue; } } + else { + Zotero.debug("Local " + type + " has not changed", 4); + } // Overwrite local below } @@ -1653,6 +1735,8 @@ Zotero.Sync.Server.Data = new function() { // Key mismatch -- different objects with same id, // so change id of local object else { + Zotero.debug("Local " + type + " " + xmlNode.@id + " differs from remote", 4); + isNewObject = true; var oldID = parseInt(xmlNode.@id); @@ -2061,11 +2145,12 @@ Zotero.Sync.Server.Data = new function() { function _reconcile(type, objectPairs, changedCreators) { var io = { dataIn: { + type: type, captions: [ // TODO: localize - 'Local Item', - 'Remote Item', - 'Merged Item' + 'Local Object', + 'Remote Object', + 'Merged Object' ], objects: objectPairs } @@ -2463,6 +2548,26 @@ 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 */