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/services/api/src/modules/schema/module.graphql.ts b/packages/services/api/src/modules/schema/module.graphql.ts index 233646e19d5..3ab65fa4037 100644 --- a/packages/services/api/src/modules/schema/module.graphql.ts +++ b/packages/services/api/src/modules/schema/module.graphql.ts @@ -879,10 +879,25 @@ export default gql` """ Schema changes that were introduced in this schema version (compared to the previous version). """ - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection """ GitHub metadata associated with the schema version. @@ -1261,9 +1276,24 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges: SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection schemaPolicyErrors: SchemaPolicyWarningConnection """ @@ -1299,10 +1329,25 @@ export default gql` schemaCompositionErrors: SchemaErrorConnection @tag(name: "public") - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection compositeSchemaSDL: String @tag(name: "public") supergraphSDL: String @tag(name: "public") @@ -1345,13 +1390,28 @@ export default gql` """ Breaking schema changes for this contract version. """ - breakingSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection """ Safe schema changes for this contract version. """ - safeSchemaChanges: SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") previousContractVersion: ContractVersion @tag(name: "public") previousDiffableContractVersion: ContractVersion @tag(name: "public") @@ -1412,12 +1472,27 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") """ Breaking changes can exist in an successful schema check if the check was manually approved. """ - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection """ Schema policy errors can exist in an successful schema check if the check was manually approved. @@ -1509,9 +1584,24 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges: SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection schemaPolicyErrors: SchemaPolicyWarningConnection 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/registry-checks.ts b/packages/services/api/src/modules/schema/providers/registry-checks.ts index 2e162ac55c5..d415b4ffa18 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,7 @@ export class RegistryChecks { /** Settings for fetching conditional breaking changes. */ conditionalBreakingChangeConfig: null | ConditionalBreakingChangeDiffConfig; failDiffOnDangerousChange: null | boolean; + filterNestedChanges: boolean; }) { let existingSchema: GraphQLSchema | null = null; let incomingSchema: GraphQLSchema | null = null; @@ -463,7 +464,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..63acb920266 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 @@ -3,6 +3,7 @@ import type { SuccessfulSchemaCheckMapper, } from '../module.graphql.mappers'; import { Injectable, Scope } from 'graphql-modules'; +import { DiffRule } from '@graphql-inspector/core'; import { Storage } from '../../shared/providers/storage'; import { formatNumber } from '../lib/number-formatting'; import { SchemaManager } from './schema-manager'; @@ -35,27 +36,70 @@ export class SchemaCheckManager { return !!(schemaCheck.breakingSchemaChanges?.length || schemaCheck.safeSchemaChanges?.length); } - getSafeSchemaChanges(schemaCheck: SchemaCheck) { + getSafeSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { if (!schemaCheck.safeSchemaChanges?.length) { return null; } + if (simplifyChanges) { + /** + * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule + * doesn't need more information than the change type and path fields. + */ + return DiffRule.simplifyChanges({ + changes: schemaCheck.safeSchemaChanges as any, + oldSchema: undefined as any, + newSchema: undefined as any, + config: undefined, + }) as unknown as typeof schemaCheck.safeSchemaChanges; + } + return schemaCheck.safeSchemaChanges; } - getAllSchemaChanges(schemaCheck: SchemaCheck) { + getAllSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { if (!schemaCheck.safeSchemaChanges?.length && !schemaCheck.breakingSchemaChanges?.length) { return null; } - return [...(schemaCheck.breakingSchemaChanges ?? []), ...(schemaCheck.safeSchemaChanges ?? [])]; + const changes = [ + ...(schemaCheck.breakingSchemaChanges ?? []), + ...(schemaCheck.safeSchemaChanges ?? []), + ]; + if (simplifyChanges) { + /** + * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule + * doesn't need more information than the change type and path fields. + */ + return DiffRule.simplifyChanges({ + changes: changes as any, + oldSchema: undefined as any, + newSchema: undefined as any, + config: undefined, + }) as unknown as typeof changes; + } + + return changes; } - getBreakingSchemaChanges(schemaCheck: SchemaCheck) { + getBreakingSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { if (!schemaCheck.breakingSchemaChanges?.length) { return null; } + if (simplifyChanges) { + /** + * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule + * doesn't need more information than the change type and path fields. + */ + return DiffRule.simplifyChanges({ + changes: schemaCheck.breakingSchemaChanges as any, + oldSchema: undefined as any, + newSchema: undefined as any, + config: undefined, + }); + } + return schemaCheck.breakingSchemaChanges; } 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..46e8b369765 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 @@ -156,7 +156,7 @@ export class SchemaVersionHelper { } @traceFn('SchemaVersionHelper._getSchemaChanges', { - initAttributes: input => ({ + initAttributes: ({ version: input }) => ({ 'hive.target.id': input.targetId, 'hive.organization.id': input.organizationId, 'hive.project.id': input.projectId, @@ -167,16 +167,26 @@ export class SchemaVersionHelper { 'hive.safe-changes.count': changes?.safe?.length, }), }) - @cache(version => version.id) - private async _getSchemaChanges(schemaVersion: SchemaVersion) { + @cache<{ version: SchemaVersion; simplified: boolean }>( + ({ version, simplified = true }) => `${version.id}${simplified ? '/simple' : ''}`, + ) + private async _getSchemaChanges({ + version: schemaVersion, + simplified, + }: { + version: SchemaVersion; + simplified: boolean; + }) { if (!schemaVersion.isComposable) { return null; } 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 +251,7 @@ export class SchemaVersionHelper { filterOutFederationChanges: project.type === ProjectType.FEDERATION, conditionalBreakingChangeConfig: null, failDiffOnDangerousChange, + filterNestedChanges: simplified, }); if (diffCheck.status === 'skipped') { @@ -274,23 +285,35 @@ export class SchemaVersionHelper { }); } - async getBreakingSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getBreakingSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return changes?.breaking ?? null; } - async getSafeSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getSafeSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return changes?.safe ?? null; } - async getAllSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getAllSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return changes?.all ?? null; } - async getHasSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getHasSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return !!changes?.breaking?.length || !!changes?.safe?.length; } diff --git a/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts b/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts index 7e5c66421a3..566126158d5 100644 --- a/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts +++ b/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts @@ -70,14 +70,14 @@ export const SchemaVersion: SchemaVersionResolvers = { schemaCompositionErrors: async (version, _, { injector }) => { return injector.get(SchemaVersionHelper).getSchemaCompositionErrors(version); }, - breakingSchemaChanges: async (version, _, { injector }) => { - return injector.get(SchemaVersionHelper).getBreakingSchemaChanges(version); + breakingSchemaChanges: async (version, { simplifyChanges }, { injector }) => { + return injector.get(SchemaVersionHelper).getBreakingSchemaChanges(version, simplifyChanges); }, - safeSchemaChanges: async (version, _, { injector }) => { - return injector.get(SchemaVersionHelper).getSafeSchemaChanges(version); + safeSchemaChanges: async (version, { simplifyChanges }, { injector }) => { + return injector.get(SchemaVersionHelper).getSafeSchemaChanges(version, simplifyChanges); }, - schemaChanges: async (version, _, { injector }) => { - return injector.get(SchemaVersionHelper).getAllSchemaChanges(version); + schemaChanges: async (version, { simplifyChanges }, { injector }) => { + return injector.get(SchemaVersionHelper).getAllSchemaChanges(version, simplifyChanges); }, supergraph: async (version, _, { injector }) => { return injector.get(SchemaVersionHelper).getSupergraphSdl(version); diff --git a/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts b/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts index 1e4cae9781b..9715780a9e2 100644 --- a/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts +++ b/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts @@ -8,11 +8,11 @@ export const SuccessfulSchemaCheck: SuccessfulSchemaCheckResolvers = { schemaVersion: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getSchemaVersion(schemaCheck); }, - safeSchemaChanges: (schemaCheck, _, { injector }) => { - return injector.get(SchemaCheckManager).getSafeSchemaChanges(schemaCheck); + safeSchemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { + return injector.get(SchemaCheckManager).getSafeSchemaChanges(schemaCheck, simplifyChanges); }, - breakingSchemaChanges: (schemaCheck, _, { injector }) => { - return injector.get(SchemaCheckManager).getBreakingSchemaChanges(schemaCheck); + breakingSchemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { + return injector.get(SchemaCheckManager).getBreakingSchemaChanges(schemaCheck, simplifyChanges); }, hasSchemaCompositionErrors: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getHasSchemaCompositionErrors(schemaCheck); @@ -70,7 +70,7 @@ export const SuccessfulSchemaCheck: SuccessfulSchemaCheckResolvers = { conditionalBreakingChangeMetadata: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getConditionalBreakingChangeMetadata(schemaCheck); }, - schemaChanges: (schemaCheck, _, { injector }) => { - return injector.get(SchemaCheckManager).getAllSchemaChanges(schemaCheck); + schemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { + return injector.get(SchemaCheckManager).getAllSchemaChanges(schemaCheck, simplifyChanges); }, }; diff --git a/packages/web/app/src/pages/target-proposal.tsx b/packages/web/app/src/pages/target-proposal.tsx index 19fb9c9994c..67d86e5bea6 100644 --- a/packages/web/app/src/pages/target-proposal.tsx +++ b/packages/web/app/src/pages/target-proposal.tsx @@ -126,7 +126,7 @@ const ProposalChangesQuery = graphql(/* GraphQL */ ` } schemaSDL serviceName - schemaChanges { + schemaChanges(simplifyChanges: false) { edges { node { ...ProposalOverview_ChangeFragment