Skip to content

Commit fa43420

Browse files
fix: mutating delta events (#9303)
Fix a bug where delta events would be mutated due to Object.assign.
1 parent aa15fbe commit fa43420

File tree

2 files changed

+72
-6
lines changed

2 files changed

+72
-6
lines changed

src/lib/features/client-feature-toggles/delta/delta-cache.test.ts

+63
Original file line numberDiff line numberDiff line change
@@ -183,4 +183,67 @@ describe('RevisionCache', () => {
183183
]),
184184
);
185185
});
186+
187+
it('should not mutate previous feature-updated events when new events with the same feature name are added', () => {
188+
const baseEvent: DeltaHydrationEvent = {
189+
eventId: 1,
190+
features: [
191+
{
192+
name: 'base-flag',
193+
type: 'release',
194+
enabled: true,
195+
project: 'streaming-deltas',
196+
stale: false,
197+
strategies: [],
198+
variants: [],
199+
description: null,
200+
impressionData: false,
201+
},
202+
],
203+
type: 'hydration',
204+
segments: [],
205+
};
206+
207+
const deltaCache = new DeltaCache(baseEvent, 10);
208+
209+
const initialFeatureEvent: DeltaEvent = {
210+
eventId: 129,
211+
type: DELTA_EVENT_TYPES.FEATURE_UPDATED,
212+
feature: {
213+
impressionData: false,
214+
enabled: false,
215+
name: 'streaming-test',
216+
description: null,
217+
project: 'streaming-deltas',
218+
stale: false,
219+
type: 'release',
220+
variants: [],
221+
strategies: [],
222+
},
223+
};
224+
// This tests is to verify that the initialFeatureEvent is not mutated when a new event with the same feature name is added
225+
// the following dirty way to clone this object is to avoid mutation on the comparison object. Because the object is passed by reference
226+
// we would be comparing the same object with itself which would cause the expect check to always pass because the comparison would
227+
// also change to match the object being compared.
228+
deltaCache.addEvents([JSON.parse(JSON.stringify(initialFeatureEvent))]);
229+
230+
const updatedFeatureEvent: DeltaEvent = {
231+
eventId: 130,
232+
type: DELTA_EVENT_TYPES.FEATURE_UPDATED,
233+
feature: {
234+
impressionData: false,
235+
enabled: true,
236+
name: 'streaming-test',
237+
description: null,
238+
project: 'streaming-deltas',
239+
stale: false,
240+
type: 'release',
241+
variants: [],
242+
strategies: [{ name: 'new-strategy', parameters: {} }],
243+
},
244+
};
245+
deltaCache.addEvents([updatedFeatureEvent]);
246+
// @ts-ignore
247+
expect(deltaCache.events[1]).toStrictEqual(initialFeatureEvent);
248+
});
186249
});

src/lib/features/client-feature-toggles/delta/delta-cache.ts

+9-6
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,13 @@ export class DeltaCache {
5555
for (const appliedEvent of events) {
5656
switch (appliedEvent.type) {
5757
case DELTA_EVENT_TYPES.FEATURE_UPDATED: {
58-
const featureToUpdate = this.hydrationEvent.features.find(
58+
const featureIndex = this.hydrationEvent.features.findIndex(
5959
(feature) => feature.name === appliedEvent.feature.name,
6060
);
61-
if (featureToUpdate) {
62-
Object.assign(featureToUpdate, appliedEvent.feature);
61+
62+
if (featureIndex > -1) {
63+
this.hydrationEvent.features[featureIndex] =
64+
appliedEvent.feature;
6365
} else {
6466
this.hydrationEvent.features.push(appliedEvent.feature);
6567
}
@@ -74,11 +76,12 @@ export class DeltaCache {
7476
break;
7577
}
7678
case DELTA_EVENT_TYPES.SEGMENT_UPDATED: {
77-
const segmentToUpdate = this.hydrationEvent.segments.find(
79+
const segmentIndex = this.hydrationEvent.segments.findIndex(
7880
(segment) => segment.id === appliedEvent.segment.id,
7981
);
80-
if (segmentToUpdate) {
81-
Object.assign(segmentToUpdate, appliedEvent.segment);
82+
if (segmentIndex > -1) {
83+
this.hydrationEvent.segments[segmentIndex] =
84+
appliedEvent.segment;
8285
} else {
8386
this.hydrationEvent.segments.push(appliedEvent.segment);
8487
}

0 commit comments

Comments
 (0)