Merge pull request #1194 from coralproject/perf-pass

Performance Pass

app.js

@@ -1,14 +1,11 @@
 const express = require('express');
-const bodyParser = require('body-parser');
 const morgan = require('morgan');
 const path = require('path');
 const merge = require('lodash/merge');
 const helmet = require('helmet');
 const compression = require('compression');
-const cookieParser = require('cookie-parser');
 const {HELMET_CONFIGURATION} = require('./config');
 const {MOUNT_PATH} = require('./url');
-const {applyLocals} = require('./services/locals');
 const routes = require('./routes');
 const debug = require('debug')('talk:app');
 
@@ -36,12 +33,6 @@ app.use(helmet(merge(HELMET_CONFIGURATION, {
 // Compress the responses if appropriate.
 app.use(compression());
 
-// Parse the cookies on the request.
-app.use(cookieParser());
-
-// Parse the body json if it's there.
-app.use(bodyParser.json());
-
 //==============================================================================
 // VIEW CONFIGURATION
 //==============================================================================
@@ -53,9 +44,6 @@ app.set('view engine', 'ejs');
 // ROUTES
 //==============================================================================
 
-// Add the locals to the app renderer.
-applyLocals(app.locals);
-
 debug(`mounting routes on the ${MOUNT_PATH} path`);
 
 // Actually apply the routes.

bin/cli-serve

@@ -1,6 +1,7 @@
 #!/usr/bin/env node
 
 const program = require('./commander');
+const util = require('./util');
 const serve = require('../serve');
 
 //==============================================================================
@@ -13,5 +14,8 @@ program
   .parse(process.argv);
 
 // Start serving.
-serve({jobs: program.jobs, websockets: program.websockets});
+serve({jobs: program.jobs, websockets: program.websockets}).catch((err) => {
+  console.error(err);
+  util.shutdown(1);
+});
 

config.js

@@ -20,6 +20,10 @@ const CONFIG = {
   // WEBPACK indicates when webpack is currently building.
   WEBPACK: process.env.WEBPACK === 'TRUE',
 
+  // When TRUE, it ensures that database indexes created in core will not add
+  // indexes.
+  CREATE_MONGO_INDEXES: process.env.DISABLE_CREATE_MONGO_INDEXES !== 'TRUE',
+
   //------------------------------------------------------------------------------
   // JWT based configuration
   //------------------------------------------------------------------------------

graph/connectors.js

@@ -5,7 +5,6 @@ const errors = require('../errors');
 const Action = require('../models/action');
 const Asset = require('../models/asset');
 const Comment = require('../models/comment');
-const Setting = require('../models/setting');
 const User = require('../models/user');
 
 // Services.
@@ -19,7 +18,6 @@ const Jwt = require('../services/jwt');
 const Karma = require('../services/karma');
 const Kue = require('../services/kue');
 const Limit = require('../services/limit');
-const Locals = require('../services/locals');
 const Mailer = require('../services/mailer');
 const Metadata = require('../services/metadata');
 const Migration = require('../services/migration');
@@ -45,7 +43,6 @@ const connectors = {
     Action,
     Asset,
     Comment,
-    Setting,
     User,
   },
   services: {
@@ -59,7 +56,6 @@ const connectors = {
     Karma,
     Kue,
     Limit,
-    Locals,
     Mailer,
     Metadata,
     Migration,

middleware/staticTemplate.js

@@ -0,0 +1,45 @@
+const {
+  BASE_URL,
+  BASE_PATH,
+  MOUNT_PATH,
+  STATIC_URL,
+} = require('../url');
+
+const {
+  RECAPTCHA_PUBLIC,
+  WEBSOCKET_LIVE_URI,
+} = require('../config');
+
+// TEMPLATE_LOCALS stores the static data that is provided as a `text/json` on
+// to the client from the template.
+const TEMPLATE_LOCALS = {
+  BASE_URL,
+  BASE_PATH,
+  MOUNT_PATH,
+  STATIC_URL,
+  data: {
+    TALK_RECAPTCHA_PUBLIC: RECAPTCHA_PUBLIC,
+    LIVE_URI: WEBSOCKET_LIVE_URI,
+    STATIC_URL,
+  },
+};
+
+// attachLocals will attach the locals to the response only.
+const attachLocals = (locals) => {
+  for (const key in TEMPLATE_LOCALS) {
+    const value = TEMPLATE_LOCALS[key];
+
+    locals[key] = value;
+  }
+};
+
+module.exports = (req, res, next) => {
+
+  // Always attach the locals.
+  attachLocals(res.locals);
+
+  // Forward the request.
+  next();
+};
+
+module.exports.attachLocals = attachLocals;

models/comment.js

@@ -108,10 +108,60 @@ CommentSchema.index({
   background: false
 });
 
+CommentSchema.index({
+  'status': 1,
+  'created_at': 1,
+}, {
+  background: true,
+});
+
+CommentSchema.index({
+  'status': 1,
+  'created_at': 1,
+  'asset_id': 1,
+}, {
+  background: true,
+});
+
 // Add an index that is optimized for sorting based on the action count data.
 CommentSchema.index({
   'created_at': 1,
-  'action_counts': 1,
+  'action_counts.flag': 1,
+}, {
+  background: true,
+});
+
+CommentSchema.index({
+  'created_at': 1,
+  'action_counts.flag': 1,
+  'status': 1,
+}, {
+  background: true,
+});
+
+// Add an index that is optimized for finding flagged comments.
+CommentSchema.index({
+  'asset_id': 1,
+  'created_at': 1,
+  'action_counts.flag': 1,
+}, {
+  background: true,
+});
+
+// Add an index for the reply sort.
+CommentSchema.index({
+  'asset_id': 1,
+  'created_at': -1,
+  'reply_count': -1,
+}, {
+  background: true,
+});
+
+// Optimize for tag searches/counts.
+CommentSchema.index({
+  'asset_id': 1,
+  'tags.tag.name': 1,
+  'status': 1,
 }, {
   background: true,
 });

plugin-api/beta/server/getReactionConfig.js

@@ -1,10 +1,24 @@
 const {SEARCH_OTHER_USERS} = require('../../../perms/constants');
 const errors = require('../../../errors');
 const pluralize = require('pluralize');
+const CommentModel = require('../../../models/comment');
+const sc = require('snake-case');
+const {CREATE_MONGO_INDEXES} = require('../../../config');
 
 function getReactionConfig(reaction) {
   reaction = reaction.toLowerCase();
 
+  if (CREATE_MONGO_INDEXES) {
+
+    // Create the index on the comment model based on the reaction config.
+    CommentModel.collection.createIndex({
+      created_at: 1,
+      [`action_counts.${sc(reaction)}`]: 1
+    }, {
+      background: true,
+    });
+  }
+
   const reactionPlural = pluralize(reaction);
   const Reaction = reaction.charAt(0).toUpperCase() + reaction.slice(1);
   const REACTION = reaction.toUpperCase();

routes/admin/index.js

@@ -1,6 +1,5 @@
 const express = require('express');
 const router = express.Router();
-const {data} = require('../static');
 
 // Get /email-confirmation expects a signed JWT in the hash
 router.get('/confirm-email', (req, res) => {
@@ -17,7 +16,7 @@ router.get('/password-reset', (req, res) => {
 });
 
 router.get('*', (req, res) => {
-  res.render('admin', {data});
+  res.render('admin');
 });
 
 module.exports = router;

routes/embed/index.js

@@ -1,13 +1,12 @@
 const express = require('express');
 const router = express.Router();
 const SettingsService = require('../../services/settings');
-const {data} = require('../static');
 
 router.use('/:embed', async (req, res, next) => {
   switch (req.params.embed) {
   case 'stream': {
-    const {customCssUrl} = await SettingsService.retrieve();
-    return res.render('embed/stream', {customCssUrl, data});
+    const {customCssUrl} = await SettingsService.retrieve('customCssUrl');
+    return res.render('embed/stream', {customCssUrl});
   }
   }
 

routes/index.js

@@ -1,17 +1,20 @@
-const express = require('express');
-const path = require('path');
-const plugins = require('../services/plugins');
-const debug = require('debug')('talk:routes');
-const authentication = require('../middleware/authentication');
-const {passport} = require('../services/passport');
-const pubsub = require('../middleware/pubsub');
-const i18n = require('../services/i18n');
-const enabled = require('debug').enabled;
-const errors = require('../errors');
-const {createGraphOptions} = require('../graph');
 const accepts = require('accepts');
 const apollo = require('graphql-server-express');
+const authentication = require('../middleware/authentication');
+const bodyParser = require('body-parser');
+const cookieParser = require('cookie-parser');
+const debug = require('debug')('talk:routes');
+const enabled = require('debug').enabled;
+const errors = require('../errors');
+const express = require('express');
+const i18n = require('../services/i18n');
+const path = require('path');
+const plugins = require('../services/plugins');
+const pubsub = require('../middleware/pubsub');
 const {DISABLE_STATIC_SERVER} = require('../config');
+const {createGraphOptions} = require('../graph');
+const {passport} = require('../services/passport');
+const staticTemplate = require('../middleware/staticTemplate');
 
 const router = express.Router();
 
@@ -61,10 +64,23 @@ if (!DISABLE_STATIC_SERVER) {
   router.get('/embed.js.map', serveFile('../dist/embed.js.map'));
 }
 
+//==============================================================================
+// STATIC ROUTES
+//==============================================================================
+
+router.use('/admin', staticTemplate, require('./admin'));
+router.use('/embed', staticTemplate, require('./embed'));
+
 //==============================================================================
 // PASSPORT MIDDLEWARE
 //==============================================================================
 
+// Parse the cookies on the request.
+router.use(cookieParser());
+
+// Parse the body json if it's there.
+router.use(bodyParser.json());
+
 const passportDebug = require('debug')('talk:passport');
 
 // Install the passport plugins.
@@ -112,13 +128,11 @@ if (process.env.NODE_ENV !== 'production') {
 //==============================================================================
 
 router.use('/api/v1', require('./api'));
-router.use('/admin', require('./admin'));
-router.use('/embed', require('./embed'));
 
+// Development routes.
 if (process.env.NODE_ENV !== 'production') {
-  router.use('/assets', require('./assets'));
-
-  router.get('/', (req, res) => {
+  router.use('/assets', staticTemplate, require('./assets'));
+  router.get('/', staticTemplate, (req, res) => {
     return res.render('article', {
       title: 'Coral Talk',
       asset_url: '',

routes/static.js

@@ -1,13 +0,0 @@
-const {
-  RECAPTCHA_PUBLIC,
-  WEBSOCKET_LIVE_URI,
-} = require('../config');
-const {
-  STATIC_URL,
-} = require('../url');
-
-module.exports.data = {
-  TALK_RECAPTCHA_PUBLIC: RECAPTCHA_PUBLIC,
-  LIVE_URI: WEBSOCKET_LIVE_URI,
-  STATIC_URL,
-};

serve.js

@@ -75,9 +75,6 @@ function normalizePort(val) {
 
 async function onListening() {
 
-  // Start the cache instance.
-  await cache.init();
-
   let addr = server.address();
   let bind = typeof addr === 'string'
     ? `pipe ${addr}`
@@ -88,7 +85,10 @@ async function onListening() {
 /**
  * Start the app.
  */
-async function serve({jobs = true, websockets = true} = {}) {
+async function serve({jobs = false, websockets = false} = {}) {
+
+  // Start the cache instance.
+  await cache.init();
 
   try {
 

services/cache.js

@@ -21,7 +21,7 @@ const keyfunc = (key) => {
  * This wraps a complicated function with a cache, in the event that the item is
  * not inside the cache, it will perform the work to get it and then set it
  * followed by returning the value.
- * @param  {Mixed} key       Either an array of items or string represening this
+ * @param  {Mixed} key       Either an array of items or string representing this
  *                           work
  * @param  {Integer} expiry  Time in seconds for the cache entry to live for
  * @param  {Function} work   A function that returns a promise that can be
@@ -30,7 +30,7 @@ const keyfunc = (key) => {
  */
 cache.wrap = async (key, expiry, work, kf = keyfunc) => {
   let value = await cache.get(key, kf);
-  if (value !== null) {
+  if (typeof value !== 'undefined' && value !== null) {
     debug('wrap: hit', kf(key));
     return value;
   }
@@ -187,11 +187,13 @@ cache.wrapMany = async (keys, expiry, work, kf = keyfunc) => {
  * @return {Promise}
  */
 cache.get = async (key, kf = keyfunc) => cache.client.get(kf(key)).then((reply) => {
-  if (reply !== null) {
+  if (typeof reply !== 'undefined' && reply !== null) {
 
     // Parse the stored cache value from JSON.
     return JSON.parse(reply);
   }
+
+  return null;
 });
 
 /**
@@ -207,7 +209,7 @@ cache.getMany = async (keys, kf = keyfunc) => cache.client.mget(keys.map(kf)).th
   for (let i = 0; i < replies.length; i++) {
     let value = null;
 
-    if (replies[i] != null) {
+    if (typeof replies[i] !== 'undefined' && replies[i] !== null) {
 
       // Parse the stored cache value from JSON.
       value = JSON.parse(replies[i]);

services/hcache.js

@@ -0,0 +1,61 @@
+const cache = require('./cache');
+const debug = require('debug')('talk:services:hcache');
+
+const kf = (key) => `hcache:${key}`;
+
+const hcache = module.exports = {};
+
+hcache.get = async (key, field = '__default__') => {
+
+  // Get the current value from redis.
+  const reply = await cache.client.hget(kf(key), field);
+
+  if (typeof reply !== 'undefined' && reply !== null) {
+    return JSON.parse(reply);
+  }
+
+  return null;
+};
+
+hcache.set = async (key, field = '__default__', value, expiry = 60) => {
+
+  // Serialize the value as JSON.
+  let reply = JSON.stringify(value);
+
+  return cache.client
+    .pipeline()
+    .hset(kf(key), field, reply)
+    .expire(kf(key), expiry)
+    .exec();
+};
+
+hcache.del = async (key, field = null) => {
+  if (field === null) {
+    return cache.client.del(kf(key));
+  }
+
+  return cache.client.hdel(kf(key), field);
+};
+
+hcache.wrap = async (key, field, expiry, work) => {
+  let value = await hcache.get(key, field);
+  if (value !== null) {
+    debug('wrap: hit', kf(key));
+    return value;
+  }
+
+  debug('wrap: miss', kf(key));
+
+  value = await work();
+
+  process.nextTick(async () => {
+    try {
+      await hcache.set(key, field, value, expiry);
+      debug('wrap: set complete');
+    } catch (err) {
+      console.error(err);
+    }
+  });
+
+  return value;
+};

services/locals.js

@@ -1,20 +0,0 @@
-const {
-  BASE_URL,
-  BASE_PATH,
-  MOUNT_PATH,
-  STATIC_URL,
-} = require('../url');
-
-const applyLocals = (locals) => {
-
-  // Apply the BASE_PATH, BASE_URL, and MOUNT_PATH on the app.locals, which will
-  // make them available on the templates and the routers.
-  locals.BASE_URL = BASE_URL;
-  locals.BASE_PATH = BASE_PATH;
-  locals.MOUNT_PATH = MOUNT_PATH;
-  locals.STATIC_URL = STATIC_URL;
-};
-
-module.exports = {
-  applyLocals,
-};

services/mailer.js

@@ -4,7 +4,7 @@ const kue = require('./kue');
 const path = require('path');
 const fs = require('fs');
 const _ = require('lodash');
-const {applyLocals} = require('./locals');
+const {attachLocals} = require('../middleware/staticTemplate');
 
 const i18n = require('./i18n');
 
@@ -97,7 +97,7 @@ const mailer = module.exports = {
     // Prefix the subject with `[Talk]`.
     subject = `[Talk] ${subject}`;
 
-    applyLocals(locals);
+    attachLocals(locals);
 
     // Attach the templating function.
     locals['t'] = i18n.t;

services/mongoose.js

@@ -5,7 +5,8 @@ const queryDebugger = require('debug')('talk:db:query');
 
 const {
   MONGO_URL,
-  WEBPACK
+  WEBPACK,
+  CREATE_MONGO_INDEXES,
 } = require('../config');
 
 // Loading the formatter from Mongoose:
@@ -56,6 +57,9 @@ if (WEBPACK) {
   mongoose
     .connect(MONGO_URL, {
       useMongoClient: true,
+      config: {
+        autoIndex: CREATE_MONGO_INDEXES,
+      },
     })
     .then(() => {
       debug('connection established');
@@ -69,7 +73,7 @@ if (WEBPACK) {
 module.exports = mongoose;
 
 // Here we include all the models that mongoose is used for, this ensures that
-// when we import mongoose that we also start up all the indexing opreations
+// when we import mongoose that we also start up all the indexing operations
 // here.
 require('../models/action');
 require('../models/asset');

services/settings.js

@@ -1,4 +1,5 @@
 const SettingModel = require('../models/setting');
+const hcache = require('./hcache');
 const errors = require('../errors');
 const {dotize} = require('./utils');
 
@@ -7,6 +8,20 @@ const {dotize} = require('./utils');
  */
 const selector = {id: '1'};
 
+const retrieve = async (fields) => {
+  let settings;
+  if (fields) {
+    settings = await SettingModel.findOne(selector).select(fields);
+  } else {
+    settings = await SettingModel.findOne(selector);
+  }
+  if (!settings) {
+    throw errors.ErrSettingsNotInit;
+  }
+
+  return settings;
+};
+
 /**
  * The Setting Service object exposing the Setting model.
  */
@@ -16,16 +31,16 @@ module.exports = class SettingsService {
    * Gets the entire settings record and sends it back
    * @return {Promise} settings the whole settings record
    */
-  static retrieve() {
-    return SettingModel
-      .findOne(selector)
-      .then((settings) => {
-        if (!settings) {
-          return Promise.reject(errors.ErrSettingsNotInit);
-        }
+  static async retrieve(fields) {
+    if (process.env.NODE_ENV === 'production') {
 
-        return settings;
-      });
+      // When in production, wrap the settings retrieval with a cache.
+      const settings = await hcache.wrap('settings', fields, 60, () => retrieve(fields));
+
+      return new SettingModel(settings);
+    }
+
+    return retrieve(fields);
   }
 
   /**
@@ -33,14 +48,20 @@ module.exports = class SettingsService {
    * @param  {object} setting a hash of whatever settings you want to update
    * @return {Promise} settings Promise that resolves to the entire (updated) settings object.
    */
-  static update(settings) {
-    return SettingModel.findOneAndUpdate(selector, {
+  static async update(settings) {
+    const updatedSettings = await SettingModel.findOneAndUpdate(selector, {
       $set: dotize(settings)
     }, {
       upsert: true,
       new: true,
       setDefaultsOnInsert: true
     });
+
+    if (process.env.NODE_ENV === 'production') {
+      await hcache.del('settings');
+    }
+
+    return updatedSettings;
   }
 
   /**

views/embed/stream.ejs

@@ -6,16 +6,15 @@
     <link rel="stylesheet" type="text/css" href="<%= STATIC_URL %>client/embed/stream/default.css">
     <link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
     <link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet">
-    <% if (locals.customCssUrl) { %>
-      <link href="<%= customCssUrl %>" rel="stylesheet" type="text/css">
-    <% } %>
-    <% if (data != null) { %>
-      <script id="data" type="application/json"><%- JSON.stringify(data) %></script>
-    <% } %>
+    <%_ if (locals.customCssUrl) { _%>
+    <link href="<%= customCssUrl %>" rel="stylesheet" type="text/css">
+    <%_ } _%>
+    <%_ if (data != null) { _%>
+    <script id="data" type="application/json"><%- JSON.stringify(data) %></script>
+    <%_ } _%>
     <base href="<%= BASE_URL %>"/>
   </head>
   <body>
-
     <div id="talk-embed-stream-container"></div>
     <script src="<%= STATIC_URL %>client/embed/stream/bundle.js"></script>
   </body>