Skip to content

Commit 9a2b25a

Browse files
authored
fix: FDv2 Only -- Adjust the behavior of initialization when only cache initializers are available. (#1304)
The main goal of this is to attach meta-data to cache initializer factories to allow for making some additional runtime decisions with regard to initialization. In doing so I am changing the factory types from functions into an interface. Which will make this kind of change easier in the future. So the majority of actual change is from that adjustment. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches FDv2 data-source orchestration and factory typings; mistakes could change initialization resolution/rejection and status reporting (especially offline/cache-only scenarios). Scope is contained to FDv2 pipeline and is covered by expanded tests. > > **Overview** > **FDv2 initialization now treats cache-only configurations as valid even on cache miss.** `FDv2DataSource` detects when *all* initializers are cache-marked and there are no synchronizers, and in that case resolves initialization (optionally asserting `VALID`) instead of rejecting with “all sources exhausted”; it also avoids overwriting previously-reported error status and guards against `close()` races during the exhaustion path. > > **Factory types were refactored from function types to `{ create(...) }` interfaces** (`InitializerFactory` adds optional `isCache`), requiring updates across `SourceManager`, `SourceFactoryProvider`, `FDv2DataManagerBase` (FDv1 fallback factory), and tests; `createCacheInitializerFactory` now returns an `isCache`-marked factory. CI package-size threshold was bumped from `38000` to `39000`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1104dc5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent cc86eab commit 9a2b25a

12 files changed

Lines changed: 387 additions & 112 deletions

.github/workflows/sdk-client.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,4 @@ jobs:
3232
target_file: 'packages/shared/sdk-client/dist/esm/index.mjs'
3333
package_name: '@launchdarkly/js-client-sdk-common'
3434
pr_number: ${{ github.event.number }}
35-
size_limit: 38000
35+
size_limit: 39000

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,10 @@ function makeFlagManager() {
9393

9494
function makeSourceFactoryProvider() {
9595
return {
96-
createInitializerFactory: jest.fn((_entry: any) => jest.fn()),
97-
createSynchronizerSlot: jest.fn((_entry: any) => createSynchronizerSlot(jest.fn())),
96+
createInitializerFactory: jest.fn((entry: any) =>
97+
entry?.type === 'cache' ? { create: jest.fn(), isCache: true } : { create: jest.fn() },
98+
),
99+
createSynchronizerSlot: jest.fn((_entry: any) => createSynchronizerSlot({ create: jest.fn() })),
98100
};
99101
}
100102

@@ -983,7 +985,7 @@ it('uses per-mode fdv1Fallback pollInterval from MODE_TABLE for background mode'
983985
const dsConfig = capturedDataSourceConfigs[0];
984986
const fdv1Slot = dsConfig.synchronizerSlots[dsConfig.synchronizerSlots.length - 1];
985987
// Invoke the factory to trigger createFDv1PollingSynchronizer.
986-
fdv1Slot.factory(() => undefined);
988+
fdv1Slot.factory.create(() => undefined);
987989

988990
// The FDv1 fallback synchronizer should use background's default (3600s = 3600000ms).
989991
expect(createFDv1PollingSynchronizer).toHaveBeenCalledWith(
@@ -1022,6 +1024,25 @@ it('resolves identify immediately when initial mode has no sources', async () =>
10221024
manager.close();
10231025
});
10241026

1027+
it('builds offline mode with a cache-marked initializer factory and no synchronizers', async () => {
1028+
// Verifies that the offline mode passes an isCache-marked factory to the
1029+
// data source. FDv2DataSource is responsible for recognizing this
1030+
// cache-only configuration and completing initialization on a cache miss.
1031+
const manager = createFDv2DataManagerBase(makeBaseConfig({ foregroundMode: 'offline' }));
1032+
1033+
const { resolve, reject } = await identifyManager(manager);
1034+
1035+
expect(resolve).toHaveBeenCalledTimes(1);
1036+
expect(reject).not.toHaveBeenCalled();
1037+
1038+
const dsConfig = capturedDataSourceConfigs[0];
1039+
expect(dsConfig.synchronizerSlots).toEqual([]);
1040+
expect(dsConfig.initializerFactories).toHaveLength(1);
1041+
expect(dsConfig.initializerFactories[0].isCache).toBe(true);
1042+
1043+
manager.close();
1044+
});
1045+
10251046
it('does not identify after close', async () => {
10261047
const manager = createFDv2DataManagerBase(makeBaseConfig());
10271048
manager.close();

packages/shared/sdk-client/__tests__/datasource/SourceFactoryProvider.test.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ beforeEach(() => {
109109
});
110110
mockCreateStreamingInitializer.mockReturnValue({ close: jest.fn() });
111111
mockCreateStreamingSynchronizer.mockReturnValue({ close: jest.fn() });
112-
mockCreateCacheInitializerFactory.mockReturnValue(jest.fn());
112+
mockCreateCacheInitializerFactory.mockReturnValue({ create: jest.fn(), isCache: true });
113113
mockMakeFDv2Requestor.mockReturnValue({ poll: jest.fn() });
114114
});
115115

@@ -124,7 +124,7 @@ it('creates a PollingInitializer for a polling initializer entry', () => {
124124

125125
expect(factory).toBeDefined();
126126
const selectorGetter = () => 'some-selector';
127-
factory!(selectorGetter);
127+
factory!.create(selectorGetter);
128128
expect(mockCreatePollingInitializer).toHaveBeenCalledWith(
129129
ctx.requestor,
130130
ctx.logger,
@@ -141,7 +141,7 @@ it('creates a StreamingInitializer for a streaming initializer entry', () => {
141141

142142
expect(factory).toBeDefined();
143143
const selectorGetter = () => 'some-selector';
144-
factory!(selectorGetter);
144+
factory!.create(selectorGetter);
145145
expect(mockCreateStreamingBase).toHaveBeenCalledWith(
146146
expect.objectContaining({
147147
requests: ctx.requests,
@@ -169,6 +169,7 @@ it('creates a CacheInitializer for a cache initializer entry', () => {
169169
logger: ctx.logger,
170170
});
171171
expect(factory).toBe(mockCreateCacheInitializerFactory.mock.results[0].value);
172+
expect(factory!.isCache).toBe(true);
172173
});
173174

174175
it('returns undefined for an unknown initializer entry type', () => {
@@ -196,7 +197,7 @@ it('creates a PollingSynchronizer slot for a polling synchronizer entry', () =>
196197
// Invoke the factory that was passed to createSynchronizerSlot
197198
const factoryArg = mockCreateSynchronizerSlot.mock.calls[0][0];
198199
const selectorGetter = () => 'sel';
199-
factoryArg(selectorGetter);
200+
factoryArg.create(selectorGetter);
200201
expect(mockCreatePollingSynchronizer).toHaveBeenCalledWith(
201202
ctx.requestor,
202203
ctx.logger,
@@ -218,7 +219,7 @@ it('creates a StreamingSynchronizer slot for a streaming synchronizer entry', ()
218219
// Invoke the factory that was passed to createSynchronizerSlot
219220
const factoryArg = mockCreateSynchronizerSlot.mock.calls[0][0];
220221
const selectorGetter = () => 'sel';
221-
factoryArg(selectorGetter);
222+
factoryArg.create(selectorGetter);
222223
expect(mockCreateStreamingBase).toHaveBeenCalledWith(
223224
expect.objectContaining({
224225
requests: ctx.requests,
@@ -255,7 +256,7 @@ it('creates a new requestor when polling entry has endpoint overrides', () => {
255256
expect(factory).toBeDefined();
256257

257258
const selectorGetter = () => undefined;
258-
factory!(selectorGetter);
259+
factory!.create(selectorGetter);
259260

260261
expect(mockMakeFDv2Requestor).toHaveBeenCalledWith(
261262
ctx.plainContextString,
@@ -288,7 +289,7 @@ it('uses per-entry pollInterval override for polling synchronizer', () => {
288289

289290
const factoryArg = mockCreateSynchronizerSlot.mock.calls[0][0];
290291
const selectorGetter = () => undefined;
291-
factoryArg(selectorGetter);
292+
factoryArg.create(selectorGetter);
292293

293294
expect(mockCreatePollingSynchronizer).toHaveBeenCalledWith(
294295
ctx.requestor,
@@ -307,7 +308,7 @@ it('uses per-entry initialReconnectDelay override for streaming initializer', ()
307308

308309
const factory = provider.createInitializerFactory(entry, ctx);
309310
expect(factory).toBeDefined();
310-
factory!(() => undefined);
311+
factory!.create(() => undefined);
311312

312313
expect(mockCreateStreamingBase).toHaveBeenCalledWith(
313314
expect.objectContaining({
@@ -328,7 +329,7 @@ it('ping handler uses the factory selector getter, not a stale reference', () =>
328329

329330
let currentSelector: string | undefined = 'selector-v1';
330331
const selectorGetter = () => currentSelector;
331-
factory!(selectorGetter);
332+
factory!.create(selectorGetter);
332333

333334
// Extract the pingHandler from the createStreamingBase call
334335
const streamingBaseArgs = mockCreateStreamingBase.mock.calls[0][0];
@@ -352,7 +353,7 @@ it('ping handler uses per-entry endpoint-overridden requestor', () => {
352353

353354
const factory = provider.createInitializerFactory(entry, ctx);
354355
expect(factory).toBeDefined();
355-
factory!(() => undefined);
356+
factory!.create(() => undefined);
356357

357358
// Extract the pingHandler from the createStreamingBase call
358359
const streamingBaseArgs = mockCreateStreamingBase.mock.calls[0][0];

packages/shared/sdk-client/__tests__/datasource/fdv2/CacheInitializer.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function createInitializer(
3939
context,
4040
logger,
4141
});
42-
return factory(noSelector);
42+
return factory.create(noSelector);
4343
}
4444

4545
describe('CacheInitializer', () => {
@@ -51,6 +51,16 @@ describe('CacheInitializer', () => {
5151
context = Context.fromLDContext({ kind: 'user', key: 'test-user' });
5252
});
5353

54+
it('returns a factory marked as a cache initializer', () => {
55+
const factory = createCacheInitializerFactory({
56+
storage: makeMemoryStorage(),
57+
crypto,
58+
environmentNamespace: TEST_NAMESPACE,
59+
context,
60+
});
61+
expect(factory.isCache).toBe(true);
62+
});
63+
5464
it('returns a changeSet with cached flags when cache is present', async () => {
5565
const storage = makeMemoryStorage();
5666
const flags: Flags = {
@@ -234,7 +244,7 @@ describe('CacheInitializer', () => {
234244
context,
235245
});
236246

237-
const initializer = factory(selectorGetter);
247+
const initializer = factory.create(selectorGetter);
238248
const result = await initializer.run();
239249

240250
expect(result.type).toBe('changeSet');

0 commit comments

Comments
 (0)