Skip to content

Commit ec0ef84

Browse files
jdollegemini-code-assist[bot]kamilkisiela
authored
Support file: protocol in link urls (#291)
File protocols are valid RFC 3986 and should be supported. Also adds support for non-RFC 3986 per the federation link spec --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>
1 parent e507fdd commit ec0ef84

8 files changed

Lines changed: 258 additions & 136 deletions

File tree

‎.changeset/eager-lilies-walk.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@theguild/federation-composition": patch
3+
---
4+
5+
Support "file:" protocol and non-RFC 3986 in imported "link" url argument

‎__tests__/composition.spec.ts‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8452,43 +8452,43 @@ testImplementations((api) => {
84528452
});
84538453

84548454
test("@external on non-key field in Federation v1", () => {
8455-
let result = api.composeServices([
8456-
{
8457-
name: "users",
8458-
typeDefs: parse(/* GraphQL */ `
8459-
type Query {
8460-
users: [User]
8461-
}
8455+
let result = api.composeServices([
8456+
{
8457+
name: "users",
8458+
typeDefs: parse(/* GraphQL */ `
8459+
type Query {
8460+
users: [User]
8461+
}
84628462
8463-
type User @key(fields: "id") {
8464-
id: ID!
8465-
name: String! @external
8466-
}
8467-
`),
8468-
},
8469-
{
8470-
name: "extra",
8471-
typeDefs: parse(/* GraphQL */ `
8472-
type User @key(fields: "id") {
8473-
id: ID!
8474-
name: String!
8475-
}
8476-
`),
8477-
},
8478-
]);
8463+
type User @key(fields: "id") {
8464+
id: ID!
8465+
name: String! @external
8466+
}
8467+
`),
8468+
},
8469+
{
8470+
name: "extra",
8471+
typeDefs: parse(/* GraphQL */ `
8472+
type User @key(fields: "id") {
8473+
id: ID!
8474+
name: String!
8475+
}
8476+
`),
8477+
},
8478+
]);
84798479

8480-
assertCompositionSuccess(result);
8481-
expect(result.supergraphSdl).toContainGraphQL(/* GraphQL */ `
8482-
type User
8483-
@join__type(graph: EXTRA, key: "id")
8484-
@join__type(graph: USERS, key: "id") {
8485-
id: ID!
8486-
name: String! @join__field(graph: EXTRA)
8487-
}
8488-
`);
8489-
});
8480+
assertCompositionSuccess(result);
8481+
expect(result.supergraphSdl).toContainGraphQL(/* GraphQL */ `
8482+
type User
8483+
@join__type(graph: EXTRA, key: "id")
8484+
@join__type(graph: USERS, key: "id") {
8485+
id: ID!
8486+
name: String! @join__field(graph: EXTRA)
8487+
}
8488+
`);
8489+
});
84908490

8491-
test("@external on non-key field in Federation v1", () => {
8491+
test("@external on non-key field in Federation v1", () => {
84928492
let result = api.composeServices([
84938493
{
84948494
name: "users",

‎__tests__/subgraph/link-directive.spec.ts‎

Lines changed: 124 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,52 +12,57 @@ testVersions((api, version) => {
1212
{
1313
name: "billing",
1414
typeDefs: graphql`
15-
extend schema
16-
@link(
17-
url: "https://specs.apollo.dev/federation/${version}"
18-
import: ["@override", "@external", "@provides"]
19-
)
20-
@link(url: "https://specs.apollo.dev", import: [{ name: "@key", as: "@renamed" }])
15+
extend schema
16+
@link(
17+
url: "https://specs.apollo.dev/federation/${version}"
18+
import: ["@override", "@external", "@provides"]
19+
)
20+
@link(url: "https://specs.apollo.dev", import: [{ name: "@key", as: "@renamed" }])
2121
22-
extend type Payment @key(fields: "id") {
23-
id: ID!
24-
amount: Int! @external
25-
}
22+
extend type Payment @key(fields: "id") {
23+
id: ID!
24+
amount: Int! @external
25+
}
2626
27-
type Invoice @key(fields: "id") {
28-
id: ID!
29-
amount: Int!
30-
payment: Payment
31-
}
32-
`,
27+
type Invoice @key(fields: "id") {
28+
id: ID!
29+
amount: Int!
30+
payment: Payment
31+
}
32+
`,
3333
},
3434
{
3535
name: "payments",
3636
typeDefs: graphql`
37-
extend schema
38-
@link(
39-
url: "https://specs.apollo.dev/federation/${version}"
40-
import: [{ name: "@key", as: "@renamed" }]
41-
)
37+
extend schema
38+
@link(
39+
url: "https://specs.apollo.dev/federation/${version}"
40+
import: [{ name: "@key", as: "@renamed" }]
41+
)
4242
43-
type Query {
44-
payments: [Payment]
45-
}
43+
type Query {
44+
payments: [Payment]
45+
}
4646
47-
type Payment @renamed(fields: "id") {
48-
id: ID!
49-
amount: Int!
50-
}
51-
`,
47+
type Payment @renamed(fields: "id") {
48+
id: ID!
49+
amount: Int!
50+
}
51+
`,
5252
},
5353
]),
5454
).toEqual(
5555
expect.objectContaining({
5656
errors: expect.arrayContaining([
5757
expect.objectContaining({
58-
message: expect.stringMatching(
59-
"Missing path in feature url 'https://specs.apollo.dev",
60-
),
58+
message:
59+
api.library === "guild"
60+
? expect.stringContaining(
61+
"Invalid @link url 'https://specs.apollo.dev': expected '/<feature>/vX.Y' (for example '/federation/v2.9')",
62+
)
63+
: expect.stringContaining(
64+
"Missing path in feature url 'https://specs.apollo.dev/'",
65+
),
6166
extensions: expect.objectContaining({
6267
code: "INVALID_LINK_IDENTIFIER",
6368
}),
@@ -73,16 +78,14 @@ testVersions((api, version) => {
7378
typeDefs: graphql`
7479
extend schema
7580
@link(
76-
url: "https://specs.apollo.dev/federation/not-a-version"
77-
import: ["@override", "@external", "@provides"]
78-
)
79-
@link(
80-
url: "https://specs.apollo.dev"
81-
import: [{ name: "@key", as: "@renamed" }]
81+
url: "https://specs.apollo.dev/federation/${version}"
82+
import: ["@override", "@external", "@provides", "@key", "@requires"]
8283
)
84+
@link(url: "https://specs.apollo.dev/federation/not-a-version", import: [{ name: "@key", as: "@renamed" }])
8385
8486
extend type Payment @key(fields: "id") {
8587
id: ID!
88+
tax: Int! @requires(fields: "amount")
8689
amount: Int! @external
8790
}
8891
@@ -98,7 +101,7 @@ testVersions((api, version) => {
98101
typeDefs: graphql`
99102
extend schema
100103
@link(
101-
url: "https://specs.apollo.dev/federation/not-a-version"
104+
url: "https://specs.apollo.dev/federation/${version}"
102105
import: [{ name: "@key", as: "@renamed" }]
103106
)
104107
@@ -118,15 +121,7 @@ testVersions((api, version) => {
118121
errors: expect.arrayContaining([
119122
expect.objectContaining({
120123
message: expect.stringContaining(
121-
"[billing] Expected a version string (of the form v1.2), got not-a-version",
122-
),
123-
extensions: expect.objectContaining({
124-
code: "INVALID_LINK_IDENTIFIER",
125-
}),
126-
}),
127-
expect.objectContaining({
128-
message: expect.stringContaining(
129-
"[payments] Expected a version string (of the form v1.2), got not-a-version",
124+
"Expected a version string (of the form v1.2), got", // Hive correctly says "got null", apollo says "got not-a-version"
130125
),
131126
extensions: expect.objectContaining({
132127
code: "INVALID_LINK_IDENTIFIER",
@@ -201,7 +196,7 @@ testVersions((api, version) => {
201196
);
202197
});
203198

204-
test(" INVALID_LINK_DIRECTIVE_USAGE: unknown element", () => {
199+
test("INVALID_LINK_DIRECTIVE_USAGE: unknown element", () => {
205200
expect(
206201
api.composeServices([
207202
{
@@ -337,4 +332,84 @@ testVersions((api, version) => {
337332
]),
338333
);
339334
});
335+
336+
/**
337+
* If url: is not a valid RFC 3986 url, then it MUST be treated as an opaque
338+
* identifier for the foreign schema. Such non‐URL inputs to url: SHOULD
339+
* NOT have name and version information extracted from them—both are null.
340+
* source: https://specs.apollo.dev/link/v1.0/#@link.url
341+
*/
342+
api.runIf("guild", () => {
343+
test("Non-RFC 3986 url", () => {
344+
let result = api.composeServices([
345+
{
346+
name: "billing",
347+
typeDefs: graphql`
348+
extend schema
349+
@link(
350+
url: "https://specs.apollo.dev/federation/${version}"
351+
import: ["@override", "@external", "@provides", "@key", "@requires", "@shareable"]
352+
)
353+
@link(url: "file:///foo/bar", import: ["Bar"])
354+
355+
scalar Bar
356+
357+
extend type Payment @key(fields: "id") {
358+
id: ID!
359+
tax: Int! @requires(fields: "amount")
360+
amount: Int! @external
361+
bar: Bar @shareable
362+
}
363+
364+
type Invoice @key(fields: "id") {
365+
id: ID!
366+
amount: Int!
367+
payment: Payment
368+
}
369+
`,
370+
},
371+
{
372+
name: "payments",
373+
typeDefs: graphql`
374+
extend schema
375+
@link(
376+
url: "https://specs.apollo.dev/federation/${version}"
377+
import: [{ name: "@key", as: "@renamed" }, "@shareable"]
378+
)
379+
@link(url: "file:///foo/bar", import: ["Bar"])
380+
381+
scalar Bar
382+
383+
type Query {
384+
payments: [Payment]
385+
}
386+
387+
type Payment @renamed(fields: "id") {
388+
id: ID!
389+
amount: Int!
390+
bar: Bar @shareable
391+
}
392+
`,
393+
},
394+
]);
395+
assertCompositionSuccess(result);
396+
397+
expect(result.supergraphSdl).toContainGraphQL(/* GraphQL */ `
398+
scalar Bar @join__type(graph: BILLING) @join__type(graph: PAYMENTS)
399+
`);
400+
401+
expect(result.supergraphSdl).toContainGraphQL(/* GraphQL */ `
402+
type Payment
403+
@join__type(graph: BILLING, key: "id", extension: true)
404+
@join__type(graph: PAYMENTS, key: "id") {
405+
id: ID!
406+
tax: Int! @join__field(graph: BILLING, requires: "amount")
407+
amount: Int!
408+
@join__field(graph: BILLING, external: true)
409+
@join__field(graph: PAYMENTS)
410+
bar: Bar
411+
}
412+
`);
413+
});
414+
});
340415
});

‎__tests__/utils/link-url.spec.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ describe("FederatedLinkUrl", () => {
1818
],
1919
["https://spec.example.com/v1.0", "https://spec.example.com", null, "v1.0"],
2020
["https://spec.example.com/vX", "https://spec.example.com/vX", "vX", null],
21+
[
22+
"file:///extensions/permissions",
23+
"file:///extensions/permissions",
24+
"permissions",
25+
null,
26+
],
2127
])(
2228
"fromUrl correctly parses the identity, name, and version",
2329
(url, identity, name, version) => {
@@ -28,6 +34,12 @@ describe("FederatedLinkUrl", () => {
2834
},
2935
);
3036

37+
test("fromUrl errors on invalid URL parses the identity, name, and version", () => {
38+
expect(() => FederatedLinkUrl.fromUrl("spec.example.com")).toThrow(
39+
`Invalid URL`,
40+
);
41+
});
42+
3143
test.each([
3244
[
3345
"https://spec.example.com/a/b/mySchema/v1.2/",

‎src/specifications/link.spec.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,19 @@ import { parseLinkImport, parseLinkUrl } from "./link.js";
33

44
describe("parseLinkUrl", () => {
55
test.each`
6-
url | name | version | identity
7-
${"https://spec.example.com/a/b/mySchema/v1.0/"} | ${"mySchema"} | ${"v1.0"} | ${"https://spec.example.com/a/b/mySchema"}
8-
${"https://spec.example.com/a/b/mySchema/v1.0"} | ${"mySchema"} | ${"v1.0"} | ${"https://spec.example.com/a/b/mySchema"}
9-
${"https://spec.example.com"} | ${null} | ${null} | ${"https://spec.example.com"}
10-
${"https://spec.example.com/mySchema/v0.1?q=v#frag"} | ${"mySchema"} | ${"v0.1"} | ${"https://spec.example.com/mySchema"}
11-
${"https://spec.example.com/mySchema/not-a-version"} | ${"mySchema"} | ${"not-a-version"} | ${"https://spec.example.com/mySchema"}
12-
${"https://spec.example.com/v1.0"} | ${null} | ${"v1.0"} | ${"https://spec.example.com"}
13-
${"https://spec.example.com/vX"} | ${"vX"} | ${null} | ${"https://spec.example.com/vX"}
6+
url | name | version | identity
7+
${"https://spec.example.com/a/b/mySchema/v1.0/"} | ${"mySchema"} | ${"v1.0"} | ${"https://spec.example.com/a/b/mySchema"}
8+
${"https://spec.example.com/a/b/mySchema/v1.0"} | ${"mySchema"} | ${"v1.0"} | ${"https://spec.example.com/a/b/mySchema"}
9+
${"https://spec.example.com"} | ${null} | ${null} | ${"https://spec.example.com"}
10+
${"https://spec.example.com/mySchema/v0.1?q=v#frag"} | ${"mySchema"} | ${"v0.1"} | ${"https://spec.example.com/mySchema"}
11+
${"https://spec.example.com/mySchema/not-a-version"} | ${"not-a-version"} | ${null} | ${"https://spec.example.com/mySchema/not-a-version"}
12+
${"https://spec.example.com/v1.0"} | ${null} | ${"v1.0"} | ${"https://spec.example.com"}
13+
${"https://spec.example.com/vX"} | ${"vX"} | ${null} | ${"https://spec.example.com/vX"}
14+
${"file:///extensions/permissions"} | ${"permissions"} | ${null} | ${"file:///extensions/permissions"}
15+
${"https://specs.apollo.dev/federation/not-a-version"} | ${"not-a-version"} | ${null} | ${"https://specs.apollo.dev/federation/not-a-version"}
16+
${"https://specs.apollo.dev"} | ${null} | ${null} | ${"https://specs.apollo.dev"}
17+
${"specs.apollo.dev"} | ${null} | ${null} | ${"specs.apollo.dev"}
18+
${"not-valid.apollo.dev/v1"} | ${null} | ${null} | ${"not-valid.apollo.dev/v1"}
1419
`("$url", ({ url, name, version, identity }) => {
1520
expect(parseLinkUrl(url)).toEqual({ name, version, identity });
1621
});

0 commit comments

Comments
 (0)