Better handling of wrapped types in list size validations (#3157)

Better handling of wrapped types in list size validations

Author: tninesling

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

@@ -1584,8 +1584,16 @@ describe('@listSize', () => {
  @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"])
+ sliced(
+ first: String
+ second: Int
+ third: Int!
+ fourth: [Int]
+ fifth: [Int]!
+ ): [String]
+ @listSize(
+ slicingArguments: ["first", "second", "third", "fourth", "fifth"]
+ )
  }
  `;
 
@@ -1594,6 +1602,14 @@ describe('@listSize', () => {
  'LIST_SIZE_INVALID_SLICING_ARGUMENT',
  `[S] Slicing argument "Query.sliced(first:)" must be Int or Int!`,
  ],
+ [
+ 'LIST_SIZE_INVALID_SLICING_ARGUMENT',
+ `[S] Slicing argument "Query.sliced(fourth:)" must be Int or Int!`,
+ ],
+ [
+ 'LIST_SIZE_INVALID_SLICING_ARGUMENT',
+ `[S] Slicing argument "Query.sliced(fifth:)" must be Int or Int!`,
+ ],
  ]);
  });
 
@@ -1620,7 +1636,7 @@ describe('@listSize', () => {
  expect(buildForErrors(doc)).toStrictEqual([
  [
  'LIST_SIZE_INVALID_SIZED_FIELD',
- `[S] Sized fields cannot be used because "Int" is not an object type`,
+ `[S] Sized fields cannot be used because "Int" is not a composite type`,
  ],
  ]);
  });
@@ -1653,10 +1669,16 @@ describe('@listSize', () => {
  @link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"])
 
  type Query {
- a: A @listSize(assumedSize: 5, sizedFields: ["notList"])
+ a: A
+ @listSize(
+ assumedSize: 5
+ sizedFields: ["list", "nonNullList", "notList"]
+ )
  }
 
  type A {
+ list: [String]
+ nonNullList: [String]!
  notList: String
  }
  `;

internals-js/src/federation.ts

@@ -37,8 +37,9 @@ import {
  isWrapperType,
  possibleRuntimeTypes,
  isIntType,
+ Type,
 } from "./definitions";
-import { assert, MultiMap, printHumanReadableList, OrderedMap, mapValues, assertUnreachable, isDefined } from "./utils";
+import { assert, MultiMap, printHumanReadableList, OrderedMap, mapValues, assertUnreachable } from "./utils";
 import { SDLValidationRule } from "graphql/validation/ValidationContext";
 import { specifiedSDLRules } from "graphql/validation/specifiedRules";
 import {
@@ -1097,14 +1098,18 @@ function validateAssumedSizeNotNegative(
  // 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) {
+ if (assumedSize !== undefined && assumedSize !== null && assumedSize < 0) {
  errorCollector.push(ERRORS.LIST_SIZE_INVALID_ASSUMED_SIZE.err(
  `Assumed size of "${parent.coordinate}" cannot be negative`,
  { nodes: sourceASTs(application, parent) },
  ));
  }
 }
 
+function isNonNullIntType(ty: Type): boolean {
+ return isNonNullType(ty) && isIntType(ty.ofType)
+}
+
 function validateSlicingArgumentsAreValidIntegers(
  application: Directive<SchemaElement<any, any>, ListSizeDirectiveArguments>,
  parent: FieldDefinition<CompositeType>,
@@ -1120,7 +1125,7 @@ function validateSlicingArgumentsAreValidIntegers(
  `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()))) {
+ } else if (!isIntType(slicingArgument.type) && !isNonNullIntType(slicingArgument.type)) {
  // 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!`,
@@ -1130,6 +1135,10 @@ function validateSlicingArgumentsAreValidIntegers(
  }
 }
 
+function isNonNullListType(ty: Type): boolean {
+ return isNonNullType(ty) && isListType(ty.ofType)
+}
+
 function validateSizedFieldsAreValidLists(
  application: Directive<SchemaElement<any, any>, ListSizeDirectiveArguments>,
  parent: FieldDefinition<CompositeType>,
@@ -1141,7 +1150,7 @@ function validateSizedFieldsAreValidLists(
  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`,
+ `Sized fields cannot be used because "${parent.type}" is not a composite type`,
  { nodes: sourceASTs(application, parent)}
  ));
  } else {
@@ -1153,7 +1162,7 @@ function validateSizedFieldsAreValidLists(
  `Sized field "${sizedFieldName}" is not a field on type "${parent.type.coordinate}"`,
  { nodes: sourceASTs(application, parent) }
  ));
- } else if (!sizedField.type || !isListType(sizedField.type)) {
+ } else if (!sizedField.type || !(isListType(sizedField.type) || isNonNullListType(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`,