Skip to content

Commit 11f7a9c

Browse files
dariuszkucsachindshinde
dariuszkuc
and
sachindshinde
authored
fix: update connector spec to allow re-entry (#3310)
Updates connector spec to follow the same patterns as other federation spec blueprints (i.e. register types/directives in the constructor and use default logic for adding them to the schema that checks whether they need to be added or not). NOTE: Support for handling input objects in the spec is severely limited and only handles `@connect` spec. For additional details on limitations see #3311. <!-- FED-809 --> --------- Co-authored-by: Sachin D. Shinde <[email protected]>
1 parent 4faa114 commit 11f7a9c

File tree

4 files changed

+546
-215
lines changed

4 files changed

+546
-215
lines changed

‎composition-js/src/__tests__/connectors.test.ts‎

Lines changed: 123 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,123 +1,155 @@
11
import { composeServices } from "../compose";
2-
import { printSchema } from "@apollo/federation-internals";
2+
import { buildSubgraph, printSchema } from "@apollo/federation-internals";
33
import { parse } from "graphql/index";
44

55
describe("connect spec and join__directive", () => {
6-
it("composes", () => {
7-
const subgraphs = [
8-
{
9-
name: "with-connectors",
10-
typeDefs: parse(`
11-
extend schema
12-
@link(
13-
url: "https://specs.apollo.dev/federation/v2.10"
14-
import: ["@key"]
15-
)
16-
@link(
17-
url: "https://specs.apollo.dev/connect/v0.1"
18-
import: ["@connect", "@source"]
19-
)
20-
@source(name: "v1", http: { baseURL: "http://v1" })
6+
const subgraphSdl = `
7+
extend schema
8+
@link(
9+
url: "https://specs.apollo.dev/federation/v2.10"
10+
import: ["@key"]
11+
)
12+
@link(
13+
url: "https://specs.apollo.dev/connect/v0.1"
14+
import: ["@connect", "@source"]
15+
)
16+
@source(name: "v1", http: { baseURL: "http://v1" })
17+
18+
type Query {
19+
resources: [Resource!]!
20+
@connect(source: "v1", http: { GET: "/resources" }, selection: "")
21+
}
2122
22-
type Query {
23-
resources: [Resource!]!
24-
@connect(source: "v1", http: { GET: "/resources" }, selection: "")
25-
}
23+
type Resource @key(fields: "id") {
24+
id: ID!
25+
name: String!
26+
}
27+
`;
28+
29+
const expectedSupergraphSdl = `
30+
"schema
31+
@link(url: \\"https://specs.apollo.dev/link/v1.0\\")
32+
@link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
33+
@link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION)
34+
@join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
35+
@join__directive(graphs: [WITH_CONNECTORS], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
36+
{
37+
query: Query
38+
}
2639
27-
type Resource @key(fields: "id") {
28-
id: ID!
29-
name: String!
30-
}
31-
`),
32-
},
33-
];
40+
directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
3441
35-
const result = composeServices(subgraphs);
36-
expect(result.errors ?? []).toEqual([]);
37-
const printed = printSchema(result.schema!);
38-
expect(printed).toMatchInlineSnapshot(`
39-
"schema
40-
@link(url: \\"https://specs.apollo.dev/link/v1.0\\")
41-
@link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
42-
@link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION)
43-
@join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
44-
@join__directive(graphs: [WITH_CONNECTORS], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
45-
{
46-
query: Query
47-
}
42+
directive @join__graph(name: String!, url: String!) on ENUM_VALUE
4843
49-
directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
44+
directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
5045
51-
directive @join__graph(name: String!, url: String!) on ENUM_VALUE
46+
directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String, contextArguments: [join__ContextArgument!]) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
5247
53-
directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
48+
directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE
5449
55-
directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String, contextArguments: [join__ContextArgument!]) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
50+
directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION
5651
57-
directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE
52+
directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
5853
59-
directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION
54+
directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION
6055
61-
directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
56+
enum link__Purpose {
57+
\\"\\"\\"
58+
\`SECURITY\` features provide metadata necessary to securely resolve fields.
59+
\\"\\"\\"
60+
SECURITY
6261
63-
directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION
62+
\\"\\"\\"
63+
\`EXECUTION\` features provide metadata necessary for operation execution.
64+
\\"\\"\\"
65+
EXECUTION
66+
}
6467
65-
enum link__Purpose {
66-
\\"\\"\\"
67-
\`SECURITY\` features provide metadata necessary to securely resolve fields.
68-
\\"\\"\\"
69-
SECURITY
68+
scalar link__Import
7069
71-
\\"\\"\\"
72-
\`EXECUTION\` features provide metadata necessary for operation execution.
73-
\\"\\"\\"
74-
EXECUTION
75-
}
70+
enum join__Graph {
71+
WITH_CONNECTORS @join__graph(name: \\"with-connectors\\", url: \\"\\")
72+
}
7673
77-
scalar link__Import
74+
scalar join__FieldSet
7875
79-
enum join__Graph {
80-
WITH_CONNECTORS @join__graph(name: \\"with-connectors\\", url: \\"\\")
81-
}
76+
scalar join__DirectiveArguments
8277
83-
scalar join__FieldSet
78+
scalar join__FieldValue
8479
85-
scalar join__DirectiveArguments
80+
input join__ContextArgument {
81+
name: String!
82+
type: String!
83+
context: String!
84+
selection: join__FieldValue!
85+
}
8686
87-
scalar join__FieldValue
87+
type Query
88+
@join__type(graph: WITH_CONNECTORS)
89+
{
90+
resources: [Resource!]! @join__directive(graphs: [WITH_CONNECTORS], name: \\"connect\\", args: {source: \\"v1\\", http: {GET: \\"/resources\\"}, selection: \\"\\"})
91+
}
8892
89-
input join__ContextArgument {
90-
name: String!
91-
type: String!
92-
context: String!
93-
selection: join__FieldValue!
94-
}
93+
type Resource
94+
@join__type(graph: WITH_CONNECTORS, key: \\"id\\")
95+
{
96+
id: ID!
97+
name: String!
98+
}"
99+
`;
100+
101+
const expectedApiSdl = `
102+
"type Query {
103+
resources: [Resource!]!
104+
}
95105
96-
type Query
97-
@join__type(graph: WITH_CONNECTORS)
98-
{
99-
resources: [Resource!]! @join__directive(graphs: [WITH_CONNECTORS], name: \\"connect\\", args: {source: \\"v1\\", http: {GET: \\"/resources\\"}, selection: \\"\\"})
100-
}
106+
type Resource {
107+
id: ID!
108+
name: String!
109+
}"
110+
`;
101111

102-
type Resource
103-
@join__type(graph: WITH_CONNECTORS, key: \\"id\\")
112+
it("composes", () => {
113+
const subgraphs = [
104114
{
105-
id: ID!
106-
name: String!
107-
}"
108-
`);
115+
name: "with-connectors",
116+
typeDefs: parse(subgraphSdl),
117+
},
118+
];
119+
120+
const result = composeServices(subgraphs);
121+
expect(result.errors ?? []).toEqual([]);
122+
const printed = printSchema(result.schema!);
123+
expect(printed).toMatchInlineSnapshot(expectedSupergraphSdl);
109124

110125
if (result.schema) {
111-
expect(printSchema(result.schema.toAPISchema())).toMatchInlineSnapshot(`
112-
"type Query {
113-
resources: [Resource!]!
114-
}
126+
expect(printSchema(result.schema.toAPISchema()))
127+
.toMatchInlineSnapshot(expectedApiSdl);
128+
}
129+
});
115130

116-
type Resource {
117-
id: ID!
118-
name: String!
119-
}"
120-
`);
131+
it("composes with completed definitions", () => {
132+
const completedSubgraphSdl = printSchema(buildSubgraph(
133+
"with-connectors",
134+
"",
135+
subgraphSdl,
136+
).schema);
137+
138+
const subgraphs = [
139+
{
140+
name: "with-connectors",
141+
typeDefs: parse(completedSubgraphSdl),
142+
},
143+
];
144+
145+
const result = composeServices(subgraphs);
146+
expect(result.errors ?? []).toEqual([]);
147+
const printed = printSchema(result.schema!);
148+
expect(printed).toMatchInlineSnapshot(expectedSupergraphSdl);
149+
150+
if (result.schema) {
151+
expect(printSchema(result.schema.toAPISchema()))
152+
.toMatchInlineSnapshot(expectedApiSdl);
121153
}
122154
});
123155

‎internals-js/src/buildSchema.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ import {
5656
} from "./definitions";
5757
import { ERRORS, errorCauses, withModifiedErrorNodes } from "./error";
5858
import { introspectionTypeNames } from "./introspection";
59+
import { coreFeatureDefinitionIfKnown } from "./knownCoreFeatures";
60+
import { connectIdentity } from "./specs/connectSpec";
61+
5962

6063
function buildValue(value?: ValueNode): any {
6164
return value ? valueFromASTUntyped(value) : undefined;
@@ -143,6 +146,48 @@ export function buildSchemaFromAST(
143146
buildSchemaDefinitionInner(schemaExtension, schema.schemaDefinition, errors, schema.schemaDefinition.newExtension());
144147
}
145148

149+
// The following block of code is a one-off to support input objects in the
150+
// connect spec. It will be non-maintainable/bug-prone to do this again, and
151+
// has various limitations/unsupported edge cases already.
152+
//
153+
// There's work to be done to support input objects more generally; please see
154+
// https://github.com/apollographql/federation/pull/3311 for more information.
155+
const connectFeature = schema.coreFeatures?.getByIdentity(connectIdentity);
156+
const handledConnectTypeNames = new Set<string>();
157+
if (connectFeature) {
158+
const connectFeatureDefinition =
159+
coreFeatureDefinitionIfKnown(connectFeature.url);
160+
if (connectFeatureDefinition) {
161+
const connectTypeNamesInSchema = new Set(
162+
connectFeatureDefinition.typeSpecs()
163+
.map(({ name }) => connectFeature.typeNameInSchema(name))
164+
);
165+
for (const typeNode of typeDefinitions) {
166+
if (connectTypeNamesInSchema.has(typeNode.name.value)
167+
&& typeNode.kind === 'InputObjectTypeDefinition'
168+
) {
169+
handledConnectTypeNames.add(typeNode.name.value)
170+
} else {
171+
continue;
172+
}
173+
buildNamedTypeInner(typeNode, schema.type(typeNode.name.value)!, schema.blueprint, errors);
174+
}
175+
for (const typeExtensionNode of typeExtensions) {
176+
if (connectTypeNamesInSchema.has(typeExtensionNode.name.value)
177+
&& typeExtensionNode.kind === 'InputObjectTypeExtension'
178+
) {
179+
handledConnectTypeNames.add(typeExtensionNode.name.value)
180+
} else {
181+
continue;
182+
}
183+
const toExtend = schema.type(typeExtensionNode.name.value)!;
184+
const extension = toExtend.newExtension();
185+
extension.sourceAST = typeExtensionNode;
186+
buildNamedTypeInner(typeExtensionNode, toExtend, schema.blueprint, errors, extension);
187+
}
188+
}
189+
}
190+
146191
// The following is a no-op for "standard" schema, but for federation subgraphs, this is where we handle the auto-addition
147192
// of imported federation directive definitions. That is why we have avoid looking at directive applications within
148193
// directive definition earlier: if one of those application was of an imported federation directive, the definition
@@ -155,9 +200,15 @@ export function buildSchemaFromAST(
155200
}
156201

157202
for (const typeNode of typeDefinitions) {
203+
if (handledConnectTypeNames.has(typeNode.name.value)) {
204+
continue;
205+
}
158206
buildNamedTypeInner(typeNode, schema.type(typeNode.name.value)!, schema.blueprint, errors);
159207
}
160208
for (const typeExtensionNode of typeExtensions) {
209+
if (handledConnectTypeNames.has(typeExtensionNode.name.value)) {
210+
continue;
211+
}
161212
const toExtend = schema.type(typeExtensionNode.name.value)!;
162213
const extension = toExtend.newExtension();
163214
extension.sourceAST = typeExtensionNode;

‎internals-js/src/directiveAndTypeSpecification.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,13 @@ export type FieldSpecification = {
6666
args?: ResolvedArgumentSpecification[],
6767
}
6868

69-
type ResolvedArgumentSpecification = {
69+
export type ResolvedArgumentSpecification = {
70+
name: string,
71+
type: InputType,
72+
defaultValue?: any,
73+
}
74+
75+
export type InputFieldSpecification = {
7076
name: string,
7177
type: InputType,
7278
defaultValue?: any,
@@ -338,7 +344,7 @@ export function createEnumTypeSpecification({
338344
}
339345
}
340346

341-
function ensureSameTypeKind(expected: NamedType['kind'], actual: NamedType): GraphQLError[] {
347+
export function ensureSameTypeKind(expected: NamedType['kind'], actual: NamedType): GraphQLError[] {
342348
return expected === actual.kind
343349
? []
344350
: [

0 commit comments

Comments
 (0)