Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Commit

Permalink
fix(addon): #741 Refactor PreviewProvider to hide link processing
Browse files Browse the repository at this point in the history
  • Loading branch information
@sarracini
sarracini committed Jun 22, 2016
1 parent 6e6b778 commit bc84ab7
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 50 deletions.
34 changes: 9 additions & 25 deletions lib/ActivityStreams.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,20 +200,11 @@ ActivityStreams.prototype = {
/*
* Process the passed in links, save them, get from cache and response to content.
*/
_processAndSendLinks(links, responseType, worker, options) {
_processAndSendLinks(placesLinks, responseType, worker, options) {
let {append, previewsOnly, skipPreviewRequest} = options || {};
let processedLinks = this._previewProvider.processLinks(links);
const event = this._tabTracker.generateEvent({source: responseType});
if (!skipPreviewRequest) {
this._previewProvider.asyncSaveLinks(processedLinks, event);
}
const cachedLinks = this._previewProvider.getEnhancedLinks(processedLinks, previewsOnly);
this._previewProvider.getFaviconColors(cachedLinks)
.then(links => {
this._handlePerformanceEvent(event, processedLinks, cachedLinks);
this.send(am.actions.Response(responseType, links, {append}), worker);
});

const cachedLinks = this._previewProvider.getLinkMetadata(placesLinks, event, skipPreviewRequest, previewsOnly);
cachedLinks.then(linksToSend => this.send(am.actions.Response(responseType, linksToSend, {append}), worker));
},

/**
Expand Down Expand Up @@ -301,12 +292,6 @@ ActivityStreams.prototype = {
this._tabTracker.handleUserEvent(msg.data);
},

_handlePerformanceEvent(event, processedLinks, cachedLinks) {
this._tabTracker.handlePerformanceEvent(event, "previewCacheRequest", processedLinks.length);
this._tabTracker.handlePerformanceEvent(event, "previewCacheHits", cachedLinks.length);
this._tabTracker.handlePerformanceEvent(event, "previewCacheMisses", processedLinks.length - cachedLinks.length);
},

_onRouteChange({msg} = {}) {
if (msg) {
this._tabTracker.handleRouteChange(tabs.activeTab, msg.data);
Expand Down Expand Up @@ -415,29 +400,28 @@ ActivityStreams.prototype = {
_asyncBuildPreviewCache: Task.async(function*() {
if (this._populatingCache && !this._populatingCache.preview) {
this._populatingCache.preview = true;
let linksToSend = [];
let placesLinks = [];
let promises = [];

promises.push(PlacesProvider.links.getTopFrecentSites().then(links => {
linksToSend.push(...links);
placesLinks.push(...links);
}));

promises.push(PlacesProvider.links.getRecentBookmarks().then(links => {
linksToSend.push(...links);
placesLinks.push(...links);
}));

promises.push(PlacesProvider.links.getRecentLinks().then(links => {
linksToSend.push(...links);
placesLinks.push(...links);
}));

promises.push(PlacesProvider.links.getHighlightsLinks().then(links => {
linksToSend.push(...links);
placesLinks.push(...links);
}));

yield Promise.all(promises);
const event = this._tabTracker.generateEvent({source: "BUILD_PREVIEW_CACHE"});
linksToSend = this._previewProvider.processLinks(linksToSend);
yield this._previewProvider.asyncSaveLinks(linksToSend, event);
yield this._previewProvider.asyncBuildCache(placesLinks, event);
this._populatingCache.preview = false;
Services.obs.notifyObservers(null, "activity-streams-previews-cache-complete", null);
}
Expand Down
54 changes: 43 additions & 11 deletions lib/PreviewProvider.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ PreviewProvider.prototype = {
* Process the raw links that come in,
* adds a sanitizeURL and cacheKey
*/
processLinks(links) {
_processLinks(links) {
return links
.filter(this._URLFilter(URL_FILTERS))
.map(link => {
Expand All @@ -182,7 +182,10 @@ PreviewProvider.prototype = {
});
},

getFaviconColors(links) {
/**
* Get the main colors from the favicon
*/
_getFaviconColors(links) {
return Promise.all(
links.map(link => {
return new Promise(resolve => {
Expand All @@ -198,17 +201,35 @@ PreviewProvider.prototype = {
);
},

/**
* Collects all the metadata about the set of links that are requested
*/
getLinkMetadata(links, event = {}, skipPreviewRequest = false, previewsOnly = false) {
let processedLinks = this._processLinks(links);
if (!skipPreviewRequest) {
this._asyncSaveLinks(processedLinks, event);
}

let cachedLinks = this._getEnhancedLinks(processedLinks, previewsOnly, event);
return this._getFaviconColors(cachedLinks).then(linksToSend => {
return linksToSend;
});
},

/**
* Returns links with previews if available. Optionally return those with previews only
* Also, collect some metrics on how many links were returned by PlacesProvider vs how
* how many were returned by the cache
*/
getEnhancedLinks(links, previewsOnly = false) {
_getEnhancedLinks(processedLinks, previewsOnly, event) {
this._tabTracker.handlePerformanceEvent(event, "previewCacheRequest", processedLinks.length);
if (!this.enabled) {
return links;
return processedLinks;
}

let now = Date.now();

let results = links.map(link => {
let results = processedLinks.map(link => {
if (!link) {
return link;
}
Expand All @@ -229,14 +250,15 @@ PreviewProvider.prototype = {
// preview cache. This is for the case where one hasn't opened
// the browser since `cacheTTL`
this.cleanUpCacheMaybe();

this._tabTracker.handlePerformanceEvent(event, "previewCacheHits", results.length);
this._tabTracker.handlePerformanceEvent(event, "previewCacheMisses", processedLinks.length - results.length);
return results;
},

/**
* Determine if a cached link has expired
*/
isLinkExpired(link) {
_isLinkExpired(link) {
const cachedLink = ss.storage.embedlyData[link.cacheKey];
if (!cachedLink) {
return false;
Expand All @@ -245,17 +267,25 @@ PreviewProvider.prototype = {
return (currentTime - cachedLink.refreshTime) > this.options.cacheRefreshAge;
},

/**
* Build the preview cache
*/
asyncBuildCache: Task.async(function*(placesLinks, event = {}) {
let processedLinks = this._processLinks(placesLinks);
yield this._asyncSaveLinks(processedLinks, event);
}),

/**
* Request links from embedly, optionally filtering out known links
*/
asyncSaveLinks: Task.async(function*(links, event = {}, updateAccessTime = true) {
let linksList = this._uniqueLinks(links)
_asyncSaveLinks: Task.async(function*(processedLinks, event, updateAccessTime = true) {
let linksList = this._uniqueLinks(processedLinks)
.filter(link => link)
// If a request is in progress, don't re-request it
.filter(link => !this._alreadyRequested.has(link.cacheKey))
// If we already have the link in the cache, don't request it again...
// ... UNLESS it has expired
.filter(link => !ss.storage.embedlyData[link.cacheKey] || this.isLinkExpired(link));
.filter(link => !ss.storage.embedlyData[link.cacheKey] || this._isLinkExpired(link));

linksList.forEach(link => this._alreadyRequested.add(link.cacheKey));

Expand Down Expand Up @@ -290,7 +320,9 @@ PreviewProvider.prototype = {
}),

/**
* Extracts data from embedly and caches it
* Extracts data from embedly and caches it.
* Also, collect metrics on how many requests were made, how much time each
* request took to complete, and their success or failure status
*/
_asyncFetchAndCache: Task.async(function*(newLinks, event, updateAccessTime = true) {
if (!this.enabled) {
Expand Down
29 changes: 15 additions & 14 deletions test/test-PreviewProvider.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ exports.test_access_update = function*(assert) {
let currentTime = Date.now();
let twoDaysAgo = currentTime - (2 * 24 * 60 * 60 * 1000);
ss.storage.embedlyData.item_1 = {accessTime: twoDaysAgo};
gPreviewProvider.getEnhancedLinks([{cacheKey: "item_1"}]);
gPreviewProvider._getEnhancedLinks([{cacheKey: "item_1"}]);
assert.ok(ss.storage.embedlyData.item_1.accessTime > twoDaysAgo, "access time is updated");
};

exports.test_long_hibernation = function*(assert) {
let currentTime = Date.now();
let fortyDaysAgo = currentTime - (40 * 24 * 60 * 60 * 1000);
ss.storage.embedlyData.item_1 = {accessTime: fortyDaysAgo};
gPreviewProvider.getEnhancedLinks([{cacheKey: "item_1"}]);
gPreviewProvider._getEnhancedLinks([{cacheKey: "item_1"}]);
assert.ok(ss.storage.embedlyData.item_1.accessTime >= currentTime, "access time is updated");
};

Expand Down Expand Up @@ -129,8 +129,8 @@ exports.test_only_request_links_once = function*(assert) {
response.write(JSON.stringify({"urls": {urlsRequested}}));
});

gPreviewProvider.asyncSaveLinks(msg1);
yield gPreviewProvider.asyncSaveLinks(msg2);
gPreviewProvider._asyncSaveLinks(msg1);
yield gPreviewProvider._asyncSaveLinks(msg2);

for (let url in urlsRequested) {
assert.equal(urlsRequested[url], 1, "URL was requested only once");
Expand All @@ -144,16 +144,17 @@ exports.test_only_request_links_once = function*(assert) {
exports.test_is_link_expired = function(assert) {
const refreshTime = Date.now() - (gPreviewProvider.options.cacheRefreshAge + 1000);
ss.storage.embedlyData["a.com"] = {"url": "a.com", "sanitizedURL": "a.com", "cacheKey": "a.com", refreshTime};
assert.equal(gPreviewProvider.isLinkExpired({cacheKey: "a.com"}), true, "expired link should return true");
assert.equal(gPreviewProvider._isLinkExpired({cacheKey: "a.com"}), true, "expired link should return true");
ss.storage.embedlyData["b.com"] = {"url": "b.com", "sanitizedURL": "b.com", "cacheKey": "b.com", refreshTime: new Date()};
assert.equal(gPreviewProvider.isLinkExpired({cacheKey: "b.com"}), false, "non-expired link should return false");
assert.equal(gPreviewProvider._isLinkExpired({cacheKey: "b.com"}), false, "non-expired link should return false");
};

exports.test_request_links_if_expired = function*(assert) {
const oldTime = Date.now() - (gPreviewProvider.options.cacheRefreshAge + 1000);
const links = [{"url": "a.com", "sanitizedURL": "a.com", "cacheKey": "a.com"},
{"url": "b.com", "sanitizedURL": "b.com", "cacheKey": "b.com"},
{"url": "c.com", "sanitizedURL": "c.com", "cacheKey": "c.com"}];

links.forEach(link => {
ss.storage.embedlyData[link.cacheKey] = Object.assign({}, link, {refreshTime: new Date()});
});
Expand All @@ -175,7 +176,7 @@ exports.test_request_links_if_expired = function*(assert) {
response.write(JSON.stringify({"urls": {urlsRequested}}));
});

yield gPreviewProvider.asyncSaveLinks(links);
yield gPreviewProvider._asyncSaveLinks(links);

assert.deepEqual(urlsRequested, ["a.com"], "we should only request the expired URL");

Expand Down Expand Up @@ -262,7 +263,7 @@ exports.test_process_links = function*(assert) {
{"url": "https://foo.com/", "title": "blah"}
];

const processedLinks = gPreviewProvider.processLinks(fakeData);
const processedLinks = gPreviewProvider._processLinks(fakeData);

assert.equal(fakeData.length, processedLinks.length, "should not deduplicate or remove any links");

Expand Down Expand Up @@ -329,7 +330,7 @@ exports.test_throw_out_non_requested_responses = function*(assert) {
response.write(JSON.stringify(fakeResponse));
});

yield gPreviewProvider.asyncSaveLinks(fakeData);
yield gPreviewProvider._asyncSaveLinks(fakeData);

// cache should contain example1.com and example2.com
assert.ok(ss.storage.embedlyData[fakeSite1.cacheKey].embedlyMetaData, "first site was saved as expected");
Expand Down Expand Up @@ -373,12 +374,12 @@ exports.test_mock_embedly_request = function*(assert) {
response.write(JSON.stringify(fakeDataCached));
});

yield gPreviewProvider.asyncSaveLinks(fakeData);
yield gPreviewProvider._asyncSaveLinks(fakeData);

assert.deepEqual(ss.storage.embedlyData[fakeSite.cacheKey].embedlyMetaData, "some embedly metadata", "the cache saved the embedly data");
assert.ok(ss.storage.embedlyData[fakeSite.cacheKey].accessTime, "the cached saved a time stamp");

let cachedLinks = gPreviewProvider.getEnhancedLinks(fakeData);
let cachedLinks = gPreviewProvider._getEnhancedLinks(fakeData);
assert.equal(cachedLinks[0].lastVisitDate, fakeSite.lastVisitDate, "getEnhancedLinks should prioritize new data");
assert.equal(cachedLinks[0].bookmarkDateCreated, fakeSite.bookmarkDateCreated, "getEnhancedLinks should prioritize new data");
assert.ok(ss.storage.embedlyData[fakeSite.cacheKey], "the cached link is now retrieved next time");
Expand All @@ -393,18 +394,18 @@ exports.test_get_enhanced_disabled = function*(assert) {
{url: "http://foo.com/", lastVisitDate: 1459537019061}
];
simplePrefs.prefs["previews.enabled"] = false;
let cachedLinks = gPreviewProvider.getEnhancedLinks(fakeData);
let cachedLinks = gPreviewProvider._getEnhancedLinks(fakeData);
assert.deepEqual(cachedLinks, fakeData, "if disabled, should return links as is");
};

exports.test_get_enhanced_previews_only = function*(assert) {
ss.storage.embedlyData["example.com/"] = {sanitizedURL: "http://example.com/", cacheKey: "example.com/", url: "http://example.com/"};
let links;

links = gPreviewProvider.getEnhancedLinks([{cacheKey: "example.com/"}, {cacheKey: "foo.com"}]);
links = gPreviewProvider._getEnhancedLinks([{cacheKey: "example.com/"}, {cacheKey: "foo.com"}]);
assert.equal(links.length, 2, "by default getEnhancedLinks returns links with and without previews");

links = gPreviewProvider.getEnhancedLinks([{cacheKey: "example.com/"}, {cacheKey: "foo.com"}], true);
links = gPreviewProvider._getEnhancedLinks([{cacheKey: "example.com/"}, {cacheKey: "foo.com"}], true);
assert.equal(links.length, 1, "when previewOnly is set, return only links with previews");
};

Expand Down

0 comments on commit bc84ab7

Please sign in to comment.