Set 'synced' to false automatically on save, unless value is changed

And add 'skipSyncedUpdate' option to leave untouched

Also move some save logic into Zotero.DataObject.prototype._saveData(),
and call that first.

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

@@ -105,7 +105,10 @@ Zotero.Collection.prototype._set = function (field, value) {
 	
 	if (this['_' + field] != value) {
 		this._markFieldChange(field, this['_' + field]);
-		this._changed.primaryData = true;
+		if (!this._changed.primaryData) {
+			this._changed.primaryData = {};
+		}
+		this._changed.primaryData[field] = true;
 		
 		switch (field) {
 			default:
@@ -286,51 +289,35 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env)
 	var options = env.options;
 
 	var collectionID = env.id = this._id = this.id ? this.id : yield Zotero.ID.get('collections');
-	var libraryID = env.libraryID = this.libraryID || Zotero.Libraries.userLibraryID;
-	var key = env.key = this._key = this.key ? this.key : this._generateKey();
-	var libraryType = env.libraryType = Zotero.Libraries.getType(libraryID);
 	
 	Zotero.debug("Saving collection " + this.id);
 	
-	var columns = [
+	env.sqlColumns.push(
-		'collectionID',
 		'collectionName',
-		'parentCollectionID',
+		'parentCollectionID'
-		'libraryID',
+	);
-		'key',
+	env.sqlValues.push(
-		'version',
-		'synced'
-	];
-	var sqlValues = [
-		collectionID ? { int: collectionID } : null,
 		{ string: this.name },
-		env.parent ? env.parent : null,
+		env.parent ? env.parent : null
-		this.libraryID,
+	);
-		key,
-		this.version ? this.version : 0,
-		this.synced ? 1 : 0
-	];
-	if (isNew || !options.skipClientDateModified) {
-		columns.push('clientDateModified');
-		sqlValues.push(Zotero.DB.transactionDateTime);
-	}
 	
 	if (isNew) {
-		let placeholders = columns.map(function () '?').join();
+		env.sqlColumns.unshift('collectionID');
-		let sql = "INSERT INTO collections (" + columns.join(', ') + ") "
+		env.sqlValues.unshift(collectionID ? { int: collectionID } : null);
+		
+		let placeholders = env.sqlColumns.map(function () '?').join();
+		let sql = "INSERT INTO collections (" + env.sqlColumns.join(', ') + ") "
 			+ "VALUES (" + placeholders + ")";
-		var insertID = yield Zotero.DB.queryAsync(sql, sqlValues);
+		var insertID = yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		if (!collectionID) {
 			collectionID = env.id = insertID;
 		}
 	}
 	else {
-		columns.shift();
-		sqlValues.push(sqlValues.shift());
 		let sql = 'UPDATE collections SET '
-			+ columns.map(function (x) x + '=?').join(', ')
+			+ env.sqlColumns.map(function (x) x + '=?').join(', ') + ' WHERE collectionID=?';
-			+ ' WHERE collectionID=?';
+		env.sqlValues.push(collectionID ? { int: collectionID } : null);
-		yield Zotero.DB.queryAsync(sql, sqlValues);
+		yield Zotero.DB.queryAsync(sql, env.sqlValues);
 	}
 	
 	if (this._changed.parentKey) {

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

@@ -587,6 +587,7 @@ Zotero.DataObject.prototype.save = Zotero.Promise.coroutine(function* (options)
 		// Create transaction
 		if (env.options.tx) {
 			let result = yield Zotero.DB.executeTransaction(function* () {
+				Zotero.DataObject.prototype._saveData.call(this, env);
 				yield this._saveData(env);
 				yield Zotero.DataObject.prototype._finalizeSave.call(this, env);
 				return this._finalizeSave(env);
@@ -596,6 +597,7 @@ Zotero.DataObject.prototype.save = Zotero.Promise.coroutine(function* (options)
 		// Use existing transaction
 		else {
 			Zotero.DB.requireTransaction();
+			Zotero.DataObject.prototype._saveData.call(this, env);
 			yield this._saveData(env);
 			yield Zotero.DataObject.prototype._finalizeSave.call(this, env);
 			return this._finalizeSave(env);
@@ -662,8 +664,38 @@ Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env)
 	return true;
 });
 
-Zotero.DataObject.prototype._saveData = function () {
+Zotero.DataObject.prototype._saveData = function (env) {
-	throw new Error("_saveData is an abstract method");
+	var libraryID = env.libraryID = this.libraryID || Zotero.Libraries.userLibraryID;
+	var key = env.key = this._key = this.key ? this.key : this._generateKey();
+	
+	env.sqlColumns = [
+		'libraryID',
+		'key'
+	];
+	env.sqlValues = [
+		libraryID,
+		key
+	];
+	
+	if (this._changed.primaryData && this._changed.primaryData.version) {
+		env.sqlColumns.push('version');
+		env.sqlValues.push(this.version || 0);
+	}
+	
+	if (this._changed.primaryData && this._changed.primaryData.synced) {
+		env.sqlColumns.push('synced');
+		env.sqlValues.push(this.synced ? 1 : 0);
+	}
+	// Set synced to 0 by default
+	else if (!env.isNew && !env.options.skipSyncedUpdate) {
+		env.sqlColumns.push('synced');
+		env.sqlValues.push(0);
+	}
+	
+	if (env.isNew || !env.options.skipClientDateModified) {
+		env.sqlColumns.push('clientDateModified');
+		env.sqlValues.push(Zotero.DB.transactionDateTime);
+	}
 };
 
 Zotero.DataObject.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) {

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

@@ -1181,14 +1181,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	
 	var isNew = env.isNew;
 	var options = env.options;
+	var libraryType = env.libraryType = Zotero.Libraries.getType(env.libraryID);
 	
 	var itemTypeID = this.itemTypeID;
 	if (!itemTypeID) {
 		throw new Error("Item type must be set before saving");
 	}
 	
-	var sqlColumns = [];
-	var sqlValues = [];
 	var reloadParentChildItems = {};
 	
 	//
@@ -1196,26 +1195,14 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	//
 	// If available id value, use it -- otherwise we'll use autoincrement
 	var itemID = env.id = this._id = this.id ? this.id : yield Zotero.ID.get('items');
-	var libraryID = env.libraryID = this.libraryID || Zotero.Libraries.userLibraryID;
-	var key = env.key = this._key = this.key ? this.key : this._generateKey();
-	var libraryType = env.libraryType = Zotero.Libraries.getType(libraryID);
 	
-	sqlColumns.push(
+	env.sqlColumns.push(
 		'itemTypeID',
-		'dateAdded',
+		'dateAdded'
-		'libraryID',
-		'key',
-		'version',
-		'synced'
 	);
-	
+	env.sqlValues.push(
-	sqlValues.push(
 		{ int: itemTypeID },
-		this.dateAdded ? this.dateAdded : Zotero.DB.transactionDateTime,
+		this.dateAdded ? this.dateAdded : Zotero.DB.transactionDateTime
-		this.libraryID,
-		key,
-		this.version ? this.version : 0,
-		this.synced ? 1 : 0
 	);
 	
 	// If a new item and Date Modified hasn't been provided, or an existing item and
@@ -1224,29 +1211,24 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	if (!this.dateModified
 			|| ((!this._changed.primaryData || !this._changed.primaryData.dateModified)
 				&& !options.skipDateModifiedUpdate)) {
-		sqlColumns.push('dateModified');
+		env.sqlColumns.push('dateModified');
-		sqlValues.push(Zotero.DB.transactionDateTime);
+		env.sqlValues.push(Zotero.DB.transactionDateTime);
 	}
 	// Otherwise, if a new Date Modified was provided, use that. (This would also work when
 	// skipDateModifiedUpdate was passed and there's an existing value, but in that case we
 	// can just not change the field at all.)
 	else if (this._changed.primaryData && this._changed.primaryData.dateModified) {
-		sqlColumns.push('dateModified');
+		env.sqlColumns.push('dateModified');
-		sqlValues.push(this.dateModified);
+		env.sqlValues.push(this.dateModified);
-	}
-	
-	if (isNew || !options.skipClientDateModifiedUpdate) {
-		sqlColumns.push('clientDateModified');
-		sqlValues.push(Zotero.DB.transactionDateTime);
 	}
 	
 	if (isNew) {
-		sqlColumns.unshift('itemID');
+		env.sqlColumns.unshift('itemID');
-		sqlValues.unshift(parseInt(itemID));
+		env.sqlValues.unshift(parseInt(itemID));
 		
-		let sql = "INSERT INTO items (" + sqlColumns.join(", ") + ") "
+		let sql = "INSERT INTO items (" + env.sqlColumns.join(", ") + ") "
-			+ "VALUES (" + sqlValues.map(function () "?").join() + ")";
+			+ "VALUES (" + env.sqlValues.map(function () "?").join() + ")";
-		var insertID = yield Zotero.DB.queryAsync(sql, sqlValues);
+		var insertID = yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		if (!itemID) {
 			itemID = env.id = insertID;
 		}
@@ -1256,9 +1238,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		}
 	}
 	else {
-		let sql = "UPDATE items SET " + sqlColumns.join("=?, ") + "=? WHERE itemID=?";
+		let sql = "UPDATE items SET " + env.sqlColumns.join("=?, ") + "=? WHERE itemID=?";
-		sqlValues.push(parseInt(itemID));
+		env.sqlValues.push(parseInt(itemID));
-		yield Zotero.DB.queryAsync(sql, sqlValues);
+		yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		
 		if (!env.options.skipNotifier) {
 			Zotero.Notifier.trigger('modify', 'item', itemID, env.notifierData);

chrome/content/zotero/xpcom/search.js

@@ -145,48 +145,32 @@ Zotero.Search.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	var options = env.options;
 	
 	var searchID = env.id = this._id = this.id ? this.id : yield Zotero.ID.get('savedSearches');
-	var libraryID = env.libraryID = this.libraryID || Zotero.Libraries.userLibraryID;
-	var key = env.key = this._key = this.key ? this.key : this._generateKey();
-	var libraryType = env.libraryType = Zotero.Libraries.getType(libraryID);
 	
-	var columns = [
+	env.sqlColumns.push(
-		'savedSearchID',
 		'savedSearchName',
-		'libraryID',
+		'savedSearchID'
-		'key',
+	);
-		'version',
+	env.sqlValues.push(
-		'synced'
+		{ string: this.name }
-	];
+	);
-	var placeholders = columns.map(function () '?').join();
-	var sqlValues = [
-		searchID ? { int: searchID } : null,
-		{ string: this.name },
-		this.libraryID,
-		key,
-		this.version ? this.version : 0,
-		this.synced ? 1 : 0
-	];
-	if (isNew || !options.skipClientDateModified) {
-		columns.push('clientDateModified');
-		sqlValues.push(Zotero.DB.transactionDateTime);
-	}
 	
 	if (isNew) {
-		let placeholders = columns.map(function () '?').join();
+		env.sqlColumns.unshift('savedSearchID');
-		let sql = "INSERT INTO savedSearches (" + columns.join(', ') + ") "
+		env.sqlValues.unshift(searchID ? { int: searchID } : null);
+		
+		let placeholders = env.sqlColumns.map(function () '?').join();
+		let sql = "INSERT INTO savedSearches (" + env.sqlColumns.join(', ') + ") "
 			+ "VALUES (" + placeholders + ")";
-		var insertID = yield Zotero.DB.queryAsync(sql, sqlValues);
+		var insertID = yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		if (!searchID) {
 			searchID = env.id = insertID;
 		}
 	}
 	else {
-		columns.shift();
-		sqlValues.push(sqlValues.shift());
 		let sql = 'UPDATE savedSearches SET '
-			+ columns.map(function (x) x + '=?').join(', ')
+			+ env.sqlColumns.map(function (x) x + '=?').join(', ') + ' WHERE savedSearchID=?';
-			+ ' WHERE savedSearchID=?';
+		env.sqlValues.push(searchID ? { int: searchID } : null);
-		yield Zotero.DB.queryAsync(sql, sqlValues);
+		yield Zotero.DB.queryAsync(sql, env.sqlValues);
 	}
 	
 	if (!isNew) {

test/tests/dataObjectTest.js

@@ -29,6 +29,62 @@ describe("Zotero.DataObject", function() {
 		})
 	})
 	
+	describe("#synced", 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);
+			yield Zotero.Items.erase(id);
+		});
+		
+		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.save();
+			assert.ok(item.synced);
+			
+			yield Zotero.Items.erase(id);
+		});
+		
+		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.save();
+			
+			yield item.loadItemData();
+			item.setField('title', 'test');
+			yield item.save();
+			assert.isFalse(item.synced);
+			
+			yield Zotero.Items.erase(id);
+		});
+		
+		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.save();
+			
+			yield item.loadItemData();
+			item.setField('title', 'test');
+			yield item.save({
+				skipSyncedUpdate: true
+			});
+			assert.ok(item.synced);
+			
+			yield Zotero.Items.erase(id);
+		});
+	})
+	
 	describe("#save()", function () {
 		it("should add new identifiers to cache", function* () {
 			// Collection