From dd8fd2b1ac134e99ecfd44649fb4ec331d08e20a Mon Sep 17 00:00:00 2001 From: Adomas Ven Date: Wed, 22 Jun 2016 10:24:22 +0300 Subject: [PATCH] Feed syncing (#1044) Closes #1036 Also: - Store sync info for feeds more compactly. Address #1037 --- chrome/content/zotero/xpcom/data/feed.js | 10 +--- chrome/content/zotero/xpcom/data/feeds.js | 54 +++++++++++++----- chrome/content/zotero/xpcom/notifier.js | 13 ++++- .../content/zotero/xpcom/sync/syncRunner.js | 3 +- chrome/content/zotero/xpcom/syncedSettings.js | 56 ++++++++++++++++--- test/tests/feedTest.js | 19 ++++--- test/tests/feedsTest.js | 9 --- 7 files changed, 112 insertions(+), 52 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/feed.js b/chrome/content/zotero/xpcom/data/feed.js index 09a05efa0..72c642f07 100644 --- a/chrome/content/zotero/xpcom/data/feed.js +++ b/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] = { - url: this.url, - name: this.name, - cleanupAfter: this.cleanupAfter, - refreshInterval: this.refreshInterval - }; + syncedFeeds[this.url] = [this.name, this.cleanupAfter, this.refreshInterval]; return Zotero.SyncedSettings.set(Zotero.Libraries.userLibraryID, 'feeds', syncedFeeds); }); diff --git a/chrome/content/zotero/xpcom/data/feeds.js b/chrome/content/zotero/xpcom/data/feeds.js index d34173683..12243569a 100644 --- a/chrome/content/zotero/xpcom/data/feeds.js +++ b/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].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]; - } + 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]); - yield feed.saveTx(); - yield feed.updateFeed(); + let feed = new Zotero.Feed({ + url, + 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; + }; } diff --git a/chrome/content/zotero/xpcom/notifier.js b/chrome/content/zotero/xpcom/notifier.js index 99de5cb5a..e1101ece0 100644 --- a/chrome/content/zotero/xpcom/notifier.js +++ b/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); diff --git a/chrome/content/zotero/xpcom/sync/syncRunner.js b/chrome/content/zotero/xpcom/sync/syncRunner.js index 250ee1a81..c99fa8f35 100644 --- a/chrome/content/zotero/xpcom/sync/syncRunner.js +++ b/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); } }); diff --git a/chrome/content/zotero/xpcom/syncedSettings.js b/chrome/content/zotero/xpcom/syncedSettings.js index e80c08b4f..ab83f9756 100644 --- a/chrome/content/zotero/xpcom/syncedSettings.js +++ b/chrome/content/zotero/xpcom/syncedSettings.js @@ -35,6 +35,40 @@ Zotero.SyncedSettings = (function () { var module = { 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=? " - + "WHERE setting=? AND libraryID=?"; - yield Zotero.DB.queryAsync( - sql, [JSON.stringify(value), version, synced, setting, libraryID] - ); + var sql = "UPDATE syncedSettings SET " + (version > 0 ? "version=?, " : "") + + "value=?, synced=? WHERE setting=? AND libraryID=?"; + var args = [JSON.stringify(value), synced, setting, libraryID]; + if (version > 0) { + args.unshift(version) + } + yield Zotero.DB.queryAsync(sql, args); } else { var sql = "INSERT INTO syncedSettings " @@ -188,13 +224,19 @@ Zotero.SyncedSettings = (function () { sql, [setting, libraryID, JSON.stringify(value), version, synced] ); } + + 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; }), diff --git a/test/tests/feedTest.js b/test/tests/feedTest.js index 194066b8c..ba06a6c86 100644 --- a/test/tests/feedTest.js +++ b/test/tests/feedTest.js @@ -200,17 +200,18 @@ describe("Zotero.Feed", function() { }); describe("#storeSyncedSettings", function() { - it("should store updated settings for the feed", function* () { - let settings = { - name: Zotero.Utilities.randomString(), - url: 'http://' + Zotero.Utilities.randomString().toLowerCase() + '.com/feed.rss', - refreshInterval: 1, - cleanupAfter: 1 - }; - let feed = yield createFeed(settings); + it("should store settings for feed in compact format", function* () { + let url = 'http://' + Zotero.Utilities.randomString().toLowerCase() + '.com/feed.rss'; + let settings = [Zotero.Utilities.randomString(), 1, 1]; + let feed = yield createFeed({ + url, + name: settings[0], + cleanupAfter: settings[1], + refreshInterval: settings[2] + }); let syncedFeeds = Zotero.SyncedSettings.get(Zotero.Libraries.userLibraryID, 'feeds'); - assert.deepEqual(syncedFeeds[feed.url], settings); + assert.deepEqual(syncedFeeds[url], settings); }); }); diff --git a/test/tests/feedsTest.js b/test/tests/feedsTest.js index 7413ca7a5..d7c0c43f6 100644 --- a/test/tests/feedsTest.js +++ b/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;