Merge pull request #1204 from coralproject/document-optimization-reapply

Reapply optimizations

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) {

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,

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);
+};

client/coral-framework/graphql/reduceDocument.js

@@ -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];
-  };
-}

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 ||

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) {
 

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.

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({}))
   }
 });

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))

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),
+    }
+  };
+};

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({

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",

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);

services/hcache.js

@@ -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;
-};

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;

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

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"