diff --git a/.changeset/eight-views-design.md b/.changeset/eight-views-design.md new file mode 100644 index 00000000000..3e83710356c --- /dev/null +++ b/.changeset/eight-views-design.md @@ -0,0 +1,6 @@ +--- +'hive': minor +--- + +adjust change capture and resolvers to optionally provide a full list or the simplified list of +changes diff --git a/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts b/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts index 5c4c90edf01..c672cf6e743 100644 --- a/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts +++ b/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts @@ -83,6 +83,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -166,6 +167,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -192,6 +194,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -275,6 +278,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -301,6 +305,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -394,6 +399,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -420,6 +426,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -571,6 +578,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -597,6 +605,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -745,6 +754,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let sdlStoreCount = await db.oneFirst(sql`SELECT count(*) as total FROM sdl_store`); @@ -856,6 +866,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -1016,6 +1027,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -1061,6 +1073,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( diff --git a/packages/services/api/src/modules/schema/providers/inspector.ts b/packages/services/api/src/modules/schema/providers/inspector.ts index 34ed1bcb2fe..54ac5fc543e 100644 --- a/packages/services/api/src/modules/schema/providers/inspector.ts +++ b/packages/services/api/src/modules/schema/providers/inspector.ts @@ -10,7 +10,7 @@ import { type GraphQLSchema, } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; -import { Change, ChangeType, diff, DiffRule, TypeOfChangeType } from '@graphql-inspector/core'; +import { Change, ChangeType, diff, Rule, TypeOfChangeType } from '@graphql-inspector/core'; import { traceFn } from '@hive/service-common'; import { HiveSchemaChangeModel } from '@hive/storage'; import { Logger } from '../../shared/providers/logger'; @@ -31,10 +31,10 @@ export class Inspector { 'hive.diff.changes.count': result.length, }), }) - async diff(existing: GraphQLSchema, incoming: GraphQLSchema) { + async diff(existing: GraphQLSchema, incoming: GraphQLSchema, rules?: Rule[]) { this.logger.debug('Comparing Schemas'); - const changes = await diff(existing, incoming, [DiffRule.simplifyChanges]); + const changes = await diff(existing, incoming, rules); return changes .filter(dropTrimmedDescriptionChangedChange) diff --git a/packages/services/api/src/modules/schema/providers/models/composite.ts b/packages/services/api/src/modules/schema/providers/models/composite.ts index e0acee26e1b..e84fc679207 100644 --- a/packages/services/api/src/modules/schema/providers/models/composite.ts +++ b/packages/services/api/src/modules/schema/providers/models/composite.ts @@ -78,6 +78,7 @@ export class CompositeModel { existingSdl: contract.latestValidVersion?.compositeSchemaSdl ?? null, incomingSdl: contractCompositionResult?.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange: args.failDiffOnDangerousChange, + filterNestedChanges: true, }), }; }), @@ -103,6 +104,7 @@ export class CompositeModel { conditionalBreakingChangeDiffConfig, contracts, failDiffOnDangerousChange, + filterNestedChanges, }: { input: { sdl: string; @@ -136,6 +138,7 @@ export class CompositeModel { approvedChanges: Map | null; } > | null; + filterNestedChanges: boolean; }): Promise { const incoming: PushedCompositeSchema = { kind: 'composite', @@ -228,6 +231,7 @@ export class CompositeModel { compositionCheck.result?.fullSchemaSdl ?? compositionCheck.reason?.fullSchemaSdl ?? null, conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig, failDiffOnDangerousChange, + filterNestedChanges, }), this.checks.policyCheck({ selector, @@ -487,6 +491,7 @@ export class CompositeModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges: true, // filter because publish is never associated to schema proposals in this way. }); const contractChecks = await this.getContractChecks({ @@ -652,6 +657,7 @@ export class CompositeModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges: true, // filter because deletes are never associated with schema proposals in this way. }); const contractChecks = await this.getContractChecks({ diff --git a/packages/services/api/src/modules/schema/providers/models/single.ts b/packages/services/api/src/modules/schema/providers/models/single.ts index a7f6ad6e724..a39099aafea 100644 --- a/packages/services/api/src/modules/schema/providers/models/single.ts +++ b/packages/services/api/src/modules/schema/providers/models/single.ts @@ -43,6 +43,7 @@ export class SingleModel { approvedChanges, conditionalBreakingChangeDiffConfig, failDiffOnDangerousChange, + filterNestedChanges, }: { input: { sdl: string; @@ -68,6 +69,7 @@ export class SingleModel { approvedChanges: Map; conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig; failDiffOnDangerousChange: boolean; + filterNestedChanges: boolean; }): Promise { const incoming: SingleSchema = { kind: 'single', @@ -130,6 +132,7 @@ export class SingleModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges, }), this.checks.policyCheck({ selector, @@ -267,6 +270,7 @@ export class SingleModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges: true, // publish is never associated with schema proposals in this way. So always show the minimal changeset. }), ]); diff --git a/packages/services/api/src/modules/schema/providers/registry-checks.ts b/packages/services/api/src/modules/schema/providers/registry-checks.ts index 2e162ac55c5..59e67d3b8c8 100644 --- a/packages/services/api/src/modules/schema/providers/registry-checks.ts +++ b/packages/services/api/src/modules/schema/providers/registry-checks.ts @@ -2,7 +2,7 @@ import { URL } from 'node:url'; import { type GraphQLSchema } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; import hashObject from 'object-hash'; -import { ChangeType, CriticalityLevel, TypeOfChangeType } from '@graphql-inspector/core'; +import { ChangeType, CriticalityLevel, DiffRule, TypeOfChangeType } from '@graphql-inspector/core'; import type { CheckPolicyResponse } from '@hive/policy'; import type { CompositionFailureError, ContractsInputType } from '@hive/schema'; import { traceFn } from '@hive/service-common'; @@ -426,6 +426,11 @@ export class RegistryChecks { /** Settings for fetching conditional breaking changes. */ conditionalBreakingChangeConfig: null | ConditionalBreakingChangeDiffConfig; failDiffOnDangerousChange: null | boolean; + /** + * Set to true to reduce the number of changes to only what's relevant to the user. + * Use false for schema proposals in order to capture every single change record for the patch function. + */ + filterNestedChanges: boolean; }) { let existingSchema: GraphQLSchema | null = null; let incomingSchema: GraphQLSchema | null = null; @@ -463,7 +468,11 @@ export class RegistryChecks { } satisfies CheckResult; } - let inspectorChanges = await this.inspector.diff(existingSchema, incomingSchema); + let inspectorChanges = await this.inspector.diff( + existingSchema, + incomingSchema, + args.filterNestedChanges ? [DiffRule.simplifyChanges] : [], + ); // Filter out federation specific changes as they are not relevant for the schema diff and were in previous schema versions by accident. if (args.filterOutFederationChanges === true) { diff --git a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts index 9a63b89a8ec..7b5de099b12 100644 --- a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts +++ b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts @@ -48,7 +48,12 @@ export class SchemaCheckManager { return null; } - return [...(schemaCheck.breakingSchemaChanges ?? []), ...(schemaCheck.safeSchemaChanges ?? [])]; + const changes = [ + ...(schemaCheck.breakingSchemaChanges ?? []), + ...(schemaCheck.safeSchemaChanges ?? []), + ]; + + return changes; } getBreakingSchemaChanges(schemaCheck: SchemaCheck) { diff --git a/packages/services/api/src/modules/schema/providers/schema-publisher.ts b/packages/services/api/src/modules/schema/providers/schema-publisher.ts index d7a343d40e8..308b8257d08 100644 --- a/packages/services/api/src/modules/schema/providers/schema-publisher.ts +++ b/packages/services/api/src/modules/schema/providers/schema-publisher.ts @@ -492,7 +492,6 @@ export class SchemaPublisher { } if (github != null) { - // @todo should proposals do anything special here? const result = await this.createGithubCheckRunStartForSchemaCheck({ organization, project, @@ -623,6 +622,7 @@ export class SchemaPublisher { conditionalBreakingChangeDiffConfig: conditionalBreakingChangeConfiguration?.conditionalBreakingChangeDiffConfig ?? null, failDiffOnDangerousChange, + filterNestedChanges: !input.schemaProposalId, }); break; case ProjectType.FEDERATION: @@ -668,6 +668,7 @@ export class SchemaPublisher { conditionalBreakingChangeDiffConfig: conditionalBreakingChangeConfiguration?.conditionalBreakingChangeDiffConfig ?? null, failDiffOnDangerousChange, + filterNestedChanges: !input.schemaProposalId, }); break; default: @@ -732,7 +733,7 @@ export class SchemaPublisher { breakingSchemaChanges: contract.schemaChanges?.breaking ?? null, safeSchemaChanges: contract.schemaChanges?.safe ?? null, })) ?? null, - schemaProposalId: input.schemaProposalId, + schemaProposalId: input.schemaProposalId ?? null, }); this.logger.info('created failed schema check. (schemaCheckId=%s)', schemaCheck.id); } else if (checkResult.conclusion === SchemaCheckConclusion.Success) { @@ -779,7 +780,7 @@ export class SchemaPublisher { breakingSchemaChanges: contract.schemaChanges?.breaking ?? null, safeSchemaChanges: contract.schemaChanges?.safe ?? null, })) ?? null, - schemaProposalId: input.schemaProposalId, + schemaProposalId: input.schemaProposalId ?? null, }); this.logger.info('created successful schema check. (schemaCheckId=%s)', schemaCheck.id); } else if (checkResult.conclusion === SchemaCheckConclusion.Skip) { @@ -858,7 +859,7 @@ export class SchemaPublisher { })), ) : null, - schemaProposalId: input.schemaProposalId, + schemaProposalId: input.schemaProposalId ?? null, }); this.logger.info('created skipped schema check. (schemaCheckId=%s)', schemaCheck.id); } diff --git a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts index d65d6982d2e..249dca50f2b 100644 --- a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts +++ b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts @@ -174,9 +174,11 @@ export class SchemaVersionHelper { } if (schemaVersion.hasPersistedSchemaChanges) { - const changes = await this.storage.getSchemaChangesForVersion({ - versionId: schemaVersion.id, - }); + const changes: null | Array = await this.storage.getSchemaChangesForVersion( + { + versionId: schemaVersion.id, + }, + ); const safeChanges: Array = []; const breakingChanges: Array = []; @@ -241,6 +243,7 @@ export class SchemaVersionHelper { filterOutFederationChanges: project.type === ProjectType.FEDERATION, conditionalBreakingChangeConfig: null, failDiffOnDangerousChange, + filterNestedChanges: true, }); if (diffCheck.status === 'skipped') { diff --git a/packages/services/storage/src/schema-change-model.ts b/packages/services/storage/src/schema-change-model.ts index 02e6a8ef206..5ccd95fadd2 100644 --- a/packages/services/storage/src/schema-change-model.ts +++ b/packages/services/storage/src/schema-change-model.ts @@ -1012,16 +1012,16 @@ export const ObjectTypeInterfaceRemovedModel = implement().with({ type: SchemaQueryTypeChangedLiteral, meta: z.object({ - oldQueryTypeName: z.string(), - newQueryTypeName: z.string(), + oldQueryTypeName: z.string().nullable(), + newQueryTypeName: z.string().nullable(), }), }); export const SchemaMutationTypeChangedModel = implement().with({ type: SchemaMutationTypeChangedLiteral, meta: z.object({ - oldMutationTypeName: z.string(), - newMutationTypeName: z.string(), + oldMutationTypeName: z.string().nullable(), + newMutationTypeName: z.string().nullable(), }), }); @@ -1029,8 +1029,8 @@ export const SchemaSubscriptionTypeChangedModel = implement().with({ type: SchemaSubscriptionTypeChangedLiteral, meta: z.object({ - oldSubscriptionTypeName: z.string(), - newSubscriptionTypeName: z.string(), + oldSubscriptionTypeName: z.string().nullable(), + newSubscriptionTypeName: z.string().nullable(), }), }); @@ -1414,6 +1414,7 @@ const SchemaCheckSharedOutputFields = { serviceUrl: z.string().nullable(), targetId: z.string(), schemaVersionId: z.string().nullable(), + schemaProposalId: z.string().nullable(), meta: z .object({ author: z.string(), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dac413729ac..c996a011222 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -19669,8 +19669,8 @@ snapshots: dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.596.0(@aws-sdk/client-sts@3.596.0) - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sso-oidc': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/core': 3.592.0 '@aws-sdk/credential-provider-node': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0) '@aws-sdk/middleware-host-header': 3.577.0 @@ -19822,11 +19822,11 @@ snapshots: transitivePeerDependencies: - aws-crt - '@aws-sdk/client-sso-oidc@3.596.0(@aws-sdk/client-sts@3.596.0)': + '@aws-sdk/client-sso-oidc@3.596.0': dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/core': 3.592.0 '@aws-sdk/credential-provider-node': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0) '@aws-sdk/middleware-host-header': 3.577.0 @@ -19865,7 +19865,6 @@ snapshots: '@smithy/util-utf8': 3.0.0 tslib: 2.8.1 transitivePeerDependencies: - - '@aws-sdk/client-sts' - aws-crt '@aws-sdk/client-sso-oidc@3.723.0(@aws-sdk/client-sts@3.723.0)': @@ -20085,11 +20084,11 @@ snapshots: transitivePeerDependencies: - aws-crt - '@aws-sdk/client-sts@3.596.0': + '@aws-sdk/client-sts@3.596.0(@aws-sdk/client-sso-oidc@3.596.0)': dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.596.0(@aws-sdk/client-sts@3.596.0) + '@aws-sdk/client-sso-oidc': 3.596.0 '@aws-sdk/core': 3.592.0 '@aws-sdk/credential-provider-node': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0) '@aws-sdk/middleware-host-header': 3.577.0 @@ -20128,6 +20127,7 @@ snapshots: '@smithy/util-utf8': 3.0.0 tslib: 2.8.1 transitivePeerDependencies: + - '@aws-sdk/client-sso-oidc' - aws-crt '@aws-sdk/client-sts@3.723.0': @@ -20359,7 +20359,7 @@ snapshots: '@aws-sdk/credential-provider-ini@3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0)': dependencies: - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/credential-provider-env': 3.587.0 '@aws-sdk/credential-provider-http': 3.596.0 '@aws-sdk/credential-provider-process': 3.587.0 @@ -20606,7 +20606,7 @@ snapshots: '@aws-sdk/credential-provider-web-identity@3.587.0(@aws-sdk/client-sts@3.596.0)': dependencies: - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/types': 3.577.0 '@smithy/property-provider': 3.1.11 '@smithy/types': 3.7.2 @@ -20995,7 +20995,7 @@ snapshots: '@aws-sdk/token-providers@3.587.0(@aws-sdk/client-sso-oidc@3.596.0)': dependencies: - '@aws-sdk/client-sso-oidc': 3.596.0(@aws-sdk/client-sts@3.596.0) + '@aws-sdk/client-sso-oidc': 3.596.0 '@aws-sdk/types': 3.577.0 '@smithy/property-provider': 3.1.11 '@smithy/shared-ini-file-loader': 3.1.12