From 6bc9e6a0bbf1ab71839c448c2f4334088888c483 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 5 Dec 2017 11:17:44 -0700 Subject: [PATCH] added support for deep route caching, settings cache, template optim --- app.js | 12 ------- bin/cli-serve | 6 +++- graph/connectors.js | 2 -- middleware/staticTemplate.js | 45 ++++++++++++++++++++++++++ routes/admin/index.js | 3 +- routes/embed/index.js | 5 ++- routes/index.js | 48 ++++++++++++++++++---------- routes/static.js | 13 -------- serve.js | 8 ++--- services/cache.js | 10 +++--- services/hcache.js | 61 ++++++++++++++++++++++++++++++++++++ services/locals.js | 20 ------------ services/mailer.js | 4 +-- services/mongoose.js | 2 +- services/settings.js | 43 ++++++++++++++++++------- views/embed/stream.ejs | 13 ++++---- 16 files changed, 196 insertions(+), 99 deletions(-) create mode 100644 middleware/staticTemplate.js delete mode 100644 routes/static.js create mode 100644 services/hcache.js delete mode 100644 services/locals.js diff --git a/app.js b/app.js index d9b7ed5e1..597da28b9 100644 --- a/app.js +++ b/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'); const {ENABLE_TRACING, APOLLO_ENGINE_KEY, PORT} = require('./config'); @@ -53,12 +50,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 //============================================================================== @@ -70,9 +61,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. diff --git a/bin/cli-serve b/bin/cli-serve index 4324cac92..723558651 100755 --- a/bin/cli-serve +++ b/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); +}); diff --git a/graph/connectors.js b/graph/connectors.js index f5310ecea..38d6977fe 100644 --- a/graph/connectors.js +++ b/graph/connectors.js @@ -19,7 +19,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'); @@ -59,7 +58,6 @@ const connectors = { Karma, Kue, Limit, - Locals, Mailer, Metadata, Migration, diff --git a/middleware/staticTemplate.js b/middleware/staticTemplate.js new file mode 100644 index 000000000..823ab6624 --- /dev/null +++ b/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; diff --git a/routes/admin/index.js b/routes/admin/index.js index b3872d626..d6cb481de 100644 --- a/routes/admin/index.js +++ b/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; diff --git a/routes/embed/index.js b/routes/embed/index.js index c360c1f76..852d9afa7 100644 --- a/routes/embed/index.js +++ b/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}); } } diff --git a/routes/index.js b/routes/index.js index 1fde4b915..8f67a5549 100644 --- a/routes/index.js +++ b/routes/index.js @@ -1,18 +1,21 @@ -const express = require('express'); -const path = require('path'); -const plugins = require('../services/plugins'); -const debug = require('debug')('talk:routes'); +const SetupService = require('../services/setup'); +const apollo = require('apollo-server-express'); const authentication = require('../middleware/authentication'); -const {passport} = require('../services/passport'); -const pubsub = require('../middleware/pubsub'); -const i18n = require('../services/i18n'); +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 {createGraphOptions} = require('../graph'); -const apollo = require('apollo-server-express'); +const express = require('express'); +const i18n = require('../services/i18n'); +const path = require('path'); +const plugins = require('../services/plugins'); +const staticTemplate = require('../middleware/staticTemplate'); +const pubsub = require('../middleware/pubsub'); +const staticMiddleware = require('express-static-gzip'); const {DISABLE_STATIC_SERVER} = require('../config'); -const SetupService = require('../services/setup'); -const static = require('express-static-gzip'); +const {createGraphOptions} = require('../graph'); +const {passport} = require('../services/passport'); const router = express.Router(); @@ -26,7 +29,7 @@ if (!DISABLE_STATIC_SERVER) { * Serve the directories under public/dist from this router. */ router.use('/public', express.static(path.join(__dirname, '../public'))); - router.use('/static', static(path.resolve(path.join(__dirname, '../dist')), { + router.use('/static', staticMiddleware(path.resolve(path.join(__dirname, '../dist')), { indexFromEmptyFile: false, enableBrotli: true, customCompressions: [ @@ -38,10 +41,23 @@ if (!DISABLE_STATIC_SERVER) { })); } +//============================================================================== +// 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. @@ -89,13 +105,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('/', async (req, res) => { + router.use('/assets', staticTemplate, require('./assets')); + router.get('/', staticTemplate, async (req, res) => { try { await SetupService.isAvailable(); return res.redirect('/admin/install'); diff --git a/routes/static.js b/routes/static.js deleted file mode 100644 index 12bf01282..000000000 --- a/routes/static.js +++ /dev/null @@ -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, -}; diff --git a/serve.js b/serve.js index 52b0bdf68..7f7b4cc87 100644 --- a/serve.js +++ b/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 { diff --git a/services/cache.js b/services/cache.js index 77db0ce1f..155992fe0 100644 --- a/services/cache.js +++ b/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]); diff --git a/services/hcache.js b/services/hcache.js new file mode 100644 index 000000000..72e52d9eb --- /dev/null +++ b/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; +}; diff --git a/services/locals.js b/services/locals.js deleted file mode 100644 index 9a6a6bec8..000000000 --- a/services/locals.js +++ /dev/null @@ -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, -}; diff --git a/services/mailer.js b/services/mailer.js index f04a6dc92..a4f694b30 100644 --- a/services/mailer.js +++ b/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; diff --git a/services/mongoose.js b/services/mongoose.js index 94b357d19..747df2d66 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -73,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'); diff --git a/services/settings.js b/services/settings.js index 310884002..90ff6bbd6 100644 --- a/services/settings.js +++ b/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; } /** diff --git a/views/embed/stream.ejs b/views/embed/stream.ejs index 76d50ec22..c84605afe 100644 --- a/views/embed/stream.ejs +++ b/views/embed/stream.ejs @@ -6,16 +6,15 @@ - <% if (locals.customCssUrl) { %> - - <% } %> - <% if (data != null) { %> - - <% } %> + <%_ if (locals.customCssUrl) { _%> + + <%_ } _%> + <%_ if (data != null) { _%> + + <%_ } _%> -