Skip to content

Commit ba634bb

Browse files
committed
Add proposal change approval tracking job
1 parent 2548cda commit ba634bb

9 files changed

Lines changed: 176 additions & 77 deletions

File tree

packages/services/api/src/modules/proposals/providers/schema-proposal-manager.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -409,16 +409,13 @@ export class SchemaProposalManager {
409409
change as unknown as Change<any>,
410410
) && implementedChange.schemaVersionId,
411411
);
412-
if (implementation) {
413-
return {
414-
schemaProposal: { id: schemaProposalId },
415-
implementedBy: implementation?.schemaVersionId
416-
? {
417-
id: implementation.schemaVersionId,
418-
}
419-
: null,
420-
};
421-
}
422-
return null;
412+
return {
413+
schemaProposal: { id: schemaProposalId },
414+
implementedBy: implementation?.schemaVersionId
415+
? {
416+
id: implementation.schemaVersionId,
417+
}
418+
: null,
419+
};
423420
}
424421
}

packages/services/api/src/modules/proposals/providers/schema-proposal-storage.ts

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
SchemaChangeModel,
1313
} from '@hive/storage';
1414
import { TaskScheduler } from '@hive/workflows/kit';
15+
import { SchemaProposalApprovalTask } from '@hive/workflows/tasks/schema-proposal-approval';
1516
import { SchemaProposalCompositionTask } from '@hive/workflows/tasks/schema-proposal-composition';
1617
import { SchemaProposalStage } from '../../../__generated__/types';
1718
import { Logger } from '../../shared/providers/logger';
@@ -61,6 +62,10 @@ export class SchemaProposalStorage {
6162
await this.taskScheduler.scheduleTask(SchemaProposalCompositionTask, input);
6263
}
6364

65+
async runBackgroundApproval(input: { proposalId: string; targetId: string }) {
66+
await this.taskScheduler.scheduleTask(SchemaProposalApprovalTask, input);
67+
}
68+
6469
private async assertSchemaProposalsEnabled(args: {
6570
organizationId: string;
6671
targetId: string;
@@ -140,9 +145,15 @@ export class SchemaProposalStorage {
140145
)
141146
RETURNING ${schemaProposalReviewFields}
142147
`);
148+
143149
return SchemaProposalReviewModel.parse(row);
144150
});
145151

152+
// @todo rollback if this fails
153+
if (args.stage === 'APPROVED') {
154+
this.runBackgroundApproval({ proposalId: args.id, targetId: args.targetId });
155+
}
156+
146157
return {
147158
type: 'ok' as const,
148159
review,
@@ -384,37 +395,6 @@ export class SchemaProposalStorage {
384395
};
385396
}
386397

387-
async _approveChanges(
388-
conn: CommonQueryMethods,
389-
records: {
390-
change: Change;
391-
proposalId: string;
392-
service?: string;
393-
targetId: string;
394-
}[],
395-
) {
396-
const values = records.map(
397-
r => psql`(
398-
${generateChangeHash(r.change)}
399-
,${JSON.stringify(r.change)}
400-
,${r.proposalId}
401-
,${r.service ?? null}
402-
,${r.targetId}
403-
)`,
404-
);
405-
406-
await conn.query(psql`
407-
INSERT INTO "proposal_approved_changes" (
408-
hash
409-
,change
410-
,proposal_id
411-
,service
412-
,target_id
413-
)
414-
VALUES ${psql.join(values, psql.fragment`,`)}
415-
`);
416-
}
417-
418398
/**
419399
* Updates an approved change to set the schema version where it has been implemented.
420400
*/

packages/services/api/src/modules/proposals/resolvers/SchemaProposalChangeDetails.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export const SchemaProposalChangeDetails: SchemaProposalChangeDetailsResolvers =
3737
},
3838
schemaProposal: async (parent, _arg, { injector }) => {
3939
if (!parent) {
40+
// this should never happen
4041
throw new Error('Uh oh');
4142
}
4243
const schemaProposalId = parent.schemaProposal.id;
@@ -46,7 +47,7 @@ export const SchemaProposalChangeDetails: SchemaProposalChangeDetailsResolvers =
4647
.getProposal({ id: schemaProposalId });
4748

4849
if (!proposal) {
49-
throw new Error('Something went wrong!');
50+
throw new Error('Proposal not found');
5051
}
5152
return proposal;
5253
},

packages/services/api/src/modules/schema/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createModule } from 'graphql-modules';
2+
import { SchemaProposalManager } from '../proposals/providers/schema-proposal-manager';
23
import { SchemaProposalStorage } from '../proposals/providers/schema-proposal-storage';
34
import { BreakingSchemaChangeUsageHelper } from './providers/breaking-schema-changes-helper';
45
import { Contracts } from './providers/contracts';
@@ -34,5 +35,6 @@ export const schemaModule = createModule({
3435
CompositionOrchestrator,
3536
...models,
3637
SchemaProposalStorage,
38+
SchemaProposalManager,
3739
],
3840
});

packages/services/workflows/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
},
1111
"devDependencies": {
1212
"@graphql-hive/logger": "1.0.9",
13+
"@graphql-inspector/compare-changes": "0.1.0-alpha-20260328000712-2cf98744a012fa66614588ed89ba99719b0f461b",
1314
"@graphql-inspector/core": "7.1.2",
1415
"@graphql-inspector/patch": "0.1.3",
1516
"@graphql-yoga/redis-event-target": "3.0.3",

packages/services/workflows/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const modules = await Promise.all([
4343
import('./tasks/usage-rate-limit-exceeded.js'),
4444
import('./tasks/usage-rate-limit-warning.js'),
4545
import('./tasks/schema-proposal-composition.js'),
46+
import('./tasks/schema-proposal-approval.js'),
4647
]);
4748

4849
const crontab = `

packages/services/workflows/src/lib/schema/provider.ts

Lines changed: 93 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { DocumentNode, GraphQLError, parse, print, SourceLocation } from 'graphql';
22
import { z } from 'zod';
33
import type { Logger } from '@graphql-hive/logger';
4+
import { generateChangeHash } from '@graphql-inspector/compare-changes';
45
import type { Change } from '@graphql-inspector/core';
56
import { errors, patch } from '@graphql-inspector/patch';
67
import type { Project, SchemaObject } from '@hive/api';
78
import type { ComposeAndValidateResult } from '@hive/api/shared/entities';
8-
import { PostgresDatabasePool, psql } from '@hive/postgres';
9+
import { CommonQueryMethods, PostgresDatabasePool, psql } from '@hive/postgres';
910
import type { ContractsInputType, SchemaBuilderApi } from '@hive/schema';
1011
import { decodeCreatedAtAndUUIDIdBasedCursor, HiveSchemaChangeModel } from '@hive/storage';
1112
import { createTRPCProxyClient, httpLink } from '@trpc/client';
@@ -32,6 +33,8 @@ const SchemaProposalChangesModel = z.object({
3233
createdAt: z.string(),
3334
});
3435

36+
type SchemaProposalChangesModelType = z.TypeOf<typeof SchemaProposalChangesModel>;
37+
3538
function createExternalConfig(config: Project['externalComposition']) {
3639
// : ExternalCompositionConfig {
3740
if (config && config.enabled) {
@@ -270,12 +273,25 @@ export function schemaProvider(providerConfig: SchemaProviderConfig) {
270273
return baseSchema;
271274
},
272275

273-
async proposedSchemas(args: {
274-
targetId: string;
275-
proposalId: string;
276-
cursor?: string | null;
277-
pool: PostgresDatabasePool;
278-
}) {
276+
/**
277+
* Loops through all schema checks that are part of the schema proposal. This uses pagination
278+
* to avoid loading too much at once
279+
*
280+
* This is hard capped at 2_000 subgraphs for safety.
281+
**/
282+
async forEachProposalCheck(
283+
args: {
284+
targetId: string;
285+
proposalId: string;
286+
cursor?: string | null;
287+
pool: PostgresDatabasePool;
288+
},
289+
callback: (
290+
change: Omit<SchemaProposalChangesModelType, 'schemaProposalChanges'> & {
291+
schemaProposalChanges: Change[];
292+
},
293+
) => void | Promise<void>,
294+
) {
279295
const now = new Date().toISOString();
280296
let cursor: {
281297
createdAt: string;
@@ -288,11 +304,6 @@ export function schemaProvider(providerConfig: SchemaProviderConfig) {
288304

289305
// fetch all latest schemas. Support up to 2_000 subgraphs.
290306
const maxLoops = 100;
291-
const services = await this.latestComposableSchemas({
292-
targetId: args.targetId,
293-
pool: args.pool,
294-
});
295-
296307
let nextCursor = cursor;
297308
// collect changes in paginated requests to avoid stalling the db
298309
let i = 0;
@@ -350,48 +361,96 @@ export function schemaProvider(providerConfig: SchemaProviderConfig) {
350361
LIMIT 20
351362
`);
352363

353-
const changes = result.map(row => {
354-
const value = SchemaProposalChangesModel.parse(row);
364+
const checks = result.map(row => {
365+
const check = SchemaProposalChangesModel.parse(row);
355366
return {
356-
...value,
357-
schemaProposalChanges: value.schemaProposalChanges.map(c => {
358-
const change: Change<any> = {
359-
...c,
367+
...check,
368+
schemaProposalChanges: check.schemaProposalChanges.map((c): Change<any> => {
369+
return {
370+
message: c.message,
371+
meta: c.meta,
372+
type: c.type,
360373
path: c.path ?? undefined,
361374
criticality: {
362375
level: c.criticality,
363376
},
364-
};
365-
return change;
377+
} satisfies Change<any>;
366378
}),
367379
};
368380
});
369381

370-
if (changes.length === 20) {
382+
for (const check of checks) {
383+
await callback(check);
384+
}
385+
386+
if (checks.length === 20) {
371387
nextCursor = {
372388
// Keep the created at because we want the same set of checks when joining on the "latest".
373-
createdAt: nextCursor?.createdAt ?? changes[0]?.createdAt ?? now,
374-
id: changes[changes.length - 1].id,
389+
createdAt: nextCursor?.createdAt ?? checks[0]?.createdAt ?? now,
390+
id: checks[checks.length - 1].id,
375391
};
376392
} else {
377393
nextCursor = null;
378394
}
395+
} while (nextCursor && ++i < maxLoops);
396+
},
379397

380-
for (const change of changes) {
381-
const service = services.find(s => change.serviceName === s.serviceName);
382-
if (service) {
383-
const ast = parse(service.sdl, { noLocation: true });
384-
service.sdl = print(
385-
patch(ast, change.schemaProposalChanges, { onError: errors.looseErrorHandler }),
386-
);
387-
if (change.serviceUrl) {
388-
service.serviceUrl = change.serviceUrl;
389-
}
398+
async proposedSchemas(args: {
399+
targetId: string;
400+
proposalId: string;
401+
cursor?: string | null;
402+
pool: PostgresDatabasePool;
403+
}) {
404+
const services = await this.latestComposableSchemas({
405+
targetId: args.targetId,
406+
pool: args.pool,
407+
});
408+
409+
await this.forEachProposalCheck(args, change => {
410+
const service = services.find(s => change.serviceName === s.serviceName);
411+
if (service) {
412+
const ast = parse(service.sdl, { noLocation: true });
413+
service.sdl = print(
414+
patch(ast, change.schemaProposalChanges, { onError: errors.looseErrorHandler }),
415+
);
416+
if (change.serviceUrl) {
417+
service.serviceUrl = change.serviceUrl;
390418
}
391419
}
392-
} while (nextCursor && ++i < maxLoops);
420+
});
393421

394422
return services;
395423
},
424+
425+
async approveChanges(
426+
conn: CommonQueryMethods,
427+
records: {
428+
change: Change;
429+
proposalId: string;
430+
service?: string;
431+
targetId: string;
432+
}[],
433+
) {
434+
const values = records.map(
435+
r => psql`(
436+
${generateChangeHash(r.change)}
437+
,${JSON.stringify(r.change)}
438+
,${r.proposalId}
439+
,${r.service ?? null}
440+
,${r.targetId}
441+
)`,
442+
);
443+
444+
await conn.query(psql`
445+
INSERT INTO "proposal_approved_changes" (
446+
hash
447+
,change
448+
,proposal_id
449+
,service
450+
,target_id
451+
)
452+
VALUES ${psql.join(values, psql.fragment`,`)}
453+
`);
454+
},
396455
};
397456
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* When a schema proposal is approved, then all changes for all checks associated
3+
* that proposal must be added to the approved changes list. This is to allow
4+
* future checks/publishes/etc to validate their change against this approved list.
5+
*
6+
* This job listens for a schema proposal approval, loops over all the latest
7+
* checks for that proposal, and inserts those change records into the approved list table.
8+
*/
9+
10+
import { z } from 'zod';
11+
import { defineTask, implementTask } from '../kit.js';
12+
13+
export const SchemaProposalApprovalTask = defineTask({
14+
name: 'schemaProposalApproval',
15+
schema: z.object({
16+
proposalId: z.string(),
17+
targetId: z.string(),
18+
}),
19+
});
20+
21+
export const task = implementTask(SchemaProposalApprovalTask, async args => {
22+
const pool = args.context.pg;
23+
try {
24+
// collect all changes
25+
const changes: Parameters<typeof args.context.schema.approveChanges>[1] = [];
26+
await args.context.schema.forEachProposalCheck(
27+
{
28+
pool,
29+
proposalId: args.input.proposalId,
30+
targetId: args.input.targetId,
31+
},
32+
async check => {
33+
const checkChanges = check.schemaProposalChanges.map(change => ({
34+
change,
35+
proposalId: args.input.proposalId,
36+
service: check.serviceName ?? undefined,
37+
targetId: args.input.targetId,
38+
}));
39+
changes.push(...checkChanges);
40+
},
41+
);
42+
43+
args.logger.info(
44+
'Approving changes (count=%d, proposal=%s)',
45+
changes.length,
46+
args.input.proposalId,
47+
);
48+
// Approve all changes in a single call. If this becomes a bottleneck, then
49+
// implement snapshots to allow inserting in chunks
50+
await args.context.schema.approveChanges(pool, changes);
51+
} catch (e: unknown) {
52+
args.logger.error('Proposal approval failed from %s', String(e));
53+
throw e;
54+
}
55+
});

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)