Compare schema coordinates of composable versions

- [ ] truncate the table
graphql-hive · Aug 16, 2024 · e4a2947 · e4a2947
1 parent d1aa2c6
commit e4a2947
Show file tree

Hide file tree

Showing 8 changed files with 290 additions and 159 deletions.
diff --git a/packages/services/api/src/modules/schema/providers/models/composite-legacy.ts b/packages/services/api/src/modules/schema/providers/models/composite-legacy.ts
@@ -161,6 +161,7 @@ export class CompositeLegacyModel {
     input,
     target,
     latest,
+    latestComposable,
     project,
     organization,
     baseSchema,
@@ -175,6 +176,11 @@ export class CompositeLegacyModel {
       sdl: string | null;
       schemas: PushedCompositeSchema[];
     } | null;
+    latestComposable: {
+      isComposable: boolean;
+      sdl: string | null;
+      schemas: PushedCompositeSchema[];
+    } | null;
     baseSchema: string | null;
     conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
   }): Promise<SchemaPublishResult> {
@@ -275,15 +281,26 @@ export class CompositeLegacyModel {
       contracts: null,
     });
 
-    const previousVersionSdl = await this.checks.retrievePreviousVersionSdl({
-      orchestrator,
-      version: latestVersion,
-      organization,
-      project,
-      targetId: target.id,
-    });
+    const [previousVersionSdl, previousComposableVersionSdl] = await Promise.all([
+      this.checks.retrievePreviousVersionSdl({
+        orchestrator,
+        version: latestVersion,
+        organization,
+        project,
+        targetId: target.id,
+      }),
+      latestComposable
+        ? this.checks.retrievePreviousVersionSdl({
+            orchestrator,
+            version: latestComposable,
+            organization,
+            project,
+            targetId: target.id,
+          })
+        : null,
+    ]);
 
-    const [diffCheck, metadataCheck] = await Promise.all([
+    const [diffCheck, metadataCheck, coordinatesDiff] = await Promise.all([
       this.checks.diff({
         includeUrlChanges: {
           schemasBefore: latestVersion?.schemas ?? [],
@@ -300,6 +317,10 @@ export class CompositeLegacyModel {
             status: 'skipped' as const,
           }
         : this.checks.metadata(incoming, previousService ?? null),
+      this.checks.coordinatesDiff({
+        existingComposableSchema: previousComposableVersionSdl,
+        incomingComposableSchema: compositionCheck.result?.fullSchemaSdl ?? null,
+      }),
     ]);
 
     const compositionErrors =
@@ -347,11 +368,7 @@ export class CompositeLegacyModel {
           messages,
           changes,
           breakingChanges: breakingChanges ?? null,
-          coordinatesDiff:
-            diffCheck.result?.coordinatesDiff ??
-            diffCheck.reason?.coordinatesDiff ??
-            diffCheck.data?.coordinatesDiff ??
-            null,
+          coordinatesDiff,
           compositionErrors,
           schema: incoming,
           schemas,
@@ -377,7 +394,7 @@ export class CompositeLegacyModel {
         code: PublishFailureReasonCode.BreakingChanges,
         changes: diffCheck.reason.all ?? [],
         breakingChanges: diffCheck.reason.breaking ?? [],
-        coordinatesDiff: diffCheck.reason?.coordinatesDiff ?? null,
+        coordinatesDiff,
       });
     }
 

diff --git a/packages/services/api/src/modules/schema/providers/models/composite.ts b/packages/services/api/src/modules/schema/providers/models/composite.ts
@@ -441,31 +441,47 @@ export class CompositeModel {
       };
     }
 
-    const previousVersionSdl = await this.checks.retrievePreviousVersionSdl({
-      orchestrator,
-      version: schemaVersionToCompareAgainst,
-      organization,
-      project,
-      targetId: target.id,
-    });
-
-    const diffCheck = await this.checks.diff({
-      conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig,
-      includeUrlChanges: {
-        schemasBefore: schemaVersionToCompareAgainst?.schemas ?? [],
-        schemasAfter: schemas,
-      },
-      filterOutFederationChanges: project.type === ProjectType.FEDERATION,
-      approvedChanges: null,
-      existingSdl: previousVersionSdl,
-      incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
-    });
+    const [previousVersionSdl, previousComposableVersionSdl] = await Promise.all([
+      this.checks.retrievePreviousVersionSdl({
+        orchestrator,
+        version: schemaVersionToCompareAgainst,
+        organization,
+        project,
+        targetId: target.id,
+      }),
+      latestComposable
+        ? this.checks.retrievePreviousVersionSdl({
+            orchestrator,
+            version: latestComposable,
+            organization,
+            project,
+            targetId: target.id,
+          })
+        : null,
+    ]);
 
-    const contractChecks = await this.getContractChecks({
-      contracts,
-      compositionCheck,
-      conditionalBreakingChangeDiffConfig,
-    });
+    const [diffCheck, coordinatesDiff, contractChecks] = await Promise.all([
+      this.checks.diff({
+        conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig,
+        includeUrlChanges: {
+          schemasBefore: schemaVersionToCompareAgainst?.schemas ?? [],
+          schemasAfter: schemas,
+        },
+        filterOutFederationChanges: project.type === ProjectType.FEDERATION,
+        approvedChanges: null,
+        existingSdl: previousVersionSdl,
+        incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
+      }),
+      this.checks.coordinatesDiff({
+        existingComposableSchema: previousComposableVersionSdl,
+        incomingComposableSchema: compositionCheck.result?.fullSchemaSdl ?? null,
+      }),
+      this.getContractChecks({
+        contracts,
+        compositionCheck,
+        conditionalBreakingChangeDiffConfig,
+      }),
+    ]);
 
     const messages: string[] = [];
 
@@ -483,11 +499,7 @@ export class CompositeModel {
         composable: compositionCheck.status === 'completed',
         initial: latestVersion === null,
         changes: diffCheck.result?.all ?? diffCheck.reason?.all ?? null,
-        coordinatesDiff:
-          diffCheck.result?.coordinatesDiff ??
-          diffCheck.reason?.coordinatesDiff ??
-          diffCheck.data?.coordinatesDiff ??
-          null,
+        coordinatesDiff,
         messages,
         breakingChanges: null,
         compositionErrors: compositionCheck.reason?.errors ?? null,
@@ -606,31 +618,47 @@ export class CompositeModel {
         })) ?? null,
     });
 
-    const previousVersionSdl = await this.checks.retrievePreviousVersionSdl({
-      orchestrator,
-      version: compareToLatestComposable ? latestComposable : latest,
-      organization,
-      project,
-      targetId: selector.target,
-    });
-
-    const diffCheck = await this.checks.diff({
-      conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig,
-      includeUrlChanges: {
-        schemasBefore: latestVersion.schemas,
-        schemasAfter: schemas,
-      },
-      filterOutFederationChanges: project.type === ProjectType.FEDERATION,
-      approvedChanges: null,
-      existingSdl: previousVersionSdl,
-      incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
-    });
+    const [previousVersionSdl, previousComposableVersionSdl] = await Promise.all([
+      this.checks.retrievePreviousVersionSdl({
+        orchestrator,
+        version: compareToLatestComposable ? latestComposable : latest,
+        organization,
+        project,
+        targetId: selector.target,
+      }),
+      latestComposable
+        ? this.checks.retrievePreviousVersionSdl({
+            orchestrator,
+            version: latestComposable,
+            organization,
+            project,
+            targetId: selector.target,
+          })
+        : null,
+    ]);
 
-    const contractChecks = await this.getContractChecks({
-      contracts,
-      compositionCheck,
-      conditionalBreakingChangeDiffConfig,
-    });
+    const [diffCheck, coordinatesDiff, contractChecks] = await Promise.all([
+      this.checks.diff({
+        conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig,
+        includeUrlChanges: {
+          schemasBefore: latestVersion.schemas,
+          schemasAfter: schemas,
+        },
+        filterOutFederationChanges: project.type === ProjectType.FEDERATION,
+        approvedChanges: null,
+        existingSdl: previousVersionSdl,
+        incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
+      }),
+      this.checks.coordinatesDiff({
+        existingComposableSchema: previousComposableVersionSdl,
+        incomingComposableSchema: compositionCheck.result?.fullSchemaSdl ?? null,
+      }),
+      this.getContractChecks({
+        contracts,
+        compositionCheck,
+        conditionalBreakingChangeDiffConfig,
+      }),
+    ]);
 
     if (
       compositionCheck.status === 'failed' &&
@@ -693,11 +721,7 @@ export class CompositeModel {
         ...composablePartial,
         changes,
         breakingChanges,
-        coordinatesDiff:
-          diffCheck.result?.coordinatesDiff ??
-          diffCheck.reason?.coordinatesDiff ??
-          diffCheck.data?.coordinatesDiff ??
-          null,
+        coordinatesDiff,
         compositionErrors: compositionCheck.reason?.errors ?? [],
         supergraph: compositionCheck.result?.supergraph ?? null,
         tags: compositionCheck.result?.tags ?? null,

diff --git a/packages/services/api/src/modules/schema/providers/models/shared.ts b/packages/services/api/src/modules/schema/providers/models/shared.ts
@@ -208,7 +208,7 @@ export type SchemaPublishFailureReason =
       code: (typeof PublishFailureReasonCode)['BreakingChanges'];
       breakingChanges: Array<SchemaChangeType>;
       changes: Array<SchemaChangeType>;
-      coordinatesDiff: SchemaCoordinatesDiffResult;
+      coordinatesDiff: SchemaCoordinatesDiffResult | null;
     };
 
 type ContractResult = {

diff --git a/packages/services/api/src/modules/schema/providers/models/single-legacy.ts b/packages/services/api/src/modules/schema/providers/models/single-legacy.ts
@@ -144,6 +144,7 @@ export class SingleLegacyModel {
     input,
     target,
     latest,
+    latestComposable,
     project,
     organization,
     baseSchema,
@@ -158,6 +159,11 @@ export class SingleLegacyModel {
       sdl: string | null;
       schemas: [SingleSchema];
     } | null;
+    latestComposable: {
+      isComposable: boolean;
+      sdl: string | null;
+      schemas: [SingleSchema];
+    } | null;
     baseSchema: string | null;
     conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
   }): Promise<SchemaPublishResult> {
@@ -215,15 +221,26 @@ export class SingleLegacyModel {
       contracts: null,
     });
 
-    const previousVersionSdl = await this.checks.retrievePreviousVersionSdl({
-      orchestrator: this.orchestrator,
-      version: latestVersion,
-      organization,
-      project,
-      targetId: target.id,
-    });
+    const [previousVersionSdl, previousComposableVersionSdl] = await Promise.all([
+      this.checks.retrievePreviousVersionSdl({
+        orchestrator: this.orchestrator,
+        version: latestVersion,
+        organization,
+        project,
+        targetId: target.id,
+      }),
+      latestComposable
+        ? this.checks.retrievePreviousVersionSdl({
+            orchestrator: this.orchestrator,
+            version: latestComposable,
+            organization,
+            project,
+            targetId: target.id,
+          })
+        : null,
+    ]);
 
-    const [diffCheck, metadataCheck] = await Promise.all([
+    const [diffCheck, metadataCheck, coordinatesDiff] = await Promise.all([
       this.checks.diff({
         conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig,
         includeUrlChanges: false,
@@ -233,6 +250,10 @@ export class SingleLegacyModel {
         incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
       }),
       this.checks.metadata(incoming, latestVersion ? latestVersion.schemas[0] : null),
+      this.checks.coordinatesDiff({
+        existingComposableSchema: previousComposableVersionSdl,
+        incomingComposableSchema: compositionCheck.result?.fullSchemaSdl ?? null,
+      }),
     ]);
 
     const compositionErrors =
@@ -279,11 +300,7 @@ export class SingleLegacyModel {
           messages,
           changes,
           breakingChanges: breakingChanges ?? null,
-          coordinatesDiff:
-            diffCheck.result?.coordinatesDiff ??
-            diffCheck.reason?.coordinatesDiff ??
-            diffCheck.data?.coordinatesDiff ??
-            null,
+          coordinatesDiff,
           compositionErrors,
           schema: incoming,
           schemas,
@@ -309,7 +326,7 @@ export class SingleLegacyModel {
         code: PublishFailureReasonCode.BreakingChanges,
         changes: diffCheck.reason.all ?? [],
         breakingChanges: diffCheck.reason.breaking ?? [],
-        coordinatesDiff: diffCheck.reason?.coordinatesDiff ?? null,
+        coordinatesDiff,
       });
     }