Addresses #352, Make sure data layer doesn't allow bad data via the API

Don't allow a save() with noncontiguous creator order indexes

chrome/content/zotero/xpcom/data_access.js

@@ -38,9 +38,9 @@ Zotero.Item.prototype._init = function(){
 	//
 	// Public members for access by public methods -- do not access directly
 	//
-	this._data = new Array();
+	this._data = [];
-	this._creators = new Zotero.Hash();
+	this._creators = [];
-	this._itemData = new Array();
+	this._itemData = [];
 	
 	this._creatorsLoaded = false;
 	this._itemDataLoaded = false;
@@ -231,7 +231,7 @@ Zotero.Item.prototype.hasCreatorAt = function(pos){
 		this._loadCreators();
 	}
 	
-	return this._creators.has(pos);
+	return !!this._creators[pos];
 }
 
 
@@ -245,10 +245,7 @@ Zotero.Item.prototype.getCreator = function(pos){
 		this._loadCreators();
 	}
 	
-	if (!this._creators.items[pos]){
+	return this._creators[pos] ? this._creators[pos] : false;
-		return false;
-	}
-	return this._creators.items[pos];
 }
 
 
@@ -259,7 +256,9 @@ Zotero.Item.prototype.getCreator = function(pos){
  */
 Zotero.Item.prototype.getCreators = function(){
 	var creators = [];
-	for (var i=0, len=this.numCreators(); i<len; i++){
+	Zotero.debug(this._creators);
+	Zotero.debug(this._creators.length);
+	for (var i=0; i<this._creators.length; i++){
 		creators.push(this.getCreator(i));
 	}
 	return creators;
@@ -296,11 +295,11 @@ Zotero.Item.prototype.setCreator = function(orderIndex, firstName, lastName, cre
 	creatorTypeID = Zotero.CreatorTypes.getID(creatorTypeID);
 	
 	// If creator at this position hasn't changed, cancel
-	if (this._creators.has(orderIndex) &&
+	if (this._creators[orderIndex] &&
-		this._creators.get(orderIndex)['firstName']==firstName &&
+		this._creators[orderIndex]['firstName']==firstName &&
-		this._creators.get(orderIndex)['lastName']==lastName &&
+		this._creators[orderIndex]['lastName']==lastName &&
-		this._creators.get(orderIndex)['creatorTypeID']==creatorTypeID &&
+		this._creators[orderIndex]['creatorTypeID']==creatorTypeID &&
-		this._creators.get(orderIndex)['fieldMode']==fieldMode){
+		this._creators[orderIndex]['fieldMode']==fieldMode){
 		return false;
 	}
 	
@@ -308,13 +307,14 @@ Zotero.Item.prototype.setCreator = function(orderIndex, firstName, lastName, cre
 		creatorTypeID = 1;
 	}
 	
-	var creator = new Array();
+	var creator = {
-	creator['firstName'] = firstName;
+		firstName: firstName,
-	creator['lastName'] = lastName;
+		lastName: lastName,
-	creator['creatorTypeID'] = creatorTypeID;
+		creatorTypeID: creatorTypeID,
-	creator['fieldMode'] = fieldMode;
+		fieldMode: fieldMode
+	}
 	
-	this._creators.set(orderIndex, creator);
+	this._creators[orderIndex] = creator;
 	this._changedCreators.set(orderIndex);
 	return true;
 }
@@ -328,16 +328,15 @@ Zotero.Item.prototype.removeCreator = function(orderIndex){
 		this._loadCreators();
 	}
 	
-	if (!this._creators.has(orderIndex)){
+	if (!this._creators[orderIndex]){
 		throw ('No creator exists at position ' + orderIndex);
 	}
-	this._creators.remove(orderIndex);
+	this._creators[orderIndex] = false;
 	
 	// Go to length+1 so we clear the last one
 	for (var i=orderIndex, max=this._creators.length+1; i<max; i++){
-		var next =
+		var next = this._creators[i+1] ? this._creators[i+1] : false;
-			this._creators.items[i+1] ? this._creators.items[i+1] : false;
+		this._creators[i] = next;
-		this._creators.set(i, next);
 		this._changedCreators.set(i);
 	}
 	return true;
@@ -469,6 +468,18 @@ Zotero.Item.prototype.save = function(){
 		return false;
 	}
 	
+	// Make sure there are no gaps in the creator indexes
+	var creators = this.getCreators();
+	for (var i=0; i<creators.length; i++){
+		if (!creators[i] || (!creators[i].firstName && !creators[i].lastName)){
+			var lastCreator = true;
+			continue;
+		}
+		if (lastCreator){
+			throw("Creator indices not contiguous or don't start at 0");
+		}
+	}
+	
 	//
 	// Existing item, update
 	//
@@ -527,8 +538,7 @@ Zotero.Item.prototype.save = function(){
 					Zotero.DB.query(sql2);
 					
 					// If empty, move on
-					if (typeof creator['firstName'] == 'undefined'
+					if (!creator['firstName'] && !creator['lastName']){
-						&& typeof creator['lastName'] == 'undefined'){
 						continue;
 					}
 					
@@ -1752,16 +1762,14 @@ Zotero.Item.prototype._loadCreators = function(){
 		return true;
 	}
 	
-	this._creators = new Zotero.Hash();
+	this._creators = [];
 	for (var i=0; i<creators.length; i++){
-		var creator = {
+		this._creators[creators[i]['orderIndex']] = {
 			firstName: creators[i]['firstName'],
 			lastName: creators[i]['lastName'],
 			creatorTypeID: creators[i]['creatorTypeID'],
 			fieldMode: creators[i]['fieldMode']
-		}
+		};
-		// Save creator data into Hash, indexed by orderIndex
-		this._creators.set(creators[i]['orderIndex'], creator);
 	}
 	
 	return true;
@@ -2585,6 +2593,7 @@ Zotero.Collections = new function(){
 		
 		var result = Zotero.DB.query(sql);
 		
+		if (result){
 			for (var i=0; i<result.length; i++){
 				var collectionID = result[i]['collectionID'];
 				
@@ -2599,6 +2608,7 @@ Zotero.Collections = new function(){
 					_collections[collectionID].loadFromRow(result[i]);
 				}
 			}
+		}
 		_collectionsLoaded = true;
 	}
 }