diff --git a/client/coral-embed-stream/src/graphql/utils.js b/client/coral-embed-stream/src/graphql/utils.js index 0c46b45e3..f4be867dd 100644 --- a/client/coral-embed-stream/src/graphql/utils.js +++ b/client/coral-embed-stream/src/graphql/utils.js @@ -142,17 +142,21 @@ export function findCommentWithId(nodes, id) { } export function findCommentInEmbedQuery(root, callbackOrId) { + return findCommentInAsset(root.asset, callbackOrId); +} + +export function findCommentInAsset(asset, callbackOrId) { let callback = callbackOrId; if (typeof callbackOrId === 'string') { callback = (node) => node.id === callbackOrId; } - if (root.asset.comment) { - return findComment([getTopLevelParent(root.asset.comment)], callback); + if (asset.comment) { + return findComment([getTopLevelParent(asset.comment)], callback); } - if (!root.asset.comments) { + if (!asset.comments) { return false; } - return findComment(root.asset.comments.nodes, callback); + return findComment(asset.comments.nodes, callback); } function findAndInsertFetchedComments(parent, comments, parent_id) { diff --git a/client/coral-embed-stream/src/tabs/stream/containers/Stream.js b/client/coral-embed-stream/src/tabs/stream/containers/Stream.js index dad69b960..e132ab954 100644 --- a/client/coral-embed-stream/src/tabs/stream/containers/Stream.js +++ b/client/coral-embed-stream/src/tabs/stream/containers/Stream.js @@ -19,6 +19,7 @@ import {Spinner} from 'coral-ui'; import {can} from 'coral-framework/services/perms'; import { findCommentInEmbedQuery, + findCommentInAsset, insertCommentIntoEmbedQuery, removeCommentFromEmbedQuery, insertFetchedCommentsIntoEmbedQuery, @@ -108,7 +109,7 @@ class StreamContainer extends React.Component { } loadNewReplies = (parent_id) => { - const comment = findCommentInEmbedQuery(this.props.root, parent_id); + const comment = findCommentInAsset(this.props.asset, parent_id); return this.props.data.fetchMore({ query: LOAD_MORE_QUERY, diff --git a/client/coral-embed/src/index.js b/client/coral-embed/src/index.js index f60a88439..935972795 100644 --- a/client/coral-embed/src/index.js +++ b/client/coral-embed/src/index.js @@ -70,10 +70,9 @@ export class Talk { // Extract the asset url. if (opts.asset_url) { query.asset_url = opts.asset_url; - } else if (!opts.asset_id) { + } else { - // The asset url was not provided and the asset id was also not provided, - // we need to infer the asset url from details on the page. + // The asset url was not provided so we need to infer the asset url from // details on the page. try { query.asset_url = document.querySelector('link[rel="canonical"]').href; diff --git a/client/coral-framework/graphql/anywhere.js b/client/coral-framework/graphql/anywhere.js new file mode 100644 index 000000000..7a93f16ea --- /dev/null +++ b/client/coral-framework/graphql/anywhere.js @@ -0,0 +1,23 @@ +import graphql from '@coralproject/graphql-anywhere-optimized'; +import {createTypeGetter} from 'graphql-ast-tools'; +import introspectionData from './introspection.json'; + +// Use typeGetter to get more optimized documents. +const typeGetter = createTypeGetter(introspectionData); + +// Use global fragment cache for transformed fragments. +const fragmentMap = {}; + +export default (...args) => { + while (args.length < 7) { + args.push(undefined); + } + + const transformOptions = { + typeGetter, + fragmentMap, + }; + + args[6] = transformOptions; + return graphql(...args); +}; diff --git a/client/coral-framework/graphql/reduceDocument.js b/client/coral-framework/graphql/reduceDocument.js deleted file mode 100644 index 12e0954d7..000000000 --- a/client/coral-framework/graphql/reduceDocument.js +++ /dev/null @@ -1,293 +0,0 @@ -import { - getMainDefinition, - getFragmentDefinitions, - createFragmentMap, - shouldInclude, - getOperationDefinition, -} from 'apollo-utilities'; - -function getDirectivesID(directives) { - let id = ''; - directives.forEach((directive) => { - id += `@${directive.name.value}(`; - let first = true; - directive.arguments.forEach((arg) => { - if (!first) { - id += ','; - } - first = false; - const value = arg.value.kind === 'Variable' - ? `$${arg.value.name.value}` - : arg.value.value; - id += `${arg.name.value}:${value}`; - }); - id += ')'; - }); - return id; -} - -// If two definitions have the same id, they can be merged. -function getDefinitionID(definition) { - - // Only merge when directives are exactly the same. - const trailing = definition.directives.length - ? `_${getDirectivesID(definition.directives)}` - : ''; - - switch (definition.kind) { - case 'FragmentSpread': - return `FragmentSpread_${definition.name.value}`; - case 'Field': - return `Field_${definition.alias ? definition.alias.value : definition.name.value}${trailing}`; - case 'InlineFragment': - return `InlineFragment_${definition.typeCondition.name.value}${trailing}`; - default: - throw new Error(`unknown definition kind ${definition.kind}`); - } -} - -/** - * Merge selections of 2 definitions. - */ -export function mergeDefinitions(a, b) { - const name = getDefinitionID(a); - - if (!!a.selectionSet !== !!b.selectionSet) { - throw Error(`incompatible field definition for ${name}`); - } - - if (!a.selectionSet) { - return b; - } - - const selectionSet = mergeSelectionSets(a.selectionSet, b.selectionSet); - - return { - ...b, - selectionSet, - }; -} - -/** - * Merge selectionSets - */ -export function mergeSelectionSets(a, b) { - const selectionsMap = [...a.selections, ...b.selections].reduce((o, sel) => { - const selName = getDefinitionID(sel); - if (!(selName in o)) { - o[selName] = sel; - return o; - } - o[selName] = mergeDefinitions(o[selName], sel); - return o; - }, {}); - - const selections = Object.keys(selectionsMap).map((key) => selectionsMap[key]); - - return { - ...b, - selections, - }; -} - -function getFragmentOrDie(name, execContext) { - const { - rawFragmentMap, - fragmentMap, - } = execContext; - - if (!(name in fragmentMap)) { - const fragment = rawFragmentMap[name]; - - if (!fragment) { - throw new Error(`fragment ${fragment.name.value} does not exist`); - } - - const typeCondition = fragment.typeCondition.name.value; - const transformed = transformDefinition(fragment, execContext, `type.${typeCondition}`, typeCondition); - fragmentMap[name] = transformed; - } - - return fragmentMap[name]; -} - -/** - * Return selections with resolved named fragments and directives. - */ -function getTransformedSelections(definition, path, gqlType, execContext) { - const { - variables, - } = execContext; - - const selectionsMap = definition.selectionSet.selections.reduce((o, sel) => { - if (variables && !shouldInclude(sel, variables)) { - - // Skip this entirely - return o; - } - if (sel.kind !== 'FragmentSpread') { - const transformed = transformDefinition(sel, execContext, path, gqlType); - const name = getDefinitionID(sel); - - // Merge existing value. - if (name in o) { - o[name] = mergeDefinitions(o[name], transformed); - return o; - } - - o[name] = transformed; - return o; - } - - const fragment = getFragmentOrDie(sel.name.value, execContext); - const typeCondition = fragment.typeCondition.name.value; - - // Turn NamedFragment into an InlineFragment. - if (gqlType !== typeCondition || fragment.directives.length) { - const node = { - ...fragment, - kind: 'InlineFragment', - }; - const name = getDefinitionID(node); - - // Merge existing value. - if (name in o) { - o[name] = mergeDefinitions(o[name], node); - return o; - } - - o[name] = node; - return o; - } - - // Merge NamedFragment directly into selections. - const fragmentSelections = fragment.selectionSet.selections; - fragmentSelections.forEach((s) => { - - if (variables && !shouldInclude(s, variables)) { - - // Skip this entirely - return; - } - - const selName = getDefinitionID(s); - if (!(selName in o)) { - o[selName] = s; - return; - } - - o[selName] = mergeDefinitions(o[selName], s); - }); - return o; - }, {}); - - const selections = Object.keys(selectionsMap).map((key) => selectionsMap[key]); - return selections; -} - -/** - * Resolve named fragments and directives in a definition. - */ -function transformDefinition(definition, execContext, path = '', type = null) { - if (!definition.selectionSet) { - return definition; - } - - const {typeGetter} = execContext; - - if (definition.kind === 'Field') { - const fieldName = definition.name.value; - path = `${path}.${fieldName}`; - - if (typeGetter) { - type = typeGetter(path); - } - } - - // InlineFragments - else if(!type && typeGetter) { - type = typeGetter(path); - } - - return { - ...definition, - selectionSet: { - ...definition.selectionSet, - selections: getTransformedSelections(definition, path, type, execContext), - }, - }; -} - -export default function reduceDocument(document, options = {}) { - const mainDefinition = getMainDefinition(document); - const fragments = getFragmentDefinitions(document); - const operationDefinition = getOperationDefinition(document); - const path = operationDefinition - ? operationDefinition.operation - : `type.${mainDefinition.typeCondition.name.value}`; - - const execContext = { - rawFragmentMap: createFragmentMap(fragments), - fragmentMap: options.fragmentMap || {}, - variables: options.variables, - typeGetter: options.typeGetter || (() => null), - }; - - return { - kind: 'Document', - definitions: [transformDefinition(mainDefinition, execContext, path)], - }; -} - -function getObjectType(fieldType) { - if (['NON_NULL', 'LIST'].indexOf(fieldType.kind) > -1) { - return getObjectType(fieldType.ofType); - } - return fieldType.name; -} - -function getFieldType(parentType, fieldName) { - const field = parentType.fields.find((f) => f.name === fieldName); - return getObjectType(field.type); -} - -export function createTypeGetter(introspectionData) { - const types = {}; - introspectionData.__schema.types.forEach((type) => types[type.name] = type); - - const result = { - 'query': introspectionData.__schema.queryType.name, - 'mutation': introspectionData.__schema.mutationType.name, - 'subscription': introspectionData.__schema.subscriptionType.name, - }; - - return (path) => { - if (result[path]) { - return result[path]; - } - let currentPath = ''; - const parts = path.split('.'); - for (let i = 0; i < parts.length; i++) { - const part = parts[i]; - - // Handle special path e.g. 'type.ROOT_QUERY.fieldName' - if (part === 'type') { - const type = parts[i + 1]; - const nextPath = `type.${type}`; - result[nextPath] = type; - currentPath = nextPath; - i++; - continue; - } - - const nextPath = currentPath ? `${currentPath}.${part}` : part; - if (nextPath in result) { - currentPath = nextPath; - continue; - } - result[nextPath] = getFieldType(types[result[currentPath]], part); - currentPath = nextPath; - } - return result[path]; - }; -} diff --git a/client/coral-framework/hocs/withQuery.js b/client/coral-framework/hocs/withQuery.js index 8f0155a11..280191f89 100644 --- a/client/coral-framework/hocs/withQuery.js +++ b/client/coral-framework/hocs/withQuery.js @@ -108,6 +108,12 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { this.subscriptionQueue = []; }, 1000); + handleOnLoaded() { + + // Trigger subscription queue processing after query has loaded. + setTimeout(() => this.processSubscriptionQueue(), 1000); + } + subscribeToMoreThrottled = ({document, variables, updateQuery}) => { // We need to add the typenames and resolve fragments. @@ -122,8 +128,12 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { if (data) { this.subscriptionQueue.push([updateQuery, data]); - // Triggers the throttled subscription queue processor. - this.processSubscriptionQueue(); + // Only trigger handler when query has been loaded. + if (!this.data.loading) { + + // Triggers the throttled subscription queue processor. + this.processSubscriptionQueue(); + } } }; @@ -145,6 +155,11 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { // If data was previously set, we update it in a immutable way. if (this.data) { + + if (this.data.loading && !data.loading) { + this.handleOnLoaded(); + } + if (this.data.networkStatus !== data.networkStatus || this.data.loading !== data.loading || this.data.error !== data.error || diff --git a/client/coral-framework/services/bootstrap.js b/client/coral-framework/services/bootstrap.js index 3ef6629c1..1328b169d 100644 --- a/client/coral-framework/services/bootstrap.js +++ b/client/coral-framework/services/bootstrap.js @@ -124,10 +124,6 @@ export async function createContext({ const plugins = createPluginsService(pluginsConfig); const graphql = createGraphQLService( createGraphQLRegistry(plugins.getSlotFragments.bind(plugins)), - { - introspectionData, - optimize: process.env.NODE_ENV === 'production', - }, ); if (!notification) { diff --git a/client/coral-framework/services/graphql.js b/client/coral-framework/services/graphql.js index 859d57d8a..d7b132bd4 100644 --- a/client/coral-framework/services/graphql.js +++ b/client/coral-framework/services/graphql.js @@ -1,4 +1,3 @@ -import reduceDocument, {createTypeGetter} from '../graphql/reduceDocument'; import {addTypenameToDocument} from 'apollo-client/queries/queryTransform'; /** @@ -6,18 +5,7 @@ import {addTypenameToDocument} from 'apollo-client/queries/queryTransform'; * @param {string} basename base path of the url * @return {Object} histor service */ -export function createGraphQLService(registry, { - introspectionData, - optimize = false, -}) { - const reduceOptions = { - typeGetter: optimize && introspectionData ? createTypeGetter(introspectionData) : null, - - // Use shared fragment map. - // Attention: Fragment names must be unique otherwise weird things will happen. - fragmentMap: {}, - }; - +export function createGraphQLService(registry) { return { registry, resolveDocument(documentOrCallback, props, context) { @@ -26,10 +14,6 @@ export function createGraphQLService(registry, { : documentOrCallback; document = registry.resolveFragments(document); - if (optimize) { - document = reduceDocument(document, reduceOptions); - } - // We also add typenames to the document which apollo would usually do, // but we also use the network interface in subscriptions directly // which require the resolved typenames. diff --git a/docs/_docs/01-01-talk-quickstart.md b/docs/_docs/01-01-talk-quickstart.md index c5e7a20ee..811bfe5ed 100644 --- a/docs/_docs/01-01-talk-quickstart.md +++ b/docs/_docs/01-01-talk-quickstart.md @@ -26,7 +26,7 @@ to persist data. The following versions are supported: An optional dependency for Talk is [Docker](https://www.docker.com/community-edition#/download){:target="_blank"}. -It is used during [development](#development) to setup the database and can be +It is used during [development](#development) to set up the database and can be used to [install via Docker](#installation-from-docker). We have tested Talk and this documentation with versions {{ site.versions.docker }}. @@ -182,4 +182,4 @@ machine. At this point you've successfully installed, configured, and ran your very own instance of Talk! Continue through this documentation on this site to learn more -on how to configure, develop with, and contribute to Talk! \ No newline at end of file +on how to configure, develop with, and contribute to Talk! diff --git a/graph/loaders/assets.js b/graph/loaders/assets.js index 832b7b473..6ea7e6599 100644 --- a/graph/loaders/assets.js +++ b/graph/loaders/assets.js @@ -1,31 +1,14 @@ const DataLoader = require('dataloader'); -const url = require('url'); +const {URL} = require('url'); +const {singleJoinBy, SingletonResolver} = require('./util'); -const errors = require('../../errors'); -const scraper = require('../../services/scraper'); -const util = require('./util'); - -const AssetModel = require('../../models/asset'); -const AssetsService = require('../../services/assets'); - -/** - * Retrieves assets by an array of ids. - * @param {Object} context the context of the request - * @param {Array} ids array of ids to lookup - */ -const genAssetsByID = (context, ids) => AssetModel.find({ +const genAssetsByID = ({connectors: {models: {Asset}}}, ids) => Asset.find({ id: { $in: ids } -}).then(util.singleJoinBy(ids, 'id')); +}).then(singleJoinBy(ids, 'id')); -/** - * [getAssetsByQuery description] - * @param {Object} context the context of the request - * @param {Object} query the query - * @return {Promise} resolves the assets - */ -const getAssetsByQuery = async (context, query) => { +const getAssetsByQuery = async ({connectors: {services: {Assets}}}, query) => { // If we are requesting based on a limit, ask for one more than we want. const limit = query.limit; @@ -33,7 +16,7 @@ const getAssetsByQuery = async (context, query) => { query.limit += 1; } - const nodes = await AssetsService.search(query); + const nodes = await Assets.search(query); // The hasNextPage is always handled the same (ask for one more than we need, // if there is one more, than there is more). @@ -54,58 +37,121 @@ const getAssetsByQuery = async (context, query) => { }; }; -/** - * This endpoint find or creates an asset at the given url when it is loaded. - * @param {Object} context the context of the request - * @param {String} asset_url the url passed in from the query - * @returns {Promise} resolves to the asset - */ -const findOrCreateAssetByURL = async (context, asset_url) => { +const findOrCreateAssetByURL = async (ctx, url) => { - // Verify that the asset_url is parsable. - let parsed_asset_url = url.parse(asset_url); - if (!parsed_asset_url.protocol) { - throw errors.ErrInvalidAssetURL; + // Pull our connectors out of the context. + const { + loaders: { + Assets, + Settings, + }, + connectors: { + models: { + Asset, + }, + services: { + DomainList, + Scraper, + }, + errors: { + ErrInvalidAssetURL, + }, + }, + } = ctx; + + // Try to validate that the url is valid. If the URL constructor throws an + // error, throw our internal ErrInvalidAssetURL instead. This will validate + // that the url contains a valid scheme. + try { + new URL(url); + } catch (err) { + throw ErrInvalidAssetURL; } - let asset = await AssetsService.findOrCreateByUrl(asset_url); + // Try the easy lookup first. + let asset = await Assets.findByUrl(url); + if (asset) { + return asset; + } - // If the asset wasn't scraped before, scrape it! Otherwise just return - // the asset. + // Seems the asset wasn't here yet.. We should do some validation. + + // Check for whitelisting + get the settings at the same time. + const [ + whitelisted, + settings, + ] = await Promise.all([ + DomainList.urlCheck(url), + Settings.load('autoCloseStream closedTimeout'), + ]); + + // If the domain wasn't whitelisted, then we shouldn't create this asset! + if (!whitelisted) { + throw ErrInvalidAssetURL; + } + + // Construct the update operator that we'll use to create the asset. + const update = { + $setOnInsert: { + url, + }, + }; + + // If the auto-close stream is enabled, close the stream after the designated + // timeout. + if (settings.autoCloseStream) { + update.$setOnInsert.closedAt = new Date(Date.now() + settings.closedTimeout * 1000); + } + + // We're using the findOneAndUpdate here instead of a insert to protect + // against race conditions. + asset = await Asset.findOneAndUpdate({ + url, + }, update, { + + // Ensure that if it's new, we return the new object created. + new: true, + + // Perform an upsert in the event that this doesn't exist. + upsert: true, + + // Set the default values if not provided based on the mongoose models. + setDefaultsOnInsert: true, + + // Ensure that we validate the input that we do have. + runValidators: true, + }); + + // If this is a new asset, then we need to scrape it! if (!asset.scraped) { - await scraper.create(asset); + + // Create the Scraper job. + await Scraper.create(asset); } return asset; }; -const findByUrl = async (context, asset_url) => { +const findByUrl = async ({connectors: {errors, services: {Assets}}}, asset_url) => { - // Verify that the asset_url is parsable. - let parsed_asset_url = url.parse(asset_url); - if (!parsed_asset_url.protocol) { + // Try to validate that the url is valid. If the URL constructor throws an + // error, throw our internal ErrInvalidAssetURL instead. This will validate + // that the url contains a valid scheme. + try { + new URL(asset_url); + } catch (err) { throw errors.ErrInvalidAssetURL; } - return AssetsService.findByUrl(asset_url); + return Assets.findByUrl(asset_url); }; -/** - * Creates a set of loaders based on a GraphQL context. - * @param {Object} context the context of the GraphQL request - * @return {Object} object of loaders - */ - -module.exports = (context) => ({ +module.exports = (ctx) => ({ Assets: { - - // TODO: decide whether we want to move these to mutators or not, as in fact - // this operation create a new asset if one isn't found. - getByURL: (url) => findOrCreateAssetByURL(context, url), - - findByUrl: (url) => findByUrl(context, url), - getByQuery: (query) => getAssetsByQuery(context, query), - getByID: new DataLoader((ids) => genAssetsByID(context, ids)), - getAll: new util.SingletonResolver(() => AssetModel.find({})) + getByURL: (url) => findOrCreateAssetByURL(ctx, url), + findByUrl: (url) => findByUrl(ctx, url), + getByQuery: (query) => getAssetsByQuery(ctx, query), + getByID: new DataLoader((ids) => genAssetsByID(ctx, ids)), + getAll: new SingletonResolver(() => ctx.connectors.models.Asset.find({})) } }); diff --git a/graph/loaders/comments.js b/graph/loaders/comments.js index 2ad94deac..050bd6340 100644 --- a/graph/loaders/comments.js +++ b/graph/loaders/comments.js @@ -82,54 +82,52 @@ const getParentCountsByAssetID = (context, asset_ids) => { /** * Retrieves the count of comments based on the passed in query. - * @param {Object} context graph context + * @param {Object} ctx graph context * @param {Object} query query to execute against the comments collection * to compute the counts * @return {Promise} resolves to the counts of the comments from the * query */ -const getCommentCountByQuery = (context, {ids, statuses, asset_id, parent_id, author_id, tags, action_type}) => { - let query = CommentModel.find(); +const getCommentCountByQuery = (ctx, options) => { + const {statuses, asset_id, parent_id, author_id, tags, action_type} = options; // If user queries for statuses other than NONE and/or ACCEPTED statuses, it needs // special privileges. if ( (!statuses || statuses.some((status) => !['NONE', 'ACCEPTED'].includes(status))) && - (context.user == null || !context.user.can(SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS)) + (ctx.user == null || !ctx.user.can(SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS)) ) { return null; } - if (statuses && statuses.length > 0) { - query = query.where({status: {$in: statuses}}); - } - - if (ids) { - query = query.where({id: {$in: ids}}); - } + const query = CommentModel.find(); if (asset_id != null) { - query = query.where({asset_id}); + query.merge({asset_id}); } if (parent_id !== undefined) { - query = query.where({parent_id}); + query.merge({parent_id}); } if (author_id) { - query = query.where({author_id}); + query.merge({author_id}); } - if (context.user != null && context.user.can(SEARCH_OTHERS_COMMENTS) && action_type) { - query = query.where({ + if (ctx.user != null && ctx.user.can(SEARCH_OTHERS_COMMENTS) && action_type) { + query.merge({ [`action_counts.${sc(action_type.toLowerCase())}`]: { $gt: 0, }, }); } - if (tags) { - query = query.find({ + if (statuses && statuses.length > 0) { + query.merge({status: {$in: statuses}}); + } + + if (tags && tags.length > 0) { + query.merge({ 'tags.tag.name': { $in: tags, }, @@ -289,7 +287,7 @@ const getCommentsByQuery = async (ctx, {ids, statuses, asset_id, parent_id, auth let comments = CommentModel.find(); // If user queries for statuses other than NONE and/or ACCEPTED statuses, it needs - // special priviledges. + // special privileges. if ( (!statuses || statuses.some((status) => !['NONE', 'ACCEPTED'].includes(status))) && (ctx.user == null || !ctx.user.can(SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS)) diff --git a/graph/loaders/settings.js b/graph/loaders/settings.js index 2d6189b7d..a7a4b74e1 100644 --- a/graph/loaders/settings.js +++ b/graph/loaders/settings.js @@ -1,11 +1,19 @@ const SettingsService = require('../../services/settings'); -const {SingletonResolver} = require('./util'); +const DataLoader = require('dataloader'); /** * Creates a set of loaders based on a GraphQL context. * @param {Object} context the context of the GraphQL request * @return {Object} object of loaders */ -module.exports = () => ({ - Settings: new SingletonResolver(() => SettingsService.retrieve()) -}); +module.exports = () => { + const loader = new DataLoader((selections) => Promise.all(selections.map((fields) => { + return SettingsService.retrieve(fields); + }))); + + return { + Settings: { + load: (fields = false) => loader.load(fields), + } + }; +}; diff --git a/models/comment.js b/models/comment.js index bc4cecd5d..ae6aa558f 100644 --- a/models/comment.js +++ b/models/comment.js @@ -123,20 +123,23 @@ CommentSchema.index({ background: true, }); -// Add an index that is optimized for sorting based on the action count data. -CommentSchema.index({ - 'created_at': 1, - 'action_counts.flag': 1, -}, { - background: true, -}); - +// Create a sparse index to search across. CommentSchema.index({ 'created_at': 1, 'action_counts.flag': 1, 'status': 1, }, { background: true, + sparse: true, +}); + +// Create a sparse index to search across. +CommentSchema.index({ + 'action_counts.flag': 1, + 'status': 1, +}, { + background: true, + sparse: true, }); // Add an index that is optimized for finding flagged comments. @@ -166,6 +169,15 @@ CommentSchema.index({ background: true, }); +// Optimize for tag searches/counts. +CommentSchema.index({ + 'tags.tag.name': 1, + 'status': 1, +}, { + background: true, + sparse: true, +}); + // Add an index that is optimized for sorting based on the created_at timestamp // but also good at locating comments that have a specific asset id. CommentSchema.index({ diff --git a/package.json b/package.json index 6bf075809..0ddcf075e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "talk", - "version": "3.8.1", + "version": "3.8.2", "description": "A better commenting experience from Mozilla, The New York Times, and the Washington Post. https://coralproject.net", "main": "app.js", "private": true, @@ -61,7 +61,6 @@ "@coralproject/graphql-anywhere-optimized": "^0.1.0", "accepts": "^1.3.4", "apollo-client": "^1.9.1", - "apollo-utilities": "^1.0.3", "app-module-path": "^2.2.0", "autoprefixer": "^6.5.2", "babel-cli": "6.26.0", @@ -107,6 +106,7 @@ "fs-extra": "^4.0.1", "gql-merge": "^0.0.4", "graphql": "^0.9.1", + "graphql-ast-tools": "0.2.3", "graphql-docs": "0.2.0", "graphql-errors": "^2.1.0", "graphql-redis-subscriptions": "1.3.0", diff --git a/services/cache.js b/services/cache.js index 155992fe0..cac05c091 100644 --- a/services/cache.js +++ b/services/cache.js @@ -61,30 +61,36 @@ cache.init = async () => { // This is designed to increment a key and add an expiry iff the key already // exists. - const INCR_SCRIPT = ` - if redis.call('GET', KEYS[1]) ~= false then - redis.call('INCR', KEYS[1]) - redis.call('EXPIRE', KEYS[1], ARGV[1]) - end - `; - cache.client.defineCommand('increx', { numberOfKeys: 1, - lua: INCR_SCRIPT, + lua: ` + if redis.call('GET', KEYS[1]) ~= false then + redis.call('INCR', KEYS[1]) + redis.call('EXPIRE', KEYS[1], ARGV[1]) + end + `, }); // This is designed to decrement a key and add an expiry iff the key already // exists. - const DECR_SCRIPT = ` - if redis.call('GET', KEYS[1]) ~= false then - redis.call('DECR', KEYS[1]) - redis.call('EXPIRE', KEYS[1], ARGV[1]) - end - `; - cache.client.defineCommand('decrex', { numberOfKeys: 1, - lua: DECR_SCRIPT, + lua: ` + if redis.call('GET', KEYS[1]) ~= false then + redis.call('DECR', KEYS[1]) + redis.call('EXPIRE', KEYS[1], ARGV[1]) + end + `, + }); + + cache.client.defineCommand('hincrbyex', { + numberOfKeys: 2, + lua: ` + if redis.call('HGET', KEYS[1], KEYS[2]) ~= false then + redis.call('HINCRBY', KEYS[1], KEYS[2], ARGV[1]) + redis.call('EXPIRE', KEYS[1], ARGV[2]) + end + `, }); }; @@ -270,3 +276,69 @@ 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/hcache.js b/services/hcache.js deleted file mode 100644 index 72e52d9eb..000000000 --- a/services/hcache.js +++ /dev/null @@ -1,61 +0,0 @@ -const cache = require('./cache'); -const debug = require('debug')('talk:services:hcache'); - -const kf = (key) => `hcache:${key}`; - -const hcache = module.exports = {}; - -hcache.get = async (key, field = '__default__') => { - - // Get the current value from redis. - const reply = await cache.client.hget(kf(key), field); - - if (typeof reply !== 'undefined' && reply !== null) { - return JSON.parse(reply); - } - - return null; -}; - -hcache.set = async (key, field = '__default__', value, expiry = 60) => { - - // Serialize the value as JSON. - let reply = JSON.stringify(value); - - return cache.client - .pipeline() - .hset(kf(key), field, reply) - .expire(kf(key), expiry) - .exec(); -}; - -hcache.del = async (key, field = null) => { - if (field === null) { - return cache.client.del(kf(key)); - } - - return cache.client.hdel(kf(key), field); -}; - -hcache.wrap = async (key, field, expiry, work) => { - let value = await hcache.get(key, field); - if (value !== null) { - debug('wrap: hit', kf(key)); - return value; - } - - debug('wrap: miss', kf(key)); - - value = await work(); - - process.nextTick(async () => { - try { - await hcache.set(key, field, value, expiry); - debug('wrap: set complete'); - } catch (err) { - console.error(err); - } - }); - - return value; -}; diff --git a/services/settings.js b/services/settings.js index 90ff6bbd6..e8537c9c9 100644 --- a/services/settings.js +++ b/services/settings.js @@ -1,5 +1,5 @@ const SettingModel = require('../models/setting'); -const hcache = require('./hcache'); +const cache = require('./cache'); const errors = require('../errors'); const {dotize} = require('./utils'); @@ -35,7 +35,7 @@ module.exports = class SettingsService { if (process.env.NODE_ENV === 'production') { // When in production, wrap the settings retrieval with a cache. - const settings = await hcache.wrap('settings', fields, 60, () => retrieve(fields)); + const settings = await cache.h.wrap('settings', fields, 60, () => retrieve(fields)); return new SettingModel(settings); } @@ -58,7 +58,7 @@ module.exports = class SettingsService { }); if (process.env.NODE_ENV === 'production') { - await hcache.del('settings'); + await cache.h.invalidate('settings'); } return updatedSettings; diff --git a/webpack.config.js b/webpack.config.js index 60a49b6eb..14ac7caf5 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -120,7 +120,7 @@ const config = { }, resolve: { alias: { - 'graphql-anywhere': '@coralproject/graphql-anywhere-optimized', + 'graphql-anywhere': path.resolve(__dirname, 'client/coral-framework/graphql/anywhere'), 'plugin-api': path.resolve(__dirname, 'plugin-api/'), plugins: path.resolve(__dirname, 'plugins/'), pluginsConfig: pluginsPath diff --git a/yarn.lock b/yarn.lock index 78ded3707..ea99f0cfa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12,8 +12,10 @@ eslint-plugin-react "^7.3.0" "@coralproject/graphql-anywhere-optimized@^0.1.0": - version "0.1.0" - resolved "https://registry.yarnpkg.com/@coralproject/graphql-anywhere-optimized/-/graphql-anywhere-optimized-0.1.0.tgz#3456f16f790d8593b0ca4355910578ab1c6edab5" + version "0.1.5" + resolved "https://registry.yarnpkg.com/@coralproject/graphql-anywhere-optimized/-/graphql-anywhere-optimized-0.1.5.tgz#67c862bf908ea717d9521ea76266b5bc9f109c65" + dependencies: + graphql-ast-tools "^0.2.2" "@kadira/storybook-deployer@^1.1.0": version "1.2.0" @@ -3519,6 +3521,12 @@ graphql-anywhere@^3.0.1: version "3.1.0" resolved "https://registry.yarnpkg.com/graphql-anywhere/-/graphql-anywhere-3.1.0.tgz#3ea0d8e8646b5cee68035016a9a7557c15c21e96" +graphql-ast-tools@0.2.3, graphql-ast-tools@^0.2.2: + version "0.2.3" + resolved "https://registry.yarnpkg.com/graphql-ast-tools/-/graphql-ast-tools-0.2.3.tgz#447fb05905ebb90f0a5bba81d5715249e9937135" + dependencies: + apollo-utilities "^1.0.3" + graphql-docs@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/graphql-docs/-/graphql-docs-0.2.0.tgz#cf803f9c9d354fa03e89073d74e419261a5bfa74"