diff --git a/src/methods/validate-fixture-input.ts b/src/methods/validate-fixture-input.ts index 79e74a5..3249c58 100644 --- a/src/methods/validate-fixture-input.ts +++ b/src/methods/validate-fixture-input.ts @@ -47,13 +47,12 @@ export function validateFixtureInput( const inlineFragmentSpreadsAst = inlineNamedFragmentSpreads(queryAST); const typeInfo = new TypeInfo(schema); const valueStack: any[][] = [[value]]; - // based on field depth const typeStack: (GraphQLNamedType | undefined)[] = []; - // based on selection set depth const possibleTypesStack: Set[] = [ new Set([schema.getQueryType()!.name]), ]; const typenameResponseKeyStack: (string | undefined)[] = []; + const expectedFieldsStack: Map>[] = [new Map()]; const errors: string[] = []; @@ -65,16 +64,20 @@ export function validateFixtureInput( let possibleTypes = new Set( possibleTypesStack[possibleTypesStack.length - 1], ); + if (node.typeCondition !== null && node.typeCondition !== undefined) { const namedType = schema.getType(node.typeCondition.name.value); - if (namedType && isAbstractType(namedType)) { - possibleTypes = possibleTypes.intersection( - new Set( - schema.getPossibleTypes(namedType).map((type) => type.name), - ), - ); - } else if (namedType && isObjectType(namedType)) { - possibleTypes = new Set([namedType.name]); + + if (namedType) { + if (isAbstractType(namedType)) { + possibleTypes = possibleTypes.intersection( + new Set( + schema.getPossibleTypes(namedType).map((type) => type.name), + ), + ); + } else if (isObjectType(namedType)) { + possibleTypes = new Set([namedType.name]); + } } } possibleTypesStack.push(possibleTypes); @@ -90,6 +93,19 @@ export function validateFixtureInput( const responseKey = node.alias?.value || node.name.value; + // Track this field in the appropriate set based on whether we're in an inline fragment + const currentExpectedFields = + expectedFieldsStack[expectedFieldsStack.length - 1]; + const currentPossibleTypes = + possibleTypesStack[possibleTypesStack.length - 1]; + + for (const concreteTypeName of currentPossibleTypes) { + if (!currentExpectedFields.has(concreteTypeName)) { + currentExpectedFields.set(concreteTypeName, new Set()); + } + currentExpectedFields.get(concreteTypeName)!.add(responseKey); + } + const fieldDefinition = typeInfo.getFieldDef(); if (fieldDefinition === undefined || fieldDefinition === null) { errors.push( @@ -221,6 +237,11 @@ export function validateFixtureInput( }, SelectionSet: { enter(node, _key, parent) { + // If this SelectionSet belongs to a Field, prepare to track expected fields + if (parent && "kind" in parent && parent.kind === Kind.FIELD) { + expectedFieldsStack.push(new Map()); + } + // Look ahead to find __typename field and track its response key const typenameField = node.selections.find( (selection) => @@ -268,12 +289,33 @@ export function validateFixtureInput( } return undefined; }, - leave() { + leave(_node, _key, parent) { + // If this SelectionSet belongs to a Field or is the root (OperationDefinition), validate for extra fields + if ( + parent && + "kind" in parent && + (parent.kind === Kind.FIELD || + parent.kind === Kind.OPERATION_DEFINITION) + ) { + const expectedFields = expectedFieldsStack.pop()!; + const nestedValues = valueStack[valueStack.length - 1]; + const typenameResponseKey = + typenameResponseKeyStack[typenameResponseKeyStack.length - 1]; + errors.push( + ...checkForExtraFields( + nestedValues, + expectedFields, + typenameResponseKey, + ), + ); + } + typenameResponseKeyStack.pop(); }, }, }), ); + return { errors }; } @@ -401,3 +443,72 @@ function isValueExpectedForType( return possibleTypes.has(valueTypename); } + +/** + * Checks fixture objects for fields that are not present in the GraphQL query. + * Supports type discrimination for abstract types (unions/interfaces) using __typename. + * + * @param fixtureObjects - Array of fixture objects to validate + * @param expectedFields - Expected fields structure with common fields and type-specific fields + * @param typenameResponseKey - The response key for the __typename field (supports aliases like `type: __typename`) + * @returns Array of error messages for any extra fields found (empty if valid) + * + * @remarks + * - Only validates object types - skips null values and arrays + * - Uses __typename to determine which inline fragment fields apply to each object + * - Common fields (outside inline fragments) are expected on all objects + * - Type-specific fields (inside inline fragments) are only expected on matching types + * - No schema lookups needed - possible types were pre-computed during traversal + */ +function checkForExtraFields( + fixtureObjects: any[], + expectedFields: Map>, + typenameResponseKey: string | undefined, +): string[] { + const errors: string[] = []; + + for (const fixtureObject of fixtureObjects) { + if ( + typeof fixtureObject === "object" && + fixtureObject !== null && + !Array.isArray(fixtureObject) + ) { + const fixtureFields = Object.keys(fixtureObject); + + // Build the set of expected fields for this specific object + let expectedForThisObject = new Set(); + + const objectTypename = typenameResponseKey + ? fixtureObject[typenameResponseKey] + : fixtureObject.__typename; + + if (objectTypename) { + // Object has __typename - direct lookup by concrete type name + const typeSpecificFields = expectedFields.get(objectTypename); + if (typeSpecificFields) { + expectedForThisObject = typeSpecificFields; + } + } else if (expectedFields.size > 0) { + // No __typename - allow union of all fragment fields + // Without __typename we can't discriminate which fragment applies + // Note: We use > 0 (not === 1) to handle nested fragments (e.g., ... on HasId { ... on HasName { ... }}) + // where byType.size can be > 1. For 2+ sibling fragments (e.g., ... on Item / ... on Metadata) + // without __typename, validation BREAKs early (line 277) to enforce __typename requirement. + expectedFields.forEach((fields) => { + fields.forEach((field) => expectedForThisObject.add(field)); + }); + } + + // Check each field in the fixture object + for (const fixtureField of fixtureFields) { + if (!expectedForThisObject.has(fixtureField)) { + errors.push( + `Extra field "${fixtureField}" found in fixture data not in query`, + ); + } + } + } + } + + return errors; +} diff --git a/test-app/extensions/discount-function-rs/tests/fixtures/delivery-valid-fixture.json b/test-app/extensions/discount-function-rs/tests/fixtures/delivery-valid-fixture.json index 2b2b0a9..5ee44a7 100644 --- a/test-app/extensions/discount-function-rs/tests/fixtures/delivery-valid-fixture.json +++ b/test-app/extensions/discount-function-rs/tests/fixtures/delivery-valid-fixture.json @@ -11,12 +11,7 @@ { "id": "gid://shopify/CartDeliveryGroup/1" } - ], - "cost": { - "subtotalAmount": { - "amount": "100.0" - } - } + ] } }, "output": { diff --git a/test/fixtures/test-schema.graphql b/test/fixtures/test-schema.graphql index 7f1c6e3..4c576f6 100644 --- a/test/fixtures/test-schema.graphql +++ b/test/fixtures/test-schema.graphql @@ -5,6 +5,7 @@ schema { type Query { data: DataContainer + version: String } type DataContainer { diff --git a/test/methods/validate-fixture-input.test.ts b/test/methods/validate-fixture-input.test.ts index 8d09e5c..d2313c3 100644 --- a/test/methods/validate-fixture-input.test.ts +++ b/test/methods/validate-fixture-input.test.ts @@ -78,15 +78,11 @@ describe("validateFixtureInput", () => { { id: "gid://test/Item/1", count: 5, - details: { - name: "First Item", - }, }, ], secondItems: [ { id: "gid://test/Item/1", - count: 5, details: { name: "First Item", }, @@ -1233,11 +1229,7 @@ describe("validateFixtureInput", () => { ); }); - // This test is skipped because the validator doesn't yet detect extra fields - // in fixture data that aren't present in the query. Currently, it only validates - // that all required fields from the query are present in the fixture, but doesn't - // flag additional fields that shouldn't be there. - it.skip("detects extra fields not in query", () => { + it("detects extra fields not in query", () => { const queryAST = parse(` query Query { data { @@ -1261,10 +1253,568 @@ describe("validateFixtureInput", () => { const result = validateFixtureInput(queryAST, schema, fixtureInput); - // When implemented, should detect that 'count' is not in the query + // Should detect that 'count' is not in the query expect(result.errors).toHaveLength(1); - expect(result.errors[0]).toContain("count"); - expect(result.errors[0]).toContain("not in query"); + expect(result.errors[0]).toBe( + 'Extra field "count" found in fixture data not in query', + ); + }); + + it("detects extra fields with multiple aliases for the same field", () => { + const queryAST = parse(` + query Query { + data { + firstItems: items { + id + count + } + secondItems: items { + id + details { + name + } + } + } + } + `); + + const fixtureInput = { + data: { + firstItems: [ + { + id: "gid://test/Item/1", + count: 5, + details: { + name: "First Item", + }, + }, + ], + secondItems: [ + { + id: "gid://test/Item/1", + count: 5, + details: { + name: "First Item", + }, + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Each alias is validated independently, so extra fields in each should be detected + expect(result.errors).toHaveLength(2); + expect(result.errors[0]).toBe( + 'Extra field "details" found in fixture data not in query', + ); + expect(result.errors[1]).toBe( + 'Extra field "count" found in fixture data not in query', + ); + }); + + it("detects extra fields at root level", () => { + const queryAST = parse(` + query Query { + data { + items { + id + } + } + } + `); + + const fixtureInput = { + data: { + items: [ + { + id: "gid://test/Item/1", + }, + ], + }, + version: "1.0.0", // Real field from schema, but not selected in query + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect the version field since it wasn't selected in the query + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toBe( + 'Extra field "version" found in fixture data not in query', + ); + }); + + it("detects extra fields with complex nesting, typename aliases, and type discrimination", () => { + const queryAST = parse(` + query { + queryType: __typename + data { + searchResults { + resultType: __typename + ... on Item { + id + details { + __typename + name + } + } + ... on Metadata { + email + } + } + interfaceImplementers { + implType: __typename + ... on HasId { + id + } + ... on HasName { + name + } + } + implementersNoType: interfaceImplementers { + ... on HasId { + id + } + } + nested { + nestedType: __typename + ... on NestedOuterA { + id + inner { + innerType: __typename + ... on NestedInnerA { + name + } + ... on NestedInnerB { + value + } + } + } + ... on NestedOuterB { + email + } + } + } + } + `); + + const fixtureInput = { + queryType: "Query", + extraRootField: "should not be here", // Extra at root + data: { + searchResults: [ + { + resultType: "Item", + id: "1", + count: 999, // Extra - count not queried for Item + details: { + __typename: "ItemDetails", + name: "Details", + extraDetailField: "wrong", // Extra at nested level + }, + }, + { + resultType: "Metadata", + email: "test@example.com", + phone: "555-1234", // Extra - phone not queried + }, + ], + interfaceImplementers: [ + { + implType: "InterfaceImplementer1", + id: "impl1", + name: "First", + description: "extra", // Extra - description not queried + }, + { + implType: "InterfaceImplementer2", + id: "impl2", + name: "Second", + extraField: "also wrong", // Generic extra field + }, + ], + implementersNoType: [ + { id: "impl3" }, // Valid - has id from HasId fragment + {}, // Empty object - valid without __typename (single fragment, union of all fields) + ], + nested: [ + { + nestedType: "NestedOuterA", + id: "outer1", + email: "cross-contamination", // Extra - email is from NestedOuterB + inner: [ + { + innerType: "NestedInnerA", + name: "Inner", + value: "cross-contamination", // Extra - value is from NestedInnerB + }, + ], + }, + { + nestedType: "NestedOuterB", + email: "outer@example.com", + id: "cross-contamination", // Extra - id is from NestedOuterA + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect extra fields at all levels with cross-contamination and typename aliases + // Empty object in implementersNoType is valid (single fragment without __typename - union mode) + // Errors appear in post-order traversal (deepest to shallowest): + expect(result.errors).toHaveLength(9); + expect(result.errors[0]).toBe( + 'Extra field "extraDetailField" found in fixture data not in query', + ); // details (deepest) + expect(result.errors[1]).toBe( + 'Extra field "count" found in fixture data not in query', + ); // Item + expect(result.errors[2]).toBe( + 'Extra field "phone" found in fixture data not in query', + ); // Metadata + expect(result.errors[3]).toBe( + 'Extra field "description" found in fixture data not in query', + ); // InterfaceImplementer1 + expect(result.errors[4]).toBe( + 'Extra field "extraField" found in fixture data not in query', + ); // InterfaceImplementer2 + expect(result.errors[5]).toBe( + 'Extra field "value" found in fixture data not in query', + ); // NestedInnerA cross-contamination + expect(result.errors[6]).toBe( + 'Extra field "email" found in fixture data not in query', + ); // NestedOuterA cross-contamination + expect(result.errors[7]).toBe( + 'Extra field "id" found in fixture data not in query', + ); // NestedOuterB cross-contamination + expect(result.errors[8]).toBe( + 'Extra field "extraRootField" found in fixture data not in query', + ); // root (last) + }); + + it("detects extra fields in union types with inline fragments", () => { + const queryAST = parse(` + query { + data { + searchResults { + __typename + ... on Item { + id + } + ... on Metadata { + email + } + } + } + } + `); + + const fixtureInput = { + data: { + searchResults: [ + { + __typename: "Item", + id: "gid://test/Item/1", + count: 5, // Extra field - queried id but not count + }, + { + __typename: "Metadata", + email: "test@example.com", + phone: "555-0001", // Extra field - queried email but not phone + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect extra fields in both union members + expect(result.errors).toHaveLength(2); + expect(result.errors[0]).toBe( + 'Extra field "count" found in fixture data not in query', + ); + expect(result.errors[1]).toBe( + 'Extra field "phone" found in fixture data not in query', + ); + }); + + it("detects fields from wrong fragment type in unions (cross-contamination)", () => { + const queryAST = parse(` + query { + data { + searchResults { + __typename + ... on Item { + id + count + } + ... on Metadata { + email + phone + } + } + } + } + `); + + const fixtureInput = { + data: { + searchResults: [ + { + __typename: "Item", + id: "gid://test/Item/1", + count: 5, + email: "item@example.com", // email is only in Metadata fragment + phone: "555-1234", // phone is only in Metadata fragment + }, + { + __typename: "Metadata", + email: "metadata@example.com", + phone: "555-5678", + id: "wrong-id", // id is only in Item fragment + count: 10, // count is only in Item fragment + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect fields from wrong fragment types + // Item should NOT have email/phone (those are Metadata fields) + // Metadata should NOT have id/count (those are Item fields) + expect(result.errors).toHaveLength(4); + expect(result.errors[0]).toBe( + 'Extra field "email" found in fixture data not in query', + ); + expect(result.errors[1]).toBe( + 'Extra field "phone" found in fixture data not in query', + ); + expect(result.errors[2]).toBe( + 'Extra field "id" found in fixture data not in query', + ); + expect(result.errors[3]).toBe( + 'Extra field "count" found in fixture data not in query', + ); + }); + + it("detects extra fields in interface fragments with type discrimination", () => { + const queryAST = parse(` + query { + data { + interfaceImplementers { + __typename + ... on HasId { + id + } + ... on HasName { + name + } + ... on HasDescription { + description + } + } + } + } + `); + + const fixtureInput = { + data: { + interfaceImplementers: [ + { + __typename: "InterfaceImplementer1", // Implements HasId, HasName, HasDescription + id: "1", + name: "First", + description: "Desc", + extraField1: "should not be here", // Extra field + }, + { + __typename: "InterfaceImplementer2", // Implements HasId, HasName only + id: "2", + name: "Second", + description: "Wrong!", // Does NOT implement HasDescription + extraField2: "also wrong", + }, + { + __typename: "InterfaceImplementer3", // Implements HasId only + id: "3", + name: "Wrong!", // Does NOT implement HasName + description: "Wrong!", // Does NOT implement HasDescription + extraField3: "also wrong", + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect: + // - extraField1 on InterfaceImplementer1 (implements all interfaces, but field not in query) + // - description and extraField2 on InterfaceImplementer2 (doesn't implement HasDescription) + // - name, description, and extraField3 on InterfaceImplementer3 (doesn't implement HasName or HasDescription) + expect(result.errors).toHaveLength(6); + expect(result.errors[0]).toBe( + 'Extra field "extraField1" found in fixture data not in query', + ); + expect(result.errors[1]).toBe( + 'Extra field "description" found in fixture data not in query', + ); + expect(result.errors[2]).toBe( + 'Extra field "extraField2" found in fixture data not in query', + ); + expect(result.errors[3]).toBe( + 'Extra field "name" found in fixture data not in query', + ); + expect(result.errors[4]).toBe( + 'Extra field "description" found in fixture data not in query', + ); + expect(result.errors[5]).toBe( + 'Extra field "extraField3" found in fixture data not in query', + ); + }); + + it("detects extra fields in truly nested inline fragments (fragment within fragment)", () => { + const queryAST = parse(` + query { + data { + nested { + __typename + ... on NestedOuterA { + id + inner { + __typename + ... on NestedInnerA { + name + } + ... on NestedInnerB { + value + } + } + } + ... on NestedOuterB { + email + } + } + } + } + `); + + const fixtureInput = { + data: { + nested: [ + { + __typename: "NestedOuterA", + id: "1", + email: "wrongField", // email is from NestedOuterB, not NestedOuterA + inner: [ + { + __typename: "NestedInnerA", + name: "Inner name", + value: "wrongField", // value is from NestedInnerB, not NestedInnerA + }, + ], + }, + { + __typename: "NestedOuterB", + email: "outer@example.com", + id: "wrongField", // id is from NestedOuterA, not NestedOuterB + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect: + // - value on NestedInnerA (nested level validated first) + // - email on NestedOuterA (outer level) + // - id on NestedOuterB (outer level) + expect(result.errors).toHaveLength(3); + expect(result.errors[0]).toBe( + 'Extra field "value" found in fixture data not in query', + ); + expect(result.errors[1]).toBe( + 'Extra field "email" found in fixture data not in query', + ); + expect(result.errors[2]).toBe( + 'Extra field "id" found in fixture data not in query', + ); + }); + + it("detects extra fields in nested inline fragments on concrete union types", () => { + const queryAST = parse(` + query { + data { + interfaceImplementers { + __typename + ... on InterfaceImplementer1 { + id + name + } + ... on InterfaceImplementer2 { + id + } + ... on InterfaceImplementer3 { + id + } + } + } + } + `); + + const fixtureInput = { + data: { + interfaceImplementers: [ + { + __typename: "InterfaceImplementer1", + id: "1", + name: "First", + description: "extra field", // Not queried for InterfaceImplementer1 + extraField1: "should not be here", + }, + { + __typename: "InterfaceImplementer2", + id: "2", + name: "Wrong!", // InterfaceImplementer2 fragment only queries id + extraField2: "also wrong", + }, + { + __typename: "InterfaceImplementer3", + id: "3", + name: "Wrong!", // InterfaceImplementer3 fragment only queries id + description: "also wrong", + }, + ], + }, + }; + + const result = validateFixtureInput(queryAST, schema, fixtureInput); + + // Should detect: + // - description and extraField1 on InterfaceImplementer1 (only id, name queried) + // - name and extraField2 on InterfaceImplementer2 (only id queried) + // - name and description on InterfaceImplementer3 (only id queried) + expect(result.errors).toHaveLength(6); + expect(result.errors[0]).toBe( + 'Extra field "description" found in fixture data not in query', + ); + expect(result.errors[1]).toBe( + 'Extra field "extraField1" found in fixture data not in query', + ); + expect(result.errors[2]).toBe( + 'Extra field "name" found in fixture data not in query', + ); + expect(result.errors[3]).toBe( + 'Extra field "extraField2" found in fixture data not in query', + ); + expect(result.errors[4]).toBe( + 'Extra field "name" found in fixture data not in query', + ); + expect(result.errors[5]).toBe( + 'Extra field "description" found in fixture data not in query', + ); }); it("detects type mismatches (object vs scalar)", () => { diff --git a/test/methods/validate-test-assets.test.ts b/test/methods/validate-test-assets.test.ts index 3c9f4dd..b67e3ba 100644 --- a/test/methods/validate-test-assets.test.ts +++ b/test/methods/validate-test-assets.test.ts @@ -157,12 +157,15 @@ describe("validateTestAssets", () => { expect(result.inputQuery.errors).toHaveLength(0); - // Input fixture should be invalid due to missing fields - expect(result.inputFixture.errors.length).toBe(2); + // Input fixture should be invalid due to missing fields and extra field + expect(result.inputFixture.errors.length).toBe(3); expect(result.inputFixture.errors[0]).toBe( "Missing expected fixture data for details", ); expect(result.inputFixture.errors[1]).toBe( + 'Extra field "invalidField" found in fixture data not in query', + ); + expect(result.inputFixture.errors[2]).toBe( "Missing expected fixture data for metadata", );