Merge pull request #525 from aurimasv/async_db-review

[Async DB] Documentation/minor tweaks

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

@@ -1008,8 +1008,3 @@ Zotero.Collection.prototype._refreshChildItems = Zotero.Promise.coroutine(functi
 		return this.loadChildItems(true);
 	}
 });
-
-
-Zotero.Collection.prototype._generateKey = function () {
-	return Zotero.Utilities.generateObjectKey();
-}

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

@@ -23,10 +23,21 @@
     ***** END LICENSE BLOCK *****
 */
 
-Zotero.DataObject = function (objectType, objectTypePlural, dataTypes) {
+/**
+ *
+ * @param {String} objectType
+ * @param {String[]} dataTypes A set of data types that can be loaded for this data object
+ *
+ * @property {String} (readOnly) objectType
+ * @property {String} (readOnly) libraryKey
+ * @property {String|null} parentKey Null if no parent
+ * @property {Integer|false|undefined} parentID False if no parent. Undefined if not applicable (e.g. search objects)
+ */
+
+Zotero.DataObject = function (objectType, dataTypes) {
 	this._objectType = objectType;
 	this._ObjectType = objectType[0].toUpperCase() + objectType.substr(1);
-	this._objectTypePlural = objectTypePlural ? objectTypePlural : objectType + 's';
+	this._objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
 	this._dataTypes = dataTypes;
 	
 	this._id = null;
@@ -46,12 +57,20 @@ Zotero.DataObject = function (objectType, objectTypePlural, dataTypes) {
 	this._clearChanged();
 };
 
-Zotero.DataObject.prototype.__defineGetter__('objectType', function () { return this._objectType; });
+Zotero.Utilities.Internal.defineProperty(Zotero.DataObject, 'objectType', {
-Zotero.DataObject.prototype.__defineGetter__('libraryKey', function () this.libraryID + "/" + this.key);
+	get: function() this._objectType
-Zotero.DataObject.prototype.__defineGetter__('parentKey', function () this._parentKey );
+});
-Zotero.DataObject.prototype.__defineSetter__('parentKey', function (val) this._setParentKey(val) );
+Zotero.Utilities.Internal.defineProperty(Zotero.DataObject, 'libraryKey', {
-Zotero.DataObject.prototype.__defineGetter__('parentID', function () this._getParentID() );
+	get: function() this._libraryID + "/" + this._key
-Zotero.DataObject.prototype.__defineSetter__('parentID', function (val) this._setParentID(val) );
+});
+Zotero.Utilities.Internal.defineProperty(Zotero.DataObject, 'parentKey', {
+	get: function() this._parentKey,
+	set: function(v) this._setParentKey(v)
+});
+Zotero.Utilities.Internal.defineProperty(Zotero.DataObject, 'parentID', {
+	get: function() this._getParentID(),
+	set: function(v) this._setParentID(v)
+});
 
 
 Zotero.DataObject.prototype._get = function (field) {
@@ -82,7 +101,7 @@ Zotero.DataObject.prototype._setIdentifier = function (field, value) {
 		if (this._key) {
 			throw new Error("Cannot set id if key is already set");
 		}
-		value = parseInt(value);
+		value = Zotero.DataObjectUtilities.checkDataID(value);
 		this._identified = true;
 		break;
 		
@@ -91,12 +110,13 @@ Zotero.DataObject.prototype._setIdentifier = function (field, value) {
 		break;
 		
 	case 'key':
-		if (this._libraryID === undefined) {
+		if (this._libraryID === null) {
 			throw new Error("libraryID must be set before key");
 		}
 		if (this._id) {
 			throw new Error("Cannot set key if id is already set");
 		}
+		value = Zotero.DataObjectUtilities.checkKey(value);
 		this._identified = true;
 	}
 	
@@ -129,25 +149,36 @@ Zotero.DataObject.prototype._getParentID = function () {
  * Set the id of the parent object
  *
  * @param {Number|false} [id=false]
+ * @return {Boolean} True if changed, false if stayed the same
  */
 Zotero.DataObject.prototype._setParentID = function (id) {
-	return this._setParentKey(id ? this._getClass().getLibraryAndKeyFromID(id)[1] : false);
+	return this._setParentKey(
+		id
+		? this._getClass().getLibraryAndKeyFromID(Zotero.DataObjectUtilities.checkDataID(id))[1]
+		: null
+	);
 }
 
-
+/**
+ * Set the key of the parent object
+ *
+ * @param {String|null} [key=null]
+ * @return {Boolean} True if changed, false if stayed the same
+ */
 Zotero.DataObject.prototype._setParentKey = function(key) {
 	if (this._objectType == 'item') {
 		if (!this.isNote() && !this.isAttachment()) {
 			throw new Error("_setParentKey() can only be called on items of type 'note' or 'attachment'");
 		}
 	}
+	key = Zotero.DataObjectUtilities.checkKey(key);
 	
 	if (this._parentKey == key) {
 		return false;
 	}
 	this._markFieldChange('parentKey', this._parentKey);
 	this._changed.parentKey = true;
-	this._parentKey = key ? key : null;
+	this._parentKey = key;
 	this._parentID = null;
 	return true;
 }
@@ -156,7 +187,8 @@ Zotero.DataObject.prototype._setParentKey = function(key) {
 /**
  * Returns all relations of the object
  *
- * @return object Object with predicates as keys and URIs as values
+ * @return {object} Object with predicates as keys and URI[], or URI (as string)
+ *   in the case of a single object, as values
  */
 Zotero.DataObject.prototype.getRelations = function () {
 	this._requireData('relations');
@@ -187,14 +219,15 @@ Zotero.DataObject.prototype.getRelations = function () {
 /**
  * Updates the object's relations
  *
- * @param {Object} newRelations Object with predicates as keys and URIs/arrays-of-URIs as values
+ * @param {Object} newRelations Object with predicates as keys and URI[] as values
+ * @return {Boolean} True if changed, false if stayed the same
  */
 Zotero.DataObject.prototype.setRelations = function (newRelations) {
 	this._requireData('relations');
 	
 	// There can be more than one object for a given predicate, so build
-	// flat arrays with individual predicate-object pairs converted to
+	// flat arrays with individual predicate-object pairs so we can use
-	// JSON strings so we can use array_diff to determine what changed
+	// array_diff to determine what changed
 	var oldRelations = this._relations;
 	
 	var sortFunc = function (a, b) {
@@ -208,15 +241,10 @@ Zotero.DataObject.prototype.setRelations = function (newRelations) {
 	var newRelationsFlat = [];
 	for (let predicate in newRelations) {
 		let object = newRelations[predicate];
-		if (Array.isArray(object)) {
 		for (let i=0; i<object.length; i++) {
 			newRelationsFlat.push([predicate, object[i]]);
 		}
 	}
-		else {
-			newRelationsFlat.push([predicate, object]);
-		}
-	}
 	
 	var changed = false;
 	if (oldRelations.length != newRelationsFlat.length) {
@@ -227,7 +255,7 @@ Zotero.DataObject.prototype.setRelations = function (newRelations) {
 		newRelationsFlat.sort(sortFunc);
 		
 		for (let i=0; i<oldRelations.length; i++) {
-			if (!newRelationsFlat || oldRelations[i] != newRelationsFlat[i]) {
+			if (oldRelations[i] != newRelationsFlat[i]) {
 				changed = true;
 				break;
 			}
@@ -240,17 +268,20 @@ Zotero.DataObject.prototype.setRelations = function (newRelations) {
 	}
 	
 	this._markFieldChange('relations', this._relations);
+	this._changed.relations = true;
 	// Store relations internally as array of predicate-object pairs
 	this._relations = newRelationsFlat;
-	this._changed.relations = true;
+	return true;
 }
 
 
 /**
  * Return an object in the specified library equivalent to this object
+ * @param {Integer} [libraryID=0]
+ * @return {Object|false} Linked item, or false if not found
  */
 Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function* (libraryID) {
-	if (libraryID == this.libraryID) {
+	if (libraryID == this._libraryID) {
 		throw new Error(this._ObjectType + " is already in library " + libraryID);
 	}
 	
@@ -273,6 +304,7 @@ Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function
 	else {
 		var libraryObjectPrefix = Zotero.URI.getCurrentUserURI() + "/" + this._objectTypePlural + "/";
 	}
+	
 	for (let i=0; i<links.length; i++) {
 		let link = links[i];
 		if (link.indexOf(libraryObjectPrefix) == 0) {
@@ -292,7 +324,7 @@ Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function
 /**
  * Reloads loaded, changed data
  *
- * @param {Array} [dataTypes] - Data types to reload, or all loaded types if not provide
+ * @param {String[]} [dataTypes] - Data types to reload, or all loaded types if not provide
  * @param {Boolean} [reloadUnchanged=false] - Reload even data that hasn't changed internally.
  *                                            This should be set to true for data that was
  *                                            changed externally (e.g., globally renamed tags).
@@ -319,8 +351,18 @@ Zotero.DataObject.prototype.reload = Zotero.Promise.coroutine(function* (dataTyp
 	}
 });
 
-
+/**
+ * Checks wheteher a given data type has been loaded
+ *
+ * @param {String} [dataType=primaryData] Data type to check
+ * @throws {Zotero.DataObjects.UnloadedDataException} If not loaded, unless the
+ *   data has not yet been "identified"
+ */
 Zotero.DataObject.prototype._requireData = function (dataType) {
+	if (this._loaded[dataType] === undefined) {
+		throw new Error(dataType + " is not a valid data type for " + this._ObjectType + " objects");
+	}
+	
 	if (dataType != 'primaryData') {
 		this._requireData('primaryData');
 	}
@@ -332,18 +374,24 @@ Zotero.DataObject.prototype._requireData = function (dataType) {
 		throw new Zotero.Exception.UnloadedDataException(
 			"'" + dataType + "' not loaded for " + this._objectType + " ("
 				+ this._id + "/" + this._libraryID + "/" + this._key + ")",
-			this._objectType + dataType[0].toUpperCase() + dataType.substr(1)
+			dataType
 		);
 	}
 }
 
-
+/**
-
+ * Returns a global Zotero class object given a data object. (e.g. Zotero.Items)
+ * @return {obj} One of Zotero data classes
+ */
 Zotero.DataObject.prototype._getClass = function () {
 	return Zotero.DataObjectUtilities.getClassForObjectType(this._objectType);
 }
 
-
+/**
+ * Loads data for a given data type
+ * @param {String} dataType
+ * @param {Boolean} reload
+ */
 Zotero.DataObject.prototype._loadDataType = function (dataType, reload) {
 	return this["load" + dataType[0].toUpperCase() + dataType.substr(1)](reload);
 }
@@ -351,6 +399,8 @@ Zotero.DataObject.prototype._loadDataType = function (dataType, reload) {
 
 /**
  * Save old version of data that's being changed, to pass to the notifier
+ * @param {String} field
+ * @param {} oldValue
  */
 Zotero.DataObject.prototype._markFieldChange = function (field, oldValue) {
 	// Only save if object already exists and field not already changed
@@ -360,7 +410,10 @@ Zotero.DataObject.prototype._markFieldChange = function (field, oldValue) {
 	this._previousData[field] = oldValue;
 }
 
-
+/**
+ * Clears log of changed values
+ * @param {String} [dataType] data type/field to clear. Defaults to clearing everything
+ */
 Zotero.DataObject.prototype._clearChanged = function (dataType) {
 	if (dataType) {
 		delete this._changed[dataType];
@@ -372,12 +425,18 @@ Zotero.DataObject.prototype._clearChanged = function (dataType) {
 	}
 }
 
-
+/**
+ * Clears field change log
+ * @param {String} field
+ */
 Zotero.DataObject.prototype._clearFieldChange = function (field) {
 	delete this._previousData[field];
 }
 
-
+/**
+ * Generates data object key
+ * @return {String} key
+ */
 Zotero.DataObject.prototype._generateKey = function () {
 	return Zotero.Utilities.generateObjectKey();
 }

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

@@ -27,17 +27,32 @@
 Zotero.DataObjectUtilities = {
 	"checkLibraryID": function (libraryID) {
 		if (libraryID === null) {
-			Zotero.debug("Deprecated: libraryID cannot be NULL\n\n" + Components.stack, 2);
+			Zotero.debug("Deprecated: libraryID cannot be NULL", 2, 1);
 		}
 		else {
 			var intValue = parseInt(libraryID);
-			if (libraryID != intValue) {
+			if (libraryID != intValue || intValue < 0) {
-				throw new Error("libraryID must be an integer");
+				throw new Error("libraryID must be a positive integer");
 			}
 		}
 		return intValue;
 	},
 	
+	"checkDataID": function(dataID) {
+		var intValue = parseInt(dataID);
+		if (dataID != intValue || dataID < 0)
+			throw new Error("id must be a positive integer");
+		return intValue;
+	},
+	
+	"checkKey": function(key) {
+		if (!key) return null;
+		if (!Zotero.Utilities.isValidObjectKey(key)) {
+			throw new Error("key is not valid");
+		}
+		return key;
+	},
+	
 	"getObjectTypePlural": function getObjectTypePlural(objectType) {
 		return objectType == 'search' ? 'searches' : objectType + 's';
 	},

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

@@ -4694,14 +4694,12 @@ Zotero.Item.prototype._getRelatedItems = function () {
 	
 	var predicate = Zotero.Relations.relatedItemPredicate;
 	
-	var relations = this.getRelations();
+	var relatedItemURIs = this.getRelations()[predicate];
-	if (!relations[predicate]) {
+	if (!relatedItemURIs) {
 		return [];
 	}
 	
-	var relatedItemURIs = typeof relations[predicate] == 'string'
+	if (typeof relatedItemURIs == 'string') relatedItemURIs = [relatedItemURIs];
-		? [relations[predicate]]
-		: relations[predicate];
 	
 	// Pull out object values from related-item relations, turn into items, and pull out keys
 	var keys = [];

chrome/content/zotero/xpcom/debug.js

@@ -41,7 +41,7 @@ Zotero.Debug = new function () {
 		this.enabled = _console || _store;
 	}
 	
-	this.log = function (message, level) {
+	this.log = function (message, level, stack) {
 		if (!_console && !_store) {
 			return;
 		}
@@ -92,15 +92,24 @@ Zotero.Debug = new function () {
 			deltaStr = '(+' + delta + ')';
 		}
 		
-		if (_stackTrace) {
+		if (stack === true) {
-			var stack = (new Error()).stack;
+			// Display stack starting from where this was called
-			var nl1Index = stack.indexOf("\n")
+			stack = Components.stack.caller;
-			var nl2Index = stack.indexOf("\n", nl1Index+1);
+		} else if (stack >= 0) {
-			var line2 = stack.substring(nl1Index+2, nl2Index-1);
+			let i = stack;
-			var debugLine = line2.substr(line2.indexOf("@"));
+			stack = Components.stack.caller;
+			while(stack && i--) {
+				stack = stack.caller;
+			}
+		} else if (_stackTrace) {
+			// Stack trace enabled globally
+			stack = Components.stack.caller;
+		} else {
+			stack = undefined;
+		}
 		
-			stack = stack.substring(nl2Index, stack.length-1);
+		if (stack) {
-			message += "\n"+debugLine+stack;
+			message += '\n' + Zotero.Debug.stackToString(stack);
 		}
 		
 		if (_console) {
@@ -194,4 +203,20 @@ Zotero.Debug = new function () {
 	this.clear = function () {
 		_output = [];
 	}
+	
+	/**
+	 * Format a stack trace for output in the same way that Error.stack does
+	 * @param {Components.stack} stack
+	 * @param {Integer} [lines=5] Number of lines to format
+	 */
+	this.stackToString = function (stack, lines) {
+		if (!lines) lines = 5;
+		var str = '';
+		while(stack && lines--) {
+			str += '\n  ' + (stack.name || '') + '@' + stack.filename
+				+ ':' + stack.lineNumber;
+			stack = stack.caller;
+		}
+		return str.substr(1);
+	};
 }

chrome/content/zotero/xpcom/id.js

@@ -83,13 +83,6 @@ Zotero.ID_Tracker = function () {
 		}
 	});
 	
-	
-	this.isValidKey = function (value) {
-		var re = /^[23456789ABCDEFGHIJKLMNPQRSTUVWXYZ]{8}$/
-		return re.test(value);
-	}
-	
-	
 	function getBigInt(max) {
 		if (!max) {
 			max = 9007199254740991;

chrome/content/zotero/xpcom/search.js

@@ -1642,13 +1642,6 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 	this._sqlParams = sqlParams.length ? sqlParams : false;
 });
 
-
-Zotero.Search.prototype._generateKey = function () {
-	return Zotero.Utilities.generateObjectKey();
-}
-
-
-
 Zotero.Searches = new function(){
 	Zotero.DataObjects.apply(this, ['search', 'searches', 'savedSearch', 'savedSearches']);
 	this.constructor.prototype = new Zotero.DataObjects();

chrome/content/zotero/xpcom/utilities.js

@@ -1813,13 +1813,23 @@ Zotero.Utilities = {
 							? "attachment-pdf" : "attachment-snapshot");
 	},
 	
+	"allowedKeyChars": "23456789ABCDEFGHIJKLMNPQRSTUVWXYZ",
+	
 	/**
 	 * Generates a valid object key for the server API
 	 */
 	"generateObjectKey":function generateObjectKey() {
-		// TODO: add 'L' and 'Y' after 3.0.11 cut-off
+		return Zotero.Utilities.randomString(8, Zotero.Utilities.allowedKeyChars);
-		var baseString = "23456789ABCDEFGHIJKMNPQRSTUVWXZ";
+	},
-		return Zotero.Utilities.randomString(8, baseString);
+	
+	/**
+	 * Check if an object key is in a valid format
+	 */
+	"isValidObjectKey":function(key) {
+		if (!Zotero.Utilities.objectKeyRegExp) {
+			Zotero.Utilities.objectKeyRegExp = new RegExp('^[' + Zotero.Utilities.allowedKeyChars + ']{8}$');
+		}
+		return Zotero.Utilities.objectKeyRegExp.test(key);
 	},
 	
 	/**

chrome/content/zotero/xpcom/utilities_internal.js

@@ -366,6 +366,24 @@ Zotero.Utilities.Internal = {
 				break;
 			}
 		}
+	},
+	
+	/**
+	 * Defines property on the object's prototype.
+	 * More compact way to do Object.defineProperty
+	 *
+	 * @param {Object} obj Target object
+	 * @param {String} prop Property to be defined
+	 * @param {Object} desc Propery descriptor. If not overriden, "enumerable" is true
+	 */
+	"defineProperty": function(obj, prop, desc) {
+		if (typeof prop != 'string') throw new Error("Property must be a string");
+		var d = { __proto__: null, enumerable: true }; // Enumerable by default
+		for (let p in desc) {
+			if (!desc.hasOwnProperty(p)) continue;
+			d[p] = desc[p];
+		}
+		Object.defineProperty(obj.prototype, prop, d);
 	}
 }
 

chrome/content/zotero/xpcom/zotero.js

@@ -1217,10 +1217,21 @@ Components.utils.import("resource://gre/modules/osfile.jsm");
 	 *
 	 * Uses prefs e.z.debug.log and e.z.debug.level (restart required)
 	 *
-	 * Defaults to log level 3 if level not provided
+	 * @param {} message
+	 * @param {Integer} [level=3]
+	 * @param {Boolean|Integer} [stack] Whether to display the calling stack.
+	 *   If true, stack is displayed starting from the caller. If an integer,
+	 *   that many stack levels will be omitted starting from the caller.
 	 */
-	function debug(message, level) {
+	function debug(message, level, stack) {
-		Zotero.Debug.log(message, level);
+		// Account for this alias
+		if (stack === true) {
+			stack = 1;
+		} else if (stack >= 0) {
+			stack++;
+		}
+		
+		Zotero.Debug.log(message, level, stack);
 	}