From 2e7c8f3c94013f890f42ca1054287841478ba7a6 Mon Sep 17 00:00:00 2001 From: Dmitry Til Date: Thu, 22 Sep 2022 12:12:19 +0200 Subject: [PATCH] collect input fields from variables (#379) Closes #369 Co-authored-by: Kamil Kisiela --- .changeset/cool-pugs-nail.md | 5 + packages/libraries/client/src/apollo.ts | 1 + .../libraries/client/src/internal/types.ts | 6 + .../libraries/client/src/internal/usage.ts | 65 +++++++++-- .../libraries/client/src/internal/utils.ts | 26 +++-- .../client/tests/usage-collector.spec.ts | 104 ++++++++++++++++-- 6 files changed, 183 insertions(+), 24 deletions(-) create mode 100644 .changeset/cool-pugs-nail.md diff --git a/.changeset/cool-pugs-nail.md b/.changeset/cool-pugs-nail.md new file mode 100644 index 000000000..6f84add85 --- /dev/null +++ b/.changeset/cool-pugs-nail.md @@ -0,0 +1,5 @@ +--- +"@graphql-hive/client": patch +--- + +Collect input fields from variables (opt-in with `processVariables` flag) diff --git a/packages/libraries/client/src/apollo.ts b/packages/libraries/client/src/apollo.ts index 061d6b855..be8824ada 100644 --- a/packages/libraries/client/src/apollo.ts +++ b/packages/libraries/client/src/apollo.ts @@ -94,6 +94,7 @@ export function hiveApollo(clientOrOptions: HiveClient | HivePluginOptions): Apo }, operationName: context.operationName, contextValue: context.context, + variableValues: context.request.variables, }); if (isLegacyV0) { diff --git a/packages/libraries/client/src/internal/types.ts b/packages/libraries/client/src/internal/types.ts index ca9f4ce97..ceeae72b6 100644 --- a/packages/libraries/client/src/internal/types.ts +++ b/packages/libraries/client/src/internal/types.ts @@ -65,6 +65,12 @@ export interface HiveUsagePluginOptions { * Default: 1.0 */ sampleRate?: number; + /** + * Enables collecting Input fields usage based on the variables passed to the operation. + * + * Default: false + */ + processVariables?: boolean; } export interface HiveReportingPluginOptions { diff --git a/packages/libraries/client/src/internal/usage.ts b/packages/libraries/client/src/internal/usage.ts index bc744c3f2..6f34a8dca 100644 --- a/packages/libraries/client/src/internal/usage.ts +++ b/packages/libraries/client/src/internal/usage.ts @@ -12,6 +12,7 @@ import { GraphQLType, GraphQLUnionType, isEnumType, + isInputObjectType, isListType, isNonNullType, isScalarType, @@ -155,8 +156,9 @@ export function createUsage(pluginOptions: HivePluginOptions): UsageCollector { schema: args.schema, max: options.max ?? 1000, ttl: options.ttl, + processVariables: options.processVariables ?? false, }); - const { key, value: info } = collect(document); + const { key, value: info } = collect(document, args.variableValues); agent.capture({ key, @@ -190,10 +192,20 @@ interface CacheResult { fields: string[]; } -export function createCollector({ schema, max, ttl }: { schema: GraphQLSchema; max?: number; ttl?: number }) { +export function createCollector({ + schema, + max, + ttl, + processVariables, +}: { + schema: GraphQLSchema; + max?: number; + ttl?: number; + processVariables?: boolean; +}) { const typeInfo = new TypeInfo(schema); - function collect(doc: DocumentNode): CacheResult { + function collect(doc: DocumentNode, variables: ExecutionArgs['variableValues']): CacheResult { const entries = new Set(); const collected_entire_named_types = new Set(); @@ -237,7 +249,11 @@ export function createCollector({ schema, max, ttl }: { schema: GraphQLSchema; m if (node.value.kind === Kind.ENUM) { // Collect only a specific enum value collectInputType(inputTypeName, node.value.value); - } else if (node.value.kind !== Kind.OBJECT && node.value.kind !== Kind.LIST) { + } else if ( + node.value.kind !== Kind.OBJECT && + node.value.kind !== Kind.LIST && + node.value.kind !== Kind.VARIABLE + ) { collectInputType(inputTypeName); } } @@ -276,6 +292,29 @@ export function createCollector({ schema, max, ttl }: { schema: GraphQLSchema; m } } + function collectVariable(namedType: GraphQLNamedInputType, variableValue: unknown) { + const variableValueArray = Array.isArray(variableValue) ? variableValue : [variableValue]; + if (isInputObjectType(namedType)) { + variableValueArray.forEach(variable => { + if (variable) { + // Collect only the used fields + for (const fieldName in variable) { + const field = namedType.getFields()[fieldName]; + if (field) { + collectInputType(namedType.name, fieldName); + collectVariable(unwrapType(field.type), variable[fieldName]); + } + } + } else { + // Collect the entire type + collectInputType(namedType.name); + } + }); + } else { + collectInputType(namedType.name); + } + } + visit( doc, visitWithTypeInfo(typeInfo, { @@ -285,9 +324,16 @@ export function createCollector({ schema, max, ttl }: { schema: GraphQLSchema; m markAsUsed(makeId(parent.name, field.name)); }, - VariableDefinition() { + VariableDefinition(node) { const inputType = typeInfo.getInputType()!; - collectInputType(resolveTypeName(inputType)); + if (!variables) { + collectInputType(resolveTypeName(inputType)); + } else { + const variableName = node.variable.name.value; + const variableValue = variables[variableName]; + const namedType = unwrapType(inputType); + collectVariable(namedType, variableValue); + } }, Argument(node) { const parent = typeInfo.getParentType()!; @@ -340,7 +386,12 @@ export function createCollector({ schema, max, ttl }: { schema: GraphQLSchema; m }; } - return cache(collect, cacheDocumentKey, LRU(max, ttl)); + return cache( + (doc: DocumentNode, variables: ExecutionArgs['variableValues']) => + collect(doc, processVariables ? variables : undefined), + cacheDocumentKey, + LRU(max, ttl) + ); } function resolveTypeName(inputType: GraphQLType): string { diff --git a/packages/libraries/client/src/internal/utils.ts b/packages/libraries/client/src/internal/utils.ts index d4fa34480..d9754cf3e 100644 --- a/packages/libraries/client/src/internal/utils.ts +++ b/packages/libraries/client/src/internal/utils.ts @@ -22,17 +22,17 @@ export function memo(fn: (arg: A) => R, cacheKeyFn: (arg: A) => K): (ar }; } -export function cache( - fn: (arg: A) => R, - cacheKeyFn: (arg: A) => K, +export function cache( + fn: (arg: A, arg2: V) => R, + cacheKeyFn: (arg: A, arg2: V) => K, cacheMap: { has(key: K): boolean; set(key: K, value: R): void; get(key: K): R | undefined; } ) { - return (arg: A) => { - const key = cacheKeyFn(arg); + return (arg: A, arg2: V) => { + const key = cacheKeyFn(arg, arg2); const cachedValue = cacheMap.get(key); if (cachedValue !== null && typeof cachedValue !== 'undefined') { @@ -42,7 +42,7 @@ export function cache( }; } - const value = fn(arg); + const value = fn(arg, arg2); cacheMap.set(key, value); return { @@ -52,8 +52,18 @@ export function cache( }; } -export function cacheDocumentKey(doc: T) { - return createHash('md5').update(JSON.stringify(doc)).digest('hex'); +export function cacheDocumentKey(doc: T, variables?: V) { + const cacheKeySource: { doc: T; variables?: string } = { doc }; + if (variables) { + cacheKeySource['variables'] = JSON.stringify(variables, (key, value) => { + if ((value && typeof value === 'object' && Object.keys(value).length) || (Array.isArray(value) && value.length)) { + return value; + } + + return ''; + }); + } + return createHash('md5').update(JSON.stringify(cacheKeySource)).digest('hex'); } const HR_TO_NS = 1e9; diff --git a/packages/libraries/client/tests/usage-collector.spec.ts b/packages/libraries/client/tests/usage-collector.spec.ts index 8478a83c4..10a5b6c94 100644 --- a/packages/libraries/client/tests/usage-collector.spec.ts +++ b/packages/libraries/client/tests/usage-collector.spec.ts @@ -20,6 +20,7 @@ const schema = buildSchema(/* GraphQL */ ` input FilterInput { type: ProjectType pagination: PaginationInput + order: [ProjectOrderByInput!] } input PaginationInput { @@ -27,6 +28,16 @@ const schema = buildSchema(/* GraphQL */ ` offset: Int } + input ProjectOrderByInput { + field: String! + direction: OrderDirection + } + + enum OrderDirection { + ASC + DESC + } + type ProjectSelector { organization: ID! project: ID! @@ -80,7 +91,7 @@ test('collect fields', async () => { schema, max: 1, }); - const info = collect(op).value; + const info = collect(op, {}).value; expect(info.fields).toContain(`Mutation.deleteProject`); expect(info.fields).toContain(`Project.id`); @@ -91,7 +102,7 @@ test('collect input object types', async () => { schema, max: 1, }); - const info = collect(op).value; + const info = collect(op, {}).value; expect(info.fields).toContain(`ProjectSelectorInput.organization`); expect(info.fields).toContain(`ProjectSelectorInput.project`); @@ -109,7 +120,8 @@ test('collect enums and scalars as inputs', async () => { id } } - `) + `), + {} ).value; expect(info.fields).toContain(`Int`); @@ -131,7 +143,8 @@ test('collect enum values from object fields', async () => { id } } - `) + `), + {} ).value; expect(info.fields).toContain(`Int`); @@ -153,7 +166,8 @@ test('collect enum values from arguments', async () => { id } } - `) + `), + {} ).value; expect(info.fields).toContain(`ProjectType.FEDERATION`); @@ -174,7 +188,8 @@ test('collect arguments', async () => { id } } - `) + `), + {} ).value; expect(info.fields).toContain(`Query.projects.filter`); @@ -192,7 +207,8 @@ test('collect used-only input fields', async () => { id } } - `) + `), + {} ).value; expect(info.fields).toContain(`FilterInput.pagination`); @@ -201,7 +217,7 @@ test('collect used-only input fields', async () => { expect(info.fields).not.toContain(`PaginationInput.offset`); }); -test('collect all input fields when it is impossible to pick only those used', async () => { +test('collect all input fields when `processVariables` has not been passed and input is passed as a variable', async () => { const collect = createCollector({ schema, max: 1, @@ -213,7 +229,8 @@ test('collect all input fields when it is impossible to pick only those used', a id } } - `) + `), + {} ).value; expect(info.fields).toContain(`FilterInput.pagination`); @@ -221,3 +238,72 @@ test('collect all input fields when it is impossible to pick only those used', a expect(info.fields).toContain(`PaginationInput.limit`); expect(info.fields).toContain(`PaginationInput.offset`); }); + +test('collect used-only input fields if input is passed as a variable', async () => { + const collect = createCollector({ + schema, + max: 1, + processVariables: true, + }); + const info = collect( + parse(/* GraphQL */ ` + query getProjects($pagination: PaginationInput!, $type: ProjectType!) { + projects(filter: { pagination: $pagination, type: $type }) { + id + } + } + `), + { + pagination: { + limit: 1, + }, + type: 'STITCHING', + } + ).value; + + expect(info.fields).toContain(`FilterInput.pagination`); + expect(info.fields).toContain(`FilterInput.type`); + expect(info.fields).toContain(`PaginationInput.limit`); + expect(info.fields).not.toContain(`PaginationInput.offset`); +}); + +test('collect used-only input fields if input array is passed as a variable', async () => { + const collect = createCollector({ + schema, + max: 1, + processVariables: true, + }); + const info = collect( + parse(/* GraphQL */ ` + query getProjects($filter: FilterInput) { + projects(filter: $filter) { + id + } + } + `), + { + filter: { + order: [ + { + field: 'name', + }, + { + field: 'buildUrl', + direction: 'DESC', + }, + ], + pagination: { + limit: 10, + }, + }, + } + ).value; + + expect(info.fields).toContain(`FilterInput.pagination`); + expect(info.fields).toContain(`PaginationInput.limit`); + expect(info.fields).toContain(`FilterInput.order`); + expect(info.fields).toContain(`ProjectOrderByInput.field`); + expect(info.fields).toContain(`ProjectOrderByInput.direction`); + expect(info.fields).not.toContain(`FilterInput.type`); + expect(info.fields).not.toContain(`PaginationInput.offset`); +});