Update feeds on metadata change fixes #1969

addon/ActivityStreams.js

@@ -118,7 +118,7 @@ ActivityStreams.prototype = {
     }
     this._tabTracker.init(this.appURLs, this._experimentProvider.experimentId, this._store);
     this._searchProvider.init();
-    this._initializePreviewProvider(this._metadataStore, this._tabTracker);
+    this._initializePreviewProvider(this._metadataStore, this._tabTracker, this._store);
     this._initializePageScraper(this._previewProvider, this._tabTracker);
     this._initializeShareProvider(this._tabTracker);
     this._initializePrefProvider(this._tabTracker);
@@ -189,8 +189,8 @@ ActivityStreams.prototype = {
     this._appURLHider = new AppURLHider(this.appURLs);
   },
 
-  _initializePreviewProvider(metadataStore, tabTracker) {
-    this._previewProvider = new PreviewProvider(tabTracker, metadataStore);
+  _initializePreviewProvider(metadataStore, tabTracker, store) {
+    this._previewProvider = new PreviewProvider(tabTracker, metadataStore, store);
   },
 
   _initializePrefProvider(tabTracker) {

addon/Feeds/HighlightsFeed.js

@@ -12,7 +12,6 @@ module.exports = class HighlightsFeed extends Feed {
   constructor(options) {
     super(options);
     this.baselineRecommender = null; // Added in initializeRecommender, if the experiment is turned on
-    this.retryHighlightsCount = 0;
   }
 
   /**
@@ -60,14 +59,8 @@ module.exports = class HighlightsFeed extends Feed {
     }
     return PlacesProvider.links.getRecentlyVisited()
       .then(links => this.options.getCachedMetadata(links, "HIGHLIGHTS_RESPONSE"))
-      .then(links => ({metadataLinks: links, weightedLinks: this.baselineRecommender.scoreEntries(links)}))
-      .then(({metadataLinks, weightedLinks}) => {
-        if (metadataLinks.length && !weightedLinks.length && this.retryHighlightsCount <= 5) {
-          this.retryHighlightsCount++;
-          return am.actions.Response("HIGHLIGHTS_AWAITING_METADATA");
-        }
-        return am.actions.Response("HIGHLIGHTS_RESPONSE", weightedLinks);
-      });
+      .then(links => this.baselineRecommender.scoreEntries(links))
+      .then(links => am.actions.Response("HIGHLIGHTS_RESPONSE", links));
   }
 
   onAction(state, action) {
@@ -80,10 +73,7 @@ module.exports = class HighlightsFeed extends Feed {
         // We always want new bookmarks
         this.refresh("a bookmark was added");
         break;
-      case am.type("HIGHLIGHTS_AWAITING_METADATA"):
-        this.refresh("metadata for highlights is being fetched");
-        break;
-      case am.type("METADATA_FEED_UPDATED"):
+      case am.type("METADATA_UPDATED"):
         // If the user visits a site and we don't have enough weighted highlights yet, refresh the data.
         if (state.Highlights.rows.length < (HIGHLIGHTS_LENGTH + TOP_SITES_LENGTH)) {
           this.refresh("there were not enough sites");

addon/Feeds/MetadataFeed.js

@@ -33,9 +33,9 @@ module.exports = class MetadataFeed extends Feed {
 
     // if we are in the experiment, make a network request through PageScraper
     if (simplePrefs.prefs["experiments.locallyFetchMetadata20"]) {
-      return this.options.fetchNewMetadataLocally(links, "METADATA_FEED_REQUEST").then(() => (am.actions.Response("METADATA_FEED_UPDATED")));
+      return this.options.fetchNewMetadataLocally(links, "METADATA_FEED_REQUEST").then(() => (am.actions.Response("METADATA_UPDATED")));
     }
-    return this.options.fetchNewMetadata(links, "METADATA_FEED_REQUEST").then(() => (am.actions.Response("METADATA_FEED_UPDATED")));
+    return this.options.fetchNewMetadata(links, "METADATA_FEED_REQUEST").then(() => (am.actions.Response("METADATA_UPDATED")));
   }
   onAction(state, action) {
     switch (action.type) {

addon/PreviewProvider.js

@@ -36,12 +36,13 @@ const DEFAULT_OPTIONS = {
   initFresh: false
 };
 
-function PreviewProvider(tabTracker, metadataStore, options = {}) {
+function PreviewProvider(tabTracker, metadataStore, store, options = {}) {
   this.options = Object.assign({}, DEFAULT_OPTIONS, options);
   this._onPrefChange = this._onPrefChange.bind(this);
   this._tippyTopProvider = new TippyTopProvider();
   this._tabTracker = tabTracker;
   this._metadataStore = metadataStore;
+  this._store = store;
   this.init();
 }
 
@@ -366,7 +367,9 @@ PreviewProvider.prototype = {
       expired_at: (this.options.metadataTTL) + Date.now(),
       metadata_source: metadataSource
     }));
-    this._metadataStore.asyncInsert(linksToInsert, true).catch(err => {
+    return this._metadataStore.asyncInsert(linksToInsert, true).then(() => {
+      this._store.dispatch({type: "METADATA_UPDATED"});
+    }).catch(err => {
       // TODO: add more exception handling code, e.g. sending exception report
       Cu.reportError(err);
     });
@@ -378,7 +381,7 @@ PreviewProvider.prototype = {
    */
   processAndInsertMetadata(link, metadataSource) {
     const processedLink = this._processLinks([link]);
-    this.insertMetadata(processedLink, metadataSource);
+    return this.insertMetadata(processedLink, metadataSource);
   },
 
   /**

common/action-manager.js

@@ -14,7 +14,7 @@ const am = new ActionManager([
   "LOCALE_UPDATED",
   "MANY_LINKS_CHANGED",
   "MERGE_STORE",
-  "METADATA_FEED_UPDATED",
+  "METADATA_UPDATED",
   "NOTIFY_BLOCK_URL",
   "NOTIFY_BOOKMARK_ADD",
   "NOTIFY_BOOKMARK_DELETE",

content-test/addon/Feeds/HighlightsFeed.test.js

@@ -99,30 +99,6 @@ describe("HighlightsFeed", () => {
           assert.lengthOf(action.data, 2);
         });
     });
-    it("should resolve with a HIGHLIGHTS_AWAITING_METADATA action if there are no weighted highlights", () => {
-      instance.baselineRecommender = {scoreEntries: () => []};
-      return instance.getData()
-        .then(action => {
-          assert.isObject(action);
-          assert.equal(action.type, "HIGHLIGHTS_AWAITING_METADATA");
-        });
-    });
-    it("should resolve with a HIGHLIGHTS_RESPONSE action if we've tried more than 5 times", () => {
-      instance.retryHighlightsCount = 5;
-      instance.baselineRecommender = {scoreEntries: () => []};
-      return instance.getData()
-        .then(action => {
-          assert.isObject(action);
-          assert.equal(action.type, "HIGHLIGHTS_AWAITING_METADATA");
-          assert.equal(instance.retryHighlightsCount, 6);
-          return instance.getData();
-        })
-        .then(action => {
-          assert.isObject(action);
-          assert.equal(action.type, "HIGHLIGHTS_RESPONSE");
-          assert.equal(instance.retryHighlightsCount, 6);
-        });
-    });
     it("should run sites through getCachedMetadata", () => {
       instance.baselineRecommender = {scoreEntries: links => links};
       return instance.getData()
@@ -161,30 +137,30 @@ describe("HighlightsFeed", () => {
       assert.calledOnce(instance.refresh);
       assert.calledWith(instance.refresh, "a bookmark was added");
     });
-    it("should call refresh on METADATA_FEED_UPDATED if there are not enough sites", () => {
+    it("should call refresh on METADATA_UPDATED if there are not enough sites", () => {
       store.state.Highlights = {rows: Array(HIGHLIGHTS_LENGTH + TOP_SITES_LENGTH - 1).fill("site")};
-      instance.onAction(store.getState(), {type: "METADATA_FEED_UPDATED"});
+      instance.onAction(store.getState(), {type: "METADATA_UPDATED"});
       assert.calledOnce(instance.refresh);
       assert.calledWith(instance.refresh, "there were not enough sites");
     });
-    it("should not call refresh on METADATA_FEED_UPDATED if there are enough sites", () => {
+    it("should not call refresh on METADATA_UPDATED if there are enough sites", () => {
       store.state.Highlights = {rows: Array(HIGHLIGHTS_LENGTH + TOP_SITES_LENGTH).fill("site")};
-      instance.onAction(store.getState(), {type: "METADATA_FEED_UPDATED"});
+      instance.onAction(store.getState(), {type: "METADATA_UPDATED"});
       assert.notCalled(instance.refresh);
     });
-    it("should call refresh on METADATA_FEED_UPDATED if .lastUpdated is too old", () => {
+    it("should call refresh on METADATA_UPDATED if .lastUpdated is too old", () => {
       store.state.Highlights = {rows: Array(HIGHLIGHTS_LENGTH + TOP_SITES_LENGTH).fill("site")};
       instance.lastUpdated = 0;
       clock.tick(HighlightsFeed.UPDATE_TIME);
-      instance.onAction(store.getState(), {type: "METADATA_FEED_UPDATED"});
+      instance.onAction(store.getState(), {type: "METADATA_UPDATED"});
       assert.calledOnce(instance.refresh);
       assert.calledWith(instance.refresh, "the sites were too old");
     });
-    it("should not call refresh on METADATA_FEED_UPDATED if .lastUpdated is less than update time", () => {
+    it("should not call refresh on METADATA_UPDATED if .lastUpdated is less than update time", () => {
       store.state.Highlights = {rows: Array(HIGHLIGHTS_LENGTH + TOP_SITES_LENGTH).fill("site")};
       instance.lastUpdated = 0;
       clock.tick(HighlightsFeed.UPDATE_TIME - 1);
-      instance.onAction(store.getState(), {type: "METADATA_FEED_UPDATED"});
+      instance.onAction(store.getState(), {type: "METADATA_UPDATED"});
       assert.notCalled(instance.refresh);
     });
     it("should update the coefficients if the weightedHighlightsCoefficients pref is changed", () => {

content-test/addon/Feeds/MetadataFeed.test.js

@@ -43,7 +43,7 @@ describe("MetadataFeed", () => {
     it("should resolve with an action, but no data", () => (
       instance.getData().then(action => {
         assert.isObject(action);
-        assert.equal(action.type, "METADATA_FEED_UPDATED");
+        assert.equal(action.type, "METADATA_UPDATED");
         assert.isUndefined(action.data);
       })
     ));

test/test-PreviewProvider-MetadataStore.js

@@ -175,6 +175,7 @@ before(exports, function*() {
   yield gMetadataStore.asyncConnect();
   let mockTabTracker = {handlePerformanceEvent() {}, generateEvent() {}};
   gPreviewProvider = new PreviewProvider(mockTabTracker, gMetadataStore, {initFresh: true});
+  gPreviewProvider._store = {dispatch: () => {}};
   gPreviewProvider._getFaviconColors = function() {
     return Promise.resolve(null);
   };

test/test-PreviewProvider.js

@@ -196,18 +196,22 @@ exports.test_process_links = function(assert) {
   });
 };
 
-exports.test_process_and_insert_links = function(assert) {
+exports.test_process_and_insert_links = function*(assert) {
   const fakeData = {"url": "http://example.com/1", "title": "Title for example.com/1"};
 
+  const mockActions = [];
+  gPreviewProvider._store = {dispatch: action => mockActions.push(action)};
+
   // process and insert the links
-  gPreviewProvider.processAndInsertMetadata(fakeData, "metadata_source");
+  yield gPreviewProvider.processAndInsertMetadata(fakeData, "metadata_source");
   assert.equal(gMetadataStore[0].length, 1, "saved one item");
 
   // check the first site inserted in the metadata DB
   assert.equal(gMetadataStore[0][0].url, fakeData.url, "site was saved as expected");
   assert.equal(gMetadataStore[0][0].cache_key, "example.com/1", "we added a cache_key for the site");
   assert.equal(gMetadataStore[0][0].metadata_source, "metadata_source", "we added a metadata_source for the site");
   assert.equal(gMetadataStore[0][0].title, fakeData.title, "we added the title from the metadata for the site");
+  assert.equal(mockActions[0].type, "METADATA_UPDATED");
 };
 
 exports.test_look_for_link_in_DB = function*(assert) {
@@ -438,6 +442,7 @@ exports.test_copy_over_correct_data_from_firefox = function*(assert) {
 exports.test_compute_image_sizes = function*(assert) {
   let mockExperimentProvider = {data: {metadataService: false}};
   gPreviewProvider = new PreviewProvider(gMockTabTracker, gMockMetadataStore, mockExperimentProvider, {initFresh: true});
+  gPreviewProvider._store = {dispatch: () => {}};
   let metadataObj = {
     url: "https://www.hasAnImage.com",
     images: [{url: "data:image;base64,R0lGODlhAQABAAD/ACwAAAAAAQABAAA"}] // a 1x1 pixel image
@@ -463,6 +468,7 @@ before(exports, () => {
   simplePrefs.prefs["metadata.endpoint"] = `${gEndpointPrefix}${gMetadataServiceEndpoint}`;
   simplePrefs.prefs["previews.enabled"] = true;
   gPreviewProvider = new PreviewProvider(gMockTabTracker, gMockMetadataStore, {initFresh: true});
+  gPreviewProvider._store = {dispatch: () => {}};
   gPreviewProvider._getFaviconColors = function() {
     return Promise.resolve(null);
   };