Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 34 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
f2578b2
Add dev container
mikechu-optimizely Jul 31, 2023
77344fd
Update package lock
mikechu-optimizely Jul 31, 2023
3ea8a24
Add Jest extension settings
mikechu-optimizely Jul 31, 2023
0ecd776
Update config defaults; Added jsdoc
mikechu-optimizely Jul 31, 2023
1fa6a8e
Update logged warning message
mikechu-optimizely Jul 31, 2023
eff72d1
Allow polling under 1 second; only warn < 30s
mikechu-optimizely Jul 31, 2023
ebfa196
Fix jestCommandLine
mikechu-optimizely Jul 31, 2023
e747584
WIP: Add polling testing spec
mikechu-optimizely Jul 31, 2023
ae7181c
Add typeRoots
mikechu-optimizely Aug 1, 2023
85ae5af
Allow any intervals but only soft warn about them
mikechu-optimizely Aug 1, 2023
5a9e2dc
Finish unit tests around soft warning
mikechu-optimizely Aug 1, 2023
73c4cd2
Update copyright
mikechu-optimizely Aug 1, 2023
69b10c1
Add postStartCommand
mikechu-optimizely Aug 1, 2023
2fd3064
NIT: Apache link
mikechu-optimizely Aug 1, 2023
7cb90c3
Update node versions in CI
mikechu-optimizely Aug 1, 2023
6578aa7
Merge branch 'master' into mike/polling-warning
mikechu-optimizely Aug 1, 2023
34bfc12
Remove dev container postStartCommand
mikechu-optimizely Aug 2, 2023
21fd929
Rename update interval check func
mikechu-optimizely Aug 2, 2023
8c0623c
Add back Node 14 to CI
mikechu-optimizely Aug 2, 2023
64ecd88
Merge branch 'mike/polling-warning' of https://github.com/optimizely/…
mikechu-optimizely Aug 2, 2023
e16122b
Remove Node v14 from CI
mikechu-optimizely Aug 2, 2023
df25c88
Remove unnecessary function
mikechu-optimizely Aug 2, 2023
3f1ac72
Fix backward logic
mikechu-optimizely Aug 2, 2023
7bd9743
Allow for 30s, but not 29s for warn
mikechu-optimizely Aug 2, 2023
b51fe03
Add back back Node v14
mikechu-optimizely Aug 2, 2023
834afde
Merge branch 'master' into mike/polling-warning
mikechu-optimizely Aug 2, 2023
6170c9e
Run only unit test for node 14
mikechu-optimizely Aug 2, 2023
7958ae5
Add console.log
mikechu-optimizely Aug 2, 2023
5fff265
Remove typeRoots
mikechu-optimizely Aug 2, 2023
0eec4b7
Reuse existing TestDatafileManager
mikechu-optimizely Aug 2, 2023
43b9845
Revert package-lock upgrade
mikechu-optimizely Aug 2, 2023
aa3bb69
Return typeRoots
mikechu-optimizely Aug 2, 2023
4084e4f
Remove console.log & return CI
mikechu-optimizely Aug 2, 2023
00b7966
Re-remove typeRoots
mikechu-optimizely Aug 3, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
}
}
4 changes: 2 additions & 2 deletions .github/workflows/javascript.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest.rootPath": "/workspaces/javascript-sdk/packages/optimizely-sdk",
"jest.jestCommandLine": "./node_modules/.bin/jest",
"jest.autoRevealOutput": "on-exec-error"
}
14 changes: 10 additions & 4 deletions packages/optimizely-sdk/lib/modules/datafile-manager/config.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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`;

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';

Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>[] = [];
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});