diff --git a/config.js b/config.js index dd9aed13c..9b7c7adcc 100644 --- a/config.js +++ b/config.js @@ -44,6 +44,10 @@ const CONFIG = { // fetching again. SETTINGS_CACHE_TIME: ms(process.env.TALK_SETTINGS_CACHE_TIME || '1hr'), + // ALLOW_NO_LIMIT_QUERIES enables some queries to specify a limit of -1 to + // request all of the records. Otherwise, minimum limits of 0 are enforced. + ALLOW_NO_LIMIT_QUERIES: process.env.TALK_ALLOW_NO_LIMIT_QUERIES === 'TRUE', + //------------------------------------------------------------------------------ // JWT based configuration //------------------------------------------------------------------------------ diff --git a/docs/_docs/02-02-advanced-configuration.md b/docs/_docs/02-02-advanced-configuration.md index e985f8f20..fb083f63c 100644 --- a/docs/_docs/02-02-advanced-configuration.md +++ b/docs/_docs/02-02-advanced-configuration.md @@ -486,4 +486,10 @@ Used to set the key for use with [Apollo Engine](https://www.apollographql.com/engine/){:target="_blank"} for tracing of GraphQL requests. -**Note: Apollo Engine is a premium service, charges may apply.** \ No newline at end of file +**Note: Apollo Engine is a premium service, charges may apply.** + +## ALLOW_NO_LIMIT_QUERIES + +Setting this to `TRUE` will allow queries to execute without a limit (returns +all documents). This introduces a significant performance regression, and should +be used with caution. (Default `FALSE`) \ No newline at end of file diff --git a/graph/loaders/comments.js b/graph/loaders/comments.js index 7f0dbea00..37d40a219 100644 --- a/graph/loaders/comments.js +++ b/graph/loaders/comments.js @@ -4,7 +4,10 @@ const { SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS, SEARCH_OTHERS_COMMENTS, } = require('../../perms/constants'); -const { CACHE_EXPIRY_COMMENT_COUNT } = require('../../config'); +const { + CACHE_EXPIRY_COMMENT_COUNT, + ALLOW_NO_LIMIT_QUERIES, +} = require('../../config'); const ms = require('ms'); const sc = require('snake-case'); @@ -148,12 +151,11 @@ const getCommentCountByQuery = (ctx, options) => { * @param {Object} params the params from the client describing the query */ const getStartCursor = (ctx, nodes, { cursor, sortBy }) => { - switch (sortBy) { - case 'CREATED_AT': - return nodes.length ? nodes[0].created_at : null; - case 'REPLIES': - // The cursor is the start! This is using numeric pagination. - return cursor != null ? cursor : 0; + if (sortBy === 'CREATED_AT') { + return nodes.length ? nodes[0].created_at : null; + } else if (sortBy === 'REPLIES') { + // The cursor is the start! This is using numeric pagination. + return cursor != null ? cursor : 0; } const SORT_KEY = sortBy.toLowerCase(); @@ -181,11 +183,10 @@ const getStartCursor = (ctx, nodes, { cursor, sortBy }) => { * @param {Object} params the params from the client describing the query */ const getEndCursor = (ctx, nodes, { cursor, sortBy }) => { - switch (sortBy) { - case 'CREATED_AT': - return nodes.length ? nodes[nodes.length - 1].created_at : null; - case 'REPLIES': - return nodes.length ? (cursor != null ? cursor : 0) + nodes.length : null; + if (sortBy === 'CREATED_AT') { + return nodes.length ? nodes[nodes.length - 1].created_at : null; + } else if (sortBy === 'REPLIES') { + return nodes.length ? (cursor != null ? cursor : 0) + nodes.length : null; } const SORT_KEY = sortBy.toLowerCase(); @@ -212,36 +213,33 @@ const getEndCursor = (ctx, nodes, { cursor, sortBy }) => { * @param {Object} params the params from the client describing the query */ const applySort = (ctx, query, { cursor, sortOrder, sortBy }) => { - switch (sortBy) { - case 'CREATED_AT': { - if (cursor) { - if (sortOrder === 'DESC') { - query = query.where({ - created_at: { - $lt: cursor, - }, - }); - } else { - query = query.where({ - created_at: { - $gt: cursor, - }, - }); - } + if (sortBy === 'CREATED_AT') { + if (cursor) { + if (sortOrder === 'DESC') { + query = query.where({ + created_at: { + $lt: cursor, + }, + }); + } else { + query = query.where({ + created_at: { + $gt: cursor, + }, + }); } - - return query.sort({ created_at: sortOrder === 'DESC' ? -1 : 1 }); } - case 'REPLIES': { - if (cursor) { - query = query.skip(cursor); - } - return query.sort({ - reply_count: sortOrder === 'DESC' ? -1 : 1, - created_at: sortOrder === 'DESC' ? -1 : 1, - }); + return query.sort({ created_at: sortOrder === 'DESC' ? -1 : 1 }); + } else if (sortBy === 'REPLIES') { + if (cursor) { + query = query.skip(cursor); } + + return query.sort({ + reply_count: sortOrder === 'DESC' ? -1 : 1, + created_at: sortOrder === 'DESC' ? -1 : 1, + }); } const SORT_KEY = sortBy.toLowerCase(); @@ -280,7 +278,7 @@ const executeWithSort = async ( query = applySort(ctx, query, { cursor, sortOrder, sortBy }); // Apply the limit (if it exists, as it's applied universally). - if (limit) { + if (limit >= 0) { query = query.limit(limit + 1); } @@ -290,7 +288,7 @@ const executeWithSort = async ( // The hasNextPage is always handled the same (ask for one more than we need, // if there is one more, than there is more). let hasNextPage = false; - if (limit && nodes.length > limit) { + if (limit >= 0 && nodes.length > limit) { // There was one more than we expected! Set hasNextPage = true and remove // the last item from the array that we requested. hasNextPage = true; @@ -302,11 +300,9 @@ const executeWithSort = async ( return { startCursor: getStartCursor(ctx, nodes, { cursor, - sortOrder, sortBy, - limit, }), - endCursor: getEndCursor(ctx, nodes, { cursor, sortOrder, sortBy, limit }), + endCursor: getEndCursor(ctx, nodes, { cursor, sortBy }), hasNextPage, nodes, }; @@ -338,6 +334,11 @@ const getCommentsByQuery = async ( ) => { let comments = CommentModel.find(); + // Enforce that the limit must be gte 0 if this option is not true. + if (!ALLOW_NO_LIMIT_QUERIES && limit < 0) { + throw new Error('cannot query for limit < 0'); + } + // If user queries for statuses other than NONE and/or ACCEPTED statuses, it needs // special privileges. if (