Merge pull request #888 from coralproject/debugging-optim

Increased debugging and optimized a query path

graph/loaders/users.js

@@ -5,9 +5,20 @@ const util = require('./util');
 const UsersService = require('../../services/users');
 const UserModel = require('../../models/user');
 
-const genUserByIDs = (context, ids) => UsersService
-  .findByIdArray(ids)
-  .then(util.singleJoinBy(ids, 'id'));
+const genUserByIDs = async (context, ids) => {
+  if (!ids || ids.length === 0) {
+    return [];
+  }
+
+  if (ids.length === 1) {
+    const user = await UsersService.findById(ids[0]);
+    return [user];
+  }
+
+  return UsersService
+    .findByIdArray(ids)
+    .then(util.singleJoinBy(ids, 'id'));
+};
 
 /**
  * Retrieves users based on the passed in query that is filtered by the

graph/setupFunctions.js

@@ -0,0 +1,117 @@
+const {
+  SUBSCRIBE_COMMENT_ACCEPTED,
+  SUBSCRIBE_COMMENT_REJECTED,
+  SUBSCRIBE_COMMENT_FLAGGED,
+  SUBSCRIBE_ALL_COMMENT_EDITED,
+  SUBSCRIBE_ALL_COMMENT_ADDED,
+  SUBSCRIBE_ALL_USER_SUSPENDED,
+  SUBSCRIBE_ALL_USER_BANNED,
+  SUBSCRIBE_ALL_USERNAME_REJECTED,
+} = require('../perms/constants');
+
+const merge = require('lodash/merge');
+const debug = require('debug')('talk:graph:setupFunctions');
+const plugins = require('../services/plugins');
+
+/**
+ * Plugin support requires that we merge in existing setupFunctions with our new
+ * plugin based ones. This allows plugins to extend existing setupFunctions as well
+ * as provide new ones.
+ */
+const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plugin, setupFunctions}) => {
+  debug(`added plugin '${plugin.name}'`);
+
+  return merge(acc, setupFunctions);
+}, {
+  commentAdded: (options, args) => ({
+    commentAdded: {
+      filter: (comment, context) => {
+        if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_ADDED))) {
+          return false;
+        }
+        return !args.asset_id || comment.asset_id === args.asset_id;
+      }
+    },
+  }),
+  commentEdited: (options, args) => ({
+    commentEdited: {
+      filter: (comment, context) => {
+        if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_EDITED))) {
+          return false;
+        }
+        return !args.asset_id || comment.asset_id === args.asset_id;
+      }
+    },
+  }),
+  commentFlagged: (options, args) => ({
+    commentFlagged: {
+      filter: (comment, context) => {
+        if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_FLAGGED)) {
+          return false;
+        }
+        return !args.asset_id || comment.asset_id === args.asset_id;
+      }
+    },
+  }),
+  commentAccepted: (options, args) => ({
+    commentAccepted: {
+      filter: (comment, context) => {
+        if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_ACCEPTED)) {
+          return false;
+        }
+        return !args.asset_id || comment.asset_id === args.asset_id;
+      }
+    },
+  }),
+  commentRejected: (options, args) => ({
+    commentRejected: {
+      filter: (comment, context) => {
+        if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_REJECTED)) {
+          return false;
+        }
+        return !args.asset_id || comment.asset_id === args.asset_id;
+      }
+    },
+  }),
+  userSuspended: (options, args) => ({
+    userSuspended: {
+      filter: (user, context) => {
+        if (
+          !context.user
+          || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_SUSPENDED)
+        ) {
+          return false;
+        }
+        return !args.user_id || user.id === args.user_id;
+      }
+    },
+  }),
+  userBanned: (options, args) => ({
+    userBanned: {
+      filter: (user, context) => {
+        if (
+          !context.user
+          || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_BANNED)
+        ) {
+          return false;
+        }
+        return !args.user_id || user.id === args.user_id;
+      }
+    },
+  }),
+  usernameRejected: (options, args) => ({
+    usernameRejected: {
+      filter: (user, context) => {
+        if (
+          !context.user
+          || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USERNAME_REJECTED)
+        ) {
+          return false;
+        }
+        return !args.user_id || user.id === args.user_id;
+      }
+    },
+  }),
+});
+
+module.exports = setupFunctions;

graph/subscriptions.js

@@ -1,133 +1,56 @@
 const {SubscriptionManager} = require('graphql-subscriptions');
 const {SubscriptionServer} = require('subscriptions-transport-ws');
-const _ = require('lodash');
 const debug = require('debug')('talk:graph:subscriptions');
 
 const pubsub = require('../services/pubsub');
 const schema = require('./schema');
 const Context = require('./context');
-const plugins = require('../services/plugins');
 
 const {deserializeUser} = require('../services/subscriptions');
+const setupFunctions = require('./setupFunctions');
 
 const ms = require('ms');
 const {
   KEEP_ALIVE
 } = require('../config');
 
-const {
-  SUBSCRIBE_COMMENT_ACCEPTED,
-  SUBSCRIBE_COMMENT_REJECTED,
-  SUBSCRIBE_COMMENT_FLAGGED,
-  SUBSCRIBE_ALL_COMMENT_EDITED,
-  SUBSCRIBE_ALL_COMMENT_ADDED,
-  SUBSCRIBE_ALL_USER_SUSPENDED,
-  SUBSCRIBE_ALL_USER_BANNED,
-  SUBSCRIBE_ALL_USERNAME_REJECTED,
-} = require('../perms/constants');
-
 const {BASE_PATH} = require('../url');
 
-/**
- * Plugin support requires that we merge in existing setupFunctions with our new
- * plugin based ones. This allows plugins to extend existing setupFunctions as well
- * as provide new ones.
- */
-const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plugin, setupFunctions}) => {
-  debug(`added plugin '${plugin.name}'`);
+const onConnect = ({token}, connection) => {
 
-  return _.merge(acc, setupFunctions);
-}, {
-  commentAdded: (options, args) => ({
-    commentAdded: {
-      filter: (comment, context) => {
-        if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_ADDED))) {
-          return false;
-        }
-        return !args.asset_id || comment.asset_id === args.asset_id;
-      }
-    },
-  }),
-  commentEdited: (options, args) => ({
-    commentEdited: {
-      filter: (comment, context) => {
-        if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_EDITED))) {
-          return false;
-        }
-        return !args.asset_id || comment.asset_id === args.asset_id;
-      }
-    },
-  }),
-  commentFlagged: (options, args) => ({
-    commentFlagged: {
-      filter: (comment, context) => {
-        if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_FLAGGED)) {
-          return false;
-        }
-        return !args.asset_id || comment.asset_id === args.asset_id;
-      }
-    },
-  }),
-  commentAccepted: (options, args) => ({
-    commentAccepted: {
-      filter: (comment, context) => {
-        if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_ACCEPTED)) {
-          return false;
-        }
-        return !args.asset_id || comment.asset_id === args.asset_id;
-      }
-    },
-  }),
-  commentRejected: (options, args) => ({
-    commentRejected: {
-      filter: (comment, context) => {
-        if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_REJECTED)) {
-          return false;
-        }
-        return !args.asset_id || comment.asset_id === args.asset_id;
-      }
-    },
-  }),
-  userSuspended: (options, args) => ({
-    userSuspended: {
-      filter: (user, context) => {
-        if (
-          !context.user
-          || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_SUSPENDED)
-        ) {
-          return false;
-        }
-        return !args.user_id || user.id === args.user_id;
-      }
-    },
-  }),
-  userBanned: (options, args) => ({
-    userBanned: {
-      filter: (user, context) => {
-        if (
-          !context.user
-          || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_BANNED)
-        ) {
-          return false;
-        }
-        return !args.user_id || user.id === args.user_id;
-      }
-    },
-  }),
-  usernameRejected: (options, args) => ({
-    usernameRejected: {
-      filter: (user, context) => {
-        if (
-          !context.user
-          || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USERNAME_REJECTED)
-        ) {
-          return false;
-        }
-        return !args.user_id || user.id === args.user_id;
-      }
-    },
-  }),
-});
+  // Attach the token from the connection options if it was provided.
+  if (token) {
+
+    debug('token sent via onConnect, attaching to the headers of the upgrade request');
+
+    // Attach it to the upgrade request.
+    connection.upgradeReq.headers['authorization'] = `Bearer ${token}`;
+  }
+};
+
+const onOperation = (parsedMessage, baseParams, connection) => {
+
+  // Cache the upgrade request.
+  let upgradeReq = connection.upgradeReq;
+
+  // Attach the context per request.
+  baseParams.context = async () => {
+    let req;
+
+    try {
+      req = await deserializeUser(upgradeReq);
+      debug(`user ${req.user ? 'was' : 'was not'} on websocket request`);
+    } catch (e) {
+      console.error(e);
+
+      return new Context({});
+    }
+
+    return new Context(req);
+  };
+
+  return baseParams;
+};
 
 /**
  * This creates a new subscription manager.
@@ -138,37 +61,8 @@ const createSubscriptionManager = (server) => new SubscriptionServer({
     pubsub: pubsub.getClient(),
     setupFunctions,
   }),
-  onConnect: ({token}, connection) => {
-
-    // Attach the token from the connection options if it was provided.
-    if (token) {
-
-      // Attach it to the upgrade request.
-      connection.upgradeReq.headers['authorization'] = `Bearer ${token}`;
-    }
-  },
-  onOperation: (parsedMessage, baseParams, connection) => {
-
-    // Cache the upgrade request.
-    let upgradeReq = connection.upgradeReq;
-
-    // Attach the context per request.
-    baseParams.context = async () => {
-      let req;
-
-      try {
-        req = await deserializeUser(upgradeReq);
-      } catch (e) {
-        console.error(e);
-
-        return new Context({});
-      }
-
-      return new Context(req);
-    };
-
-    return baseParams;
-  },
+  onConnect,
+  onOperation,
   keepAlive: ms(KEEP_ALIVE)
 }, {
   server,

middleware/authentication.js

@@ -1,16 +1,22 @@
 const {passport} = require('../services/passport');
+const debug = require('debug')('talk:middleware:authentication');
 
 const authentication = (req, res, next) => passport.authenticate('jwt', {
   session: false
 }, (err, user) => {
   if (err) {
+    debug(`cannot get the user: ${err}`);
     return next(err);
   }
 
   if (user) {
 
+    debug('user was on request');
+
     // Attach the user to the request object, now that we know it exists.
     req.user = user;
+  } else {
+    debug('user was not on request');
   }
 
   next();

services/passport.js

@@ -54,11 +54,14 @@ const GenerateToken = (user) => {
 const SetTokenForSafari = (req, res, token) => {
   const browser = bowser._detect(req.headers['user-agent']);
   if (browser.ios || browser.safari) {
+    debug('browser was safari/ios, setting a cookie');
     res.cookie(JWT_SIGNING_COOKIE_NAME, token, {
       httpOnly: true,
       secure: process.env.NODE_ENV === 'production',
       expires: new Date(Date.now() + ms(JWT_EXPIRY))
     });
+  } else {
+    debug('browser wasn\'t safari/ios, didn\'t set a cookie');
   }
 };
 
@@ -170,6 +173,7 @@ const HandleLogout = (req, res, next) => {
 
     // Only clear the cookie on logout if enabled.
     if (JWT_CLEAR_COOKIE_LOGOUT) {
+      debug('clearing the login cookie');
       res.clearCookie(JWT_SIGNING_COOKIE_NAME);
     }