Merge pull request #65 from funidata/bugfix/issue-60/user-caching

Bugfix/issue 60/user caching

app/src/middleware.js

@@ -33,9 +33,7 @@ exports.enableMiddleware = ({ app, userCache }) => {
       return;
     }
     // The user ID is found in many different places depending on the type of action taken
-    // FIXME: Dirty fix to pass linting. Refactor.
-    const userId =
-      payload.user || payload.user_id || body.user.id || body.event.message.user || undefined;
+    const userId = payload.user || payload.user_id || body.user?.id || body.event?.message?.user;
     // Approve requests which don't include any of the above (couldn't find any)
     if (!userId) {
       console.log("alert: guest check skipped!");

app/src/scheduler/scheduledMessage.js

@@ -63,16 +63,10 @@ const postRegistrationsWithUsergroup = async (
  * @param {*} userCache - userCache instance
  * @returns 
  */
-const sendScheduledMessage = async (app, channelId, usergroups, userCache) => {
+const sendScheduledMessage = async (app, channelId, usergroups) => {
   console.log('delivering scheduled posts')
   const date = DateTime.now().toISODate()
   const registrations = await service.getRegistrationsFor(date)
-  // Freshen up user cache to provide data for string generation
-  const userPromises = registrations.map((uid) => userCache.getCachedUser(uid))
-  // Wait for said freshening up to finish before continuing with message generation.
-  // Otherwise we can get empty strings for all our users, unless they've already used the application
-  // during this particular execution of the application. (Trust me, it's happened to me.)
-  await Promise.all(userPromises)
 
   const usergroupIds = usergroups.getUsergroupsForChannel(channelId)
   // No Slack user groups are added to this channel.

app/src/scheduler/scheduler.js

@@ -30,9 +30,7 @@ async function unScheduleMessage(channelId) {
  * @param app Optional. Needed only when creating a job.
  * @param usergroups Optional. Needed only when creating a job.
  */
-async function scheduleMessage({
-    channelId, time, app, usergroups, userCache,
-}) {
+async function scheduleMessage({ channelId, time, app, usergroups }) {
     const rule = new schedule.RecurrenceRule();
     rule.tz = 'Europe/Helsinki';
     rule.dayOfWeek = [1, 2, 3, 4, 5];
@@ -46,7 +44,7 @@ async function scheduleMessage({
         await service.addJob(channelId, time ? time.toSQLTime() : null);
         foundJob.reschedule(rule);
     } else { // create job
-        const sendMessage = () => sendScheduledMessage(app, channelId, usergroups, userCache)
+        const sendMessage = () => sendScheduledMessage(app, channelId, usergroups)
         // 1st arg: when to execute, 2nd arg: what to execute
         const job = schedule.scheduleJob(rule, sendMessage)
         // add the job to the map so that we can reschedule it later
@@ -57,7 +55,7 @@ async function scheduleMessage({
 /**
 * Schedules jobs to send a daily message to every channel the bot is a member of.
 */
-async function startScheduling({ app, usergroups, userCache }) {
+async function startScheduling({ app, usergroups }) {
     let channels = await helper.getMemberChannelIds(app);
     channels = channels.map((channel) => ({
         channel_id: channel,
@@ -68,9 +66,7 @@ async function startScheduling({ app, usergroups, userCache }) {
     await Promise.all(allJobs.map((job) => {
         const channelId = job.channelId
         const time = job.time ? DateTime.fromSQL(job.time) : null
-        return scheduleMessage({
-            channelId, time, app, usergroups, userCache
-        })
+        return scheduleMessage({ channelId, time, app, usergroups })
     }))
 }
 

app/src/userCache.js

@@ -30,63 +30,77 @@ const OBJECT_AGE = 60000;
  * @see getCachedUser
  */
 const generatePlaintextString = (userId) => {
-    if (!userId) {
-        return '';
-    }
-    const u = usercache[userId];
-    if (!u) {
-        // fall back to a mention string if user is not found
-        return `<@${userId}>`;
-    }
-    return `${u.user.profile.real_name || u.user.profile.display_name}`;
+  if (!userId) {
+    return "";
+  }
+  const u = usercache[userId];
+  if (!u) {
+    // fall back to a mention string if user is not found
+    return `<@${userId}>`;
+  }
+  return `${u.user.profile.real_name || u.user.profile.display_name}`;
 };
 
 const generateNameAndMention = (userId) => {
-    return `${generatePlaintextString(userId)} (<@${userId}>)`
-}
+  return `${generatePlaintextString(userId)} (<@${userId}>)`;
+};
 
 module.exports = {
-    enableUserCache: ({ app }) => {
-        /**
-         * Try to cache our user data so that getUserRestriction() doesn't bump into rate limits
-         * @param {*} userId
-         * @returns {Object} The user object as originally returned by Slack
-         */
-        const getCachedUser = async (userId) => {
-            // don't bother with undefineds and nulls and falses and empty strings and such
-            // these caused some api errors too at some point in earlier development
-            if (!userId) {
-                return null;
-            }
-            if (usercache[userId] && usercache[userId].date + OBJECT_AGE > Date.now()) {
-                console.log(`cache hit for user ${userId}`);
-                return usercache[userId].user;
-            }
-            try {
-                const user = await app.client.users.info({ user: userId });
-                // something went wrong
-                if (!user.ok) {
-                    console.log(`users.info failed for uid ${userId}`);
-                    return null;
-                }
-                // success
-                console.log(`caching user ${userId}`);
-                usercache[userId] = {
-                    user: user.user,
-                    date: Date.now(),
-                };
-                return user.user;
-            } catch (err) {
-                console.log(`users.info failed for uid ${userId}: ${err}`);
-            }
-            return null;
+  enableUserCache: ({ app }) => {
+    /**
+     * Try to cache our user data so that getUserRestriction() doesn't bump into rate limits
+     * @param {*} userId
+     * @returns {Object} The user object as originally returned by Slack
+     */
+    const getCachedUser = async (userId) => {
+      // don't bother with undefineds and nulls and falses and empty strings and such
+      // these caused some api errors too at some point in earlier development
+      if (!userId) {
+        return null;
+      }
+      if (usercache[userId] && usercache[userId].date + OBJECT_AGE > Date.now()) {
+        console.log(`cache hit for user ${userId}`);
+        return usercache[userId].user;
+      }
+      try {
+        const user = await app.client.users.info({ user: userId });
+        // something went wrong
+        if (!user.ok) {
+          console.log(`users.info failed for uid ${userId}`);
+          return null;
+        }
+        // success
+        console.log(`caching user ${userId}`);
+        usercache[userId] = {
+          user: user.user,
+          date: Date.now(),
         };
+        return user.user;
+      } catch (err) {
+        console.log(`users.info failed for uid ${userId}: ${err}`);
+      }
+      return null;
+    };
 
-        return {
-            getCachedUser,
-            generatePlaintextString
+    const initUserCache = async () => {
+      const users = await app.client.users.list();
+      const date = Date.now();
+      for (const user of users.members) {
+        usercache[user.id] = {
+          user: user,
+          date: date,
         };
-    },
-    generatePlaintextString,
-    generateNameAndMention
+      }
+      console.log("initialized user cache with every channel member");
+    };
+
+    initUserCache();
+
+    return {
+      getCachedUser,
+      generatePlaintextString,
+    };
+  },
+  generatePlaintextString,
+  generateNameAndMention,
 };