diff --git a/controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts b/controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts index 08f5885297..a100d75fa2 100644 --- a/controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts +++ b/controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts @@ -81,7 +81,9 @@ export function getFeatureFlagsInLatestCompositionByFederatedGraph( namespaceId: namespace.id, includeSubgraphs: false, }); - if (flag) { + // A disabled feature flag is no longer served in the latest composition (its router config is + // removed without recomposing), so exclude it even though its schema version rows still exist. + if (flag && flag.isEnabled) { featureFlags.push(flag); } } diff --git a/controlplane/src/core/repositories/FeatureFlagRepository.ts b/controlplane/src/core/repositories/FeatureFlagRepository.ts index cc255b8145..0d64c93875 100644 --- a/controlplane/src/core/repositories/FeatureFlagRepository.ts +++ b/controlplane/src/core/repositories/FeatureFlagRepository.ts @@ -1307,13 +1307,21 @@ export class FeatureFlagRepository { }: { baseSchemaVersionId: string; }) { + // A feature flag can have multiple composed schema versions against the same base schema version + // (e.g. recomposing the feature flag recomposes it against the unchanged base, so rows accumulate). + // Deduplicate by feature flag, keeping the latest composed version, so callers get one entry per flag. const ffSchemaVersions = await this.db - .select({ + .selectDistinctOn([federatedGraphsToFeatureFlagSchemaVersions.featureFlagId], { id: federatedGraphsToFeatureFlagSchemaVersions.composedSchemaVersionId, featureFlagId: federatedGraphsToFeatureFlagSchemaVersions.featureFlagId, }) .from(federatedGraphsToFeatureFlagSchemaVersions) + .innerJoin( + schemaVersion, + eq(schemaVersion.id, federatedGraphsToFeatureFlagSchemaVersions.composedSchemaVersionId), + ) .where(eq(federatedGraphsToFeatureFlagSchemaVersions.baseCompositionSchemaVersionId, baseSchemaVersionId)) + .orderBy(federatedGraphsToFeatureFlagSchemaVersions.featureFlagId, desc(schemaVersion.createdAt)) .execute(); if (ffSchemaVersions.length === 0) { diff --git a/controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts b/controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts index 847d8590d4..fb925a7e73 100644 --- a/controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts +++ b/controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts @@ -1,6 +1,7 @@ import fs from 'node:fs'; import { join } from 'node:path'; import { EnumStatusCode } from '@wundergraph/cosmo-connect/dist/common/common_pb'; +import { formatISO } from 'date-fns'; import { afterAll, beforeAll, describe, expect, test } from 'vitest'; import { afterAllSetup, beforeAllSetup, genID, genUniqueLabel } from '../../src/core/test-util.js'; import { @@ -11,6 +12,7 @@ import { DEFAULT_ROUTER_URL, DEFAULT_SUBGRAPH_URL_ONE, SetupTest, + toggleFeatureFlag, } from '../test-util.js'; let dbname = ''; @@ -66,6 +68,87 @@ describe('GetFeatureFlagsInLatestCompositionByFederatedGraph', () => { expect(resp.featureFlags.some((f) => f.name === flagName)).toBe(true); }); + test('Should return each feature flag only once when it has multiple compositions against the same base', async (testContext) => { + const { client, server } = await SetupTest({ dbname, enabledFeatures: ['split-config-loading'] }); + testContext.onTestFinished(() => server.close()); + + const labels = [genUniqueLabel()]; + const federatedGraphName = genID('fedGraph'); + + await createAndPublishSubgraph( + client, + 'users', + 'default', + fs.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/users.graphql')).toString(), + labels, + DEFAULT_SUBGRAPH_URL_ONE, + ); + + await createThenPublishFeatureSubgraph( + client, + 'users-feature', + 'users', + 'default', + fs.readFileSync(join(process.cwd(), 'test/test-data/feature-flags/users-feature.graphql')).toString(), + labels, + 'http://localhost:4101', + ); + + const federatedGraphLabels = labels.map(({ key, value }) => `${key}=${value}`); + await createFederatedGraph(client, federatedGraphName, 'default', federatedGraphLabels, DEFAULT_ROUTER_URL); + + const flagName = genID('flag'); + await createFeatureFlag(client, flagName, labels, ['users-feature'], 'default', true); + + // recomposeFeatureFlag recomposes only the feature flag against the existing base composition (the base + // schema version is unchanged). Each call creates another feature flag composition for the same\ + // (base composition, feature flag) pair, which is the source of the duplicates in the dropdown. + const recomposeResp1 = await client.recomposeFeatureFlag({ name: flagName, namespace: 'default' }); + expect(recomposeResp1.response?.code).toBe(EnumStatusCode.OK); + + const recomposeResp2 = await client.recomposeFeatureFlag({ name: flagName, namespace: 'default' }); + expect(recomposeResp2.response?.code).toBe(EnumStatusCode.OK); + + const resp = await client.getFeatureFlagsInLatestCompositionByFederatedGraph({ + federatedGraphName, + namespace: 'default', + }); + + expect(resp.response?.code).toBe(EnumStatusCode.OK); + // Despite multiple accumulated composition rows, the flag must appear exactly once. + expect(resp.featureFlags).toStrictEqual(expect.arrayContaining([expect.objectContaining({ name: flagName })])); + expect(resp.featureFlags).toHaveLength(1); + + // Create a second, enabled feature flag. It is composed into the latest composition, so it shows up too. + const secondFlagName = genID('flag'); + await createFeatureFlag(client, secondFlagName, labels, ['users-feature'], 'default', true); + + const withSecondFlag = await client.getFeatureFlagsInLatestCompositionByFederatedGraph({ + federatedGraphName, + namespace: 'default', + }); + expect(withSecondFlag.response?.code).toBe(EnumStatusCode.OK); + expect(withSecondFlag.featureFlags).toStrictEqual( + expect.arrayContaining([ + expect.objectContaining({ name: flagName }), + expect.objectContaining({ name: secondFlagName }), + ]), + ); + expect(withSecondFlag.featureFlags).toHaveLength(2); + + await toggleFeatureFlag(client, secondFlagName, false, 'default'); + + const afterDisable = await client.getFeatureFlagsInLatestCompositionByFederatedGraph({ + federatedGraphName, + namespace: 'default', + }); + expect(afterDisable.response?.code).toBe(EnumStatusCode.OK); + expect(afterDisable.featureFlags).toHaveLength(1); + expect(afterDisable.featureFlags).toStrictEqual( + expect.arrayContaining([expect.objectContaining({ name: flagName })]), + ); + }); + test('Should return empty list when no feature flags exist', async (testContext) => { const { client, server } = await SetupTest({ dbname }); testContext.onTestFinished(() => server.close());