From b3deb2a640d57a889782cda3766665d6fa533225 Mon Sep 17 00:00:00 2001 From: benji7425 Date: Sun, 1 Oct 2017 23:19:10 +0100 Subject: [PATCH] Refactor multiple areas for simplicity and readability This is a squash commit of many commits --- app/commands/add-feed.js | 24 +++++------- app/index.js | 16 ++++---- app/models/feed-data.js | 78 +++++++++++++++++++++----------------- app/models/guild-data.js | 17 +++++---- discord-bot-core/Client.js | 4 +- discord-bot-core/index.js | 2 +- 6 files changed, 74 insertions(+), 67 deletions(-) diff --git a/app/commands/add-feed.js b/app/commands/add-feed.js index 9116471..d83a491 100644 --- a/app/commands/add-feed.js +++ b/app/commands/add-feed.js @@ -14,31 +14,25 @@ module.exports = new Core.Command({ }); function invoke({ message, params, guildData, client }) { - const feedUrl = [...GetUrls(message.content)][0]; - const channel = message.mentions.channels.first(); + const feedUrl = [...GetUrls(message.content)][0], + channel = message.mentions.channels.first(); if (!feedUrl || !channel) return Promise.reject("Please provide both a channel and an RSS feed URL. You can optionally @mention a role also."); - const role = message.mentions.roles.first(); - - const feedData = new FeedData({ - url: feedUrl, - channelName: channel.name, - roleName: role ? role.name : null, - maxCacheSize: Config.maxCacheSize - }); + const role = message.mentions.roles.first(), + feedData = new FeedData({ + url: feedUrl, + channelID: channel.id, + roleID: role ? role.id : null, + maxCacheSize: Config.maxCacheSize + }); return new Promise((resolve, reject) => { //ask the user if they're happy with the details they set up, save if yes, don't if no Core.util.ask(client, message.channel, message.member, "Are you happy with this (yes/no)?\n" + feedData.toString()) .then(responseMessage => { - - //if they responded yes, save the feed and let them know, else tell them to start again if (responseMessage.content.toLowerCase() === "yes") { - if (!guildData) - guildData = new GuildData({ id: message.guild.id, feeds: [] }); - guildData.feeds.push(feedData); guildData.cachePastPostedLinks(message.guild) .then(() => resolve("Your new feed has been saved!")); diff --git a/app/index.js b/app/index.js index cc512b0..a451453 100644 --- a/app/index.js +++ b/app/index.js @@ -16,15 +16,15 @@ client.on("beforeLogin", () => { client.on("ready", () => { parseLinksInGuilds(client.guilds, client.guildsData) .then(() => checkFeedsInGuilds(client.guildsData)); -}); -client.on("message", message => { - const guildData = client.guildsData[message.guild.id]; - if (guildData) - guildData.feeds.forEach(feedData => { - if (message.channel.name === feedData.channelName) - feedData.cachedLinks.push(...GetUrls(message.content)); - }); + client.on("message", message => { + const guildData = client.guildsData[message.guild.id]; + if (guildData) + guildData.feeds.forEach(feedData => { + if (message.channel.name === feedData.channelName) + feedData.cachedLinks.push(...GetUrls(message.content)); + }); + }); }); client.bootstrap(); diff --git a/app/models/feed-data.js b/app/models/feed-data.js index 5d3bed6..4663ee1 100644 --- a/app/models/feed-data.js +++ b/app/models/feed-data.js @@ -9,68 +9,78 @@ const GetUrls = require("get-urls"); //for extracting urls from messages const ShortID = require("shortid"); //to provide ids for each feed, allowing guilds to remove them module.exports = class FeedData { - constructor({ id = null, url, channelName, roleName, cachedLinks = null, maxCacheSize }) { + constructor({ id = null, url, channelID, roleID, cachedLinks = null, maxCacheSize }) { this.id = id || ShortID.generate(); this.url = url; - this.channelName = channelName; - this.roleName = roleName; + this.channelID = channelID; + this.roleID = roleID; this.cachedLinks = cachedLinks || []; this.maxCacheSize = maxCacheSize || 10; this.cachedLinks.push = (...elements) => { - const unique = elements - .map(el => normaliseUrl(el)) //normalise all the urls - .filter(el => !this.cachedLinks.includes(el)); //filter out any already cached - Array.prototype.push.apply(this.cachedLinks, unique); + Array.prototype.push.apply( + this.cachedLinks, + elements + .map(el => normaliseUrl(el)) + .filter(el => !this.cachedLinks.includes(el)) + ); - if (this.cachedLinks.length > this.maxCacheSize) - this.cachedLinks.splice(0, this.cachedLinks.length - this.maxCacheSize); //remove the # of elements above the max from the beginning + //seeing as new links come in at the end of the array, we need to remove the old links from the beginning + this.cachedLinks.splice(0, this.cachedLinks.length - this.maxCacheSize); }; } /**@param param*/ updatePastPostedLinks(guild) { - const channel = guild.channels.find(ch => ch.type === "text" && ch.name === this.channelName); - + const channel = guild.channels.get(this.channelID); + + if (!channel) + return Promise.reject("Channel not found!"); + return new Promise((resolve, reject) => { channel.fetchMessages({ limit: 100 }) .then(messages => { - new Map([...messages].reverse()).forEach(m => this.cachedLinks.push(...GetUrls(m.content))); //push all the links in each message into our links array - resolve(this); + /* we want to push the links in oldest first, but discord.js returns messages newest first, so we need to reverse them + * discord.js returns a map, and maps don't have .reverse methods, hence needing to spread the elements into an array first */ + [...messages.values()].reverse().forEach(m => this.cachedLinks.push(...GetUrls(m.content))); + resolve(); }) .catch(reject); }); } /**@param param */ - check(guild) { - Dns.resolve(Url.parse(this.url).host || "", err => { //check we can resolve the host, so we can throw an appropriate error if it fails + fetchLatest(guild) { + Dns.resolve(Url.parse(this.url).host || "", err => { if (err) - DiscordUtil.dateError("Connection Error: Can't resolve host", err); //log our error if we can't resolve the host + DiscordUtil.dateError("Connection Error: Can't resolve host", err.message || err); else - FeedRead(this.url, (err, articles) => { //check the feed - if (err) - DiscordUtil.dateError(err); - else { - let latest = articles[0].link; //extract the latest link - latest = normaliseUrl(latest); //standardise it a bit - - //if we don't have it cached already, cache it and callback - if (!this.cachedLinks.includes(latest)) { - this.cachedLinks.push(latest); - - const channel = guild.channels.find(ch => ch.type === "text" && ch.name.toLowerCase() === this.channelName.toLowerCase()); - const role = this.roleName ? guild.roles.find(role => role.name.toLowerCase() === this.roleName.toLowerCase()) : null; - channel.send((role ? role + " " : "") + latest).catch(err => DiscordUtil.dateError(`Error posting in ${channel.id}`)); - } - } - }); + this._doFetchRSS(guild); }); } toString() { const blacklist = ["cachedLinks", "maxCacheSize"]; - return `\`\`\`JavaScript\n ${JSON.stringify(this, (k, v) => !blacklist.includes(k) ? v : undefined, "\t")} \`\`\``; + return `\`\`\`JavaScript\n ${JSON.stringify(this, (k, v) => !blacklist.find(x => x === k) ? v : undefined, "\t")} \`\`\``; + } + + _doFetchRSS(guild) { + FeedRead(this.url, (err, articles) => { + if (err) + return DiscordUtil.dateError(err.message || err); + + const latest = normaliseUrl(articles[0].link); + + if (!this.cachedLinks.includes(latest)) { + this.cachedLinks.push(latest); + + const channel = guild.channels.get(this.channelID), + role = guild.roles.get(this.roleID); + + channel.send((role ? role + " " : "") + latest) + .catch(err => DiscordUtil.dateError(`Error posting in ${channel.id}: ${err.message || err}`)); + } + }); } }; diff --git a/app/models/guild-data.js b/app/models/guild-data.js index e16063c..95a5d1f 100644 --- a/app/models/guild-data.js +++ b/app/models/guild-data.js @@ -2,23 +2,26 @@ const DiscordUtil = require("../../discord-bot-core").util; const Core = require("../../discord-bot-core"); const FeedData = require("./feed-data.js"); -module.exports = class GuildData extends Core.BaseGuildData{ - constructor({ id, feeds }) { +module.exports = class GuildData extends Core.BaseGuildData { + constructor({ id, feeds = [] }) { super(id); - this.feeds = (feeds || []).map(feed => new FeedData(feed)); + this.feeds = feeds.map(feed => new FeedData(feed)); } cachePastPostedLinks(guild) { const promises = []; - this.feeds.forEach(feed => { - promises.push(feed.updatePastPostedLinks(guild).catch(err => DiscordUtil.dateError(`Error reading history in ${err.path}`))); - }); + this.feeds.forEach(feed => + promises.push( + feed.updatePastPostedLinks(guild) + .catch(err => DiscordUtil.dateError(`Error reading history in ${err.path}`)) + ) + ); return Promise.all(promises); } checkFeeds(guilds) { - this.feeds.forEach(feed => feed.check(guilds.get(this.id))); + this.feeds.forEach(feed => feed.fetchLatest(guilds.get(this.id))); } }; \ No newline at end of file diff --git a/discord-bot-core/Client.js b/discord-bot-core/Client.js index 2cadcfd..d4f1803 100644 --- a/discord-bot-core/Client.js +++ b/discord-bot-core/Client.js @@ -2,7 +2,7 @@ const FileSystem = require("fs"); const Discord = require("discord.js"); const JsonFile = require("jsonfile"); const RequireAll = require("require-all"); -const CoreUtil = require("./util.js"); +const CoreUtil = require("./Util.js"); const HandleMessage = require("./HandleMessage.js"); // @ts-ignore const InternalConfig = require("./internal-config.json"); @@ -49,7 +49,7 @@ module.exports = class Client extends Discord.Client { onMessage(message) { if (message.channel.type === "text" && message.member) { if(!this.guildsData[message.guild.id]) - this.guildsData[message.guild.id] = new this.guildDataModel(message.guild.id); + this.guildsData[message.guild.id] = new this.guildDataModel({ id: message.guild.id }); HandleMessage(this, message, this.commands, this.guildsData[message.guild.id]); } } diff --git a/discord-bot-core/index.js b/discord-bot-core/index.js index 4691597..efaf166 100644 --- a/discord-bot-core/index.js +++ b/discord-bot-core/index.js @@ -2,7 +2,7 @@ const InternalConfig = require("./internal-config.json"); module.exports = { - Client: require("./client.js"), + Client: require("./Client.js"), BaseGuildData: require("./BaseGuildData.js"), Command: require("./Command.js"), util: require("./Util.js"),