Bug 1440768 - Improve saved to pocket stories caching and updating draft
authorUrsula Sarracini <usarracini@mozilla.com>
Mon, 26 Feb 2018 14:17:09 -0500
changeset 759859 2a50c2db445a42a558f8e396d32b9ecf6cf5d3f9
parent 759663 02aa9c921aedfd0e768a92a6a8c5cba1b14191c1
push id100496
push userusarracini@mozilla.com
push dateMon, 26 Feb 2018 19:20:07 +0000
bugs1440768
milestone60.0a1
Bug 1440768 - Improve saved to pocket stories caching and updating MozReview-Commit-ID: 3pHJSBVekWE
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -59,17 +59,20 @@ const TOPIC_GATHER_TELEMETRY = "gather-t
 const ACTIVITY_STREAM_DEFAULT_FRECENCY = 150;
 
 // Some default query limit for Activity Stream requests
 const ACTIVITY_STREAM_DEFAULT_LIMIT = 12;
 
 // Some default seconds ago for Activity Stream recent requests
 const ACTIVITY_STREAM_DEFAULT_RECENT = 5 * 24 * 60 * 60;
 
-const POCKET_UPDATE_TIME = 60 * 60 * 1000; // 1 hour
+const POCKET_UPDATE_TIME = 24 * 60 * 60 * 1000; // 1 day
+const POCKET_INACTIVE_TIME = 7 * 24 * 60 * 60 * 1000; // 1 week
+const PREF_POCKET_LATEST_SINCE = "extensions.pocket.settings.latestSince";
+
 /**
  * Calculate the MD5 hash for a string.
  * @param aValue
  *        The string to convert.
  * @return The base64 representation of the MD5 hash.
  */
 function toHash(aValue) {
   let value = gUnicodeConverter.convertToByteArray(aValue);
@@ -982,19 +985,23 @@ var ActivityStreamProvider = {
     })));
   },
 
   /**
    * Helper function which makes the call to the Pocket API to fetch the user's
    * saved Pocket items.
    */
   fetchSavedPocketItems(requestData) {
-    if (!pktApi.isUserLoggedIn()) {
+    const latestSince = (Services.prefs.getStringPref(PREF_POCKET_LATEST_SINCE, 0) * 1000);
+
+    // Do not fetch Pocket items for users that have been inactive for too long, or are not logged in
+    if (!pktApi.isUserLoggedIn() || (Date.now() - latestSince > POCKET_INACTIVE_TIME)) {
       return Promise.resolve(null);
     }
+
     return new Promise((resolve, reject) => {
       pktApi.retrieve(requestData, {
         success(data) {
           resolve(data);
         },
         error(error) {
           reject(error);
         }
@@ -1299,16 +1306,17 @@ var ActivityStreamProvider = {
 };
 
 /**
  * A set of actions which influence what sites shown on the Activity Stream page
  */
 var ActivityStreamLinks = {
   _savedPocketStories: null,
   _pocketLastUpdated: 0,
+  _pocketLastLatest: 0,
 
  /**
    * Block a url
    *
    * @param {Object} aLink
    *          The link which contains a URL to add to the block list
    */
   blockURL(aLink) {
@@ -1449,22 +1457,27 @@ var ActivityStreamLinks = {
 
     // First get bookmarks if we want them
     if (!aOptions.excludeBookmarks) {
       results.push(...await ActivityStreamProvider.getRecentBookmarks(aOptions));
     }
 
     // Add the Pocket items if we need more and want them
     if (aOptions.numItems - results.length > 0 && !aOptions.excludePocket) {
-      // If we do not have saved to Pocket stories already cached, or it has been
-      // more than 1 hour since we last got Pocket stories, invalidate the cache,
-      // get new stories, and update the timestamp
-      if (!this._savedPocketStories || (Date.now() - this._pocketLastUpdated > POCKET_UPDATE_TIME)) {
+      const latestSince = ~~(Services.prefs.getStringPref(PREF_POCKET_LATEST_SINCE, 0));
+      // Invalidate the cache, get new stories, and update timestamps if:
+      //  1. we do not have saved to Pocket stories already cached OR
+      //  2. it has been too long since we last got Pocket stories OR
+      //  3. there has been a paged saved to pocket since we last got new stories
+      if (!this._savedPocketStories ||
+          (Date.now() - this._pocketLastUpdated > POCKET_UPDATE_TIME) ||
+          (this._pocketLastLatest < latestSince)) {
         this._savedPocketStories = await ActivityStreamProvider.getRecentlyPocketed(aOptions);
         this._pocketLastUpdated = Date.now();
+        this._pocketLastLatest = latestSince;
       }
       results.push(...this._savedPocketStories);
     }
 
     // Add in history if we need more and want them
     if (aOptions.numItems - results.length > 0 && !aOptions.excludeHistory) {
       // Use the same numItems as bookmarks above in case we remove duplicates
       const history = await ActivityStreamProvider.getRecentHistory(aOptions);
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -539,16 +539,17 @@ add_task(async function getHighlightsWit
   await PlacesUtils.bookmarks.insert(bookmark);
   await PlacesTestUtils.addVisits(bookmark.url);
 
   NewTabUtils.activityStreamProvider.fetchSavedPocketItems = () => fakeResponse;
   let provider = NewTabUtils.activityStreamLinks;
 
   // Force a cache invalidation
   NewTabUtils.activityStreamLinks._pocketLastUpdated = Date.now() - (70 * 60 * 1000);
+  NewTabUtils.activityStreamLinks._pocketLastLatest = -1;
   let links = await provider.getHighlights();
 
   // We should have 1 bookmark followed by 1 pocket story in highlights
   // We should not have stored the second pocket item since it was deleted
   Assert.equal(links.length, 2, "Should have 2 links in highlights");
 
   // First highlight should be a bookmark
   Assert.equal(links[0].url, bookmark.url, "The first link is the bookmark");
@@ -605,16 +606,17 @@ add_task(async function getHighlightsWit
 
   // Call getHighlights again - this time we should get the cached links since we just updated
   links = await provider.getHighlights();
   Assert.equal(links.length, 1, "We still got 1 link back for highlights");
   Assert.equal(links[0].url, fakeResponse.list["123"].resolved_url, "It was still the same pocket story");
 
   // Now force a cache invalidation and call getHighlights again
   NewTabUtils.activityStreamLinks._pocketLastUpdated = Date.now() - (70 * 60 * 1000);
+  NewTabUtils.activityStreamLinks._pocketLastLatest = -1;
   links = await provider.getHighlights();
   Assert.equal(links.length, 2, "This time we got fresh links with the new response");
   Assert.equal(links[0].url, fakeResponse.list["123"].resolved_url, "First link is unchanged");
   Assert.equal(links[1].url, fakeResponse.list["789"].resolved_url, "Second link is the new link");
 
   NewTabUtils.activityStreamLinks._savedPocketStories = null;
 });
 
@@ -623,29 +625,31 @@ add_task(async function getHighlightsWit
 
   NewTabUtils.activityStreamProvider.fetchSavedPocketItems = function() {
     throw new Error();
   };
   let provider = NewTabUtils.activityStreamLinks;
 
   // Force a cache invalidation
   NewTabUtils.activityStreamLinks._pocketLastUpdated = Date.now() - (70 * 60 * 1000);
+  NewTabUtils.activityStreamLinks._pocketLastLatest = -1;
   let links = await provider.getHighlights();
   Assert.equal(links.length, 0, "Return empty links if we reject the promise");
 });
 
 add_task(async function getHighlightsWithPocketNoData() {
   await setUpActivityStreamTest();
 
   NewTabUtils.activityStreamProvider.fetchSavedPocketItems = () => {};
 
   let provider = NewTabUtils.activityStreamLinks;
 
   // Force a cache invalidation
   NewTabUtils.activityStreamLinks._pocketLastUpdated = Date.now() - (70 * 60 * 1000);
+  NewTabUtils.activityStreamLinks._pocketLastLatest = -1;
   let links = await provider.getHighlights();
   Assert.equal(links.length, 0, "Return empty links if we got no data back from the response");
 });
 
 add_task(async function getTopFrecentSites() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;