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

chore: added property to know when instance of ConfigBody is created #1011

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 7 additions & 5 deletions lib/shared/bucketing-test-data/src/data/testData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
UserSubType,
VariableType,
} from '@devcycle/types'
import { plainToInstance } from 'class-transformer'

import moment from 'moment'

Expand Down Expand Up @@ -441,7 +442,7 @@
} {
const auds: { [id: string]: Omit<PublicAudience<string>, '_id'> } = {}
audiences.forEach((aud: PublicAudience) => {
const { _id, ...rest } = aud

Check warning on line 445 in lib/shared/bucketing-test-data/src/data/testData.ts

View workflow job for this annotation

GitHub Actions / build (22.12)

'_id' is assigned a value but never used
auds[aud._id] = {
...rest,
}
Expand All @@ -449,7 +450,7 @@
return auds
}

export const config: ConfigBody = {
export const config = {
project,
environment,
audiences: configBodyAudiences(reusableAudiences),
Expand Down Expand Up @@ -653,7 +654,7 @@
clientSDKKey: 'test',
}

export const barrenConfig: ConfigBody = {
export const barrenConfig = {
project,
environment,
audiences: {},
Expand Down Expand Up @@ -730,7 +731,7 @@
variableHashes: {},
}

export const configWithNullCustomData: ConfigBody = {
export const configWithNullCustomData = {
project,
environment,
audiences: configBodyAudiences(reusableAudiences),
Expand Down Expand Up @@ -783,9 +784,10 @@
clientSDKKey: 'test',
}

const instancedConfig = plainToInstance(ConfigBody, config)
export const configWithBucketingKey = (bucketingKey: string): ConfigBody => ({
...config,
features: config.features.map((feature) => ({
...instancedConfig,
features: instancedConfig.features.map((feature) => ({
...feature,
configuration: {
...feature.configuration,
Expand Down
51 changes: 47 additions & 4 deletions lib/shared/bucketing/__tests__/bucketing.test.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,36 @@
/* eslint-disable max-len */
import { Audience, FeatureType, PublicRollout, Rollout } from '@devcycle/types'
import {
Audience,
FeatureType,
PublicRollout,
Rollout,
ConfigBody,
} from '@devcycle/types'
import {
generateBoundedHashes,
decideTargetVariation,
generateBucketedConfig,
doesUserPassRollout,
} from '../src/bucketing'
import {
config,
barrenConfig,
configWithNullCustomData,
config as plainConfig,
barrenConfig as plainBarrenConfig,
configWithNullCustomData as plainConfigWithNullCustomData,
configWithBucketingKey,
} from '@devcycle/bucketing-test-data'

import moment from 'moment'
import times from 'lodash/times'
import filter from 'lodash/filter'
import * as uuid from 'uuid'
import { plainToInstance } from 'class-transformer'

const config = plainToInstance(ConfigBody, plainConfig)
const barrenConfig = plainToInstance(ConfigBody, plainBarrenConfig)
const configWithNullCustomData = plainToInstance(
ConfigBody,
plainConfigWithNullCustomData,
)

describe('User Hashing and Bucketing', () => {
it('generates buckets approximately in the same distribution as the variation distributions', () => {
Expand Down Expand Up @@ -1597,6 +1611,35 @@ describe('Rollout Logic', () => {
expect(doesUserPassRollout({ boundedHash: 0.9 })).toBeTruthy()
})

it('should not throw when given a ConfigBody config', () => {
expect(() =>
generateBucketedConfig({
config,
user: {
user_id: 'asuh',
},
}),
).not.toThrow()
})

it('should throw when given a non-ConfigBody config', () => {
const config = {} as ConfigBody

expect(() =>
generateBucketedConfig({
config,
user: {
user_id: 'asuh',
},
}),
).toThrow()
})

it('should have isConfigBody set to true', () => {
const config = plainToInstance(ConfigBody, plainConfig)
expect(config.isConfigBody).toBeTruthy()
})

describe('overrides', () => {
it('correctly overrides a bucketing decision and a feature that doesnt normally pass segmentation', () => {
const user = {
Expand Down
5 changes: 5 additions & 0 deletions lib/shared/bucketing/src/bucketing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ export const generateBucketedConfig = ({
user: DVCBucketingUser
overrides?: Record<string, string>
}): BucketedUserConfig => {
if (!config.isConfigBody) {
throw new Error(
'Config is not a ConfigBody, transform config using plainToInstance',
)
}
const variableMap: BucketedUserConfig['variables'] = {}
const featureKeyMap: BucketedUserConfig['features'] = {}
const featureVariationMap: BucketedUserConfig['featureVariationMap'] = {}
Expand Down
7 changes: 7 additions & 0 deletions lib/shared/types/src/types/config/configBody.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ export {
}

export class ConfigBody<IdType = string> {
/**
* This is to ensure usage of a proper ConfigBody instance by the bucketing library,
* using a regular JSON object will throw an error without this property, which is populated
* by using plainToInstance from class-transformer.
*/
isConfigBody = true
jsalaber marked this conversation as resolved.
Show resolved Hide resolved

/**
* Basic project data used for building bucketing response
*/
Expand Down
7 changes: 6 additions & 1 deletion lib/shared/vercel-edge-config/src/edge-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ describe('EdgeConfigSource', () => {
expect(get).toHaveBeenCalledWith('devcycle-config-v2-server-sdk-key')

expect(result).toEqual({
config: { key: 'value', lastModified: 'some date' },
config: {
key: 'value',
lastModified: 'some date',
isConfigBody: true,
},
lastModified: 'some date',
metaData: { resLastModified: 'some date' },
})
Expand Down Expand Up @@ -64,6 +68,7 @@ describe('EdgeConfigSource', () => {

expect(result).toEqual({
config: {
isConfigBody: true,
key: 'value',
lastModified: 'some date',
features: [
Expand Down
Loading