Improve #379 and add more tests (#397)

packages/libraries/client/src/internal/types.ts

@@ -66,7 +66,7 @@ export interface HiveUsagePluginOptions {
    */
   sampleRate?: number;
   /**
-   * Enables collecting Input fields usage based on the variables passed to the operation.
+   * (Experimental) Enables collecting Input fields usage based on the variables passed to the operation.
    *
    * Default: false
    */

packages/libraries/client/src/internal/usage.ts

@@ -158,7 +158,7 @@ export function createUsage(pluginOptions: HivePluginOptions): UsageCollector {
               ttl: options.ttl,
               processVariables: options.processVariables ?? false,
             });
-            const { key, value: info } = collect(document, args.variableValues);
+            const { key, value: info } = collect(document, args.variableValues ?? null);
 
             agent.capture({
               key,
@@ -196,7 +196,7 @@ export function createCollector({
   schema,
   max,
   ttl,
-  processVariables,
+  processVariables = false,
 }: {
   schema: GraphQLSchema;
   max?: number;
@@ -205,9 +205,15 @@ export function createCollector({
 }) {
   const typeInfo = new TypeInfo(schema);
 
-  function collect(doc: DocumentNode, variables: ExecutionArgs['variableValues']): CacheResult {
+  function collect(
+    doc: DocumentNode,
+    variables: {
+      [key: string]: unknown;
+    } | null
+  ): CacheResult {
     const entries = new Set<string>();
     const collected_entire_named_types = new Set<string>();
+    const shouldAnalyzeVariableValues = processVariables === true && variables !== null;
 
     function markAsUsed(id: string) {
       if (!entries.has(id)) {
@@ -249,11 +255,16 @@ export function createCollector({
       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 &&
-        node.value.kind !== Kind.VARIABLE
-      ) {
+      } else if (node.value.kind !== Kind.OBJECT && node.value.kind !== Kind.LIST) {
+        // When processing of variables is enabled,
+        // we want to skip collecting full input types of variables
+        // and only collect specific fields.
+        // That's why the following condition is added.
+        // Otherwise we would mark entire input types as used, and not granular fields.
+        if (node.value.kind === Kind.VARIABLE && shouldAnalyzeVariableValues) {
+          return;
+        }
+
         collectInputType(inputTypeName);
       }
     }
@@ -326,13 +337,17 @@ export function createCollector({
         },
         VariableDefinition(node) {
           const inputType = typeInfo.getInputType()!;
-          if (!variables) {
-            collectInputType(resolveTypeName(inputType));
-          } else {
+
+          if (shouldAnalyzeVariableValues) {
+            // Granular collection of variable values is enabled
             const variableName = node.variable.name.value;
             const variableValue = variables[variableName];
             const namedType = unwrapType(inputType);
+
             collectVariable(namedType, variableValue);
+          } else {
+            // Collect the entire type without processing the variables
+            collectInputType(resolveTypeName(inputType));
           }
         },
         Argument(node) {
@@ -386,12 +401,7 @@ export function createCollector({
     };
   }
 
-  return cache(
-    (doc: DocumentNode, variables: ExecutionArgs['variableValues']) =>
-      collect(doc, processVariables ? variables : undefined),
-    cacheDocumentKey,
-    LRU<CacheResult>(max, ttl)
-  );
+  return cache(collect, cacheDocumentKey, LRU<CacheResult>(max, ttl));
 }
 
 function resolveTypeName(inputType: GraphQLType): string {

packages/libraries/client/src/internal/utils.ts

@@ -52,18 +52,25 @@ export function cache<R, A, K, V>(
   };
 }
 
-export function cacheDocumentKey<T, V>(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;
-      }
+export function cacheDocumentKey<T, V>(doc: T, variables: V | null) {
+  const hasher = createHash('md5').update(JSON.stringify(doc));
 
-      return '';
-    });
+  if (variables) {
+    hasher.update(
+      JSON.stringify(variables, (_, 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');
+
+  return hasher.digest('hex');
 }
 
 const HR_TO_NS = 1e9;

packages/libraries/client/tests/usage-collector.spec.ts

@@ -239,7 +239,7 @@ test('collect all input fields when `processVariables` has not been passed and i
   expect(info.fields).toContain(`PaginationInput.offset`);
 });
 
-test('collect used-only input fields if input is passed as a variable', async () => {
+test('(processVariables: true) collect used-only input fields', async () => {
   const collect = createCollector({
     schema,
     max: 1,
@@ -267,7 +267,7 @@ test('collect used-only input fields if input is passed as a variable', async ()
   expect(info.fields).not.toContain(`PaginationInput.offset`);
 });
 
-test('collect used-only input fields if input array is passed as a variable', async () => {
+test('(processVariables: true) collect used-only input type fields from an array', async () => {
   const collect = createCollector({
     schema,
     max: 1,

packages/libraries/client/tests/utils.spec.ts

@@ -0,0 +1,43 @@
+import { cacheDocumentKey } from '../src/internal/utils';
+
+test('produce identical hash for the same document and the same keys but different values in variables', () => {
+  const left = cacheDocumentKey('doc', { a: true });
+  const right = cacheDocumentKey('doc', { a: false });
+  expect(left).toEqual(right);
+});
+
+test('produce identical hash for the same document but with an empty array', () => {
+  const left = cacheDocumentKey('doc', { a: [] });
+  const right = cacheDocumentKey('doc', { a: [] });
+  expect(left).toEqual(right);
+});
+
+test('produce identical hash for the same document but with and without an empty array', () => {
+  const left = cacheDocumentKey('doc', { a: [] });
+  const right = cacheDocumentKey('doc', { a: null });
+  expect(left).toEqual(right);
+});
+
+test('produce identical hash for the same document but with an array of primitive values', () => {
+  const left = cacheDocumentKey('doc', { a: [1, 2, 3] });
+  const right = cacheDocumentKey('doc', { a: [4, 5, 6] });
+  expect(left).toEqual(right);
+});
+
+test('produce different hash for the same document but with different keys in variables', () => {
+  const left = cacheDocumentKey('doc', { a: true });
+  const right = cacheDocumentKey('doc', { b: true });
+  expect(left).not.toEqual(right);
+});
+
+test('produce different hash for the same document but with and without variables', () => {
+  const left = cacheDocumentKey('doc', { a: true });
+  const right = cacheDocumentKey('doc', null);
+  expect(left).not.toEqual(right);
+});
+
+test('produce different hash for the same document but with and without variables (empty object)', () => {
+  const left = cacheDocumentKey('doc', { a: true });
+  const right = cacheDocumentKey('doc', {});
+  expect(left).not.toEqual(right);
+});