Skip to content

Commit 4bda3a4

Browse files
authored
fix: fixed demand control validations (#3314)
Validations are run against subgraphs so we need to verify`@cost`/`@listSize` from federation spec and not the supergraph cost spec.
1 parent 05754da commit 4bda3a4

File tree

4 files changed

+22
-40
lines changed

4 files changed

+22
-40
lines changed

.changeset/lemon-toes-sort.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@apollo/federation-internals": patch
3+
---
4+
5+
Fixed demand control validations
6+
7+
Updated `@cost`/`@listSize` validations to use correct federation spec to look them up in the schema.

internals-js/src/__tests__/subgraphValidation.test.ts

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,9 +1488,6 @@ describe('@interfaceObject/@key on interfaces validation', () => {
14881488
describe('@cost', () => {
14891489
it('rejects applications on interfaces', () => {
14901490
const doc = gql`
1491-
extend schema
1492-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@cost"])
1493-
14941491
type Query {
14951492
a: A
14961493
}
@@ -1500,7 +1497,7 @@ describe('@cost', () => {
15001497
}
15011498
`;
15021499

1503-
expect(buildForErrors(doc)).toStrictEqual([
1500+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
15041501
[
15051502
'COST_APPLIED_TO_INTERFACE_FIELD',
15061503
`[S] @cost cannot be applied to interface "A.x"`,
@@ -1512,9 +1509,6 @@ describe('@cost', () => {
15121509
describe('@listSize', () => {
15131510
it('rejects applications on non-lists (unless it uses sizedFields)', () => {
15141511
const doc = gql`
1515-
extend schema
1516-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1517-
15181512
type Query {
15191513
a1: A @listSize(assumedSize: 5)
15201514
a2: A @listSize(assumedSize: 10, sizedFields: ["ints"])
@@ -1525,23 +1519,20 @@ describe('@listSize', () => {
15251519
}
15261520
`;
15271521

1528-
expect(buildForErrors(doc)).toStrictEqual([
1522+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
15291523
['LIST_SIZE_APPLIED_TO_NON_LIST', `[S] "Query.a1" is not a list`],
15301524
]);
15311525
});
15321526

15331527
it('rejects negative assumedSize', () => {
15341528
const doc = gql`
1535-
extend schema
1536-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1537-
15381529
type Query {
15391530
a: [Int] @listSize(assumedSize: -5)
15401531
b: [Int] @listSize(assumedSize: 0)
15411532
}
15421533
`;
15431534

1544-
expect(buildForErrors(doc)).toStrictEqual([
1535+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
15451536
[
15461537
'LIST_SIZE_INVALID_ASSUMED_SIZE',
15471538
`[S] Assumed size of "Query.a" cannot be negative`,
@@ -1551,9 +1542,6 @@ describe('@listSize', () => {
15511542

15521543
it('rejects slicingArguments which are not arguments of the field', () => {
15531544
const doc = gql`
1554-
extend schema
1555-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1556-
15571545
type Query {
15581546
myField(something: Int): [String]
15591547
@listSize(slicingArguments: ["missing1", "missing2"])
@@ -1562,7 +1550,7 @@ describe('@listSize', () => {
15621550
}
15631551
`;
15641552

1565-
expect(buildForErrors(doc)).toStrictEqual([
1553+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
15661554
[
15671555
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
15681556
`[S] Slicing argument "missing1" is not an argument of "Query.myField"`,
@@ -1580,9 +1568,6 @@ describe('@listSize', () => {
15801568

15811569
it('rejects slicingArguments which are not Int or Int!', () => {
15821570
const doc = gql`
1583-
extend schema
1584-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1585-
15861571
type Query {
15871572
sliced(
15881573
first: String
@@ -1597,7 +1582,7 @@ describe('@listSize', () => {
15971582
}
15981583
`;
15991584

1600-
expect(buildForErrors(doc)).toStrictEqual([
1585+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
16011586
[
16021587
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
16031588
`[S] Slicing argument "Query.sliced(first:)" must be Int or Int!`,
@@ -1615,9 +1600,6 @@ describe('@listSize', () => {
16151600

16161601
it('rejects sizedFields when the output type is not an object', () => {
16171602
const doc = gql`
1618-
extend schema
1619-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1620-
16211603
type Query {
16221604
notObject: Int @listSize(assumedSize: 1, sizedFields: ["anything"])
16231605
a: A @listSize(assumedSize: 5, sizedFields: ["ints"])
@@ -1633,7 +1615,7 @@ describe('@listSize', () => {
16331615
}
16341616
`;
16351617

1636-
expect(buildForErrors(doc)).toStrictEqual([
1618+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
16371619
[
16381620
'LIST_SIZE_INVALID_SIZED_FIELD',
16391621
`[S] Sized fields cannot be used because "Int" is not a composite type`,
@@ -1643,9 +1625,6 @@ describe('@listSize', () => {
16431625

16441626
it('rejects sizedFields which are not fields of the output type', () => {
16451627
const doc = gql`
1646-
extend schema
1647-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1648-
16491628
type Query {
16501629
a: A @listSize(assumedSize: 5, sizedFields: ["notOnA"])
16511630
}
@@ -1655,7 +1634,7 @@ describe('@listSize', () => {
16551634
}
16561635
`;
16571636

1658-
expect(buildForErrors(doc)).toStrictEqual([
1637+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
16591638
[
16601639
'LIST_SIZE_INVALID_SIZED_FIELD',
16611640
`[S] Sized field "notOnA" is not a field on type "A"`,
@@ -1665,9 +1644,6 @@ describe('@listSize', () => {
16651644

16661645
it('rejects sizedFields which are not lists', () => {
16671646
const doc = gql`
1668-
extend schema
1669-
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
1670-
16711647
type Query {
16721648
a: A
16731649
@listSize(
@@ -1683,7 +1659,7 @@ describe('@listSize', () => {
16831659
}
16841660
`;
16851661

1686-
expect(buildForErrors(doc)).toStrictEqual([
1662+
expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([
16871663
[
16881664
'LIST_SIZE_APPLIED_TO_NON_LIST',
16891665
`[S] Sized field "A.notList" is not a list`,

internals-js/src/__tests__/testUtils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ export function buildForErrors(
99
options?: {
1010
subgraphName?: string,
1111
asFed2?: boolean,
12+
includeAllImports?: boolean,
1213
}
1314
): [string, string][] | undefined {
1415
try {
15-
const doc = (options?.asFed2 ?? true) ? asFed2SubgraphDocument(subgraphDefs) : subgraphDefs;
16+
const doc = (options?.asFed2 ?? true) ? asFed2SubgraphDocument(subgraphDefs, { ...options }) : subgraphDefs;
1617
const name = options?.subgraphName ?? 'S';
1718
buildSubgraph(name, `http://${name}`, doc).validate();
1819
return undefined;

internals-js/src/federation.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ import { createObjectTypeSpecification, createScalarTypeSpecification, createUni
9797
import { didYouMean, suggestionList } from "./suggestions";
9898
import { coreFeatureDefinitionIfKnown } from "./knownCoreFeatures";
9999
import { joinIdentity } from "./specs/joinSpec";
100-
import { COST_VERSIONS, CostDirectiveArguments, ListSizeDirectiveArguments, costIdentity } from "./specs/costSpec";
100+
import { CostDirectiveArguments, ListSizeDirectiveArguments } from "./specs/costSpec";
101101

102102
const linkSpec = LINK_VERSIONS.latest();
103103
const tagSpec = TAG_VERSIONS.latest();
@@ -1820,18 +1820,16 @@ export class FederationBlueprint extends SchemaBlueprint {
18201820
}
18211821
}
18221822

1823-
const costFeature = schema.coreFeatures?.getByIdentity(costIdentity);
1824-
const costSpec = costFeature && COST_VERSIONS.find(costFeature.url.version);
1825-
const costDirective = costSpec?.costDirective(schema);
1826-
const listSizeDirective = costSpec?.listSizeDirective(schema);
1823+
const costDirective = metadata.costDirective();
1824+
const listSizeDirective = metadata.listSizeDirective();
18271825

18281826
// Validate @cost
1829-
for (const application of costDirective?.applications() ?? []) {
1827+
for (const application of costDirective.applications()) {
18301828
validateCostNotAppliedToInterface(application, errorCollector);
18311829
}
18321830

18331831
// Validate @listSize
1834-
for (const application of listSizeDirective?.applications() ?? []) {
1832+
for (const application of listSizeDirective.applications()) {
18351833
const parent = application.parent;
18361834
assert(parent instanceof FieldDefinition, "@listSize can only be applied to FIELD_DEFINITION");
18371835
validateListSizeAppliedToList(application, parent, errorCollector);

0 commit comments

Comments
 (0)