Skip to content

Commit 9956bce

Browse files
kinyoklionkinyoklion
authored
feat: support per-mode FDv1 fallback configuration (#1246)
The FDv1 fallback polling interval and endpoints are now configurable per connection mode instead of using a single global pollInterval. Background mode defaults to 1 hour, foreground modes default to 5 minutes. When a user overrides a mode without specifying fdv1Fallback, the built-in default is preserved. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Moderate risk because it changes how the FDv1 fallback synchronizer is constructed (poll interval and service endpoints) based on per-mode configuration, which could affect data sync behavior across modes. Changes are scoped and covered by new validation and data manager tests. > > **Overview** > Adds per-connection-mode configuration for the FDv1 polling fallback via a new `ModeDefinition.fdv1Fallback` (poll interval + endpoint overrides) and exports `FDv1FallbackConfig`. > > Updates the built-in `MODE_TABLE` to include FDv1 fallback defaults (5 min for foreground modes, 1 hr for `background`) and extends validation (`modeDefinitionValidators`, `recordOf` defaults precedence, and top-level `dataSystem` defaults) so partial mode overrides preserve these built-in fallback defaults. > > Changes `FDv2DataManagerBase` to build the FDv1 fallback synchronizer using the mode’s `fdv1Fallback` poll interval and optional per-mode `ServiceEndpoints`, and adds tests covering defaulting/merging and correct poll interval selection for background mode and endpoint-only overrides. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 749c59f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/launchdarkly/js-core/pull/1246" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end -->
1 parent d87a6c4 commit 9956bce

11 files changed

Lines changed: 693 additions & 14 deletions

File tree

‎packages/shared/sdk-client/__tests__/configuration/validateOptions.test.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { LDLogger, TypeValidators } from '@launchdarkly/js-sdk-common';
2+
3+
import { recordOf, validatorOf } from '../../src/configuration/validateOptions';
4+
5+
let logger: LDLogger;
6+
7+
beforeEach(() => {
8+
logger = {
9+
debug: jest.fn(),
10+
info: jest.fn(),
11+
warn: jest.fn(),
12+
error: jest.fn(),
13+
};
14+
});
15+
16+
describe('recordOf with built-in defaults', () => {
17+
const valueValidator = validatorOf({
18+
name: TypeValidators.String,
19+
count: TypeValidators.numberWithMin(1),
20+
});
21+
22+
const keyValidator = TypeValidators.oneOf('a', 'b', 'c');
23+
24+
const builtInDefaults = {
25+
a: { name: 'alpha', count: 10 },
26+
b: { name: 'beta', count: 20 },
27+
c: { name: 'gamma', count: 30 },
28+
};
29+
30+
const validator = recordOf(keyValidator, valueValidator, { defaults: builtInDefaults });
31+
32+
it('uses built-in defaults for keys not provided by the caller', () => {
33+
const result = validator.validate({ a: { name: 'custom' } }, 'test', logger);
34+
35+
expect(result?.value).toEqual({
36+
a: { name: 'custom', count: 10 },
37+
b: { name: 'beta', count: 20 },
38+
c: { name: 'gamma', count: 30 },
39+
});
40+
expect(logger.warn).not.toHaveBeenCalled();
41+
});
42+
43+
it('fills in missing fields from the per-key default', () => {
44+
const result = validator.validate({ b: { count: 99 } }, 'test', logger);
45+
46+
// name comes from built-in default for key 'b'
47+
expect((result?.value as any).b).toEqual({ name: 'beta', count: 99 });
48+
});
49+
50+
it('returns built-in defaults when input is empty object', () => {
51+
const result = validator.validate({}, 'test', logger);
52+
53+
expect(result?.value).toEqual(builtInDefaults);
54+
});
55+
56+
it('returns undefined when input is undefined', () => {
57+
const result = validator.validate(undefined, 'test', logger);
58+
59+
expect(result).toBeUndefined();
60+
});
61+
62+
it('returns undefined when input is null', () => {
63+
const result = validator.validate(null, 'test', logger);
64+
65+
expect(result).toBeUndefined();
66+
});
67+
68+
it('prefers built-in defaults over caller-provided defaults', () => {
69+
const callerDefaults = {
70+
a: { name: 'caller-alpha', count: 1 },
71+
};
72+
73+
const result = validator.validate({}, 'test', logger, callerDefaults);
74+
75+
// Built-in defaults take priority over caller defaults.
76+
expect(result?.value).toEqual(builtInDefaults);
77+
});
78+
});
79+
80+
describe('recordOf without built-in defaults', () => {
81+
const valueValidator = validatorOf({
82+
name: TypeValidators.String,
83+
count: TypeValidators.numberWithMin(1),
84+
});
85+
86+
const keyValidator = TypeValidators.oneOf('a', 'b');
87+
88+
const validator = recordOf(keyValidator, valueValidator);
89+
90+
it('uses caller-provided defaults when no built-in defaults', () => {
91+
const callerDefaults = {
92+
a: { name: 'alpha', count: 10 },
93+
b: { name: 'beta', count: 20 },
94+
};
95+
96+
const result = validator.validate({ a: { name: 'custom' } }, 'test', logger, callerDefaults);
97+
98+
expect((result?.value as any).a).toEqual({ name: 'custom', count: 10 });
99+
expect((result?.value as any).b).toEqual({ name: 'beta', count: 20 });
100+
});
101+
102+
it('uses empty defaults when neither built-in nor caller defaults are provided', () => {
103+
const result = validator.validate({ a: { name: 'custom' } }, 'test', logger);
104+
105+
// No defaults to fill in count, so only name is present.
106+
expect((result?.value as any).a).toEqual({ name: 'custom' });
107+
});
108+
});

‎packages/shared/sdk-client/__tests__/datasource/ConnectionModeConfig.test.ts‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { LDLogger } from '@launchdarkly/js-sdk-common';
22

33
import validateOptions from '../../src/configuration/validateOptions';
44
import {
5+
BACKGROUND_POLL_INTERVAL_SECONDS,
56
connectionModesValidator,
7+
DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS,
68
MODE_DEFINITION_DEFAULTS,
79
MODE_TABLE,
810
modeDefinitionValidators,
@@ -497,6 +499,7 @@ describe('given a partial override', () => {
497499
expect(result.streaming).toEqual({
498500
initializers: [{ type: 'polling' }],
499501
synchronizers: [{ type: 'streaming' }],
502+
fdv1Fallback: { pollInterval: DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS },
500503
});
501504
// Other modes retain their defaults.
502505
expect(result.polling).toEqual(MODE_TABLE.polling);
@@ -555,6 +558,7 @@ describe('given unknown mode names', () => {
555558
expect(result.polling).toEqual({
556559
initializers: [],
557560
synchronizers: [{ type: 'polling', pollInterval: 60 }],
561+
fdv1Fallback: { pollInterval: DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS },
558562
});
559563
expect(result.streaming).toEqual(MODE_TABLE.streaming);
560564
expect(logger.warn).toHaveBeenCalledTimes(1);
@@ -610,6 +614,7 @@ describe('given a custom defaults table', () => {
610614
expect(result.polling).toEqual({
611615
initializers: [],
612616
synchronizers: [{ type: 'polling', pollInterval: 120 }],
617+
fdv1Fallback: { pollInterval: DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS },
613618
});
614619
// Custom default retained for streaming (not the built-in MODE_TABLE default).
615620
expect(result.streaming).toEqual(customDefaults.streaming);
@@ -640,3 +645,186 @@ describe('given no logger for validateModeTable', () => {
640645
expect(result.polling).toEqual(MODE_TABLE.polling);
641646
});
642647
});
648+
649+
// ----------------------------- fdv1Fallback validation --------------------------------
650+
651+
describe('given MODE_TABLE fdv1Fallback defaults', () => {
652+
it('has the default poll interval for streaming mode', () => {
653+
expect(MODE_TABLE.streaming.fdv1Fallback).toEqual({
654+
pollInterval: DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS,
655+
});
656+
});
657+
658+
it('has the default poll interval for polling mode', () => {
659+
expect(MODE_TABLE.polling.fdv1Fallback).toEqual({
660+
pollInterval: DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS,
661+
});
662+
});
663+
664+
it('has the background poll interval for background mode', () => {
665+
expect(MODE_TABLE.background.fdv1Fallback).toEqual({
666+
pollInterval: BACKGROUND_POLL_INTERVAL_SECONDS,
667+
});
668+
});
669+
670+
it('does not have fdv1Fallback for offline mode', () => {
671+
expect(MODE_TABLE.offline.fdv1Fallback).toBeUndefined();
672+
});
673+
674+
it('does not have fdv1Fallback for one-shot mode', () => {
675+
expect(MODE_TABLE['one-shot'].fdv1Fallback).toBeUndefined();
676+
});
677+
});
678+
679+
describe('given a valid fdv1Fallback in a mode definition', () => {
680+
it('passes through a valid fdv1Fallback with pollInterval', () => {
681+
const input = {
682+
initializers: [{ type: 'cache' }],
683+
synchronizers: [{ type: 'polling' }],
684+
fdv1Fallback: { pollInterval: 600 },
685+
};
686+
687+
const result = validateModeDefinition(input, 'testMode', logger);
688+
689+
expect(result.fdv1Fallback).toEqual({ pollInterval: 600 });
690+
expect(logger.warn).not.toHaveBeenCalled();
691+
});
692+
693+
it('passes through fdv1Fallback with endpoint overrides', () => {
694+
const input = {
695+
initializers: [],
696+
synchronizers: [{ type: 'polling' }],
697+
fdv1Fallback: {
698+
pollInterval: 120,
699+
endpoints: { pollingBaseUri: 'https://relay.example.com' },
700+
},
701+
};
702+
703+
const result = validateModeDefinition(input, 'testMode', logger);
704+
705+
expect(result.fdv1Fallback).toEqual({
706+
pollInterval: 120,
707+
endpoints: { pollingBaseUri: 'https://relay.example.com' },
708+
});
709+
expect(logger.warn).not.toHaveBeenCalled();
710+
});
711+
712+
it('passes through fdv1Fallback with only endpoints', () => {
713+
const input = {
714+
initializers: [],
715+
synchronizers: [{ type: 'polling' }],
716+
fdv1Fallback: {
717+
endpoints: { pollingBaseUri: 'https://relay.example.com' },
718+
},
719+
};
720+
721+
const result = validateModeDefinition(input, 'testMode', logger);
722+
723+
expect(result.fdv1Fallback).toEqual({
724+
endpoints: { pollingBaseUri: 'https://relay.example.com' },
725+
});
726+
expect(logger.warn).not.toHaveBeenCalled();
727+
});
728+
});
729+
730+
describe('given invalid fdv1Fallback in a mode definition', () => {
731+
it('clamps pollInterval to minimum when below 30', () => {
732+
const input = {
733+
initializers: [],
734+
synchronizers: [{ type: 'polling' }],
735+
fdv1Fallback: { pollInterval: 5 },
736+
};
737+
738+
const result = validateModeDefinition(input, 'testMode', logger);
739+
740+
expect(result.fdv1Fallback).toEqual({ pollInterval: 30 });
741+
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('pollInterval'));
742+
});
743+
744+
it('drops pollInterval when it is a string and warns', () => {
745+
const input = {
746+
initializers: [],
747+
synchronizers: [{ type: 'polling' }],
748+
fdv1Fallback: { pollInterval: 'fast' },
749+
};
750+
751+
const result = validateModeDefinition(input, 'testMode', logger);
752+
753+
// validatorOf returns undefined when all nested fields are invalid/dropped.
754+
expect(result.fdv1Fallback).toBeUndefined();
755+
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('pollInterval'));
756+
});
757+
758+
it('drops fdv1Fallback when it is not an object and warns', () => {
759+
const input = {
760+
initializers: [],
761+
synchronizers: [{ type: 'polling' }],
762+
fdv1Fallback: 'invalid',
763+
};
764+
765+
const result = validateModeDefinition(input, 'testMode', logger);
766+
767+
expect(result.fdv1Fallback).toBeUndefined();
768+
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('fdv1Fallback'));
769+
});
770+
771+
it('drops invalid endpoint fields within fdv1Fallback', () => {
772+
const input = {
773+
initializers: [],
774+
synchronizers: [{ type: 'polling' }],
775+
fdv1Fallback: {
776+
pollInterval: 60,
777+
endpoints: { pollingBaseUri: 123 },
778+
},
779+
};
780+
781+
const result = validateModeDefinition(input, 'testMode', logger);
782+
783+
expect(result.fdv1Fallback).toEqual({ pollInterval: 60 });
784+
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('pollingBaseUri'));
785+
});
786+
});
787+
788+
describe('given mode table overrides with fdv1Fallback', () => {
789+
it('preserves default fdv1Fallback when user override does not specify it', () => {
790+
const result = validateModeTable(
791+
{
792+
streaming: {
793+
initializers: [{ type: 'polling' }],
794+
synchronizers: [{ type: 'streaming' }],
795+
},
796+
},
797+
MODE_TABLE,
798+
logger,
799+
);
800+
801+
// The validatorOf merges defaults from MODE_TABLE, so fdv1Fallback
802+
// is carried through even when the user doesn't specify it.
803+
expect((result.streaming as any).fdv1Fallback).toEqual({
804+
pollInterval: DEFAULT_FDV1_FALLBACK_POLL_INTERVAL_SECONDS,
805+
});
806+
expect((result.streaming as any).initializers).toEqual([{ type: 'polling' }]);
807+
expect((result.streaming as any).synchronizers).toEqual([{ type: 'streaming' }]);
808+
// Non-overridden modes retain their defaults including fdv1Fallback.
809+
expect((result.background as any).fdv1Fallback).toEqual({
810+
pollInterval: BACKGROUND_POLL_INTERVAL_SECONDS,
811+
});
812+
});
813+
814+
it('uses user-specified fdv1Fallback when provided', () => {
815+
const result = validateModeTable(
816+
{
817+
background: {
818+
initializers: [{ type: 'cache' }],
819+
synchronizers: [{ type: 'polling', pollInterval: 7200 }],
820+
fdv1Fallback: { pollInterval: 7200 },
821+
},
822+
},
823+
MODE_TABLE,
824+
logger,
825+
);
826+
827+
expect((result.background as any).fdv1Fallback).toEqual({ pollInterval: 7200 });
828+
expect(logger.warn).not.toHaveBeenCalled();
829+
});
830+
});

‎packages/shared/sdk-client/__tests__/datasource/FDv2DataManagerBase.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,50 @@ it('appends a blocked FDv1 fallback synchronizer when fdv1Endpoints are configur
951951
manager.close();
952952
});
953953

954+
it('uses per-mode fdv1Fallback pollInterval from MODE_TABLE for background mode', async () => {
955+
const sourceFactoryProvider = makeSourceFactoryProvider();
956+
const fdv1Endpoints = {
957+
polling: jest.fn(() => ({
958+
pathGet: jest.fn(),
959+
pathReport: jest.fn(),
960+
pathPost: jest.fn(),
961+
pathPing: jest.fn(),
962+
})),
963+
streaming: jest.fn(() => ({
964+
pathGet: jest.fn(),
965+
pathReport: jest.fn(),
966+
pathPost: jest.fn(),
967+
pathPing: jest.fn(),
968+
})),
969+
};
970+
971+
(makeRequestor as jest.Mock).mockReturnValue({});
972+
(createFDv1PollingSynchronizer as jest.Mock).mockReturnValue({ close: jest.fn() });
973+
974+
const manager = createFDv2DataManagerBase(
975+
makeBaseConfig({
976+
sourceFactoryProvider,
977+
fdv1Endpoints,
978+
foregroundMode: 'background',
979+
}),
980+
);
981+
await identifyManager(manager);
982+
983+
const dsConfig = capturedDataSourceConfigs[0];
984+
const fdv1Slot = dsConfig.synchronizerSlots[dsConfig.synchronizerSlots.length - 1];
985+
// Invoke the factory to trigger createFDv1PollingSynchronizer.
986+
fdv1Slot.factory(() => undefined);
987+
988+
// The FDv1 fallback synchronizer should use background's default (3600s = 3600000ms).
989+
expect(createFDv1PollingSynchronizer).toHaveBeenCalledWith(
990+
expect.anything(),
991+
3600 * 1000,
992+
expect.anything(),
993+
);
994+
995+
manager.close();
996+
});
997+
954998
it('resolves identify immediately when initial mode has no sources', async () => {
955999
// Use a custom mode table where the initial mode has empty initializers and synchronizers.
9561000
const sourceFactoryProvider = makeSourceFactoryProvider();

0 commit comments

Comments
 (0)