Skip to content

Commit

Permalink
fix(hot-tracks): Refactor hot tracks using dependency injection (#534)
Browse files Browse the repository at this point in the history 
Inspired by [Writing Automated Tests on a Legacy Node.js Back-End](https://www.infoq.com/articles/testing-legacy-nodejs-app/).
  • Loading branch information
@adrienjoly
adrienjoly committed Jul 18, 2023
1 parent 8e3ef11 commit 729de79
Show file tree
Hide file tree
Showing 9 changed files with 479 additions and 884 deletions.
2 changes: 1 addition & 1 deletion app/controllers/hotTracks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@ exports.controller = function (request, reqParams, response) {
}

var params = { skip: reqParams.skip, limit: reqParams.limit + 1 };
trackModel.fetchPosts(params, renderHotTracks);
trackModel.getHotTracksFromDb(params, renderHotTracks);
};
42 changes: 42 additions & 0 deletions app/features/hot-tracks.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
const snip = require('../../app/snip.js');

const FIELDS_TO_SUM = {
nbP: true, // number of plays
nbL: true, // number of likes (from lov[] field)
nbR: true, // number of posts/reposts
};

exports.FIELDS_TO_SUM = FIELDS_TO_SUM;

const FIELDS_TO_COPY = {
name: true,
img: true,
score: true,
};

exports.FIELDS_TO_COPY = FIELDS_TO_COPY;

const fieldList = Object.keys(FIELDS_TO_COPY)
.concat(Object.keys(FIELDS_TO_SUM))
.concat(['prev']);

function mergePostData(track, post) {
for (let f in fieldList) post[fieldList[f]] = track[fieldList[f]];
post.trackId = track._id;
post.rankIncr = track.prev - track.score;
return post;
}

exports.getHotTracks = async function (
getTracksByDescendingScore,
fetchPostsByPid
) {
const tracks = await getTracksByDescendingScore();
const pidList = snip.objArrayToValueArray(tracks, 'pId');
const posts = await fetchPostsByPid(pidList);
// complete track items with additional metadata (from posts)
return tracks.map((track) => {
const post = posts.find(({ eId }) => eId === track.eId);
return post ? mergePostData(track, post) : track;
});
};
75 changes: 19 additions & 56 deletions app/models/track.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/**
* track model
* - maintained by post model: updateByEid()
* - read by hot tracks controller: fetchPosts()
* - read by notif template: fetchPosts()
* - read by hot tracks controller: getHotTracksFromDb()
* - read by notif template: getHotTracksFromDb()
* @author adrienjoly, whyd
**/

Expand Down Expand Up @@ -36,18 +36,9 @@
var mongodb = require('./mongodb.js');
var ObjectId = mongodb.ObjectId;
var snip = require('../snip.js');
const feature = require('../features/hot-tracks.js');

var FIELDS_TO_SUM = {
nbP: true, // number of plays
nbL: true, // number of likes (from lov[] field)
nbR: true, // number of posts/reposts
};

var FIELDS_TO_COPY = {
name: true,
img: true,
score: true,
};
const { FIELDS_TO_SUM, FIELDS_TO_COPY } = feature;

var COEF_REPOST = 100;
var COEF_LIKE = 50;
Expand Down Expand Up @@ -124,57 +115,29 @@ exports.fetchTrackByEid = function (eId, cb) {

// functions for fetching tracks and corresponding posts

var fieldList = Object.keys(FIELDS_TO_COPY)
.concat(Object.keys(FIELDS_TO_SUM))
.concat(['prev']);

function mergePostData(track, post) {
for (let f in fieldList) post[fieldList[f]] = track[fieldList[f]];
post.trackId = track._id;
post.rankIncr = track.prev - track.score;
return post;
}

function fetchPostsByPid(pId, cb) {
var pidList = (pId && Array.isArray(pId) ? pId : []).map(function (id) {
const makeObjectIdList = (pId) =>
(pId && Array.isArray(pId) ? pId : []).map(function (id) {
return ObjectId('' + id);
});
//for (let i in pidList) pidList[i] = ObjectId("" + pidList[i]);
mongodb.collections['post'].find(
{ _id: { $in: pidList } },
POST_FETCH_OPTIONS,
function (err, cursor) {
cursor.toArray(function (err, posts) {
cb(posts);
});
}
);

function fetchPostsByPid(pId) {
return mongodb.collections['post']
.find({ _id: { $in: makeObjectIdList(pId) } }, POST_FETCH_OPTIONS)
.toArray();
}

/* fetch top hot tracks, and include complete post data (from the "post" collection), score, and rank increment */
exports.fetchPosts = function (params, handler) {
params = params || {};
exports.getHotTracksFromDb = function (params, handler) {
params.skip = parseInt(params.skip || 0);
exports.fetch(params, function (tracks) {
var pidList = snip.objArrayToValueArray(tracks, 'pId');
fetchPostsByPid(pidList, function (posts) {
var postsByEid = snip.objArrayToSet(posts, 'eId');
for (let i in tracks) {
var track = tracks[i];
if (!track) {
console.error('warning: skipping null track in track.fetchPosts()');
continue;
}
var post = postsByEid[tracks[i].eId];
if (!post) {
//console.error("warning: skipping null post in track.fetchPosts()");
continue;
}
tracks[i] = mergePostData(track, post);
}
const getTracksByDescendingScore = () =>
new Promise((resolve) => {
exports.fetch(params, resolve);
});
feature
.getHotTracks(getTracksByDescendingScore, fetchPostsByPid)
.then((tracks) => {
handler(tracks);
});
});
};

// update function
Expand Down
2 changes: 1 addition & 1 deletion app/templates/notif.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ exports.generateRegWelcome = function (user, inviteSender) {

exports.generateRegWelcomeAsync = function (user, inviteSender, cb) {
getSuggestedUsers().then((suggestedUsers) => {
trackModel.fetchPosts({ limit: MAX_HOT_TRACKS }, (hotPosts) => {
trackModel.getHotTracksFromDb({ limit: MAX_HOT_TRACKS }, (hotPosts) => {
const hotUsers = suggestedUsers
.slice(0, MAX_RECOM_USERS)
.map((user = {}) => ({
Expand Down
707 changes: 356 additions & 351 deletions scripts/code-analysis/arkit.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 8 additions & 6 deletions scripts/code-analysis/code-complexity.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/notif.js │ 528 │ 9 │ 4752 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/track.js │ 189 │ 25 │ 4725 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/api/post.js │ 288 │ 12 │ 3456 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/logging.js │ 211 │ 15 │ 3165 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/track.js │ 222 │ 14 │ 3108 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/mongodb.js │ 221 │ 14 │ 3094 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/userLibrary.js │ 180 │ 15 │ 2700 │
Expand All @@ -27,6 +27,8 @@
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/templates/feed.js │ 190 │ 11 │ 2090 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/templates/notif.js │ 286 │ 7 │ 2002 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/LibUser.js │ 164 │ 12 │ 1968 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/private/register.js │ 179 │ 10 │ 1790 │
Expand All @@ -37,8 +39,6 @@
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/notifEmails.js │ 190 │ 8 │ 1520 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/templates/notif.js │ 286 │ 5 │ 1430 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/uploadedFile.js │ 202 │ 7 │ 1414 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/admin/invites.js │ 230 │ 6 │ 1380 │
Expand Down Expand Up @@ -95,6 +95,8 @@
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/LibFriends.js │ 70 │ 8 │ 560 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/hotTracks.js │ 70 │ 8 │ 560 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/templates/adminLists.js │ 180 │ 3 │ 540 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/consent.js │ 133 │ 4 │ 532 │
Expand All @@ -111,8 +113,6 @@
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/api/lastFm.js │ 145 │ 3 │ 435 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/hotTracks.js │ 70 │ 6 │ 420 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/templates/search.js │ 103 │ 4 │ 412 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/admin/db.js │ 102 │ 4 │ 408 │
Expand Down Expand Up @@ -161,6 +161,8 @@
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/controllers/admin/test.js │ 69 │ 3 │ 207 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/features/hot-tracks.js │ 34 │ 6 │ 204 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/htmlDom.js │ 67 │ 3 │ 201 │
├────────────────────────────────────────────────┼────────────┼───────┼───────┤
│ app/models/search.js │ 37 │ 5 │ 185 │
Expand Down
Loading

0 comments on commit 729de79

Please sign in to comment.