From f84892dc6a252190bd6945e08793b80d2a624326 Mon Sep 17 00:00:00 2001 From: Calvin Montgomery Date: Thu, 22 Jul 2021 22:34:16 -0700 Subject: [PATCH] Refactor polls --- src/channel/poll.js | 45 +++---- src/channel/voteskip.js | 30 +++-- src/poll.js | 132 +++++++++++++------- test/channel/voteskip.js | 8 +- test/poll.js | 261 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 390 insertions(+), 86 deletions(-) create mode 100644 test/poll.js diff --git a/src/channel/poll.js b/src/channel/poll.js index 52717e7c..12b39810 100644 --- a/src/channel/poll.js +++ b/src/channel/poll.js @@ -42,12 +42,7 @@ PollModule.prototype.unload = function () { PollModule.prototype.load = function (data) { if ("poll" in data) { if (data.poll !== null) { - this.poll = new Poll(data.poll.initiator, "", [], data.poll.obscured); - this.poll.title = data.poll.title; - this.poll.options = data.poll.options; - this.poll.counts = data.poll.counts; - this.poll.votes = data.poll.votes; - this.poll.timestamp = data.poll.timestamp; + this.poll = Poll.fromChannelData(data.poll); } } @@ -60,15 +55,7 @@ PollModule.prototype.save = function (data) { return; } - data.poll = { - title: this.poll.title, - initiator: this.poll.initiator, - options: this.poll.options, - counts: this.poll.counts, - votes: this.poll.votes, - obscured: this.poll.obscured, - timestamp: this.poll.timestamp - }; + data.poll = this.poll.toChannelData(); }; PollModule.prototype.onUserPostJoin = function (user) { @@ -97,8 +84,7 @@ PollModule.prototype.addUserToPollRoom = function (user) { }; PollModule.prototype.onUserPart = function(user) { - if (this.poll) { - this.poll.unvote(user.realip); + if (this.poll && this.poll.uncountVote(user.realip)) { this.broadcastPoll(false); } }; @@ -112,10 +98,10 @@ PollModule.prototype.sendPoll = function (user) { user.socket.emit("closePoll"); if (perms.canViewHiddenPoll(user)) { - var unobscured = this.poll.packUpdate(true); + var unobscured = this.poll.toUpdateFrame(true); user.socket.emit("newPoll", unobscured); } else { - var obscured = this.poll.packUpdate(false); + var obscured = this.poll.toUpdateFrame(false); user.socket.emit("newPoll", obscured); } }; @@ -125,8 +111,8 @@ PollModule.prototype.broadcastPoll = function (isNewPoll) { return; } - var obscured = this.poll.packUpdate(false); - var unobscured = this.poll.packUpdate(true); + var obscured = this.poll.toUpdateFrame(false); + var unobscured = this.poll.toUpdateFrame(true); const event = isNewPoll ? "newPoll" : "updatePoll"; if (isNewPoll) { @@ -197,7 +183,7 @@ PollModule.prototype.handleNewPoll = function (user, data, ack) { return; } - var poll = new Poll(user.getName(), data.title, data.opts, data.obscured); + var poll = Poll.create(user.getName(), data.title, data.opts, { hideVotes: data.obscured }); var self = this; if (data.hasOwnProperty("timeout")) { poll.timer = setTimeout(function () { @@ -223,9 +209,10 @@ PollModule.prototype.handleVote = function (user, data) { } if (this.poll) { - this.poll.vote(user.realip, data.option); - this.dirty = true; - this.broadcastPoll(false); + if (this.poll.countVote(user.realip, data.option)) { + this.dirty = true; + this.broadcastPoll(false); + } } }; @@ -235,9 +222,9 @@ PollModule.prototype.handleClosePoll = function (user) { } if (this.poll) { - if (this.poll.obscured) { - this.poll.obscured = false; - this.channel.broadcastAll("updatePoll", this.poll.packUpdate(true)); + if (this.poll.hideVotes) { + this.poll.hideVotes = false; + this.channel.broadcastAll("updatePoll", this.poll.toUpdateFrame(true)); } if (this.poll.timer) { @@ -270,7 +257,7 @@ PollModule.prototype.handlePollCmd = function (obscured, user, msg, _meta) { return; } - var poll = new Poll(user.getName(), title, args, obscured); + var poll = Poll.create(user.getName(), title, args, { hideVotes: obscured }); this.poll = poll; this.dirty = true; this.broadcastPoll(true); diff --git a/src/channel/voteskip.js b/src/channel/voteskip.js index 40150d7a..67171264 100644 --- a/src/channel/voteskip.js +++ b/src/channel/voteskip.js @@ -37,10 +37,10 @@ VoteskipModule.prototype.handleVoteskip = function (user) { } if (!this.poll) { - this.poll = new Poll("[server]", "voteskip", ["skip"], false); + this.poll = Poll.create("[server]", "voteskip", ["skip"]); } - if (!this.poll.vote(user.realip, 0)) { + if (!this.poll.countVote(user.realip, 0)) { // Vote was already recorded for this IP, no update needed return; } @@ -62,7 +62,7 @@ VoteskipModule.prototype.unvote = function(ip) { return; } - this.poll.unvote(ip); + this.poll.uncountVote(ip); }; VoteskipModule.prototype.update = function () { @@ -78,10 +78,11 @@ VoteskipModule.prototype.update = function () { return; } + const { counts } = this.poll.toUpdateFrame(false); const { total, eligible, noPermission, afk } = this.calcUsercounts(); const need = Math.ceil(eligible * this.channel.modules.options.get("voteskip_ratio")); - if (this.poll.counts[0] >= need) { - const info = `${this.poll.counts[0]}/${eligible} skipped; ` + + if (counts[0] >= need) { + const info = `${counts[0]}/${eligible} skipped; ` + `eligible voters: ${eligible} = total (${total}) - AFK (${afk}) ` + `- no permission (${noPermission}); ` + `ratio = ${this.channel.modules.options.get("voteskip_ratio")}`; @@ -107,11 +108,20 @@ VoteskipModule.prototype.update = function () { VoteskipModule.prototype.sendVoteskipData = function (users) { const { eligible } = this.calcUsercounts(); - var data = { - count: this.poll ? this.poll.counts[0] : 0, - need: this.poll ? Math.ceil(eligible * this.channel.modules.options.get("voteskip_ratio")) - : 0 - }; + let data; + + if (this.poll) { + const { counts } = this.poll.toUpdateFrame(false); + data = { + count: counts[0], + need: Math.ceil(eligible * this.channel.modules.options.get("voteskip_ratio")) + }; + } else { + data = { + count: 0, + need: 0 + }; + } var perms = this.channel.modules.permissions; diff --git a/src/poll.js b/src/poll.js index 3ae5ccb7..212a98a8 100644 --- a/src/poll.js +++ b/src/poll.js @@ -1,58 +1,100 @@ const link = /(\w+:\/\/(?:[^:/[\]\s]+|\[[0-9a-f:]+\])(?::\d+)?(?:\/[^/\s]*)*)/ig; -var XSS = require("./xss"); +const XSS = require('./xss'); -var Poll = function(initiator, title, options, obscured) { - this.initiator = initiator; - title = XSS.sanitizeText(title); - this.title = title.replace(link, "$1"); - this.options = options; - for (let i = 0; i < this.options.length; i++) { - this.options[i] = XSS.sanitizeText(this.options[i]); - this.options[i] = this.options[i].replace(link, "$1"); +function sanitizedWithLinksReplaced(text) { + return XSS.sanitizeText(text) + .replace(link, '$1'); +} +class Poll { + static create(createdBy, title, choices, options = { hideVotes: false }) { + let poll = new Poll(); + poll.createdAt = new Date(); + poll.createdBy = createdBy; + poll.title = sanitizedWithLinksReplaced(title); + poll.choices = choices.map(choice => sanitizedWithLinksReplaced(choice)); + poll.hideVotes = options.hideVotes; + poll.votes = new Map(); + return poll; } - this.obscured = obscured || false; - this.counts = new Array(options.length); - for(let i = 0; i < this.counts.length; i++) { - this.counts[i] = 0; - } - this.votes = {}; - this.timestamp = Date.now(); -}; -Poll.prototype.vote = function(ip, option) { - if(!(ip in this.votes) || this.votes[ip] == null) { - this.votes[ip] = option; - this.counts[option]++; - return true; + static fromChannelData({ initiator, title, options, _counts, votes, timestamp, obscured }) { + let poll = new Poll(); + if (timestamp === undefined) // Very old polls still in the database lack timestamps + timestamp = Date.now(); + poll.createdAt = new Date(timestamp); + poll.createdBy = initiator; + poll.title = title; + poll.choices = options; + poll.votes = new Map(); + Object.keys(votes).forEach(key => { + if (votes[key] !== null) + poll.votes.set(key, votes[key]); + }); + poll.hideVotes = obscured; + return poll; } - return false; -}; -Poll.prototype.unvote = function(ip) { - if(ip in this.votes && this.votes[ip] != null) { - this.counts[this.votes[ip]]--; - this.votes[ip] = null; + toChannelData() { + let counts = new Array(this.choices.length); + counts.fill(0); + + // TODO: it would be desirable one day to move away from using an Object here. + // This is just for backwards-compatibility with the existing format. + let votes = {}; + + this.votes.forEach((index, key) => { + votes[key] = index; + counts[index]++; + }); + + return { + title: this.title, + initiator: this.createdBy, + options: this.choices, + counts, + votes, + obscured: this.hideVotes, + timestamp: this.createdAt.getTime() + }; } -}; -Poll.prototype.packUpdate = function (showhidden) { - var counts = Array.prototype.slice.call(this.counts); - if (this.obscured) { - for(var i = 0; i < counts.length; i++) { - if (!showhidden) - counts[i] = ""; - counts[i] += "?"; + countVote(key, choiceId) { + if (choiceId < 0 || choiceId >= this.choices.length) + return false; + + let changed = !this.votes.has(key) || this.votes.get(key) !== choiceId; + this.votes.set(key, choiceId); + return changed; + } + + uncountVote(key) { + let changed = this.votes.has(key); + this.votes.delete(key); + return changed; + } + + toUpdateFrame(showHiddenVotes) { + let counts = new Array(this.choices.length); + counts.fill(0); + + this.votes.forEach(index => counts[index]++); + + if (this.hideVotes) { + counts = counts.map(c => { + if (showHiddenVotes) return `${c}?`; + else return '?'; + }); } + + return { + title: this.title, + options: this.choices, + counts: counts, + initiator: this.createdBy, + timestamp: this.createdAt.getTime() + }; } - var packed = { - title: this.title, - options: this.options, - counts: counts, - initiator: this.initiator, - timestamp: this.timestamp - }; - return packed; -}; +} exports.Poll = Poll; diff --git a/test/channel/voteskip.js b/test/channel/voteskip.js index da5f3f81..4dca2813 100644 --- a/test/channel/voteskip.js +++ b/test/channel/voteskip.js @@ -77,7 +77,9 @@ describe('VoteskipModule', () => { }; voteskipModule.poll = { - counts: [1] + toUpdateFrame() { + return { counts: [1] }; + } }; voteskipModule.update(); assert.equal(voteskipModule.poll, false, 'Expected voteskip poll to be reset to false'); @@ -93,7 +95,9 @@ describe('VoteskipModule', () => { sentMessage = true; }; voteskipModule.poll = { - counts: [1] + toUpdateFrame() { + return { counts: [1] }; + } }; voteskipModule.update(); assert(sentMessage, 'Expected voteskip passed message'); diff --git a/test/poll.js b/test/poll.js new file mode 100644 index 00000000..b32f546b --- /dev/null +++ b/test/poll.js @@ -0,0 +1,261 @@ +const assert = require('assert'); +const { Poll } = require('../lib/poll'); + +describe('Poll', () => { + describe('constructor', () => { + it('constructs a poll', () => { + let poll = Poll.create( + 'pollster', + 'Which is better?', + [ + 'Coke', + 'Pepsi' + ] + /* default opts */ + ); + + assert.strictEqual(poll.createdBy, 'pollster'); + assert.strictEqual(poll.title, 'Which is better?'); + assert.deepStrictEqual(poll.choices, ['Coke', 'Pepsi']); + assert.strictEqual(poll.hideVotes, false); + }); + + it('constructs a poll with hidden vote setting', () => { + let poll = Poll.create( + 'pollster', + 'Which is better?', + [ + 'Coke', + 'Pepsi' + ], + { hideVotes: true } + ); + + assert.strictEqual(poll.hideVotes, true); + }); + + it('sanitizes title and choices', () => { + let poll = Poll.create( + 'pollster', + 'Which is better? ', + [ + 'Coke', + 'Pepsi' + ] + /* default opts */ + ); + + assert.strictEqual(poll.title, 'Which is better? <script></script>'); + assert.deepStrictEqual(poll.choices, ['<strong>Coke</strong>', 'Pepsi']); + }); + + it('replaces URLs in title and choices', () => { + let poll = Poll.create( + 'pollster', + 'Which is better? https://example.com', + [ + 'Coke https://example.com', + 'Pepsi' + ] + /* default opts */ + ); + + assert.strictEqual( + poll.title, + 'Which is better? https://example.com' + ); + assert.deepStrictEqual( + poll.choices, + [ + 'Coke https://example.com', + 'Pepsi' + ] + ); + }); + }); + + describe('#countVote', () => { + let poll; + beforeEach(() => { + poll = Poll.create( + 'pollster', + 'Which is better?', + [ + 'Coke', + 'Pepsi' + ] + /* default opts */ + ); + }); + + it('counts a new vote', () => { + assert.strictEqual(poll.countVote('userA', 0), true); + assert.strictEqual(poll.countVote('userB', 1), true); + assert.strictEqual(poll.countVote('userC', 0), true); + + let { counts } = poll.toUpdateFrame(); + assert.deepStrictEqual(counts, [2, 1]); + }); + + it('does not count a revote for the same choice', () => { + assert.strictEqual(poll.countVote('userA', 0), true); + assert.strictEqual(poll.countVote('userA', 0), false); + + let { counts } = poll.toUpdateFrame(); + assert.deepStrictEqual(counts, [1, 0]); + }); + + it('changes a vote to a different choice', () => { + assert.strictEqual(poll.countVote('userA', 0), true); + assert.strictEqual(poll.countVote('userA', 1), true); + + let { counts } = poll.toUpdateFrame(); + assert.deepStrictEqual(counts, [0, 1]); + }); + + it('ignores out of range votes', () => { + assert.strictEqual(poll.countVote('userA', 1000), false); + assert.strictEqual(poll.countVote('userA', -10), false); + + let { counts } = poll.toUpdateFrame(); + assert.deepStrictEqual(counts, [0, 0]); + }); + }); + + describe('#uncountVote', () => { + let poll; + beforeEach(() => { + poll = Poll.create( + 'pollster', + 'Which is better?', + [ + 'Coke', + 'Pepsi' + ] + /* default opts */ + ); + }); + + it('uncounts an existing vote', () => { + assert.strictEqual(poll.countVote('userA', 0), true); + assert.strictEqual(poll.uncountVote('userA', 0), true); + + let { counts } = poll.toUpdateFrame(); + assert.deepStrictEqual(counts, [0, 0]); + }); + + it('does not uncount if there is no existing vote', () => { + assert.strictEqual(poll.uncountVote('userA', 0), false); + + let { counts } = poll.toUpdateFrame(); + assert.deepStrictEqual(counts, [0, 0]); + }); + }); + + describe('#toUpdateFrame', () => { + let poll; + beforeEach(() => { + poll = Poll.create( + 'pollster', + 'Which is better?', + [ + 'Coke', + 'Pepsi' + ] + /* default opts */ + ); + poll.countVote('userA', 0); + poll.countVote('userB', 1); + poll.countVote('userC', 0); + }); + + it('generates an update frame', () => { + assert.deepStrictEqual( + poll.toUpdateFrame(), + { + title: 'Which is better?', + options: ['Coke', 'Pepsi'], + counts: [2, 1], + initiator: 'pollster', + timestamp: poll.createdAt.getTime() + } + ); + }); + + it('hides votes when poll is hidden', () => { + poll.hideVotes = true; + + assert.deepStrictEqual( + poll.toUpdateFrame(), + { + title: 'Which is better?', + options: ['Coke', 'Pepsi'], + counts: ['?', '?'], + initiator: 'pollster', + timestamp: poll.createdAt.getTime() + } + ); + }); + + it('displays hidden votes when requested', () => { + poll.hideVotes = true; + + assert.deepStrictEqual( + poll.toUpdateFrame(true), + { + title: 'Which is better?', + options: ['Coke', 'Pepsi'], + counts: ['2?', '1?'], + initiator: 'pollster', + timestamp: poll.createdAt.getTime() + } + ); + }); + }); + + describe('#toChannelData/fromChannelData', () => { + it('round trips a poll', () => { + let data = { + title: '<strong>ready?</strong>', + initiator: 'aUser', + options: ['yes', 'no'], + counts: [0, 1], + votes:{ + '1.2.3.4': null, // Previous poll code would set removed votes to null + '5.6.7.8': 1 + }, + obscured: false, + timestamp: 1483414981110 + }; + + let poll = Poll.fromChannelData(data); + + // New code does not store null votes + data.votes = { '5.6.7.8': 1 }; + assert.deepStrictEqual(poll.toChannelData(), data); + }); + + it('coerces a missing timestamp to the current time', () => { + let data = { + title: '<strong>ready?</strong>', + initiator: 'aUser', + options: ['yes', 'no'], + counts: [0, 1], + votes:{ + '1.2.3.4': null, + '5.6.7.8': 1 + }, + obscured: false + }; + + let now = Date.now(); + let poll = Poll.fromChannelData(data); + const { timestamp } = poll.toChannelData(); + if (typeof timestamp !== 'number' || isNaN(timestamp)) + assert.fail(`Unexpected timestamp: ${timestamp}`); + + if (Math.abs(timestamp - now) > 1000) + assert.fail(`Unexpected timestamp: ${timestamp}`); + }); + }); +});