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

Demand control directive validations #3148

Merged
merged 8 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
5 changes: 5 additions & 0 deletions .changeset/tidy-trees-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/federation-internals": patch
---

Add validations for demand control directive applications
2 changes: 1 addition & 1 deletion docs/source/federation-versions.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ First release

Minimum router version

**TBD**
**1.53.0**

</div>

Expand Down
185 changes: 185 additions & 0 deletions internals-js/src/__tests__/subgraphValidation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1484,3 +1484,188 @@ describe('@interfaceObject/@key on interfaces validation', () => {
]);
});
});

describe('@cost', () => {
it('rejects applications on interfaces', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@cost"])

type Query {
a: A
}

interface A {
x: Int @cost(weight: 10)
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'COST_APPLIED_TO_INTERFACE_FIELD',
`[S] @cost cannot be applied to interface "A.x"`,
],
]);
});
});

describe('@listSize', () => {
it('rejects applications on non-lists (unless it uses sizedFields)', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
a1: A @listSize(assumedSize: 5)
a2: A @listSize(assumedSize: 10, sizedFields: ["ints"])
}

type A {
ints: [Int]
}
`;

expect(buildForErrors(doc)).toStrictEqual([
['LIST_SIZE_APPLIED_TO_NON_LIST', `[S] "Query.a1" is not a list`],
]);
});

it('rejects negative assumedSize', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
a: [Int] @listSize(assumedSize: -5)
b: [Int] @listSize(assumedSize: 0)
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'LIST_SIZE_INVALID_ASSUMED_SIZE',
`[S] Assumed size of "Query.a" cannot be negative`,
],
]);
});

it('rejects slicingArguments which are not arguments of the field', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
myField(something: Int): [String]
@listSize(slicingArguments: ["missing1", "missing2"])
myOtherField(somethingElse: String): [Int]
@listSize(slicingArguments: ["alsoMissing"])
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
`[S] Slicing argument "missing1" is not an argument of "Query.myField"`,
],
[
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
`[S] Slicing argument "missing2" is not an argument of "Query.myField"`,
],
[
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
`[S] Slicing argument "alsoMissing" is not an argument of "Query.myOtherField"`,
],
]);
});

it('rejects slicingArguments which are not Int or Int!', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
sliced(first: String, second: Int, third: Int!): [String]
@listSize(slicingArguments: ["first", "second", "third"])
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
`[S] Slicing argument "Query.sliced(first:)" must be Int or Int!`,
],
]);
});

it('rejects sizedFields when the output type is not an object', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
notObject: Int @listSize(assumedSize: 1, sizedFields: ["anything"])
a: A @listSize(assumedSize: 5, sizedFields: ["ints"])
b: B @listSize(assumedSize: 10, sizedFields: ["ints"])
}

type A {
ints: [Int]
}

interface B {
ints: [Int]
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'LIST_SIZE_INVALID_SIZED_FIELD',
`[S] Sized fields cannot be used because "Int" is not an object type`,
],
]);
});

it('rejects sizedFields which are not fields of the output type', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
a: A @listSize(assumedSize: 5, sizedFields: ["notOnA"])
}

type A {
ints: [Int]
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'LIST_SIZE_INVALID_SIZED_FIELD',
`[S] Sized field "notOnA" is not a field on type "A"`,
],
]);
});

it('rejects sizedFields which are not lists', () => {
const doc = gql`
extend schema
@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])

type Query {
a: A @listSize(assumedSize: 5, sizedFields: ["notList"])
}

type A {
notList: String
}
`;

expect(buildForErrors(doc)).toStrictEqual([
[
'LIST_SIZE_APPLIED_TO_NON_LIST',
`[S] Sized field "A.notList" is not a list`,
],
]);
});
});
36 changes: 36 additions & 0 deletions internals-js/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,36 @@ const CONTEXTUAL_ARGUMENT_NOT_CONTEXTUAL_IN_ALL_SUBGRAPHS = makeCodeDefinition(
{ addedIn: '2.7.0' },
);

const COST_APPLIED_TO_INTERFACE_FIELD = makeCodeDefinition(
'COST_APPLIED_TO_INTERFACE_FIELD',
'The `@cost` directive must be applied to concrete types',
{ addedIn: '2.9.2' },
);

const LIST_SIZE_APPLIED_TO_NON_LIST = makeCodeDefinition(
'LIST_SIZE_APPLIED_TO_NON_LIST',
'The `@listSize` directive must be applied to list types',
{ addedIn: '2.9.2' },
);

const LIST_SIZE_INVALID_ASSUMED_SIZE = makeCodeDefinition(
'LIST_SIZE_INVALID_ASSUMED_SIZE',
'The `@listSize` directive assumed size cannot be negative',
{ addedIn: '2.9.2' },
);

const LIST_SIZE_INVALID_SLICING_ARGUMENT = makeCodeDefinition(
'LIST_SIZE_INVALID_SLICING_ARGUMENT',
'The `@listSize` directive must have existing integer slicing arguments',
{ addedIn: '2.9.2' },
);

const LIST_SIZE_INVALID_SIZED_FIELD = makeCodeDefinition(
'LIST_SIZE_INVALID_SIZED_FIELD',
'The `@listSize` directive must reference existing list fields as sized fields',
{ addedIn: '2.9.2' },
);

export const ERROR_CATEGORIES = {
DIRECTIVE_FIELDS_MISSING_EXTERNAL,
DIRECTIVE_UNSUPPORTED_ON_INTERFACE,
Expand Down Expand Up @@ -824,6 +854,12 @@ export const ERRORS = {
SOURCE_FIELD_SELECTION_INVALID,
SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD,
CONTEXTUAL_ARGUMENT_NOT_CONTEXTUAL_IN_ALL_SUBGRAPHS,
// Errors related to demand control
COST_APPLIED_TO_INTERFACE_FIELD,
LIST_SIZE_APPLIED_TO_NON_LIST,
LIST_SIZE_INVALID_ASSUMED_SIZE,
LIST_SIZE_INVALID_SIZED_FIELD,
LIST_SIZE_INVALID_SLICING_ARGUMENT,
};

const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {});
Expand Down
97 changes: 96 additions & 1 deletion internals-js/src/federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
isListType,
isWrapperType,
possibleRuntimeTypes,
isIntType,
} from "./definitions";
import { assert, MultiMap, printHumanReadableList, OrderedMap, mapValues, assertUnreachable } from "./utils";
import { SDLValidationRule } from "graphql/validation/ValidationContext";
Expand Down Expand Up @@ -100,7 +101,7 @@ import {
SourceFieldDirectiveArgs,
SourceTypeDirectiveArgs,
} from "./specs/sourceSpec";
import { CostDirectiveArguments, ListSizeDirectiveArguments } from "./specs/costSpec";
import { COST_VERSIONS, CostDirectiveArguments, ListSizeDirectiveArguments, costIdentity } from "./specs/costSpec";

const linkSpec = LINK_VERSIONS.latest();
const tagSpec = TAG_VERSIONS.latest();
Expand Down Expand Up @@ -1736,6 +1737,100 @@ export class FederationBlueprint extends SchemaBlueprint {
}
}

tninesling marked this conversation as resolved.
Show resolved Hide resolved
const costFeature = schema.coreFeatures?.getByIdentity(costIdentity);
const costSpec = costFeature && COST_VERSIONS.find(costFeature.url.version);
const costDirective = costSpec?.costDirective(schema);
const listSizeDirective = costSpec?.listSizeDirective(schema);

// Validate @cost
for (const application of costDirective?.applications() ?? []) {
const parent = application.parent;
// @cost cannot be used on interfaces https://ibm.github.io/graphql-specs/cost-spec.html#sec-No-Cost-on-Interface-Fields
if (parent instanceof FieldDefinition && parent.parent instanceof InterfaceType) {
errorCollector.push(ERRORS.COST_APPLIED_TO_INTERFACE_FIELD.err(
`@cost cannot be applied to interface "${parent.coordinate}"`,
{ nodes: sourceASTs(application, parent) }
));
}
}

// Validate @listSize
for (const application of listSizeDirective?.applications() ?? []) {
const parent = application.parent;
// @listSize can only be applied to FIELD_DEFINITION
if (parent instanceof FieldDefinition && parent.type) {
tninesling marked this conversation as resolved.
Show resolved Hide resolved
const { assumedSize, slicingArguments = [], sizedFields = [] } = application.arguments();
// @listSize must be applied to a list https://ibm.github.io/graphql-specs/cost-spec.html#sec-Valid-List-Size-Target
if (!sizedFields.length && !isListType(parent.type)) {
errorCollector.push(ERRORS.LIST_SIZE_APPLIED_TO_NON_LIST.err(
`"${parent.coordinate}" is not a list`,
{ nodes: sourceASTs(application, parent) },
));
}

// Validate assumed size, but we differ from https://ibm.github.io/graphql-specs/cost-spec.html#sec-Valid-Assumed-Size.
// Assumed size is used as a backup for slicing arguments in the event they are both specified.
// The spec aims to rule out cases when the assumed size will never be used because there is always
// a slicing argument. Two applications which are compliant with that validation rule can be merged
// into an application which is not compliant, thus we need to handle this case gracefully at runtime regardless.
// We omit this check to keep the validations to those that will otherwise cause runtime failures.
//
// With all that said, assumed size should not be negative.
if (assumedSize && assumedSize < 0) {
tninesling marked this conversation as resolved.
Show resolved Hide resolved
errorCollector.push(ERRORS.LIST_SIZE_INVALID_ASSUMED_SIZE.err(
`Assumed size of "${parent.coordinate}" cannot be negative`,
{ nodes: sourceASTs(application, parent) },
));
}

// Validate slicingArguments https://ibm.github.io/graphql-specs/cost-spec.html#sec-Valid-Slicing-Arguments-Target
for (const slicingArgumentName of slicingArguments) {
const slicingArgument = parent.argument(slicingArgumentName);
if (!slicingArgument?.type) {
// Slicing arguments must be one of the field's arguments
errorCollector.push(ERRORS.LIST_SIZE_INVALID_SLICING_ARGUMENT.err(
`Slicing argument "${slicingArgumentName}" is not an argument of "${parent.coordinate}"`,
{ nodes: sourceASTs(application, parent) }
));
} else if (!isIntType(slicingArgument.type) && !(isNonNullType(slicingArgument.type) && isIntType(slicingArgument.type.baseType()))) {
// Slicing arguments must be Int or Int!
errorCollector.push(ERRORS.LIST_SIZE_INVALID_SLICING_ARGUMENT.err(
`Slicing argument "${slicingArgument.coordinate}" must be Int or Int!`,
{ nodes: sourceASTs(application, parent) }
));
}
}

// Validate sizedFields https://ibm.github.io/graphql-specs/cost-spec.html#sec-Valid-Sized-Fields-Target
if (sizedFields.length) {
if (!isCompositeType(parent.type)) {
// The output type must have fields
errorCollector.push(ERRORS.LIST_SIZE_INVALID_SIZED_FIELD.err(
`Sized fields cannot be used because "${parent.type}" is not an object type`,
{ nodes: sourceASTs(application, parent)}
));
} else {
for (const sizedFieldName of sizedFields) {
const sizedField = parent.type.field(sizedFieldName);
if (!sizedField) {
// Sized fields must be present on the output type
errorCollector.push(ERRORS.LIST_SIZE_INVALID_SIZED_FIELD.err(
`Sized field "${sizedFieldName}" is not a field on type "${parent.type.coordinate}"`,
{ nodes: sourceASTs(application, parent) }
));
} else if (!sizedField.type || !isListType(sizedField.type)) {
// Sized fields must be lists
errorCollector.push(ERRORS.LIST_SIZE_APPLIED_TO_NON_LIST.err(
`Sized field "${sizedField.coordinate}" is not a list`,
{ nodes: sourceASTs(application, parent) },
));
}
}
}
}
}
}

return errorCollector;
}

Expand Down