From fa43420ab9c1204ba72557bc5dbde7f9bc3770bf Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Fri, 14 Feb 2025 09:30:14 +0100 Subject: [PATCH] fix: mutating delta events (#9303) Fix a bug where delta events would be mutated due to Object.assign. --- .../delta/delta-cache.test.ts | 63 +++++++++++++++++++ .../delta/delta-cache.ts | 15 +++-- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/lib/features/client-feature-toggles/delta/delta-cache.test.ts b/src/lib/features/client-feature-toggles/delta/delta-cache.test.ts index 07a84c96da..e39ba087b7 100644 --- a/src/lib/features/client-feature-toggles/delta/delta-cache.test.ts +++ b/src/lib/features/client-feature-toggles/delta/delta-cache.test.ts @@ -183,4 +183,67 @@ describe('RevisionCache', () => { ]), ); }); + + it('should not mutate previous feature-updated events when new events with the same feature name are added', () => { + const baseEvent: DeltaHydrationEvent = { + eventId: 1, + features: [ + { + name: 'base-flag', + type: 'release', + enabled: true, + project: 'streaming-deltas', + stale: false, + strategies: [], + variants: [], + description: null, + impressionData: false, + }, + ], + type: 'hydration', + segments: [], + }; + + const deltaCache = new DeltaCache(baseEvent, 10); + + const initialFeatureEvent: DeltaEvent = { + eventId: 129, + type: DELTA_EVENT_TYPES.FEATURE_UPDATED, + feature: { + impressionData: false, + enabled: false, + name: 'streaming-test', + description: null, + project: 'streaming-deltas', + stale: false, + type: 'release', + variants: [], + strategies: [], + }, + }; + // This tests is to verify that the initialFeatureEvent is not mutated when a new event with the same feature name is added + // the following dirty way to clone this object is to avoid mutation on the comparison object. Because the object is passed by reference + // we would be comparing the same object with itself which would cause the expect check to always pass because the comparison would + // also change to match the object being compared. + deltaCache.addEvents([JSON.parse(JSON.stringify(initialFeatureEvent))]); + + const updatedFeatureEvent: DeltaEvent = { + eventId: 130, + type: DELTA_EVENT_TYPES.FEATURE_UPDATED, + feature: { + impressionData: false, + enabled: true, + name: 'streaming-test', + description: null, + project: 'streaming-deltas', + stale: false, + type: 'release', + variants: [], + strategies: [{ name: 'new-strategy', parameters: {} }], + }, + }; + deltaCache.addEvents([updatedFeatureEvent]); + // @ts-ignore + expect(deltaCache.events[1]).toStrictEqual(initialFeatureEvent); + }); }); diff --git a/src/lib/features/client-feature-toggles/delta/delta-cache.ts b/src/lib/features/client-feature-toggles/delta/delta-cache.ts index abaa2d300c..98c9f955d4 100644 --- a/src/lib/features/client-feature-toggles/delta/delta-cache.ts +++ b/src/lib/features/client-feature-toggles/delta/delta-cache.ts @@ -55,11 +55,13 @@ export class DeltaCache { for (const appliedEvent of events) { switch (appliedEvent.type) { case DELTA_EVENT_TYPES.FEATURE_UPDATED: { - const featureToUpdate = this.hydrationEvent.features.find( + const featureIndex = this.hydrationEvent.features.findIndex( (feature) => feature.name === appliedEvent.feature.name, ); - if (featureToUpdate) { - Object.assign(featureToUpdate, appliedEvent.feature); + + if (featureIndex > -1) { + this.hydrationEvent.features[featureIndex] = + appliedEvent.feature; } else { this.hydrationEvent.features.push(appliedEvent.feature); } @@ -74,11 +76,12 @@ export class DeltaCache { break; } case DELTA_EVENT_TYPES.SEGMENT_UPDATED: { - const segmentToUpdate = this.hydrationEvent.segments.find( + const segmentIndex = this.hydrationEvent.segments.findIndex( (segment) => segment.id === appliedEvent.segment.id, ); - if (segmentToUpdate) { - Object.assign(segmentToUpdate, appliedEvent.segment); + if (segmentIndex > -1) { + this.hydrationEvent.segments[segmentIndex] = + appliedEvent.segment; } else { this.hydrationEvent.segments.push(appliedEvent.segment); }