mirror: add support for connections of union types (#882)

Summary:
Almost every GitHub connection has nodes of an object type, like `User`
or `IssueComment`. But a few have nodes of union type, including
`IssueTimelineItemConnection` (which we will likely want to query), and
those require special handling. This commit adds susupport for such
connections.

Analysis to determine which connections have non-object elements:
<https://gist.github.com/wchargin/647fa7ed8d6d17ae2e204bd098104407>

Test Plan:
Unit tests modified appropriately, retaining full coverage.

The easiest way to verify the snapshot is probably to copy the raw
contents (everything inside the quotes) into `/tmp/snapshot`, then run:

```shell
$ sed -e 's/\\//g' </tmp/snapshot >/tmp/query  # Jest adds backslashes
$ jq -csR '{query: ., variables: {}}' </tmp/query >/tmp/payload
$ ENDPOINT='https://api.github.com/graphql'
$ AUTH="Authorization: bearer ${SOURCECRED_GITHUB_TOKEN}"
$ curl "$ENDPOINT" -X POST -H "$AUTH" -d @- </tmp/payload >/tmp/result
```

and then execute the JQ program mentioned in the comment in the test
case, and verify that it prints `true`.

wchargin-branch: connection-of-union
This commit is contained in:
William Chargin 2018-09-21 16:20:58 -07:00 committed by GitHub
parent 23ae7e2f08
commit 838092194b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 518 additions and 76 deletions

View File

@ -2,7 +2,7 @@
exports[`graphql/mirror Mirror _queryConnection snapshot test for actual GitHub queries 1`] = `
"query TestQuery {
initialQuery: node(id: \\"MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=\\") {
objectInitial: node(id: \\"MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=\\") {
... on Repository {
issues(first: 2) {
totalCount
@ -17,7 +17,7 @@ exports[`graphql/mirror Mirror _queryConnection snapshot test for actual GitHub
}
}
}
updateQuery: node(id: \\"MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=\\") {
objectUpdate: node(id: \\"MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=\\") {
... on Repository {
issues(first: 2 after: \\"Y3Vyc29yOnYyOpHOEe_nRA==\\") {
totalCount
@ -32,7 +32,7 @@ exports[`graphql/mirror Mirror _queryConnection snapshot test for actual GitHub
}
}
}
expectedIds: node(id: \\"MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=\\") {
objectExpectedIds: node(id: \\"MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=\\") {
... on Repository {
issues(first: 4) {
nodes {
@ -41,5 +41,194 @@ exports[`graphql/mirror Mirror _queryConnection snapshot test for actual GitHub
}
}
}
unionInitial: node(id: \\"MDU6SXNzdWUzMDA5MzQ4MTg=\\") {
... on Issue {
timeline(first: 1) {
totalCount
pageInfo {
endCursor
hasNextPage
}
nodes {
__typename
... on Commit {
id
}
... on IssueComment {
id
}
... on CrossReferencedEvent {
id
}
... on ClosedEvent {
id
}
... on ReopenedEvent {
id
}
... on SubscribedEvent {
id
}
... on UnsubscribedEvent {
id
}
... on ReferencedEvent {
id
}
... on AssignedEvent {
id
}
... on UnassignedEvent {
id
}
... on LabeledEvent {
id
}
... on UnlabeledEvent {
id
}
... on MilestonedEvent {
id
}
... on DemilestonedEvent {
id
}
... on RenamedTitleEvent {
id
}
... on LockedEvent {
id
}
... on UnlockedEvent {
id
}
}
}
}
}
unionUpdate: node(id: \\"MDU6SXNzdWUzMDA5MzQ4MTg=\\") {
... on Issue {
timeline(first: 1 after: \\"MQ==\\") {
totalCount
pageInfo {
endCursor
hasNextPage
}
nodes {
__typename
... on Commit {
id
}
... on IssueComment {
id
}
... on CrossReferencedEvent {
id
}
... on ClosedEvent {
id
}
... on ReopenedEvent {
id
}
... on SubscribedEvent {
id
}
... on UnsubscribedEvent {
id
}
... on ReferencedEvent {
id
}
... on AssignedEvent {
id
}
... on UnassignedEvent {
id
}
... on LabeledEvent {
id
}
... on UnlabeledEvent {
id
}
... on MilestonedEvent {
id
}
... on DemilestonedEvent {
id
}
... on RenamedTitleEvent {
id
}
... on LockedEvent {
id
}
... on UnlockedEvent {
id
}
}
}
}
}
unionExpectedIds: node(id: \\"MDU6SXNzdWUzMDA5MzQ4MTg=\\") {
... on Issue {
timeline(first: 2) {
nodes {
... on Commit {
id
}
... on IssueComment {
id
}
... on CrossReferencedEvent {
id
}
... on ClosedEvent {
id
}
... on ReopenedEvent {
id
}
... on SubscribedEvent {
id
}
... on UnsubscribedEvent {
id
}
... on ReferencedEvent {
id
}
... on AssignedEvent {
id
}
... on UnassignedEvent {
id
}
... on LabeledEvent {
id
}
... on UnlabeledEvent {
id
}
... on MilestonedEvent {
id
}
... on DemilestonedEvent {
id
}
... on RenamedTitleEvent {
id
}
... on LockedEvent {
id
}
... on UnlockedEvent {
id
}
}
}
}
}
}"
`;

View File

@ -416,15 +416,15 @@ export class Mirror {
/**
* Create a GraphQL selection set required to identify the typename
* and ID for an object. This is the minimal information required to
* register an object in our database, so we query this information
* and ID for an object of the given declared type, which may be
* either an object type or a union type. This is the minimal
* whenever we find a reference to an object that we want to traverse
* later.
*
* The resulting GraphQL should be embedded in any node context. For
* instance, it might replace the `?` in any of the following queries:
*
* repository(owner: "foo", name: "bar") { ? }
* The resulting GraphQL should be embedded in the context of any node
* of the provided type. For instance, `_queryShallow("Issue")`
* returns a selection set that might replace the `?` in any of the
* following queries:
*
* repository(owner: "foo", name: "bar") {
* issues(first: 1) {
@ -432,13 +432,34 @@ export class Mirror {
* }
* }
*
* nodes(ids: ["baz", "quux"]) { ? }
* nodes(ids: ["issue#1", "issue#2"]) { ? }
*
* The result of this query has type `NodeFieldResult`.
*
* This function is pure: it does not interact with the database.
*/
_queryShallow(): Queries.Selection[] {
_queryShallow(typename: Schema.Typename): Queries.Selection[] {
const type = this._schema[typename];
if (type == null) {
// Should not be reachable via APIs.
throw new Error("No such type: " + JSON.stringify(typename));
}
const b = Queries.build;
switch (type.type) {
case "OBJECT":
return [b.field("__typename"), b.field("id")];
case "UNION":
return [
b.field("__typename"),
...this._schemaInfo.unionTypes[typename].clauses.map(
(clause: Schema.Typename) =>
b.inlineFragment(clause, [b.field("id")])
),
];
// istanbul ignore next
default:
throw new Error((type.type: empty));
}
}
/**
@ -478,7 +499,10 @@ export class Mirror {
}
/**
* Create a GraphQL selection set to fetch elements from a collection.
* Create a GraphQL selection set to fetch elements from a collection,
* specified by its enclosing object type and the connection field
* name (for instance, "Repository" and "issues").
*
* If the connection has been queried before and you wish to fetch new
* elements, use an appropriate end cursor. Use `undefined` otherwise.
* Note that `null` is a valid end cursor and is distinct from
@ -515,10 +539,35 @@ export class Mirror {
* See: `_updateConnection`.
*/
_queryConnection(
typename: Schema.Typename,
fieldname: Schema.Fieldname,
endCursor: EndCursor | void,
connectionPageSize: number
): Queries.Selection[] {
if (this._schema[typename] == null) {
throw new Error("No such type: " + JSON.stringify(typename));
}
if (this._schema[typename].type !== "OBJECT") {
const s = JSON.stringify;
throw new Error(
`Cannot query connection on non-object type ${s(typename)} ` +
`(${this._schema[typename].type})`
);
}
const field = this._schemaInfo.objectTypes[typename].fields[fieldname];
if (field == null) {
const s = JSON.stringify;
throw new Error(
`Object type ${s(typename)} has no field ${s(fieldname)}`
);
}
if (field.type !== "CONNECTION") {
const s = JSON.stringify;
throw new Error(
`Cannot query non-connection field ${s(typename)}.${s(fieldname)} ` +
`(${field.type})`
);
}
const b = Queries.build;
const connectionArguments: Queries.Arguments = {
first: b.literal(connectionPageSize),
@ -530,7 +579,7 @@ export class Mirror {
b.field(fieldname, connectionArguments, [
b.field("totalCount"),
b.field("pageInfo", {}, [b.field("endCursor"), b.field("hasNextPage")]),
b.field("nodes", {}, this._queryShallow()),
b.field("nodes", {}, this._queryShallow(field.elementType)),
]),
];
}

View File

@ -10,9 +10,30 @@ import * as Queries from "./queries";
import {_buildSchemaInfo, _inTransaction, Mirror} from "./mirror";
describe("graphql/mirror", () => {
function issueTimelineItemClauses() {
return [
"Commit",
"IssueComment",
"CrossReferencedEvent",
"ClosedEvent",
"ReopenedEvent",
"SubscribedEvent",
"UnsubscribedEvent",
"ReferencedEvent",
"AssignedEvent",
"UnassignedEvent",
"LabeledEvent",
"UnlabeledEvent",
"MilestonedEvent",
"DemilestonedEvent",
"RenamedTitleEvent",
"LockedEvent",
"UnlockedEvent",
];
}
function buildGithubSchema(): Schema.Schema {
const s = Schema;
return s.schema({
const types: {[Schema.Typename]: Schema.NodeType} = {
Repository: s.object({
id: s.id(),
url: s.primitive(),
@ -25,12 +46,14 @@ describe("graphql/mirror", () => {
repository: s.node("Repository"),
title: s.primitive(),
comments: s.connection("IssueComment"),
timeline: s.connection("IssueTimelineItem"),
}),
IssueComment: s.object({
id: s.id(),
body: s.primitive(),
author: s.node("Actor"),
}),
IssueTimelineItem: s.union(issueTimelineItemClauses()),
Actor: s.union(["User", "Bot", "Organization"]), // actually an interface
User: s.object({
id: s.id(),
@ -47,7 +70,13 @@ describe("graphql/mirror", () => {
url: s.primitive(),
login: s.primitive(),
}),
});
};
for (const clause of issueTimelineItemClauses()) {
if (types[clause] == null) {
types[clause] = s.object({id: s.id(), actor: s.node("Actor")});
}
}
return s.schema(types);
}
describe("Mirror", () => {
@ -79,7 +108,8 @@ describe("graphql/mirror", () => {
.pluck()
.all();
expect(tables.sort()).toEqual(
[
Array.from(
new Set([
// Structural tables
"meta",
"updates",
@ -94,7 +124,9 @@ describe("graphql/mirror", () => {
"primitives_User",
"primitives_Bot",
"primitives_Organization",
].sort()
...issueTimelineItemClauses().map((x) => `primitives_${x}`),
])
).sort()
);
});
@ -240,7 +272,7 @@ describe("graphql/mirror", () => {
)
.pluck()
.all(issueId)
).toEqual(["comments"]);
).toEqual(["comments", "timeline"].sort());
expect(
db
.prepare(
@ -445,6 +477,19 @@ describe("graphql/mirror", () => {
hasNextPage: +false,
endCursor: "cursor:issue4.comments",
});
for (const n of [1, 2, 3, 4]) {
// The "timeline" connection doesn't provide any extra useful
// info; just mark them all updated.
const objectId = `issue:ab/cd#${n}`;
setConnectionData({
objectId,
fieldname: "timeline",
update: lateUpdate.id,
totalCount: 0,
hasNextPage: +false,
endCursor: "cursor:whatever",
});
}
const actual = mirror._findOutdated(new Date(midUpdate.time));
const expected = {
@ -482,6 +527,36 @@ describe("graphql/mirror", () => {
});
});
describe("_queryShallow", () => {
it("fails when given a nonexistent type", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(() => {
mirror._queryShallow("Wat");
}).toThrow('No such type: "Wat"');
});
it("handles an object type", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
const b = Queries.build;
expect(mirror._queryShallow("Issue")).toEqual([
b.field("__typename"),
b.field("id"),
]);
});
it("handles a union type", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
const b = Queries.build;
expect(mirror._queryShallow("Actor")).toEqual([
b.field("__typename"),
b.inlineFragment("User", [b.field("id")]),
b.inlineFragment("Bot", [b.field("id")]),
b.inlineFragment("Organization", [b.field("id")]),
]);
});
});
describe("_getEndCursor", () => {
it("fails when the object does not exist", () => {
const db = new Database(":memory:");
@ -550,12 +625,47 @@ describe("graphql/mirror", () => {
});
describe("_queryConnection", () => {
it("fails when given a nonexistent type", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(() => {
mirror._queryConnection("Wat", "wot", undefined, 23);
}).toThrow('No such type: "Wat"');
});
it("fails when given a non-object type", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(() => {
mirror._queryConnection("Actor", "issues", undefined, 23);
}).toThrow(
'Cannot query connection on non-object type "Actor" (UNION)'
);
});
it("fails when given a nonexistent field name", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(() => {
mirror._queryConnection("Issue", "mcguffins", undefined, 23);
}).toThrow('Object type "Issue" has no field "mcguffins"');
});
it("fails when given a non-connection field name", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(() => {
mirror._queryConnection("Issue", "author", undefined, 23);
}).toThrow('Cannot query non-connection field "Issue"."author" (NODE)');
});
it("creates a query when no cursor is specified", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
const pageLimit = 23;
const endCursor = undefined;
const actual = mirror._queryConnection("comments", endCursor, 23);
const actual = mirror._queryConnection(
"Issue",
"comments",
endCursor,
23
);
const b = Queries.build;
expect(actual).toEqual([
b.field("comments", {first: b.literal(pageLimit)}, [
@ -573,7 +683,12 @@ describe("graphql/mirror", () => {
const mirror = new Mirror(db, buildGithubSchema());
const pageLimit = 23;
const endCursor = null;
const actual = mirror._queryConnection("comments", endCursor, 23);
const actual = mirror._queryConnection(
"Issue",
"comments",
endCursor,
23
);
const b = Queries.build;
expect(actual).toEqual([
b.field(
@ -595,7 +710,12 @@ describe("graphql/mirror", () => {
const mirror = new Mirror(db, buildGithubSchema());
const pageLimit = 23;
const endCursor = "c29tZS1jdXJzb3I=";
const actual = mirror._queryConnection("comments", endCursor, 23);
const actual = mirror._queryConnection(
"Issue",
"comments",
endCursor,
23
);
const b = Queries.build;
expect(actual).toEqual([
b.field(
@ -614,51 +734,64 @@ describe("graphql/mirror", () => {
});
it("snapshot test for actual GitHub queries", () => {
// This test emits as a snapshot a valid query against GitHub's
// GraphQL API. You can copy-and-paste the snapshot into
// <https://developer.github.com/v4/explorer/> to run it. The
// resulting IDs in `initialQuery` and `updateQuery` should
// concatenate to match those in `expectedIds`. In particular,
// the following JQ program should output `true` when passed the
// query result from GitHub:
// GraphQL API. You should be able to copy-and-paste the
// snapshot into <https://developer.github.com/v4/explorer/> to
// run it.* The resulting IDs in `initialQuery` and
// `updateQuery` should concatenate to match those in
// `expectedIds`. In particular, the following JQ program should
// output `true` when passed the query result from GitHub:
//
// jq '.data |
// ([.initialQuery, .updateQuery] | map(.issues.nodes[].id))
// == [.expectedIds.issues.nodes[].id]
// jq '
// .data |
// (([.objectInitial, .objectUpdate] | map(.issues.nodes[].id))
// == [.objectExpectedIds.issues.nodes[].id])
// and
// (([.unionInitial, .unionUpdate] | map(.timeline.nodes[].id))
// == [.unionExpectedIds.timeline.nodes[].id])
// '
//
// * This may not actually work, because the query text is
// somewhat large (a few kilobytes), and sometimes GitHub's
// GraphiQL explorer chokes on such endpoints. Posting directly
// to the input with curl(1) works. You could also temporarily
// change the `multilineLayout` to an `inlineLayout` to shave
// off some bytes and possibly appease the GraphiQL gods.
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
const b = Queries.build;
// Queries for a connection whose declared type is an object.
function objectConnectionQuery(): Queries.Selection[] {
const exampleGithubRepoId = "MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY=";
const pageLimit = 2;
const b = Queries.build;
const initialQuery = mirror._queryConnection(
"Repository",
"issues",
undefined,
pageLimit
);
const expectedEndCursor = "Y3Vyc29yOnYyOpHOEe_nRA==";
const updateQuery = mirror._queryConnection(
"Repository",
"issues",
expectedEndCursor,
pageLimit
);
const query = b.query(
"TestQuery",
[],
[
return [
b.alias(
"initialQuery",
"objectInitial",
b.field("node", {id: b.literal(exampleGithubRepoId)}, [
b.inlineFragment("Repository", initialQuery),
])
),
b.alias(
"updateQuery",
"objectUpdate",
b.field("node", {id: b.literal(exampleGithubRepoId)}, [
b.inlineFragment("Repository", updateQuery),
])
),
b.alias(
"expectedIds",
"objectExpectedIds",
b.field("node", {id: b.literal(exampleGithubRepoId)}, [
b.inlineFragment("Repository", [
b.field("issues", {first: b.literal(pageLimit * 2)}, [
@ -667,7 +800,73 @@ describe("graphql/mirror", () => {
]),
])
),
]
];
}
// Queries for a connection whose declared type is a union.
function unionConnectionQuery(): Queries.Selection[] {
// Almost all GitHub connections return OBJECTs for nodes, but
// a very few return UNIONs:
//
// - `IssueTimelineConnection`,
// - `PullRequestTimelineConnection`, and
// - `SearchResultItemConnection`.
//
// Of these, `SearchResultItemConnection` does not adhere to
// the same interface as the rest of the connections (it does
// not have a `totalCount` field), so it will not work with
// our system. But the two timeline connections are actually
// important---they let us see who assigns labels---so we test
// one of them.
const exampleIssueId = "MDU6SXNzdWUzMDA5MzQ4MTg=";
const pageLimit = 1;
const initialQuery = mirror._queryConnection(
"Issue",
"timeline",
undefined,
pageLimit
);
const expectedEndCursor = "MQ==";
const updateQuery = mirror._queryConnection(
"Issue",
"timeline",
expectedEndCursor,
pageLimit
);
return [
b.alias(
"unionInitial",
b.field("node", {id: b.literal(exampleIssueId)}, [
b.inlineFragment("Issue", initialQuery),
])
),
b.alias(
"unionUpdate",
b.field("node", {id: b.literal(exampleIssueId)}, [
b.inlineFragment("Issue", updateQuery),
])
),
b.alias(
"unionExpectedIds",
b.field("node", {id: b.literal(exampleIssueId)}, [
b.inlineFragment("Issue", [
b.field("timeline", {first: b.literal(pageLimit * 2)}, [
b.field("nodes", {}, [
...issueTimelineItemClauses().map((clause) =>
b.inlineFragment(clause, [b.field("id")])
),
]),
]),
]),
])
),
];
}
const query = b.query(
"TestQuery",
[],
[...objectConnectionQuery(), ...unionConnectionQuery()]
);
const format = (body: Queries.Body): string =>
Queries.stringify.body(body, Queries.multilineLayout(" "));
@ -881,14 +1080,17 @@ describe("graphql/mirror", () => {
it("processes object types properly", () => {
const result = _buildSchemaInfo(buildGithubSchema());
expect(Object.keys(result.objectTypes).sort()).toEqual(
[
Array.from(
new Set([
"Repository",
"Issue",
"IssueComment",
"User",
"Bot",
"Organization",
].sort()
...issueTimelineItemClauses(),
])
).sort()
);
expect(result.objectTypes["Issue"].fields).toEqual(
(buildGithubSchema().Issue: any).fields
@ -901,11 +1103,13 @@ describe("graphql/mirror", () => {
);
expect(
result.objectTypes["Issue"].connectionFieldNames.slice().sort()
).toEqual(["comments"].sort());
).toEqual(["comments", "timeline"].sort());
});
it("processes union types correctly", () => {
const result = _buildSchemaInfo(buildGithubSchema());
expect(Object.keys(result.unionTypes).sort()).toEqual(["Actor"].sort());
expect(Object.keys(result.unionTypes).sort()).toEqual(
["Actor", "IssueTimelineItem"].sort()
);
expect(result.unionTypes["Actor"].clauses.slice().sort()).toEqual(
["User", "Bot", "Organization"].sort()
);