diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 91e356a94..06dc5f389 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -82,40 +82,6 @@ Zotero.defineProperty(Zotero.Collection.prototype, 'parent', { } }); -Zotero.Collection.prototype._set = function (field, value) { - if (field == 'id' || field == 'libraryID' || field == 'key') { - return this._setIdentifier(field, value); - } - - this._requireData('primaryData'); - - switch (field) { - case 'name': - value = value.trim().normalize(); - break; - - case 'version': - value = parseInt(value); - break; - - case 'synced': - value = !!value; - break; - } - - if (this['_' + field] != value) { - this._markFieldChange(field, this['_' + field]); - if (!this._changed.primaryData) { - this._changed.primaryData = {}; - } - this._changed.primaryData[field] = true; - - switch (field) { - default: - this['_' + field] = value; - } - } -} Zotero.Collection.prototype.getID = function() { Zotero.debug('Collection.getID() deprecated -- use Collection.id'); @@ -340,15 +306,14 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env) }); Zotero.Collection.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) { - var isNew = env.isNew; - if (isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) { + if (env.isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) { var groupID = Zotero.Groups.getGroupIDFromLibraryID(this.libraryID); var group = Zotero.Groups.get(groupID); group.clearCollectionCache(); } if (!env.options.skipNotifier) { - if (isNew) { + if (env.isNew) { Zotero.Notifier.trigger('add', 'collection', this.id, env.notifierData); } else { @@ -361,17 +326,12 @@ Zotero.Collection.prototype._finalizeSave = Zotero.Promise.coroutine(function* ( this.ObjectsClass.refreshChildCollections(env.parentIDs); } - // New collections have to be reloaded via Zotero.Collections.get(), so mark them as disabled - if (isNew) { - var id = this.id; - this._disabled = true; - return id; + if (!env.skipCache) { + yield this.reload(); + this._clearChanged(); } - yield this.reload(); - this._clearChanged(); - - return true; + return env.isNew ? this.id : true; }); diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index 5cda6d362..3c0b88980 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -50,6 +50,9 @@ Zotero.DataObject = function () { this._parentID = null; this._parentKey = null; + // Set in dataObjects.js + this._inCache = false; + this._loaded = {}; this._skipDataTypeLoad = {}; this._markAllDataTypeLoadStates(false); @@ -90,6 +93,8 @@ Zotero.defineProperty(Zotero.DataObject.prototype, 'ObjectsClass', { Zotero.DataObject.prototype._get = function (field) { + if (field != 'id') this._disabledCheck(); + if (this['_' + field] !== null) { return this['_' + field]; } @@ -98,6 +103,44 @@ Zotero.DataObject.prototype._get = function (field) { } +Zotero.DataObject.prototype._set = function (field, value) { + this._disabledCheck(); + + if (field == 'id' || field == 'libraryID' || field == 'key') { + return this._setIdentifier(field, value); + } + + this._requireData('primaryData'); + + switch (field) { + case 'name': + value = value.trim().normalize(); + break; + + case 'version': + value = parseInt(value); + break; + + case 'synced': + value = !!value; + break; + } + + if (this['_' + field] != value) { + this._markFieldChange(field, this['_' + field]); + if (!this._changed.primaryData) { + this._changed.primaryData = {}; + } + this._changed.primaryData[field] = true; + + switch (field) { + default: + this['_' + field] = value; + } + } +} + + Zotero.DataObject.prototype._setIdentifier = function (field, value) { // If primary data is loaded, the only allowed identifier change is libraryID // (allowed mainly for the library switcher in the advanced search window), @@ -544,6 +587,14 @@ Zotero.DataObject.prototype.editCheck = function () { /** * Save changes to database * + * @params {Object} [options] + * @params {Boolean} [options.skipCache] - Don't save add new object to the cache; if set, object + * is disabled after save + * @params {Boolean} [options.skipDateModifiedUpdate] + * @params {Boolean} [options.skipClientDateModifiedUpdate] + * @params {Boolean} [options.skipNotifier] - Don't trigger Zotero.Notifier events + * @params {Boolean} [options.skipSelect] - Don't select object automatically in trees + * @params {Boolean} [options.skipSyncedUpdate] - Don't automatically set 'synced' to false * @return {Promise} Promise for itemID of new item, * TRUE on item update, or FALSE if item was unchanged */ @@ -650,7 +701,7 @@ Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env) return false; } - // Undo registerIdentifiers() on failure + // Undo registerObject() on failure var func = function () { this.ObjectsClass.unload(env.id); }.bind(this); @@ -699,9 +750,18 @@ Zotero.DataObject.prototype._saveData = function (env) { }; Zotero.DataObject.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) { - // Register this object's identifiers in Zotero.DataObjects if (env.isNew) { - this.ObjectsClass.registerIdentifiers(env.id, env.libraryID, env.key); + if (!env.skipCache) { + // Register this object's identifiers in Zotero.DataObjects + this.ObjectsClass.registerObject(this); + } + // If object isn't being reloaded, disable it, since its data may be out of date + else { + this._disabled = true; + } + } + else if (env.skipCache) { + Zotero.logError("skipCache is only for new objects"); } }); @@ -778,3 +838,10 @@ Zotero.DataObject.prototype._erasePostCommit = function(env) { Zotero.DataObject.prototype._generateKey = function () { return Zotero.Utilities.generateObjectKey(); } + +Zotero.DataObject.prototype._disabledCheck = function () { + if (this._disabled) { + Zotero.logError(this._ObjectType + " is disabled -- " + + "use Zotero." + this._ObjectTypePlural + ".getAsync()"); + } +} diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index 29b98c53c..1a04bb407 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -395,13 +395,19 @@ Zotero.DataObjects.prototype.reloadAll = function (libraryID) { } -Zotero.DataObjects.prototype.registerIdentifiers = function (id, libraryID, key) { +Zotero.DataObjects.prototype.registerObject = function (obj) { + var id = obj.id; + var libraryID = obj.libraryID; + var key = obj.key; + Zotero.debug("Registering " + this._ZDO_object + " " + id + " as " + libraryID + "/" + key); if (!this._objectIDs[libraryID]) { this._objectIDs[libraryID] = {}; } this._objectIDs[libraryID][key] = id; this._objectKeys[id] = [libraryID, key]; + this._objectCache[id] = obj; + obj._inCache = true; } @@ -521,6 +527,7 @@ Zotero.DataObjects.prototype._load = Zotero.Promise.coroutine(function* (library obj.loadFromRow(rowObj, true); if (!options || !options.noCache) { this._objectCache[id] = obj; + obj._inCache = true; } } loaded[id] = obj; diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 2f47c6621..243580a8f 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -178,10 +178,14 @@ Zotero.Item.prototype.isPrimaryField = function (fieldName) { return this.ObjectsClass.isPrimaryField(fieldName); } -Zotero.Item.prototype._get = function (fieldName) { +Zotero.Item.prototype._get = function () { throw new Error("_get is not valid for items"); } +Zotero.Item.prototype._set = function () { + throw new Error("_set is not valid for items"); +} + Zotero.Item.prototype._setParentKey = function() { if (!this.isNote() && !this.isAttachment()) { throw new Error("_setParentKey() can only be called on items of type 'note' or 'attachment'"); @@ -207,11 +211,7 @@ Zotero.Item.prototype._setParentKey = function() { * @return {String} Value as string or empty string if value is not present */ Zotero.Item.prototype.getField = function(field, unformatted, includeBaseMapped) { - // We don't allow access after saving to force use of the centrally cached - // object, but we make an exception for the id - if (field != 'id') { - this._disabledCheck(); - } + if (field != 'id') this._disabledCheck(); //Zotero.debug('Requesting field ' + field + ' for item ' + this._id, 4); @@ -694,12 +694,12 @@ Zotero.Item.prototype.getFieldsNotInType = function (itemTypeID, allowBaseConver * Field can be passed as fieldID or fieldName */ Zotero.Item.prototype.setField = function(field, value, loadIn) { + this._disabledCheck(); + if (typeof value == 'string') { value = value.trim().normalize(); } - this._disabledCheck(); - //Zotero.debug("Setting field '" + field + "' to '" + value + "' (loadIn: " + (loadIn ? 'true' : 'false') + ") for item " + this.id + " "); if (!field) { @@ -1730,21 +1730,16 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { }); Zotero.Item.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) { - // New items have to be reloaded via Zotero.Items.get(), so mark them as disabled - if (env.isNew) { - var id = this.id; - this._disabled = true; - return id; + if (!env.skipCache) { + // Always reload primary data. DataObject.reload() only reloads changed data types, so + // it won't reload, say, dateModified and firstCreator if only creator data was changed + // and not primaryData. + yield this.loadPrimaryData(true); + yield this.reload(); + this._clearChanged(); } - // Always reload primary data. DataObject.reload() only reloads changed data types, so - // it won't reload, say, dateModified and firstCreator if only creator data was changed - // and not primaryData. - yield this.loadPrimaryData(true); - yield this.reload(); - this._clearChanged(); - - return true; + return env.isNew ? this.id : true; }); /** @@ -4807,12 +4802,3 @@ Zotero.Item.prototype._getOldCreators = function () { } return oldCreators; } - - -Zotero.Item.prototype._disabledCheck = function () { - if (this._disabled) { - var msg = "New Zotero.Item objects shouldn't be accessed after save -- use Zotero.Items.get()"; - Zotero.debug(msg, 2, true); - Components.utils.reportError(msg); - } -} diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js index 0d46ec524..37eabdb80 100644 --- a/chrome/content/zotero/xpcom/search.js +++ b/chrome/content/zotero/xpcom/search.js @@ -87,40 +87,6 @@ Zotero.defineProperty(Zotero.Search.prototype, 'conditions', { get: function() this.getConditions() }); -Zotero.Search.prototype._set = function (field, value) { - if (field == 'id' || field == 'libraryID' || field == 'key') { - return this._setIdentifier(field, value); - } - - this._requireData('primaryData'); - - switch (field) { - case 'name': - value = value.trim().normalize(); - break; - - case 'version': - value = parseInt(value); - break; - - case 'synced': - value = !!value; - break; - } - - if (this['_' + field] != value) { - this._markFieldChange(field, this['_' + field]); - if (!this._changed.primaryData) { - this._changed.primaryData = {}; - } - this._changed.primaryData[field] = true; - - switch (field) { - default: - this['_' + field] = value; - } - } -} Zotero.Search.prototype.loadFromRow = function (row) { this._id = row.savedSearchID; @@ -205,31 +171,26 @@ Zotero.Search.prototype._saveData = Zotero.Promise.coroutine(function* (env) { }); Zotero.Search.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) { - var isNew = env.isNew; - if (isNew) { + if (env.isNew) { Zotero.Notifier.trigger('add', 'search', this.id, env.notifierData); } else if (!env.options.skipNotifier) { Zotero.Notifier.trigger('modify', 'search', this.id, env.notifierData); } - if (isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) { + if (env.isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) { var groupID = Zotero.Groups.getGroupIDFromLibraryID(this.libraryID); var group = yield Zotero.Groups.get(groupID); group.clearSearchCache(); } - if (isNew) { - var id = this.id; - this._disabled = true; - return id; + if (!env.skipCache) { + yield this.loadPrimaryData(true); + yield this.reload(); + this._clearChanged(); } - yield this.loadPrimaryData(true); - yield this.reload(); - this._clearChanged(); - - return isNew ? this.id : true; + return env.isNew ? this.id : true; }); diff --git a/test/tests/collectionTest.js b/test/tests/collectionTest.js index 8cdf9a280..2a4847e58 100644 --- a/test/tests/collectionTest.js +++ b/test/tests/collectionTest.js @@ -7,6 +7,7 @@ describe("Zotero.Collection", function() { var collection = new Zotero.Collection; collection.name = name; var id = yield collection.saveTx(); + assert.equal(collection.name, name); collection = yield Zotero.Collections.getAsync(id); assert.equal(collection.name, name); }); @@ -19,6 +20,7 @@ describe("Zotero.Collection", function() { collection.version = version; collection.name = "Test"; var id = yield collection.saveTx(); + assert.equal(collection.version, version); collection = yield Zotero.Collections.getAsync(id); assert.equal(collection.version, version); }); @@ -35,6 +37,7 @@ describe("Zotero.Collection", function() { col.name = "Child"; col.parentKey = parentKey; var id = yield col.saveTx(); + assert.equal(col.parentKey, parentKey); col = yield Zotero.Collections.getAsync(id); assert.equal(col.parentKey, parentKey); }); @@ -51,7 +54,6 @@ describe("Zotero.Collection", function() { col.name = "Child"; col.parentKey = parentKey; var id = yield col.saveTx(); - col = yield Zotero.Collections.getAsync(id); // Create new parent collection var newParentCol = new Zotero.Collection @@ -62,6 +64,7 @@ describe("Zotero.Collection", function() { // Change parent collection col.parentKey = newParentKey; yield col.saveTx(); + assert.equal(col.parentKey, newParentKey); col = yield Zotero.Collections.getAsync(id); assert.equal(col.parentKey, newParentKey); }); @@ -78,7 +81,6 @@ describe("Zotero.Collection", function() { col.name = "Child"; col.parentKey = parentKey; var id = yield col.saveTx(); - col = yield Zotero.Collections.getAsync(id); // Set to existing parent col.parentKey = parentKey; @@ -89,7 +91,6 @@ describe("Zotero.Collection", function() { var col = new Zotero.Collection col.name = "Test"; var id = yield col.saveTx(); - col = yield Zotero.Collections.getAsync(id); col.parentKey = false; var ret = yield col.saveTx(); diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index abb452563..74271663a 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -66,7 +66,6 @@ describe("Zotero.CollectionTreeView", function() { var collection = new Zotero.Collection; collection.name = "No select on modify"; var id = yield collection.saveTx(); - collection = yield Zotero.Collections.getAsync(id); resetSelection(); @@ -82,7 +81,6 @@ describe("Zotero.CollectionTreeView", function() { var collection = new Zotero.Collection; collection.name = "Reselect on modify"; var id = yield collection.saveTx(); - collection = yield Zotero.Collections.getAsync(id); var selected = collectionsView.getSelectedCollection(true); assert.equal(selected, id); diff --git a/test/tests/dataObjectTest.js b/test/tests/dataObjectTest.js index 04a0d57ea..98b908506 100644 --- a/test/tests/dataObjectTest.js +++ b/test/tests/dataObjectTest.js @@ -7,7 +7,6 @@ describe("Zotero.DataObject", function() { it("should load data on a regular item", function* () { var item = new Zotero.Item('book'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); yield item.loadAllData(); assert.throws(item.getNote.bind(item), 'getNote() can only be called on notes and attachments'); }) @@ -15,7 +14,6 @@ describe("Zotero.DataObject", function() { it("should load data on an attachment item", function* () { var item = new Zotero.Item('attachment'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); yield item.loadAllData(); assert.equal(item.getNote(), ''); }) @@ -23,7 +21,6 @@ describe("Zotero.DataObject", function() { it("should load data on a note item", function* () { var item = new Zotero.Item('note'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); yield item.loadAllData(); assert.equal(item.getNote(), ''); }) @@ -52,7 +49,6 @@ describe("Zotero.DataObject", function() { it("should be set to false after creating item", function* () { var item = new Zotero.Item("book"); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); assert.isFalse(item.synced); item.eraseTx(); }); @@ -60,7 +56,6 @@ describe("Zotero.DataObject", function() { it("should be set to true when changed", function* () { var item = new Zotero.Item("book"); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); item.synced = 1; yield item.saveTx(); @@ -72,7 +67,6 @@ describe("Zotero.DataObject", function() { it("should be set to false after modifying item", function* () { var item = new Zotero.Item("book"); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); item.synced = 1; yield item.saveTx(); @@ -88,7 +82,6 @@ describe("Zotero.DataObject", function() { it("should be unchanged if skipSyncedUpdate passed", function* () { var item = new Zotero.Item("book"); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); item.synced = 1; yield item.saveTx(); diff --git a/test/tests/dataObjectUtilitiesTest.js b/test/tests/dataObjectUtilitiesTest.js index 63679d397..787e367db 100644 --- a/test/tests/dataObjectUtilitiesTest.js +++ b/test/tests/dataObjectUtilitiesTest.js @@ -13,12 +13,10 @@ describe("Zotero.DataObjectUtilities", function() { yield Zotero.DB.executeTransaction(function* () { var item = new Zotero.Item('book'); id1 = yield item.save(); - item = yield Zotero.Items.getAsync(id1); json1 = yield item.toJSON(); var item = new Zotero.Item('book'); id2 = yield item.save(); - item = yield Zotero.Items.getAsync(id2); json2 = yield item.toJSON(); }); diff --git a/test/tests/itemPaneTest.js b/test/tests/itemPaneTest.js index 8370bed1a..f7260b6e6 100644 --- a/test/tests/itemPaneTest.js +++ b/test/tests/itemPaneTest.js @@ -14,7 +14,6 @@ describe("Item pane", function () { it("should refresh on item update", function* () { var item = new Zotero.Item('book'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); var itemBox = doc.getElementById('zotero-editpane-item-box'); var label = doc.getAnonymousNodes(itemBox)[0].getElementsByAttribute('fieldname', 'title')[1]; @@ -34,8 +33,6 @@ describe("Item pane", function () { it("should refresh on note update", function* () { var item = new Zotero.Item('note'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); - // Wait for the editor var noteBox = doc.getElementById('zotero-note-editor'); diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index b2e259ef3..dbc3e2c0f 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -109,6 +109,7 @@ describe("Zotero.Item", function () { var item = new Zotero.Item('book'); item.dateModified = dateModified; var id = yield item.saveTx(); + assert.equal(item.dateModified, dateModified); item = yield Zotero.Items.getAsync(id); assert.equal(item.dateModified, dateModified); }) @@ -120,6 +121,7 @@ describe("Zotero.Item", function () { var id = yield item.saveTx({ skipDateModifiedUpdate: true }); + assert.equal(item.dateModified, dateModified); item = yield Zotero.Items.getAsync(id); assert.equal(item.dateModified, dateModified); }) diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js index 85e15f075..022ff355a 100644 --- a/test/tests/itemTreeViewTest.js +++ b/test/tests/itemTreeViewTest.js @@ -115,7 +115,6 @@ describe("Zotero.ItemTreeView", function() { // Create item var item = new Zotero.Item('book'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); itemsView.selection.clearSelection(); assert.lengthOf(itemsView.getSelectedItems(), 0); @@ -139,7 +138,6 @@ describe("Zotero.ItemTreeView", function() { // Create item var item = new Zotero.Item('book'); var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); yield itemsView.selectItem(id); var selected = itemsView.getSelectedItems(true);