From 5a25fa3e0c506e9ba2cc1e8f3ba8a53c4899c1f8 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 30 Jan 2007 10:21:01 +0000 Subject: [PATCH] Updated Notifier trigger architecture and data layer to send a pre-modification copy from toArray() of items/collections/tags as part of 'modify'/'delete' notifications Sent as a fourth parameter to notify() -- parameter is an array of objects (in the same order as the ids) that currently contain a single property, 'old', which holds the toArray() object Copies are not sent with 'modify' when it's only meant to refresh the UI and there's another trigger that covers the data change (e.g. removing a tag from an item sends both an item modify and an item-tag add, but the modify doesn't get a pre-change copy of the item since any consumers that care should just monitor item-tag) Also: - Removed Notifier.enable()/disable() - Notifier no longer sends modify() if item already deleted - New methods: Collection.toArray(), Zotero.Tags.toArray(tagID) - Removed a few extraneous triggers --- chrome/content/zotero/xpcom/data_access.js | 183 +++++++++++++++------ chrome/content/zotero/xpcom/notifier.js | 98 ++++++----- 2 files changed, 189 insertions(+), 92 deletions(-) diff --git a/chrome/content/zotero/xpcom/data_access.js b/chrome/content/zotero/xpcom/data_access.js index 711560343..603a926df 100644 --- a/chrome/content/zotero/xpcom/data_access.js +++ b/chrome/content/zotero/xpcom/data_access.js @@ -509,6 +509,7 @@ Zotero.Item.prototype.save = function(){ Zotero.debug('Updating database with new item data', 4); var itemID = this.getID(); + var preItemArray = this.toArray(); try { Zotero.DB.beginTransaction(); @@ -899,7 +900,7 @@ Zotero.Item.prototype.save = function(){ } else { if (!Zotero.DB.transactionInProgress()){ - Zotero.Notifier.trigger('modify', 'item', this.getID()); + Zotero.Notifier.trigger('modify', 'item', this.getID(), { old: preItemArray }); } return true; } @@ -966,6 +967,8 @@ Zotero.Item.prototype.updateNote = function(text){ Zotero.DB.beginTransaction(); + var preItemArray = this.toArray(); + if (this.isNote()){ var sourceItemID = this.getSource(); } @@ -988,7 +991,7 @@ Zotero.Item.prototype.updateNote = function(text){ Zotero.DB.commitTransaction(); this.updateNoteCache(text, this.isAbstract()); - Zotero.Notifier.trigger('modify', 'item', this.getID()); + Zotero.Notifier.trigger('modify', 'item', this.getID(), { old: preItemArray }); } else { Zotero.DB.commitTransaction(); @@ -1025,9 +1028,14 @@ Zotero.Item.prototype.setSource = function(sourceItemID){ Zotero.DB.beginTransaction(); + var preItemArray = this.toArray(); + var newItem = Zotero.Items.get(sourceItemID); // FK check - if (sourceItemID && !newItem){ + if (sourceItemID && newItem) { + var preNewItemArray = newItem.toArray(); + } + else { Zotero.DB.rollbackTransaction(); throw ("Cannot set " + type + " source to invalid item " + sourceItemID); } @@ -1041,9 +1049,12 @@ Zotero.Item.prototype.setSource = function(sourceItemID){ } var oldItem = Zotero.Items.get(oldSourceItemID); - if (oldSourceItemID && !oldItem){ - Zotero.debug("Old source item " + oldSourceItemID - + "didn't exist in setSource()", 2); + if (oldSourceItemID && oldItem) { + var preOldItemArray = oldItem.toArray(); + } + else { + var preOldItemArray = false; + Zotero.debug("Old source item " + oldSourceItemID + "didn't exist in setSource()", 2); } // If this was an independent item, remove from any collections where it @@ -1078,7 +1089,7 @@ Zotero.Item.prototype.setSource = function(sourceItemID){ this.updateDateModified(); Zotero.DB.commitTransaction(); - Zotero.Notifier.trigger('modify', 'item', this.getID()); + Zotero.Notifier.trigger('modify', 'item', this.getID(), { old: preItemArray }); // Update the counts of the previous and new sources if (oldItem){ @@ -1090,8 +1101,9 @@ Zotero.Item.prototype.setSource = function(sourceItemID){ oldItem.decrementAttachmentCount(); break; } - Zotero.Notifier.trigger('modify', 'item', oldSourceItemID); + Zotero.Notifier.trigger('modify', 'item', oldSourceItemID, { old: preOldItemArray }); } + if (newItem){ switch (type){ case 'note': @@ -1101,7 +1113,7 @@ Zotero.Item.prototype.setSource = function(sourceItemID){ newItem.incrementAttachmentCount(); break; } - Zotero.Notifier.trigger('modify', 'item', sourceItemID); + Zotero.Notifier.trigger('modify', 'item', sourceItemID, { old: preNewItemArray }); } return true; @@ -1234,9 +1246,14 @@ Zotero.Item.prototype.setAbstract = function(set) { throw ("Cannot make a non-child note an abstract"); } + var notifierData = [{ old: this.toArray() }]; + + var sourceItem = Zotero.Items.get(sourceItemID); + notifierData.push({ old: sourceItem.toArray() }); + + // If existing abstract, clear it if (set) { - // If existing abstract, clear it - var oldAbstractID = Zotero.Items.get(sourceItemID).getAbstract(); + var oldAbstractID = sourceItem.getAbstract(); if (oldAbstractID) { var oldAbstractItem = Zotero.Items.get(oldAbstractID); oldAbstractItem.setAbstract(false); @@ -1256,7 +1273,7 @@ Zotero.Item.prototype.setAbstract = function(set) { this._noteIsAbstract = !!set; - Zotero.Notifier.trigger('modify', 'item', [this.getID(), sourceItemID]); + Zotero.Notifier.trigger('modify', 'item', [this.getID(), sourceItemID], notifierData); } @@ -1733,17 +1750,20 @@ Zotero.Item.prototype.removeAllTags = function(){ Zotero.DB.beginTransaction(); var tagIDs = this.getTagIDs(); + if (!tagIDs) { + Zotero.DB.commitTransaction(); + return; + } + Zotero.DB.query("DELETE FROM itemTags WHERE itemID=?", this.getID()); Zotero.Tags.purge(); Zotero.DB.commitTransaction(); Zotero.Notifier.trigger('modify', 'item', this.getID()); - if (tagIDs) { - for (var i in tagIDs) { - tagIDs[i] = this.getID() + '-' + tagIDs[i]; - } - Zotero.Notifier.trigger('remove', 'item-tag', tagIDs); + for (var i in tagIDs) { + tagIDs[i] = this.getID() + '-' + tagIDs[i]; } + Zotero.Notifier.trigger('remove', 'item-tag', tagIDs); } @@ -1764,7 +1784,9 @@ Zotero.Item.prototype.addSeeAlso = function(itemID){ Zotero.DB.beginTransaction(); - if (!Zotero.Items.get(itemID)){ + var relatedItem = Zotero.Items.get(itemID); + + if (!relatedItem){ Zotero.DB.commitTransaction(); throw ("Cannot add invalid item " + itemID + " as See Also"); return false; @@ -1780,10 +1802,15 @@ Zotero.Item.prototype.addSeeAlso = function(itemID){ return false; } + var notifierData = [ + { old: this.toArray() }, + { old: relatedItem.toArray() } + ]; + var sql = "INSERT INTO itemSeeAlso VALUES (?,?)"; Zotero.DB.query(sql, [this.getID(), {int:itemID}]); Zotero.DB.commitTransaction(); - Zotero.Notifier.trigger('modify', 'item', [this.getID(), itemID]); + Zotero.Notifier.trigger('modify', 'item', [this.getID(), itemID], notifierData); return true; } @@ -1793,12 +1820,25 @@ Zotero.Item.prototype.removeSeeAlso = function(itemID){ } Zotero.DB.beginTransaction(); + + var relatedItem = Zotero.Items.get(itemID); + if (!relatedItem) { + Zotero.DB.commitTransaction(); + throw ("Cannot remove invalid item " + itemID + " as See Also"); + return false; + } + + var notifierData = [ + { old: this.toArray() }, + { old: relatedItem.toArray() } + ]; + var sql = "DELETE FROM itemSeeAlso WHERE itemID=? AND linkedItemID=?"; Zotero.DB.query(sql, [this.getID(), itemID]); var sql = "DELETE FROM itemSeeAlso WHERE itemID=? AND linkedItemID=?"; Zotero.DB.query(sql, [itemID, this.getID()]); Zotero.DB.commitTransaction(); - Zotero.Notifier.trigger('modify', 'item', [this.getID(), itemID]); + Zotero.Notifier.trigger('modify', 'item', [this.getID(), itemID], notifierData); } Zotero.Item.prototype.removeAllRelated = function() { @@ -1808,13 +1848,24 @@ Zotero.Item.prototype.removeAllRelated = function() { Zotero.DB.beginTransaction(); var relateds = this.getSeeAlso(); + if (!relateds) { + Zotero.DB.commitTransaction(); + return; + } + + var notifierData = [ { old: this.toArray() } ]; + for each(var id in relateds) { + var item = Zotero.Items.get(id); + notifierData.push(item ? { old: item.toArray() } : false); + } + Zotero.DB.query("DELETE FROM itemSeeAlso WHERE itemID=?", this.getID()); Zotero.DB.query("DELETE FROM itemSeeAlso WHERE linkedItemID=?", this.getID()); Zotero.DB.commitTransaction(); - if (relateds) { - Zotero.Notifier.trigger('modify', 'item', relateds); - } + var ids = [this.getID()].concat(relateds); + + Zotero.Notifier.trigger('modify', 'item', ids, notifierData); } Zotero.Item.prototype.getSeeAlso = function(){ @@ -1865,6 +1916,10 @@ Zotero.Item.prototype.erase = function(deleteChildren){ Zotero.DB.beginTransaction(); + // For Notifier + var preItemArray = this.toArray(); + var notifierData = []; + // Remove item from parent collections var parentCollectionIDs = this.getCollections(); if (parentCollectionIDs){ @@ -1880,6 +1935,7 @@ Zotero.Item.prototype.erase = function(deleteChildren){ var sourceItemID = Zotero.DB.valueQuery(sql); if (sourceItemID){ var sourceItem = Zotero.Items.get(sourceItemID); + notifierData.push({ old: sourceItem.toArray() }); sourceItem.decrementNoteCount(); changedItems.push(sourceItemID); } @@ -1891,6 +1947,7 @@ Zotero.Item.prototype.erase = function(deleteChildren){ var sourceItemID = Zotero.DB.valueQuery(sql); if (sourceItemID){ var sourceItem = Zotero.Items.get(sourceItemID); + notifierData.push({ old: sourceItem.toArray() }); sourceItem.decrementAttachmentCount(); changedItems.push(sourceItemID); } @@ -1933,6 +1990,10 @@ Zotero.Item.prototype.erase = function(deleteChildren){ var sql = "SELECT itemID FROM itemNotes WHERE sourceItemID=" + this.getID(); var childNotes = Zotero.DB.columnQuery(sql); if (childNotes){ + for each(var id in childNotes) { + var i = Zotero.Items.get(id); + notifierData.push({ old: i.toArray() }); + } changedItems.push(childNotes); } var sql = "UPDATE itemNotes SET sourceItemID=NULL WHERE sourceItemID=" @@ -1943,6 +2004,10 @@ Zotero.Item.prototype.erase = function(deleteChildren){ var sql = "SELECT itemID FROM itemAttachments WHERE sourceItemID=" + this.getID(); var childAttachments = Zotero.DB.columnQuery(sql); if (childAttachments){ + for each(var id in childAttachments) { + var i = Zotero.Items.get(id); + notifierData.push({ old: i.toArray() }); + } changedItems.push(childAttachments); } var sql = "UPDATE itemAttachments SET sourceItemID=NULL WHERE sourceItemID=" @@ -1951,9 +2016,13 @@ Zotero.Item.prototype.erase = function(deleteChildren){ } // Flag See Also links for notification - var seeAlso = this.getSeeAlso(); - if (seeAlso){ - changedItems = changedItems.concat(seeAlso); + var relateds = this.getSeeAlso(); + if (relateds){ + for each(var id in relateds) { + var i = Zotero.Items.get(id); + notifierData.push({ old: i.toArray() }); + } + changedItems = changedItems.concat(relateds); } // Clear fulltext cache @@ -1995,11 +2064,10 @@ Zotero.Item.prototype.erase = function(deleteChildren){ // Send notification of changed items if (changedItems.length){ - Zotero.Notifier.trigger('modify', 'item', changedItems); + Zotero.Notifier.trigger('modify', 'item', changedItems, notifierData); } - // If we're not in the middle of a larger commit, trigger the notifier now - Zotero.Notifier.trigger('delete', 'item', this.getID()); + Zotero.Notifier.trigger('delete', 'item', this.getID(), { old: preItemArray }); } @@ -2363,7 +2431,7 @@ Zotero.Items = new function(){ var item = this.get(id); if (!item) { Zotero.debug('Item ' + id + ' does not exist in Items.erase()!', 1); - Zotero.Notifier.trigger('delete', 'item', id); + Zotero.Notifier.trigger('delete', 'item', id, [false]); continue; } item.erase(eraseChildren); // calls unload() @@ -2475,8 +2543,9 @@ Zotero.Notes = new function(){ note.updateNoteCache(text, isAbstract); if (sourceItemID){ + var notifierData = { old: sourceItem.toArray() }; sourceItem.incrementNoteCount(); - Zotero.Notifier.trigger('modify', 'item', sourceItemID); + Zotero.Notifier.trigger('modify', 'item', sourceItemID, notifierData); } Zotero.Notifier.trigger('add', 'item', note.getID()); @@ -2582,12 +2651,14 @@ Zotero.Collection.prototype.rename = function(name){ return false; } + var notifierData = { old: this.toArray() }; + var sql = "UPDATE collections SET collectionName=? " + "WHERE collectionID=?"; Zotero.DB.query(sql, [{'string':name},{'int':this.getID()}]); this._name = name; - Zotero.Notifier.trigger('modify', 'collection', this.getID()); + Zotero.Notifier.trigger('modify', 'collection', this.getID(), notifierData); return true; } @@ -2627,6 +2698,8 @@ Zotero.Collection.prototype.changeParent = function(parent){ } } + var notifierData = { old: this.toArray() }; + var parentParam = parent ? {'int':parent} : {'null':true}; var sql = "UPDATE collections SET parentCollectionID=? " @@ -2643,7 +2716,7 @@ Zotero.Collection.prototype.changeParent = function(parent){ // TODO: only reload the necessary ones Zotero.Collections.reloadAll(); - Zotero.Notifier.trigger('move', 'collection', notifyIDs); + Zotero.Notifier.trigger('move', 'collection', notifyIDs, notifierData); return true; } @@ -2673,8 +2746,6 @@ Zotero.Collection.prototype.addItem = function(itemID){ // If this was previously empty, update and send a notification to the tree if (!this._hasChildItems){ this._hasChildItems = true; - // DEBUG: is this necessary? - Zotero.Notifier.trigger('modify', 'collection', this.getID()); } Zotero.Notifier.trigger('add', 'collection-item', this.getID() + '-' + itemID); @@ -2714,8 +2785,6 @@ Zotero.Collection.prototype.removeItem = function(itemID){ // If this was the last item, set collection to empty if (!this._childItems.length){ this._hasChildItems = false; - // DEBUG: is this necessary? if so, it's no longer called during item deletes... - Zotero.Notifier.trigger('modify', 'collection', this.getID()); } Zotero.Notifier.trigger('remove', 'collection-item', this.getID() + '-' + itemID); @@ -2752,18 +2821,20 @@ Zotero.Collection.prototype.erase = function(deleteItems){ var descendents = this.getDescendents(); var collections = [this.getID()], items = []; + var notifierData = [{ old: this.toArray() }]; for(var i=0, len=descendents.length; i