Skip to content
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

feat: DVC-9059 pull out EnvironmentConfigManager to its own library, change nodejs build to define external pacakges #564

Merged
merged 11 commits into from
Oct 24, 2023
Merged
18 changes: 18 additions & 0 deletions lib/shared/config-manager/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": ["../../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
12 changes: 12 additions & 0 deletions lib/shared/config-manager/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# config-manager

This library extracts the `EnvironmentConfigManager` Server SDK logic to be used across the NodeJS SDK
and Edge Worker SDKs.

## Building

Run `nx build config-manager` to build the library.

## Running unit tests

Run `nx test config-manager` to execute the unit tests via [Jest](https://jestjs.io).
Original file line number Diff line number Diff line change
@@ -1,23 +1,40 @@
jest.mock('../src/request')
jest.useFakeTimers()
jest.spyOn(global, 'setInterval')
jest.mock('../src/bucketing')

import { EnvironmentConfigManager } from '../src/environmentConfigManager'
import { importBucketingLib, getBucketingLib } from '../src/bucketing'
import { EnvironmentConfigManager } from '../src'
import { mocked } from 'jest-mock'
import { Response } from 'cross-fetch'
import { dvcDefaultLogger, ResponseError } from '@devcycle/js-cloud-server-sdk'
import {
DevCycleOptions,
dvcDefaultLogger,
ResponseError,
} from '@devcycle/js-cloud-server-sdk'
import { DVCLogger } from '@devcycle/types'
import { getEnvironmentConfig } from '../src/request'

const setInterval_mock = mocked(setInterval)
const getEnvironmentConfig_mock = mocked(getEnvironmentConfig)
const logger = dvcDefaultLogger()

const mockSDKConfig = jest.fn()

function getConfigManager(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving test to new config-manager lib

logger: DVCLogger,
sdkKey: string,
options: DevCycleOptions,
) {
return new EnvironmentConfigManager(
logger,
sdkKey,
mockSDKConfig,
setInterval,
clearInterval,
options,
)
}

describe('EnvironmentConfigManager Unit Tests', () => {
beforeAll(async () => {
await importBucketingLib()
})
beforeEach(() => {
getEnvironmentConfig_mock.mockReset()
setInterval_mock.mockReset()
Expand All @@ -43,18 +60,15 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 1000,
configPollingTimeoutMS: 1000,
})
await envConfig.fetchConfigPromise
expect(setInterval_mock).toHaveBeenCalledTimes(1)

await envConfig._fetchConfig()
expect(getBucketingLib().setConfigDataUTF8).toHaveBeenCalledWith(
'sdkKey',
Buffer.from('{}', 'utf8'),
)
expect(mockSDKConfig).toHaveBeenCalledWith('sdkKey', '{}')

expect(envConfig).toEqual(
expect.objectContaining({
Expand All @@ -72,7 +86,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 10,
configPollingTimeoutMS: 10000,
})
Expand All @@ -97,7 +111,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 1000,
configPollingTimeoutMS: 1000,
})
Expand Down Expand Up @@ -129,7 +143,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 500 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
expect(envConfig.fetchConfigPromise).rejects.toThrow(
'Failed to download DevCycle config.',
)
Expand All @@ -140,7 +154,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 403 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
expect(envConfig.fetchConfigPromise).rejects.toThrow(
'Invalid SDK key provided:',
)
Expand All @@ -150,7 +164,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
it('should throw error fetching config throws', () => {
getEnvironmentConfig_mock.mockRejectedValue(new Error('Error'))

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
expect(envConfig.fetchConfigPromise).rejects.toThrow(
'Failed to download DevCycle config.',
)
Expand All @@ -162,7 +176,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200, data: config }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 1000,
configPollingTimeoutMS: 1000,
})
Expand All @@ -184,7 +198,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 500 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
await expect(envConfig.fetchConfigPromise).rejects.toThrow()
expect(setInterval_mock).toHaveBeenCalledTimes(1)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@ import fetch, { Response } from 'cross-fetch'

global.fetch = fetch

import { getEnvironmentConfig } from '../src/request'
const fetchRequestMock = fetch as jest.MockedFn<typeof fetch>

import { publishEvents, getEnvironmentConfig } from '../src/request'
import { dvcDefaultLogger } from '@devcycle/js-cloud-server-sdk'
const logger = dvcDefaultLogger()

describe('request.ts Unit Tests', () => {
beforeEach(() => {
fetchRequestMock.mockReset()
Expand Down
13 changes: 13 additions & 0 deletions lib/shared/config-manager/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/* eslint-disable */
export default {
displayName: 'config-manager',
preset: '../../../jest.preset.js',
transform: {
'^.+\\.[tj]s$': [
'ts-jest',
{ tsconfig: '<rootDir>/tsconfig.spec.json' },
],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../../coverage/lib/shared/config-manager',
}
10 changes: 10 additions & 0 deletions lib/shared/config-manager/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "@devcycle/config-manager",
"private": true,
"version": "1.0.0",
"type": "commonjs",
"dependencies": {
"@devcycle/js-cloud-server-sdk": "^1.0.0",
"@devcycle/types": "^1.1.15"
}
}
41 changes: 41 additions & 0 deletions lib/shared/config-manager/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"name": "config-manager",
"$schema": "../../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "lib/shared/config-manager/src",
"projectType": "library",
"targets": {
"build": {
jonathannorris marked this conversation as resolved.
Show resolved Hide resolved
"executor": "@nx/js:tsc",
"outputs": ["{options.outputPath}"],
"options": {
"outputPath": "dist/lib/shared/config-manager",
"main": "lib/shared/config-manager/src/index.ts",
"tsConfig": "lib/shared/config-manager/tsconfig.lib.json",
"assets": ["lib/shared/config-manager/*.md"],
"external": ["shared-types", "js-cloud-server-sdk"]
}
},
"lint": {
"executor": "@nx/linter:eslint",
"outputs": ["{options.outputFile}"],
"options": {
"lintFilePatterns": ["lib/shared/config-manager/**/*.ts"]
}
},
"test": {
"executor": "@nx/jest:jest",
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
"options": {
"jestConfig": "lib/shared/config-manager/jest.config.ts",
"passWithNoTests": true
},
"configurations": {
"ci": {
"ci": true,
"codeCoverage": true
}
}
}
},
"tags": []
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
import { DVCLogger } from '@devcycle/types'
import { getBucketingLib } from './bucketing'
import { UserError } from './utils/userError'
// import { UserError } from './utils/userError'
import { getEnvironmentConfig } from './request'
import { ResponseError, DevCycleOptions } from '@devcycle/js-cloud-server-sdk'

type ConfigPollingOptions = DevCycleOptions & {
cdnURI?: string
}

type SetIntervalInterface = (handler: () => void, timeout?: number) => any
type ClearIntervalInterface = (intervalTimeout: any) => void

type SetConfigBuffer = (sdkKey: string, projectConfig: string) => void

export class UserError extends Error {
constructor(error: Error | string) {
super(error instanceof Error ? error.message : error)
this.name = 'UserError'
this.stack = error instanceof Error ? error.stack : undefined
}
}

export class EnvironmentConfigManager {
private readonly logger: DVCLogger
private readonly sdkKey: string
Expand All @@ -17,12 +29,18 @@ export class EnvironmentConfigManager {
private readonly requestTimeoutMS: number
private readonly cdnURI: string
fetchConfigPromise: Promise<void>
private intervalTimeout?: NodeJS.Timeout
private intervalTimeout?: any
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this need to be any?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically because NodeJS.Timeout isn't supported in webworker / browser environments, they just return a number I believe. So the strategy here was to add a setInterval + clearInterval interface that would be passed in, so this needs to be an Any if we want it to compile when used in the non-NodeJS SDKs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you wanted to get fancy you could use a type generic for this. Type it so that the generic is what is returned by the SetIntervalInterface and then use the same generic here.

private disablePolling = false
private readonly setConfigBuffer: SetConfigBuffer
private readonly setInterval: SetIntervalInterface
private readonly clearInterval: ClearIntervalInterface

constructor(
logger: DVCLogger,
sdkKey: string,
setConfigBuffer: SetConfigBuffer,
setInterval: SetIntervalInterface,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this polling config fetcher seems very specific to the nodejs local bucketing SDK, what purpose would it serve in the worker SDK other than to fetch it once?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workers do last for a while (I think I've seen 10's of minutes if not longer), we'd still want polling for them.

clearInterval: ClearIntervalInterface,
{
configPollingIntervalMS = 10000,
configPollingTimeoutMS = 5000,
Expand All @@ -32,6 +50,11 @@ export class EnvironmentConfigManager {
) {
this.logger = logger
this.sdkKey = sdkKey

this.setConfigBuffer = setConfigBuffer
this.setInterval = setInterval
this.clearInterval = clearInterval

this.pollingIntervalMS =
configPollingIntervalMS >= 1000 ? configPollingIntervalMS : 1000
this.requestTimeoutMS =
Expand All @@ -48,19 +71,19 @@ export class EnvironmentConfigManager {
if (this.disablePolling) {
return
}
this.intervalTimeout = setInterval(async () => {
this.intervalTimeout = this.setInterval(async () => {
try {
await this._fetchConfig()
} catch (ex) {
this.logger.error(ex.message)
this.logger.error((ex as Error).message)
}
}, this.pollingIntervalMS)
})
}

stopPolling(): void {
this.disablePolling = true
clearInterval(this.intervalTimeout)
this.clearInterval(this.intervalTimeout)
}

cleanup(): void {
Expand Down Expand Up @@ -119,8 +142,7 @@ export class EnvironmentConfigManager {
} else if (res?.status === 200 && projectConfig) {
try {
const etag = res?.headers.get('etag') || ''
const configBuffer = Buffer.from(projectConfig, 'utf8')
getBucketingLib().setConfigDataUTF8(this.sdkKey, configBuffer)
this.setConfigBuffer(this.sdkKey, projectConfig)
this.hasConfig = true
this.configEtag = etag
return
Expand Down
38 changes: 38 additions & 0 deletions lib/shared/config-manager/src/request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { RequestInitWithRetry } from 'fetch-retry'
import { get } from '@devcycle/js-cloud-server-sdk'

export async function getEnvironmentConfig(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the request.ts methods only used by the config-manager here.

url: string,
requestTimeout: number,
etag?: string,
): Promise<Response> {
const headers: Record<string, string> = etag
? { 'If-None-Match': etag }
: {}

return await getWithTimeout(
url,
{
headers: headers,
retries: 1,
},
requestTimeout,
)
}

async function getWithTimeout(
url: string,
requestConfig: RequestInit | RequestInitWithRetry,
timeout: number,
): Promise<Response> {
const controller = new AbortController()
const id = setTimeout(() => {
controller.abort()
}, timeout)
const response = await get(url, {
...requestConfig,
signal: controller.signal,
})
clearTimeout(id)
return response
}
13 changes: 13 additions & 0 deletions lib/shared/config-manager/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "../../../tsconfig.base.json",
"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.lib.json"
},
{
"path": "./tsconfig.spec.json"
}
]
}
Loading
Loading