diff --git a/controlplane/src/core/bufservices/proposal/createProposal.ts b/controlplane/src/core/bufservices/proposal/createProposal.ts index 0ecc7a091b..17e499193b 100644 --- a/controlplane/src/core/bufservices/proposal/createProposal.ts +++ b/controlplane/src/core/bufservices/proposal/createProposal.ts @@ -12,6 +12,7 @@ import { Composer } from '../../composition/composer.js'; import { AuditLogRepository } from '../../repositories/AuditLogRepository.js'; import { ContractRepository } from '../../repositories/ContractRepository.js'; import { FederatedGraphRepository } from '../../repositories/FederatedGraphRepository.js'; +import { FeatureFlagRepository } from '../../repositories/FeatureFlagRepository.js'; import { GraphCompositionRepository } from '../../repositories/GraphCompositionRepository.js'; import { DefaultNamespace, NamespaceRepository } from '../../repositories/NamespaceRepository.js'; import { OrganizationRepository } from '../../repositories/OrganizationRepository.js'; @@ -420,6 +421,7 @@ export function createProposal( const schemaCheckRepo = new SchemaCheckRepository(opts.db); const contractRepo = new ContractRepository(logger, opts.db, authContext.organizationId); const graphCompostionRepo = new GraphCompositionRepository(logger, opts.db); + const featureFlagRepo = new FeatureFlagRepository(logger, opts.db, authContext.organizationId); const trafficInspector = new SchemaUsageTrafficInspector(opts.chClient!); const composer = new Composer( logger, @@ -428,6 +430,7 @@ export function createProposal( subgraphRepo, contractRepo, graphCompostionRepo, + featureFlagRepo, opts.chClient, opts.webhookProxyUrl, ); diff --git a/controlplane/src/core/bufservices/proposal/updateProposal.ts b/controlplane/src/core/bufservices/proposal/updateProposal.ts index b3de861308..6575a9ba77 100644 --- a/controlplane/src/core/bufservices/proposal/updateProposal.ts +++ b/controlplane/src/core/bufservices/proposal/updateProposal.ts @@ -16,6 +16,7 @@ import { enrichLogger, getLogger, handleError } from '../../util.js'; import { AuditLogRepository } from '../../repositories/AuditLogRepository.js'; import { ContractRepository } from '../../repositories/ContractRepository.js'; import { FederatedGraphRepository } from '../../repositories/FederatedGraphRepository.js'; +import { FeatureFlagRepository } from '../../repositories/FeatureFlagRepository.js'; import { GraphCompositionRepository } from '../../repositories/GraphCompositionRepository.js'; import { DefaultNamespace, NamespaceRepository } from '../../repositories/NamespaceRepository.js'; import { OrganizationRepository } from '../../repositories/OrganizationRepository.js'; @@ -485,6 +486,7 @@ export function updateProposal( const schemaCheckRepo = new SchemaCheckRepository(opts.db); const contractRepo = new ContractRepository(logger, opts.db, authContext.organizationId); const graphCompostionRepo = new GraphCompositionRepository(logger, opts.db); + const featureFlagRepo = new FeatureFlagRepository(logger, opts.db, authContext.organizationId); const trafficInspector = new SchemaUsageTrafficInspector(opts.chClient!); const composer = new Composer( logger, @@ -493,6 +495,7 @@ export function updateProposal( subgraphRepo, contractRepo, graphCompostionRepo, + featureFlagRepo, opts.chClient, opts.webhookProxyUrl, ); diff --git a/controlplane/src/core/bufservices/subgraph/fixSubgraphSchema.ts b/controlplane/src/core/bufservices/subgraph/fixSubgraphSchema.ts index 4831a0a500..d5d3052516 100644 --- a/controlplane/src/core/bufservices/subgraph/fixSubgraphSchema.ts +++ b/controlplane/src/core/bufservices/subgraph/fixSubgraphSchema.ts @@ -11,6 +11,7 @@ import { buildSchema } from '../../composition/composition.js'; import { OpenAIGraphql } from '../../openai-graphql/index.js'; import { ContractRepository } from '../../repositories/ContractRepository.js'; import { FederatedGraphRepository } from '../../repositories/FederatedGraphRepository.js'; +import { FeatureFlagRepository } from '../../repositories/FeatureFlagRepository.js'; import { GraphCompositionRepository } from '../../repositories/GraphCompositionRepository.js'; import { DefaultNamespace, NamespaceRepository } from '../../repositories/NamespaceRepository.js'; import { OrganizationRepository } from '../../repositories/OrganizationRepository.js'; @@ -33,6 +34,7 @@ export function fixSubgraphSchema( const subgraphRepo = new SubgraphRepository(logger, opts.db, authContext.organizationId); const contractRepo = new ContractRepository(logger, opts.db, authContext.organizationId); const graphCompositionRepo = new GraphCompositionRepository(logger, opts.db); + const featureFlagRepo = new FeatureFlagRepository(logger, opts.db, authContext.organizationId); const namespaceRepo = new NamespaceRepository(opts.db, authContext.organizationId); const composer = new Composer( @@ -42,6 +44,7 @@ export function fixSubgraphSchema( subgraphRepo, contractRepo, graphCompositionRepo, + featureFlagRepo, opts.chClient, opts.webhookProxyUrl, ); diff --git a/controlplane/src/core/composition/composer.ts b/controlplane/src/core/composition/composer.ts index 6220409f22..ad0d3cb85f 100644 --- a/controlplane/src/core/composition/composer.ts +++ b/controlplane/src/core/composition/composer.ts @@ -17,11 +17,7 @@ import { RouterConfigSchema, } from '@wundergraph/cosmo-connect/dist/node/v1/node_pb'; -import type { - FeatureFlagRouterExecutionConfig, - FeatureFlagRouterExecutionConfigs, - RouterConfig, -} from '@wundergraph/cosmo-connect/dist/node/v1/node_pb'; +import type { FeatureFlagRouterExecutionConfig, RouterConfig } from '@wundergraph/cosmo-connect/dist/node/v1/node_pb'; import { PostgresJsDatabase } from 'drizzle-orm/postgres-js'; import { FederatedGraphDTO, Label, SubgraphDTO } from '../../types/index.js'; @@ -29,6 +25,7 @@ import { BlobStorage } from '../blobstorage/index.js'; import { audiences, nowInSeconds, signJwtHS256 } from '../crypto/jwt.js'; import { ContractRepository } from '../repositories/ContractRepository.js'; import { FederatedGraphRepository } from '../repositories/FederatedGraphRepository.js'; +import { FeatureFlagRepository } from '../repositories/FeatureFlagRepository.js'; import { SubgraphRepository } from '../repositories/SubgraphRepository.js'; import { AdmissionError, @@ -157,6 +154,7 @@ export class Composer { private subgraphRepo: SubgraphRepository, private contractRepo: ContractRepository, private graphCompositionRepository: GraphCompositionRepository, + private featureFlagRepo: FeatureFlagRepository, private chClient?: ClickHouseClient, private proxyUrl?: string, ) {} @@ -462,9 +460,17 @@ export class Composer { featureFlagId: string; splitConfigEnabled: boolean; }): Promise { - const prevValidFederatedSDL = await this.federatedGraphRepo.getLatestValidSchemaVersion({ - targetId: composedGraph.targetID, - }); + // For a feature-flag composition the baseline is the previous composition of the same feature flag (not the base + // graph's latest valid version, which would produce a meaningless base-vs-feature-flag diff). Computed before + // addSchemaVersion inserts the new version, so it resolves to the chronologically prior feature-flag version. + const prevValidFederatedSDL = isFeatureFlagComposition + ? await this.featureFlagRepo.getLatestValidFeatureFlagSchemaVersion({ + targetId: composedGraph.targetID, + featureFlagId, + }) + : await this.federatedGraphRepo.getLatestValidSchemaVersion({ + targetId: composedGraph.targetID, + }); const updatedFederatedGraph = await this.federatedGraphRepo.addSchemaVersion({ targetId: composedGraph.targetID, @@ -480,8 +486,8 @@ export class Composer { splitConfigEnabled, }); - // If the composed schema is invalid, or it is a feature flag composition, we do not create a changelog - if (!routerExecutionConfig || !updatedFederatedGraph?.composedSchemaVersionId || isFeatureFlagComposition) { + // If the composed schema is invalid, we do not create a changelog + if (!routerExecutionConfig || !updatedFederatedGraph?.composedSchemaVersionId) { return { schemaVersionId: updatedFederatedGraph?.composedSchemaVersionId || '', }; diff --git a/controlplane/src/core/repositories/FeatureFlagRepository.ts b/controlplane/src/core/repositories/FeatureFlagRepository.ts index 4f79dabf8e..330fdd7edf 100644 --- a/controlplane/src/core/repositories/FeatureFlagRepository.ts +++ b/controlplane/src/core/repositories/FeatureFlagRepository.ts @@ -1618,6 +1618,41 @@ export class FeatureFlagRepository { return ffSchemaVersion; } + public async getLatestValidFeatureFlagSchemaVersion(data: { targetId: string; featureFlagId: string }) { + const latest = await this.db + .select({ + schema: schemaVersion.schemaSDL, + clientSchema: schemaVersion.clientSchema, + schemaVersionId: schemaVersion.id, + }) + .from(federatedGraphsToFeatureFlagSchemaVersions) + .innerJoin( + schemaVersion, + eq(schemaVersion.id, federatedGraphsToFeatureFlagSchemaVersions.composedSchemaVersionId), + ) + .innerJoin(graphCompositions, eq(graphCompositions.schemaVersionId, schemaVersion.id)) + .where( + and( + eq(schemaVersion.targetId, data.targetId), + eq(schemaVersion.organizationId, this.organizationId), + eq(federatedGraphsToFeatureFlagSchemaVersions.featureFlagId, data.featureFlagId), + eq(graphCompositions.isFeatureFlagComposition, true), + eq(graphCompositions.isComposable, true), + or(isNull(graphCompositions.deploymentError), eq(graphCompositions.deploymentError, '')), + or(isNull(graphCompositions.admissionError), eq(graphCompositions.admissionError, '')), + ), + ) + .orderBy(desc(graphCompositions.createdAt)) + .limit(1) + .execute(); + + if (latest.length === 0) { + return undefined; + } + + return latest[0]; + } + public async delete(featureFlagId: string) { await this.db .delete(featureFlags) diff --git a/controlplane/src/core/repositories/SubgraphRepository.ts b/controlplane/src/core/repositories/SubgraphRepository.ts index 15bd99afa9..eed274a6e7 100644 --- a/controlplane/src/core/repositories/SubgraphRepository.ts +++ b/controlplane/src/core/repositories/SubgraphRepository.ts @@ -2515,6 +2515,7 @@ export class SubgraphRepository { subgraphRepo, contractRepo, graphCompostionRepo, + featureFlagRepo, chClient, webhookProxyUrl, ); diff --git a/controlplane/src/core/services/CompositionService.ts b/controlplane/src/core/services/CompositionService.ts index f9564864a2..6de83ac2d7 100644 --- a/controlplane/src/core/services/CompositionService.ts +++ b/controlplane/src/core/services/CompositionService.ts @@ -439,6 +439,7 @@ export class CompositionService { new SubgraphRepository(this.logger, this.db, this.organizationId), new ContractRepository(this.logger, this.db, this.organizationId), new GraphCompositionRepository(this.logger, this.db), + new FeatureFlagRepository(this.logger, this.db, this.organizationId), this.chClient, this.webhookProxyUrl, ); @@ -1328,6 +1329,7 @@ export class CompositionService { new SubgraphRepository(this.logger, this.db, this.organizationId), new ContractRepository(this.logger, this.db, this.organizationId), new GraphCompositionRepository(this.logger, this.db), + new FeatureFlagRepository(this.logger, this.db, this.organizationId), this.chClient, this.webhookProxyUrl, ); diff --git a/controlplane/test/composition/getCompositionDetails.test.ts b/controlplane/test/composition/getCompositionDetails.test.ts index 9a8bf6de87..e58587bbae 100644 --- a/controlplane/test/composition/getCompositionDetails.test.ts +++ b/controlplane/test/composition/getCompositionDetails.test.ts @@ -603,4 +603,212 @@ describe('getCompositionDetails', () => { ]); }, ); + + test( + 'that a feature flag composition changelog reflects the schema changes against the previous feature flag composition', + getDebugTestOptions(isDebugMode), + async (testContext) => { + const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] }); + testContext.onTestFinished(() => server.close()); + + const namespace = genID('namespace').toLowerCase(); + const labels: Label[] = []; + const baseGraphName = genID('baseFederatedGraphName'); + const ffName = genID('feature-flag'); + + await createNamespace(client, namespace); + await featureFlagIntegrationTestSetUp( + client, + [ + { name: 'users', hasFeatureSubgraph: true }, + { name: 'products-standalone', hasFeatureSubgraph: true }, + ], + baseGraphName, + labels, + namespace, + ); + + await createFeatureFlag(client, ffName, labels, ['products-standalone-feature'], namespace, true); + + // Update the feature subgraph with a genuinely new client-visible field, producing a feature flag composition + // whose supergraph differs from the previous (creation) one. + const updateSubgraphResp = await client.publishFederatedSubgraph({ + name: 'products-standalone-feature', + namespace, + schema: ` + type Product @key(fields: "upc sku") { + upc: Int! + sku: String! + details: String! + isPremium: Boolean! @tag(name: "exclude") + newField: String! + changelogTestField: String! + } + + type Query { + products: [Product!]! + } + `, + }); + expect(updateSubgraphResp.response?.code).toBe(EnumStatusCode.OK); + + const compositionsResp = await client.getCompositions({ + fedGraphName: baseGraphName, + namespace, + startDate: formatISO(subDays(new Date(), 1)), + endDate: formatISO(addMinutes(new Date(), 1)), + excludeFeatureFlagCompositions: false, + }); + expect(compositionsResp.response?.code).toBe(EnumStatusCode.OK); + + // The latest composition is the feature flag composition triggered by the feature subgraph update. + const latestFfComposition = compositionsResp.compositions[0]; + expect(latestFfComposition.isFeatureFlagComposition).toBe(true); + expect(latestFfComposition.featureFlagName).toBe(ffName); + + const detailsResp = await client.getCompositionDetails({ + compositionId: latestFfComposition.id, + namespace, + }); + expect(detailsResp.response?.code).toBe(EnumStatusCode.OK); + // The changelog is diffed against the PREVIOUS feature flag composition of the same flag (not the base graph), + // so the single added field (`changelogTestField`) must be reflected as exactly one addition and no deletions. + expect(detailsResp.changeCounts?.additions).toBe(2); + expect(detailsResp.changeCounts?.deletions).toBe(0); + }, + ); + + test( + 'that publishing a base subgraph without a feature subgraph produces a feature flag composition changelog', + getDebugTestOptions(isDebugMode), + async (testContext) => { + const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] }); + testContext.onTestFinished(() => server.close()); + + const namespace = genID('namespace').toLowerCase(); + const labels: Label[] = []; + const baseGraphName = genID('baseFederatedGraphName'); + const ffName = genID('feature-flag'); + + await createNamespace(client, namespace); + await featureFlagIntegrationTestSetUp( + client, + [ + // `users` has no feature subgraph, so the feature flag composition consumes the base `users` schema. + { name: 'users', hasFeatureSubgraph: false }, + { name: 'products-standalone', hasFeatureSubgraph: true }, + ], + baseGraphName, + labels, + namespace, + ); + + await createFeatureFlag(client, ffName, labels, ['products-standalone-feature'], namespace, true); + + // Publish the base `users` subgraph with a genuinely new client-visible field. Even though `users` is not + // overridden by a feature subgraph, it is part of the feature flag composition, so this must trigger a new + // feature flag composition whose supergraph differs from the previous (creation) one. + const updateSubgraphResp = await client.publishFederatedSubgraph({ + name: 'users', + namespace, + schema: ` + type User @key(fields: "id") { + id: ID! + name: String! + email: String! + isPremium: Boolean! @tag(name: "exclude") + changelogTestField: String! + } + + type Query { + user(id: ID!): User + users: [User!]! + } + `, + }); + expect(updateSubgraphResp.response?.code).toBe(EnumStatusCode.OK); + + const compositionsResp = await client.getCompositions({ + fedGraphName: baseGraphName, + namespace, + startDate: formatISO(subDays(new Date(), 1)), + endDate: formatISO(addMinutes(new Date(), 1)), + excludeFeatureFlagCompositions: false, + }); + expect(compositionsResp.response?.code).toBe(EnumStatusCode.OK); + + // Publishing a base subgraph triggers both a base composition and a feature flag composition (compositions + // are returned newest-first), so pick the most recent feature flag composition for the flag rather than + // assuming it is the very latest entry. + const latestFfComposition = compositionsResp.compositions.find( + (c) => c.isFeatureFlagComposition && c.featureFlagName === ffName, + ); + expect(latestFfComposition).toBeDefined(); + + const detailsResp = await client.getCompositionDetails({ + compositionId: latestFfComposition!.id, + namespace, + }); + expect(detailsResp.response?.code).toBe(EnumStatusCode.OK); + // The changelog is diffed against the PREVIOUS feature flag composition of the same flag, so the single added + // field (`changelogTestField`) must be reflected as exactly one addition and no deletions. + expect(detailsResp.changeCounts?.additions).toBe(1); + expect(detailsResp.changeCounts?.deletions).toBe(0); + }, + ); + + test( + 'that recomposing a feature flag without schema changes produces an empty changelog', + getDebugTestOptions(isDebugMode), + async (testContext) => { + const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] }); + testContext.onTestFinished(() => server.close()); + + const namespace = genID('namespace').toLowerCase(); + const labels: Label[] = []; + const baseGraphName = genID('baseFederatedGraphName'); + const ffName = genID('feature-flag'); + + await createNamespace(client, namespace); + await featureFlagIntegrationTestSetUp( + client, + [ + { name: 'users', hasFeatureSubgraph: true }, + { name: 'products-standalone', hasFeatureSubgraph: true }, + ], + baseGraphName, + labels, + namespace, + ); + + await createFeatureFlag(client, ffName, labels, ['products-standalone-feature'], namespace, true); + + // Recompose the feature flag without changing any schema; this forces a new feature flag composition. + const recomposeResp = await client.recomposeFeatureFlag({ name: ffName, namespace }); + expect(recomposeResp.response?.code).toBe(EnumStatusCode.OK); + + const compositionsResp = await client.getCompositions({ + fedGraphName: baseGraphName, + namespace, + startDate: formatISO(subDays(new Date(), 1)), + endDate: formatISO(addMinutes(new Date(), 1)), + excludeFeatureFlagCompositions: false, + }); + expect(compositionsResp.response?.code).toBe(EnumStatusCode.OK); + + // The latest composition is the recompose, whose supergraph is identical to the previous feature flag + // composition. A no-op recompose must not add anything to the changelog. + const recomposition = compositionsResp.compositions[0]; + expect(recomposition.isFeatureFlagComposition).toBe(true); + expect(recomposition.featureFlagName).toBe(ffName); + + const detailsResp = await client.getCompositionDetails({ + compositionId: recomposition.id, + namespace, + }); + expect(detailsResp.response?.code).toBe(EnumStatusCode.OK); + expect(detailsResp.changeCounts?.additions ?? 0).toBe(0); + expect(detailsResp.changeCounts?.deletions ?? 0).toBe(0); + }, + ); });