8000 [FSSDK-9506] Allow any polling interval but implement warning log (#… · chrisvaf/javascript-sdk@bc51f5d · GitHub
[go: up one dir, main page]
Skip to content

Commit bc51f5d

Browse files
mikechu-optimizelymikechu-optimizely
authored
[FSSDK-9506] Allow any polling interval but implement warning log (optimizely#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
1 parent 2f892be commit bc51f5d

File tree

8 files changed

+115
-22
lines changed

8 files changed

+115
-22
lines changed

‎.devcontainer/devcontainer.json

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"name": "Javascript SDK",
3+
"image": "mcr.microsoft.com/devcontainers/typescript-node:1-18-bookworm",
4+
5+
"postCreateCommand": "cd /workspaces/javascript-sdk/packages/optimizely-sdk && npm install -g npm && npm install",
6+
7+
"customizations": {
8+
"vscode": {
9+
"extensions": [
10+
"dbaeumer.vscode-eslint",
11+
"eamodio.gitlens",
12+
"esbenp.prettier-vscode",
13+
"Gruntfuggly.todo-tree",
14+
"github.vscode-github-actions",
15+
"Orta.vscode-jest",
16+
"ms-vscode.test-adapter-converter"
17+
]
18+
}
19+
}
20+
}

‎.github/workflows/javascript.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ jobs:
1717
- name: Set up Node
1818
uses: actions/setup-node@v3
1919
with:
20-
node-version: 12
20+
node-version: 14
2121
cache-dependency-path: packages/optimizely-sdk/package-lock.json
2222
cache: 'npm'
2323
- name: Run linting
@@ -118,7 +118,7 @@ jobs:
118118
- name: Set up Node
119119
uses: actions/setup-node@v3
120120
with:
121-
node-version: 12
121+
node-version: 14
122122
cache: 'npm'
123123
cache-dependency-path: ${{ matrix.package }}/package-lock.json
124124
- name: Test sub packages

‎.vscode/settings.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"jest.rootPath": "/workspaces/javascript-sdk/packages/optimizely-sdk",
3+
"jest.jestCommandLine": "./node_modules/.bin/jest",
4+
"jest.autoRevealOutput": "on-exec-error"
5+
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/**
2-
* Copyright 2022, Optimizely
2+
* Copyright 2022-2023, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* https://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -14,9 +14,15 @@
1414
* limitations under the License.
1515
*/
1616

17-
export const DEFAULT_UPDATE_INTERVAL = 5 * 60 * 1000; // 5 minutes
17+
const DEFAULT_UPDATE_INTERVAL_MINUTES = 5;
18+
/** Standard interval (5 minutes in milliseconds) for polling datafile updates.; */
19+
export const DEFAULT_UPDATE_INTERVAL = DEFAULT_UPDATE_INTERVAL_MINUTES * 60 * 1000;
1820

19-
export const MIN_UPDATE_INTERVAL = 1000;
21+
const MIN_UPDATE_INTERVAL_SECONDS = 30;
22+
/** Minimum allowed interval (30 seconds in milliseconds) for polling datafile updates. */
23+
export const MIN_UPDATE_INTERVAL = MIN_UPDATE_INTERVAL_SECONDS * 1000;
24+
25+
export const UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE = `Polling intervals below ${MIN_UPDATE_INTERVAL_SECONDS} seconds are not recommended.`;
2026

2127
export const DEFAULT_URL_TEMPLATE = `https://cdn.optimizely.com/datafiles/%s.json`;
2228

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/**
2-
* Copyright 2022, Optimizely
2+
* Copyright 2022-2023, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* https://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -40,6 +40,7 @@ export interface DatafileManagerConfig {
4040
autoUpdate?: boolean;
4141
datafile?: string;
4242
sdkKey: string;
43+
/** Polling interval in milliseconds to check for datafile updates. */
4344
updateInterval?: number;
4445
urlTemplate?: string;
4546
cache?: PersistentKeyValueCache;

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

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* https://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -19,7 +19,7 @@ import { sprintf } from '../../../lib/utils/fns';
1919
import { DatafileManager, DatafileManagerConfig, DatafileUpdate } from './datafileManager';
2020
import EventEmitter, { Disposer } from './eventEmitter';
2121
import { AbortableRequest, Response, Headers } from './http';
22-
import { DEFAULT_UPDATE_INTERVAL, MIN_UPDATE_INTERVAL, DEFAULT_URL_TEMPLATE } from './config';
22+
import { DEFAULT_UPDATE_INTERVAL, MIN_UPDATE_INTERVAL, DEFAULT_URL_TEMPLATE, UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE } from './config';
2323
import BackoffController from './backoffController';
2424
import PersistentKeyValueCache from './persistentKeyValueCache';
2525

@@ -30,10 +30,6 @@ const logger = getLogger('DatafileManager');
3030

3131
const UPDATE_EVT = 'update';
3232

33-
function isValidUpdateInterval(updateInterval: number): boolean {
34-
return updateInterval >= MIN_UPDATE_INTERVAL;
35-
}
36-
3733
function isSuccessStatusCode(statusCode: number): boolean {
3834
return statusCode >= 200 && statusCode < 400;
3935
}
@@ -124,8 +120,8 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
124120
this.cacheKey = 'opt-datafile-' + sdkKey;
125121
this.sdkKey = sdkKey;
126122
this.isReadyPromiseSettled = false;
127-
this.readyPromiseResolver = (): void => {};
128-
this.readyPromiseRejecter = (): void => {};
123+
this.readyPromiseResolver = (): void => { };
124+
this.readyPromiseRejecter = (): void => { };
129125
this.readyPromise = new Promise((resolve, reject) => {
130126
this.readyPromiseResolver = resolve;
131127
this.readyPromiseRejecter = reject;
@@ -145,16 +141,20 @@ export default abstract class HttpPollingDatafileManager implements DatafileMana
145141
this.datafileUrl = sprintf(urlTemplate, sdkKey);
146142

147143
this.emitter = new EventEmitter();
144+
148145
this.autoUpdate = autoUpdate;
149-
if (isValidUpdateInterval(updateInterval)) {
150-
this.updateInterval = updateInterval;
151-
} else {
152-
logger.warn('Invalid updateInterval %s, defaulting to %s', updateInterval, DEFAULT_UPDATE_INTERVAL);
153-
this.updateInterval = DEFAULT_UPDATE_INTERVAL;
146+
147+
this.updateInterval = updateInterval;
148+
if (this.updateInterval < MIN_UPDATE_INTERVAL) {
149+
logger.warn(UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE);
154150
}
151+
155152
this.currentTimeout = null;
153+
156154
this.currentRequest = null;
155+
157156
this.backoffController = new BackoffController();
157+
158158
this.syncOnCurrentRequestComplete = false;
159159
}
160160

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import BackoffController from '../lib/modules/datafile-manager/backoffController
3535

3636
// Test implementation:
3737
// - Does not make any real requests: just resolves with queued responses (tests push onto queuedResponses)
38-
class TestDatafileManager extends HttpPollingDatafileManager {
38+
export class TestDatafileManager extends HttpPollingDatafileManager {
3939
queuedResponses: (Response | Error)[] = [];
4040

4141
responsePromises: Promise<Response>[] = [];

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/**
2+
* Copyright 2023 Optimizely
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { resetCalls, spy, verify } from 'ts-mockito';
18+
import { LogLevel, LoggerFacade, getLogger, setLogLevel } from '../lib/modules/logging';
19+
import { UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE } from '../lib/modules/datafile-manager/config';
20+
import { TestDatafileManager } from './httpPollingDatafileManager.spec';
21+
22+
describe('HttpPollingDatafileManager polling', () => {
23+
let spiedLogger: LoggerFacade;
24+
25+
const loggerName = 'DatafileManager';
26+
const sdkKey = 'not-real-sdk';
27+
28+
beforeAll(() => {
29+
setLogLevel(LogLevel.DEBUG);
30+
const actualLogger = getLogger(loggerName);
31+
spiedLogger = spy(actualLogger);
32+
});
33+
34+
beforeEach(() => {
35+
resetCalls(spiedLogger);
36+
});
37+
38+
39+
it('should log polling interval below 30 seconds', () => {
40+
const below30Seconds = 29 * 1000;
41+
42+
new TestDatafileManager({
43+
sdkKey,
44+
updateInterval: below30Seconds,
45+
});
46+
47+
48+
verify(spiedLogger.warn(UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE)).once();
49+
});
50+
51+
it('should not log when polling interval above 30s', () => {
52+
const oneMinute = 60 * 1000;
53+
54+
new TestDatafileManager({
55+
sdkKey,
56+
updateInterval: oneMinute,
57+
});
58+
59+
verify(spiedLogger.warn(UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE)).never();
60+
});
61+
});

0 commit comments

Comments
 (0)
0