From ef57b4e0167073d3f83b6ec1ca009de1baff3e2b Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sun, 24 May 2015 03:08:22 -0400
Subject: [PATCH] Relations fixes and cleanup

Relations need a complete overhaul, but this makes them generally work
again.
---
 chrome/content/zotero/xpcom/data/creators.js  |  10 +-
 chrome/content/zotero/xpcom/data/group.js     |  26 +-
 chrome/content/zotero/xpcom/data/relation.js  | 260 ------------------
 chrome/content/zotero/xpcom/data/relations.js | 117 ++------
 components/zotero-service.js                  |   1 -
 test/tests/collectionTreeViewTest.js          |   2 +
 6 files changed, 44 insertions(+), 372 deletions(-)
 delete mode 100644 chrome/content/zotero/xpcom/data/relation.js

diff --git a/chrome/content/zotero/xpcom/data/creators.js b/chrome/content/zotero/xpcom/data/creators.js
index 7290f52146..e65d6e0507 100644
--- a/chrome/content/zotero/xpcom/data/creators.js
+++ b/chrome/content/zotero/xpcom/data/creators.js
@@ -28,7 +28,7 @@ Zotero.Creators = new function() {
 	this.fields = ['firstName', 'lastName', 'fieldMode'];
 	this.totes = 0;
 	
-	var _creatorCache = {};
+	var _cache = {};
 	
 	/*
 	 * Returns creator data in internal format for a given creatorID
@@ -38,8 +38,8 @@ Zotero.Creators = new function() {
 			throw new Error("creatorID not provided");
 		}
 		
-		if (_creatorCache[creatorID]) {
-			return this.cleanData(_creatorCache[creatorID]);
+		if (_cache[creatorID]) {
+			return this.cleanData(_cache[creatorID]);
 		}
 		
 		var sql = "SELECT * FROM creators WHERE creatorID=?";
@@ -47,7 +47,7 @@ Zotero.Creators = new function() {
 		if (!row) {
 			throw new Error("Creator " + creatorID + " not found");
 		}
-		return _creatorCache[creatorID] = this.cleanData({
+		return _cache[creatorID] = this.cleanData({
 			firstName: row.firstName, // avoid "DB column 'name' not found" warnings from the DB row Proxy
 			lastName: row.lastName,
 			fieldMode: row.fieldMode
@@ -128,7 +128,7 @@ Zotero.Creators = new function() {
 		if (toDelete.length) {
 			// Clear creator entries in internal array
 			for (let i=0; i<toDelete.length; i++) {
-				delete _creatorCache[toDelete[i]];
+				delete _cache[toDelete[i]];
 			}
 			
 			var sql = "DELETE FROM creators WHERE creatorID NOT IN "
diff --git a/chrome/content/zotero/xpcom/data/group.js b/chrome/content/zotero/xpcom/data/group.js
index 181c59e49a..a0cfaf9c5c 100644
--- a/chrome/content/zotero/xpcom/data/group.js
+++ b/chrome/content/zotero/xpcom/data/group.js
@@ -301,34 +301,22 @@ Zotero.Group.prototype.erase = Zotero.Promise.coroutine(function* () {
 			ids = yield Zotero.DB.columnQueryAsync(sql, this.libraryID);
 			for (let i = 0; i < ids.length; i++) {
 				let id = ids[i];
-				let obj = yield Zotero.Items.getAsync(id, { noCache: true });
+				let obj = yield objectsClass.getAsync(id, { noCache: true });
+				// Descendent object may have already been deleted
+				if (!obj) {
+					continue;
+				}
 				yield obj.erase({
 					skipNotifier: true
 				});
 			}
 		}
 		
-		/*// Delete tags
-		sql = "SELECT tagID FROM tags WHERE libraryID=?";
-		ids = yield Zotero.DB.columnQueryAsync(sql, this.libraryID);
-		yield Zotero.Tags.erase(ids);*/
-		
-		// Delete delete log entries
-		sql = "DELETE FROM syncDeleteLog WHERE libraryID=?";
-		yield Zotero.DB.queryAsync(sql, this.libraryID);
-		
 		var prefix = "groups/" + this.id;
 		yield Zotero.Relations.eraseByURIPrefix(Zotero.URI.defaultPrefix + prefix);
 		
-		// Delete settings
-		sql = "DELETE FROM syncedSettings WHERE libraryID=?";
-		yield Zotero.DB.queryAsync(sql, this.libraryID);
-		
-		// Delete group
-		sql = "DELETE FROM groups WHERE groupID=?";
-		yield Zotero.DB.queryAsync(sql, this.id)
-		
-		// Delete library
+		// Delete library row, which deletes from tags, syncDeleteLog, syncedSettings, and groups
+		// tables via cascade. If any of those gain caching, they should be deleted separately.
 		sql = "DELETE FROM libraries WHERE libraryID=?";
 		yield Zotero.DB.queryAsync(sql, this.libraryID)
 		
diff --git a/chrome/content/zotero/xpcom/data/relation.js b/chrome/content/zotero/xpcom/data/relation.js
deleted file mode 100644
index 119f1eb81e..0000000000
--- a/chrome/content/zotero/xpcom/data/relation.js
+++ /dev/null
@@ -1,260 +0,0 @@
-/*
-    ***** BEGIN LICENSE BLOCK *****
-    
-    Copyright © 2009 Center for History and New Media
-                     George Mason University, Fairfax, Virginia, USA
-                     http://zotero.org
-    
-    This file is part of Zotero.
-    
-    Zotero is free software: you can redistribute it and/or modify
-    it under the terms of the GNU Affero General Public License as published by
-    the Free Software Foundation, either version 3 of the License, or
-    (at your option) any later version.
-    
-    Zotero is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU Affero General Public License for more details.
-    
-    You should have received a copy of the GNU Affero General Public License
-    along with Zotero.  If not, see <http://www.gnu.org/licenses/>.
-    
-    ***** END LICENSE BLOCK *****
-*/
-
-Zotero.Relation = function () {
-	this._id = null;
-	this._libraryID = null;
-	this._subject = null;
-	this._predicate = null;
-	this._object = null;
-	this._clientDateModified = null;
-	
-	this._loaded = false;
-}
-
-Zotero.Relation.prototype.__defineGetter__('objectType', function () 'relation');
-Zotero.Relation.prototype.__defineGetter__('id', function () this._id);
-Zotero.Relation.prototype.__defineSetter__('id', function (val) { this._set('id', val); });
-Zotero.Relation.prototype.__defineGetter__('libraryID', function () this._get('libraryID'));
-Zotero.Relation.prototype.__defineSetter__('libraryID', function (val) { return this._set('libraryID', val); });
-Zotero.Relation.prototype.__defineGetter__('key', function () this._id);
-//Zotero.Relation.prototype.__defineSetter__('key', function (val) { this._set('key', val) });
-Zotero.Relation.prototype.__defineGetter__('dateModified', function () this._get('dateModified'));
-Zotero.Relation.prototype.__defineGetter__('subject', function () this._get('subject'));
-Zotero.Relation.prototype.__defineSetter__('subject', function (val) { this._set('subject', val); });
-Zotero.Relation.prototype.__defineGetter__('predicate', function () this._get('predicate'));
-Zotero.Relation.prototype.__defineSetter__('predicate', function (val) { this._set('predicate', val); });
-Zotero.Relation.prototype.__defineGetter__('object', function () this._get('object'));
-Zotero.Relation.prototype.__defineSetter__('object', function (val) { this._set('object', val); });
-
-
-Zotero.Relation.prototype._get = function (field) {
-	if (!this._loaded) {
-		throw new Error("Data not loaded for relation " + this._id);
-	}
-	return this['_' + field];
-}
-
-
-Zotero.Relation.prototype._set = function (field, val) {
-	switch (field) {
-		case 'id':
-		case 'libraryID':
-			if (field == 'libraryID' && !val) {
-				throw new Error("libraryID cannot be empty in Zotero.Relation._set()");
-			}
-			
-			if (val == this['_' + field]) {
-				return;
-			}
-			
-			if (this._loaded) {
-				throw ("Cannot set " + field + " after object is already loaded in Zotero.Relation._set()");
-			}
-			
-			if (field == 'libraryID') {
-				val = parseInt(val);
-			}
-			this['_' + field] = val;
-			return;
-	}
-	
-	if (this.id) {
-		if (!this._loaded) {
-			this.load();
-		}
-	}
-	else {
-		this._loaded = true;
-	}
-	
-	if (this['_' + field] != val) {
-		//this._prepFieldChange(field);
-		
-		switch (field) {
-			default:
-				this['_' + field] = val;
-		}
-	}
-}
-
-
-/**
- * Check if search exists in the database
- *
- * @return	bool			TRUE if the relation exists, FALSE if not
- */
-Zotero.Relation.prototype.exists = Zotero.Promise.coroutine(function* () {
-	if (this.id) {
-		var sql = "SELECT COUNT(*) FROM relations WHERE relationID=?";
-		return !!(yield Zotero.DB.valueQueryAsync(sql, this.id));
-	}
-	
-	if (this.libraryID && this.subject && this.predicate && this.object) {
-		var sql = "SELECT COUNT(*) FROM relations WHERE libraryID=? AND "
-					+ "subject=? AND predicate=? AND object=?";
-		var params = [this.libraryID, this.subject, this.predicate, this.object];
-		return !!(yield Zotero.DB.valueQueryAsync(sql, params));
-	}
-	
-	throw ("ID or libraryID/subject/predicate/object not set in Zotero.Relation.exists()");
-});
-
-
-
-Zotero.Relation.prototype.load = Zotero.Promise.coroutine(function* () {
-	var id = this._id;
-	if (!id) {
-		throw new Error("ID not set");
-	}
-	
-	var sql = "SELECT * FROM relations WHERE ROWID=?";
-	var row = yield Zotero.DB.rowQueryAsync(sql, id);
-	if (!row) {
-		return;
-	}
-	
-	this._libraryID = row.libraryID;
-	this._subject = row.subject;
-	this._predicate = row.predicate;
-	this._object = row.object;
-	this._clientDateModified = row.clientDateModified;
-	this._loaded = true;
-	
-	return true;
-});
-
-
-// TODO: async
-Zotero.Relation.prototype.save = Zotero.Promise.coroutine(function* () {
-	if (this.id) {
-		throw ("Existing relations cannot currently be altered in Zotero.Relation.save()");
-	}
-	
-	if (!this.subject) {
-		throw ("Missing subject in Zotero.Relation.save()");
-	}
-	if (!this.predicate) {
-		throw ("Missing predicate in Zotero.Relation.save()");
-	}
-	if (!this.object) {
-		throw ("Missing object in Zotero.Relation.save()");
-	}
-	
-	Zotero.Relations.editCheck(this);
-	
-	var sql = "INSERT INTO relations "
-				+ "(libraryID, subject, predicate, object, clientDateModified) "
-				+ "VALUES (?, ?, ?, ?, ?)";
-	try {
-		var insertID = yield Zotero.DB.queryAsync(
-			sql,
-			[
-				this.libraryID,
-				this.subject,
-				this.predicate,
-				this.object,
-				Zotero.DB.transactionDateTime
-			]
-		);
-	}
-	catch (e) {
-		// If above failed, try deleting existing row, in case libraryID has changed
-		yield Zotero.DB.executeTransaction(function* () {
-			var sql2 = "SELECT COUNT(*) FROM relations WHERE subject=? AND predicate=? AND object=?";
-			if (yield Zotero.DB.valueQueryAsync(sql2, [this.subject, this.predicate, this.object])) {
-				// Delete
-				sql2 = "DELETE FROM relations WHERE subject=? AND predicate=? AND object=?";
-				yield Zotero.DB.queryAsync(sql2, [this.subject, this.predicate, this.object]);
-				
-				// Insert with original query
-				var insertID = yield Zotero.DB.queryAsync(
-					sql,
-					[
-						this.libraryID,
-						this.subject,
-						this.predicate,
-						this.object,
-						Zotero.DB.transactionDateTime
-					]
-				);
-			}
-		}.bind(this));
-	}
-	return insertID;
-});
-
-
-Zotero.Relation.prototype.erase = Zotero.Promise.coroutine(function* () {
-	if (!this.id) {
-		throw ("ID not set in Zotero.Relation.erase()");
-	}
-	
-	var deleteData = {};
-	deleteData[this.id] = {
-		libraryID: this.libraryID,
-		key: Zotero.Utilities.Internal.md5(this.subject + "_" + this.predicate + "_" + this.object)
-	}
-	
-	var sql = "DELETE FROM relations WHERE ROWID=?";
-	yield Zotero.DB.queryAsync(sql, [this.id]);
-	
-	Zotero.Notifier.trigger('delete', 'relation', [this.id], deleteData);
-});
-
-
-Zotero.Relation.prototype.toXML = function (doc) {
-	var relationXML = doc.createElement('relation');
-	relationXML.setAttribute('libraryID', this.libraryID);
-	
-	var elem = doc.createElement('subject');
-	elem.appendChild(doc.createTextNode(this.subject));
-	relationXML.appendChild(elem);
-	
-	var elem = doc.createElement('predicate');
-	elem.appendChild(doc.createTextNode(this.predicate));
-	relationXML.appendChild(elem);
-	
-	var elem = doc.createElement('object');
-	elem.appendChild(doc.createTextNode(this.object));
-	relationXML.appendChild(elem);
-	
-	return relationXML;
-}
-
-
-Zotero.Relation.prototype.serialize = function () {
-	// Use a hash of the parts as the object key
-	var key = Zotero.Utilities.Internal.md5(this.subject + "_" + this.predicate + "_" + this.object);
-	
-	var obj = {
-		libraryID: this.libraryID,
-		key: key,
-		subject: this.subject,
-		predicate: this.predicate,
-		object: this.object
-	};
-	return obj;
-}
diff --git a/chrome/content/zotero/xpcom/data/relations.js b/chrome/content/zotero/xpcom/data/relations.js
index 3538301135..4ba998e17c 100644
--- a/chrome/content/zotero/xpcom/data/relations.js
+++ b/chrome/content/zotero/xpcom/data/relations.js
@@ -23,25 +23,15 @@
     ***** END LICENSE BLOCK *****
 */
 
-Zotero.Relations = function () {
-	this.constructor = null;
-	
-	this._ZDO_object = 'relation';
-	this._ZDO_idOnly = true;
-	
+Zotero.Relations = new function () {
 	Zotero.defineProperty(this, 'relatedItemPredicate', {value: 'dc:relation'});
 	Zotero.defineProperty(this, 'linkedObjectPredicate', {value: 'owl:sameAs'});
 	Zotero.defineProperty(this, 'deletedItemPredicate', {value: 'dc:isReplacedBy'});
 	
-	this.get = function (id) {
-		if (typeof id != 'number') {
-			throw ("id '" + id + "' must be an integer in Zotero.Relations.get()");
-		}
-		
-		var relation = new Zotero.Relation;
-		relation.id = id;
-		return relation;
-	}
+	this._namespaces = {
+		dc: 'http://purl.org/dc/elements/1.1/',
+		owl: 'http://www.w3.org/2002/07/owl#'
+	};
 	
 	
 	/**
@@ -71,19 +61,15 @@ Zotero.Relations = function () {
 			params.push(object);
 		}
 		var rows = yield Zotero.DB.columnQueryAsync(sql, params);
-		if (!rows) {
-			return [];
-		}
-		
 		var toReturn = [];
-		var loads = [];
 		for (let i=0; i<rows.length; i++) {
-			var relation = new Zotero.Relation;
-			relation.id = rows[i];
-			loads.push(relation.load());
-			toReturn.push(relation);
+			let row = rows[i];
+			toReturn.push({
+				subject: row.subject,
+				predicate: row.predicate,
+				object: row.object
+			});
 		}
-		yield Zotero.Promise.all(loads);
 		return toReturn;
 	});
 	
@@ -124,7 +110,7 @@ Zotero.Relations = function () {
 		
 		yield Zotero.DB.executeTransaction(function* () {
 			var sql = "UPDATE relations SET libraryID=? WHERE libraryID=?";
-			Zotero.DB.query(sql, [toLibraryID, fromLibraryID]);
+			yield Zotero.DB.queryAsync(sql, [toLibraryID, fromLibraryID]);
 			
 			sql = "UPDATE relations SET "
 					+ "subject=REPLACE(subject, 'zotero.org/users/" + fromUserID + "', "
@@ -132,20 +118,16 @@ Zotero.Relations = function () {
 					+ "object=REPLACE(object, 'zotero.org/users/" + fromUserID + "', "
 						+ "'zotero.org/users/" + toUserID + "') "
 						+ "WHERE predicate IN (?, ?)";
-			Zotero.DB.query(sql, [this.linkedObjectPredicate, this.deletedItemPredicate]);
+			yield Zotero.DB.queryAsync(sql, [this.linkedObjectPredicate, this.deletedItemPredicate]);
 		}.bind(this));
 	});
 	
 	
 	this.add = Zotero.Promise.coroutine(function* (libraryID, subject, predicate, object) {
 		predicate = this._getPrefixAndValue(predicate).join(':');
-		
-		var relation = new Zotero.Relation;
-		relation.libraryID = parseInt(libraryID);
-		relation.subject = subject;
-		relation.predicate = predicate;
-		relation.object = object;
-		yield relation.save();
+		var sql = "INSERT INTO relations (libraryID, subject, predicate, object) "
+			+ "VALUES (?, ?, ?, ?)";
+		yield Zotero.DB.queryAsync(sql, [libraryID, subject, predicate, object]);
 	});
 	
 	
@@ -166,13 +148,16 @@ Zotero.Relations = function () {
 	
 	
 	/**
+	 * Deletes relations directly from the DB by URI prefix
+	 *
+	 * This does not update associated objects.
+	 *
 	 * @param {String} prefix
 	 * @param {String[]} ignorePredicates
 	 */
-	this.eraseByURIPrefix = Zotero.Promise.coroutine(function* (prefix, ignorePredicates) {
-		Zotero.DB.requireTransaction();
+	this.eraseByURIPrefix = Zotero.Promise.method(function (prefix, ignorePredicates) {
 		prefix = prefix + '%';
-		var sql = "SELECT ROWID FROM relations WHERE (subject LIKE ? OR object LIKE ?)";
+		var sql = "DELETE FROM relations WHERE (subject LIKE ? OR object LIKE ?)";
 		var params = [prefix, prefix];
 		if (ignorePredicates) {
 			for each(var ignorePredicate in ignorePredicates) {
@@ -180,22 +165,19 @@ Zotero.Relations = function () {
 				params.push(ignorePredicate);
 			}
 		}
-		var ids = yield Zotero.DB.columnQueryAsync(sql, params);
-		
-		for (let i=0; i<ids.length; i++) {
-			let relation = yield this.getAsync(ids[i]);
-			yield relation.load();
-			yield relation.erase();
-		}
+		yield Zotero.DB.queryAsync(sql, params);
 	});
 	
 	
 	/**
+	 * Deletes relations directly from the DB by URI prefix
+	 *
+	 * This does not update associated objects.
+	 *
 	 * @return {Promise}
 	 */
 	this.eraseByURI = Zotero.Promise.coroutine(function* (uri, ignorePredicates) {
-		Zotero.DB.requireTransaction();
-		var sql = "SELECT ROWID FROM relations WHERE (subject=? OR object=?)";
+		var sql = "DELETE FROM relations WHERE (subject=? OR object=?)";
 		var params = [uri, uri];
 		if (ignorePredicates) {
 			for each(var ignorePredicate in ignorePredicates) {
@@ -203,13 +185,7 @@ Zotero.Relations = function () {
 				params.push(ignorePredicate);
 			}
 		}
-		var ids = yield Zotero.DB.columnQueryAsync(sql, params);
-		
-		for (let i=0; i<ids.length; i++) {
-			let relation = yield this.getAsync(ids[i]);
-			yield relation.load();
-			yield relation.erase();
-		}
+		yield Zotero.DB.queryAsync(sql, params);
 	});
 	
 	
@@ -240,35 +216,6 @@ Zotero.Relations = function () {
 		}
 	});
 	
-	
-	this.xmlToRelation = function (relationNode) {
-		var relation = new Zotero.Relation;
-		var libraryID = relationNode.getAttribute('libraryID');
-		if (libraryID) {
-			relation.libraryID = parseInt(libraryID);
-		}
-		else {
-			libraryID = Zotero.Users.getCurrentLibraryID();
-			if (!libraryID) {
-				libraryID = Zotero.Users.getLocalUserKey();
-			}
-			relation.libraryID = parseInt(libraryID);
-		}
-		
-		var elems = Zotero.Utilities.xpath(relationNode, 'subject');
-		relation.subject = elems.length ? elems[0].textContent : "";
-		var elems = Zotero.Utilities.xpath(relationNode, 'predicate');
-		relation.predicate = elems.length ? elems[0].textContent : "";
-		var elems = Zotero.Utilities.xpath(relationNode, 'object');
-		relation.object = elems.length ? elems[0].textContent : "";
-		return relation;
-	}
-	
-	this._namespaces = {
-		dc: 'http://purl.org/dc/elements/1.1/',
-		owl: 'http://www.w3.org/2002/07/owl#'
-	};
-	
 	this._getPrefixAndValue = function(uri) {
 		var [prefix, value] = uri.split(':');
 		if (prefix && value) {
@@ -286,8 +233,4 @@ Zotero.Relations = function () {
 		}
 		throw ("Invalid namespace in URI '" + uri + "' in Zotero.Relations._getPrefixAndValue()");
 	}
-	
-	Zotero.DataObjects.call(this);
-	
-	return this;
-}.bind(Object.create(Zotero.DataObjects.prototype))();
\ No newline at end of file
+}
\ No newline at end of file
diff --git a/components/zotero-service.js b/components/zotero-service.js
index 2da4089189..6eb884c3dc 100644
--- a/components/zotero-service.js
+++ b/components/zotero-service.js
@@ -76,7 +76,6 @@ const xpcomFilesLocal = [
 	'data/groups',
 	'data/itemFields',
 	'data/libraries',
-	'data/relation',
 	'data/relations',
 	'data/tags',
 	'db',
diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js
index 5e3350a468..bc27fad8c5 100644
--- a/test/tests/collectionTreeViewTest.js
+++ b/test/tests/collectionTreeViewTest.js
@@ -262,6 +262,8 @@ describe("Zotero.CollectionTreeView", function() {
 			assert.equal(itemsView.rowCount, 1);
 			var treeRow = itemsView.getRow(0);
 			assert.equal(treeRow.ref.id, ids[0]);
+			
+			yield group.erase();
 		})
 	})
 })