Skip to content

Commit

Permalink
Merge pull request #1831 from pcmanus/1824-print-schema-fixes
Browse files Browse the repository at this point in the history
Fix `printSubgraphSchema` method.
  • Loading branch information
Sylvain Lebresne authored May 9, 2022
2 parents 5be2d72 + 96cbf21 commit 6480e5c
Show file tree
Hide file tree
Showing 20 changed files with 802 additions and 568 deletions.
4 changes: 0 additions & 4 deletions composition-js/src/__tests__/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ describe('composition', () => {

expect(subgraphs.get('subgraphA')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand All @@ -287,7 +286,6 @@ describe('composition', () => {

expect(subgraphs.get('subgraphB')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down Expand Up @@ -343,7 +341,6 @@ describe('composition', () => {
// Of course, the federation directives should be rebuilt in the extracted subgraphs.
expect(subgraphs.get('subgraphA')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand All @@ -363,7 +360,6 @@ describe('composition', () => {

expect(subgraphs.get('subgraphB')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down
7 changes: 0 additions & 7 deletions composition-js/src/__tests__/composeFed1Subgraphs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ describe('basic type extensions', () => {

expect(subgraphs.get('subgraphA')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand All @@ -89,7 +88,6 @@ describe('basic type extensions', () => {
// an extension.
expect(subgraphs.get('subgraphB')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down Expand Up @@ -150,7 +148,6 @@ describe('basic type extensions', () => {
// Same remark than in prevoius test
expect(subgraphs.get('subgraphA')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand All @@ -166,7 +163,6 @@ describe('basic type extensions', () => {

expect(subgraphs.get('subgraphB')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down Expand Up @@ -239,7 +235,6 @@ describe('basic type extensions', () => {

expect(subgraphs.get('subgraphA')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand All @@ -255,7 +250,6 @@ describe('basic type extensions', () => {

expect(subgraphs.get('subgraphB')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand All @@ -273,7 +267,6 @@ describe('basic type extensions', () => {

expect(subgraphs.get('subgraphC')!.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down
1 change: 1 addition & 0 deletions gateway-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The
## vNext

- Fix bug with type extension of empty type definition [PR #1821](https://github.com/apollographql/federation/pull/1821)
- Fix output of `printSubgraphSchema` method, ensuring it can be read back by composition and `buildSubgraphSchema` [PR #1831](https://github.com/apollographql/federation/pull/1831).

## 2.0.2-alpha.2

Expand Down
2 changes: 1 addition & 1 deletion gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('lifecycle hooks', () => {
// the supergraph (even just formatting differences), this ID will change
// and this test will have to updated.
expect(secondCall[0]!.compositionId).toEqual(
'730a2fe4036db8e2c847096ba2a62f78ff8f3c08c9ee092a5b1b37e1aa00ef5a',
'68b144680261e3b57be90f127f35c88d0ce6cb7aaadf04d0a5d60fedcc2500b3',
);
// second call should have previous info in the second arg
expect(secondCall[1]!.compositionId).toEqual(expectedFirstId);
Expand Down
156 changes: 156 additions & 0 deletions internals-js/src/__tests__/graphQLJSSchemaToAST.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import {
buildClientSchema,
buildSchema,
GraphQLSchema,
introspectionFromSchema,
print
} from "graphql";
import { graphQLJSSchemaToAST } from "../graphQLJSSchemaToAST";
import './matchers';

function validateRoundtrip(schemaStr: string, expectedWithoutASTNodes: string | undefined = schemaStr) {
const schema = buildSchema(schemaStr);
expect(print(graphQLJSSchemaToAST(schema))).toMatchString(schemaStr);
if (expectedWithoutASTNodes) {
expect(print(graphQLJSSchemaToAST(withoutASTNodes(schema)))).toMatchString(expectedWithoutASTNodes);
}
}

function withoutASTNodes(schema: GraphQLSchema): GraphQLSchema {
return buildClientSchema(introspectionFromSchema(schema));
}

it('round-trip for all type definitions', () => {
const schema = `
type Query {
a: A
b: B
c: C
d(arg: D): Int
}
interface I {
x: Int
}
type A implements I {
x: Int
y: Int
}
union B = A | Query
enum C {
V1
V2
}
input D {
m: Int
n: Int = 3
}
`;

validateRoundtrip(schema);
});

it('round-trip with default arguments', () => {
const schemaFct = (v: string) => `
type Query {
f(arg: V = ${v}): Int
}
input V {
x: Int
y: Int = 3
}
`;

const schema = schemaFct('{x: 2}');
// We go through introspection to ensure the AST nodes are
// removed, but that also somehow expand default values (which is
// fine, we just have to account for it in our assertion).
const schemaWithDefaultExpanded = schemaFct('{x: 2, y: 3}');

validateRoundtrip(schema, schemaWithDefaultExpanded);
});

it('round-trip for directive definitions and applications', () => {
const directiveDefinitions = `directive @schemaDirective(v: Int!) on SCHEMA
directive @typeDirective repeatable on OBJECT
directive @fieldDirective(s: String, m: Int = 3) on FIELD_DEFINITION
`;

const schema = `
schema @schemaDirective(v: 3) {
query: Query
}
type Query @typeDirective @typeDirective {
f: Int @fieldDirective(s: "foo")
g: Int @deprecated
}
${directiveDefinitions}
`;

// With the ast nodes removed, we lose custom directive applications
const noApplications = `
type Query {
f: Int
g: Int @deprecated
}
${directiveDefinitions}
`;

validateRoundtrip(schema, noApplications);
});

it('round-trip with extensions', () => {
const common = `scalar federation_FieldSet
scalar link_Import
directive @link(url: String!, import: link_Import) on SCHEMA
directive @key(fields: federation_FieldSet) repeatable on OBJECT
`;

const schema = `
extend schema @link(url: "https://specs.apollo.dev", import: ["@key"])
type Query {
t: T
}
type T
extend type T @key(fields: "id") {
id: ID!
x: Int
}
${common}
`;

// No AST means we lose both the directive applications, but also whether something is an
// extension or not.
const noAST = `
type Query {
t: T
}
type T {
id: ID!
x: Int
}
${common}
`;

validateRoundtrip(schema, noAST);
});

2 changes: 0 additions & 2 deletions internals-js/src/__tests__/schemaUpgrader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ test('upgrade complex schema', () => {

expect(res.subgraphs?.get('s1')?.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down Expand Up @@ -149,7 +148,6 @@ test('update federation directive non-string arguments', () => {

expect(res.subgraphs?.get('s')?.toString()).toMatchString(`
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
${FEDERATION2_LINK_WTH_FULL_IMPORTS}
{
query: Query
Expand Down
34 changes: 34 additions & 0 deletions internals-js/src/__tests__/subgraphValidation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,19 @@ describe('@core/@link handling', () => {
directive @federation__external(reason: String) on OBJECT | FIELD_DEFINITION
`,
gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.0")
type T {
k: ID!
}
enum link__Purpose {
EXECUTION
SECURITY
}
`,
];

// Note that we cannot use `validateFullSchema` as-is for those examples because the order or directive is going
Expand Down Expand Up @@ -862,6 +875,27 @@ describe('@core/@link handling', () => {
]]);
});

it('errors on invalid definition for @link Purpose', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.0")
type T {
k: ID!
}
enum link__Purpose {
EXECUTION
RANDOM
}
`;

expect(buildForErrors(doc, { asFed2: false })).toStrictEqual([[
'TYPE_DEFINITION_INVALID',
'[S] Invalid definition for type "Purpose": expected values [EXECUTION, SECURITY] but found [EXECUTION, RANDOM].',
]]);
});

it('allows any (non-scalar) type in redefinition when expected type is a scalar', () => {
const doc = gql`
extend schema
Expand Down
Loading

0 comments on commit 6480e5c

Please sign in to comment.