Bug 1440768 - Improve saved to pocket stories caching and updating
MozReview-Commit-ID: 3pHJSBVekWE
--- 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;