mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-04-15 03:01:37 -05:00
Optimised queries made by get helper for posts (#19859)
ref ENG-747 ref https://linear.app/tryghost/issue/ENG-747 H'okay - so what we're trying to do here is make get helper queries more cacheable. The way we're doing that is by modifying the filter used when we're trying to remove a single post from the query. The idea is that we can remove that restriction on the filter, increase the number of posts fetched by 1 and then filter the fetched posts back down, this means that the same query, but filtering different posts, will be updated to make _exactly_ the same query, and so share a cache! We've been purposefully restrictive in the types of filters we manipulate, so that we only deal with the simplest cases and the code is easier to understand.
This commit is contained in:
Fabien 'egg' O'Carroll
GitHub
parent
48782df301
commit
52a28c0059
2 changed files with 158 additions and 2 deletions
|
|
@ -11,6 +11,7 @@ const Sentry = require('@sentry/node');
|
|||
|
||||
const _ = require('lodash');
|
||||
const jsonpath = require('jsonpath');
|
||||
const nqlLang = require('@tryghost/nql-lang');
|
||||
|
||||
const messages = {
|
||||
mustBeCalledAsBlock: 'The {\\{{helperName}}} helper must be called as a block. E.g. {{#{helperName}}}...{{/{helperName}}}',
|
||||
|
|
@ -131,6 +132,54 @@ function parseOptions(globals, data, options) {
|
|||
*/
|
||||
async function makeAPICall(resource, controllerName, action, apiOptions) {
|
||||
const controller = api[controllerName];
|
||||
let makeRequest = () => controller[action](apiOptions);
|
||||
|
||||
// Only do the optimisation on posts
|
||||
if (resource === 'posts' && apiOptions.filter) {
|
||||
try {
|
||||
const parsedFilter = nqlLang.parse(apiOptions.filter);
|
||||
// Support either `id:blah` or `id:blah+other:stuff`
|
||||
if (parsedFilter.$and || parsedFilter.id) {
|
||||
const queries = parsedFilter.$and || [parsedFilter.id];
|
||||
|
||||
for (const query of queries) {
|
||||
if ('id' in query) {
|
||||
if ('$ne' in query.id) {
|
||||
// This checks that there is only one occurence of a negative id filter
|
||||
// So we know it's safe to use the `replace` method
|
||||
if (apiOptions.filter.split('id:-').length === 2) {
|
||||
const idToFilter = query.id.$ne;
|
||||
|
||||
// The default limit is 15, the addition order is to cast apiOptions.limit to a number
|
||||
let limit = apiOptions.limit;
|
||||
limit = apiOptions.limit && apiOptions.limit !== 'all' ? 1 + apiOptions.limit : 16;
|
||||
|
||||
// We replace with id:-null so we don't have to deal with leading/trailing AND operators
|
||||
const filter = apiOptions.filter.replace(/id:-[a-f0-9A-F]{24}/, 'id:-null');
|
||||
|
||||
makeRequest = async () => {
|
||||
const result = await controller[action]({
|
||||
...apiOptions,
|
||||
limit,
|
||||
filter
|
||||
});
|
||||
|
||||
return {
|
||||
...result,
|
||||
posts: result?.posts?.filter((post) => {
|
||||
return post.id !== idToFilter;
|
||||
}).slice(0, limit - 1) || []
|
||||
};
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
logging.warn(err);
|
||||
}
|
||||
}
|
||||
|
||||
let timer;
|
||||
|
||||
|
|
@ -141,7 +190,7 @@ async function makeAPICall(resource, controllerName, action, apiOptions) {
|
|||
const logLevel = config.get('optimization:getHelper:timeout:level') || 'error';
|
||||
const threshold = config.get('optimization:getHelper:timeout:threshold');
|
||||
|
||||
const apiResponse = controller[action](apiOptions);
|
||||
const apiResponse = makeRequest();
|
||||
|
||||
const timeout = new Promise((resolve) => {
|
||||
timer = setTimeout(() => {
|
||||
|
|
@ -161,7 +210,7 @@ async function makeAPICall(resource, controllerName, action, apiOptions) {
|
|||
response = await Promise.race([apiResponse, timeout]);
|
||||
clearTimeout(timer);
|
||||
} else {
|
||||
response = await controller[action](apiOptions);
|
||||
response = await makeRequest();
|
||||
}
|
||||
|
||||
return response;
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
const assert = require('assert/strict');
|
||||
const should = require('should');
|
||||
const sinon = require('sinon');
|
||||
const testUtils = require('../../utils');
|
||||
|
|
@ -98,6 +99,112 @@ describe('e2e {{#get}} helper', function () {
|
|||
locals = {root: {_locals: {}}};
|
||||
});
|
||||
|
||||
describe('Filter optimisation', function () {
|
||||
it('Returns the correct posts with a solo negative filter', async function () {
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
limit: 1
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
const firstPostUsually = fn.firstCall.args[0].posts[0];
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `id:-${firstPostUsually.id}`,
|
||||
limit: 5
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
assert.equal(fn.secondCall.args[0].posts.length, 5);
|
||||
const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id);
|
||||
assert.equal(foundFilteredPost, undefined);
|
||||
});
|
||||
it('Returns the correct posts with a sandwiched negative filter', async function () {
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `tag:-hash-hidden+visibility:public`,
|
||||
limit: 1
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
const firstPostUsually = fn.firstCall.args[0].posts[0];
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `visibility:public+id:-${firstPostUsually.id}+tag:-hash-hidden`,
|
||||
limit: 5
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
assert.equal(fn.secondCall.args[0].posts.length, 5);
|
||||
const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id);
|
||||
assert.equal(foundFilteredPost, undefined);
|
||||
});
|
||||
it('Returns the correct posts with a prefix negative filter', async function () {
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `tag:-hash-hidden`,
|
||||
limit: 1
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
const firstPostUsually = fn.firstCall.args[0].posts[0];
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `id:-${firstPostUsually.id}+tag:-hash-hidden`,
|
||||
limit: 5
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
assert.equal(fn.secondCall.args[0].posts.length, 5);
|
||||
const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id);
|
||||
assert.equal(foundFilteredPost, undefined);
|
||||
});
|
||||
it('Returns the correct posts with a suffix negative filter', async function () {
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `tag:-hash-hidden`,
|
||||
limit: 1
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
const firstPostUsually = fn.firstCall.args[0].posts[0];
|
||||
await get.call({}, 'posts', {
|
||||
hash: {
|
||||
filter: `tag:-hash-hidden+id:-${firstPostUsually.id}`,
|
||||
limit: 5
|
||||
},
|
||||
data: {},
|
||||
locals,
|
||||
fn,
|
||||
inverse
|
||||
});
|
||||
assert.equal(fn.secondCall.args[0].posts.length, 5);
|
||||
const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id);
|
||||
assert.equal(foundFilteredPost, undefined);
|
||||
});
|
||||
});
|
||||
|
||||
describe('{{access}} property', function () {
|
||||
let member;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue