From 171969485bbeaa251551da75acdabd0b77e7848c Mon Sep 17 00:00:00 2001 From: SachaG Date: Sat, 30 Sep 2017 08:37:15 +0900 Subject: [PATCH] Fix score calculation --- .../lib/server/comments/callbacks/voting.js | 6 +- .../lib/server/posts/callbacks/voting.js | 6 +- .../vulcan-voting/lib/containers/withVote.js | 1 + packages/vulcan-voting/lib/modules/scoring.js | 21 ++++ packages/vulcan-voting/lib/modules/vote.js | 52 ++++++--- .../vulcan-voting/lib/server/callbacks.js | 108 +++++++++--------- packages/vulcan-voting/lib/server/cron.js | 12 +- packages/vulcan-voting/lib/server/main.js | 1 - packages/vulcan-voting/lib/server/scoring.js | 44 +++---- 9 files changed, 147 insertions(+), 104 deletions(-) create mode 100644 packages/vulcan-voting/lib/modules/scoring.js diff --git a/packages/example-forum/lib/server/comments/callbacks/voting.js b/packages/example-forum/lib/server/comments/callbacks/voting.js index 60e71e5dc..11bae5fc4 100644 --- a/packages/example-forum/lib/server/comments/callbacks/voting.js +++ b/packages/example-forum/lib/server/comments/callbacks/voting.js @@ -2,14 +2,14 @@ import Users from 'meteor/vulcan:users'; import { addCallback } from 'meteor/vulcan:core'; import { Comments } from '../../../modules/comments/index.js'; -import { operateOnItem } from 'meteor/vulcan:voting'; +import { performVoteServer } from 'meteor/vulcan:voting'; /** * @summary Make users upvote their own new comments */ function CommentsNewUpvoteOwnComment(comment) { var commentAuthor = Users.findOne(comment.userId); - return {...comment, ...operateOnItem(Comments, comment, commentAuthor, 'upvote', false)}; + return {...comment, ...performVoteServer({ document: comment, voteType: 'upvote', collection: Comments, user: commentAuthor })}; } -addCallback('comments.new.sync', CommentsNewUpvoteOwnComment); +addCallback('comments.new.async', CommentsNewUpvoteOwnComment); \ No newline at end of file diff --git a/packages/example-forum/lib/server/posts/callbacks/voting.js b/packages/example-forum/lib/server/posts/callbacks/voting.js index 3cada933a..d238241db 100644 --- a/packages/example-forum/lib/server/posts/callbacks/voting.js +++ b/packages/example-forum/lib/server/posts/callbacks/voting.js @@ -7,14 +7,14 @@ Voting callbacks import { Posts } from '../../../modules/posts/index.js'; import Users from 'meteor/vulcan:users'; import { addCallback } from 'meteor/vulcan:core'; -import { operateOnItem } from 'meteor/vulcan:voting'; +import { performVoteServer } from 'meteor/vulcan:voting'; /** * @summary Make users upvote their own new posts */ function PostsNewUpvoteOwnPost(post) { var postAuthor = Users.findOne(post.userId); - return {...post, ...operateOnItem(Posts, post, postAuthor, 'upvote', false)}; + return {...post, ...performVoteServer({ document: post, voteType: 'upvote', collection: Posts, user: postAuthor })}; } -addCallback('posts.new.sync', PostsNewUpvoteOwnPost); +addCallback('posts.new.async', PostsNewUpvoteOwnPost); diff --git a/packages/vulcan-voting/lib/containers/withVote.js b/packages/vulcan-voting/lib/containers/withVote.js index e50abeb90..927af2032 100644 --- a/packages/vulcan-voting/lib/containers/withVote.js +++ b/packages/vulcan-voting/lib/containers/withVote.js @@ -19,6 +19,7 @@ export const withVote = component => { power } baseScore + score } `).join('\n')} } diff --git a/packages/vulcan-voting/lib/modules/scoring.js b/packages/vulcan-voting/lib/modules/scoring.js new file mode 100644 index 000000000..794735172 --- /dev/null +++ b/packages/vulcan-voting/lib/modules/scoring.js @@ -0,0 +1,21 @@ +export const recalculateScore = item => { + + // Age Check + + const postedAt = item.postedAt.valueOf(); + const now = new Date().getTime(); + const age = now - postedAt; + const ageInHours = age / (60 * 60 * 1000); + + // time decay factor + const f = 1.3; + + // use baseScore if defined, if not just use 0 + const baseScore = item.baseScore || 0; + + // HN algorithm + const newScore = Math.round((baseScore / Math.pow(ageInHours + 2, f))*1000000)/1000000; + + return newScore; + +}; diff --git a/packages/vulcan-voting/lib/modules/vote.js b/packages/vulcan-voting/lib/modules/vote.js index 20767bfb9..3fb8deba0 100644 --- a/packages/vulcan-voting/lib/modules/vote.js +++ b/packages/vulcan-voting/lib/modules/vote.js @@ -2,6 +2,7 @@ import { runCallbacksAsync, runCallbacks, addCallback } from 'meteor/vulcan:core import { createError } from 'apollo-errors'; import Votes from './votes/collection.js'; import Users from 'meteor/vulcan:users'; +import { recalculateScore } from './scoring.js'; /* @@ -61,10 +62,10 @@ Add a vote of a specific type on the client const addVoteClient = ({ document, collection, voteType, user, voteId }) => { const newDocument = { - _id: document._id, + ...document, baseScore: document.baseScore || 0, __typename: collection.options.typeName, - currentUserVotes: document.currentUserVotes || [] + currentUserVotes: document.currentUserVotes || [], }; // create new vote and add it to currentUserVotes array @@ -73,6 +74,7 @@ const addVoteClient = ({ document, collection, voteType, user, voteId }) => { // increment baseScore newDocument.baseScore += vote.power; + newDocument.score = recalculateScore(newDocument); return newDocument; } @@ -84,6 +86,8 @@ Add a vote of a specific type on the server */ const addVoteServer = ({ document, collection, voteType, user, voteId }) => { + const newDocument = _.clone(document); + // create vote and insert it const vote = createVote({ document, collectionName: collection.options.collectionName, voteType, user, voteId }); delete vote.__typename; @@ -92,7 +96,10 @@ const addVoteServer = ({ document, collection, voteType, user, voteId }) => { // update document score collection.update({_id: document._id}, {$inc: {baseScore: vote.power }}); - return vote; + newDocument.baseScore += vote.power; + newDocument.score = recalculateScore(newDocument); + + return newDocument; } /* @@ -106,6 +113,7 @@ const cancelVoteClient = ({ document, voteType }) => { if (vote) { // subtract vote scores newDocument.baseScore -= vote.power; + newDocument.score = recalculateScore(newDocument); const newVotes = _.reject(document.currentUserVotes, vote => vote.voteType === voteType); @@ -124,6 +132,7 @@ Clear *all* votes for a given document and user (client) const clearVotesClient = ({ document }) => { const newDocument = _.clone(document); newDocument.baseScore -= calculateTotalPower(document.currentUserVotes); + newDocument.score = recalculateScore(newDocument); newDocument.currentUserVotes = []; return newDocument } @@ -134,11 +143,15 @@ Clear all votes for a given document and user (server) */ const clearVotesServer = ({ document, user, collection }) => { + const newDocument = _.clone(document); const votes = Votes.find({ documentId: document._id, userId: user._id}).fetch(); if (votes.length) { Votes.remove({documentId: document._id}); collection.update({_id: document._id}, {$inc: {baseScore: -calculateTotalPower(votes) }}); + newDocument.baseScore -= calculateTotalPower(votes); + newDocument.score = recalculateScore(newDocument); } + return newDocument; } /* @@ -148,6 +161,8 @@ Cancel votes of a specific type on a given document (server) */ const cancelVoteServer = ({ document, voteType, collection, user }) => { + const newDocument = _.clone(document); + const vote = Votes.findOne({documentId: document._id, userId: user._id, voteType}) // remove vote object @@ -156,7 +171,10 @@ const cancelVoteServer = ({ document, voteType, collection, user }) => { // update document score collection.update({_id: document._id}, {$inc: {baseScore: -vote.power }}); - return vote; + newDocument.baseScore -= vote.power; + newDocument.score = recalculateScore(newDocument); + + return newDocument; } /* @@ -245,15 +263,15 @@ export const performVoteClient = ({ document, collection, voteType = 'upvote', u Server-side database operation */ -export const performVoteServer = ({ documentId, voteType = 'upvote', collection, voteId, user }) => { +export const performVoteServer = ({ documentId, document, voteType = 'upvote', collection, voteId, user }) => { const collectionName = collection.options.collectionName; - const document = collection.findOne(documentId); + document = document || collection.findOne(documentId); - // console.log('// performVoteMutation') - // console.log('collectionName: ', collectionName) - // console.log('document: ', collection.findOne(documentId)) - // console.log('voteType: ', voteType) + console.log('// performVoteMutation') + console.log('collectionName: ', collectionName) + console.log('document: ', document) + console.log('voteType: ', voteType) const voteOptions = {document, collection, voteType, user, voteId}; @@ -267,7 +285,7 @@ export const performVoteServer = ({ documentId, voteType = 'upvote', collection, // console.log('action: cancel') // runCallbacks(`votes.cancel.sync`, document, collection, user); - cancelVoteServer(voteOptions); + document = cancelVoteServer(voteOptions); // runCallbacksAsync(`votes.cancel.async`, vote, document, collection, user); } else { @@ -275,17 +293,17 @@ export const performVoteServer = ({ documentId, voteType = 'upvote', collection, // console.log('action: vote') if (voteTypes[voteType].exclusive) { - clearVotesServer(voteOptions) + document = clearVotesServer(voteOptions) } // runCallbacks(`votes.${voteType}.sync`, document, collection, user); - addVoteServer(voteOptions); + document = addVoteServer(voteOptions); // runCallbacksAsync(`votes.${voteType}.async`, vote, document, collection, user); } - const newDocument = collection.findOne(documentId); - newDocument.__typename = collection.options.typeName; - return newDocument; + // const newDocument = collection.findOne(documentId); + document.__typename = collection.options.typeName; + return document; -} \ No newline at end of file +} diff --git a/packages/vulcan-voting/lib/server/callbacks.js b/packages/vulcan-voting/lib/server/callbacks.js index d1264167c..101aacb67 100644 --- a/packages/vulcan-voting/lib/server/callbacks.js +++ b/packages/vulcan-voting/lib/server/callbacks.js @@ -12,14 +12,15 @@ import { updateScore } from './scoring.js'; * @param {object} collection - The collection the item belongs to * @param {string} operation - The operation being performed */ -function updateItemScore(item, user, collection, operation, context) { - updateScore({collection: collection, item: item, forceUpdate: true}); -} +// function updateItemScore(item, user, collection, operation, context) { +// updateScore({collection: collection, item: item, forceUpdate: true}); +// } + +// addCallback("upvote.async", updateItemScore); +// addCallback("downvote.async", updateItemScore); +// addCallback("cancelUpvote.async", updateItemScore); +// addCallback("cancelDownvote.async", updateItemScore); -addCallback("upvote.async", updateItemScore); -addCallback("downvote.async", updateItemScore); -addCallback("cancelUpvote.async", updateItemScore); -addCallback("cancelDownvote.async", updateItemScore); /** @@ -29,48 +30,47 @@ addCallback("cancelDownvote.async", updateItemScore); * @param {object} collection - The collection the item belongs to * @param {string} operation - The operation being performed */ -function updateUser(item, user, collection, operation, context) { +// function updateUser(item, user, collection, operation, context) { - // uncomment for debug - // console.log(item); - // console.log(user); - // console.log(collection._name); - // console.log(operation); +// // uncomment for debug +// // console.log(item); +// // console.log(user); +// // console.log(collection._name); +// // console.log(operation); - const update = {}; - const votePower = getVotePower(user); - const vote = { - itemId: item._id, - votedAt: new Date(), - power: votePower - }; +// const update = {}; +// const votePower = getVotePower(user); +// const vote = { +// itemId: item._id, +// votedAt: new Date(), +// power: votePower +// }; - const collectionName = Utils.capitalize(collection._name); +// const collectionName = Utils.capitalize(collection._name); - switch (operation) { - case "upvote": - update.$addToSet = {[`upvoted${collectionName}`]: vote}; - break; - case "downvote": - update.$addToSet = {[`downvoted${collectionName}`]: vote}; - break; - case "cancelUpvote": - update.$pull = {[`upvoted${collectionName}`]: {itemId: item._id}}; - break; - case "cancelDownvote": - update.$pull = {[`downvoted${collectionName}`]: {itemId: item._id}}; - break; - } +// switch (operation) { +// case "upvote": +// update.$addToSet = {[`upvoted${collectionName}`]: vote}; +// break; +// case "downvote": +// update.$addToSet = {[`downvoted${collectionName}`]: vote}; +// break; +// case "cancelUpvote": +// update.$pull = {[`upvoted${collectionName}`]: {itemId: item._id}}; +// break; +// case "cancelDownvote": +// update.$pull = {[`downvoted${collectionName}`]: {itemId: item._id}}; +// break; +// } - Users.update({_id: user._id}, update); +// Users.update({_id: user._id}, update); -} - -addCallback("upvote.async", updateUser); -addCallback("downvote.async", updateUser); -addCallback("cancelUpvote.async", updateUser); -addCallback("cancelDownvote.async", updateUser); +// } +// addCallback("upvote.async", updateUser); +// addCallback("downvote.async", updateUser); +// addCallback("cancelUpvote.async", updateUser); +// addCallback("cancelDownvote.async", updateUser); /** * @summary Update the karma of the item's owner @@ -79,19 +79,19 @@ addCallback("cancelDownvote.async", updateUser); * @param {object} collection - The collection the item belongs to * @param {string} operation - The operation being performed */ -function updateKarma(item, user, collection, operation, context) { +// function updateKarma(item, user, collection, operation, context) { - const votePower = getVotePower(user); - const karmaAmount = (operation === "upvote" || operation === "cancelDownvote") ? votePower : -votePower; +// const votePower = getVotePower(user); +// const karmaAmount = (operation === "upvote" || operation === "cancelDownvote") ? votePower : -votePower; - // only update karma is the operation isn't done by the item's author - if (item.userId !== user._id) { - Users.update({_id: item.userId}, {$inc: {"karma": karmaAmount}}); - } +// // only update karma is the operation isn't done by the item's author +// if (item.userId !== user._id) { +// Users.update({_id: item.userId}, {$inc: {"karma": karmaAmount}}); +// } -} +// } -addCallback("upvote.async", updateKarma); -addCallback("downvote.async", updateKarma); -addCallback("cancelUpvote.async", updateKarma); -addCallback("cancelDownvote.async", updateKarma); +// addCallback("upvote.async", updateKarma); +// addCallback("downvote.async", updateKarma); +// addCallback("cancelUpvote.async", updateKarma); +// addCallback("cancelDownvote.async", updateKarma); diff --git a/packages/vulcan-voting/lib/server/cron.js b/packages/vulcan-voting/lib/server/cron.js index 3e061d6ff..aba95fcb0 100644 --- a/packages/vulcan-voting/lib/server/cron.js +++ b/packages/vulcan-voting/lib/server/cron.js @@ -1,4 +1,4 @@ -import { getSetting, registerSetting } from 'meteor/vulcan:core'; +import { getSetting, registerSetting, debug } from 'meteor/vulcan:core'; import { updateScore } from './scoring.js'; import { VoteableCollections } from '../modules/make_voteable.js'; @@ -7,7 +7,7 @@ registerSetting('voting.scoreUpdateInterval', 60, 'How often to update scores, i // TODO use a node cron or at least synced-cron Meteor.startup(function () { - const scoreInterval = parseInt(getSetting('voting.scoreUpdateInterval', 60)); + const scoreInterval = parseInt(getSetting('voting.scoreUpdateInterval')); if (scoreInterval > 0) { @@ -15,25 +15,29 @@ Meteor.startup(function () { // active items get updated every N seconds Meteor.setInterval(function () { + let updatedDocuments = 0; // console.log('tick ('+scoreInterval+')'); collection.find({'inactive': {$ne : true}}).forEach(document => { updatedDocuments += updateScore({collection, item: document}); }); - // console.log(`Updated ${updatedDocuments} active documents in collection ${collection.options.collectionName}`) + + debug(`[vulcan:voting] Updated scores for ${updatedDocuments} active documents in collection ${collection.options.collectionName}`) }, scoreInterval * 1000); // inactive items get updated every hour Meteor.setInterval(function () { + + let updatedDocuments = 0; collection.find({'inactive': true}).forEach(document => { updatedDocuments += updateScore({collection, item: document}); }); - // console.log(`Updated ${updatedDocuments} inactive documents in collection ${collection.options.collectionName}`) + debug(`[vulcan:voting] Updated scores for ${updatedDocuments} inactive documents in collection ${collection.options.collectionName}`) }, 3600 * 1000); diff --git a/packages/vulcan-voting/lib/server/main.js b/packages/vulcan-voting/lib/server/main.js index d8ab572bd..4d5033e3f 100644 --- a/packages/vulcan-voting/lib/server/main.js +++ b/packages/vulcan-voting/lib/server/main.js @@ -1,6 +1,5 @@ import './graphql.js'; import './cron.js'; -import './callbacks.js'; import './scoring.js'; export * from '../modules/index.js'; diff --git a/packages/vulcan-voting/lib/server/scoring.js b/packages/vulcan-voting/lib/server/scoring.js index aec57a1df..1955e6157 100644 --- a/packages/vulcan-voting/lib/server/scoring.js +++ b/packages/vulcan-voting/lib/server/scoring.js @@ -1,14 +1,19 @@ +import { recalculateScore } from '../modules/scoring.js'; + +/* + +Update a document's score if necessary. + +Returns how many documents have been updated (1 or 0). + +*/ export const updateScore = ({collection, item, forceUpdate}) => { - // Status Check - - if (!!item.status && item.status !== 2) // if item has a status and is not approved, don't update its score - return 0; - // Age Check // If for some reason item doesn't have a "postedAt" property, abort - if (!item.postedAt) + // Or, if post has been scheduled in the future, don't update its score + if (!item.postedAt || postedAt > now) return 0; const postedAt = item.postedAt.valueOf(); @@ -16,9 +21,6 @@ export const updateScore = ({collection, item, forceUpdate}) => { const age = now - postedAt; const ageInHours = age / (60 * 60 * 1000); - if (postedAt > now) // if post has been scheduled in the future, don't update its score - return 0; - // For performance reasons, the database is only updated if the difference between the old score and the new score // is meaningful enough. To find out, we calculate the "power" of a single vote after n days. // We assume that after n days, a single vote will not be powerful enough to affect posts' ranking order. @@ -29,23 +31,21 @@ export const updateScore = ({collection, item, forceUpdate}) => { const n = 30; // x = score increase amount of a single vote after n days (for n=100, x=0.000040295) const x = 1/Math.pow(n*24+2,1.3); - // time decay factor - const f = 1.3; - - // use baseScore if defined, if not just use 0 - const baseScore = item.baseScore || 0; // HN algorithm - const newScore = baseScore / Math.pow(ageInHours + 2, f); - - // console.log(now) - // console.log(age) - // console.log(ageInHours) - // console.log(baseScore) - // console.log(newScore) + const newScore = recalculateScore(item); // Note: before the first time updateScore runs on a new item, its score will be at 0 - const scoreDiff = Math.abs(item.score - newScore); + const scoreDiff = Math.abs(item.score || 0 - newScore); + + // console.log('// now: ', now) + // console.log('// age: ', age) + // console.log('// ageInHours: ', ageInHours) + // console.log('// baseScore: ', baseScore) + // console.log('// item.score: ', item.score) + // console.log('// newScore: ', newScore) + // console.log('// scoreDiff: ', scoreDiff) + // console.log('// x: ', x) // only update database if difference is larger than x to avoid unnecessary updates if (forceUpdate || scoreDiff > x) {