From a800d9d45ecf94ea58fa783be7dac464986e8fef Mon Sep 17 00:00:00 2001 From: Lennart Date: Mon, 18 Oct 2021 17:28:12 +0200 Subject: [PATCH] fix(gatsby): Update internal usage of .runQuery (#33571) * initial * update tests * update * try fix * accept GatsbyIterable in materialization Co-authored-by: Vladimir Razuvaev --- .../src/query/__tests__/data-tracking.js | 11 +- .../src/schema/__tests__/kitchen-sink.js | 7 +- .../gatsby/src/schema/__tests__/node-model.js | 189 ++++++++---------- .../gatsby/src/schema/__tests__/queries.js | 20 +- packages/gatsby/src/schema/node-model.js | 4 +- packages/gatsby/src/schema/resolvers.ts | 45 +++-- 6 files changed, 126 insertions(+), 150 deletions(-) diff --git a/packages/gatsby/src/query/__tests__/data-tracking.js b/packages/gatsby/src/query/__tests__/data-tracking.js index 696a77fbcd884..ccf6de2c16f8e 100644 --- a/packages/gatsby/src/query/__tests__/data-tracking.js +++ b/packages/gatsby/src/query/__tests__/data-tracking.js @@ -1291,9 +1291,8 @@ describe(`query caching between builds`, () => { createSchemaCustomization: ({ actions: { createTypes }, schema }) => { // Define fields with custom resolvers first const resolveOne = type => (value, args, context) => - context.nodeModel.runQuery({ + context.nodeModel.findOne({ query: { testId: { eq: value.id } }, - firstOnly: true, type, }) @@ -1321,11 +1320,13 @@ describe(`query caching between builds`, () => { }, fooList: { type: [`Foo`], - resolve: (value, args, context) => - context.nodeModel.runQuery({ + resolve: async (value, args, context) => { + const { entries } = await context.nodeModel.findAll({ query: { testId: { eq: value.id } }, type: `Foo`, - }), + }) + return entries + }, }, barList: { type: [`Bar`], diff --git a/packages/gatsby/src/schema/__tests__/kitchen-sink.js b/packages/gatsby/src/schema/__tests__/kitchen-sink.js index f84629b961b9e..d8629ed370b2c 100644 --- a/packages/gatsby/src/schema/__tests__/kitchen-sink.js +++ b/packages/gatsby/src/schema/__tests__/kitchen-sink.js @@ -342,9 +342,11 @@ const mockCreateResolvers = ({ createResolvers }) => { likedEnough: { type: `[PostsJson]`, async resolve(parent, args, context) { - const result = await context.nodeModel.runQuery({ + const { entries } = await context.nodeModel.findAll({ type: `PostsJson`, query: { + limit: 2, + skip: 0, filter: { likes: { ne: null, @@ -356,9 +358,8 @@ const mockCreateResolvers = ({ createResolvers }) => { order: [`DESC`], }, }, - firstOnly: false, }) - return result.slice(0, 2) + return entries }, }, }, diff --git a/packages/gatsby/src/schema/__tests__/node-model.js b/packages/gatsby/src/schema/__tests__/node-model.js index b823600af3713..27846ba8462db 100644 --- a/packages/gatsby/src/schema/__tests__/node-model.js +++ b/packages/gatsby/src/schema/__tests__/node-model.js @@ -330,11 +330,9 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = true nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const result = await nodeModel.findOne({ query, - firstOnly, type, }) expect(result.id).toBe(`post1`) @@ -345,14 +343,14 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const { entries, totalCount } = await nodeModel.findAll({ query, - firstOnly, type, }) - expect(result.length).toBe(2) + const result = Array.from(entries) + const count = await totalCount() + expect(count).toBe(2) expect(result[0].id).toBe(`post1`) expect(result[1].id).toBe(`post3`) }) @@ -362,12 +360,10 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query, - firstOnly, type, }, { path: `/` } @@ -384,11 +380,9 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - await nodeModel.withContext({ path: `/` }).runQuery({ + await nodeModel.withContext({ path: `/` }).findAll({ query, - firstOnly, type, }) expect(createPageDependency).toHaveBeenCalledTimes(1) @@ -403,12 +397,10 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query, - firstOnly, type, }, { path: `/`, connectionType: `Post` } @@ -425,12 +417,10 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query, - firstOnly, type, }, { path: `/`, connectionType: null } @@ -451,12 +441,10 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query, - firstOnly, type, }, { path: `/`, track: false } @@ -469,12 +457,10 @@ describe(`NodeModel`, () => { const query = { filter: { frontmatter: { published: { eq: false } } }, } - const firstOnly = false nodeModel.replaceFiltersCache() - await nodeModel.withContext({ path: `/` }).runQuery( + await nodeModel.withContext({ path: `/` }).findAll( { query, - firstOnly, type, }, { track: false } @@ -485,11 +471,9 @@ describe(`NodeModel`, () => { it(`doesn't allow querying union types`, () => { const type = `AllFiles` const query = {} - const firstOnly = true nodeModel.replaceFiltersCache() - const result = nodeModel.runQuery({ + const result = nodeModel.findOne({ query, - firstOnly, type, }) return expect(result).rejects.toThrowError( @@ -500,11 +484,9 @@ describe(`NodeModel`, () => { it(`handles interface types`, async () => { const type = `TeamMember` const query = { name: { ne: null } } - const firstOnly = true nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const result = await nodeModel.findOne({ query, - firstOnly, type, }) expect(result.name).toBe(`Person1`) @@ -517,14 +499,14 @@ describe(`NodeModel`, () => { children: { elemMatch: { internal: { type: { eq: `Post` } } } }, }, } - const firstOnly = false nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const { entries, totalCount } = await nodeModel.findAll({ query, - firstOnly, type, }) - expect(result.length).toBe(2) + const result = Array.from(entries) + const count = await totalCount() + expect(count).toBe(2) expect(result[0].id).toBe(`file1`) expect(result[1].id).toBe(`file3`) }) @@ -536,11 +518,9 @@ describe(`NodeModel`, () => { nestedObject: { elemMatch: { nestedValue: { eq: `2` } } }, }, } - const firstOnly = true nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const result = await nodeModel.findOne({ query, - firstOnly, type, }) expect(result).toBeDefined() @@ -561,15 +541,15 @@ describe(`NodeModel`, () => { }, }, } - const firstOnly = false nodeModel.replaceTypeKeyValueCache() - const result = await nodeModel.runQuery({ + const { entries, totalCount } = await nodeModel.findAll({ query, - firstOnly, type, }) + const result = Array.from(entries) + const count = await totalCount() expect(result).toBeDefined() - expect(result.length).toEqual(2) + expect(count).toEqual(2) expect(result[0].id).toEqual(`post2`) expect(result[1].id).toEqual(`post3`) }) @@ -873,10 +853,9 @@ describe(`NodeModel`, () => { it(`should not resolve prepared nodes more than once`, async () => { nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { betterTitle: { eq: `foo` } } }, - firstOnly: false, type: `Test`, }, { path: `/` } @@ -884,10 +863,9 @@ describe(`NodeModel`, () => { expect(resolveBetterTitleMock.mock.calls.length).toBe(2) expect(resolveOtherTitleMock.mock.calls.length).toBe(0) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { betterTitle: { eq: `foo` } } }, - firstOnly: false, type: `Test`, }, { path: `/` } @@ -895,12 +873,11 @@ describe(`NodeModel`, () => { expect(resolveBetterTitleMock.mock.calls.length).toBe(2) expect(resolveOtherTitleMock.mock.calls.length).toBe(0) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { betterTitle: { eq: `foo` }, otherTitle: { eq: `Bar` } }, }, - firstOnly: false, type: `Test`, }, { path: `/` } @@ -908,12 +885,11 @@ describe(`NodeModel`, () => { expect(resolveBetterTitleMock.mock.calls.length).toBe(2) expect(resolveOtherTitleMock.mock.calls.length).toBe(2) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { betterTitle: { eq: `foo` }, otherTitle: { eq: `Bar` } }, }, - firstOnly: false, type: `Test`, }, { path: `/` } @@ -921,12 +897,11 @@ describe(`NodeModel`, () => { expect(resolveBetterTitleMock.mock.calls.length).toBe(2) expect(resolveOtherTitleMock.mock.calls.length).toBe(2) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findOne( { query: { filter: { betterTitle: { eq: `foo` }, otherTitle: { eq: `Bar` } }, }, - firstOnly: true, type: `Test`, }, { path: `/` } @@ -937,10 +912,9 @@ describe(`NodeModel`, () => { it(`should not resolve prepared nodes more than once (with mixed interfaces and node types)`, async () => { nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { slug: { eq: `id1` } } }, - firstOnly: false, type: `Test`, }, { path: `/` } @@ -948,10 +922,9 @@ describe(`NodeModel`, () => { expect(resolveSlugMock.mock.calls.length).toBe(2) expect(resolveBetterTitleMock.mock.calls.length).toBe(0) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { slug: { eq: `id1` } } }, - firstOnly: false, type: `TestInterface`, }, { path: `/` } @@ -959,12 +932,11 @@ describe(`NodeModel`, () => { expect(resolveSlugMock.mock.calls.length).toBe(2) expect(resolveBetterTitleMock.mock.calls.length).toBe(0) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { slug: { eq: `id1` }, betterTitle: { eq: `foo` } }, }, - firstOnly: false, type: `Test`, }, { path: `/` } @@ -972,12 +944,11 @@ describe(`NodeModel`, () => { expect(resolveSlugMock.mock.calls.length).toBe(2) expect(resolveBetterTitleMock.mock.calls.length).toBe(2) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findAll( { query: { filter: { slug: { eq: `id1` } }, }, - firstOnly: false, type: `TestInterface`, }, { path: `/` } @@ -985,12 +956,11 @@ describe(`NodeModel`, () => { expect(resolveSlugMock.mock.calls.length).toBe(2) expect(resolveBetterTitleMock.mock.calls.length).toBe(2) nodeModel.replaceFiltersCache() - await nodeModel.runQuery( + await nodeModel.findOne( { query: { filter: { slug: { eq: `id1` }, betterTitle: { eq: `foo` } }, }, - firstOnly: true, type: `Test`, }, { path: `/` } @@ -1001,17 +971,18 @@ describe(`NodeModel`, () => { it(`can filter by resolved fields`, async () => { nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery( + const { entries, totalCount } = await nodeModel.findAll( { query: { filter: { hidden: { eq: false } }, }, - firstOnly: false, type: `Test`, }, { path: `/` } ) - expect(result.length).toBe(2) + const result = Array.from(entries) + const count = await totalCount() + expect(count).toBe(2) expect(result[0].id).toBe(`id1`) expect(result[1].id).toBe(`id2`) }) @@ -1019,40 +990,44 @@ describe(`NodeModel`, () => { it(`merges query caches when filtering by nested field`, async () => { // See https://github.com/gatsbyjs/gatsby/issues/26056 nodeModel.replaceFiltersCache() - const result1 = await nodeModel.runQuery( - { - query: { - filter: { nested: { foo: { eq: `foo1` } } }, + const { entries: entries1, totalCount: totalCount1 } = + await nodeModel.findAll( + { + query: { + filter: { nested: { foo: { eq: `foo1` } } }, + }, + type: `Test`, }, - firstOnly: false, - type: `Test`, - }, - { path: `/` } - ) - const result2 = await nodeModel.runQuery( - { - query: { - filter: { nested: { bar: { eq: `bar2` } } }, + { path: `/` } + ) + const { entries: entries2, totalCount: totalCount2 } = + await nodeModel.findAll( + { + query: { + filter: { nested: { bar: { eq: `bar2` } } }, + }, + type: `Test`, }, - firstOnly: false, - type: `Test`, - }, - { path: `/` } - ) + { path: `/` } + ) + const result1 = Array.from(entries1) + const result2 = Array.from(entries2) + const count1 = await totalCount1() + const count2 = await totalCount2() expect(result1).toBeTruthy() - expect(result1.length).toBe(1) + expect(count1).toBe(1) expect(result1[0].id).toBe(`id1`) expect(result2).toBeTruthy() - expect(result2.length).toBe(1) + expect(count2).toBe(1) expect(result2[0].id).toBe(`id2`) }) it(`always uses a custom resolvers for query fields`, async () => { // See https://github.com/gatsbyjs/gatsby/issues/27368 nodeModel.replaceFiltersCache() - const result1 = await nodeModel.runQuery( + const { entries: entries1 } = await nodeModel.findAll( { query: { sort: { @@ -1060,12 +1035,11 @@ describe(`NodeModel`, () => { order: [`desc`], }, }, - firstOnly: false, type: `Test4`, }, { path: `/` } ) - const result2 = await nodeModel.runQuery( + const { entries: entries2 } = await nodeModel.findAll( { query: { filter: { Meta: { Category: { eq: `Gatsby` } } }, @@ -1074,16 +1048,18 @@ describe(`NodeModel`, () => { order: [`desc`], }, }, - firstOnly: false, type: `Test4`, }, { path: `/` } ) - expect(Array.isArray(result1)).toBeTruthy() + const result1 = Array.from(entries1) + const result2 = Array.from(entries2) + + expect(result1).toBeTruthy() expect(result1.map(node => node.id)).toEqual([`id4`, `id5`]) - expect(Array.isArray(result2)).toBeTruthy() + expect(result2).toBeTruthy() expect(result2.map(node => node.id)).toEqual([`id4`, `id5`]) }) @@ -1092,19 +1068,18 @@ describe(`NodeModel`, () => { nodeModel.replaceFiltersCache() // This is required to setup a state after which the error reveals itself - const result1 = await nodeModel.runQuery( + const result1 = await nodeModel.findOne( { query: { filter: { id: { regex: `/non-existing/` } }, }, - firstOnly: true, type: `Test5`, }, { path: `/` } ) // Filter by the same regex with sorting - const result2 = await nodeModel.runQuery( + const { entries } = await nodeModel.findAll( { query: { filter: { id: { regex: `/id/` } }, @@ -1113,11 +1088,11 @@ describe(`NodeModel`, () => { order: [`desc`], }, }, - firstOnly: false, type: `Test5`, }, { path: `/` } ) + const result2 = Array.from(entries) expect(result1).toEqual(null) @@ -1127,29 +1102,29 @@ describe(`NodeModel`, () => { it(`handles nulish values within array of interface type`, async () => { nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery( + const { entries, totalCount } = await nodeModel.findAll( { query: { filter: { arrayWithNulls: { elemMatch: { foo: { eq: `id1` } } } }, }, - firstOnly: false, type: `Test`, }, { path: `/` } ) + const result = Array.from(entries) + const count = await totalCount() expect(result).toBeTruthy() - expect(result.length).toEqual(1) + expect(count).toEqual(1) expect(result[0].id).toEqual(`id1`) }) it(`handles fields with custom resolvers on interfaces having multiple implementations`, async () => { nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery( + const result = await nodeModel.findOne( { query: { filter: { slug: { eq: `id3` } }, }, - firstOnly: true, type: `TestInterface`, }, { path: `/` } @@ -1285,13 +1260,14 @@ describe(`NodeModel`, () => { describe(`Tracks nodes returned by queries`, () => { it(`Tracks objects when running query without filter`, async () => { nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const { entries, totalCount } = await nodeModel.findAll({ query: {}, type: schema.getType(`Test`), - firstOnly: false, }) + const result = Array.from(entries) + const count = await totalCount() - expect(result.length).toEqual(2) + expect(count).toEqual(2) expect(nodeModel.findRootNodeAncestor(result[0].inlineObject)).toEqual( result[0] ) @@ -1302,7 +1278,7 @@ describe(`NodeModel`, () => { it(`Tracks objects when running query with filter`, async () => { nodeModel.replaceFiltersCache() - const result = await nodeModel.runQuery({ + const { entries, totalCount } = await nodeModel.findAll({ query: { filter: { inlineObject: { @@ -1313,10 +1289,11 @@ describe(`NodeModel`, () => { }, }, type: schema.getType(`Test`), - firstOnly: false, }) + const result = Array.from(entries) + const count = await totalCount() - expect(result.length).toEqual(1) + expect(count).toEqual(1) expect(nodeModel.findRootNodeAncestor(result[0].inlineObject)).toEqual( result[0] ) diff --git a/packages/gatsby/src/schema/__tests__/queries.js b/packages/gatsby/src/schema/__tests__/queries.js index 491311f584e75..5769f52172312 100644 --- a/packages/gatsby/src/schema/__tests__/queries.js +++ b/packages/gatsby/src/schema/__tests__/queries.js @@ -57,12 +57,11 @@ describe(`Query schema`, () => { [`frontmatter.authorNames`]: { type: `[String!]!`, async resolve(source, args, context, info) { - const authors = await context.nodeModel.runQuery({ + const { entries } = await context.nodeModel.findAll({ type: `Author`, query: { filter: { email: { in: source.authors } } }, - firstOnly: false, }) - return authors.map(author => author.name) + return entries.map(author => author.name) }, }, [`frontmatter.anotherField`]: { @@ -103,17 +102,8 @@ describe(`Query schema`, () => { }, Author: { posts: { - resolve(source, args, context, info) { - // NOTE: One of the differences between using `runQuery` and - // `getAllNodes` is that the latter will always get the nodes - // which will be queried directly from the store, while `runQuery` - // will first try to call field resolvers, e.g. to expand - // foreign-key fields to full nodes. Here for example we can - // query `authors.email`. - // Another thing to note is that we don't have to use the - // `$elemMatch` operator when querying arrays of objects - // (although we could). - return context.nodeModel.runQuery({ + async resolve(source, args, context, info) { + const { entries } = await context.nodeModel.findAll({ type: `Markdown`, query: { filter: { @@ -125,8 +115,8 @@ describe(`Query schema`, () => { }, }, }, - firstOnly: false, }) + return entries }, }, }, diff --git a/packages/gatsby/src/schema/node-model.js b/packages/gatsby/src/schema/node-model.js index 1a22f3a9e6d91..0be271fcac15f 100644 --- a/packages/gatsby/src/schema/node-model.js +++ b/packages/gatsby/src/schema/node-model.js @@ -20,7 +20,7 @@ import { getNodesByType, getTypes, } from "../datastore" -import { isIterable } from "../datastore/common/iterable" +import { GatsbyIterable, isIterable } from "../datastore/common/iterable" import { reportOnce } from "../utils/report-once" type TypeOrTypeName = string | GraphQLOutputType @@ -784,7 +784,7 @@ async function resolveRecursive( ) } else if ( isCompositeType(gqlFieldType) && - _.isArray(innerValue) && + (_.isArray(innerValue) || innerValue instanceof GatsbyIterable) && gqlNonNullType instanceof GraphQLList ) { innerValue = await Promise.all( diff --git a/packages/gatsby/src/schema/resolvers.ts b/packages/gatsby/src/schema/resolvers.ts index 0fae66f20a28a..89b0c1c6f62cc 100644 --- a/packages/gatsby/src/schema/resolvers.ts +++ b/packages/gatsby/src/schema/resolvers.ts @@ -403,25 +403,33 @@ export function link( } } - const resultOrPromise = context.nodeModel.runQuery( - { - query: runQueryArgs, - firstOnly, - type, - stats: context.stats, - tracer: context.tracer, - }, - { path: context.path } - ) - - // Note: for this function, at scale, conditional .then is more efficient than generic await - if (typeof resultOrPromise?.then === `function`) { - return resultOrPromise.then(result => - linkResolverQueryResult(fieldValue, result, returnType) - ) + if (firstOnly) { + return context.nodeModel + .findOne( + { + query: runQueryArgs, + type, + stats: context.stats, + tracer: context.tracer, + }, + { path: context.path } + ) + .then(result => linkResolverQueryResult(fieldValue, result, returnType)) } - return linkResolverQueryResult(fieldValue, resultOrPromise, returnType) + return context.nodeModel + .findAll( + { + query: runQueryArgs, + type, + stats: context.stats, + tracer: context.tracer, + }, + { path: context.path } + ) + .then(({ entries }) => + linkResolverQueryResult(fieldValue, Array.from(entries), returnType) + ) } function linkResolverQueryResult( @@ -492,7 +500,7 @@ export function fileByPath( } function queryNodeByPath(relPath: string): Promise { - return context.nodeModel.runQuery({ + return context.nodeModel.findOne({ query: { filter: { absolutePath: { @@ -500,7 +508,6 @@ export function fileByPath( }, }, }, - firstOnly: true, type: `File`, }) }