diff --git a/.changeset/tidy-trees-dress.md b/.changeset/tidy-trees-dress.md new file mode 100644 index 000000000..eb8718314 --- /dev/null +++ b/.changeset/tidy-trees-dress.md @@ -0,0 +1,5 @@ +--- +"@apollo/federation-internals": patch +--- + +Add validations for demand control directive applications diff --git a/docs/source/federation-versions.mdx b/docs/source/federation-versions.mdx index d2143ec0b..0bd370106 100644 --- a/docs/source/federation-versions.mdx +++ b/docs/source/federation-versions.mdx @@ -44,7 +44,7 @@ First release Minimum router version -**TBD** +**1.53.0** diff --git a/internals-js/src/__tests__/subgraphValidation.test.ts b/internals-js/src/__tests__/subgraphValidation.test.ts index ab0292e12..3544b1200 100644 --- a/internals-js/src/__tests__/subgraphValidation.test.ts +++ b/internals-js/src/__tests__/subgraphValidation.test.ts @@ -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`, + ], + ]); + }); +}); diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index f3fa90e89..79b6a1f53 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -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, @@ -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; }, {}); diff --git a/internals-js/src/federation.ts b/internals-js/src/federation.ts index f24145ae1..52e528e58 100644 --- a/internals-js/src/federation.ts +++ b/internals-js/src/federation.ts @@ -36,8 +36,9 @@ import { isListType, isWrapperType, possibleRuntimeTypes, + isIntType, } from "./definitions"; -import { assert, MultiMap, printHumanReadableList, OrderedMap, mapValues, assertUnreachable } from "./utils"; +import { assert, MultiMap, printHumanReadableList, OrderedMap, mapValues, assertUnreachable, isDefined } from "./utils"; import { SDLValidationRule } from "graphql/validation/ValidationContext"; import { specifiedSDLRules } from "graphql/validation/specifiedRules"; import { @@ -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(); @@ -1055,6 +1056,115 @@ function validateShareableNotRepeatedOnSameDeclaration( } } } + +function validateCostNotAppliedToInterface(application: Directive, CostDirectiveArguments>, errorCollector: GraphQLError[]) { + 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) } + )); + } +} + +function validateListSizeAppliedToList( + application: Directive, ListSizeDirectiveArguments>, + parent: FieldDefinition, + errorCollector: GraphQLError[], +) { + const { 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 && parent.type && !isListType(parent.type)) { + errorCollector.push(ERRORS.LIST_SIZE_APPLIED_TO_NON_LIST.err( + `"${parent.coordinate}" is not a list`, + { nodes: sourceASTs(application, parent) }, + )); + } +} + +function validateAssumedSizeNotNegative( + application: Directive, ListSizeDirectiveArguments>, + parent: FieldDefinition, + errorCollector: GraphQLError[] +) { + const { assumedSize } = application.arguments(); + // 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 (isDefined(assumedSize) && assumedSize < 0) { + errorCollector.push(ERRORS.LIST_SIZE_INVALID_ASSUMED_SIZE.err( + `Assumed size of "${parent.coordinate}" cannot be negative`, + { nodes: sourceASTs(application, parent) }, + )); + } +} + +function validateSlicingArgumentsAreValidIntegers( + application: Directive, ListSizeDirectiveArguments>, + parent: FieldDefinition, + errorCollector: GraphQLError[] +) { + const { slicingArguments = [] } = application.arguments(); + // 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) } + )); + } + } +} + +function validateSizedFieldsAreValidLists( + application: Directive, ListSizeDirectiveArguments>, + parent: FieldDefinition, + errorCollector: GraphQLError[] +) { + const { sizedFields = [] } = application.arguments(); + // Validate sizedFields https://ibm.github.io/graphql-specs/cost-spec.html#sec-Valid-Sized-Fields-Target + if (sizedFields.length) { + if (!parent.type || !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) }, + )); + } + } + } + } +} + export class FederationMetadata { private _externalTester?: ExternalTester; private _sharingPredicate?: (field: FieldDefinition) => boolean; @@ -1736,6 +1846,26 @@ export class FederationBlueprint extends SchemaBlueprint { } } + 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() ?? []) { + validateCostNotAppliedToInterface(application, errorCollector); + } + + // Validate @listSize + for (const application of listSizeDirective?.applications() ?? []) { + const parent = application.parent; + assert(parent instanceof FieldDefinition, "@listSize can only be applied to FIELD_DEFINITION"); + validateListSizeAppliedToList(application, parent, errorCollector); + validateAssumedSizeNotNegative(application, parent, errorCollector); + validateSlicingArgumentsAreValidIntegers(application, parent, errorCollector); + validateSizedFieldsAreValidLists(application, parent, errorCollector); + } + return errorCollector; }