Skip to content

Commit

Permalink
feat: add breaking changes total operations option (#6254)
Browse files Browse the repository at this point in the history
  • Loading branch information
jdolle authored Jan 14, 2025
1 parent 63cdff0 commit b58d2c5
Show file tree
Hide file tree
Showing 13 changed files with 372 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/sharp-melons-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'hive': minor
---

Add option for checking breaking changes by a fixed request count
7 changes: 7 additions & 0 deletions integration-tests/testkit/seed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
updateTargetValidationSettings,
} from './flow';
import {
BreakingChangeFormula,
OrganizationAccessScope,
ProjectAccessScope,
ProjectType,
Expand Down Expand Up @@ -632,9 +633,13 @@ export function initSeed() {
excludedClients,
percentage,
target: ttarget = target,
requestCount,
breakingChangeFormula,
}: {
excludedClients?: string[];
percentage: number;
requestCount?: number;
breakingChangeFormula?: BreakingChangeFormula;
target?: TargetOverwrite;
}) {
const result = await updateTargetValidationSettings(
Expand All @@ -644,6 +649,8 @@ export function initSeed() {
targetSlug: ttarget.slug,
excludedClients,
percentage,
requestCount,
breakingChangeFormula,
period: 2,
targetIds: [target.id],
},
Expand Down
166 changes: 164 additions & 2 deletions integration-tests/tests/api/target/usage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { subHours } from 'date-fns/subHours';
import { buildASTSchema, buildSchema, parse, print, TypeInfo } from 'graphql';
import { createLogger } from 'graphql-yoga';
import { graphql } from 'testkit/gql';
import { ProjectType } from 'testkit/gql/graphql';
import { BreakingChangeFormula, ProjectType } from 'testkit/gql/graphql';
import { execute } from 'testkit/graphql';
import { getServiceHost } from 'testkit/utils';
import { UTCDate } from '@date-fns/utc';
Expand Down Expand Up @@ -2135,7 +2135,7 @@ const SubscriptionSchemaCheckQuery = graphql(/* GraphQL */ `
`);

test.concurrent(
'test threshold when using conditional breaking change detection',
'test threshold when using conditional breaking change "PERCENTAGE" detection',
async ({ expect }) => {
const { createOrg } = await initSeed().createOwner();
const { createProject } = await createOrg();
Expand Down Expand Up @@ -2347,6 +2347,168 @@ test.concurrent(
},
);

test.concurrent(
'test threshold when using conditional breaking change "REQUEST_COUNT" detection',
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
}
`);

function collectA() {
client.collectUsage()(
{
document: queryA,
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 unused = await token
.checkSchema(/* GraphQL */ `
type Query {
b: String
c: String
}
`)
.then(r => r.expectNoGraphQLErrors());

if (unused.schemaCheck.__typename !== 'SchemaCheckSuccess') {
throw new Error(`Expected SchemaCheckSuccess, got ${unused.schemaCheck.__typename}`);
}

expect(unused.schemaCheck.changes).toEqual(
expect.objectContaining({
nodes: expect.arrayContaining([
expect.objectContaining({
message: "Field 'a' was removed from object type 'Query' (non-breaking based on usage)",
}),
]),
total: 1,
}),
);

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();

await waitFor(8000);

const below = await token
.checkSchema(/* GraphQL */ `
type Query {
b: String
c: String
}
`)
.then(r => r.expectNoGraphQLErrors());

if (below.schemaCheck.__typename !== 'SchemaCheckSuccess') {
throw new Error(`Expected SchemaCheckSuccess, got ${below.schemaCheck.__typename}`);
}

expect(below.schemaCheck.changes).toEqual(
expect.objectContaining({
nodes: expect.arrayContaining([
expect.objectContaining({
message: "Field 'a' was removed from object type 'Query' (non-breaking based on usage)",
}),
]),
total: 1,
}),
);

// Now let's make Query.a above threshold by making a 2nd query for Query.a
collectA();

await waitFor(8000);

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 }) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { type MigrationExecutor } from '../pg-migrator';

export default {
name: '2025.01.10T00.00.00.breaking-changes-request-count.ts',
run: ({ sql }) => sql`
CREATE TYPE
breaking_change_formula AS ENUM('PERCENTAGE', 'REQUEST_COUNT');
ALTER TABLE
targets
ADD COLUMN
validation_request_count INT NOT NULL DEFAULT 1;
ALTER TABLE
targets
ADD COLUMN
validation_breaking_change_formula breaking_change_formula NOT NULL DEFAULT 'PERCENTAGE';
`,
} satisfies MigrationExecutor;
1 change: 1 addition & 0 deletions packages/migrations/src/run-pg-migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,5 +153,6 @@ export const runPGMigrations = async (args: { slonik: DatabasePool; runTo?: stri
await import('./actions/2025.01.02T00-00-00.cascade-deletion-indices'),
await import('./actions/2025.01.02T00-00-00.legacy-user-org-cleanup'),
await import('./actions/2025.01.09T00-00-00.legacy-member-scopes'),
await import('./actions/2025.01.10T00.00.00.breaking-changes-request-count'),
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ type ConditionalBreakingChangeConfiguration = {
conditionalBreakingChangeDiffConfig: ConditionalBreakingChangeDiffConfig;
retentionInDays: number;
percentage: number;
requestCount: number;
breakingChangeFormula: 'PERCENTAGE' | 'REQUEST_COUNT';
totalRequestCount: number;
};

Expand Down Expand Up @@ -196,12 +198,15 @@ export class SchemaPublisher {
excludedClientNames: settings.validation.excludedClients?.length
? settings.validation.excludedClients
: null,
requestCountThreshold: Math.ceil(
totalRequestCount * (settings.validation.percentage / 100),
),
requestCountThreshold:
settings.validation.breakingChangeFormula === 'PERCENTAGE'
? Math.ceil(totalRequestCount * (settings.validation.percentage / 100))
: settings.validation.requestCount,
},
retentionInDays: settings.validation.period,
percentage: settings.validation.percentage,
requestCount: settings.validation.requestCount,
breakingChangeFormula: settings.validation.breakingChangeFormula,
totalRequestCount,
};
} catch (error: unknown) {
Expand All @@ -228,6 +233,8 @@ export class SchemaPublisher {
retentionInDays: args.conditionalBreakingChangeConfiguration.retentionInDays,
excludedClientNames: conditionalBreakingChangeDiffConfig.excludedClientNames,
percentage: args.conditionalBreakingChangeConfiguration.percentage,
requestCount: args.conditionalBreakingChangeConfiguration.requestCount,
breakingChangeFormula: args.conditionalBreakingChangeConfiguration.breakingChangeFormula,
targets: await Promise.all(
conditionalBreakingChangeDiffConfig.targetIds.map(async targetId => {
return {
Expand Down
27 changes: 27 additions & 0 deletions packages/services/api/src/modules/target/module.graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ export default gql`
targetSlug: String!
period: Int!
percentage: Float!
requestCount: Int! = 1
breakingChangeFormula: BreakingChangeFormula! = PERCENTAGE
targetIds: [ID!]!
excludedClients: [String!]
}
Expand All @@ -122,6 +124,7 @@ export default gql`
type UpdateTargetValidationSettingsInputErrors {
percentage: String
period: String
requestCount: String
}
type UpdateTargetValidationSettingsError implements Error {
Expand Down Expand Up @@ -178,11 +181,35 @@ export default gql`
type TargetValidationSettings {
enabled: Boolean!
period: Int!
"""
If TargetValidationSettings.breakingChangeFormula is PERCENTAGE, then this
is the percent of the total operations over the TargetValidationSettings.period
required for a change to be considered breaking.
"""
percentage: Float!
"""
If TargetValidationSettings.breakingChangeFormula is REQUEST_COUNT, then this
is the total number of operations over the TargetValidationSettings.period
required for a change to be considered breaking.
"""
requestCount: Int!
"""
Determines which formula is used to determine if a change is considered breaking
or not. Only one formula can be used at a time.
"""
breakingChangeFormula: BreakingChangeFormula!
targets: [Target!]!
excludedClients: [String!]!
}
enum BreakingChangeFormula {
REQUEST_COUNT
PERCENTAGE
}
input CreateTargetInput {
organizationSlug: String!
projectSlug: String!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { OrganizationManager } from '../../../organization/providers/organizatio
import { IdTranslator } from '../../../shared/providers/id-translator';
import { TargetManager } from '../../providers/target-manager';
import { PercentageModel } from '../../validation';
import type { MutationResolvers } from './../../../../__generated__/types';
import { MutationResolvers } from './../../../../__generated__/types';

export const updateTargetValidationSettings: NonNullable<
MutationResolvers['updateTargetValidationSettings']
Expand All @@ -24,6 +24,8 @@ export const updateTargetValidationSettings: NonNullable<
period: z.number().min(1).max(org.monthlyRateLimit.retentionInDays).int(),
targetIds: z.array(z.string()).min(1),
excludedClients: z.optional(z.array(z.string())),
requestCount: z.number().min(1),
breakingChangeFormula: z.enum(['PERCENTAGE', 'REQUEST_COUNT']),
});

const result = UpdateTargetValidationSettingsModel.safeParse(input);
Expand All @@ -35,6 +37,7 @@ export const updateTargetValidationSettings: NonNullable<
inputErrors: {
percentage: result.error.formErrors.fieldErrors.percentage?.[0],
period: result.error.formErrors.fieldErrors.period?.[0],
requestCount: result.error.formErrors.fieldErrors.requestCount?.[0],
},
},
};
Expand All @@ -44,6 +47,8 @@ export const updateTargetValidationSettings: NonNullable<
await targetManager.updateTargetValidationSettings({
period: input.period,
percentage: input.percentage,
requestCount: input.requestCount ?? 1,
breakingChangeFormula: input.breakingChangeFormula ?? 'PERCENTAGE',
targetId: target,
projectId: project,
organizationId: organization,
Expand Down
2 changes: 2 additions & 0 deletions packages/services/api/src/shared/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,8 @@ export interface TargetSettings {
enabled: boolean;
period: number;
percentage: number;
requestCount: number;
breakingChangeFormula: 'PERCENTAGE' | 'REQUEST_COUNT';
targets: string[];
excludedClients: string[];
};
Expand Down
Loading

0 comments on commit b58d2c5

Please sign in to comment.