Feed syncing (#1044)

Closes #1036

Also:

- Store sync info for feeds more compactly. Address #1037

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

@@ -70,7 +70,6 @@ Zotero.Feed = function(params = {}) {
 	this._feedUnreadCount = null;
 	
 	this._updating = false;
-	this._syncedSettings = null;
 	this._previousURL = null;
 }
 
@@ -301,8 +300,8 @@ Zotero.Feed.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) {
 	}
 	if (env.isNew || this._previousURL) {
 		Zotero.Feeds.register(this);
-		yield this.storeSyncedSettings();
 	}
+	yield this.storeSyncedSettings();
 	this._previousURL = null;
 	
 });
@@ -335,12 +334,7 @@ Zotero.Feed.prototype.erase = Zotero.Promise.coroutine(function* (options = {})
 
 Zotero.Feed.prototype.storeSyncedSettings = Zotero.Promise.coroutine(function* () {
 	let syncedFeeds = Zotero.SyncedSettings.get(Zotero.Libraries.userLibraryID, 'feeds') || {};
-	syncedFeeds[this.url] = {
+	syncedFeeds[this.url] = [this.name, this.cleanupAfter, this.refreshInterval];
-		url: this.url,
-		name: this.name,
-		cleanupAfter: this.cleanupAfter,
-		refreshInterval: this.refreshInterval
-	};
 	return Zotero.SyncedSettings.set(Zotero.Libraries.userLibraryID, 'feeds', syncedFeeds);
 });
 

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

@@ -30,6 +30,18 @@ Zotero.Feeds = new function() {
 			this.scheduleNextFeedCheck();
 			this._timeoutID = null;
 		}, 5000);
+		
+		Zotero.SyncedSettings.onSyncDownload.addListener(Zotero.Libraries.userLibraryID, 'feeds', 
+			(oldValue, newValue, conflict) => { 
+				Zotero.Feeds.restoreFromJSON(newValue, conflict);
+			}
+		);
+		
+		Zotero.Notifier.registerObserver({notify: function(event) {
+			if (event == 'finish') {
+				Zotero.Feeds.updateFeeds();
+			}
+		}}, ['sync'], 'feedsUpdate');
 	};
 	
 	this.uninit = function () {
@@ -126,37 +138,37 @@ Zotero.Feeds = new function() {
 		Zotero.debug(json);
 		if (merge) {
 			let syncedFeeds = Zotero.SyncedSettings.get(Zotero.Libraries.userLibraryID, 'feeds');
+			// Overwrite with remote values for names, etc.
 			for (let url in json) {
-				if (syncedFeeds[url]) {
+				syncedFeeds[url] = json[url];
-					syncedFeeds[url].name = json[url].name;
-					syncedFeeds[url].cleanupAfter = json[url].cleanupAfter;
-					syncedFeeds[url].refreshInterval = json[url].refreshInterval;
-					for (let guid in json[url].markedAsRead) {
-						syncedFeeds[url].markedAsRead[guid] = true;
-					}
-				} else {
-					syncedFeeds[url] = json[url];
-				}
 			}
+			// But keep all local feeds
 			json = syncedFeeds;
 		}
+		json = this._compactifyFeedJSON(json);
 		yield Zotero.SyncedSettings.set(Zotero.Libraries.userLibraryID, 'feeds', json);
 		let feeds = Zotero.Feeds.getAll();
 		for (let feed of feeds) {
 			if (json[feed.url]) {
 				Zotero.debug("Feed " + feed.url + " exists remotely and locally");
-				delete json[feed.url];
+				feed.name = json[feed.url][0];
+				feed.cleanupAfter = json[feed.url][1];
+				feed.refreshInterval = json[feed.url][2];
 			} else {
 				Zotero.debug("Feed " + feed.url + " does not exist in remote JSON. Deleting");
-				yield feed.eraseTx();
+				yield feed.erase();
 			}
 		}
 		// Because existing json[feed.url] got deleted, `json` now only contains new feeds
 		for (let url in json) {
 			Zotero.debug("Feed " + url + " exists remotely but not locally. Creating");
-			let feed = new Zotero.Feed(json[url]);
+			let feed = new Zotero.Feed({
-			yield feed.saveTx();
+				url, 
-			yield feed.updateFeed();
+				name: json[url][0], 
+				cleanupAfter: json[url][1], 
+				refreshInterval: json[url[2]]
+			});
+			yield feed.save();
 		}
 	});
 	
@@ -259,4 +271,16 @@ Zotero.Feeds = new function() {
 		Zotero.debug("All feed updates done");
 		this.scheduleNextFeedCheck();
 	});
+	
+	// Conversion from expansive to compact format sync json
+	// TODO: Remove after beta
+	this._compactifyFeedJSON = function(json) {
+		for (let url in json) {
+			if(Array.isArray(json[url])) {
+				continue;
+			}
+			json[url] = [json[url].name, json[url].cleanupAfter, json[url].refreshInterval];
+		}
+		return json;
+	};
 }

chrome/content/zotero/xpcom/notifier.js

@@ -30,12 +30,19 @@ Zotero.Notifier = new function(){
 	var _types = [
 		'collection', 'search', 'share', 'share-items', 'item', 'file',
 		'collection-item', 'item-tag', 'tag', 'setting', 'group', 'trash', 'publications',
-		'bucket', 'relation', 'feed', 'feedItem'
+		'bucket', 'relation', 'feed', 'feedItem', 'sync'
 	];
 	var _inTransaction;
 	var _queue = {};
 
 
+	/**
+	 * @param ref {Object} - signature {notify: function(event, type, ids, extraData) {}}
+	 * @param types {Array} - a list of types of events observer should be triggered on
+	 * @param id {String} - an id of the observer used in debug output
+	 * @param priority {Integer} - lower numbers correspond to higher priority of observer execution
+	 * @returns {string}
+	 */
 	this.registerObserver = function (ref, types, id, priority) {
 		if (types){
 			types = Zotero.flattenArguments(types);

chrome/content/zotero/xpcom/sync/syncRunner.js

@@ -149,7 +149,7 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 				firstInSession: _firstInSession
 			};
 			
-			let librariesToSync = options.libraries = yield this.checkLibraries(
+			var librariesToSync = options.libraries = yield this.checkLibraries(
 				client,
 				options,
 				keyInfo,
@@ -223,6 +223,7 @@ Zotero.Sync.Runner_Module = function (options = {}) {
 			}
 			
 			Zotero.debug("Done syncing");
+			Zotero.Notifier.trigger('finish', 'sync', librariesToSync);
 		}
 	});
 	

chrome/content/zotero/xpcom/syncedSettings.js

@@ -36,6 +36,40 @@ Zotero.SyncedSettings = (function () {
 		idColumn: "setting",
 		table: "syncedSettings",
 
+		/**
+		 * An event which allows to tap into the sync transaction and update
+		 * parts of the client which rely on synced settings.
+		 */
+		onSyncDownload: {
+			listeners: {},
+			addListener: function(libraryID, setting, fn, bindTarget=null) {
+				if (!this.listeners[libraryID]) {
+					this.listeners[libraryID] = {};
+				}
+				if (!this.listeners[libraryID][setting]) {
+					this.listeners[libraryID][setting] = [];
+				}
+				this.listeners[libraryID][setting].push([fn, bindTarget]);
+			},
+			/**
+			 * @param {Integer} libraryID
+			 * @param {String} setting - name of the setting
+			 * @param {Object} oldValue
+			 * @param {Object} newValue
+			 * @param {Boolean} conflict - true if both client and remote values had changed before sync
+			 */
+			trigger: Zotero.Promise.coroutine(function* (libraryID, setting, oldValue, newValue, conflict) {
+				var libListeners = this.listeners[libraryID] || {};
+				var settingListeners = libListeners[setting] || [];
+				Array.prototype.splice.call(arguments, 0, 2);
+				if (settingListeners) {
+					for (let listener of settingListeners) {
+						yield Zotero.Promise.resolve(listener[0].apply(listener[1], arguments));
+					}
+				}
+			})
+		},
+		
 		loadAll: Zotero.Promise.coroutine(function* (libraryID) {
 			Zotero.debug("Loading synced settings for library " + libraryID);
 			
@@ -175,11 +209,13 @@ Zotero.SyncedSettings = (function () {
 			version = parseInt(version);
 			
 			if (hasCurrentValue) {
-				var sql = "UPDATE syncedSettings SET value=?, version=?, synced=? "
+				var sql = "UPDATE syncedSettings SET " + (version > 0 ? "version=?, " : "") + 
-					+ "WHERE setting=? AND libraryID=?";
+					"value=?, synced=? WHERE setting=? AND libraryID=?";
-				yield Zotero.DB.queryAsync(
+				var args = [JSON.stringify(value), synced, setting, libraryID];
-					sql, [JSON.stringify(value), version, synced, setting, libraryID]
+				if (version > 0) {
-				);
+					args.unshift(version)
+				}
+				yield Zotero.DB.queryAsync(sql, args);
 			}
 			else {
 				var sql = "INSERT INTO syncedSettings "
@@ -189,12 +225,18 @@ Zotero.SyncedSettings = (function () {
 				);
 			}
 
+			var metadata = this.getMetadata(libraryID, setting);
+			
 			_cache[libraryID][setting] = {
 				value,
 				synced: !!synced,
-				version
+				version: version > 0 ? version : metadata.version
-			}
+			};
 			
+			var conflict = metadata && !metadata.synced && metadata.version < version;
+			if (version > 0) {
+				yield this.onSyncDownload.trigger(libraryID, setting, currentValue, value, conflict);
+			}
 			yield Zotero.Notifier.trigger(event, 'setting', [id], extraData);
 			return true;
 		}),

test/tests/feedTest.js

@@ -200,17 +200,18 @@ describe("Zotero.Feed", function() {
 	});
 	
 	describe("#storeSyncedSettings", function() {
-		it("should store updated settings for the feed", function* () {
+		it("should store settings for feed in compact format", function* () {
-			let settings = { 
+			let url = 'http://' + Zotero.Utilities.randomString().toLowerCase() + '.com/feed.rss';
-				name: Zotero.Utilities.randomString(), 
+			let settings = [Zotero.Utilities.randomString(), 1, 1];
-				url: 'http://' + Zotero.Utilities.randomString().toLowerCase() + '.com/feed.rss', 
+			let feed = yield createFeed({
-				refreshInterval: 1,
+				url,
-				cleanupAfter: 1
+				name: settings[0],
-			};
+				cleanupAfter: settings[1],
-			let feed = yield createFeed(settings);
+				refreshInterval: settings[2]
+			});
 			
 			let syncedFeeds = Zotero.SyncedSettings.get(Zotero.Libraries.userLibraryID, 'feeds');
-			assert.deepEqual(syncedFeeds[feed.url], settings);
+			assert.deepEqual(syncedFeeds[url], settings);
 		});
 	});
 	

test/tests/feedsTest.js

@@ -47,14 +47,6 @@ describe("Zotero.Feeds", function () {
 		var json = {};
 		var expiredFeedURL, existingFeedURL;
 		
-		before(function() {
-			sinon.stub(Zotero.Feed.prototype, 'updateFeed').resolves();
-		});
-		
-		after(function() {
-			Zotero.Feed.prototype.updateFeed.restore();
-		});
-		
 		beforeEach(function* () {
 			yield clearFeeds();
 		
@@ -65,7 +57,6 @@ describe("Zotero.Feeds", function () {
 					name: Zotero.Utilities.randomString(),
 					refreshInterval: 5,
 					cleanupAfter: 3,
-					markedAsRead: []
 				};
 				if (i == 0) {
 					existingFeedURL = url;