From 745c579b82a5e3fe37462c5984a25b94c2aff280 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 17 Jul 2017 13:34:04 -0600 Subject: [PATCH 01/18] Adjust redis to not start during webpack build - Added new WEBPACK env var which is enabled during yarn build scripts - Defered redis starting until listen is called - Moved pubsub to a factory pattern init - Async/Await'ed the routes - Moved pubsub handle for routes into middleware - Adjusted redis cache and job processors to have lazy connection starting - Disabled mongo from auto-connecting on require - Adjusted package redis clients to act as factory singletons instead --- app.js | 14 +- bin/cli-serve | 14 +- config.js | 3 + graph/context.js | 2 +- graph/subscriptions.js | 2 +- package.json | 4 +- routes/api/account/index.js | 108 +++++++------- routes/api/assets/index.js | 182 +++++++++++------------- routes/api/auth/index.js | 9 +- routes/api/index.js | 12 +- routes/api/settings/index.js | 31 ++-- routes/api/setup/index.js | 53 +++---- routes/api/users/index.js | 269 +++++++++++++++++------------------ services/cache.js | 48 +++---- services/kue.js | 16 ++- services/mailer.js | 15 +- services/mongoose.js | 26 ++-- services/passport.js | 17 ++- services/pubsub.js | 11 +- services/scraper.js | 17 ++- services/users.js | 22 ++- test/server/redis.js | 3 +- 22 files changed, 456 insertions(+), 422 deletions(-) diff --git a/app.js b/app.js index 959aed0a3..d7f46ceef 100644 --- a/app.js +++ b/app.js @@ -6,6 +6,7 @@ const helmet = require('helmet'); const authentication = require('./middleware/authentication'); const {passport} = require('./services/passport'); const plugins = require('./services/plugins'); +const pubsub = require('./services/pubsub'); const i18n = require('./services/i18n'); const enabled = require('debug').enabled; const errors = require('./errors'); @@ -95,6 +96,17 @@ app.use(passport.initialize()); // (if present) the JWT on the request. app.use('/api', authentication); +// To handle dependancy injection safer, we inject the pubsub handle onto the +// request object. +app.use('/api', (req, res, next) => { + + // Attach the pubsub handle to the requests. + req.pubsub = pubsub(); + + // Forward on the request. + next(); +}); + //============================================================================== // GraphQL Router //============================================================================== @@ -159,7 +171,7 @@ app.use('/', (err, req, res, next) => { } i18n.init(req); - + if (err instanceof errors.APIError) { res.status(err.status); res.render('error', { diff --git a/bin/cli-serve b/bin/cli-serve index f2a6d3592..46fe51ff9 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -12,6 +12,7 @@ const SetupService = require('../services/setup'); const kue = require('../services/kue'); const mongoose = require('../services/mongoose'); const util = require('./util'); +const cache = require('../services/cache'); const {createSubscriptionManager} = require('../graph/subscriptions'); const { PORT @@ -80,7 +81,11 @@ function normalizePort(val) { * Event listener for HTTP server "listening" event. */ -function onListening() { +async function onListening() { + + // Start the cache instance. + await cache.init(); + let addr = server.address(); let bind = typeof addr === 'string' ? `pipe ${addr}` @@ -135,6 +140,11 @@ async function startApp(program) { /** * Listen on provided port, on all network interfaces. */ + server.on('error', onError); + server.on('listening', onListening); + server.on('listening', () => { + + }); server.listen(port, () => { // Mount the websocket server if requested. @@ -145,8 +155,6 @@ async function startApp(program) { createSubscriptionManager(server); } }); - server.on('error', onError); - server.on('listening', onListening); } //============================================================================== diff --git a/config.js b/config.js index ce8931235..9264e210d 100644 --- a/config.js +++ b/config.js @@ -13,6 +13,9 @@ require('env-rewrite').rewrite(); const CONFIG = { + // WEBPACK indicates when webpack is currently building. + WEBPACK: process.env.WEBPACK === 'true', + //------------------------------------------------------------------------------ // JWT based configuration //------------------------------------------------------------------------------ diff --git a/graph/context.js b/graph/context.js index d2306cbc8..0a94dc30b 100644 --- a/graph/context.js +++ b/graph/context.js @@ -53,7 +53,7 @@ class Context { this.plugins = decorateContextPlugins(this, contextPlugins); // Bind the publish/subscribe to the context. - this.pubsub = pubsub; + this.pubsub = pubsub(); } } diff --git a/graph/subscriptions.js b/graph/subscriptions.js index ce1eb1a09..ca505b96d 100644 --- a/graph/subscriptions.js +++ b/graph/subscriptions.js @@ -133,7 +133,7 @@ const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plu const createSubscriptionManager = (server) => new SubscriptionServer({ subscriptionManager: new SubscriptionManager({ schema, - pubsub, + pubsub: pubsub(), setupFunctions, }), onConnect: ({token}, connection) => { diff --git a/package.json b/package.json index 76b0c299a..11dab3cd0 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,8 @@ "postinstall": "./bin/cli plugins reconcile --skip-remote", "start": "./bin/cli serve -j -w", "dev-start": "nodemon -w . -w bin/cli -w bin/cli-serve --config .nodemon.json --exec \"./bin/cli -c .env serve -j -w\"", - "build": "NODE_ENV=production webpack -p --config webpack.config.js --bail", - "build-watch": "NODE_ENV=development webpack --progress --config webpack.config.js --watch", + "build": "WEBPACK=true NODE_ENV=production webpack -p --config webpack.config.js --bail", + "build-watch": "WEBPACK=true NODE_ENV=development webpack --progress --config webpack.config.js --watch", "lint": "eslint bin/* .", "lint-fix": "eslint bin/* . --fix", "test": "TEST_MODE=unit NODE_ENV=test mocha -R ${MOCHA_REPORTER:-spec}", diff --git a/routes/api/account/index.js b/routes/api/account/index.js index 4c49c00a5..99cb9a468 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -19,7 +19,7 @@ router.get('/', authorization.needed(), (req, res, next) => { // POST /email/confirm takes the password confirmation token available as a // payload parameter and if it verifies, it updates the confirmed_at date on the // local profile. -router.post('/email/verify', (req, res, next) => { +router.post('/email/verify', async (req, res, next) => { const { token @@ -29,57 +29,47 @@ router.post('/email/verify', (req, res, next) => { return next(errors.ErrMissingToken); } - UsersService - .verifyEmailConfirmation(token) - .then(({referer}) => { - res.json({redirectUri: referer}); - }) - .catch((err) => { - next(err); - }); + try { + let {referer} = await UsersService.verifyEmailConfirmation(token); + res.json({redirectUri: referer}); + } catch (e) { + return next(e); + } }); /** * this endpoint takes an email (username) and checks if it belongs to a User account * if it does, create a JWT and send an email */ -router.post('/password/reset', (req, res, next) => { +router.post('/password/reset', async (req, res, next) => { const {email, loc} = req.body; if (!email) { - return next('you must submit an email when requesting a password.'); + return next(errors.ErrMissingEmail); } - UsersService - .createPasswordResetToken(email, loc) - .then((token) => { - - // Check to see if the token isn't defined. - if (!token) { - - // As it isn't, don't send any emails! - return; - } - - return mailer.sendSimple({ - template: 'password-reset', // needed to know which template to render! - locals: { // specifies the template locals. - token, - rootURL: ROOT_URL - }, - subject: 'Password Reset', - to: email - }); - }) - .then(() => { - - // we want to send a 204 regardless of the user being found in the db - // if we fail on missing emails, it would reveal if people are registered or not. + try { + let token = await UsersService.createPasswordResetToken(email, loc); + if (!token) { res.status(204).end(); - }) - .catch((err) => { - next(err); + return; + } + + // Send the password reset email. + await mailer.sendSimple({ + template: 'password-reset', // needed to know which template to render! + locals: { // specifies the template locals. + token, + rootURL: ROOT_URL + }, + subject: 'Password Reset', + to: email }); + + res.status(204).end(); + } catch (e) { + return next(e); + } }); /** @@ -87,7 +77,7 @@ router.post('/password/reset', (req, res, next) => { * 1) the token that was in the url of the email link {String} * 2) the new password {String} */ -router.put('/password/reset', (req, res, next) => { +router.put('/password/reset', async (req, res, next) => { const { token, @@ -102,28 +92,26 @@ router.put('/password/reset', (req, res, next) => { return next(errors.ErrPasswordTooShort); } - UsersService - .verifyPasswordResetToken(token) - .then(([user, loc]) => { - return Promise.all([UsersService.changePassword(user.id, password), loc]); - }) - .then(([ , loc]) => { - res.json({redirect: loc}); - }) - .catch(() => { - next(authorization.ErrNotAuthorized); - }); + try { + let [user, loc] = await UsersService.verifyPasswordResetToken(token); + + // Change the users' password. + await UsersService.changePassword(user.id, password); + + res.json({redirect: loc}); + } catch (e) { + console.error(e); + return next(errors.ErrNotAuthorized); + } }); -router.put('/username', authorization.needed(), (req, res, next) => { - UsersService - .editName(req.user.id, req.body.username) - .then(() => { - res.status(204).end(); - }) - .catch((err) => { - next(err); - }); +router.put('/username', authorization.needed(), async (req, res, next) => { + try { + await UsersService.editName(req.user.id, req.body.username); + res.status(204).end(); + } catch (e) { + return next(e); + } }); module.exports = router; diff --git a/routes/api/assets/index.js b/routes/api/assets/index.js index f02b49f7e..a306b36b4 100644 --- a/routes/api/assets/index.js +++ b/routes/api/assets/index.js @@ -2,12 +2,39 @@ const express = require('express'); const router = express.Router(); const scraper = require('../../../services/scraper'); +const errors = require('../../../errors'); const AssetsService = require('../../../services/assets'); const AssetModel = require('../../../models/asset'); +const FilterOpenAssets = (query, filter) => { + switch(filter) { + case 'open': + return query.merge({ + $or: [ + { + closedAt: null + }, + { + closedAt: { + $gt: Date.now() + } + } + ] + }); + case 'closed': + return query.merge({ + closedAt: { + $lt: Date.now() + } + }); + default: + return query; + } +}; + // List assets. -router.get('/', (req, res, next) => { +router.get('/', async (req, res, next) => { const { limit = 20, @@ -18,138 +45,97 @@ router.get('/', (req, res, next) => { search = '' } = req.query; - const FilterOpenAssets = (query, filter) => { - switch(filter) { - case 'open': - return query.merge({ - $or: [ - { - closedAt: null - }, - { - closedAt: { - $gt: Date.now() - } - } - ] - }); - case 'closed': - return query.merge({ - closedAt: { - $lt: Date.now() - } - }); - default: - return query; - } - }; + try { - // Find all the assets. - Promise.all([ + // Find all the assets. + let [result, count] = await Promise.all([ - // Find the actuall assets. - FilterOpenAssets(AssetsService.search({value: search}), filter) - .sort({[field]: (sort === 'asc') ? 1 : -1}) - .skip(parseInt(skip)) - .limit(parseInt(limit)), + // Find the actuall assets. + FilterOpenAssets(AssetsService.search({value: search}), filter) + .sort({[field]: (sort === 'asc') ? 1 : -1}) + .skip(parseInt(skip)) + .limit(parseInt(limit)), - // Get the count of actual assets. - FilterOpenAssets(AssetsService.search({value: search}), filter) - .count() - ]) - .then(([result, count]) => { + // Get the count of actual assets. + FilterOpenAssets(AssetsService.search({value: search}), filter) + .count() + ]); // Send back the asset data. res.json({ result, count }); - }) - .catch((err) => { - next(err); - }); + } catch (e) { + return next(e); + } }); // Get an asset by id. -router.get('/:asset_id', (req, res, next) => { +router.get('/:asset_id', async (req, res, next) => { + try { - // Send back the asset. - AssetsService - .findById(req.params.asset_id) - .then((asset) => { - if (!asset) { - res.status(404).end(); - return; - } + // Send back the asset. + let asset = await AssetsService.findById(req.params.asset_id); + if (!asset) { + return next(errors.ErrNotFound); + } - res.json(asset); - }) - .catch((err) => { - next(err); - }); + return res.json(asset); + } catch (e) { + return next(e); + } }); // Adds the asset id to the queue to be scraped. -router.post('/:asset_id/scrape', (req, res, next) => { +router.post('/:asset_id/scrape', async (req, res, next) => { + try { - // Create a new asset scrape job. - AssetsService - .findById(req.params.asset_id) - .then((asset) => { - if (!asset) { - res.status(404).end(); - return; - } + // Send back the asset. + let asset = await AssetsService.findById(req.params.asset_id); + if (!asset) { + return next(errors.ErrNotFound); + } - return scraper - .create(asset) - .then((job) => { + let job = await scraper.create(asset); - // Send the job back for monitoring. - res.status(201).json(job); - }); - }) - .catch((err) => { - next(err); - }); + // Send the job back for monitoring. + res.status(201).json(job); + } catch (e) { + return next(e); + } }); -router.put('/:asset_id/settings', (req, res, next) => { - - // Override the settings for the asset. - AssetsService - .overrideSettings(req.params.asset_id, req.body) - .then(() => { - res.status(204).end(); - }) - .catch((err) => { - next(err); - }); +router.put('/:asset_id/settings', async (req, res, next) => { + try { + await AssetsService.overrideSettings(req.params.asset_id, req.body); + res.status(204).end(); + } catch (e) { + return next(e); + } }); -router.put('/:asset_id/status', (req, res, next) => { - - const id = req.params.asset_id; - +router.put('/:asset_id/status', async (req, res, next) => { const { closedAt, closedMessage } = req.body; - AssetModel - .update({id}, { + try { + await AssetModel.update({ + id: req.params.asset_id + }, { $set: { closedAt, closedMessage } - }) - .then(() => { - res.status(204).json(); - }) - .catch((err) => { - next(err); }); + + res.status(204).json(); + } catch (e) { + return next(e); + } }); module.exports = router; diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index 392b04e9b..791573898 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -7,14 +7,11 @@ const router = express.Router(); * This returns the user if they are logged in. */ router.get('/', (req, res, next) => { - - if (req.user) { - return next(); + if (!req.user) { + res.status(204).end(); + return; } - res.status(204).end(); -}, (req, res) => { - // Send back the user object. res.json({user: req.user}); }); diff --git a/routes/api/index.js b/routes/api/index.js index 764612f78..a3bc552f6 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -1,6 +1,9 @@ const express = require('express'); const authorization = require('../../middleware/authorization'); const pkg = require('../../package.json'); +const { + WEBPACK +} = require('../../config'); const router = express.Router(); @@ -15,7 +18,12 @@ router.use('/users', require('./users')); router.use('/account', require('./account')); router.use('/setup', require('./setup')); -// Bind the kue handler to the /kue path. -router.use('/kue', authorization.needed('ADMIN'), require('../../services/kue').kue.app); +// Enable the kue app only if we aren't in webpack mode. +if (!WEBPACK) { + + // Bind the kue handler to the /kue path. + router.use('/kue', authorization.needed('ADMIN'), require('../../services/kue').kue.app); + +} module.exports = router; diff --git a/routes/api/settings/index.js b/routes/api/settings/index.js index 5c4b03568..fb54494d1 100644 --- a/routes/api/settings/index.js +++ b/routes/api/settings/index.js @@ -3,25 +3,22 @@ const SettingsService = require('../../../services/settings'); const router = express.Router(); -router.get('/', (req, res, next) => { - SettingsService - .retrieve() - .then((settings) => { - res.json(settings); - }) - .catch((err) => { - next(err); - }); +router.get('/', async (req, res, next) => { + try { + let settings = await SettingsService.retrieve(); + res.json(settings); + } catch (e) { + return next(e); + } }); -router.put('/', (req, res, next) => { - SettingsService - .update(req.body).then(() => { - res.status(204).end(); - }) - .catch((err) => { - next(err); - }); +router.put('/', async (req, res, next) => { + try { + await SettingsService.update(req.body); + res.status(204).end(); + } catch (e) { + return next(e); + } }); module.exports = router; diff --git a/routes/api/setup/index.js b/routes/api/setup/index.js index dfddc64d9..efb3c5d45 100644 --- a/routes/api/setup/index.js +++ b/routes/api/setup/index.js @@ -4,48 +4,33 @@ const SetupService = require('../../../services/setup'); const router = express.Router(); -router.get('/', (req, res, next) => { - SetupService - .isAvailable() - .then(() => { - res.json({installed: false}); - }) - .catch(() => { - res.json({installed: true}); - }); +router.get('/', async (req, res, next) => { + try { + await SetupService.isAvailable(); + res.json({installed: false}); + } catch (e) { + res.json({installed: true}); + } }); -router.post('/', (req, res, next) => { - - SetupService - .isAvailable() - .then(() => { - - // Allow the request to keep going here. - next(); - }) - .catch((err) => { - next(err); - }); - -}, (req, res, next) => { +router.post('/', async (req, res, next) => { + try { + await SetupService.isAvailable(); + } catch (e) { + return next(e); + } const { settings, user: {email, password, username} } = req.body; - SetupService - .setup({settings, user: {email, password, username}}) - .then(() => { - - // We're setup! - res.status(204).end(); - }) - .catch((err) => { - next(err); - }); - + try { + await SetupService.setup({settings, user: {email, password, username}}); + res.status(204).end(); + } catch (err) { + return next(err); + } }); module.exports = router; diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 47ff46853..60c568253 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -2,7 +2,6 @@ const express = require('express'); const router = express.Router(); const UsersService = require('../../../services/users'); const mailer = require('../../../services/mailer'); -const pubsub = require('../../../services/pubsub'); const errors = require('../../../errors'); const authorization = require('../../../middleware/authorization'); const i18n = require('../../../services/i18n'); @@ -11,7 +10,7 @@ const { } = require('../../../config'); // get a list of users. -router.get('/', authorization.needed('ADMIN'), (req, res, next) => { +router.get('/', authorization.needed('ADMIN'), async (req, res, next) => { const { value = '', field = 'created_at', @@ -20,15 +19,17 @@ router.get('/', authorization.needed('ADMIN'), (req, res, next) => { limit = 50 // Total Per Page } = req.query; - Promise.all([ - UsersService + try { + + let [result, count] = await Promise.all([ + UsersService .search(value) .sort({[field]: (asc === 'true') ? 1 : -1}) .skip((page - 1) * limit) .limit(limit), - UsersService.count() - ]) - .then(([result, count]) => { + UsersService.count() + ]); + res.json({ result, limit: Number(limit), @@ -36,72 +37,74 @@ router.get('/', authorization.needed('ADMIN'), (req, res, next) => { page: Number(page), totalPages: Math.ceil(count / (limit === 0 ? 1 : limit)) }); - }) - .catch(next); + + } catch (e) { + next(e); + } + }); -router.post('/:user_id/role', authorization.needed('ADMIN'), (req, res, next) => { - UsersService - .addRoleToUser(req.params.user_id, req.body.role) - .then(() => { - res.status(204).end(); - }) - .catch(next); +router.post('/:user_id/role', authorization.needed('ADMIN'), async (req, res, next) => { + try { + await UsersService.addRoleToUser(req.params.user_id, req.body.role); + res.status(204).end(); + } catch (e) { + next(e); + } }); // update the status of a user -router.post('/:user_id/status', authorization.needed('ADMIN'), (req, res, next) => { - UsersService - .setStatus(req.params.user_id, req.body.status) - .then((user) => { +router.post('/:user_id/status', authorization.needed('ADMIN'), async (req, res, next) => { + let {status} = req.body; - // TODO: current updating status behavior is weird. - if (user) { - if (user.status === 'BANNED') { - pubsub.publish('userBanned', user); - } - res.status(201).json(user.status); - } else { - res.status(500).json(); - } - }) - .catch(next); + try { + let user = await UsersService.setStatus(req.params.user_id, status); + if (!user) { + return next(errors.ErrNotFound); + } + + if (user.status === 'BANNED') { + req.pubsub.publish('userBanned', user); + } + + // TODO: investigate why this is returning a value? Also why is this a POST vs PUT? + res.status(201).json(user.status); + } catch (e) { + next(e); + } }); -router.post('/:user_id/username-enable', authorization.needed('ADMIN'), (req, res, next) => { - UsersService - .toggleNameEdit(req.params.user_id, true) - .then(() => { - res.status(204).end(); +router.post('/:user_id/username-enable', authorization.needed('ADMIN'), async (req, res, next) => { + try { + await UsersService.toggleNameEdit(req.params.user_id, true); + res.status(204).end(); + } catch (e) { + next(e); + } +}); + +router.post('/:user_id/email', authorization.needed('ADMIN'), async (req, res, next) => { + try { + let user = await UsersService.findById(req.params.user_id); + + let localProfile = user.profiles.find((profile) => profile.provider === 'local'); + if (!localProfile) { + return next(errors.ErrMissingEmail); + } + + await mailer.sendSimple({ + template: 'notification', // needed to know which template to render! + locals: { // specifies the template locals. + body: req.body.body + }, + subject: req.body.subject, + to: localProfile.id // This only works if the user has registered via e-mail. }); -}); -router.post('/:user_id/email', authorization.needed('ADMIN'), (req, res, next) => { - - UsersService.findById(req.params.user_id) - .then((user) => { - let localProfile = user.profiles.find((profile) => profile.provider === 'local'); - - if (localProfile) { - const options = - { - template: 'notification', // needed to know which template to render! - locals: { // specifies the template locals. - body: req.body.body - }, - subject: req.body.subject, - to: localProfile.id // This only works if the user has registered via e-mail. - // We may want a standard way to access a user's e-mail address in the future - }; - return mailer.sendSimple(options); - } else { - res.json({error: 'User does not have an e-mail address.'}); - } - }) - .then(() => { - res.status(204).end(); - }) - .catch(next); + res.status(204).end(); + } catch (e) { + next(e); + } }); /** @@ -110,72 +113,61 @@ router.post('/:user_id/email', authorization.needed('ADMIN'), (req, res, next) = * @param {String} userID the id for the user to send the email to * @param {String} email the email for the user to send the email to */ -const SendEmailConfirmation = (app, userID, email, referer) => UsersService - .createEmailConfirmToken(userID, email, referer) - .then((token) => { - return mailer.sendSimple({ - template: 'email-confirm', // needed to know which template to render! - locals: { // specifies the template locals. - token, - rootURL: ROOT_URL, - email - }, - subject: i18n.t('email.confirm.subject'), - to: email - }); +const SendEmailConfirmation = async (app, userID, email, referer) => { + let token = await UsersService.createEmailConfirmToken(userID, email, referer); + + return mailer.sendSimple({ + template: 'email-confirm', // needed to know which template to render! + locals: { // specifies the template locals. + token, + rootURL: ROOT_URL, + email + }, + subject: i18n.t('email.confirm.subject'), + to: email }); +}; // create a local user. -router.post('/', (req, res, next) => { +router.post('/', async (req, res, next) => { const {email, password, username} = req.body; const redirectUri = req.header('X-Pym-Url') || req.header('Referer'); - UsersService - .createLocalUser(email, password, username) - .then((user) => { + try { + let user = await UsersService.createLocalUser(email, password, username); // Send an email confirmation. The Front end will know about the // requireEmailConfirmation as it's included in the settings get endpoint. - return SendEmailConfirmation(req.app, user.id, email, redirectUri) - .then(() => { + await SendEmailConfirmation(req.app, user.id, email, redirectUri); - // Then send back the user. - res.status(201).json(user); - }); - }) - .catch((err) => { - next(err); - }); + res.status(201).json(user); + } catch (e) { + return next(e); + } }); -router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { +router.post('/:user_id/actions', authorization.needed(), async (req, res, next) => { const { action_type, metadata } = req.body; - UsersService - .addAction(req.params.user_id, req.user.id, action_type, metadata) - .then((action) => { + try { + let action = await UsersService.addAction(req.params.user_id, req.user.id, action_type, metadata); - // Set the user status to "pending" for review by moderators - if (action_type === 'FLAG') { - return UsersService.setStatus(req.params.user_id, 'PENDING') - .then(() => action); - } else { - return action; - } - }) - .then((action) => { - res.status(201).json(action); - }) - .catch((err) => { - next(err); - }); + // Set the user status to "pending" for review by moderators + if (action_type === 'FLAG') { + await UsersService.setStatus(req.params.user_id, 'PENDING'); + } + + res.status(201).json(action); + } catch (e) { + return next(e); + } }); // trigger an email confirmation re-send by a new user -router.post('/resend-verify', (req, res, next) => { +router.post('/resend-verify', async (req, res, next) => { const {email} = req.body; const redirectUri = req.header('X-Pym-Url') || req.header('Referer'); @@ -183,48 +175,45 @@ router.post('/resend-verify', (req, res, next) => { return next(errors.ErrMissingEmail); } - // find user by email. - // if the local profile is verified, return an error code? - // send a 204 after the email is re-sent - SendEmailConfirmation(req.app, null, email, redirectUri) - .then(() => { - res.status(204).end(); - }) - .catch(next); + try { + + // find user by email. + // if the local profile is verified, return an error code? + // send a 204 after the email is re-sent + await SendEmailConfirmation(req.app, null, email, redirectUri); + + res.status(204).end(); + } catch (e) { + return next(e); + } }); // trigger an email confirmation re-send from the admin panel -router.post('/:user_id/email/confirm', authorization.needed('ADMIN'), (req, res, next) => { +router.post('/:user_id/email/confirm', authorization.needed('ADMIN'), async (req, res, next) => { const { user_id } = req.params; - UsersService - .findById(user_id) - .then((user) => { - if (!user) { - res.status(404).end(); - return; - } + try { - // Find the first local profile. - let localProfile = user.profiles.find((profile) => profile.provider === 'local'); + let user = await UsersService.findById(user_id); + if (!user) { + return next(errors.ErrNotFound); + } - // If there was no local profile for the user, error out. - if (!localProfile) { - res.status(404).end(); - return; - } + // Find the first local profile. + let localProfile = user.profiles.find((profile) => profile.provider === 'local'); + if (!localProfile) { + return next(errors.ErrMissingEmail); + } - // Send the email to the first local profile that was found. - return SendEmailConfirmation(req.app, user.id, localProfile.id) - .then(() => { - res.status(204).end(); - }); - }) - .catch((err) => { - next(err); - }); + // Send the email to the first local profile that was found. + await SendEmailConfirmation(req.app, user.id, localProfile.id); + + res.status(204).end(); + } catch (e) { + return next(e); + } }); module.exports = router; diff --git a/services/cache.js b/services/cache.js index 25642e1d0..526178a99 100644 --- a/services/cache.js +++ b/services/cache.js @@ -2,9 +2,7 @@ const redis = require('./redis'); const debug = require('debug')('talk:services:cache'); const crypto = require('crypto'); -const cache = module.exports = { - client: redis.createClient() -}; +const cache = module.exports = {}; /** * This collects a key that may either be an array or a string and creates a @@ -70,9 +68,6 @@ if redis.call('GET', KEYS[1]) ~= false then end `; -// Stores the SHA1 hash of INCR_SCRIPT, used for executing via EVALSHA. -let INCR_SCRIPT_HASH; - // This is designed to decrement a key and add an expiry iff the key already // exists. const DECR_SCRIPT = ` @@ -82,9 +77,6 @@ if redis.call('GET', KEYS[1]) ~= false then end `; -// Stores the SHA1 hash of DECR_SCRIPT, used for executing via EVALSHA. -let DECR_SCRIPT_HASH; - // Load the script into redis and track the script hash that we will use to exec // increments on. const loadScript = (name, script) => new Promise((resolve, reject) => { @@ -121,18 +113,24 @@ const loadScript = (name, script) => new Promise((resolve, reject) => { }); }); -// Load the INCR_SCRIPT and DECR_SCRIPT into Redis. -Promise.all([ - loadScript('INCR_SCRIPT', INCR_SCRIPT), - loadScript('DECR_SCRIPT', DECR_SCRIPT) -]) -.then(([incrScriptHash, decrScriptHash]) => { - INCR_SCRIPT_HASH = incrScriptHash; - DECR_SCRIPT_HASH = decrScriptHash; -}) -.catch((err) => { - throw err; -}); +/** + * Init sets up the scripts used in Redis with the incr/decr commands. + */ +cache.init = async () => { + + // Create the redis instance. + cache.client = redis.createClient(); + + // Load the INCR_SCRIPT and DECR_SCRIPT into Redis. + let [incrScriptHash, decrScriptHash] = await Promise.all([ + loadScript('INCR_SCRIPT', INCR_SCRIPT), + loadScript('DECR_SCRIPT', DECR_SCRIPT) + ]); + + // Set the globally scoped cache hashes. + cache.INCR_SCRIPT_HASH = incrScriptHash; + cache.DECR_SCRIPT_HASH = decrScriptHash; +}; /** * This will increment a key in redis and update the expiry iff it already @@ -140,7 +138,7 @@ Promise.all([ */ cache.incr = (key, expiry, kf = keyfunc) => new Promise((resolve, reject) => { cache.client - .evalsha(INCR_SCRIPT_HASH, 1, kf(key), expiry, (err) => { + .evalsha(cache.INCR_SCRIPT_HASH, 1, kf(key), expiry, (err) => { if (err) { return reject(err); } @@ -155,7 +153,7 @@ cache.incr = (key, expiry, kf = keyfunc) => new Promise((resolve, reject) => { */ cache.decr = (key, expiry, kf = keyfunc) => new Promise((resolve, reject) => { cache.client - .evalsha(DECR_SCRIPT_HASH, 1, kf(key), expiry, (err) => { + .evalsha(cache.DECR_SCRIPT_HASH, 1, kf(key), expiry, (err) => { if (err) { return reject(err); } @@ -174,7 +172,7 @@ cache.incrMany = (keys, expiry, kf = keyfunc) => { keys.forEach((key) => { // Queue up the evalsha command. - multi.evalsha(INCR_SCRIPT_HASH, 1, kf(key), expiry); + multi.evalsha(cache.INCR_SCRIPT_HASH, 1, kf(key), expiry); }); return new Promise((resolve, reject) => { @@ -198,7 +196,7 @@ cache.decrMany = (keys, expiry, kf = keyfunc) => { keys.forEach((key) => { // Queue up the evalsha command. - multi.evalsha(DECR_SCRIPT_HASH, 1, kf(key), expiry); + multi.evalsha(cache.DECR_SCRIPT_HASH, 1, kf(key), expiry); }); return new Promise((resolve, reject) => { diff --git a/services/kue.js b/services/kue.js index ac299a3d5..b6622d0b7 100644 --- a/services/kue.js +++ b/services/kue.js @@ -8,18 +8,24 @@ const kue = module.exports.kue = require('kue'); // Note that unlike what the name createQueue suggests, it currently returns a // singleton Queue instance. So you can configure and use only a single Queue // object within your node.js process. -const Queue = module.exports.queue = kue.createQueue({ - redis: { - createClientFactory: () => redis.createClient() - } -}); +let Queue = module.exports.queue; class Task { constructor({name, attempts = 3, delay = 1000}) { + debug(`Created new Task[${name}]`); + this.name = name; this.attempts = attempts; this.delay = delay; + + if (!Queue) { + module.exports.queue = Queue = kue.createQueue({ + redis: { + createClientFactory: () => redis.createClient() + } + }); + } } /** diff --git a/services/mailer.js b/services/mailer.js index 27672a35c..05cc7e88a 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -75,9 +75,18 @@ const mailer = module.exports = { /** * Create the new Task kue. */ - task: new kue.Task({ - name: 'mailer' - }), + _task: null, + get task() { + if (mailer._task) { + return mailer._task; + } + + mailer._task = new kue.Task({ + name: 'mailer' + }); + + return mailer._task; + }, sendSimple({template, locals, to, subject}) { diff --git a/services/mongoose.js b/services/mongoose.js index c697e38ef..c11bdbb78 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -4,7 +4,8 @@ const enabled = require('debug').enabled; const queryDebuger = require('debug')('talk:db:query'); const { - MONGO_URL + MONGO_URL, + WEBPACK } = require('../config'); // Loading the formatter from Mongoose: @@ -40,14 +41,23 @@ if (enabled('talk:db')) { mongoose.set('debug', debugQuery); } -// Connect to the Mongo instance. -mongoose.connect(MONGO_URL, (err) => { - if (err) { - throw err; - } +if (WEBPACK) { - debug('connection established'); -}); + console.warn('Not connecting to mongodb during webpack build'); + mongoose.disconnect(); + +} else { + + // Connect to the Mongo instance. + mongoose.connect(MONGO_URL, (err) => { + if (err) { + throw err; + } + + debug('connection established'); + }); + +} module.exports = mongoose; diff --git a/services/passport.js b/services/passport.js index 8396cc581..ba83e7c7d 100644 --- a/services/passport.js +++ b/services/passport.js @@ -9,12 +9,21 @@ const LocalStrategy = require('passport-local').Strategy; const errors = require('../errors'); const uuid = require('uuid'); const debug = require('debug')('talk:services:passport'); -const {createClient} = require('./redis'); const bowser = require('bowser'); const ms = require('ms'); // Create a redis client to use for authentication. -const client = createClient(); +const {createClient} = require('./redis'); +let _client = null; +const getClient = () => { + if (_client) { + return _client; + } + + _client = createClient(); + + return _client; +}; const { JWT_SECRET, @@ -148,7 +157,7 @@ const HandleLogout = (req, res, next) => { const now = new Date(); const expiry = (jwt.exp - now.getTime() / 1000).toFixed(0); - client.set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry, (err) => { + getClient().set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry, (err) => { if (err) { return next(err); } @@ -159,7 +168,7 @@ const HandleLogout = (req, res, next) => { }; const checkGeneralTokenBlacklist = (jwt) => new Promise((resolve, reject) => { - client.get(`jtir[${jwt.jti}]`, (err, expiry) => { + getClient().get(`jtir[${jwt.jti}]`, (err, expiry) => { if (err) { return reject(err); } diff --git a/services/pubsub.js b/services/pubsub.js index a42ffb314..c54097915 100644 --- a/services/pubsub.js +++ b/services/pubsub.js @@ -2,4 +2,13 @@ const {RedisPubSub} = require('graphql-redis-subscriptions'); const {connectionOptions} = require('./redis'); -module.exports = new RedisPubSub({connection: connectionOptions}); +let pubsub = null; +module.exports = () => { + if (pubsub) { + return pubsub; + } + + pubsub = new RedisPubSub({connection: connectionOptions}); + + return pubsub; +}; diff --git a/services/scraper.js b/services/scraper.js index aa56bf61b..2f424185f 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -12,11 +12,20 @@ const metascraper = require('metascraper'); const scraper = { /** - * Create the new Task kue. + * Create the new Task kue singleton. */ - task: new kue.Task({ - name: 'scraper' - }), + _task: null, + get task() { + if (scraper._task) { + return scraper._task; + } + + scraper._task = new kue.Task({ + name: 'scraper' + }); + + return scraper._task; + }, /** * Creates a new scraper job and scrapes the url when it gets processed. diff --git a/services/users.js b/services/users.js index 13dddaf04..598d93760 100644 --- a/services/users.js +++ b/services/users.js @@ -11,9 +11,6 @@ const { } = require('../config'); const debug = require('debug')('talk:services:users'); -const redis = require('./redis'); -const redisClient = redis.createClient(); - const UserModel = require('../models/user'); const USER_STATUS = require('../models/enum/user_status'); const USER_ROLES = require('../models/enum/user_roles'); @@ -32,6 +29,19 @@ const PASSWORD_RESET_JWT_SUBJECT = 'password_reset'; // through during the salting process. const SALT_ROUNDS = 10; +// Create a redis client to use for authentication. +const {createClient} = require('./redis'); +let _client = null; +const getClient = () => { + if (_client) { + return _client; + } + + _client = createClient(); + + return _client; +}; + // UsersService is the interface for the application to interact with the // UserModel through. module.exports = class UsersService { @@ -67,7 +77,7 @@ module.exports = class UsersService { const rdskey = `la[${email.toLowerCase().trim()}]`; return new Promise((resolve, reject) => { - redisClient + getClient() .multi() .incr(rdskey) .expire(rdskey, RECAPTCHA_WINDOW_SECONDS) @@ -80,7 +90,7 @@ module.exports = class UsersService { if (replies[0] === 1 || replies[1] === -1) { // then expire it after the timeout - redisClient.expire(rdskey, RECAPTCHA_WINDOW_SECONDS); + getClient().expire(rdskey, RECAPTCHA_WINDOW_SECONDS); } if (replies[0] >= RECAPTCHA_INCORRECT_TRIGGER) { @@ -102,7 +112,7 @@ module.exports = class UsersService { const rdskey = `la[${email.toLowerCase().trim()}]`; return new Promise((resolve, reject) => { - redisClient + getClient() .get(rdskey, (err, reply) => { if (err) { return reject(err); diff --git a/test/server/redis.js b/test/server/redis.js index 529548918..454a2afd8 100644 --- a/test/server/redis.js +++ b/test/server/redis.js @@ -1,3 +1,4 @@ const redis = require('../helpers/redis'); +const cache = require('../../services/cache'); -beforeEach(() => redis.clearDB()); +beforeEach(() => Promise.all([redis.clearDB(), cache.init()])); From 4b74c3312ff4b6f48d532d33696d76ddce5ede1e Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Mon, 24 Jul 2017 15:36:00 -0300 Subject: [PATCH 02/18] typo --- graph/loaders/comments.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graph/loaders/comments.js b/graph/loaders/comments.js index b0475d3ef..16d2101f7 100644 --- a/graph/loaders/comments.js +++ b/graph/loaders/comments.js @@ -445,12 +445,12 @@ const genRecentComments = (_, ids) => { }; /** - * genComments returns the comments by the id's. Only admins can see non-public comments. + * getComments returns the comments by the id's. Only admins can see non-public comments. * @param {Object} context graph context * @param {Array} ids the comment id's to fetch * @return {Promise} resolves to the comments */ -const genComments = ({user}, ids) => { +const getComments = ({user}, ids) => { let comments; if (user && user.can(SEARCH_OTHERS_COMMENTS)) { comments = CommentModel.find({ @@ -478,7 +478,7 @@ const genComments = ({user}, ids) => { */ module.exports = (context) => ({ Comments: { - get: new DataLoader((ids) => genComments(context, ids)), + get: new DataLoader((ids) => getComments(context, ids)), getByQuery: (query) => getCommentsByQuery(context, query), getCountByQuery: (query) => getCommentCountByQuery(context, query), countByAssetID: new SharedCounterDataLoader('Comments.totalCommentCount', 3600, (ids) => getCountsByAssetID(context, ids)), From 36df7cb7842db4cc52e1433af796e01c79f015b0 Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Mon, 24 Jul 2017 16:31:24 -0300 Subject: [PATCH 03/18] Extended Asset with comment --- .../src/components/Stream.js | 2 +- .../src/containers/Stream.js | 18 +++++++++--------- graph/resolvers/asset.js | 6 ++++++ graph/typeDefs.graphql | 3 +++ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index 2a1381862..e6e184dfa 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -81,7 +81,7 @@ class Stream extends React.Component { setActiveReplyBox, appendItemArray, commentClassNames, - root: {asset, asset: {comments, totalCommentCount}, comment, me}, + root: {asset, asset: {comment, comments, totalCommentCount}, me}, postComment, addNotification, editComment, diff --git a/client/coral-embed-stream/src/containers/Stream.js b/client/coral-embed-stream/src/containers/Stream.js index ea0676e51..804f3d37e 100644 --- a/client/coral-embed-stream/src/containers/Stream.js +++ b/client/coral-embed-stream/src/containers/Stream.js @@ -237,16 +237,16 @@ const slots = [ const fragments = { root: gql` fragment CoralEmbedStream_Stream_root on RootQuery { - comment(id: $commentId) @include(if: $hasComment) { - ...CoralEmbedStream_Stream_comment - ${nest(` - parent { - ...CoralEmbedStream_Stream_comment - ...nest - } - `, THREADING_LEVEL)} - } asset(id: $assetId, url: $assetUrl) { + comment(id: $commentId) @include(if: $hasComment) { + ...CoralEmbedStream_Stream_comment + ${nest(` + parent { + ...CoralEmbedStream_Stream_comment + ...nest + } + `, THREADING_LEVEL)} + } id title url diff --git a/graph/resolvers/asset.js b/graph/resolvers/asset.js index 86e102631..497ea0f9b 100644 --- a/graph/resolvers/asset.js +++ b/graph/resolvers/asset.js @@ -4,6 +4,12 @@ const Asset = { recentComments({id}, _, {loaders: {Comments}}) { return Comments.genRecentComments.load(id); }, + comment({id}, {id: commentId}, {loaders: {Comments}}) { + return Comments.getByQuery({ + asset_id: id, + ids: commentId + }).then((c) => c.nodes[0]); + }, comments({id}, {sort, limit, deep, excludeIgnored, tags}, {loaders: {Comments}}) { return Comments.getByQuery({ asset_id: id, diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index a154f9f90..999aea51c 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -580,6 +580,9 @@ type Asset { deep: Boolean, ): CommentConnection! + # A Comment from the Asset by comment's ID + comment(id: ID!): Comment + # The count of top level comments on the asset. commentCount(excludeIgnored: Boolean, tags: [String!]): Int From a036a051cb8a2eb3f6f9ce20ecd054c6138b90bf Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Mon, 24 Jul 2017 16:40:08 -0300 Subject: [PATCH 04/18] Using async await --- graph/resolvers/asset.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/graph/resolvers/asset.js b/graph/resolvers/asset.js index 497ea0f9b..da9114c6a 100644 --- a/graph/resolvers/asset.js +++ b/graph/resolvers/asset.js @@ -4,11 +4,13 @@ const Asset = { recentComments({id}, _, {loaders: {Comments}}) { return Comments.genRecentComments.load(id); }, - comment({id}, {id: commentId}, {loaders: {Comments}}) { - return Comments.getByQuery({ + async comment({id}, {id: commentId}, {loaders: {Comments}}) { + const comments = await Comments.getByQuery({ asset_id: id, ids: commentId - }).then((c) => c.nodes[0]); + }); + + return comments.nodes[0]; }, comments({id}, {sort, limit, deep, excludeIgnored, tags}, {loaders: {Comments}}) { return Comments.getByQuery({ From be46ac826ba56747eb647ca47c392b4aa6026a86 Mon Sep 17 00:00:00 2001 From: Kim Gardner Date: Mon, 24 Jul 2017 16:51:35 -0400 Subject: [PATCH 05/18] Update version to 3.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9cb065320..6804944a9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "talk", - "version": "2.5.0", + "version": "3.0.0", "description": "A better commenting experience from Mozilla, The New York Times, and the Washington Post. https://coralproject.net", "main": "app.js", "scripts": { From 3a7353e4fe6700cb605e7cd15ef12ab63d9f845c Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Mon, 24 Jul 2017 23:15:30 -0300 Subject: [PATCH 06/18] fix for plugin seed builder --- bin/cli-plugins | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/cli-plugins b/bin/cli-plugins index 942036d29..8486af093 100755 --- a/bin/cli-plugins +++ b/bin/cli-plugins @@ -274,7 +274,7 @@ async function reconcilePluginDeps({skipLocal, skipRemote, dryRun, upgradeRemote } async function createSeedPlugin() { - const pluginsDir = path.join(__dirname, 'plugins'); + const pluginsDir = path.join(dir, 'plugins'); function pluginNameExists(pluginName) { const pluginNames = fs.readdirSync(pluginsDir); @@ -321,7 +321,7 @@ async function createSeedPlugin() { // Creating plugin seed //============================================================================== - const seedPlugin = path.join(__dirname, 'bin/templates/plugin'); + const seedPlugin = path.join(dir, 'bin/templates/plugin'); const newPluginPath = path.join(pluginsDir, answers.pluginName); if (fs.existsSync(seedPlugin)) { From fac63099c79360fabb5cb4fd3453036ab334aa4c Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Mon, 24 Jul 2017 23:26:18 -0300 Subject: [PATCH 07/18] Using path.resolve instead --- bin/cli-plugins | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/cli-plugins b/bin/cli-plugins index 8486af093..1de1c97d4 100755 --- a/bin/cli-plugins +++ b/bin/cli-plugins @@ -274,7 +274,7 @@ async function reconcilePluginDeps({skipLocal, skipRemote, dryRun, upgradeRemote } async function createSeedPlugin() { - const pluginsDir = path.join(dir, 'plugins'); + const pluginsDir = path.resolve(__dirname, '..', 'plugins'); function pluginNameExists(pluginName) { const pluginNames = fs.readdirSync(pluginsDir); @@ -321,7 +321,7 @@ async function createSeedPlugin() { // Creating plugin seed //============================================================================== - const seedPlugin = path.join(dir, 'bin/templates/plugin'); + const seedPlugin = path.join(__dirname, 'templates/plugin'); const newPluginPath = path.join(pluginsDir, answers.pluginName); if (fs.existsSync(seedPlugin)) { @@ -355,7 +355,7 @@ async function createSeedPlugin() { // Let's add this to the plugins.json if (answers.addPluginsJson) { - const pluginsJson = path.join(dir, 'plugins.json'); + const pluginsJson = path.resolve(__dirname, '..', 'plugins.json'); fs.readJson(pluginsJson) .then((j) => { From c2f6b9aa0906fdde30446d7d20ae5e9a45e97032 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 12:32:15 +1000 Subject: [PATCH 08/18] Suggestions for refactoring --- graph/context.js | 2 +- graph/subscriptions.js | 2 +- services/kue.js | 4 ++-- services/mailer.js | 9 +++++---- services/passport.js | 16 +++------------- services/pubsub.js | 18 ++++++++++-------- services/redis.js | 35 +++++++++++++++++++++++++---------- services/scraper.js | 11 ++++++----- services/users.js | 18 ++++-------------- 9 files changed, 57 insertions(+), 58 deletions(-) diff --git a/graph/context.js b/graph/context.js index 0a94dc30b..cffec0896 100644 --- a/graph/context.js +++ b/graph/context.js @@ -53,7 +53,7 @@ class Context { this.plugins = decorateContextPlugins(this, contextPlugins); // Bind the publish/subscribe to the context. - this.pubsub = pubsub(); + this.pubsub = pubsub.createClient(); } } diff --git a/graph/subscriptions.js b/graph/subscriptions.js index ca505b96d..b8b4f2c0f 100644 --- a/graph/subscriptions.js +++ b/graph/subscriptions.js @@ -133,7 +133,7 @@ const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plu const createSubscriptionManager = (server) => new SubscriptionServer({ subscriptionManager: new SubscriptionManager({ schema, - pubsub: pubsub(), + pubsub: pubsub.createClient(), setupFunctions, }), onConnect: ({token}, connection) => { diff --git a/services/kue.js b/services/kue.js index b6622d0b7..ffaa817a4 100644 --- a/services/kue.js +++ b/services/kue.js @@ -8,7 +8,7 @@ const kue = module.exports.kue = require('kue'); // Note that unlike what the name createQueue suggests, it currently returns a // singleton Queue instance. So you can configure and use only a single Queue // object within your node.js process. -let Queue = module.exports.queue; +let Queue = module.exports.queue = null; class Task { @@ -22,7 +22,7 @@ class Task { if (!Queue) { module.exports.queue = Queue = kue.createQueue({ redis: { - createClientFactory: () => redis.createClient() + createClientFactory: redis.createClientFactory() } }); } diff --git a/services/mailer.js b/services/mailer.js index 05cc7e88a..2e0e9d1a7 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -69,6 +69,7 @@ if (SMTP_PORT) { } const defaultTransporter = nodemailer.createTransport(options); +let taskInstance = null; const mailer = module.exports = { @@ -77,15 +78,15 @@ const mailer = module.exports = { */ _task: null, get task() { - if (mailer._task) { - return mailer._task; + if (taskInstance) { + return taskInstance; } - mailer._task = new kue.Task({ + taskInstance = new kue.Task({ name: 'mailer' }); - return mailer._task; + return taskInstance; }, sendSimple({template, locals, to, subject}) { diff --git a/services/passport.js b/services/passport.js index ba83e7c7d..9fc2631b0 100644 --- a/services/passport.js +++ b/services/passport.js @@ -13,17 +13,7 @@ const bowser = require('bowser'); const ms = require('ms'); // Create a redis client to use for authentication. -const {createClient} = require('./redis'); -let _client = null; -const getClient = () => { - if (_client) { - return _client; - } - - _client = createClient(); - - return _client; -}; +const {createClientFactory} = require('./redis'); const { JWT_SECRET, @@ -157,7 +147,7 @@ const HandleLogout = (req, res, next) => { const now = new Date(); const expiry = (jwt.exp - now.getTime() / 1000).toFixed(0); - getClient().set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry, (err) => { + createClientFactory().set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry, (err) => { if (err) { return next(err); } @@ -168,7 +158,7 @@ const HandleLogout = (req, res, next) => { }; const checkGeneralTokenBlacklist = (jwt) => new Promise((resolve, reject) => { - getClient().get(`jtir[${jwt.jti}]`, (err, expiry) => { + createClientFactory().get(`jtir[${jwt.jti}]`, (err, expiry) => { if (err) { return reject(err); } diff --git a/services/pubsub.js b/services/pubsub.js index c54097915..56e0f3e4d 100644 --- a/services/pubsub.js +++ b/services/pubsub.js @@ -2,13 +2,15 @@ const {RedisPubSub} = require('graphql-redis-subscriptions'); const {connectionOptions} = require('./redis'); -let pubsub = null; -module.exports = () => { - if (pubsub) { - return pubsub; +let pubsubInstance = null; +module.exports = { + createClient: () => { + if (pubsubInstance) { + return pubsubInstance; + } + + pubsubInstance = new RedisPubSub({connection: connectionOptions}); + + return pubsubInstance; } - - pubsub = new RedisPubSub({connection: connectionOptions}); - - return pubsub; }; diff --git a/services/redis.js b/services/redis.js index c6506eb3c..1983249f1 100644 --- a/services/redis.js +++ b/services/redis.js @@ -29,21 +29,36 @@ const connectionOptions = { } }; +const createClient = () => { + let client = redis.createClient(connectionOptions); + + client.ping((err) => { + if (err) { + console.error('Can\'t ping the redis server!'); + + throw err; + } + + debug('connection established'); + }); + + return client; +}; + module.exports = { connectionOptions, - createClient() { - let client = redis.createClient(connectionOptions); + createClient, + createClientFactory: () => { + let client = null; - client.ping((err) => { - if (err) { - console.error('Can\'t ping the redis server!'); - - throw err; + return () => { + if (client) { + return client; } - debug('connection established'); - }); + client = createClient(); - return client; + return client; + }; } }; diff --git a/services/scraper.js b/services/scraper.js index 2f424185f..66eb32da6 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -5,6 +5,8 @@ const AssetsService = require('./assets'); const metascraper = require('metascraper'); +let taskInstance = null; + /** * Exposes a service object to allow operations to execute against the scraper. * @type {Object} @@ -14,17 +16,16 @@ const scraper = { /** * Create the new Task kue singleton. */ - _task: null, get task() { - if (scraper._task) { - return scraper._task; + if (taskInstance) { + return taskInstance; } - scraper._task = new kue.Task({ + taskInstance = new kue.Task({ name: 'scraper' }); - return scraper._task; + return taskInstance; }, /** diff --git a/services/users.js b/services/users.js index 7abf62cb4..83a8c3cbe 100644 --- a/services/users.js +++ b/services/users.js @@ -30,17 +30,7 @@ const PASSWORD_RESET_JWT_SUBJECT = 'password_reset'; const SALT_ROUNDS = 10; // Create a redis client to use for authentication. -const {createClient} = require('./redis'); -let _client = null; -const getClient = () => { - if (_client) { - return _client; - } - - _client = createClient(); - - return _client; -}; +const {createClientFactory} = require('./redis'); // UsersService is the interface for the application to interact with the // UserModel through. @@ -77,7 +67,7 @@ module.exports = class UsersService { const rdskey = `la[${email.toLowerCase().trim()}]`; return new Promise((resolve, reject) => { - getClient() + createClientFactory() .multi() .incr(rdskey) .expire(rdskey, RECAPTCHA_WINDOW_SECONDS) @@ -90,7 +80,7 @@ module.exports = class UsersService { if (replies[0] === 1 || replies[1] === -1) { // then expire it after the timeout - getClient().expire(rdskey, RECAPTCHA_WINDOW_SECONDS); + createClientFactory().expire(rdskey, RECAPTCHA_WINDOW_SECONDS); } if (replies[0] >= RECAPTCHA_INCORRECT_TRIGGER) { @@ -112,7 +102,7 @@ module.exports = class UsersService { const rdskey = `la[${email.toLowerCase().trim()}]`; return new Promise((resolve, reject) => { - getClient() + createClientFactory() .get(rdskey, (err, reply) => { if (err) { return reject(err); From f13e6b0e16165e8bcabe7d601859ca796115a56d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 12:38:49 +1000 Subject: [PATCH 09/18] Fixes to impl --- app.js | 2 +- services/passport.js | 5 +++-- services/users.js | 8 +++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app.js b/app.js index d7f46ceef..a6b669b0e 100644 --- a/app.js +++ b/app.js @@ -101,7 +101,7 @@ app.use('/api', authentication); app.use('/api', (req, res, next) => { // Attach the pubsub handle to the requests. - req.pubsub = pubsub(); + req.pubsub = pubsub.createClient(); // Forward on the request. next(); diff --git a/services/passport.js b/services/passport.js index 9fc2631b0..acf3043a9 100644 --- a/services/passport.js +++ b/services/passport.js @@ -14,6 +14,7 @@ const ms = require('ms'); // Create a redis client to use for authentication. const {createClientFactory} = require('./redis'); +const client = createClientFactory(); const { JWT_SECRET, @@ -147,7 +148,7 @@ const HandleLogout = (req, res, next) => { const now = new Date(); const expiry = (jwt.exp - now.getTime() / 1000).toFixed(0); - createClientFactory().set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry, (err) => { + client().set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry, (err) => { if (err) { return next(err); } @@ -158,7 +159,7 @@ const HandleLogout = (req, res, next) => { }; const checkGeneralTokenBlacklist = (jwt) => new Promise((resolve, reject) => { - createClientFactory().get(`jtir[${jwt.jti}]`, (err, expiry) => { + client().get(`jtir[${jwt.jti}]`, (err, expiry) => { if (err) { return reject(err); } diff --git a/services/users.js b/services/users.js index 83a8c3cbe..12da5ecce 100644 --- a/services/users.js +++ b/services/users.js @@ -32,6 +32,8 @@ const SALT_ROUNDS = 10; // Create a redis client to use for authentication. const {createClientFactory} = require('./redis'); +const client = createClientFactory(); + // UsersService is the interface for the application to interact with the // UserModel through. module.exports = class UsersService { @@ -67,7 +69,7 @@ module.exports = class UsersService { const rdskey = `la[${email.toLowerCase().trim()}]`; return new Promise((resolve, reject) => { - createClientFactory() + client() .multi() .incr(rdskey) .expire(rdskey, RECAPTCHA_WINDOW_SECONDS) @@ -80,7 +82,7 @@ module.exports = class UsersService { if (replies[0] === 1 || replies[1] === -1) { // then expire it after the timeout - createClientFactory().expire(rdskey, RECAPTCHA_WINDOW_SECONDS); + client().expire(rdskey, RECAPTCHA_WINDOW_SECONDS); } if (replies[0] >= RECAPTCHA_INCORRECT_TRIGGER) { @@ -102,7 +104,7 @@ module.exports = class UsersService { const rdskey = `la[${email.toLowerCase().trim()}]`; return new Promise((resolve, reject) => { - createClientFactory() + client() .get(rdskey, (err, reply) => { if (err) { return reject(err); From 07114aed1b1d6ad67f15b47e78115657f6c705b3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 12:40:54 +1000 Subject: [PATCH 10/18] removed unused var --- services/mailer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/services/mailer.js b/services/mailer.js index 2e0e9d1a7..7825ffd48 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -76,7 +76,6 @@ const mailer = module.exports = { /** * Create the new Task kue. */ - _task: null, get task() { if (taskInstance) { return taskInstance; From 0c8def722b14fa6b91372a25802753e1269f6b18 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 12:53:52 +1000 Subject: [PATCH 11/18] Refined task singleton to factory --- services/kue.js | 16 ++++++++++++++++ services/mailer.js | 10 ++-------- services/scraper.js | 11 ++--------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/services/kue.js b/services/kue.js index ffaa817a4..1b3fb605a 100644 --- a/services/kue.js +++ b/services/kue.js @@ -138,3 +138,19 @@ if (process.env.NODE_ENV === 'test') { } else { module.exports.Task = Task; } + +module.exports.createTaskFactory = () => { + let taskInstance = null; + + return (options) => { + if (taskInstance) { + return taskInstance; + } + + options = Object.assign({}, options); + + taskInstance = new module.exports.Task(options); + + return taskInstance; + }; +}; diff --git a/services/mailer.js b/services/mailer.js index 7825ffd48..7212979df 100644 --- a/services/mailer.js +++ b/services/mailer.js @@ -1,6 +1,7 @@ const debug = require('debug')('talk:services:mailer'); const nodemailer = require('nodemailer'); const kue = require('./kue'); +const taskFactory = kue.createTaskFactory(); const path = require('path'); const fs = require('fs'); const _ = require('lodash'); @@ -69,7 +70,6 @@ if (SMTP_PORT) { } const defaultTransporter = nodemailer.createTransport(options); -let taskInstance = null; const mailer = module.exports = { @@ -77,15 +77,9 @@ const mailer = module.exports = { * Create the new Task kue. */ get task() { - if (taskInstance) { - return taskInstance; - } - - taskInstance = new kue.Task({ + return taskFactory({ name: 'mailer' }); - - return taskInstance; }, sendSimple({template, locals, to, subject}) { diff --git a/services/scraper.js b/services/scraper.js index 66eb32da6..40aac29ad 100644 --- a/services/scraper.js +++ b/services/scraper.js @@ -1,12 +1,11 @@ const kue = require('./kue'); +const taskFactory = kue.createTaskFactory(); const debug = require('debug')('talk:services:scraper'); const AssetModel = require('../models/asset'); const AssetsService = require('./assets'); const metascraper = require('metascraper'); -let taskInstance = null; - /** * Exposes a service object to allow operations to execute against the scraper. * @type {Object} @@ -17,15 +16,9 @@ const scraper = { * Create the new Task kue singleton. */ get task() { - if (taskInstance) { - return taskInstance; - } - - taskInstance = new kue.Task({ + return taskFactory({ name: 'scraper' }); - - return taskInstance; }, /** From 3f304823ca1328f01c2436d7112798f0bac4508f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 15:55:40 +1000 Subject: [PATCH 12/18] Added new config for jwt alg + added query parsing of access token --- config.js | 3 +++ services/passport.js | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/config.js b/config.js index e3dd4578c..703be06be 100644 --- a/config.js +++ b/config.js @@ -34,6 +34,9 @@ const CONFIG = { // JWT_EXPIRY is the time for which a given token is valid for. JWT_EXPIRY: process.env.TALK_JWT_EXPIRY || '1 day', + // JWT_ALG is the algorithm used for signing jwt tokens. + JWT_ALG: process.env.TALK_JWT_ALG || 'HS256', + //------------------------------------------------------------------------------ // Installation locks //------------------------------------------------------------------------------ diff --git a/services/passport.js b/services/passport.js index 8396cc581..7a178039a 100644 --- a/services/passport.js +++ b/services/passport.js @@ -21,6 +21,7 @@ const { JWT_ISSUER, JWT_EXPIRY, JWT_AUDIENCE, + JWT_ALG, RECAPTCHA_SECRET, RECAPTCHA_ENABLED } = require('../config'); @@ -219,6 +220,7 @@ passport.use(new JwtStrategy({ // Prepare the extractor from the header. jwtFromRequest: ExtractJwt.fromExtractors([ cookieExtractor, + ExtractJwt.fromUrlQueryParameter('access_token'), ExtractJwt.fromAuthHeaderWithScheme('Bearer') ]), @@ -233,7 +235,7 @@ passport.use(new JwtStrategy({ audience: JWT_AUDIENCE, // Enable only the HS256 algorithm. - algorithms: ['HS256'], + algorithms: [JWT_ALG], // Pass the request object back to the callback so we can attach the JWT to // it. From 2e74e2fe62ed52a728da5b885784f6c5b004a073 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 17:29:51 +1000 Subject: [PATCH 13/18] Added introspection results to build pipeline --- .gitignore | 2 ++ package.json | 2 ++ scripts/generateIntrospectionResult.js | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100755 scripts/generateIntrospectionResult.js diff --git a/.gitignore b/.gitignore index bd8edcd94..767066028 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,8 @@ dist npm-debug.log* dump.rdb +client/coral-framework/graphql/introspection.json + .env *.cfg diff --git a/package.json b/package.json index 551250113..5e147a781 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,9 @@ "postinstall": "./bin/cli plugins reconcile --skip-remote", "start": "./bin/cli serve -j -w", "dev-start": "nodemon -w . -w bin/cli -w bin/cli-serve --config .nodemon.json --exec \"./bin/cli -c .env serve -j -w\"", + "prebuild": "WEBPACK=true NODE_ENV=test ./scripts/generateIntrospectionResult.js", "build": "WEBPACK=true NODE_ENV=production webpack -p --config webpack.config.js --bail", + "prebuild-watch": "WEBPACK=true NODE_ENV=test ./scripts/generateIntrospectionResult.js", "build-watch": "WEBPACK=true NODE_ENV=development webpack --progress --config webpack.config.js --watch", "lint": "eslint bin/* .", "lint-fix": "eslint bin/* . --fix", diff --git a/scripts/generateIntrospectionResult.js b/scripts/generateIntrospectionResult.js new file mode 100755 index 000000000..9a22e1207 --- /dev/null +++ b/scripts/generateIntrospectionResult.js @@ -0,0 +1,22 @@ +#! /usr/bin/env node + +const path = require('path'); +const introspectionFilename = path.resolve(__dirname, '..', 'client', 'coral-framework', 'graphql', 'introspection.json'); + +const fs = require('fs'); +const {graphql, introspectionQuery} = require('graphql'); +const schema = require('../graph/schema'); + +graphql(schema, introspectionQuery) + .then(({data}) => { + + // Serialize the introspection result as JSON. + const introspectionResult = JSON.stringify(data, null, 2); + + // Write the introspection result to the filesystem. + fs.writeFileSync(introspectionFilename, introspectionResult, 'utf8'); + }) + .catch((err) => { + console.error(err); + process.exit(1); + }); From 4d6271d2555c96df322ed2fc8cdae58efb1f3c46 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 25 Jul 2017 17:31:28 +1000 Subject: [PATCH 14/18] Added log about result --- scripts/generateIntrospectionResult.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/generateIntrospectionResult.js b/scripts/generateIntrospectionResult.js index 9a22e1207..a42dcd955 100755 --- a/scripts/generateIntrospectionResult.js +++ b/scripts/generateIntrospectionResult.js @@ -15,6 +15,8 @@ graphql(schema, introspectionQuery) // Write the introspection result to the filesystem. fs.writeFileSync(introspectionFilename, introspectionResult, 'utf8'); + + console.log(`Outputted result of introspectionQuery to ${introspectionFilename}`); }) .catch((err) => { console.error(err); From b4244b867172fe2a72e8c12af34420ae5ec7d72f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 25 Jul 2017 16:51:41 +0700 Subject: [PATCH 15/18] More integration with the introspection query result - Auto generate introspection on any change when using `dev-start` - Port client to use introspection matcher --- client/coral-admin/src/index.js | 2 +- client/coral-admin/src/services/client.js | 6 -- .../src/services/fragmentMatcher.js | 65 ------------------- client/coral-admin/src/services/store.js | 2 +- client/coral-framework/services/client.js | 4 +- package.json | 9 +-- .../client/containers/Comment.js | 13 ---- 7 files changed, 10 insertions(+), 91 deletions(-) delete mode 100644 client/coral-admin/src/services/client.js delete mode 100644 client/coral-admin/src/services/fragmentMatcher.js diff --git a/client/coral-admin/src/index.js b/client/coral-admin/src/index.js index 29df3753c..382f5101d 100644 --- a/client/coral-admin/src/index.js +++ b/client/coral-admin/src/index.js @@ -3,7 +3,7 @@ import {render} from 'react-dom'; import {ApolloProvider} from 'react-apollo'; import smoothscroll from 'smoothscroll-polyfill'; -import {getClient} from './services/client'; +import {getClient} from 'coral-framework/services/client'; import store from './services/store'; import App from './components/App'; diff --git a/client/coral-admin/src/services/client.js b/client/coral-admin/src/services/client.js deleted file mode 100644 index 934de2c9e..000000000 --- a/client/coral-admin/src/services/client.js +++ /dev/null @@ -1,6 +0,0 @@ -import {getClient as getFrameworkClient} from 'coral-framework/services/client'; -import fragmentMatcher from './fragmentMatcher'; - -export function getClient() { - return getFrameworkClient({fragmentMatcher}); -} diff --git a/client/coral-admin/src/services/fragmentMatcher.js b/client/coral-admin/src/services/fragmentMatcher.js deleted file mode 100644 index 231c0fa2b..000000000 --- a/client/coral-admin/src/services/fragmentMatcher.js +++ /dev/null @@ -1,65 +0,0 @@ -import {IntrospectionFragmentMatcher} from 'apollo-client'; - -// TODO this is a short-term fix -// we need to set up something to query the server for the schema before ApolloClient initialization -// https://github.com/apollographql/apollo-client/issues/1555#issuecomment-295834774 -const fm = new IntrospectionFragmentMatcher({ - introspectionQueryResultData: { - __schema: { - types: [ - { - kind: 'INTERFACE', - name: 'UserError', - possibleTypes: [ - {name: 'GenericUserError'}, - {name: 'ValidationUserError'} - ] - }, - { - kind: 'INTERFACE', - name: 'Response', - possibleTypes: [ - {name: 'CreateCommentResponse'}, - {name: 'CreateFlagResponse'}, - {name: 'CreateDontAgreeResponse'}, - {name: 'DeleteActionResponse'}, - {name: 'SetUserStatusResponse'}, - {name: 'SuspendUserResponse'}, - {name: 'SetCommentStatusResponse'}, - {name: 'ModifyTagResponse'}, - {name: 'IgnoreUserResponse'}, - {name: 'StopIgnoringUserResponse'} - ] - }, - { - kind: 'INTERFACE', - name: 'Action', - possibleTypes: [ - {name: 'DefaultAction'}, - {name: 'FlagAction'}, - {name: 'DontAgreeAction'} - ] - }, - { - kind: 'INTERFACE', - name: 'ActionSummary', - possibleTypes: [ - {name: 'DefaultActionSummary'}, - {name: 'FlagActionSummary'}, - {name: 'DontAgreeActionSummary'} - ] - }, - { - kind: 'INTERFACE', - name: 'AssetActionSummary', - possibleTypes: [ - {name: 'DefaultAssetActionSummary'}, - {name: 'FlagAssetActionSummary'} - ] - } - ] - } - } -}); - -export default fm; diff --git a/client/coral-admin/src/services/store.js b/client/coral-admin/src/services/store.js index 06cb3c758..ca29c15c8 100644 --- a/client/coral-admin/src/services/store.js +++ b/client/coral-admin/src/services/store.js @@ -1,7 +1,7 @@ import {createStore, combineReducers, applyMiddleware, compose} from 'redux'; import thunk from 'redux-thunk'; import mainReducer from '../reducers'; -import {getClient} from './client'; +import {getClient} from 'coral-framework/services/client'; const middlewares = [ applyMiddleware(getClient().middleware()), diff --git a/client/coral-framework/services/client.js b/client/coral-framework/services/client.js index 307646db0..b266822fb 100644 --- a/client/coral-framework/services/client.js +++ b/client/coral-framework/services/client.js @@ -1,8 +1,9 @@ -import ApolloClient, {addTypename} from 'apollo-client'; +import ApolloClient, {addTypename, IntrospectionFragmentMatcher} from 'apollo-client'; import {networkInterface} from './transport'; import {SubscriptionClient, addGraphQLSubscriptions} from 'subscriptions-transport-ws'; import MessageTypes from 'subscriptions-transport-ws/dist/message-types'; import {getAuthToken} from '../helpers/request'; +import introspectionQueryResultData from '../graphql/introspection.json'; let client, wsClient = null, wsClientToken = null; @@ -59,6 +60,7 @@ export function getClient(options = {}) { ...options, connectToDevTools: true, addTypename: true, + fragmentMatcher: new IntrospectionFragmentMatcher({introspectionQueryResultData}), queryTransformer: addTypename, dataIdFromObject: (result) => { if (result.id && result.__typename) { // eslint-disable-line no-underscore-dangle diff --git a/package.json b/package.json index 5e147a781..2c7ee6a69 100644 --- a/package.json +++ b/package.json @@ -6,10 +6,10 @@ "scripts": { "postinstall": "./bin/cli plugins reconcile --skip-remote", "start": "./bin/cli serve -j -w", - "dev-start": "nodemon -w . -w bin/cli -w bin/cli-serve --config .nodemon.json --exec \"./bin/cli -c .env serve -j -w\"", - "prebuild": "WEBPACK=true NODE_ENV=test ./scripts/generateIntrospectionResult.js", + "dev-start": "nodemon -w . -w bin/cli -w bin/cli-serve --config .nodemon.json --exec \"yarn generate-introspection && ./bin/cli -c .env serve -j -w\"", + "prebuild": "yarn generate-introspection", "build": "WEBPACK=true NODE_ENV=production webpack -p --config webpack.config.js --bail", - "prebuild-watch": "WEBPACK=true NODE_ENV=test ./scripts/generateIntrospectionResult.js", + "prebuild-watch": "yarn generate-introspection", "build-watch": "WEBPACK=true NODE_ENV=development webpack --progress --config webpack.config.js --watch", "lint": "eslint bin/* .", "lint-fix": "eslint bin/* . --fix", @@ -19,7 +19,8 @@ "e2e": "NODE_ENV=test nightwatch", "poste2e": "NODE_ENV=test scripts/poste2e.sh", "embed-start": "NODE_ENV=development yarn build && ./bin/cli serve --jobs", - "heroku-postbuild": "./bin/cli plugins reconcile && yarn build" + "heroku-postbuild": "./bin/cli plugins reconcile && yarn build", + "generate-introspection": "WEBPACK=true NODE_ENV=test ./scripts/generateIntrospectionResult.js" }, "talk": { "migration": { diff --git a/plugins/talk-plugin-featured-comments/client/containers/Comment.js b/plugins/talk-plugin-featured-comments/client/containers/Comment.js index 2b3de19b5..7080627e6 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/Comment.js +++ b/plugins/talk-plugin-featured-comments/client/containers/Comment.js @@ -32,19 +32,6 @@ export default withFragments({ } } - ## - # TODO: Remove this when we have the IntrospectionFragmentMatcher. - # Currently without this loading more featured comments - # brings apollo into an inconsistent state. - action_summaries { - __typename - count - current_user { - id - } - } - ## - user { id username From 0c05da92856e4accde6467caef9d63d57c7981d8 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 25 Jul 2017 17:16:59 +0700 Subject: [PATCH 16/18] Add comment --- services/mongoose.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/mongoose.js b/services/mongoose.js index c11bdbb78..2b05aaeaa 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -44,6 +44,10 @@ if (enabled('talk:db')) { if (WEBPACK) { console.warn('Not connecting to mongodb during webpack build'); + + // @wyattjoh: We didn't call connect, but because we include mongoose, it will hold the socket ready, + // preventing node from exiting. Calling disconnect here just ensures that the application + // can quit correctly. mongoose.disconnect(); } else { From f10d1a9f0d3704be4a3e8ced5595aaccbf723cc9 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 25 Jul 2017 19:11:11 +0700 Subject: [PATCH 17/18] Adapt auto scroll when loading a permalinked comment --- client/coral-embed-stream/src/containers/Embed.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/containers/Embed.js b/client/coral-embed-stream/src/containers/Embed.js index a31dbe911..06646a6c2 100644 --- a/client/coral-embed-stream/src/containers/Embed.js +++ b/client/coral-embed-stream/src/containers/Embed.js @@ -3,6 +3,7 @@ import {compose, gql} from 'react-apollo'; import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; import isEqual from 'lodash/isEqual'; +import get from 'lodash/get'; import branch from 'recompose/branch'; import renderComponent from 'recompose/renderComponent'; @@ -91,7 +92,7 @@ class EmbedContainer extends React.Component { } componentDidUpdate(prevProps) { - if (!prevProps.root.comment && this.props.root.comment) { + if (!get(prevProps, 'root.asset.comment') && get(this.props, 'root.asset.comment')) { // Scroll to a permalinked comment if one is in the URL once the page is done rendering. setTimeout(() => pym.scrollParentToChildEl('talk-embed-stream-container'), 0); From ef759e068bcf1a6e12454c0b469c7f4e650c092c Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 25 Jul 2017 19:14:03 +0700 Subject: [PATCH 18/18] Adapt query helpers --- client/coral-embed-stream/src/graphql/utils.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/client/coral-embed-stream/src/graphql/utils.js b/client/coral-embed-stream/src/graphql/utils.js index 4f51b0ae2..1030a3547 100644 --- a/client/coral-embed-stream/src/graphql/utils.js +++ b/client/coral-embed-stream/src/graphql/utils.js @@ -12,8 +12,8 @@ function determineCommentDepth(comment) { } function applyToCommentsOrigin(root, callback) { - if (root.comment) { - let comment = root.comment; + if (root.asset.comment) { + let comment = root.asset.comment; for (let depth = 0; depth <= determineCommentDepth(comment); depth++) { let changes = {$apply: (node) => node ? callback(node) : node}; for (let i = 0; i < depth; i++) { @@ -24,7 +24,10 @@ function applyToCommentsOrigin(root, callback) { return { ...root, - comment, + asset: { + ...root.asset, + comment, + }, }; } return update(root, { @@ -135,8 +138,8 @@ export function findCommentInEmbedQuery(root, callbackOrId) { if (typeof callbackOrId === 'string') { callback = (node) => node.id === callbackOrId; } - if (root.comment) { - return findComment([getTopLevelParent(root.comment)], callback); + if (root.asset.comment) { + return findComment([getTopLevelParent(root.asset.comment)], callback); } if (!root.asset.comments) { return false;