From 60bc7561b509d3fce62b130ff8f543c828bd2c37 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Sat, 29 Feb 2020 17:06:52 -0800 Subject: [PATCH] mirror: request typenames in GraphQL queries (#1666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: The database now stores objects without typenames, so we can emit requests for those typenames in our GraphQL queries. Test Plan: Unit tests added; they’re lighter-weight than their siblings only because querying typenames is intrinsically simpler than querying own data or connections (in particular, the typenames query is a constant). wchargin-branch: mirror-typename-queryfromplan --- src/graphql/mirror.js | 68 ++++++++++++++++++++++++++++++++------ src/graphql/mirror.test.js | 48 +++++++++++++++++---------- 2 files changed, 87 insertions(+), 29 deletions(-) diff --git a/src/graphql/mirror.js b/src/graphql/mirror.js index fd6b617..4cf0853 100644 --- a/src/graphql/mirror.js +++ b/src/graphql/mirror.js @@ -654,6 +654,9 @@ export class Mirror { _queryFromPlan( queryPlan: QueryPlan, options: {| + // When fetching typenames for nodes originally referenced by + // unfaithful fields, how many typenames may we fetch at once? + +typenamesLimit: number, // When fetching own-data for nodes of a given type, how many // nodes may we fetch at once? +nodesOfTypeLimit: number, @@ -667,9 +670,21 @@ export class Mirror { +connectionPageSize: number, |} ): Queries.Selection[] { - if (queryPlan.typenames.length > 0) { - throw new Error("Typename queries not yet supported"); + const fetchTypenamesFor = queryPlan.typenames.slice( + 0, + options.typenamesLimit + ); + const typenameBatches = []; + for ( + let i = 0; + i < fetchTypenamesFor.length; + i += options.nodesOfTypeLimit + ) { + typenameBatches.push( + fetchTypenamesFor.slice(i, i + options.nodesOfTypeLimit) + ); } + // Group objects by type, so that we have to specify each type's // fieldset fewer times (only once per `nodesOfTypeLimit` nodes // instead of for every node). @@ -726,16 +741,30 @@ export class Mirror { const b = Queries.build; - // Each top-level field corresponds to either an object type - // (fetching own data for objects of that type) or a particular node - // (updating connections on that node). We alias each such field, - // which is necessary to ensure that their names are all unique. The - // names chosen are sufficient to identify which _kind_ of query the - // field corresponds to (type's own data vs node's connections), but - // do not need to identify the particular type or node in question. - // This is because all descendant selections are self-describing: - // they include the ID of any relevant objects. + // Each top-level field other than `typenames` corresponds to either + // an object type (fetching own data for objects of that type) or a + // particular node (updating connections on that node). We alias + // each such field, which is necessary to ensure that their names + // are all unique. The names chosen are sufficient to identify which + // _kind_ of query the field corresponds to (type's own data vs + // node's connections), but do not need to identify the particular + // type or node in question. This is because all descendant + // selections are self-describing: they include the ID of any + // relevant objects. return [].concat( + typenameBatches.map((typenames, i) => { + const name = `${_FIELD_PREFIXES.TYPENAMES}${i}`; + return b.alias( + name, + b.field( + "nodes", + { + ids: b.list(typenames.map((id) => b.literal(id))), + }, + this._queryTypename() + ) + ); + }), paginatedObjectsByType.map(({typename, ids}, i) => { const name = `${_FIELD_PREFIXES.OWN_DATA}${i}`; return b.alias( @@ -917,6 +946,9 @@ export class Mirror { return Promise.resolve(false); } const querySelections = this._queryFromPlan(queryPlan, { + // TODO(@wchargin): Expose `typenamesLimit` as an option and fix + // up callers. + typenamesLimit: options.nodesLimit, nodesLimit: options.nodesLimit, nodesOfTypeLimit: options.nodesOfTypeLimit, connectionPageSize: options.connectionPageSize, @@ -1634,6 +1666,20 @@ export class Mirror { // Last-updates, primitives, and links all updated: we're done. } + /** + * Create a GraphQL selection set required to fetch the typename of an + * object. The resulting GraphQL can be embedded in any node context. + * + * The result of this query has type `E`, where `E` is the element + * type of `TypenamesUpdateResult`. + * + * This function is pure: it does not interact with the database. + */ + _queryTypename(): Queries.Selection[] { + const b = Queries.build; + return [b.field("__typename"), b.field("id")]; + } + /** * Extract a structured object and all of its transitive dependencies * from the database. diff --git a/src/graphql/mirror.test.js b/src/graphql/mirror.test.js index cdfbc2c..786dd0b 100644 --- a/src/graphql/mirror.test.js +++ b/src/graphql/mirror.test.js @@ -708,6 +708,7 @@ describe("graphql/mirror", () => { }; expect(() => { mirror._queryFromPlan(plan, { + typenamesLimit: 10, nodesLimit: 10, nodesOfTypeLimit: 5, connectionLimit: 5, @@ -718,28 +719,11 @@ describe("graphql/mirror", () => { '"Issue" vs. "Repository"' ); }); - it("errors if given any typename requests", () => { - const db = new Database(":memory:"); - const mirror = new Mirror(db, buildGithubSchema()); - const plan = { - typenames: ["hmmm"], - objects: [], - connections: [], - }; - expect(() => { - mirror._queryFromPlan(plan, { - nodesLimit: 10, - nodesOfTypeLimit: 5, - connectionLimit: 5, - connectionPageSize: 23, - }); - }).toThrow("Typename queries not yet supported"); - }); it("creates a good query", () => { const db = new Database(":memory:"); const mirror = new Mirror(db, buildGithubSchema()); const plan = { - typenames: [], + typenames: ["hmm#1", "hmm#2", "hmm#3", "hmm#4", "hmm#5"], objects: [ {typename: "Issue", id: "i#1"}, {typename: "Repository", id: "repo#2"}, @@ -795,6 +779,7 @@ describe("graphql/mirror", () => { ], }; const actual = mirror._queryFromPlan(plan, { + typenamesLimit: 3, nodesLimit: 4, nodesOfTypeLimit: 2, connectionLimit: 5, @@ -802,6 +787,21 @@ describe("graphql/mirror", () => { }); const b = Queries.build; expect(actual).toEqual([ + b.alias( + "typenames_0", + b.field( + "nodes", + {ids: b.list([b.literal("hmm#1"), b.literal("hmm#2")])}, + [b.field("__typename"), b.field("id")] + ) + ), + b.alias( + "typenames_1", + b.field("nodes", {ids: b.list([b.literal("hmm#3")])}, [ + b.field("__typename"), + b.field("id"), + ]) + ), b.alias( "owndata_0", b.field( @@ -1261,6 +1261,7 @@ describe("graphql/mirror", () => { expect(spyQueryFromPlan.mock.calls[0]).toEqual([ spyFindOutdated.mock.results[0].value, { + typenamesLimit: 3, nodesOfTypeLimit: 2, nodesLimit: 3, connectionLimit: 4, @@ -1270,6 +1271,7 @@ describe("graphql/mirror", () => { expect(spyQueryFromPlan.mock.calls[1]).toEqual([ spyFindOutdated.mock.results[1].value, { + typenamesLimit: 3, nodesOfTypeLimit: 2, nodesLimit: 3, connectionLimit: 4, @@ -2584,6 +2586,16 @@ describe("graphql/mirror", () => { }); }); + describe("_queryTypename", () => { + it("generates a query with ID and typename", () => { + const db = new Database(":memory:"); + const mirror = new Mirror(db, buildGithubSchema()); + const query = mirror._queryTypename(); + const b = Queries.build; + expect(query).toEqual([b.field("__typename"), b.field("id")]); + }); + }); + describe("extract", () => { // A schema with some useful edge cases. function buildTestSchema(): Schema.Schema {