[FSSDK-9506] Allow any polling interval but implement warning log (#841)

* Add dev container

* Update package lock

* Add Jest extension settings

* Update config defaults; Added jsdoc

* Update logged warning message

* Allow polling under 1 second; only warn < 30s

* Fix jestCommandLine

* WIP: Add polling testing spec

The other file is WAY too long.  Need to fix test fails.

* Add typeRoots

* Allow any intervals but only soft warn about them

* Finish unit tests around soft warning

* Update copyright

* Add postStartCommand

* NIT: Apache link

* Update node versions in CI

* Remove dev container postStartCommand

* Rename update interval check func

* Add back Node 14 to CI

* Remove Node v14 from CI

* Remove unnecessary function

* Fix backward logic

* Allow for 30s, but not 29s for warn

* Add back back Node v14

* Run only unit test for node 14

* Add console.log

* Remove typeRoots

* Reuse existing TestDatafileManager

* Revert package-lock upgrade

* Return typeRoots

* Remove console.log & return CI

* Re-remove typeRoots

Author: mikechu-optimizely

.devcontainer/devcontainer.json

@@ -0,0 +1,20 @@
+{
+	"name": "Javascript SDK",
+	"image": "mcr.microsoft.com/devcontainers/typescript-node:1-18-bookworm",
+
+	"postCreateCommand": "cd /workspaces/javascript-sdk/packages/optimizely-sdk && npm install -g npm && npm install",
+
+	"customizations": {
+		"vscode": {
+			"extensions": [
+				"dbaeumer.vscode-eslint",
+				"eamodio.gitlens",
+				"esbenp.prettier-vscode",
+				"Gruntfuggly.todo-tree",
+				"github.vscode-github-actions",
+				"Orta.vscode-jest",
+				"ms-vscode.test-adapter-converter"
+			]
+		}
+	}
+}

.github/workflows/javascript.yml

@@ -17,7 +17,7 @@ jobs:
     - name: Set up Node
       uses: actions/setup-node@v3
       with:
-        node-version: 12
+        node-version: 14
         cache-dependency-path: packages/optimizely-sdk/package-lock.json
         cache: 'npm'
     - name: Run linting
@@ -118,7 +118,7 @@ jobs:
     - name: Set up Node
       uses: actions/setup-node@v3
       with:
-        node-version: 12
+        node-version: 14
         cache: 'npm'
         cache-dependency-path: ${{ matrix.package }}/package-lock.json
     - name: Test sub packages

.vscode/settings.json

@@ -0,0 +1,5 @@
+{
+    "jest.rootPath": "/workspaces/javascript-sdk/packages/optimizely-sdk",
+    "jest.jestCommandLine": "./node_modules/.bin/jest",
+    "jest.autoRevealOutput": "on-exec-error"
+}

packages/optimizely-sdk/lib/modules/datafile-manager/config.ts

@@ -1,11 +1,11 @@
 /**
- * Copyright 2022, Optimizely
+ * Copyright 2022-2023, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ * https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -14,9 +14,15 @@
  * limitations under the License.
  */
 
-export const DEFAULT_UPDATE_INTERVAL = 5 * 60 * 1000; // 5 minutes
+const DEFAULT_UPDATE_INTERVAL_MINUTES = 5;
+/** Standard interval (5 minutes in milliseconds) for polling datafile updates.; */
+export const DEFAULT_UPDATE_INTERVAL = DEFAULT_UPDATE_INTERVAL_MINUTES * 60 * 1000;
 
-export const MIN_UPDATE_INTERVAL = 1000;
+const MIN_UPDATE_INTERVAL_SECONDS = 30;
+/** Minimum allowed interval (30 seconds in milliseconds) for polling datafile updates. */
+export const MIN_UPDATE_INTERVAL = MIN_UPDATE_INTERVAL_SECONDS * 1000;
+
+export const UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE = `Polling intervals below ${MIN_UPDATE_INTERVAL_SECONDS} seconds are not recommended.`;
 
 export const DEFAULT_URL_TEMPLATE = `https://cdn.optimizely.com/datafiles/%s.json`;
 

packages/optimizely-sdk/lib/modules/datafile-manager/datafileManager.ts

@@ -1,11 +1,11 @@
 /**
- * Copyright 2022, Optimizely
+ * Copyright 2022-2023, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ * https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -40,6 +40,7 @@ export interface DatafileManagerConfig {
   autoUpdate?: boolean;
   datafile?: string;
   sdkKey: string;
+  /** Polling interval in milliseconds to check for datafile updates. */
   updateInterval?: number;
   urlTemplate?: string;
   cache?: PersistentKeyValueCache;

packages/optimizely-sdk/lib/modules/datafile-manager/httpPollingDatafileManager.ts

@@ -5,7 +5,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ * https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -19,7 +19,7 @@ import { sprintf } from '../../../lib/utils/fns';
 import { DatafileManager, DatafileManagerConfig, DatafileUpdate } from './datafileManager';
 import EventEmitter, { Disposer } from './eventEmitter';
 import { AbortableRequest, Response, Headers } from './http';
-import { DEFAULT_UPDATE_INTERVAL, MIN_UPDATE_INTERVAL, DEFAULT_URL_TEMPLATE } from './config';
+import { DEFAULT_UPDATE_INTERVAL, MIN_UPDATE_INTERVAL, DEFAULT_URL_TEMPLATE, UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE } from './config';
 import BackoffController from './backoffController';
 import PersistentKeyValueCache from './persistentKeyValueCache';
 
@@ -30,10 +30,6 @@ const logger = getLogger('DatafileManager');
 
 const UPDATE_EVT = 'update';
 
-function isValidUpdateInterval(updateInterval: number): boolean {
-  return updateInterval >= MIN_UPDATE_INTERVAL;
-}
-
 function isSuccessStatusCode(statusCode: number): boolean {
   return statusCode >= 200 && statusCode < 400;
 }
@@ -124,8 +120,8 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
     this.cacheKey = 'opt-datafile-' + sdkKey;
     this.sdkKey = sdkKey;
     this.isReadyPromiseSettled = false;
-    this.readyPromiseResolver = (): void => {};
-    this.readyPromiseRejecter = (): void => {};
+    this.readyPromiseResolver = (): void => { };
+    this.readyPromiseRejecter = (): void => { };
     this.readyPromise = new Promise((resolve, reject) => {
       this.readyPromiseResolver = resolve;
       this.readyPromiseRejecter = reject;
@@ -145,16 +141,20 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
     this.datafileUrl = sprintf(urlTemplate, sdkKey);
 
     this.emitter = new EventEmitter();
+
     this.autoUpdate = autoUpdate;
-    if (isValidUpdateInterval(updateInterval)) {
-      this.updateInterval = updateInterval;
-    } else {
-      logger.warn('Invalid updateInterval %s, defaulting to %s', updateInterval, DEFAULT_UPDATE_INTERVAL);
-      this.updateInterval = DEFAULT_UPDATE_INTERVAL;
+
+    this.updateInterval = updateInterval;
+    if (this.updateInterval < MIN_UPDATE_INTERVAL) {
+      logger.warn(UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE);
     }
+
     this.currentTimeout = null;
+
     this.currentRequest = null;
+
     this.backoffController = new BackoffController();
+
     this.syncOnCurrentRequestComplete = false;
   }
 

packages/optimizely-sdk/tests/httpPollingDatafileManager.spec.ts

@@ -35,7 +35,7 @@ import BackoffController from '../lib/modules/datafile-manager/backoffController
 
 // Test implementation:
 //   - Does not make any real requests: just resolves with queued responses (tests push onto queuedResponses)
-class TestDatafileManager extends HttpPollingDatafileManager {
+export class TestDatafileManager extends HttpPollingDatafileManager {
   queuedResponses: (Response | Error)[] = [];
 
   responsePromises: Promise<Response>[] = [];

packages/optimizely-sdk/tests/httpPollingDatafileManagerPolling.spec.ts

@@ -0,0 +1,61 @@
+/**
+ * Copyright 2023 Optimizely
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { resetCalls, spy, verify } from 'ts-mockito';
+import { LogLevel, LoggerFacade, getLogger, setLogLevel } from '../lib/modules/logging';
+import { UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE } from '../lib/modules/datafile-manager/config';
+import { TestDatafileManager } from './httpPollingDatafileManager.spec';
+
+describe('HttpPollingDatafileManager polling', () => {
+    let spiedLogger: LoggerFacade;
+
+    const loggerName = 'DatafileManager';
+    const sdkKey = 'not-real-sdk';
+
+    beforeAll(() => {
+        setLogLevel(LogLevel.DEBUG);
+        const actualLogger = getLogger(loggerName);
+        spiedLogger = spy(actualLogger);
+    });
+
+    beforeEach(() => {
+        resetCalls(spiedLogger);
+    });
+
+
+    it('should log polling interval below 30 seconds', () => {
+        const below30Seconds = 29 * 1000;
+
+        new TestDatafileManager({
+            sdkKey,
+            updateInterval: below30Seconds,
+        });
+
+        
+        verify(spiedLogger.warn(UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE)).once();
+    });
+
+    it('should not log when polling interval above 30s', () => {
+        const oneMinute = 60 * 1000;
+
+        new TestDatafileManager({
+            sdkKey,
+            updateInterval: oneMinute,
+        });
+
+        verify(spiedLogger.warn(UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE)).never();
+    });
+});