Skip to content

Commit 0e887ed

Browse files
kinyoklionkinyoklion
authored
chore: Remove unused Payload.id field from FDv2 protocol handler (#1260)
SDK-2005 ## Summary - Removes the `id` field from the internal `Payload` interface and all `tempId` plumbing in the protocol handler - The field was populated from `PayloadIntent.id` but never read by any downstream consumer - Wire protocol types (`PayloadIntent.id`, `PayloadTransferred.id`, `ErrorObject.payload_id`) are unchanged <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk refactor that removes an unused field and related plumbing; behavioral changes are limited to slightly different validation/log messages around intent/transfer events. > > **Overview** > Removes the internal FDv2 `Payload.id` field and the protocol handler’s `tempId` tracking, so emitted payloads are keyed only by `version`/`state`/`type`/`updates`. > > Updates SDK client/server components and tests to stop constructing or asserting on `payload.id` (including cache initialization, polling/FDv1 fallback payloads, and FDv2 source result helpers), and simplifies protocol-handler warnings/log messages accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a11185e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 6dee197 commit 0e887ed

13 files changed

Lines changed: 9 additions & 47 deletions

File tree

‎packages/shared/common/__tests__/internal/fdv2/PayloadStreamReader.test.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ it('it sets type to full when intent code is xfer-full', () => {
3131
data: '{"state": "mockState", "version": 1}',
3232
});
3333
expect(receivedPayloads.length).toEqual(1);
34-
expect(receivedPayloads[0].id).toEqual('mockId');
3534
expect(receivedPayloads[0].state).toEqual('mockState');
3635
expect(receivedPayloads[0].type).toEqual('full');
3736
});
@@ -53,7 +52,6 @@ it('it sets type to partial when intent code is xfer-changes', () => {
5352
data: '{"state": "mockState", "version": 1}',
5453
});
5554
expect(receivedPayloads.length).toEqual(1);
56-
expect(receivedPayloads[0].id).toEqual('mockId');
5755
expect(receivedPayloads[0].state).toEqual('mockState');
5856
expect(receivedPayloads[0].type).toEqual('partial');
5957
});
@@ -72,7 +70,6 @@ it('it sets type to none and emits empty payload when intent code is none', () =
7270
data: '{"payloads": [{"intentCode": "none", "id": "mockId", "target": 42}]}',
7371
});
7472
expect(receivedPayloads.length).toEqual(1);
75-
expect(receivedPayloads[0].id).toEqual('mockId');
7673
expect(receivedPayloads[0].version).toEqual(42);
7774
expect(receivedPayloads[0].type).toEqual('none');
7875
});
@@ -104,14 +101,12 @@ it('it handles xfer-full then xfer-changes', () => {
104101
data: '{"state": "mockState", "version": 1}',
105102
});
106103
expect(receivedPayloads.length).toEqual(2);
107-
expect(receivedPayloads[0].id).toEqual('mockId');
108104
expect(receivedPayloads[0].state).toEqual('mockState');
109105
expect(receivedPayloads[0].type).toEqual('full');
110106
expect(receivedPayloads[0].updates.length).toEqual(1);
111107
expect(receivedPayloads[0].updates[0].object).toEqual({ objectFieldA: 'objectValueA' });
112108
expect(receivedPayloads[0].updates[0].deleted).toEqual(undefined);
113109

114-
expect(receivedPayloads[1].id).toEqual('mockId');
115110
expect(receivedPayloads[1].state).toEqual('mockState');
116111
expect(receivedPayloads[1].type).toEqual('partial');
117112
expect(receivedPayloads[1].updates.length).toEqual(1);
@@ -145,7 +140,6 @@ it('it includes multiple types of updates in payload', () => {
145140
data: '{"state": "mockState", "version": 1}',
146141
});
147142
expect(receivedPayloads.length).toEqual(1);
148-
expect(receivedPayloads[0].id).toEqual('mockId');
149143
expect(receivedPayloads[0].state).toEqual('mockState');
150144
expect(receivedPayloads[0].type).toEqual('full');
151145
expect(receivedPayloads[0].updates.length).toEqual(3);
@@ -253,7 +247,7 @@ it('logs prescribed message when error event is encountered', () => {
253247
});
254248
expect(receivedPayloads.length).toEqual(1);
255249
expect(mockLogger.info).toHaveBeenCalledWith(
256-
'An issue was encountered receiving updates for payload mockId with reason: Womp womp.',
250+
'An issue was encountered receiving updates with reason: Womp womp.',
257251
);
258252
});
259253

@@ -309,13 +303,11 @@ it('discards partially transferred data when an error is encountered', () => {
309303
data: '{"state": "mockState2", "version": 1}',
310304
});
311305
expect(receivedPayloads.length).toEqual(2);
312-
expect(receivedPayloads[0].id).toEqual('mockId');
313306
expect(receivedPayloads[0].state).toEqual('mockState');
314307
expect(receivedPayloads[0].type).toEqual('full');
315308
expect(receivedPayloads[0].updates.length).toEqual(1);
316309
expect(receivedPayloads[0].updates[0].object).toEqual({ objectFieldB: 'objectValueB' });
317310
expect(receivedPayloads[0].updates[0].deleted).toEqual(undefined);
318-
expect(receivedPayloads[1].id).toEqual('mockId2');
319311
expect(receivedPayloads[1].state).toEqual('mockState2');
320312
expect(receivedPayloads[1].type).toEqual('full');
321313
expect(receivedPayloads[1].updates.length).toEqual(3);
@@ -350,7 +342,6 @@ it('silently ignores unrecognized kinds', () => {
350342
data: '{"state": "mockState", "version": 1}',
351343
});
352344
expect(receivedPayloads.length).toEqual(1);
353-
expect(receivedPayloads[0].id).toEqual('mockId');
354345
expect(receivedPayloads[0].state).toEqual('mockState');
355346
expect(receivedPayloads[0].type).toEqual('full');
356347
expect(receivedPayloads[0].updates.length).toEqual(1);
@@ -380,7 +371,6 @@ it('ignores additional payloads beyond the first payload in the server-intent me
380371
data: '{"state": "mockState", "version": 1}',
381372
});
382373
expect(receivedPayloads.length).toEqual(1);
383-
expect(receivedPayloads[0].id).toEqual('mockId');
384374
expect(receivedPayloads[0].state).toEqual('mockState');
385375
expect(receivedPayloads[0].type).toEqual('full');
386376
expect(receivedPayloads[0].updates.length).toEqual(1);

‎packages/shared/common/__tests__/internal/fdv2/protocolHandler.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ it('emits a full payload after xfer-full and payload-transferred', () => {
6565
if (action.type !== 'payload') return;
6666

6767
expect(action.payload.type).toBe('full');
68-
expect(action.payload.id).toBe('p1');
6968
expect(action.payload.version).toBe(52);
7069
expect(action.payload.state).toBe('(p:p1:52)');
7170
expect(action.payload.updates).toHaveLength(2);
@@ -114,7 +113,6 @@ it('returns a none-type payload immediately on intent-none', () => {
114113
if (action.type !== 'payload') return;
115114

116115
expect(action.payload.type).toBe('none');
117-
expect(action.payload.id).toBe('p1');
118116
expect(action.payload.version).toBe(42);
119117
expect(action.payload.updates).toHaveLength(0);
120118
expect(action.payload.state).toBeUndefined();

‎packages/shared/common/src/internal/fdv2/protocolHandler.ts‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ export type PayloadType = 'full' | 'partial' | 'none';
3535
* - `none`: no changes are needed; the SDK is up-to-date.
3636
*/
3737
export interface Payload {
38-
id: string;
3938
version: number;
4039
state?: string;
4140
type: PayloadType;
@@ -86,7 +85,6 @@ export function createProtocolHandler(
8685
logger?: LDLogger,
8786
): ProtocolHandler {
8887
let protocolState: ProtocolState = 'inactive';
89-
let tempId: string | undefined;
9088
let tempType: PayloadType = 'partial';
9189
let tempUpdates: Update[] = [];
9290

@@ -96,7 +94,6 @@ export function createProtocolHandler(
9694

9795
function resetAll(): void {
9896
protocolState = 'inactive';
99-
tempId = undefined;
10097
tempType = 'partial';
10198
tempUpdates = [];
10299
}
@@ -112,17 +109,14 @@ export function createProtocolHandler(
112109
}
113110

114111
function processIntentNone(intent: PayloadIntent): ProtocolAction {
115-
if (!intent.id || isNullish(intent.target)) {
116-
logger?.warn(
117-
`Ignoring 'none' intent with missing fields: id=${intent.id}, target=${intent.target}`,
118-
);
112+
if (isNullish(intent.target)) {
113+
logger?.warn(`Ignoring 'none' intent with missing fields: target=${intent.target}`);
119114
return ACTION_NONE;
120115
}
121116

122117
return {
123118
type: 'payload',
124119
payload: {
125-
id: intent.id,
126120
version: intent.target,
127121
type: 'none',
128122
updates: [],
@@ -147,19 +141,16 @@ export function createProtocolHandler(
147141
protocolState = 'full';
148142
tempUpdates = [];
149143
tempType = 'full';
150-
tempId = payload.id;
151144
return ACTION_NONE;
152145
case 'xfer-changes':
153146
protocolState = 'changes';
154147
tempUpdates = [];
155148
tempType = 'partial';
156-
tempId = payload.id;
157149
return ACTION_NONE;
158150
case 'none':
159151
protocolState = 'changes';
160152
tempUpdates = [];
161153
tempType = 'partial';
162-
tempId = payload.id;
163154
return processIntentNone(payload);
164155
default:
165156
logger?.warn(`Unable to process intent code '${payload?.intentCode}'.`);
@@ -168,7 +159,7 @@ export function createProtocolHandler(
168159
}
169160

170161
function processPutObject(data: PutObject): ProtocolAction {
171-
if (protocolState === 'inactive' || !tempId) {
162+
if (protocolState === 'inactive') {
172163
logger?.warn('Received put-object before server-intent was established. Ignoring.');
173164
return ACTION_NONE;
174165
}
@@ -196,7 +187,7 @@ export function createProtocolHandler(
196187
}
197188

198189
function processDeleteObject(data: DeleteObject): ProtocolAction {
199-
if (protocolState === 'inactive' || !tempId) {
190+
if (protocolState === 'inactive') {
200191
logger?.warn('Received delete-object before server-intent was established. Ignoring.');
201192
return ACTION_NONE;
202193
}
@@ -227,7 +218,7 @@ export function createProtocolHandler(
227218
};
228219
}
229220

230-
if (!tempId || isNullish(data.state) || isNullish(data.version)) {
221+
if (isNullish(data.state) || isNullish(data.version)) {
231222
logger?.warn(
232223
`Ignoring payload-transferred with missing fields: state=${data.state}, version=${data.version}`,
233224
);
@@ -238,7 +229,6 @@ export function createProtocolHandler(
238229
const result: ProtocolAction = {
239230
type: 'payload',
240231
payload: {
241-
id: tempId,
242232
version: data.version,
243233
state: data.state,
244234
type: tempType,
@@ -259,9 +249,7 @@ export function createProtocolHandler(
259249
}
260250

261251
function processError(data: any): ProtocolAction {
262-
logger?.info(
263-
`An issue was encountered receiving updates for payload ${tempId} with reason: ${data.reason}.`,
264-
);
252+
logger?.info(`An issue was encountered receiving updates with reason: ${data.reason}.`);
265253
resetAfterError();
266254
return { type: 'serverError', id: data.payload_id, reason: data.reason };
267255
}

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,7 @@ function makeInterrupted(): FDv2SourceResult {
3838
}
3939

4040
function makeChangeSet(): FDv2SourceResult {
41-
return changeSet(
42-
{ id: 'test', version: 1, state: 'test-state', type: 'full', updates: [] },
43-
false,
44-
);
41+
return changeSet({ version: 1, state: 'test-state', type: 'full', updates: [] }, false);
4542
}
4643

4744
// -- fallback condition --

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ it('produces a changeSet with full payload from a successful poll', async () =>
5858
expect(result.type).toBe('changeSet');
5959
if (result.type === 'changeSet') {
6060
expect(result.payload.type).toBe('full');
61-
expect(result.payload.id).toBe('FDv1Fallback');
6261
expect(result.payload.version).toBe(1);
6362
expect(result.payload.updates).toHaveLength(2);
6463

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ it('creates a changeSet result with a payload', () => {
3232
});
3333

3434
it('creates a changeSet result with fdv1Fallback flag', () => {
35-
const payload = { id: 'id', version: 1, type: 'full' as const, updates: [] };
35+
const payload = { version: 1, type: 'full' as const, updates: [] };
3636
const result = changeSet(payload, true);
3737

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

‎packages/shared/sdk-client/__tests__/datasource/fdv2/orchestrationTestHelpers.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ export function makePayload(
7575
opts: { state?: string; type?: internal.PayloadType } = {},
7676
): internal.Payload {
7777
return {
78-
id: 'test-payload',
7978
version: 1,
8079
state: opts.state ?? 'test-selector',
8180
type: opts.type ?? 'full',

‎packages/shared/sdk-client/src/datasource/fdv2/CacheInitializer.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ async function loadFromCache(config: CacheInitializerConfig): Promise<FDv2Source
6969
);
7070

7171
const payload: internal.Payload = {
72-
id: 'cache',
7372
version: 0,
7473
// No `state` field. The orchestrator sees a changeSet without a selector,
7574
// records dataReceived=true, and continues to the next initializer.

‎packages/shared/sdk-client/src/datasource/fdv2/FDv1PollingSynchronizer.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import {
2020
} from './FDv2SourceResult';
2121
import { Synchronizer } from './Synchronizer';
2222

23-
const PAYLOAD_ID = 'FDv1Fallback';
24-
2523
function flagsToPayload(flags: Flags): internal.Payload {
2624
const updates: internal.Update[] = Object.entries(flags).map(([key, flag]) => ({
2725
kind: 'flag',
@@ -31,7 +29,6 @@ function flagsToPayload(flags: Flags): internal.Payload {
3129
}));
3230

3331
return {
34-
id: PAYLOAD_ID,
3532
version: 1,
3633
type: 'full',
3734
updates,

‎packages/shared/sdk-client/src/datasource/fdv2/PollingBase.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ export async function poll(
120120
// (Spec Requirement 10.1.2)
121121
if (response.status === 304) {
122122
const nonePayload: internal.Payload = {
123-
id: '',
124123
version: 0,
125124
type: 'none',
126125
updates: [],

0 commit comments

Comments
 (0)