Fixes bug 1518171 - Adding feeds. (#4637)

* Fixes bug 1518171 - Adding feeds.

* Updates to feeds and layout tests, cache clearing, and loading function names.

common/Actions.jsm

@@ -42,6 +42,7 @@ for (const type of [
   "DISCOVERY_STREAM_CONFIG_CHANGE",
   "DISCOVERY_STREAM_CONFIG_SETUP",
   "DISCOVERY_STREAM_CONFIG_SET_VALUE",
+  "DISCOVERY_STREAM_FEEDS_UPDATE",
   "DISCOVERY_STREAM_LAYOUT_RESET",
   "DISCOVERY_STREAM_LAYOUT_UPDATE",
   "DOWNLOAD_CHANGED",

common/Reducers.jsm

@@ -455,6 +455,8 @@ function DiscoveryStream(prevState = INITIAL_STATE.DiscoveryStream, action) {
       return {...prevState, lastUpdated: action.data.lastUpdated || null, layout: action.data.layout || []};
     case at.DISCOVERY_STREAM_LAYOUT_RESET:
       return {...prevState, lastUpdated: INITIAL_STATE.DiscoveryStream.lastUpdated, layout: INITIAL_STATE.DiscoveryStream.layout};
+    case at.DISCOVERY_STREAM_FEEDS_UPDATE:
+      return {...prevState, feeds: action.data || prevState.feeds};
     default:
       return prevState;
   }

content-src/components/ASRouterAdmin/ASRouterAdmin.jsx

@@ -39,8 +39,43 @@ class DiscoveryStreamAdmin extends React.PureComponent {
     this.setConfigValue("enabled", event.target.checked);
   }
 
+  renderComponent(width, component) {
+    return (
+      <table><tbody>
+        <Row>
+          <td className="min">Type</td>
+          <td>{component.type}</td>
+        </Row>
+        <Row>
+          <td className="min">Width</td>
+          <td>{width}</td>
+        </Row>
+        {component.feed && this.renderFeed(component.feed)}
+      </tbody></table>
+    );
+  }
+
+  renderFeed(feed) {
+    const {feeds} = this.props.state;
+    if (!feed.url) {
+      return null;
+    }
+    return (
+      <React.Fragment>
+        <Row>
+          <td className="min">Feed url</td>
+          <td>{feed.url}</td>
+        </Row>
+        <Row>
+          <td className="min">Data last fetched</td>
+          <td>{relativeTime(feeds[feed.url].lastUpdated) || "(no data)"}</td>
+        </Row>
+      </React.Fragment>
+    );
+  }
+
   render() {
-    const {config, lastUpdated} = this.props.state;
+    const {config, lastUpdated, layout} = this.props.state;
     return (<div>
       <div className="dsEnabled"><input type="checkbox" checked={config.enabled} onChange={this.onEnableToggle} /> enabled</div>
 
@@ -48,6 +83,18 @@ class DiscoveryStreamAdmin extends React.PureComponent {
         <Row><td className="min">Data last fetched</td><td>{relativeTime(lastUpdated) || "(no data)"}</td></Row>
         <Row><td className="min">Endpoint</td><td>{config.layout_endpoint || "(empty)"}</td></Row>
       </tbody></table>
+
+      <h3>Layout</h3>
+
+      {layout.map((row, rowIndex) => (
+        <div key={`row-${rowIndex}`}>
+          {row.components.map((component, componentIndex) => (
+            <div key={`component-${componentIndex}`} className="ds-component">
+              {this.renderComponent(row.width, component)}
+            </div>
+          ))}
+        </div>
+      ))}
     </div>);
   }
 }
@@ -496,7 +543,7 @@ export class ASRouterAdminInner extends React.PureComponent {
         return (<React.Fragment>
           <h2>Discovery Stream</h2>
           <DiscoveryStreamAdmin state={this.props.DiscoveryStream} dispatch={this.props.dispatch} />
-          </React.Fragment>);
+        </React.Fragment>);
       default:
         return (<React.Fragment>
           <h2>Message Providers <button title="Restore all provider settings that ship with Firefox" className="button" onClick={this.resetPref}>Restore default prefs</button></h2>

content-src/components/ASRouterAdmin/ASRouterAdmin.scss

@@ -146,5 +146,9 @@
     margin-bottom: 20px;
     border: 1px solid $border-color;
   }
+
+  .ds-component {
+    margin-bottom: 20px;
+  }
 }
 

content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx

@@ -15,13 +15,13 @@ export class _DiscoveryStreamBase extends React.PureComponent {
       case "SectionTitle":
         return (<SectionTitle />);
       case "CardGrid":
-        return (<CardGrid />);
+        return (<CardGrid feed={component.feed} />);
       case "Hero":
-        return (<Hero />);
+        return (<Hero feed={component.feed} />);
       case "HorizontalRule":
         return (<HorizontalRule />);
       case "List":
-        return (<List />);
+        return (<List feed={component.feed} />);
       default:
         return (<div>{component.type}</div>);
     }

content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx

@@ -1,7 +1,9 @@
+import {connect} from "react-redux";
 import React from "react";
 
-export class CardGrid extends React.PureComponent {
+export class _CardGrid extends React.PureComponent {
   render() {
+    // const feed = this.props.DiscoveryStream.feeds[this.props.feed.url];
     return (
       <div>
         Card Grid
@@ -9,3 +11,5 @@ export class CardGrid extends React.PureComponent {
     );
   }
 }
+
+export const CardGrid = connect(state => ({DiscoveryStream: state.DiscoveryStream}))(_CardGrid);

content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx

@@ -1,7 +1,9 @@
+import {connect} from "react-redux";
 import React from "react";
 
-export class Hero extends React.PureComponent {
+export class _Hero extends React.PureComponent {
   render() {
+    // const feed = this.props.DiscoveryStream.feeds[this.props.feed.url];
     return (
       <div>
         Hero
@@ -9,3 +11,5 @@ export class Hero extends React.PureComponent {
     );
   }
 }
+
+export const Hero = connect(state => ({DiscoveryStream: state.DiscoveryStream}))(_Hero);

content-src/components/DiscoveryStreamComponents/List/List.jsx

@@ -1,11 +1,15 @@
+import {connect} from "react-redux";
 import React from "react";
 
-export class List extends React.PureComponent {
+export class _List extends React.PureComponent {
   render() {
+    // const feed = this.props.DiscoveryStream.feeds[this.props.feed.url];
     return (
-      <div>
+      <div className="ds-list">
         List
       </div>
     );
   }
 }
+
+export const List = connect(state => ({DiscoveryStream: state.DiscoveryStream}))(_List);

lib/DiscoveryStreamFeed.jsm

@@ -12,6 +12,7 @@ const {PersistentCache} = ChromeUtils.import("resource://activity-stream/lib/Per
 
 const CACHE_KEY = "discovery_stream";
 const LAYOUT_UPDATE_TIME = 30 * 60 * 1000; // 30 minutes
+const COMPONENT_FEEDS_UPDATE_TIME = 30 * 60 * 1000; // 30 minutes
 const CONFIG_PREF_NAME = "browser.newtabpage.activity-stream.discoverystream.config";
 
 this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
@@ -69,7 +70,7 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
       const response = await fetch(endpoint, {credentials: "omit"});
       if (!response.ok) {
         // istanbul ignore next
-        throw new Error(`Stories endpoint returned unexpected status: ${response.status}`);
+        throw new Error(`Layout endpoint returned unexpected status: ${response.status}`);
       }
       return response.json();
     } catch (error) {
@@ -80,7 +81,7 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
     return null;
   }
 
-  async loadCachedData() {
+  async loadLayout() {
     const cachedData = await this.cache.get() || {};
     let {layout: layoutResponse} = cachedData;
     if (!layoutResponse || !(Date.now() - layoutResponse._timestamp < LAYOUT_UPDATE_TIME)) {
@@ -104,8 +105,65 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
     }
   }
 
+  async loadComponentFeeds() {
+    const {DiscoveryStream} = this.store.getState();
+    const newFeeds = {};
+    if (DiscoveryStream && DiscoveryStream.layout) {
+      for (let row of DiscoveryStream.layout) {
+        if (!row || !row.components) {
+          continue;
+        }
+        for (let component of row.components) {
+          if (component && component.feed) {
+            const {url} = component.feed;
+            newFeeds[url] = await this.getComponentFeed(url);
+          }
+        }
+      }
+
+      await this.cache.set("feeds", newFeeds);
+      this.store.dispatch(ac.BroadcastToContent({type: at.DISCOVERY_STREAM_FEEDS_UPDATE, data: newFeeds}));
+    }
+  }
+
+  async getComponentFeed(feedUrl) {
+    const cachedData = await this.cache.get() || {};
+    const {feeds} = cachedData;
+    let feed = feeds && feeds[feedUrl];
+    if (!feed || !(Date.now() - feed.lastUpdated < COMPONENT_FEEDS_UPDATE_TIME)) {
+      const feedResponse = await this.fetchComponentFeed(feedUrl);
+      if (feedResponse) {
+        feed = {
+          lastUpdated: Date.now(),
+          data: feedResponse,
+        };
+      } else {
+        Cu.reportError("No response for feed");
+      }
+    }
+
+    return feed;
+  }
+
+  async fetchComponentFeed(feedUrl) {
+    try {
+      const response = await fetch(feedUrl, {credentials: "omit"});
+      if (!response.ok) {
+        // istanbul ignore next
+        throw new Error(`Component feed endpoint returned unexpected status: ${response.status}`);
+      }
+      return response.json();
+    } catch (error) {
+      // istanbul ignore next
+      Cu.reportError(`Failed to fetch Component feed: ${error.message}`);
+    }
+    // istanbul ignore next
+    return null;
+  }
+
   async enable() {
-    await this.loadCachedData();
+    await this.loadLayout();
+    await this.loadComponentFeeds();
     this.loaded = true;
   }
 
@@ -118,6 +176,7 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
 
   async clearCache() {
     await this.cache.set("layout", {});
+    await this.cache.set("feeds", {});
   }
 
   async onPrefChange() {

test/unit/lib/DiscoveryStreamFeed.test.js

@@ -46,7 +46,7 @@ describe("DiscoveryStreamFeed", () => {
     });
   });
 
-  describe("#loadCachedData", () => {
+  describe("#loadLayout", () => {
     it("should fetch data and populate the cache if it is empty", async () => {
       const resp = {layout: ["foo", "bar"]};
       const fakeCache = {};
@@ -55,7 +55,7 @@ describe("DiscoveryStreamFeed", () => {
 
       fetchStub.resolves({ok: true, json: () => Promise.resolve(resp)});
 
-      await feed.loadCachedData();
+      await feed.loadLayout();
 
       assert.calledOnce(fetchStub);
       assert.calledWith(feed.cache.set, "layout", resp);
@@ -70,7 +70,7 @@ describe("DiscoveryStreamFeed", () => {
       fetchStub.resolves({ok: true, json: () => Promise.resolve(resp)});
 
       clock.tick(THIRTY_MINUTES + 1);
-      await feed.loadCachedData();
+      await feed.loadLayout();
 
       assert.calledOnce(fetchStub);
       assert.calledWith(feed.cache.set, "layout", resp);
@@ -82,21 +82,95 @@ describe("DiscoveryStreamFeed", () => {
       sandbox.stub(feed.cache, "set").returns(Promise.resolve());
 
       clock.tick(THIRTY_MINUTES - 1);
-      await feed.loadCachedData();
+      await feed.loadLayout();
 
       assert.notCalled(fetchStub);
       assert.notCalled(feed.cache.set);
     });
   });
 
+  describe("#loadComponentFeeds", () => {
+    it("should populate feeds cache", async () => {
+      const fakeComponents = {components: [{feed: {url: "foo.com"}}]};
+      const fakeLayout = [fakeComponents, {components: [{}]}, {}];
+      const fakeDiscoveryStream = {DiscoveryStream: {layout: fakeLayout}};
+      sandbox.stub(feed.store, "getState").returns(fakeDiscoveryStream);
+      sandbox.stub(feed.cache, "set").returns(Promise.resolve());
+      const fakeCache = {feeds: {"foo.com": {"lastUpdated": Date.now(), "data": "data"}}};
+      sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache));
+
+      await feed.loadComponentFeeds();
+
+      assert.calledWith(feed.cache.set, "feeds", {"foo.com": {"data": "data", "lastUpdated": 0}});
+    });
+  });
+
+  describe("#getComponentFeed", () => {
+    it("should fetch fresh data if cache is empty", async () => {
+      const fakeCache = {};
+      sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache));
+      sandbox.stub(feed, "fetchComponentFeed").returns(Promise.resolve("data"));
+
+      const feedResp = await feed.getComponentFeed("foo.com");
+
+      assert.deepEqual(feedResp.data, "data");
+    });
+    it("should fetch fresh data if cache is old", async () => {
+      const fakeCache = {feeds: {"foo.com": {lastUpdated: Date.now()}}};
+      sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache));
+      sandbox.stub(feed, "fetchComponentFeed").returns(Promise.resolve("data"));
+      clock.tick(THIRTY_MINUTES + 1);
+
+      const feedResp = await feed.getComponentFeed("foo.com");
+
+      assert.equal(feedResp.data, "data");
+    });
+    it("should return data from cache if it is fresh", async () => {
+      const fakeCache = {feeds: {"foo.com": {lastUpdated: Date.now(), data: "data"}}};
+      sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache));
+      sandbox.stub(feed, "fetchComponentFeed").returns(Promise.resolve("old data"));
+      clock.tick(THIRTY_MINUTES - 1);
+
+      const feedResp = await feed.getComponentFeed("foo.com");
+
+      assert.equal(feedResp.data, "data");
+    });
+  });
+
+  describe("#fetchComponentFeed", () => {
+    it("should return old feed if fetch failed", async () => {
+      fetchStub.resolves({ok: false, json: () => Promise.resolve({})});
+      const fakeCache = {feeds: {"foo.com": {lastUpdated: Date.now(), data: "old data"}}};
+      sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache));
+      clock.tick(THIRTY_MINUTES + 1);
+
+      const feedResp = await feed.getComponentFeed("foo.com");
+
+      assert.equal(feedResp.data, "old data");
+    });
+    it("should return new feed if fetch succeeds", async () => {
+      fetchStub.resolves({ok: true, json: () => Promise.resolve("data")});
+      const fakeCache = {feeds: {"foo.com": {lastUpdated: Date.now(), data: "old data"}}};
+      sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache));
+      clock.tick(THIRTY_MINUTES + 1);
+
+      const feedResp = await feed.getComponentFeed("foo.com");
+
+      assert.equal(feedResp.data, "data");
+    });
+  });
+
   describe("#clearCache", () => {
-    it("should set .layout to {}", async () => {
+    it("should set .layout and .feeds to {}", async () => {
       sandbox.stub(feed.cache, "set").returns(Promise.resolve());
 
       await feed.clearCache();
 
-      assert.calledOnce(feed.cache.set);
-      assert.calledWith(feed.cache.set, "layout", {});
+      assert.calledTwice(feed.cache.set);
+      const {firstCall} = feed.cache.set;
+      const {secondCall} = feed.cache.set;
+      assert.deepEqual(firstCall.args, ["layout", {}]);
+      assert.deepEqual(secondCall.args, ["feeds", {}]);
     });
   });
 
@@ -105,13 +179,14 @@ describe("DiscoveryStreamFeed", () => {
       assert.isFalse(feed.loaded);
     });
     it("should load data, add pref observer, and set .loaded=true if config.enabled is true", async () => {
+      sandbox.stub(feed.cache, "set").returns(Promise.resolve());
       configPrefStub.returns(JSON.stringify({enabled: true}));
-      sandbox.stub(feed, "loadCachedData").returns(Promise.resolve());
+      sandbox.stub(feed, "loadLayout").returns(Promise.resolve());
       sandbox.stub(global.Services.prefs, "addObserver");
 
       await feed.onAction({type: at.INIT});
 
-      assert.calledOnce(feed.loadCachedData);
+      assert.calledOnce(feed.loadLayout);
       assert.calledWith(global.Services.prefs.addObserver, CONFIG_PREF_NAME, feed);
       assert.isTrue(feed.loaded);
     });
@@ -129,7 +204,7 @@ describe("DiscoveryStreamFeed", () => {
   });
 
   describe("#onAction: DISCOVERY_STREAM_CONFIG_CHANGE", () => {
-    it("should call this.loadCachedData if config.enabled changes to true ", async () => {
+    it("should call this.loadLayout if config.enabled changes to true ", async () => {
       sandbox.stub(feed.cache, "set").returns(Promise.resolve());
       // First initialize
       await feed.onAction({type: at.INIT});
@@ -140,14 +215,15 @@ describe("DiscoveryStreamFeed", () => {
       configPrefStub.returns(JSON.stringify({enabled: true}));
 
       sandbox.stub(feed, "clearCache").returns(Promise.resolve());
-      sandbox.stub(feed, "loadCachedData").returns(Promise.resolve());
+      sandbox.stub(feed, "loadLayout").returns(Promise.resolve());
       await feed.onAction({type: at.DISCOVERY_STREAM_CONFIG_CHANGE});
 
-      assert.calledOnce(feed.loadCachedData);
+      assert.calledOnce(feed.loadLayout);
       assert.calledOnce(feed.clearCache);
       assert.isTrue(feed.loaded);
     });
     it("should clear the cache if a config change happens and config.enabled is true", async () => {
+      sandbox.stub(feed.cache, "set").returns(Promise.resolve());
       // force clear cached pref value
       feed._prefCache = {};
       configPrefStub.returns(JSON.stringify({enabled: true}));
@@ -157,7 +233,7 @@ describe("DiscoveryStreamFeed", () => {
 
       assert.calledOnce(feed.clearCache);
     });
-    it("should not call this.loadCachedData if config.enabled changes to false", async () => {
+    it("should not call this.loadLayout if config.enabled changes to false", async () => {
       sandbox.stub(feed.cache, "set").returns(Promise.resolve());
       // force clear cached pref value
       feed._prefCache = {};
@@ -169,10 +245,10 @@ describe("DiscoveryStreamFeed", () => {
       feed._prefCache = {};
       configPrefStub.returns(JSON.stringify({enabled: false}));
       sandbox.stub(feed, "clearCache").returns(Promise.resolve());
-      sandbox.stub(feed, "loadCachedData").returns(Promise.resolve());
+      sandbox.stub(feed, "loadLayout").returns(Promise.resolve());
       await feed.onAction({type: at.DISCOVERY_STREAM_CONFIG_CHANGE});
 
-      assert.notCalled(feed.loadCachedData);
+      assert.notCalled(feed.loadLayout);
       assert.calledOnce(feed.clearCache);
       assert.isFalse(feed.loaded);
     });