diff --git a/bin/cli b/bin/cli index 002288648..5b52d96b9 100755 --- a/bin/cli +++ b/bin/cli @@ -4,45 +4,13 @@ * Module dependencies. */ -const program = require('commander'); -const pkg = require('../package.json'); -const dotenv = require('dotenv'); const util = require('../util'); - -//============================================================================== -// Setting up the program command line arguments. -//============================================================================== - -program - .version(pkg.version) - .option('-c, --config [path]', 'Specify the configuration file to load') - .option('--pid [path]', 'Specify a path to output the current PID to') - .parse(process.argv); - -if (program.config) { - let r = dotenv.config({ - path: program.config - }); - - if (r.error) { - throw r.error; - } -} - -// If the `--pid` flag is used, put the current pid there. -if (program.pid) { - util.pid(program.pid); -} - -// Perform rewrites to the runtime environment variables based on the contents -// of the process.env.REWRITE_ENV if it exists. This is done here as it is the -// entrypoint for the entire application. -require('env-rewrite').rewrite(); +const program = require('./commander'); program .command('serve', 'serve the application') .command('assets', 'interact with assets') - .command('settings', 'work with the application settings') + .command('setup', 'setup the application') .command('jobs', 'work with the job queues') .command('users', 'work with the application auth') .parse(process.argv); diff --git a/bin/cli-assets b/bin/cli-assets index f7d6a0bbb..76ca5735b 100755 --- a/bin/cli-assets +++ b/bin/cli-assets @@ -4,8 +4,7 @@ * Module dependencies. */ -const program = require('commander'); -const pkg = require('../package.json'); +const program = require('./commander'); const parseDuration = require('parse-duration'); const Table = require('cli-table'); const AssetModel = require('../models/asset'); @@ -86,9 +85,6 @@ function refreshAssets(ageString) { // Setting up the program command line arguments. //============================================================================== -program - .version(pkg.version); - program .command('list') .description('list all the assets in the database') diff --git a/bin/cli-jobs b/bin/cli-jobs index 879927197..4d348fabb 100755 --- a/bin/cli-jobs +++ b/bin/cli-jobs @@ -4,7 +4,7 @@ * Module dependencies. */ -const program = require('commander'); +const program = require('./commander'); const scraper = require('../services/scraper'); const mailer = require('../services/mailer'); const util = require('../util'); diff --git a/bin/cli-serve b/bin/cli-serve index c8cf37a47..0427dcf34 100755 --- a/bin/cli-serve +++ b/bin/cli-serve @@ -1,9 +1,9 @@ #!/usr/bin/env node const app = require('../app'); +const program = require('./commander'); const debug = require('debug')('talk:server'); const http = require('http'); -const init = require('../init'); const scraper = require('../services/scraper'); const mailer = require('../services/mailer'); const kue = require('../services/kue'); @@ -86,27 +86,15 @@ function onListening() { * Start the app. */ function startApp() { - init().then(() => { - /** - * Listen on provided port, on all network interfaces. - */ - server.listen(port); - server.on('error', onError); - server.on('listening', onListening); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); - }); + /** + * Listen on provided port, on all network interfaces. + */ + server.listen(port); + server.on('error', onError); + server.on('listening', onListening); } -/** - * Module dependencies. - */ - -const program = require('commander'); - //============================================================================== // Setting up the program command line arguments. //============================================================================== diff --git a/bin/cli-settings b/bin/cli-settings deleted file mode 100755 index ba4305ce3..000000000 --- a/bin/cli-settings +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env node - -/** - * Module dependencies. - */ - -const program = require('commander'); -const mongoose = require('../services/mongoose'); -const SettingsService = require('../services/settings'); -const util = require('../util'); - -// Register the shutdown criteria. -util.onshutdown([ - () => mongoose.disconnect() -]); - -//============================================================================== -// Setting up the program command line arguments. -//============================================================================== - -program - .command('init') - .description('initilizes the talk settings') - .action(() => { - const defaults = { - moderation: 'POST', - wordlist: { - banned: [], - suspect: [] - } - }; - - SettingsService - .init(defaults) - .then(() => { - console.log('Created settings object.'); - util.shutdown(); - }) - .catch((err) => { - console.error(`failed to create the settings object ${JSON.stringify(err)}`); - util.shutdown(1); - }); - }); - -program.parse(process.argv); - -// If there is no command listed, output help. -if (!process.argv.slice(2).length) { - program.outputHelp(); - util.shutdown(); -} diff --git a/bin/cli-setup b/bin/cli-setup new file mode 100755 index 000000000..eaa2b90b1 --- /dev/null +++ b/bin/cli-setup @@ -0,0 +1,88 @@ +#!/usr/bin/env node + +/** + * Module dependencies. + */ + +const program = require('./commander'); +const inquirer = require('inquirer'); +const mongoose = require('../services/mongoose'); +const SettingModel = require('../models/setting'); +const SettingsService = require('../services/settings'); +const util = require('../util'); + +// Register the shutdown criteria. +util.onshutdown([ + () => mongoose.disconnect() +]); + +//============================================================================== +// Setting up the program command line arguments. +//============================================================================== + +program + .description('runs the setup wizard to setup the application') + .option('--defaults', 'apply defaults for config instead of prompting') + .parse(process.argv); + +//============================================================================== +// Setup the application +//============================================================================== + +SettingsService + .init() + .then((settings) => { + if (program.defaults) { + return settings.save(); + } + + console.log('We\'ll ask you some questions in order to setup your installation of Talk.\n'); + + return inquirer.prompt([ + { + type: 'input', + name: 'organizationName', + message: 'Organization Name', + default: settings.organizationName, + validate: (input) => { + if (input && input.length > 0) { + return true; + } + + return 'Organization Name is required.'; + } + }, + { + type: 'list', + choices: SettingModel.MODERATION_OPTIONS, + name: 'moderation', + default: settings.moderation, + message: 'Select a moderation mode' + }, + { + type: 'confirm', + name: 'requireEmailConfirmation', + default: settings.requireEmailConfirmation, + message: 'Should emails always be confirmed' + } + ]) + .then((answers) => { + + // Update the settings that were changed. + Object.keys(answers).forEach((key) => { + if (answers[key] !== undefined) { + settings[key] = answers[key]; + } + }); + + return settings.save(); + }); + }) + .then(() => { + console.log('Talk is now installed!'); + util.shutdown(); + }) + .catch((err) => { + console.error(err); + util.shutdown(1); + }); diff --git a/bin/cli-users b/bin/cli-users index a7e9cdbda..072a143ee 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -4,105 +4,132 @@ * Module dependencies. */ -const program = require('commander'); -const pkg = require('../package.json'); -const prompt = require('prompt'); +const program = require('./commander'); +const inquirer = require('inquirer'); const UsersService = require('../services/users'); const UserModel = require('../models/user'); const mongoose = require('../services/mongoose'); const util = require('../util'); const Table = require('cli-table'); +const validateRequired = (msg = 'Field is required', len = 1) => (input) => { + if (input && input.length >= len) { + return true; + } + + return msg; +}; + // Regeister the shutdown criteria. util.onshutdown([ () => mongoose.disconnect() ]); +function getUserCreateAnswers(options) { + if (options.flag_mode) { + + let user = { + email: options.email, + password: options.password, + confirmPassword: options.password, + displayName: options.name, + roles: [] + }; + + if (options.role && UserModel.USER_ROLES.indexOf(options.role) > -1) { + user.roles = [options.role]; + } + + return Promise.resolve(user); + } + + return inquirer.prompt([ + { + name: 'email', + message: 'Email', + format: 'email', + validate: validateRequired('Email is required') + }, + { + name: 'password', + message: 'Password', + type: 'password', + filter: (password) => { + return UsersService + .isValidPassword(password) + .catch((err) => { + throw err.message; + }); + } + }, + { + name: 'confirmPassword', + message: 'Confirm Password', + type: 'password', + filter: (confirmPassword) => { + return UsersService + .isValidPassword(confirmPassword) + .catch((err) => { + throw err.message; + }); + } + }, + { + name: 'displayName', + message: 'Display Name', + filter: (displayName) => { + return UsersService + .isValidDisplayName(displayName) + .catch((err) => { + throw err.message; + }); + } + }, + { + name: 'roles', + message: 'User Role', + type: 'checkbox', + choices: UserModel.USER_ROLES + } + ]); +} + /** * Prompts for input and registers a user based on those. */ function createUser(options) { - return new Promise((resolve, reject) => { - - if (options.flag_mode) { - return resolve({ - email: options.email, - password: options.password, - displayName: options.name, - role: options.role - }); - } - - prompt.start(); - - prompt.get([ - { - name: 'email', - description: 'Email', - format: 'email', - required: true - }, - { - name: 'password', - description: 'Password', - hidden: true, - required: true - }, - { - name: 'confirmPassword', - description: 'Confirm Password', - hidden: true, - required: true - }, - { - name: 'displayName', - description: 'Display Name', - required: true - }, - { - name: 'role', - description: 'User Role', - required: false - } - ], (err, result) => { - if (err) { - return reject(err); + getUserCreateAnswers(options) + .then((answers) => { + if (answers.password !== answers.confirmPassword) { + return Promise.reject(new Error('Passwords do not match')); } - if (result.password !== result.confirmPassword) { - return reject(new Error('Passwords do not match')); - } + return answers; + }) + .then((answers) => { + return UsersService + .createLocalUser(answers.email.trim(), answers.password.trim(), answers.displayName.trim()) + .then((user) => { + console.log(`Created user ${user.id}.`); - resolve(result); - }); - }) - .then((result) => { - return UsersService.createLocalUser(result.email.trim(), result.password.trim(), result.displayName.trim()) - .then((user) => { - console.log(`Created user ${user.id}.`); - - let role = result.role ? result.role.trim() : null; - - if (role && role.length > 0) { - return UsersService - .addRoleToUser(user.id, role) - .then(() => { - console.log(`Added the admin ${result.role.trim()} to User ${user.id}.`); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(); - }); - } else { - util.shutdown(); - } + if (answers.roles.length > 0) { + return Promise.all(answers.roles.map((role) => { + return UsersService + .addRoleToUser(user.id, role) + .then(() => { + console.log(`Added the role ${role} to User ${user.id}.`); + }); + })); + } + }); + }) + .then(() => { + util.shutdown(); }) .catch((err) => { console.error(err); util.shutdown(); }); - }); } /** @@ -127,44 +154,34 @@ function deleteUser(userID) { * Changes the password for a user. */ function passwd(userID) { - prompt.start(); - - prompt.get([ + inquirer.prompt([ { name: 'password', - description: 'Password', - hidden: true, - required: true + message: 'Password', + type: 'password', + validate: validateRequired('Password is required') }, { name: 'confirmPassword', - description: 'Confirm Password', - hidden: true, - required: true + message: 'Confirm Password', + type: 'password', + validate: validateRequired('Confirm Password is required') } - ], (err, result) => { - if (err) { - console.error(err); - util.shutdown(); - return; + ]) + .then((answers) => { + if (answers.password !== answers.confirmPassword) { + return Promise.reject(new Error('Password mismatch')); } - if (result.password !== result.confirmPassword) { - console.error(new Error('Password mismatch')); - util.shutdown(1); - return; - } - - UsersService - .changePassword(userID, result.password) - .then(() => { - console.log('Password changed.'); - util.shutdown(); - }) - .catch((err) => { - console.error(err); - util.shutdown(1); - }); + return UsersService.changePassword(userID, answers.password); + }) + .then(() => { + console.log('Password changed.'); + util.shutdown(); + }) + .catch((err) => { + console.error(err); + util.shutdown(1); }); } @@ -273,6 +290,13 @@ function mergeUsers(dstUserID, srcUserID) { * @param {String} role the role to add */ function addRole(userID, role) { + + if (UserModel.USER_ROLES.indexOf(role) === -1) { + console.error(`Role '${role}' is not supported. Supported roles are ${UserModel.USER_ROLES.join(', ')}.`); + util.shutdown(1); + return; + } + UsersService .addRoleToUser(userID, role) .then(() => { @@ -291,6 +315,13 @@ function addRole(userID, role) { * @param {String} role the role to remove */ function removeRole(userID, role) { + + if (UserModel.USER_ROLES.indexOf(role) === -1) { + console.error(`Role '${role}' is not supported. Supported roles are ${UserModel.USER_ROLES.join(', ')}.`); + util.shutdown(1); + return; + } + UsersService .removeRoleFromUser(userID, role) .then(() => { @@ -375,9 +406,6 @@ function enableUser(userID) { // Setting up the program command line arguments. //============================================================================== -program - .version(pkg.version); - program .command('create') .option('--email [email]', 'Email to use') diff --git a/bin/commander.js b/bin/commander.js new file mode 100644 index 000000000..2a3d1e363 --- /dev/null +++ b/bin/commander.js @@ -0,0 +1,46 @@ +const pkg = require('../package.json'); +const dotenv = require('dotenv'); +const fs = require('fs'); +const program = require('commander'); +const util = require('../util'); + +// Perform rewrites to the runtime environment variables based on the contents +// of the process.env.REWRITE_ENV if it exists. This is done here as it is the +// entrypoint for the entire application. +require('env-rewrite').rewrite(); + +//============================================================================== +// Setting up the program command line arguments. +//============================================================================== + +const parseArgs = require('minimist')(process.argv.slice(2), { + alias: { + 'c': 'config' + }, + string: [ + 'config', + 'pid' + ], + default: { + 'config': null, + 'pid': null + } +}); + +if (parseArgs.config) { + let envConfig = dotenv.parse(fs.readFileSync(parseArgs.config, {encoding: 'utf8'})); + + Object.keys(envConfig).forEach((k) => { + process.env[k] = envConfig[k]; + }); +} + +// If the `--pid` flag is used, put the current pid there. +if (parseArgs.pid) { + util.pid(parseArgs.pid); +} + +module.exports = program + .version(pkg.version) + .option('-c, --config [path]', 'Specify the configuration file to load') + .option('--pid [path]', 'Specify a path to output the current PID to'); diff --git a/circle.yml b/circle.yml index c511715a5..99b7d852b 100644 --- a/circle.yml +++ b/circle.yml @@ -14,7 +14,7 @@ database: post: # Initialize the settings in the database, this will create indicies for the # database. - - ./bin/cli settings init + - ./bin/cli setup --defaults - sleep 2 test: diff --git a/client/coral-embed-stream/src/Embed.js b/client/coral-embed-stream/src/Embed.js index fd505e299..2a7057cca 100644 --- a/client/coral-embed-stream/src/Embed.js +++ b/client/coral-embed-stream/src/Embed.js @@ -105,7 +105,7 @@ class Embed extends Component { loading ? :
- + Settings Configure Stream diff --git a/client/coral-framework/graphql/queries/streamQuery.graphql b/client/coral-framework/graphql/queries/streamQuery.graphql index 77f8ed356..583a2db13 100644 --- a/client/coral-framework/graphql/queries/streamQuery.graphql +++ b/client/coral-framework/graphql/queries/streamQuery.graphql @@ -17,6 +17,7 @@ query AssetQuery($asset_url: String!) { charCount requireEmailConfirmation } + commentCount comments { ...commentView replies { diff --git a/client/coral-plugin-stream/Stream.js b/client/coral-plugin-stream/Stream.js index 2ec84af24..633a98502 100644 --- a/client/coral-plugin-stream/Stream.js +++ b/client/coral-plugin-stream/Stream.js @@ -73,6 +73,7 @@ query AssetQuery($asset_id: ID!) { id title url + commentCount comments { ...commentView replies { diff --git a/errors.js b/errors.js index 1b26e7170..bddcf5989 100644 --- a/errors.js +++ b/errors.js @@ -110,10 +110,15 @@ const ErrNotAuthorized = new APIError('not authorized', { status: 401 }); +// ErrSettingsNotInit is returned when the settings are required but not +// initialized. +const ErrSettingsNotInit = new Error('settings not initialized, run `./bin/cli setup` to setup the application first'); + module.exports = { ExtendableError, APIError, ErrPasswordTooShort, + ErrSettingsNotInit, ErrMissingEmail, ErrMissingPassword, ErrMissingToken, diff --git a/graph/loaders/actions.js b/graph/loaders/actions.js index cc7d43a12..c41f17219 100644 --- a/graph/loaders/actions.js +++ b/graph/loaders/actions.js @@ -3,6 +3,7 @@ const DataLoader = require('dataloader'); const util = require('./util'); const ActionsService = require('../../services/actions'); +const ActionModel = require('../../models/action'); /** * Looks up actions based on the requested id's all bounded by the user. @@ -16,6 +17,17 @@ const genActionSummariessByItemID = ({user = {}}, item_ids) => { .then(util.arrayJoinBy(item_ids, 'item_id')); }; +/** + * Search for actions based on their action_type and item_type and ensures that + * the actions returned have unique item id's. + * @param {String} action_type the action to search by + * @param {String} item_type the item id to search by + * @return {Promise} resolves to distinct items actions + */ +const getItemIdsByActionTypeAndItemType = (_, action_type, item_type) => { + return ActionModel.distinct('item_id', {action_type, item_type}); +}; + /** * Creates a set of loaders based on a GraphQL context. * @param {Object} context the context of the GraphQL request @@ -23,6 +35,7 @@ const genActionSummariessByItemID = ({user = {}}, item_ids) => { */ module.exports = (context) => ({ Actions: { - getByItemID: new DataLoader((ids) => genActionSummariessByItemID(context, ids)), + getSummariesByItemID: new DataLoader((ids) => genActionSummariessByItemID(context, ids)), + getByTypes: ({action_type, item_type}) => getItemIdsByActionTypeAndItemType(context, action_type, item_type) } }); diff --git a/graph/loaders/comments.js b/graph/loaders/comments.js index 9a67bb621..7298cbbf4 100644 --- a/graph/loaders/comments.js +++ b/graph/loaders/comments.js @@ -1,85 +1,134 @@ -const DataLoader = require('dataloader'); - const util = require('./util'); -const ActionModel = require('../../models/action'); const CommentModel = require('../../models/comment'); -const CommentsService = require('../../services/comments'); /** - * Retrieves comments by an array of asset id's, results are returned in reverse - * chronological order. - * @param {Array} ids array of ids to lookup + * Returns the comment count for all comments that are public based on their + * asset ids. + * @param {Object} context graph context + * @param {Array} asset_ids the ids of assets for which there are + * comments that we want to get */ -const genCommentsByAssetID = (context, ids) => { - return CommentModel.find({ - asset_id: { - $in: ids +const getCountsByAssetID = (context, asset_ids) => { + return CommentModel.aggregate([ + { + $match: { + asset_id: { + $in: asset_ids + }, + status: { + $in: [null, 'ACCEPTED'] + }, + parent_id: null + } }, - parent_id: null, - status: { - $in: [null, 'ACCEPTED'] + { + $group: { + _id: '$asset_id', + count: { + $sum: 1 + } + } } - }) - .sort({created_at: -1}) - .then(util.arrayJoinBy(ids, 'asset_id')); + ]) + .then(util.singleJoinBy(asset_ids, '_id')) + .then((results) => results.map((result) => result ? result.count : 0)); }; /** - * Retrieves comments by an array of parent ids, results are returned in - * chronological order. - * @param {Array} ids array of ids to lookup + * Returns the comment count for all comments that are public based on their + * parent ids. + * @param {Object} context graph context + * @param {Array} parent_ids the ids of parents for which there are + * comments that we want to get */ -const genCommentsByParentID = (context, ids) => { - return CommentModel.find({ - parent_id: { - $in: ids +const getCountsByParentID = (context, parent_ids) => { + return CommentModel.aggregate([ + { + $match: { + parent_id: { + $in: parent_ids + }, + status: { + $in: [null, 'ACCEPTED'] + } + } }, - status: { - $in: [null, 'ACCEPTED'] + { + $group: { + _id: '$parent_id', + count: { + $sum: 1 + } + } } - }) - .sort({created_at: 1}) - .then(util.arrayJoinBy(ids, 'parent_id')); + ]) + .then(util.singleJoinBy(parent_ids, '_id')) + .then((results) => results.map((result) => result ? result.count : 0)); }; -const getCommentsByStatusAndAssetID = (context, {status = null, asset_id = null}) => { +/** + * Retrieves comments based on the passed in query that is filtered by the + * current used passed in via the context. + * @param {Object} context graph context + * @param {Object} query query terms to apply to the comments query + */ +const getCommentsByQuery = ({user}, {ids, statuses, asset_id, parent_id, limit, cursor, sort}) => { + let comments = CommentModel.find(); - // TODO: remove when we move the enum over to the uppercase. - if (status) { - status = status.toLowerCase(); + // Only administrators can search for comments with statuses that are not + // `null`, or `'ACCEPTED'`. + if (user != null && user.hasRoles('ADMIN') && statuses) { + comments = comments.where({ + status: { + $in: statuses + } + }); + } else { + comments = comments.where({ + status: { + $in: [null, 'ACCEPTED'] + } + }); } - return CommentsService.moderationQueue(status, asset_id); -}; - -const getCommentsByActionTypeAndAssetID = (context, {action_type, asset_id = null}) => { - return ActionModel.find({ - action_type, - item_type: 'COMMENTS' - }).then((actions) => { - let comments = CommentModel.find({ + if (ids) { + comments = comments.find({ id: { - $in: actions.map((action) => action.item_id) + $in: ids } - }).sort({created_at: 1}); + }); + } - if (asset_id) { - comments = comments.where({asset_id}); + if (asset_id) { + comments = comments.where({asset_id}); + } + + // We perform the undefined check because, null, is a valid state for the + // search to be with, which indicates that it is at depth 0. + if (parent_id !== undefined) { + comments = comments.where({parent_id}); + } + + if (cursor) { + if (sort === 'REVERSE_CHRONOLOGICAL') { + comments = comments.where({ + created_at: { + $lt: cursor + } + }); + } else { + comments = comments.where({ + created_at: { + $gt: cursor + } + }); } + } - return comments; - }); -}; - -const genCommentsByAuthorID = (context, authorIDs) => { - return CommentModel.find({ - author_id: { - $in: authorIDs - } - }) - .sort({created_at: -1}) - .then(util.arrayJoinBy(authorIDs, 'author_id')); + return comments + .sort({created_at: sort === 'REVERSE_CHRONOLOGICAL' ? -1 : 1}) + .limit(limit); }; /** @@ -89,10 +138,8 @@ const genCommentsByAuthorID = (context, authorIDs) => { */ module.exports = (context) => ({ Comments: { - getByParentID: new DataLoader((ids) => genCommentsByParentID(context, ids)), - getByAssetID: new DataLoader((ids) => genCommentsByAssetID(context, ids)), - getByStatusAndAssetID: (query) => getCommentsByStatusAndAssetID(context, query), - getByActionTypeAndAssetID: (query) => getCommentsByActionTypeAndAssetID(context, query), - getByAuthorID: new DataLoader((authorIDs) => genCommentsByAuthorID(context, authorIDs)) + getByQuery: (query) => getCommentsByQuery(context, query), + countByAssetID: new util.SharedCacheDataLoader('Comments.countByAssetID', 3600, (ids) => getCountsByAssetID(context, ids)), + countByParentID: new util.SharedCacheDataLoader('Comments.countByParentID', 3600, (ids) => getCountsByParentID(context, ids)) } }); diff --git a/graph/loaders/util.js b/graph/loaders/util.js index e947f2fa6..4640d8245 100644 --- a/graph/loaders/util.js +++ b/graph/loaders/util.js @@ -1,4 +1,6 @@ const _ = require('lodash'); +const DataLoader = require('dataloader'); +const cache = require('../../services/cache'); /** * SingletonResolver is a cached loader for a single result. @@ -34,6 +36,7 @@ class SingletonResolver { */ const arrayJoinBy = (ids, key) => (items) => { const itemsByKey = _.groupBy(items, key); + return ids.map((id) => { if (id in itemsByKey) { return itemsByKey[id]; @@ -61,8 +64,76 @@ const singleJoinBy = (ids, key) => (items) => { }); }; +/** + * SharedCacheDataLoader provides a version of the DataLoader that wraps up a + * redis backed cache with the dataloader's request cache. + */ +class SharedCacheDataLoader extends DataLoader { + constructor(prefix, expiry, batchLoadFn, options) { + super(SharedCacheDataLoader.batchLoadFn(prefix, expiry, batchLoadFn), options); + + this._prefix = prefix; + this._expiry = expiry; + this._keyFunc = SharedCacheDataLoader.keyFunc(this._prefix); + } + + /** + * clear the key from the shared cache and the request cache + */ + clear(key) { + return cache + .invalidate(key, this._keyFunc) + .then(() => super.clear(key)); + } + + /** + * prime the shared cache and the request cache + */ + prime(key, value) { + return cache + .set(key, value, this._expiry, this._keyFunc) + .then(() => super.prime(key, value)); + } + + /** + * prime many values in the shared cache and the request cache + */ + primeMany(keys, values) { + return cache + .setMany(keys, values, this._expiry, this._keyFunc) + .then(() => keys.map((key, i) => super.prime(key, values[i]))); + } + + /** + * wraps up the prefix needed for the redis backed shared cache driver + */ + static keyFunc(prefix) { + return (key) => `cache.sbl[${prefix}][${key}]`; + } + + /** + * wraps the dataloader batchLoadFn with the shared cache's wrapper + */ + static batchLoadFn(prefix, expiry, batchLoadFn) { + return (ids) => cache.wrapMany(ids, expiry, (workKeys) => { + return batchLoadFn(workKeys); + }, SharedCacheDataLoader.keyFunc(prefix)); + } +} + +/** + * Maps an object's paths to a string that can be used as a cache key. + * @param {Array} paths paths on the object to be used to generate the cache + * key + */ +const objectCacheKeyFn = (...paths) => (obj) => { + return paths.map((path) => obj[path]).join(':'); +}; + module.exports = { singleJoinBy, arrayJoinBy, - SingletonResolver + objectCacheKeyFn, + SingletonResolver, + SharedCacheDataLoader }; diff --git a/graph/mutators/action.js b/graph/mutators/action.js index 3c4c32986..b5264fd49 100644 --- a/graph/mutators/action.js +++ b/graph/mutators/action.js @@ -18,10 +18,15 @@ const createAction = ({user = {}}, {item_id, item_type, action_type, metadata = user_id: user.id, action_type, metadata - }).then((result) => - item_type === 'USERS' && action_type === 'FLAG' ? - UsersService.setStatus(item_id, 'PENDING').then(() => result) - : result); + }).then((action) => { + if (item_type === 'USERS' && action_type === 'FLAG') { + return UsersService + .setStatus(item_id, 'PENDING') + .then(() => action); + } + + return action; + }); }; /** diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index cc62e48df..3c35b0e60 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -14,13 +14,29 @@ const Wordlist = require('../../services/wordlist'); * @param {String} [status=null] the status of the new comment * @return {Promise} resolves to the created comment */ -const createComment = ({user}, {body, asset_id, parent_id = null}, status = null) => { +const createComment = ({user, loaders: {Comments}}, {body, asset_id, parent_id = null}, status = null) => { return CommentsService.publicCreate({ body, asset_id, parent_id, status, author_id: user.id + }) + .then((comment) => { + + // TODO: explore using an `INCR` operation to update the counts here + + // If the loaders are present, clear the caches for these values because we + // just added a new comment, hence the counts should be updated. + if (Comments && Comments.countByAssetID && Comments.countByParentID) { + if (parent_id != null) { + Comments.countByParentID.clear(parent_id); + } else { + Comments.countByAssetID.clear(asset_id); + } + } + + return comment; }); }; @@ -121,7 +137,7 @@ const createPublicComment = (context, commentInput) => { if (wordlist != null) { // TODO: this is kind of fragile, we should refactor this to resolve - // all these const's that we're using like 'comments', 'flag' to be + // all these const's that we're using like 'COMMENTS', 'FLAG' to be // defined in a checkable schema. return context.mutators.Action.createAction(null, { item_id: comment.id, @@ -131,7 +147,8 @@ const createPublicComment = (context, commentInput) => { field: 'body', details: 'Matched suspect word filters.' } - }).then(() => comment); + }) + .then(() => comment); } // Finally, we return the comment. diff --git a/graph/resolvers/action.js b/graph/resolvers/action.js index e6886653e..f8aa60245 100644 --- a/graph/resolvers/action.js +++ b/graph/resolvers/action.js @@ -1,10 +1,10 @@ const Action = { // This will load the user for the specific action. We'll limit this to the - // admin users only. - user({user_id}, _, {loaders, user}) { - if (user.hasRole('ADMIN')) { - return loaders.Users.getByID.load(user_id); + // admin users only or the current logged in user. + user({user_id}, _, {loaders: {Users}, user}) { + if (user && (user.hasRole('ADMIN') || user_id === user.id)) { + return Users.getByID.load(user_id); } } }; diff --git a/graph/resolvers/asset.js b/graph/resolvers/asset.js index dcf6e1a75..60b44fa62 100644 --- a/graph/resolvers/asset.js +++ b/graph/resolvers/asset.js @@ -1,9 +1,17 @@ const Asset = { - comments({id}, _, {loaders}) { - return loaders.Comments.getByAssetID.load(id); + comments({id}, {sort, limit}, {loaders: {Comments}}) { + return Comments.getByQuery({ + asset_id: id, + sort, + limit, + parent_id: null + }); }, - settings({settings = null}, _, {loaders}) { - return loaders.Settings.load() + commentCount({id}, _, {loaders: {Comments}}) { + return Comments.countByAssetID.load(id); + }, + settings({settings = null}, _, {loaders: {Settings}}) { + return Settings.load() .then((globalSettings) => { if (settings) { settings = Object.assign({}, globalSettings.toObject(), settings); diff --git a/graph/resolvers/comment.js b/graph/resolvers/comment.js index ce2283cda..b3d79225e 100644 --- a/graph/resolvers/comment.js +++ b/graph/resolvers/comment.js @@ -1,15 +1,23 @@ const Comment = { - user({author_id}, _, {loaders}) { - return loaders.Users.getByID.load(author_id); + user({author_id}, _, {loaders: {Users}}) { + return Users.getByID.load(author_id); }, - replies({id}, _, {loaders}) { - return loaders.Comments.getByParentID.load(id); + replies({id, asset_id}, {sort, limit}, {loaders: {Comments}}) { + return Comments.getByQuery({ + asset_id, + parent_id: id, + sort, + limit + }); }, - actions({id}, _, {loaders}) { - return loaders.Actions.getByItemID.load(id); + replyCount({id}, _, {loaders: {Comments}}) { + return Comments.countByParentID.load(id); }, - asset({asset_id}, _, {loaders}) { - return loaders.Assets.getByID.load(asset_id); + actions({id}, _, {loaders: {Actions}}) { + return Actions.getSummariesByItemID.load(id); + }, + asset({asset_id}, _, {loaders: {Assets}}) { + return Assets.getByID.load(asset_id); } }; diff --git a/graph/resolvers/date.js b/graph/resolvers/date.js new file mode 100644 index 000000000..2335b9713 --- /dev/null +++ b/graph/resolvers/date.js @@ -0,0 +1,27 @@ +const GraphQLScalarType = require('graphql').GraphQLScalarType; +const Kind = require('graphql/language').Kind; + +module.exports = new GraphQLScalarType({ + name: 'Date', + description: 'Date represented as an ISO8601 string', + serialize(value) { + return value.toISOString(); + }, + parseValue(value) { + return new Date(value); + }, + parseLiteral(ast) { + switch (ast.kind) { + case Kind.STRING: + + // This handles an empty string. + if (ast.value && ast.value.length === 0) { + return null; + } + + return new Date(ast.value); + default: + return null; + } + } +}); diff --git a/graph/resolvers/index.js b/graph/resolvers/index.js index 064b1e664..f25664f34 100644 --- a/graph/resolvers/index.js +++ b/graph/resolvers/index.js @@ -2,6 +2,7 @@ const Action = require('./action'); const ActionSummary = require('./action_summary'); const Asset = require('./asset'); const Comment = require('./comment'); +const Date = require('./date'); const RootMutation = require('./root_mutation'); const RootQuery = require('./root_query'); const Settings = require('./settings'); @@ -12,6 +13,7 @@ module.exports = { ActionSummary, Asset, Comment, + Date, RootMutation, RootQuery, Settings, diff --git a/graph/resolvers/root_mutation.js b/graph/resolvers/root_mutation.js index bf108671e..ddc8e4223 100644 --- a/graph/resolvers/root_mutation.js +++ b/graph/resolvers/root_mutation.js @@ -1,15 +1,15 @@ const RootMutation = { - createComment(_, {asset_id, parent_id, body}, {mutators}) { - return mutators.Comment.create({asset_id, parent_id, body}); + createComment(_, {asset_id, parent_id, body}, {mutators: {Comment}}) { + return Comment.create({asset_id, parent_id, body}); }, - createAction(_, {action}, {mutators}) { - return mutators.Action.create(action); + createAction(_, {action}, {mutators: {Action}}) { + return Action.create(action); }, - deleteAction(_, {id}, {mutators}) { - return mutators.Action.delete({id}); + deleteAction(_, {id}, {mutators: {Action}}) { + return Action.delete({id}); }, - updateUserSettings(_, {settings}, {mutators}) { - return mutators.User.updateSettings(settings); + updateUserSettings(_, {settings}, {mutators: {User}}) { + return User.updateSettings(settings); } }; diff --git a/graph/resolvers/root_query.js b/graph/resolvers/root_query.js index 5676b4f48..8aa14d5c6 100644 --- a/graph/resolvers/root_query.js +++ b/graph/resolvers/root_query.js @@ -1,39 +1,47 @@ const RootQuery = { - assets(_, args, {loaders, user}) { + assets(_, args, {loaders: {Assets}, user}) { if (user == null || !user.hasRoles('ADMIN')) { return null; } - return loaders.Assets.getAll.load(); + return Assets.getAll.load(); }, - asset(_, query, {loaders}) { + asset(_, query, {loaders: {Assets}}) { if (query.id) { // TODO: we may not always have a comment stream here, therefore, when we // load it, we may also need to create with the url. This may also have to // move the logic over to the mutators function as an upsert operation // possibly. - return loaders.Assets.getByID.load(query.id); + return Assets.getByID.load(query.id); } - return loaders.Assets.getByURL(query.url); + return Assets.getByURL(query.url); }, - settings(_, args, {loaders}) { - return loaders.Settings.load(); + settings(_, args, {loaders: {Settings}}) { + return Settings.load(); }, // This endpoint is used for loading moderation queues, so hide it in the // event that we aren't an admin. - comments(_, {query}, {loaders, user}) { - if (user == null || !user.hasRoles('ADMIN')) { - return null; + comments(_, {query: {action_type, statuses, asset_id, parent_id, limit, cursor, sort}}, {user, loaders: {Comments, Actions}}) { + let query = {statuses, asset_id, parent_id, limit, cursor, sort}; + + if (user != null && user.hasRoles('ADMIN') && action_type) { + return Actions.getByTypes({action_type, item_type: 'COMMENTS'}) + .then((actions) => { + + // Map the actions from the items referenced byt this query. The actions + // returned by this query are explicitly going to be distinct by their + // `item_id`'s. + let ids = actions.map((action) => action.item_id); + + // Perform the query using the available resolver. + return Comments.getByQuery({ids, statuses, asset_id, parent_id, limit, cursor, sort}); + }); } - if (query.action_type) { - return loaders.Comments.getByActionTypeAndAssetID(query); - } else { - return loaders.Comments.getByStatusAndAssetID(query); - } + return Comments.getByQuery(query); }, // This returns the current user, ensure that if we aren't logged in, we diff --git a/graph/resolvers/user.js b/graph/resolvers/user.js index 65882feee..8cc11bba3 100644 --- a/graph/resolvers/user.js +++ b/graph/resolvers/user.js @@ -1,16 +1,25 @@ const User = { - actions({id}, _, {loaders}) { - return loaders.Actions.getByID.load(id); + actions({id}, _, {loaders: {Actions}}) { + return Actions.getSummariesByItemID.load(id); }, - comments({id}, _, {loaders, user}) { + comments({id}, _, {loaders: {Comments}, user}) { // If the user is not an admin, only return comment list for the owner of // the comments. - if (!user.hasRoles('ADMIN') || user.id !== id) { - return null; + if (user && (user.hasRoles('ADMIN') || user.id === id)) { + return Comments.getByAuthorID.load(id); } - return loaders.Comments.getByAuthorID.load(id); + return null; + }, + roles({id, roles}, _, {user}) { + + // If the user is not an admin, only return the current user's roles. + if (user && (user.hasRoles('ADMIN') || user.id === id)) { + return roles; + } + + return null; } }; diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 167f75f4f..744a53252 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -1,21 +1,50 @@ -interface ActionableItem { - id: ID! +# Establishes the ordering of the content by their created_at time stamp. +enum SORT_ORDER { + # newest to oldest order. + REVERSE_CHRONOLOGICAL + + # oldest to newer order. + CHRONOLOGICAL } +# Date represented as an ISO8601 string. +scalar Date + type UserSettings { # bio of the user. bio: String } -input CommentsInput { +input CommentsQuery { # current status of a comment. - status: COMMENT_STATUS + statuses: [COMMENT_STATUS] # asset that a comment is on. asset_id: ID - # action type to find comments that have an action with. + # the parent of the comment that we want to retrive. + parent_id: ID + + # comments returned will only be ones which have at least one action of this + # type. action_type: ACTION_TYPE + + # limit the number of results to be returned. + limit: Int = 10 + + # skip results from the last created_at timestamp. + cursor: Date + + # sort the results by created_at. + sort: SORT_ORDER = REVERSE_CHRONOLOGICAL +} + +enum USER_ROLES { + # an administrator of the site + ADMIN + + # a moderator of the site + MODERATOR } # Any person who can author comments, create actions, and view comments on a @@ -29,11 +58,14 @@ type User { # actions against a specific user. actions: [ActionSummary] + # the current roles of the user. + roles: [USER_ROLES] + # settings for a user. settings: UserSettings # returns all comments based on a query. - comments(query: CommentsInput): [Comment] + comments(query: CommentsQuery): [Comment] } type Comment { @@ -46,7 +78,10 @@ type Comment { user: User # the replies that were made to the comment. - replies(limit: Int = 3): [Comment] + replies(sort: SORT_ORDER = CHRONOLOGICAL, limit: Int = 3): [Comment] + + # the count of replies on a comment + replyCount: Int # the actions made against a comment. actions: [ActionSummary] @@ -58,7 +93,7 @@ type Comment { status: COMMENT_STATUS # the time when the comment was created - created_at: String! + created_at: Date! } enum ITEM_TYPE { @@ -78,11 +113,10 @@ type Action { item_id: ID! item_type: ITEM_TYPE! - item: ActionableItem user: User! - updated_at: String - created_at: String + updated_at: Date + created_at: Date } type ActionSummary { @@ -109,13 +143,31 @@ type Settings { } type Asset { + + # The current ID of the asset. id: ID! + + # The scraped title of the asset. title: String + + # The URL that the asset is locaed on. url: String - comments: [Comment] + + # The top level comments that are attached to the asset. + comments(sort: SORT_ORDER = REVERSE_CHRONOLOGICAL, limit: Int = 10): [Comment] + + # The count of top level comments on the asset. + commentCount: Int + + # The settings (rectified with the global settings) that should be applied to + # this asset. settings: Settings! - closedAt: String - created_at: String + + # The date that the asset was closed at. + closedAt: Date + + # The date that the asset was created. + created_at: Date } enum COMMENT_STATUS { @@ -125,6 +177,7 @@ enum COMMENT_STATUS { } type RootQuery { + # retrieves site wide settings and defaults. settings: Settings @@ -135,7 +188,7 @@ type RootQuery { asset(id: ID, url: String): Asset # retrieves comments based on the input query. - comments(query: CommentsInput): [Comment] + comments(query: CommentsQuery!): [Comment] # retrieves the current logged in user. me: User diff --git a/init.js b/init.js deleted file mode 100644 index cd0c0bbc9..000000000 --- a/init.js +++ /dev/null @@ -1,13 +0,0 @@ -const SettingsService = require('./services/settings'); - -module.exports = () => Promise.all([ - - // Upsert the settings object. - SettingsService.init({ - moderation: 'POST', - wordlist: { - banned: [], - suspect: [] - } - }) -]); diff --git a/models/setting.js b/models/setting.js index 73f1a8da8..25882b3f8 100644 --- a/models/setting.js +++ b/models/setting.js @@ -1,12 +1,10 @@ const mongoose = require('../services/mongoose'); const Schema = mongoose.Schema; -const WordlistSchema = new Schema({ - banned: [String], - suspect: [String] -}, { - _id: false -}); +const MODERATION_OPTIONS = [ + 'PRE', + 'POST' +]; /** * SettingSchema manages application settings that get used on front and backend. @@ -19,10 +17,7 @@ const SettingSchema = new Schema({ }, moderation: { type: String, - enum: [ - 'PRE', - 'POST' - ], + enum: MODERATION_OPTIONS, default: 'POST' }, infoBoxEnable: { @@ -33,6 +28,9 @@ const SettingSchema = new Schema({ type: String, default: '' }, + organizationName: { + type: String + }, closedTimeout: { type: Number, @@ -43,7 +41,16 @@ const SettingSchema = new Schema({ type: String, default: 'Expired' }, - wordlist: WordlistSchema, + wordlist: { + banned: { + type: Array, + default: [] + }, + suspect: { + type: Array, + default: [] + } + }, charCount: { type: Number, default: 5000 @@ -87,3 +94,4 @@ SettingSchema.method('merge', function(src) { const Setting = mongoose.model('Setting', SettingSchema); module.exports = Setting; +module.exports.MODERATION_OPTIONS = MODERATION_OPTIONS; diff --git a/package.json b/package.json index 641d85757..07e67c5b8 100644 --- a/package.json +++ b/package.json @@ -68,10 +68,12 @@ "graphql-tag": "^1.2.3", "graphql-tools": "^0.9.0", "helmet": "^3.1.0", + "inquirer": "^3.0.1", "jsonwebtoken": "^7.1.9", "kue": "^0.11.5", "lodash": "^4.16.6", "metascraper": "^1.0.6", + "minimist": "^1.2.0", "mongoose": "^4.6.5", "morgan": "^1.7.0", "natural": "^0.4.0", @@ -80,7 +82,6 @@ "passport": "^0.3.2", "passport-facebook": "^2.1.1", "passport-local": "^1.0.0", - "prompt": "^1.0.0", "react-apollo": "^0.8.1", "redis": "^2.6.3", "uuid": "^2.0.3" diff --git a/scripts/pree2e.sh b/scripts/pree2e.sh index 6210c09f0..9ed5b7b89 100755 --- a/scripts/pree2e.sh +++ b/scripts/pree2e.sh @@ -3,8 +3,8 @@ # install selenium selenium-standalone install --config=./selenium.config.js -#Init settings -./bin/cli settings init +# Init application +./bin/cli setup --defaults # Creating Admin Test User ./bin/cli users create --flag_mode --email "admin@test.com" --password "testtest" --name "AdminTestUser" --role "ADMIN" diff --git a/services/cache.js b/services/cache.js index 1d090e46d..21ae9911e 100644 --- a/services/cache.js +++ b/services/cache.js @@ -1,4 +1,5 @@ const redis = require('./redis'); +const debug = require('debug')('talk:cache'); const cache = module.exports = { client: redis.createClient() @@ -29,31 +30,103 @@ const keyfunc = (key) => { * resolved as the value to cache. * @return {Promise} Resolves to the value either retrieved from cache */ -cache.wrap = (key, expiry, work) => { +cache.wrap = (key, expiry, work, kf = keyfunc) => { return cache - .get(key) + .get(key, kf) .then((value) => { if (value !== null) { + debug('wrap: hit', kf(key)); return value; } + debug('wrap: miss', kf(key)); + return work() .then((value) => { - return cache - .set(key, value, expiry) - .then(() => value); + + process.nextTick(() => { + cache + .set(key, value, expiry, kf) + .then(() => { + debug('wrap: set complete'); + }) + .catch((err) => { + console.error(err); + }); + }); + + return value; }); }); }; +/** + * [wrapMany description] + * @param {Array} keys Either an array of objects represening + * 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 + * resolved as the value to cache. + * @param {Function} [kf=keyfunc] optional key function to use to turn the + * provided key into a string for the cache. + * @return {Promise} resovles to the values for the keys + */ +cache.wrapMany = (keys, expiry, work, kf = keyfunc) => { + return cache + .getMany(keys, kf) + .then((values) => { + + // find any of the null valued items by collecting the work + let workRefs = values + .map((value, index) => ({value, index, key: keys[index]})) + .filter(({value}) => value === null); + + let workKeys = workRefs.map(({key}) => key); + + debug(`wrapMany: hit ratio: ${keys.length - workKeys.length}/${keys.length}`); + + if (workKeys.length > 0) { + return work(workKeys) + .then((workedValues) => { + + // Set the items in the cache that we needed to retrive after the + // next process tick. + process.nextTick(() => { + cache + .setMany(workKeys, workedValues, expiry, kf) + .then(() => { + debug('wrapMany: setMany complete'); + }) + .catch((err) => { + console.error(err); + }); + }); + + return workedValues; + }) + .then((workedValues) => { + + // Walk over the worked keys to merge them with the existing values. + for (let i = 0; i < workRefs.length; i++) { + values[workRefs[i].index] = workedValues[i]; + } + + return values; + }); + } else { + return values; + } + }); +}; + /** * This returns a promise that returns a promise that resolves with the value * from the cache or null if it does not exist in the cache. * @param {Mixed} key Either an array of items composing a key or a string * @return {Promise} */ -cache.get = (key) => new Promise((resolve, reject) => { - cache.client.get(keyfunc(key), (err, reply) => { +cache.get = (key, kf = keyfunc) => new Promise((resolve, reject) => { + cache.client.get(kf(key), (err, reply) => { if (err) { return reject(err); } @@ -76,13 +149,80 @@ cache.get = (key) => new Promise((resolve, reject) => { }); }); +/** + * Returns many replies. + * @param {Array} keys Either an array of objects represening + * this work + * @param {Function} [kf=keyfunc] optional key function to use to turn the + * provided key into a string for the cache. + */ +cache.getMany = (keys, kf = keyfunc) => new Promise((resolve, reject) => { + cache.client.mget(keys.map(kf), (err, replies) => { + if (err) { + return reject(err); + } + + // Parse the replies. + for (let i = 0; i < replies.length; i++) { + let value = null; + + if (replies[i] != null) { + try { + + // Parse the stored cache value from JSON. + value = JSON.parse(replies[i]); + } catch (e) { + return reject(e); + } + } + + replies[i] = value; + } + + return resolve(replies); + }); +}); + +/** + * Sets many entries in the cache. + * @param {Array} keys array of keys + * @param {Array} values array of values to set + * @param {Function} [kf=keyfunc] optional key function to use to turn the + * provided key into a string for the cache. + */ +cache.setMany = (keys, values, expiry, kf = keyfunc) => { + let multi = cache.client.multi(); + + keys.forEach((key, index) => { + + // Serialize the value as JSON. + let reply = JSON.stringify(values[index]); + + // Queue up the set command. + multi.set(kf(key), reply, 'EX', expiry); + }); + + return new Promise((resolve, reject) => { + multi.exec((err) => { + if (err) { + return reject(err); + } + + resolve(); + }); + }); +}; + /** * This invalidates a cached entry in the cache. * @param {Mixed} key Either an array of items composing a key or a string * @return {Promise} */ -cache.invalidate = (key) => new Promise((resolve, reject) => { - cache.client.del(keyfunc(key), (err) => { +cache.invalidate = (key, kf = keyfunc) => new Promise((resolve, reject) => { + + debug(`invalidate: ${kf(key)}`); + + cache.client.del(kf(key), (err) => { if (err) { return reject(err); } @@ -94,17 +234,17 @@ cache.invalidate = (key) => new Promise((resolve, reject) => { /** * This sets a value on the key with the expiry and then resolves once it is * done. - * @param {Mixed} key Either an array of items composing a key or a string - * @param {Mixed} value Object to be serialized and set to the cache + * @param {Mixed} key Either an array of items composing a key or a string + * @param {Mixed} value Object to be serialized and set to the cache * @param {Integer} expiry Time in seconds for the cache entry to live for * @return {Promise} */ -cache.set = (key, value, expiry) => new Promise((resolve, reject) => { +cache.set = (key, value, expiry, kf = keyfunc) => new Promise((resolve, reject) => { // Serialize the value as JSON. let reply = JSON.stringify(value); - cache.client.set(keyfunc(key), reply, 'EX', expiry, (err) => { + cache.client.set(kf(key), reply, 'EX', expiry, (err) => { if (err) { return reject(err); } diff --git a/services/settings.js b/services/settings.js index d82fb91a5..2bb15319d 100644 --- a/services/settings.js +++ b/services/settings.js @@ -1,4 +1,5 @@ const SettingModel = require('../models/setting'); +const errors = require('../errors'); /** * The selector used to uniquely identify the settings document. @@ -15,7 +16,15 @@ module.exports = class SettingsService { * @return {Promise} settings the whole settings record */ static retrieve() { - return SettingModel.findOne(selector); + return SettingModel + .findOne(selector) + .then((settings) => { + if (!settings) { + return Promise.reject(errors.ErrSettingsNotInit); + } + + return settings; + }); } /** @@ -35,15 +44,14 @@ module.exports = class SettingsService { /** * This is run once when the app starts to ensure settings are populated. - * @return {Promise} null initialize the global settings object */ - static init(defaults) { + static init(defaults = {}) { + return SettingsService + .retrieve() + .catch(() => { + let settings = new SettingModel(defaults); - // Inject the defaults on top of the passed in defaults to ensure that the new - // settings conform to the required selector. - defaults = Object.assign({}, defaults, selector); - - // Actually update the settings collection. - return SettingsService.update(defaults); + return settings.save(); + }); } }; diff --git a/services/users.js b/services/users.js index 7a888ede1..635e39e47 100644 --- a/services/users.js +++ b/services/users.js @@ -189,6 +189,18 @@ module.exports = class UsersService { return Wordlist.displayNameCheck(displayName); } + static isValidPassword(password) { + if (!password) { + return Promise.reject(errors.ErrMissingPassword); + } + + if (password.length < 8) { + return Promise.reject(errors.ErrPasswordTooShort); + } + + return Promise.resolve(password); + } + /** * Creates the local user with a given email, password, and name. * @param {String} email email of the new user @@ -205,15 +217,10 @@ module.exports = class UsersService { email = email.toLowerCase().trim(); displayName = displayName.toLowerCase().trim(); - if (!password) { - return Promise.reject(errors.ErrMissingPassword); - } - - if (password.length < 8) { - return Promise.reject(errors.ErrPasswordTooShort); - } - - return UsersService.isValidDisplayName(displayName) + return Promise.all([ + UsersService.isValidDisplayName(displayName), + UsersService.isValidPassword(password) + ]) .then(() => { // displayName is valid return new Promise((resolve, reject) => { bcrypt.hash(password, SALT_ROUNDS, (err, hashedPassword) => { diff --git a/services/wordlist.js b/services/wordlist.js index a06801e87..dde032f97 100644 --- a/services/wordlist.js +++ b/services/wordlist.js @@ -203,6 +203,7 @@ class Wordlist { */ static displayNameCheck(displayName) { const wl = new Wordlist(); + return wl.load() .then(() => { displayName = displayName.replace(/_/g, ''); diff --git a/test/routes/api/auth/index.js b/test/routes/api/auth/index.js index 4882d859e..1bf8f4992 100644 --- a/test/routes/api/auth/index.js +++ b/test/routes/api/auth/index.js @@ -66,7 +66,7 @@ describe('/api/v1/auth/local', () => { describe('email confirmation enabled', () => { - beforeEach(() => SettingsService.init({requireEmailConfirmation: true})); + beforeEach(() => SettingsService.update({requireEmailConfirmation: true})); describe('#post', () => { it('should not allow a login from a user that is not confirmed', () => { @@ -74,7 +74,7 @@ describe('/api/v1/auth/local', () => { .post('/api/v1/auth/local') .send({email: 'maria@gmail.com', password: 'password!'}) .catch((err) => { - err.response.should.have.status(401); + expect(err).to.have.status(401); return UsersService.createEmailConfirmToken(mockUser.id, mockUser.profiles[0].id); }) diff --git a/util.js b/util.js index 585cfdcb7..ab9035fe5 100644 --- a/util.js +++ b/util.js @@ -20,6 +20,8 @@ util.shutdown = (defaultCode = 0, signal = null) => { debug(`Reached ${signal} signal`); } + debug(`${util.toshutdown.length} jobs now being called`); + Promise .all(util.toshutdown.map((func) => func ? func(signal) : null).filter((func) => func)) .then(() => { @@ -41,7 +43,7 @@ util.shutdown = (defaultCode = 0, signal = null) => { */ util.onshutdown = (jobs) => { - debug(`${jobs.length} jobs registered`); + debug(`${jobs.length} jobs registered to be called during shutdown`); // Add the new jobs to shutdown to the object reference. util.toshutdown = util.toshutdown.concat(jobs);