Bugfix SDK_UPDATE on server side

CHANGES.txt

@@ -1,3 +1,6 @@
+2.0.1 (November 25, 2024)
+ - Bugfixing - Fixed an issue with the SDK_UPDATE event on server-side, where it was not being emitted if there was an empty segment and the SDK received a feature flag update notification.
+
 2.0.0 (November 1, 2024)
  - Added support for targeting rules based on large segments.
  - Added `factory.destroy()` method, which invokes the `destroy` method on all SDK clients created by the factory.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "2.0.0",
+  "version": "2.0.1",
   "description": "Split JavaScript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",

src/sdkClient/sdkClient.ts

@@ -56,22 +56,18 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo
         // Mark the SDK as destroyed immediately
         sdkReadinessManager.readinessManager.destroy();
 
-        // For main client, release the SDK Key and record stat before flushing data
+        // For main client, cleanup the SDK Key, listeners and scheduled jobs, and record stat before flushing data
         if (!isSharedClient) {
           releaseApiKey(settings.core.authorizationKey);
           telemetryTracker.sessionLength();
+          signalListener && signalListener.stop();
+          uniqueKeysTracker && uniqueKeysTracker.stop();
         }
 
         // Stop background jobs
         syncManager && syncManager.stop();
 
         return __flush().then(() => {
-          // For main client, cleanup event listeners and scheduled jobs
-          if (!isSharedClient) {
-            signalListener && signalListener.stop();
-            uniqueKeysTracker && uniqueKeysTracker.stop();
-          }
-
           // Cleanup storage
           return storage.destroy();
         });

src/storages/AbstractMySegmentsCacheSync.ts

@@ -42,7 +42,7 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync
   // @TODO for client-side it should be the number of clients, but it requires a refactor of MySegments caches to simplify the code.
   abstract getKeysCount(): number
 
-  abstract getChangeNumber(name: string): number
+  abstract getChangeNumber(): number
 
   /**
    * For server-side synchronizer: the method is not used.

src/storages/inMemory/SegmentsCacheInMemory.ts

@@ -65,7 +65,7 @@ export class SegmentsCacheInMemory implements ISegmentsCacheSync {
   getChangeNumber(name: string) {
     const value = this.segmentChangeNumber[name];
 
-    return isIntegerNumber(value) ? value : -1;
+    return isIntegerNumber(value) ? value : undefined;
   }
 
   // No-op. Not used in server-side

src/storages/inRedis/SegmentsCacheInRedis.ts

@@ -44,10 +44,10 @@ export class SegmentsCacheInRedis implements ISegmentsCacheAsync {
     return this.redis.get(this.keys.buildSegmentTillKey(name)).then((value: string | null) => {
       const i = parseInt(value as string, 10);
 
-      return isNaNNumber(i) ? -1 : i;
+      return isNaNNumber(i) ? undefined : i;
     }).catch((e) => {
       this.log.error(LOG_PREFIX + 'Could not retrieve changeNumber from segments storage. Error: ' + e);
-      return -1;
+      return undefined;
     });
   }
 

src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts

@@ -17,7 +17,7 @@ describe('SEGMENTS CACHE IN REDIS', () => {
 
     expect(await cache.getChangeNumber('mocked-segment') === 1).toBe(true);
 
-    expect(await cache.getChangeNumber('inexistent-segment')).toBe(-1); // -1 if the segment doesn't exist
+    expect(await cache.getChangeNumber('inexistent-segment')).toBe(undefined); // -1 if the segment doesn't exist
 
     await cache.update('mocked-segment', ['d', 'e'], [], 2);
 

src/storages/pluggable/SegmentsCachePluggable.ts

@@ -55,10 +55,10 @@ export class SegmentsCachePluggable implements ISegmentsCacheAsync {
     return this.wrapper.get(this.keys.buildSegmentTillKey(name)).then((value: string | null) => {
       const i = parseInt(value as string, 10);
 
-      return isNaNNumber(i) ? -1 : i;
+      return isNaNNumber(i) ? undefined : i;
     }).catch((e) => {
       this.log.error(LOG_PREFIX + 'Could not retrieve changeNumber from segments storage. Error: ' + e);
-      return -1;
+      return undefined;
     });
   }
 

src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts

@@ -20,7 +20,7 @@ describe('SEGMENTS CACHE PLUGGABLE', () => {
 
     expect(await cache.getChangeNumber('mocked-segment') === 1).toBe(true);
 
-    expect(await cache.getChangeNumber('inexistent-segment')).toBe(-1); // -1 if the segment doesn't exist
+    expect(await cache.getChangeNumber('inexistent-segment')).toBe(undefined); // -1 if the segment doesn't exist
 
     await cache.update('mocked-segment', ['d', 'e'], [], 2);
 

src/storages/types.ts

@@ -237,7 +237,7 @@ export interface ISegmentsCacheBase {
   isInSegment(name: string, key?: string): MaybeThenable<boolean> // different signature on Server and Client-Side
   registerSegments(names: string[]): MaybeThenable<boolean | void> // only for Server-Side
   getRegisteredSegments(): MaybeThenable<string[]> // only for Server-Side
-  getChangeNumber(name: string): MaybeThenable<number> // only for Server-Side
+  getChangeNumber(name: string): MaybeThenable<number | undefined> // only for Server-Side
   update(name: string, addedKeys: string[], removedKeys: string[], changeNumber: number): MaybeThenable<boolean> // only for Server-Side
   clear(): MaybeThenable<boolean | void>
 }
@@ -248,7 +248,7 @@ export interface ISegmentsCacheSync extends ISegmentsCacheBase {
   registerSegments(names: string[]): boolean
   getRegisteredSegments(): string[]
   getKeysCount(): number // only used for telemetry
-  getChangeNumber(name?: string): number
+  getChangeNumber(name?: string): number | undefined
   update(name: string, addedKeys: string[], removedKeys: string[], changeNumber: number): boolean // only for Server-Side
   resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean // only for Sync Client-Side
   clear(): void
@@ -258,7 +258,7 @@ export interface ISegmentsCacheAsync extends ISegmentsCacheBase {
   isInSegment(name: string, key: string): Promise<boolean>
   registerSegments(names: string[]): Promise<boolean | void>
   getRegisteredSegments(): Promise<string[]>
-  getChangeNumber(name: string): Promise<number>
+  getChangeNumber(name: string): Promise<number | undefined>
   update(name: string, addedKeys: string[], removedKeys: string[], changeNumber: number): Promise<boolean>
   clear(): Promise<boolean | void>
 }

src/sync/polling/updaters/segmentChangesUpdater.ts

@@ -33,9 +33,9 @@ export function segmentChangesUpdaterFactory(
 
     return sincePromise.then(since => {
       // if fetchOnlyNew flag, avoid processing already fetched segments
-      return fetchOnlyNew && since !== -1 ?
+      return fetchOnlyNew && since !== undefined ?
         false :
-        segmentChangesFetcher(since, segmentName, noCache, till).then((changes) => {
+        segmentChangesFetcher(since || -1, segmentName, noCache, till).then((changes) => {
           return Promise.all(changes.map(x => {
             log.debug(`${LOG_PREFIX_SYNC_SEGMENTS}Processing ${segmentName} with till = ${x.till}. Added: ${x.added.length}. Removed: ${x.removed.length}`);
             return segments.update(segmentName, x.added, x.removed, x.till);

src/sync/polling/updaters/splitChangesUpdater.ts

@@ -19,7 +19,7 @@ function checkAllSegmentsExist(segments: ISegmentsCacheBase): Promise<boolean> {
   let registeredSegments = Promise.resolve(segments.getRegisteredSegments());
   return registeredSegments.then(segmentNames => {
     return Promise.all(segmentNames.map(segmentName => segments.getChangeNumber(segmentName)))
-      .then(changeNumbers => changeNumbers.every(changeNumber => changeNumber !== -1));
+      .then(changeNumbers => changeNumbers.every(changeNumber => changeNumber !== undefined));
   });
 }
 

src/sync/streaming/UpdateWorkers/MySegmentsUpdateWorker.ts

@@ -3,10 +3,11 @@ import { Backoff } from '../../../utils/Backoff';
 import { IUpdateWorker } from './types';
 import { ITelemetryTracker } from '../../../trackers/types';
 import { MEMBERSHIPS } from '../../../utils/constants';
-import { ISegmentsCacheSync, IStorageSync } from '../../../storages/types';
+import { IStorageSync } from '../../../storages/types';
 import { ILogger } from '../../../logger/types';
 import { FETCH_BACKOFF_MAX_RETRIES } from './constants';
 import { MEMBERSHIPS_LS_UPDATE, MEMBERSHIPS_MS_UPDATE } from '../constants';
+import { AbstractMySegmentsCacheSync } from '../../../storages/AbstractMySegmentsCacheSync';
 
 /**
  * MySegmentsUpdateWorker factory
@@ -16,7 +17,7 @@ export function MySegmentsUpdateWorker(log: ILogger, storage: Pick<IStorageSync,
   let _delay: undefined | number;
   let _delayTimeoutID: any;
 
-  function createUpdateWorker(mySegmentsCache: ISegmentsCacheSync) {
+  function createUpdateWorker(mySegmentsCache: AbstractMySegmentsCacheSync) {
 
     let maxChangeNumber = 0; // keeps the maximum changeNumber among queued events
     let currentChangeNumber = -1;
@@ -117,8 +118,8 @@ export function MySegmentsUpdateWorker(log: ILogger, storage: Pick<IStorageSync,
   }
 
   const updateWorkers = {
-    [MEMBERSHIPS_MS_UPDATE]: createUpdateWorker(storage.segments),
-    [MEMBERSHIPS_LS_UPDATE]: createUpdateWorker(storage.largeSegments!),
+    [MEMBERSHIPS_MS_UPDATE]: createUpdateWorker(storage.segments as AbstractMySegmentsCacheSync),
+    [MEMBERSHIPS_LS_UPDATE]: createUpdateWorker(storage.largeSegments as AbstractMySegmentsCacheSync),
   };
 
   return {

src/sync/streaming/UpdateWorkers/SegmentsUpdateWorker.ts

@@ -21,7 +21,7 @@ export function SegmentsUpdateWorker(log: ILogger, segmentsSyncTask: ISegmentsSy
 
     function __handleSegmentUpdateCall() {
       isHandlingEvent = true;
-      if (maxChangeNumber > segmentsCache.getChangeNumber(segment)) {
+      if (maxChangeNumber > (segmentsCache.getChangeNumber(segment) || -1)) {
         handleNewEvent = false;
 
         // fetch segments revalidating data if cached
@@ -32,7 +32,7 @@ export function SegmentsUpdateWorker(log: ILogger, segmentsSyncTask: ISegmentsSy
           } else {
             const attempts = backoff.attempts + 1;
 
-            if (maxChangeNumber <= segmentsCache.getChangeNumber(segment)) {
+            if (maxChangeNumber <= (segmentsCache.getChangeNumber(segment) || -1)) {
               log.debug(`Refresh completed${cdnBypass ? ' bypassing the CDN' : ''} in ${attempts} attempts.`);
               isHandlingEvent = false;
               return;
@@ -60,7 +60,7 @@ export function SegmentsUpdateWorker(log: ILogger, segmentsSyncTask: ISegmentsSy
 
     return {
       put(changeNumber: number) {
-        const currentChangeNumber = segmentsCache.getChangeNumber(segment);
+        const currentChangeNumber = segmentsCache.getChangeNumber(segment) || -1;
 
         if (changeNumber <= currentChangeNumber || changeNumber <= maxChangeNumber) return;
 

src/sync/syncTask.ts

@@ -68,8 +68,8 @@ export function syncTaskFactory<Input extends any[], Output = any>(log: ILogger,
     },
 
     stop() {
-      running = false;
-      if (timeoutID) {
+      if (running) {
+        running = false;
         log.debug(SYNC_TASK_STOP, [taskName]);
         clearTimeout(timeoutID);
         timeoutID = undefined;