Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix printSubgraphSchema method. #1831

Merged
merged 3 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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