Bug 1518258 - Inject pref based consumer_key into discovery stream layout_endpoint pref. r=Mardak a=lizzard
authork88hudson <k88hudson@gmail.com>
Sat, 16 Feb 2019 00:50:26 +0200
changeset 515996 d2aa4c02f9b20472e75f0a68e612e1a353e56692
parent 515995 afb752a4525cc5acf68858c6ac12d311cd198e58
child 515997 5a8c8df8a7a74d1c04fe52c728429dde5a531db9
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMardak, lizzard
bugs1518258
milestone66.0
Bug 1518258 - Inject pref based consumer_key into discovery stream layout_endpoint pref. r=Mardak a=lizzard Reviewers: Mardak Reviewed By: Mardak Bug #: 1518258 Differential Revision: https://phabricator.services.mozilla.com/D20006
browser/components/newtab/lib/ActivityStream.jsm
browser/components/newtab/lib/DiscoveryStreamFeed.jsm
browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
--- a/browser/components/newtab/lib/ActivityStream.jsm
+++ b/browser/components/newtab/lib/ActivityStream.jsm
@@ -211,20 +211,21 @@ const PREFS_CONFIG = new Map([
       // Block specific messages from this local provider
       exclude: [],
     }),
   }],
   // See browser/app/profile/firefox.js for other ASR preferences. They must be defined there to enable roll-outs.
   ["discoverystream.config", {
     title: "Configuration for the new pocket new tab",
     value: JSON.stringify({
+      api_key_pref: "extensions.pocket.oAuthConsumerKey",
       enabled: false,
       show_spocs: true,
       // This is currently an exmple layout used for dev purposes.
-      layout_endpoint: "https://getpocket.com/v3/newtab/layout?version=1&consumer_key=40249-e88c401e1b1f2242d9e441c4&layout_variant=basic",
+      layout_endpoint: "https://getpocket.com/v3/newtab/layout?version=1&consumer_key=$apiKey&layout_variant=basic",
     }),
   }],
 ]);
 
 // Array of each feed's FEEDS_CONFIG factory and values to add to PREFS_CONFIG
 const FEEDS_DATA = [
   {
     name: "aboutpreferences",
--- a/browser/components/newtab/lib/DiscoveryStreamFeed.jsm
+++ b/browser/components/newtab/lib/DiscoveryStreamFeed.jsm
@@ -26,27 +26,40 @@ this.DiscoveryStreamFeed = class Discove
     this.loaded = false;
 
     // Persistent cache for remote endpoint data.
     this.cache = new PersistentCache(CACHE_KEY, true);
     // Internal in-memory cache for parsing json prefs.
     this._prefCache = {};
   }
 
+  finalLayoutEndpoint(url, apiKey) {
+    if (url.includes("$apiKey") && !apiKey) {
+      throw new Error(`Layout Endpoint - An API key was specified but none configured: ${url}`);
+    }
+    return url.replace("$apiKey", apiKey);
+  }
+
   get config() {
     if (this._prefCache.config) {
       return this._prefCache.config;
     }
     try {
       this._prefCache.config = JSON.parse(Services.prefs.getStringPref(CONFIG_PREF_NAME, ""));
+      const layoutUrl = this._prefCache.config.layout_endpoint;
+      const apiKeyPref = this._prefCache.config.api_key_pref;
+      if (layoutUrl && apiKeyPref) {
+        const apiKey = Services.prefs.getCharPref(apiKeyPref, "");
+        this._prefCache.config.layout_endpoint = this.finalLayoutEndpoint(layoutUrl, apiKey);
+      }
     } catch (e) {
       // istanbul ignore next
       this._prefCache.config = {};
       // istanbul ignore next
-      Cu.reportError(`Could not parse preference. Try resetting ${CONFIG_PREF_NAME} in about:config.`);
+      Cu.reportError(`Could not parse preference. Try resetting ${CONFIG_PREF_NAME} in about:config. ${e}`);
     }
     return this._prefCache.config;
   }
 
   get showSpocs() {
     // showSponsored is generally a use set spoc opt out,
     // show_spocs is generally a mozilla set value.
     return this.store.getState().Prefs.values.showSponsored && this.config.show_spocs;
--- a/browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
+++ b/browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
@@ -100,16 +100,69 @@ describe("DiscoveryStreamFeed", () => {
       fetchStub.resolves({ok: true, json: () => Promise.resolve(resp)});
 
       await feed.loadLayout(feed.store.dispatch);
 
       assert.equal(feed.store.getState().DiscoveryStream.spocs.spocs_endpoint, "foo.com");
     });
   });
 
+  describe("#loadLayoutEndPointUsingPref", () => {
+    it("should return endpoint if valid key", async () => {
+      const endpoint = feed.finalLayoutEndpoint("https://somedomain.org/stories?consumer_key=$apiKey", "test_key_val");
+      assert.equal("https://somedomain.org/stories?consumer_key=test_key_val", endpoint);
+    });
+
+    it("should throw error if key is empty", async () => {
+      assert.throws(() => {
+        feed.finalLayoutEndpoint("https://somedomain.org/stories?consumer_key=$apiKey", "");
+      });
+    });
+
+    it("should return url if $apiKey is missing in layout_endpoint", async () => {
+      const endpoint = feed.finalLayoutEndpoint("https://somedomain.org/stories?consumer_key=", "test_key_val");
+      assert.equal("https://somedomain.org/stories?consumer_key=", endpoint);
+    });
+
+    it("should update config layout_endpoint based on api_key_pref value", async () => {
+      configPrefStub
+        .withArgs(CONFIG_PREF_NAME)
+        .returns(JSON.stringify({
+          api_key_pref: "test_api_key_pref",
+          enabled: true,
+          layout_endpoint: "https://somedomain.org/stories?consumer_key=$apiKey",
+        }));
+      sandbox.stub(global.Services.prefs, "getCharPref").returns("test_api_key_val");
+      assert.equal("https://somedomain.org/stories?consumer_key=test_api_key_val", feed.config.layout_endpoint);
+    });
+
+    it("should not update config layout_endpoint if api_key_pref missing", async () => {
+      configPrefStub
+        .withArgs(CONFIG_PREF_NAME)
+        .returns(JSON.stringify({
+          enabled: true,
+          layout_endpoint: "https://somedomain.org/stories?consumer_key=1234",
+        }));
+
+      sandbox.stub(global.Services.prefs, "getCharPref").returns("test_api_key_val");
+      assert.notCalled(global.Services.prefs.getCharPref);
+      assert.equal("https://somedomain.org/stories?consumer_key=1234", feed.config.layout_endpoint);
+    });
+
+    it("should not set config layout_endpoint if layout_endpoint missing in prefs", async () => {
+      configPrefStub
+        .withArgs(CONFIG_PREF_NAME)
+        .returns(JSON.stringify({enabled: true}));
+
+      sandbox.stub(global.Services.prefs, "getCharPref").returns("test_api_key_val");
+      assert.notCalled(global.Services.prefs.getCharPref);
+      assert.isUndefined(feed.config.layout_endpoint);
+    });
+  });
+
   describe("#loadComponentFeeds", () => {
     let fakeCache;
     let fakeDiscoveryStream;
     beforeEach(() => {
       fakeDiscoveryStream = {
         DiscoveryStream: {
           layout: [
             {components: [{feed: {url: "foo.com"}}]},