Don't load note/attachments counts as primary data

Zotero.Item::numNotes()/numAttachments() now require 'childItems' to
have been loaded.

Fixes #1301, Slow startup with many items in trash

chrome/content/zotero/xpcom/data/item.js

@@ -40,12 +40,6 @@ Zotero.Item = function(itemTypeOrID) {
 	this._itemTypeID = null;
 	this._firstCreator = null;
 	this._sortCreator = null;
-	this._numNotes = null;
-	this._numNotesTrashed = null;
-	this._numNotesEmbedded = null;
-	this._numNotesEmbeddedTrashed = null;
-	this._numAttachments = null;
-	this._numAttachmentsTrashed = null;
 	this._attachmentCharset = null;
 	this._attachmentLinkMode = null;
 	this._attachmentContentType = null;
@@ -341,12 +335,6 @@ Zotero.Item.prototype._parseRowData = function(row) {
 			
 			// Integer or 0
 			case 'version':
-			case 'numNotes':
-			case 'numNotesTrashed':
-			case 'numNotesEmbedded':
-			case 'numNotesEmbeddedTrashed':
-			case 'numAttachments':
-			case 'numAttachmentsTrashed':
 				val = val ? parseInt(val) : 0;
 				break;
 			
@@ -1846,7 +1834,8 @@ Zotero.Item.prototype.isTopLevelItem = function () {
 
 
 Zotero.Item.prototype.numChildren = function(includeTrashed) {
-	return this.numNotes(includeTrashed) + this.numAttachments(includeTrashed);
+	this._requireData('childItems');
+	return this._notes.rows.length + this._attachments.rows.length;
 }
 
 
@@ -1870,39 +1859,6 @@ Zotero.Item.prototype.setSourceKey = function(sourceItemKey) {
 // Methods dealing with note items
 //
 ////////////////////////////////////////////////////////
-Zotero.Item.prototype.incrementNumNotes = function () {
-	this._numNotes++;
-}
-
-Zotero.Item.prototype.incrementNumNotesTrashed = function () {
-	this._numNotesTrashed++;
-}
-
-Zotero.Item.prototype.incrementNumNotesEmbedded = function () {
-	this._numNotesEmbedded++;
-}
-
-Zotero.Item.prototype.incrementNumNotesTrashed = function () {
-	this._numNotesEmbeddedTrashed++;
-}
-
-Zotero.Item.prototype.decrementNumNotes = function () {
-	this._numNotes--;
-}
-
-Zotero.Item.prototype.decrementNumNotesTrashed = function () {
-	this._numNotesTrashed--;
-}
-
-Zotero.Item.prototype.decrementNumNotesEmbedded = function () {
-	this._numNotesEmbedded--;
-}
-
-Zotero.Item.prototype.decrementNumNotesTrashed = function () {
-	this._numNotesEmbeddedTrashed--;
-}
-
-
 /**
 * Determine if an item is a note
 **/
@@ -1929,20 +1885,15 @@ Zotero.Item.prototype.updateNote = function(text) {
  * @return	{Integer}
  */
 Zotero.Item.prototype.numNotes = function(includeTrashed, includeEmbedded) {
-	if (this.isNote()) {
+	this._requireData('childItems');
-		throw ("numNotes() cannot be called on items of type 'note'");
+	var notes = Zotero.Items.get(this.getNotes(includeTrashed));
+	var num = notes.length;
+	if (includeEmbedded) {
+		// Include embedded attachment notes that aren't empty
+		num += Zotero.Items.get(this.getAttachments(includeTrashed))
+			.filter(x => x.getNote() !== '').length;
 	}
-	var cacheKey = '_numNotes';
+	return num;
-	if (includeTrashed && includeEmbedded) {
-		return this[cacheKey] + this[cacheKey + "EmbeddedTrashed"];
-	}
-	else if (includeTrashed) {
-		return this[cacheKey] + this[cacheKey + "Trashed"];
-	}
-	else if (includeEmbedded) {
-		return this[cacheKey] + this[cacheKey + "Embedded"];
-	}
-	return this[cacheKey];
 }
 
 
@@ -2153,16 +2104,9 @@ Zotero.Item.prototype.isFileAttachment = function() {
  * @param	{Boolean}	includeTrashed		Include trashed child items in count
  * @return	<Integer>
  */
-Zotero.Item.prototype.numAttachments = function(includeTrashed) {
+Zotero.Item.prototype.numAttachments = function (includeTrashed) {
-	if (this.isAttachment()) {
+	this._requireData('childItems');
-		throw ("numAttachments() cannot be called on attachment items");
+	return this.getAttachments(includeTrashed).length;
-	}
-	
-	var cacheKey = '_numAttachments';
-	if (includeTrashed) {
-		return this[cacheKey] + this[cacheKey + "Trashed"];
-	}
-	return this[cacheKey];
 }
 
 

chrome/content/zotero/xpcom/data/items.js

@@ -52,33 +52,6 @@ Zotero.Items = function() {
 				deleted: "DI.itemID IS NOT NULL AS deleted",
 				inPublications: "PI.itemID IS NOT NULL AS inPublications",
 				
-				numNotes: "(SELECT COUNT(*) FROM itemNotes INo "
-					+ "WHERE parentItemID=O.itemID AND "
-					+ "INo.itemID NOT IN (SELECT itemID FROM deletedItems)) AS numNotes",
-				
-				numNotesTrashed: "(SELECT COUNT(*) FROM itemNotes INo "
-					+ "WHERE parentItemID=O.itemID AND "
-					+ "INo.itemID IN (SELECT itemID FROM deletedItems)) AS numNotesTrashed",
-				
-				numNotesEmbedded: "(SELECT COUNT(*) FROM itemAttachments IA "
-					+ "JOIN itemNotes USING (itemID) "
-					+ "WHERE IA.parentItemID=O.itemID AND "
-					+ "note!='' AND note!='" + Zotero.Notes.defaultNote + "' AND "
-					+ "IA.itemID NOT IN (SELECT itemID FROM deletedItems)) AS numNotesEmbedded",
-				
-				numNotesEmbeddedTrashed: "(SELECT COUNT(*) FROM itemAttachments IA "
-					+ "JOIN itemNotes USING (itemID) "
-					+ "WHERE IA.parentItemID=O.itemID AND "
-					+ "note!='' AND note!='" + Zotero.Notes.defaultNote + "' AND "
-					+ "IA.itemID IN (SELECT itemID FROM deletedItems)) "
-					+ "AS numNotesEmbeddedTrashed",
-				
-				numAttachments: "(SELECT COUNT(*) FROM itemAttachments IA WHERE parentItemID=O.itemID AND "
-					+ "IA.itemID NOT IN (SELECT itemID FROM deletedItems)) AS numAttachments",
-				
-				numAttachmentsTrashed: "(SELECT COUNT(*) FROM itemAttachments IA WHERE parentItemID=O.itemID AND "
-					+ "IA.itemID IN (SELECT itemID FROM deletedItems)) AS numAttachmentsTrashed",
-				
 				parentID: "(CASE O.itemTypeID WHEN 14 THEN IAP.itemID WHEN 1 THEN INoP.itemID END) AS parentID",
 				parentKey: "(CASE O.itemTypeID WHEN 14 THEN IAP.key WHEN 1 THEN INoP.key END) AS parentKey",
 				

chrome/content/zotero/xpcom/itemTreeView.js

@@ -1060,6 +1060,9 @@ Zotero.ItemTreeView.prototype.getCellText = function (row, column)
 	}
 	else if (column.id === "zotero-items-column-numNotes") {
 		val = obj.numNotes();
+		if (!val) {
+			val = '';
+		}
 	}
 	else {
 		var col = column.id.substring(20);
@@ -3371,7 +3374,10 @@ Zotero.ItemTreeRow.prototype.getField = function(field, unformatted)
 
 Zotero.ItemTreeRow.prototype.numNotes = function() {
 	if (this.ref.isNote()) {
-		return '';
+		return 0;
 	}
-	return this.ref.numNotes(false, true) || '';
+	if (this.ref.isAttachment()) {
+		return this.ref.getNote() !== '' ? 1 : 0;
+	}
+	return this.ref.numNotes(false, true) || 0;
 }

test/tests/itemTest.js

@@ -555,6 +555,34 @@ describe("Zotero.Item", function () {
 		});
 	})
 	
+	
+	describe("#numAttachments()", function () {
+		it("should include child attachments", function* () {
+			var item = yield createDataObject('item');
+			var attachment = yield importFileAttachment('test.png', { parentID: item.id });
+			assert.equal(item.numAttachments(), 1);
+		});
+		
+		it("shouldn't include trashed child attachments by default", function* () {
+			var item = yield createDataObject('item');
+			yield importFileAttachment('test.png', { parentID: item.id });
+			var attachment = yield importFileAttachment('test.png', { parentID: item.id });
+			attachment.deleted = true;
+			yield attachment.saveTx();
+			assert.equal(item.numAttachments(), 1);
+		});
+		
+		it("should include trashed child attachments if includeTrashed=true", function* () {
+			var item = yield createDataObject('item');
+			yield importFileAttachment('test.png', { parentID: item.id });
+			var attachment = yield importFileAttachment('test.png', { parentID: item.id });
+			attachment.deleted = true;
+			yield attachment.saveTx();
+			assert.equal(item.numAttachments(true), 2);
+		});
+	});
+	
+	
 	describe("#getAttachments()", function () {
 		it("#should return child attachments", function* () {
 			var item = yield createDataObject('item');
@@ -618,6 +646,49 @@ describe("Zotero.Item", function () {
 		});
 	})
 	
+	describe("#numNotes()", function () {
+		it("should include child notes", function* () {
+			var item = yield createDataObject('item');
+			yield createDataObject('item', { itemType: 'note', parentID: item.id });
+			yield createDataObject('item', { itemType: 'note', parentID: item.id });
+			assert.equal(item.numNotes(), 2);
+		});
+		
+		it("shouldn't include trashed child notes by default", function* () {
+			var item = yield createDataObject('item');
+			yield createDataObject('item', { itemType: 'note', parentID: item.id });
+			yield createDataObject('item', { itemType: 'note', parentID: item.id, deleted: true });
+			assert.equal(item.numNotes(), 1);
+		});
+		
+		it("should include trashed child notes with includeTrashed", function* () {
+			var item = yield createDataObject('item');
+			yield createDataObject('item', { itemType: 'note', parentID: item.id });
+			yield createDataObject('item', { itemType: 'note', parentID: item.id, deleted: true });
+			assert.equal(item.numNotes(true), 2);
+		});
+		
+		it("should include child attachment notes with includeEmbedded", function* () {
+			var item = yield createDataObject('item');
+			yield createDataObject('item', { itemType: 'note', parentID: item.id });
+			var attachment = yield importFileAttachment('test.png', { parentID: item.id });
+			attachment.setNote('test');
+			yield attachment.saveTx();
+			yield item.loadDataType('childItems');
+			assert.equal(item.numNotes(false, true), 2);
+		});
+		
+		it("shouldn't include empty child attachment notes with includeEmbedded", function* () {
+			var item = yield createDataObject('item');
+			yield createDataObject('item', { itemType: 'note', parentID: item.id });
+			var attachment = yield importFileAttachment('test.png', { parentID: item.id });
+			assert.equal(item.numNotes(false, true), 1);
+		});
+		
+		// TODO: Fix numNotes(false, true) updating after child attachment note is added or removed
+	});
+	
+	
 	describe("#getNotes()", function () {
 		it("#should return child notes", function* () {
 			var item = yield createDataObject('item');