fix: check total request count against all top operations

.changeset/nine-meals-raise.md

@@ -0,0 +1,5 @@
+---
+'hive': patch
+---
+
+Use sum instead of max of top request counts for breaking changes calculation

integration-tests/tests/api/target/usage.spec.ts

@@ -2509,6 +2509,135 @@ test.concurrent(
   },
 );
 
+test.concurrent(
+  'test threshold when using conditional breaking change "REQUEST_COUNT" detection, across multiple operations',
+  async ({ expect }) => {
+    const { createOrg } = await initSeed().createOwner();
+    const { createProject } = await createOrg();
+    const { createTargetAccessToken, toggleTargetValidation, updateTargetValidationSettings } =
+      await createProject(ProjectType.Single);
+    const token = await createTargetAccessToken({});
+    await toggleTargetValidation(true);
+    await updateTargetValidationSettings({
+      excludedClients: [],
+      requestCount: 2,
+      percentage: 0,
+      breakingChangeFormula: BreakingChangeFormula.RequestCount,
+    });
+
+    const sdl = /* GraphQL */ `
+      type Query {
+        a: String
+        b: String
+        c: String
+      }
+    `;
+
+    const queryA = parse(/* GraphQL */ `
+      query {
+        a
+      }
+    `);
+    const queryB = parse(/* GraphQL */ `
+      query {
+        a
+        b
+      }
+    `);
+
+    function collectA() {
+      client.collectUsage()(
+        {
+          document: queryA,
+          schema,
+          contextValue: {
+            request,
+          },
+        },
+        {},
+      );
+    }
+    function collectB() {
+      client.collectUsage()(
+        {
+          document: queryB,
+          schema,
+          contextValue: {
+            request,
+          },
+        },
+        {},
+      );
+    }
+
+    const schema = buildASTSchema(parse(sdl));
+
+    const schemaPublishResult = await token
+      .publishSchema({
+        sdl,
+        author: 'Kamil',
+        commit: 'initial',
+      })
+      .then(res => res.expectNoGraphQLErrors());
+
+    expect(schemaPublishResult.schemaPublish.__typename).toEqual('SchemaPublishSuccess');
+
+    const usageAddress = await getServiceHost('usage', 8081);
+
+    const client = createHive({
+      enabled: true,
+      token: token.secret,
+      usage: true,
+      debug: false,
+      agent: {
+        logger: createLogger('debug'),
+        maxSize: 1,
+      },
+      selfHosting: {
+        usageEndpoint: 'http://' + usageAddress,
+        graphqlEndpoint: 'http://noop/',
+        applicationUrl: 'http://noop/',
+      },
+    });
+
+    const request = new Request('http://localhost:4000/graphql', {
+      method: 'POST',
+      headers: {
+        'x-graphql-client-name': 'integration-tests',
+        'x-graphql-client-version': '6.6.6',
+      },
+    });
+
+    collectA();
+    collectB();
+
+    await waitFor(8000);
+
+    // try to remove `Query.a`
+    const above = await token
+      .checkSchema(/* GraphQL */ `
+        type Query {
+          b: String
+          c: String
+        }
+      `)
+      .then(r => r.expectNoGraphQLErrors());
+
+    if (above.schemaCheck.__typename !== 'SchemaCheckError') {
+      throw new Error(`Expected SchemaCheckError, got ${above.schemaCheck.__typename}`);
+    }
+
+    expect(above.schemaCheck.errors).toEqual({
+      nodes: [
+        {
+          message: "Field 'a' was removed from object type 'Query'",
+        },
+      ],
+      total: 1,
+    });
+  },
+);
+
 test.concurrent(
   'subscription operation is used for conditional breaking change detection',
   async ({ expect }) => {

packages/services/api/src/modules/operations/providers/operations-reader.ts

@@ -926,7 +926,6 @@ export class OperationsReader {
     excludedClients: null | readonly string[];
     period: DateRange;
     schemaCoordinates: string[];
-    requestCountThreshold: number;
   }) {
     const RecordArrayType = z.array(
       z.object({
@@ -987,7 +986,7 @@ export class OperationsReader {
               AND "coordinates_daily"."timestamp" >= toDateTime(${formatDate(args.period.from)}, 'UTC')
               AND "coordinates_daily"."timestamp" <= toDateTime(${formatDate(args.period.to)}, 'UTC')
               AND "coordinates_daily"."coordinate" IN (${sql.longArray(args.schemaCoordinates, 'String')})
-            HAVING "total" >= ${String(args.requestCountThreshold)}
+            HAVING "total" >= 1
             ORDER BY
               "total" DESC,
               "coordinates_daily"."hash" DESC
@@ -1050,33 +1049,42 @@ export class OperationsReader {
   }
 
   /** Get the top operations for a given schema coordinate (uses batch loader underneath). */
-  getTopOperationsForSchemaCoordinate = batchBy<
-    {
-      targetIds: readonly string[];
-      excludedClients: null | readonly string[];
-      period: DateRange;
-      schemaCoordinate: string;
-      requestCountThreshold: number;
-    },
-    Array<{
-      hash: string;
-      name: string;
-      count: number;
-    }> | null
-  >(
-    item =>
-      `${item.targetIds.join(',')}-${item.excludedClients?.join(',') ?? ''}-${item.period.from.toISOString()}-${item.period.to.toISOString()}-${item.requestCountThreshold}`,
-    async items => {
-      const schemaCoordinates = items.map(item => item.schemaCoordinate);
-      return await this._getTopOperationsForSchemaCoordinates({
-        targetIds: items[0].targetIds,
-        excludedClients: items[0].excludedClients,
-        period: items[0].period,
-        requestCountThreshold: items[0].requestCountThreshold,
-        schemaCoordinates,
-      }).then(result => result.map(result => Promise.resolve(result)));
-    },
-  );
+  getTopOperationsForSchemaCoordinate = async (selector: {
+    targetIds: readonly string[];
+    excludedClients: null | readonly string[];
+    period: DateRange;
+    schemaCoordinate: string;
+    requestCountThreshold: number;
+  }) => {
+    const result = await batchBy<
+      typeof selector,
+      Array<{
+        hash: string;
+        name: string;
+        count: number;
+      }> | null
+    >(
+      item =>
+        `${item.targetIds.join(',')}-${item.excludedClients?.join(',') ?? ''}-${item.period.from.toISOString()}-${item.period.to.toISOString()}-${item.requestCountThreshold}`,
+      async items => {
+        const schemaCoordinates = items.map(item => item.schemaCoordinate);
+        return await this._getTopOperationsForSchemaCoordinates({
+          targetIds: items[0].targetIds,
+          excludedClients: items[0].excludedClients,
+          period: items[0].period,
+          schemaCoordinates,
+        }).then(result => result.map(result => Promise.resolve(result)));
+      },
+    )(selector);
+
+    if (
+      result &&
+      result.reduce((acc, { count }) => acc + count, 0) >= selector.requestCountThreshold
+    ) {
+      return result;
+    }
+    return null;
+  };
 
   /** Result array retains the order of the input `args.schemaCoordinates`. */
   private async _getTopClientsForSchemaCoordinates(args: {