From 44ae0dcbe17f5bb1e01b7b820e9ce0b616e7bd8d Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 24 Aug 2023 20:08:36 +0700 Subject: [PATCH] Added rudimentary tests for Collections DB queries (#17820) refs https://github.com/TryGhost/Arch/issues/73 This is just an initial stab at making sure we don't introduce extra DB queries related to collections without being aware of it. --- .../__snapshots__/collections.test.js.snap | 188 ++++++++++++++++++ .../test/e2e-api/admin/collections.test.js | 160 +++++++++++++++ 2 files changed, 348 insertions(+) diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/collections.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/collections.test.js.snap index d6f104eb11..bb5f963fc3 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/collections.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/collections.test.js.snap @@ -1365,6 +1365,194 @@ Object { } `; +exports[`Collections API Browse Makes limited DB queries when browsing 1: [body] 1`] = ` +Object { + "collections": Array [ + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": "All posts", + "feature_image": null, + "filter": "", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "slug": "latest", + "title": "Latest", + "type": "automatic", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + }, + Object { + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": "Featured posts", + "feature_image": null, + "filter": "featured:true", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "slug": "featured", + "title": "Featured", + "type": "automatic", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + }, + ], + "meta": Object { + "pagination": Object { + "limit": 2, + "next": null, + "page": 1, + "pages": 1, + "prev": null, + "total": 2, + }, + }, +} +`; + +exports[`Collections API Browse Makes limited DB queries when browsing 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "576", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 1: [body] 1`] = ` +Object { + "collections": Array [ + Object { + "count": Object { + "posts": 2, + }, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": "Featured posts", + "feature_image": null, + "filter": "featured:true", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "slug": "featured", + "title": "Featured", + "type": "automatic", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + }, + ], +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "284", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 3: [body] 1`] = ` +Object { + "collections": Array [ + Object { + "count": Object { + "posts": 2, + }, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": "Featured posts", + "feature_image": null, + "filter": "featured:true", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "slug": "featured", + "title": "Featured", + "type": "automatic", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + }, + ], +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 4: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "284", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 5: [body] 1`] = ` +Object { + "collections": Array [ + Object { + "count": Object { + "posts": 3, + }, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": "Featured posts", + "feature_image": null, + "filter": "featured:true", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "slug": "featured", + "title": "Featured", + "type": "automatic", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + }, + ], +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 6: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "284", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 7: [body] 1`] = ` +Object { + "collections": Array [ + Object { + "count": Object { + "posts": 2, + }, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": "Featured posts", + "feature_image": null, + "filter": "featured:true", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "slug": "featured", + "title": "Featured", + "type": "automatic", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + }, + ], +} +`; + +exports[`Collections API Collection Posts updates automatically Makes limited DB queries when updating due to post changes 8: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "284", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Collections API Collection Posts updates automatically Updates a collection with tag filter when tag is added to posts in bulk and when tag is removed 1: [body] 1`] = ` Object { "collections": Array [ diff --git a/ghost/core/test/e2e-api/admin/collections.test.js b/ghost/core/test/e2e-api/admin/collections.test.js index d19e3a55a9..36cb608bf9 100644 --- a/ghost/core/test/e2e-api/admin/collections.test.js +++ b/ghost/core/test/e2e-api/admin/collections.test.js @@ -47,6 +47,28 @@ const matchPostShallowIncludes = { post_revisions: anyArray }; +async function trackDb(fn, skip) { + const db = require('../../../core/server/data/db'); + if (db?.knex?.client?.config?.client !== 'sqlite3') { + return skip(); + } + /** @type {import('sqlite3').Database} */ + const database = db.knex.client; + + const queries = []; + function handler(/** @type {{sql: string}} */ query) { + queries.push(query); + } + + database.on('query', handler); + + await fn(); + + database.off('query', handler); + + return queries; +} + describe('Collections API', function () { let agent; @@ -78,6 +100,26 @@ describe('Collections API', function () { }); }); + it('Makes limited DB queries when browsing', async function () { + const queries = await trackDb(async () => { + await agent + .get('/collections/') + .expectStatus(200) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }) + .matchBodySnapshot({ + collections: [ + matchCollection, + matchCollection + ] + }); + }, this.skip.bind(this)); + const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection')); + assert(collectionRelatedQueries.length === 2); + }); + it('Can browse Collections and include the posts count', async function () { await agent .get('/collections/?include=count.posts') @@ -491,6 +533,124 @@ describe('Collections API', function () { }); describe('Collection Posts updates automatically', function () { + it('Makes limited DB queries when updating due to post changes', async function () { + await agent + .get(`/collections/slug/featured/?include=count.posts`) + .expectStatus(200) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }) + .matchBodySnapshot({ + collections: [{ + ...matchCollection, + count: { + posts: 2 + } + }] + }); + + const postToAdd = { + title: 'Collection update test', + featured: false + }; + + let post; + + { + const queries = await trackDb(async () => { + const {body: {posts: [createdPost]}} = await agent + .post('/posts/') + .body({ + posts: [postToAdd] + }) + .expectStatus(201); + + await DomainEvents.allSettled(); + + post = createdPost; + }, this.skip.bind(this)); + + const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection')); + assert.equal(collectionRelatedQueries.length, 8); + } + + await agent + .get(`/collections/slug/featured/?include=count.posts`) + .expectStatus(200) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }) + .matchBodySnapshot({ + collections: [{ + ...matchCollection, + count: { + posts: 2 + } + }] + }); + + { + const queries = await trackDb(async () => { + await agent + .put(`/posts/${post.id}/`) + .body({ + posts: [Object.assign({}, post, {featured: true})] + }) + .expectStatus(200); + + await DomainEvents.allSettled(); + }, this.skip.bind(this)); + + const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection')); + assert.equal(collectionRelatedQueries.length, 14); + } + + await agent + .get(`/collections/slug/featured/?include=count.posts`) + .expectStatus(200) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }) + .matchBodySnapshot({ + collections: [{ + ...matchCollection, + count: { + posts: 3 + } + }] + }); + + { + const queries = await trackDb(async () => { + await agent + .delete(`/posts/${post.id}/`) + .expectStatus(204); + + await DomainEvents.allSettled(); + }, this.skip.bind(this)); + const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection')); + assert.equal(collectionRelatedQueries.length, 2); + } + + await agent + .get(`/collections/slug/featured/?include=count.posts`) + .expectStatus(200) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }) + .matchBodySnapshot({ + collections: [{ + ...matchCollection, + count: { + posts: 2 + } + }] + }); + }); it('Updates collections when a Post is added/edited/deleted', async function () { await agent .get(`/collections/slug/featured/?include=count.posts`)