mirror: keep updating when only `typenames` remain (#1765)

Summary:
This fixes a bug introduced in #1665, where we added a `typenames`
clause to the query plan but didn’t update the termination checking
accordingly. As a result, query plans with only `typenames` left to
update would not execute, so `extract` would fail with a `NOT NULL`
constraint violation because not all transitively needed objects had
been fetched.

Databases created before this change are still valid. Re-running the
problematic `sourcecred load` command should successfully update the
cache and proceed.

Fixes #1762.

Test Plan:
Regression test added; `aracred/ideas` and `aracred/aracred-template`
both load successfully now.

wchargin-branch: mirror-typenames-only-plan
This commit is contained in:
William Chargin 2020-04-23 10:51:50 -07:00 committed by GitHub
parent 147a65b0c5
commit f546ff4377
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 59 additions and 1 deletions

View File

@ -645,6 +645,18 @@ export class Mirror {
}); });
} }
/**
* Check whether the given query plan indicates that the mirror is
* already up to date and no further queries are required.
*/
_isUpToDate(queryPlan: QueryPlan): boolean {
return (
queryPlan.typenames.length === 0 &&
queryPlan.objects.length === 0 &&
queryPlan.connections.length === 0
);
}
/** /**
* Create a GraphQL selection set to fetch data corresponding to the * Create a GraphQL selection set to fetch data corresponding to the
* given query plan. * given query plan.
@ -948,7 +960,7 @@ export class Mirror {
|} |}
): Promise<boolean> { ): Promise<boolean> {
const queryPlan = this._findOutdated(options.since); const queryPlan = this._findOutdated(options.since);
if (queryPlan.objects.length === 0 && queryPlan.connections.length === 0) { if (this._isUpToDate(queryPlan)) {
return Promise.resolve(false); return Promise.resolve(false);
} }
const querySelections = this._queryFromPlan(queryPlan, { const querySelections = this._queryFromPlan(queryPlan, {

View File

@ -636,6 +636,52 @@ describe("graphql/mirror", () => {
}); });
}); });
describe("_isUpToDate", () => {
it("marks empty query plans as up to date", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(
mirror._isUpToDate({typenames: [], objects: [], connections: []})
).toBe(true);
});
it("marks query plans with typename queries as dirty", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(
mirror._isUpToDate({typenames: ["foo"], objects: [], connections: []})
).toBe(false);
});
it("marks query plans with object queries as dirty", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(
mirror._isUpToDate({
typenames: [],
objects: [{typename: "Repository", id: "bar"}],
connections: [],
})
).toBe(false);
});
it("marks query plans with connection queries as dirty", () => {
const db = new Database(":memory:");
const mirror = new Mirror(db, buildGithubSchema());
expect(
mirror._isUpToDate({
typenames: [],
objects: [],
connections: [
{
objectTypename: "Issue",
objectId: "baz",
fieldname: "comments",
endCursor: undefined,
},
],
})
).toBe(false);
});
});
describe("_queryFromPlan", () => { describe("_queryFromPlan", () => {
it("errors if connections for an object have distinct typename", () => { it("errors if connections for an object have distinct typename", () => {
const db = new Database(":memory:"); const db = new Database(":memory:");