From 2b1108cd883affeab55491cad2486999793afb69 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 7 May 2018 14:27:00 -0600 Subject: [PATCH] Settings Cache Refactor --- bin/cli-settings | 2 +- graph/loaders/assets.js | 2 +- graph/loaders/settings.js | 79 +++++++++++---- graph/mutators/user.js | 6 +- graph/resolvers/asset.js | 9 +- graph/resolvers/comment.js | 10 +- graph/resolvers/root_query.js | 10 +- graph/resolvers/util.js | 7 +- middleware/staticTemplate.js | 2 +- models/schema/asset.js | 3 +- models/schema/setting.js | 17 ---- package.json | 1 + .../talk-plugin-local-auth/server/mutators.js | 6 +- .../talk-plugin-notifications/server/util.js | 2 +- .../server/connect.js | 6 +- .../server/mutators.js | 6 +- services/assets.js | 47 +++------ services/cache.js | 66 ------------- services/comments.js | 2 +- services/domain_list.js | 4 +- services/moderation/index.js | 15 +-- services/passport.js | 6 +- services/settings.js | 99 +++++++++---------- services/setup.js | 2 +- services/tags.js | 50 ++++------ services/wordlist.js | 6 +- yarn.lock | 4 + 27 files changed, 209 insertions(+), 260 deletions(-) diff --git a/bin/cli-settings b/bin/cli-settings index 72f42672d..05c7f2888 100755 --- a/bin/cli-settings +++ b/bin/cli-settings @@ -18,7 +18,7 @@ async function changeOrgName() { await cache.init(); // Get the original settings. - const settings = await Settings.retrieve('organizationName'); + const settings = await Settings.select('organizationName'); const { organizationName } = await inquirer.prompt([ { diff --git a/graph/loaders/assets.js b/graph/loaders/assets.js index 40305760c..868c8af68 100644 --- a/graph/loaders/assets.js +++ b/graph/loaders/assets.js @@ -71,7 +71,7 @@ const findOrCreateAssetByURL = async (ctx, url) => { // Check for whitelisting + get the settings at the same time. const [whitelisted, settings] = await Promise.all([ DomainList.urlCheck(url), - Settings.load('autoCloseStream closedTimeout'), + Settings.select('autoCloseStream', 'closedTimeout'), ]); // If the domain wasn't whitelisted, then we shouldn't create this asset! diff --git a/graph/loaders/settings.js b/graph/loaders/settings.js index fa0a17459..a1329b619 100644 --- a/graph/loaders/settings.js +++ b/graph/loaders/settings.js @@ -1,23 +1,66 @@ -const SettingsService = require('../../services/settings'); +const Setting = require('../../models/setting'); +const Settings = require('../../services/settings'); const DataLoader = require('dataloader'); +const { zipObject } = require('lodash'); /** - * Creates a set of loaders based on a GraphQL context. - * @param {Object} context the context of the GraphQL request - * @return {Object} object of loaders + * SettingsLoader manages loading specific fields only of the Settings object. */ -module.exports = () => { - const loader = new DataLoader(selections => - Promise.all( - selections.map(fields => { - return SettingsService.retrieve(fields); - }) - ) - ); +class SettingsLoader { + constructor() { + this._loader = new DataLoader(this._batchLoadFn.bind(this)); + this._cache = null; + } - return { - Settings: { - load: (fields = false) => loader.load(fields), - }, - }; -}; + async _batchLoadFn(fields) { + // Load a settings object with all the requested fields, unless we have the + // entire object cached, in which case we'll return the whole cache. + const model = this._cache + ? await this._cache + : await Settings.retrieve(...fields); + + // Convert the model into an object for easier manipulation. + const obj = model.toObject(); + + // Return the specific fields for each of the fields that were loaded. + return fields.map(field => obj[field]); + } + + /** + * load will return the entire Settings object with all fields. + */ + load() { + if (this._cache) { + // Return the cached settings promise. + return this._cache; + } + + // Create a promise that will return the settings object. + const promise = Settings.retrieve(); + + // Set this as the cached value. + this._cache = promise; + + // Return the promised settings. + return promise; + } + + /** + * select will return a promise which resolves to the Settings object that + * contains the requested fields only. + * + * @param {Array} fields the fields from Settings we want to load. + */ + async select(...fields) { + // Load all the values for the specific fields. + const values = await this._loader.loadMany(fields); + + // Zip up the fields and values to create an object to return. + const obj = zipObject(fields, values); + + // Return the assembled Settings object. + return new Setting(obj); + } +} + +module.exports = () => ({ Settings: new SettingsLoader() }); diff --git a/graph/mutators/user.js b/graph/mutators/user.js index 8657905d9..7487eec2e 100644 --- a/graph/mutators/user.js +++ b/graph/mutators/user.js @@ -177,10 +177,10 @@ const changeUserPassword = async (ctx, oldPassword, newPassword) => { await Users.changePassword(user.id, newPassword); // Get some context for the email to be sent. - const { organizationName, organizationContactEmail } = await Settings.load([ + const { organizationName, organizationContactEmail } = await Settings.select( 'organizationName', - 'organizationContactEmail', - ]); + 'organizationContactEmail' + ); // Send the password change email. await Users.sendEmail(user, { diff --git a/graph/resolvers/asset.js b/graph/resolvers/asset.js index 6bdf4d19e..ae5188e6e 100644 --- a/graph/resolvers/asset.js +++ b/graph/resolvers/asset.js @@ -1,4 +1,4 @@ -const { decorateWithTags } = require('./util'); +const { decorateWithTags, getRequestedFields } = require('./util'); const Asset = { async comment({ id }, { id: commentId }, { loaders: { Comments } }) { @@ -64,10 +64,13 @@ const Asset = { return Comments.countByAssetID.load(id); }, - async settings({ settings = null }, _, { loaders: { Settings } }) { + async settings({ settings = null }, _, { loaders: { Settings } }, info) { + // Get the fields we want from the settings. + const fields = getRequestedFields(info); + // Load the global settings, and merge them into the asset specific settings // if we have some. - let globalSettings = await Settings.load(); + let globalSettings = await Settings.select(...fields); if (settings !== null) { settings = Object.assign({}, globalSettings.toObject(), settings); } else { diff --git a/graph/resolvers/comment.js b/graph/resolvers/comment.js index e562c062c..c40aa49e0 100644 --- a/graph/resolvers/comment.js +++ b/graph/resolvers/comment.js @@ -58,10 +58,14 @@ const Comment = { return Assets.getByID.load(asset_id); }, async editing(comment, _, { loaders: { Settings } }) { - const settings = await Settings.load(); - const editableUntil = new Date( - Number(new Date(comment.created_at)) + settings.editCommentWindowLength + const { editCommentWindowLength } = await Settings.select( + 'editCommentWindowLength' ); + + const editableUntil = new Date( + Number(new Date(comment.created_at)) + editCommentWindowLength + ); + return { edited: comment.edited, editableUntil: editableUntil, diff --git a/graph/resolvers/root_query.js b/graph/resolvers/root_query.js index 23fdef288..53296a2ca 100644 --- a/graph/resolvers/root_query.js +++ b/graph/resolvers/root_query.js @@ -1,4 +1,4 @@ -const { decorateWithPermissionCheck } = require('./util'); +const { decorateWithPermissionCheck, getRequestedFields } = require('./util'); const { SEARCH_ASSETS, SEARCH_OTHERS_COMMENTS, @@ -16,8 +16,12 @@ const RootQuery = { return Assets.getByURL(query.url); }, - settings(_, args, { loaders: { Settings } }) { - return Settings.load(); + settings(_, args, { loaders: { Settings } }, info) { + // Get the fields we want from the settings. + const fields = getRequestedFields(info); + + // Load only the requested fields. + return Settings.select(...fields); }, // This endpoint is used for loading moderation queues, so hide it in the diff --git a/graph/resolvers/util.js b/graph/resolvers/util.js index cec2a71dd..a1a745ab2 100644 --- a/graph/resolvers/util.js +++ b/graph/resolvers/util.js @@ -2,7 +2,8 @@ const { ADD_COMMENT_TAG, SEARCH_OTHER_USERS, } = require('../../perms/constants'); -const { property, isBoolean } = require('lodash'); +const { property, isBoolean, pull } = require('lodash'); +const graphqlFields = require('graphql-fields'); /** * getResolver will get the resolver from the typeResolver or apply the default @@ -207,7 +208,11 @@ const decorateWithTags = ( }; }; +const getRequestedFields = info => + pull(Object.keys(graphqlFields(info)), '__typename'); + module.exports = { + getRequestedFields, decorateUserField, decorateWithTags, decorateWithPermissionCheck, diff --git a/middleware/staticTemplate.js b/middleware/staticTemplate.js index 0dc05a7be..f16caaea9 100644 --- a/middleware/staticTemplate.js +++ b/middleware/staticTemplate.js @@ -95,7 +95,7 @@ const createResolveFactory = (() => { module.exports = async (req, res, next) => { try { // Attach the custom css url. - const { customCssUrl } = await SettingsService.retrieve('customCssUrl'); + const { customCssUrl } = await SettingsService.select('customCssUrl'); res.locals.customCssUrl = customCssUrl; } catch (err) { console.warn(err); diff --git a/models/schema/asset.js b/models/schema/asset.js index 043bc9a3f..82de92120 100644 --- a/models/schema/asset.js +++ b/models/schema/asset.js @@ -43,8 +43,7 @@ const Asset = new Schema( modified_date: Date, // This object is used exclusively for storing settings that are to override - // the base settings from the base Settings object. This is to be accessed - // always after running `rectifySettings` against it. + // the base settings from the base Settings object. settings: { default: {}, type: Object, diff --git a/models/schema/setting.js b/models/schema/setting.js index af7932910..eb9dc76d1 100644 --- a/models/schema/setting.js +++ b/models/schema/setting.js @@ -125,21 +125,4 @@ const Setting = new Schema( } ); -/** - * Merges two settings objects. - */ -Setting.method('merge', function(src) { - Setting.eachPath(path => { - // Exclude internal fields... - if (['id', '_id', '__v', 'created_at', 'updated_at'].includes(path)) { - return; - } - - // If the source object contains the path, shallow copy it. - if (path in src) { - this[path] = src[path]; - } - }); -}); - module.exports = Setting; diff --git a/package.json b/package.json index 1127dd05c..59ceceb95 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "graphql-ast-tools": "0.2.3", "graphql-docs": "0.2.0", "graphql-errors": "^2.1.0", + "graphql-fields": "^1.0.2", "graphql-redis-subscriptions": "1.3.0", "graphql-subscriptions": "^0.4.3", "graphql-tag": "^1.2.3", diff --git a/plugins/talk-plugin-local-auth/server/mutators.js b/plugins/talk-plugin-local-auth/server/mutators.js index 389574d2d..385d84c91 100644 --- a/plugins/talk-plugin-local-auth/server/mutators.js +++ b/plugins/talk-plugin-local-auth/server/mutators.js @@ -47,9 +47,9 @@ async function updateUserEmailAddress(ctx, email, confirmPassword) { ); // Get some context for the email to be sent. - const { organizationContactEmail } = await Settings.load([ - 'organizationContactEmail', - ]); + const { organizationContactEmail } = await Settings.select( + 'organizationContactEmail' + ); // Send off the email to the old email address that we have changed it. await Mailer.send({ diff --git a/plugins/talk-plugin-notifications/server/util.js b/plugins/talk-plugin-notifications/server/util.js index 7198df1e1..bbebef4b2 100644 --- a/plugins/talk-plugin-notifications/server/util.js +++ b/plugins/talk-plugin-notifications/server/util.js @@ -3,7 +3,7 @@ const getOrganizationName = async ctx => { const { loaders: { Settings } } = ctx; // Get the settings. - const { organizationName = null } = await Settings.load('organizationName'); + const { organizationName = null } = await Settings.select('organizationName'); return organizationName; }; diff --git a/plugins/talk-plugin-profile-data/server/connect.js b/plugins/talk-plugin-profile-data/server/connect.js index 0e5284e57..676d67176 100644 --- a/plugins/talk-plugin-profile-data/server/connect.js +++ b/plugins/talk-plugin-profile-data/server/connect.js @@ -36,10 +36,10 @@ module.exports = connectors => { const { organizationName, organizationContactEmail, - } = await Settings.load([ + } = await Settings.select( 'organizationName', - 'organizationContactEmail', - ]); + 'organizationContactEmail' + ); // rescheduledDeletionDate is the date in the future that we'll set the // user's account to be deleted on if this delete fails. diff --git a/plugins/talk-plugin-profile-data/server/mutators.js b/plugins/talk-plugin-profile-data/server/mutators.js index eec54772c..4a1e97791 100644 --- a/plugins/talk-plugin-profile-data/server/mutators.js +++ b/plugins/talk-plugin-profile-data/server/mutators.js @@ -81,7 +81,7 @@ async function sendDownloadLink(ctx) { // Generate the download links. const { downloadLandingURL } = await generateDownloadLinks(ctx, user.id); - const { organizationName } = await Settings.load('organizationName'); + const { organizationName } = await Settings.select('organizationName'); // Send the download link via the user's attached email account. await Users.sendEmail(user, { @@ -130,7 +130,7 @@ async function requestDeletion({ } ); - const { organizationName } = await Settings.load('organizationName'); + const { organizationName } = await Settings.select('organizationName'); // Send the download link via the user's attached email account. await Users.sendEmail(user, { @@ -171,7 +171,7 @@ async function cancelDeletion({ { $unset: { 'metadata.scheduledDeletionDate': 1 } } ); - const { organizationName } = await Settings.load('organizationName'); + const { organizationName } = await Settings.select('organizationName'); // Send the download link via the user's attached email account. await Users.sendEmail(user, { diff --git a/services/assets.js b/services/assets.js index fbed22287..794145eb3 100644 --- a/services/assets.js +++ b/services/assets.js @@ -1,13 +1,12 @@ const CommentModel = require('../models/comment'); const AssetModel = require('../models/asset'); -const SettingsService = require('./settings'); +const Settings = require('./settings'); const DomainList = require('./domain_list'); const { ErrAssetURLAlreadyExists, ErrNotFound, ErrInvalidAssetURL, } = require('../errors'); -const { merge, isEmpty } = require('lodash'); const { dotize } = require('./utils'); module.exports = class AssetsService { @@ -27,28 +26,6 @@ module.exports = class AssetsService { return AssetModel.findOne({ url }); } - /** - * Retrieves the settings given an asset query and rectifies it against the - * global settings. - * @param {Promise} assetQuery an asset query that returns a single asset. - * @return {Promise} - */ - static async rectifySettings(assetQuery, settings = null) { - const [globalSettings, asset] = await Promise.all([ - settings !== null ? settings : SettingsService.retrieve(), - assetQuery, - ]); - - // If the asset exists and has settings then return the merged object. - if (asset && asset.settings && !isEmpty(asset.settings)) { - settings = merge({}, globalSettings, asset.settings); - } else { - settings = globalSettings; - } - - return settings; - } - /** * Finds a asset by its url. * @@ -65,13 +42,13 @@ module.exports = class AssetsService { // Check the URL to confirm that is in the domain whitelist return Promise.all([ DomainList.urlCheck(url), - SettingsService.retrieve(), - ]).then(([whitelisted, settings]) => { + Settings.select('autoCloseStream', 'closedTimeout'), + ]).then(([whitelisted, { autoCloseStream, closedTimeout }]) => { const update = { $setOnInsert: { url } }; - if (settings.autoCloseStream) { + if (autoCloseStream) { update.$setOnInsert.closedAt = new Date( - Date.now() + settings.closedTimeout * 1000 + Date.now() + closedTimeout * 1000 ); } @@ -139,10 +116,10 @@ module.exports = class AssetsService { * @return {Promise} */ static search({ value, limit, open, sortOrder, cursor } = {}) { - let assets = AssetModel.find({}); + let query = AssetModel.find({}); if (value && value.length > 0) { - assets.merge({ + query.merge({ $text: { $search: value, }, @@ -151,7 +128,7 @@ module.exports = class AssetsService { if (open != null) { if (open) { - assets.merge({ + query.merge({ $or: [ { closedAt: null, @@ -164,7 +141,7 @@ module.exports = class AssetsService { ], }); } else { - assets.merge({ + query.merge({ closedAt: { $lt: Date.now(), }, @@ -174,13 +151,13 @@ module.exports = class AssetsService { if (cursor) { if (sortOrder === 'DESC') { - assets.merge({ + query.merge({ created_at: { $lt: cursor, }, }); } else { - assets.merge({ + query.merge({ created_at: { $gt: cursor, }, @@ -188,7 +165,7 @@ module.exports = class AssetsService { } } - return assets + return query .sort({ created_at: sortOrder === 'DESC' ? -1 : 1 }) .limit(limit); } diff --git a/services/cache.js b/services/cache.js index c413537a7..64f8ad934 100644 --- a/services/cache.js +++ b/services/cache.js @@ -271,69 +271,3 @@ cache.set = async (key, value, expiry, kf = keyfunc) => { return cache.client.set(kf(key), reply, 'EX', expiry); }; - -/** - * h is the hash form of the cache. - */ -cache.h = {}; - -cache.h.get = async (key, field = '__default__') => { - // Get the current value from redis. - const reply = await cache.client.hget(keyfunc(key), field); - - if (typeof reply !== 'undefined' && reply !== null) { - return JSON.parse(reply); - } - - return null; -}; - -cache.h.set = async (key, field = '__default__', value, expiry = 60) => { - // Serialize the value as JSON. - let reply = JSON.stringify(value); - - return cache.client - .pipeline() - .hset(keyfunc(key), field, reply) - .expire(keyfunc(key), expiry) - .exec(); -}; - -cache.h.invalidate = async (key, field = null) => { - if (field === null) { - return cache.invalidate(key); - } - - debug(`invalidate: ${keyfunc(key)} ${field}`); - - return cache.client.hdel(keyfunc(key), field); -}; - -cache.h.wrap = async (key, field, expiry, work) => { - let value = await cache.h.get(key, field); - if (value !== null) { - debug('wrap: hit', keyfunc(key)); - return value; - } - - debug('wrap: miss', keyfunc(key)); - - value = await work(); - - process.nextTick(async () => { - try { - await cache.h.set(key, field, value, expiry); - debug('wrap: set complete'); - } catch (err) { - console.error(err); - } - }); - - return value; -}; - -cache.h.incr = async (key, field = '__default__', expiry) => - cache.client.hincrbyex(keyfunc(key), field, 1, expiry); - -cache.h.decr = async (key, field = '__default__', expiry) => - cache.client.hincrbyex(keyfunc(key), field, -1, expiry); diff --git a/services/comments.js b/services/comments.js index b9d73d91c..4b436b5f4 100644 --- a/services/comments.js +++ b/services/comments.js @@ -100,7 +100,7 @@ module.exports = { // original query. const { editCommentWindowLength: editWindowMs, - } = await SettingsService.retrieve(); + } = await SettingsService.select('editCommentWindowLength'); const lastEditableCommentCreatedAt = new Date(Date.now() - editWindowMs); query.created_at = { $gt: lastEditableCommentCreatedAt, diff --git a/services/domain_list.js b/services/domain_list.js index aab1f9ea8..b194305b1 100644 --- a/services/domain_list.js +++ b/services/domain_list.js @@ -1,6 +1,6 @@ const debug = require('debug')('talk:services:domain_list'); const _ = require('lodash'); -const SettingsService = require('./settings'); +const Settings = require('./settings'); const { ROOT_URL } = require('../config'); @@ -19,7 +19,7 @@ class DomainList { * Loads domains white list in from the database */ async load() { - const { domains } = await SettingsService.retrieve(); + const { domains } = await Settings.select('domains'); this.upsert(domains); } diff --git a/services/moderation/index.js b/services/moderation/index.js index 530d6f2a6..27346e620 100644 --- a/services/moderation/index.js +++ b/services/moderation/index.js @@ -1,5 +1,5 @@ const { ErrNotFound } = require('../../errors'); -const get = require('lodash/get'); +const { get, merge, isEmpty } = require('lodash'); // Load in the phases to use. const { @@ -80,10 +80,7 @@ const compose = phases => async (ctx, comment, options) => { * @param {Object} comment comment object to use */ const fetchOptions = async (ctx, comment) => { - const { - connectors: { services: { Assets: AssetsService } }, - loaders: { Settings, Assets }, - } = ctx; + const { loaders: { Settings, Assets } } = ctx; // Load the settings. const settings = await Settings.load(); @@ -102,8 +99,12 @@ const fetchOptions = async (ctx, comment) => { throw new ErrNotFound(); } - // Combine the asset and the settings to get the asset settings. - asset.settings = await AssetsService.rectifySettings(asset, settings); + // If the asset exists and has settings then return the merged object. + if (asset && asset.settings && !isEmpty(asset.settings)) { + asset.settings = merge({}, settings, asset.settings); + } else { + asset.settings = settings; + } // Create the options that will be consumed by the phases. return { diff --git a/services/passport.js b/services/passport.js index 0ae06afb8..e4813e419 100644 --- a/services/passport.js +++ b/services/passport.js @@ -1,7 +1,7 @@ const passport = require('passport'); const { set, get } = require('lodash'); const UsersService = require('./users'); -const SettingsService = require('./settings'); +const Settings = require('./settings'); const TokensService = require('./tokens'); const fetch = require('node-fetch'); const FormData = require('form-data'); @@ -157,7 +157,9 @@ async function ValidateUserLogin(loginProfile, user, done) { } // The user is a local user, check if we need email confirmation. - const { requireEmailConfirmation = false } = await SettingsService.retrieve(); + const { requireEmailConfirmation = false } = await Settings.select( + 'requireEmailConfirmation' + ); // If we have the requirement of checking that emails for users are // verified, then we need to check the email address to ensure that it has diff --git a/services/settings.js b/services/settings.js index ee5125183..34b6fead4 100644 --- a/services/settings.js +++ b/services/settings.js @@ -1,59 +1,61 @@ -const SettingModel = require('../models/setting'); -const cache = require('./cache'); +const Setting = require('../models/setting'); const { ErrSettingsNotInit } = require('../errors'); const { dotize } = require('./utils'); -const { SETTINGS_CACHE_TIME } = require('../config'); +const { isEmpty, zipObject, uniq } = require('lodash'); +const DataLoader = require('dataloader'); -/** - * The selector used to uniquely identify the settings document. - */ const selector = { id: '1' }; -const retrieve = async fields => { - let settings; - if (fields) { - settings = await SettingModel.findOne(selector).select(fields); - } else { - settings = await SettingModel.findOne(selector); - } - if (!settings) { +async function loadFn(fields = []) { + const model = await Setting.findOne(selector).select(uniq(fields)); + if (!model) { throw new ErrSettingsNotInit(); } - return settings; -}; + return model; +} -/** - * The Setting Service object exposing the Setting model. - */ -module.exports = class SettingsService { - /** - * Gets the entire settings record and sends it back - * @return {Promise} settings the whole settings record - */ - static async retrieve(fields) { - if (process.env.NODE_ENV === 'production') { - // When in production, wrap the settings retrieval with a cache. - const settings = await cache.h.wrap( - 'settings', - fields, - SETTINGS_CACHE_TIME / 1000, - () => retrieve(fields) - ); +// batchLoadFn will load a settings object with all the requested fields. +async function batchLoadFn(fields) { + // Load a settings object with all the requested fields. + const model = await loadFn(fields); - return new SettingModel(settings); + // Convert the model into an object for easier manipulation. + const obj = model.toObject(); + + // Return the specific fields for each of the fields that were loaded. + return fields.map(field => obj[field]); +} + +// batchedSettingsLoader will load setting fields for each request. This isn't a +// cached loader, so this is really just for optimizing the requests made to the +// database by batching. +const batchedSettingsLoader = new DataLoader(batchLoadFn, { cache: false }); + +class Settings { + static async retrieve(...fields) { + if (!isEmpty(fields)) { + // Included for backwards compatibility. + return Settings.select(...fields); } - return retrieve(fields); + // Call the loadFn directly if we need to load all the fields. + return loadFn(fields); + } + + static async select(...fields) { + // Load all the values for the specific fields. + const values = await batchedSettingsLoader.loadMany(fields); + + // Zip up the fields and values to create an object to return. + const obj = zipObject(fields, values); + + // Return the assembled Settings object. + return new Setting(obj); } - /** - * This will update the settings object with whatever you pass in - * @param {object} setting a hash of whatever settings you want to update - * @return {Promise} settings Promise that resolves to the entire (updated) settings object. - */ static async update(settings) { - const updatedSettings = await SettingModel.findOneAndUpdate( + const updatedSettings = await Setting.findOneAndUpdate( selector, { $set: dotize(settings), @@ -65,21 +67,16 @@ module.exports = class SettingsService { } ); - if (process.env.NODE_ENV === 'production') { - await cache.h.invalidate('settings'); - } - return updatedSettings; } - /** - * This is run once when the app starts to ensure settings are populated. - */ static init(defaults = {}) { - return SettingsService.retrieve().catch(() => { - let settings = new SettingModel(defaults); + return Settings.retrieve().catch(() => { + const settings = new Setting(defaults); return settings.save(); }); } -}; +} + +module.exports = Settings; diff --git a/services/setup.js b/services/setup.js index 2517d376b..a691550ef 100644 --- a/services/setup.js +++ b/services/setup.js @@ -26,7 +26,7 @@ module.exports = class SetupService { try { // Get the current settings, we are expecting an error here. - await SettingsService.retrieve(); + await SettingsService.select('id'); // We should NOT have gotten a settings object, this means that the // application is already setup. Error out here. diff --git a/services/tags.js b/services/tags.js index cc8934e0a..b4e70cb29 100644 --- a/services/tags.js +++ b/services/tags.js @@ -1,23 +1,24 @@ -const CommentModel = require('../models/comment'); -const AssetModel = require('../models/asset'); -const UserModel = require('../models/user'); -const AssetsService = require('./assets'); -const SettingsService = require('./settings'); +const Comment = require('../models/comment'); +const Asset = require('../models/asset'); +const User = require('../models/user'); +const Assets = require('./assets'); +const Settings = require('./settings'); const { ADD_COMMENT_TAG } = require('../perms/constants'); const { ErrNotAuthorized } = require('../errors'); +const { get, has } = require('lodash'); const updateModel = async (item_type, query, update) => { // Get the model to update with. let Model; switch (item_type) { case 'COMMENTS': - Model = CommentModel; + Model = Comment; break; case 'ASSETS': - Model = AssetModel; + Model = Asset; break; case 'USERS': - Model = UserModel; + Model = User; break; default: throw new Error( @@ -44,32 +45,23 @@ const ownershipQuery = async (item_type, link, query) => { class TagsService { /** - * Retrives a global tag from the settings based on the input_type. + * Retrieves a global tag from the settings based on the input_type. */ static async getAll({ id, item_type, asset_id = null }) { - // Extract the settings from the database. - let settings; - switch (item_type) { - case 'COMMENTS': - settings = await AssetsService.rectifySettings( - AssetsService.findById(asset_id) - ); - break; - case 'ASSETS': - settings = await AssetsService.rectifySettings( - AssetsService.findById(id) - ); - break; - case 'USERS': - settings = await SettingsService.retrieve(); - break; - default: - settings = await SettingsService.retrieve(); - break; + // Optionally get an asset. + let asset; + if (item_type === 'COMMENTS') { + asset = await Assets.findById(asset_id); + } else if (item_type === 'ASSETS') { + asset = await Assets.findById(id); + } + + if (asset && has(asset, 'settings.tags')) { + return get(asset, 'settings.tags'); } // Extract the tags from the settings object. - let { tags = [] } = settings; + const { tags = [] } = await Settings.select('tags'); // Return the first tag that matches the requested form. return tags; diff --git a/services/wordlist.js b/services/wordlist.js index 9e7e1581e..22bde090a 100644 --- a/services/wordlist.js +++ b/services/wordlist.js @@ -1,12 +1,12 @@ const debug = require('debug')('talk:services:wordlist'); const _ = require('lodash'); -const SettingsService = require('./settings'); +const Settings = require('./settings'); const { ErrContainsProfanity } = require('../errors'); const memoize = require('lodash/memoize'); const { escapeRegExp } = require('./regex'); /** - * Generate a regulare expression that catches the `phrases`. + * Generate a regular expression that catches the `phrases`. */ function generateRegExp(phrases) { const inner = phrases @@ -49,7 +49,7 @@ class Wordlist { * Loads wordlists in from the database */ load() { - return SettingsService.retrieve().then(settings => { + return Settings.select('wordlist').then(settings => { // Insert the settings wordlist. this.upsert(settings.wordlist); }); diff --git a/yarn.lock b/yarn.lock index 9a455a9a6..75911aaa8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4451,6 +4451,10 @@ graphql-extensions@^0.0.x: core-js "^2.5.1" source-map-support "^0.5.0" +graphql-fields@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/graphql-fields/-/graphql-fields-1.0.2.tgz#099ee1d4445b42d0f47e06d622acebb33abc6cce" + graphql-redis-subscriptions@1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/graphql-redis-subscriptions/-/graphql-redis-subscriptions-1.3.0.tgz#bbc52b0f77bf7d50945c6bf4e8b8aba5135555b4"