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: Add migrationVariation method. #212

Merged
merged 2 commits into from
Jul 24, 2023
Merged
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
105 changes: 105 additions & 0 deletions packages/shared/sdk-server/__tests__/LDClient.migrations.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { LDClientImpl, LDMigrationStage } from '../src';
import { LDClientCallbacks } from '../src/LDClientImpl';
import TestData from '../src/integrations/test_data/TestData';
import basicPlatform from './evaluation/mocks/platform';

/**
* Basic callback handler that records errors for tests.
*/
export default function makeCallbacks(): [Error[], LDClientCallbacks] {
const errors: Error[] = [];
return [
errors,
{
onError: (error) => {
errors.push(error);
},
onFailed: () => {},
onReady: () => {},
onUpdate: () => {},
hasEventListeners: () => true,
},
];
}

describe('given an LDClient with test data', () => {
let client: LDClientImpl;
let td: TestData;
let callbacks: LDClientCallbacks;
let errors: Error[];

beforeEach(async () => {
td = new TestData();
[errors, callbacks] = makeCallbacks();
client = new LDClientImpl(
'sdk-key',
basicPlatform,
{
updateProcessor: td.getFactory(),
sendEvents: false,
},
callbacks
);

await client.waitForInitialization();
});

afterEach(() => {
client.close();
});

it.each(['off', 'dualwrite', 'shadow', 'live', 'rampdown', 'complete'])(
'handles valid migration stages: %p',
async (value) => {
const flagKey = 'migration';
td.update(td.flag(flagKey).valueForAll(value));
// Get a default value that is not the value under test.
const defaultValue = Object.values(LDMigrationStage).find((item) => item !== value);
// Verify the pre-condition that the default value is not the value under test.
expect(defaultValue).not.toEqual(value);
const res = await client.variationMigration(
flagKey,
{ key: 'test-key' },
defaultValue as LDMigrationStage
);
expect(res).toEqual(value);
}
);

it.each([
LDMigrationStage.Off,
LDMigrationStage.DualWrite,
LDMigrationStage.Shadow,
LDMigrationStage.Live,
LDMigrationStage.Rampdown,
LDMigrationStage.Complete,
])('returns the default value if the flag does not exist: default = %p', async (stage) => {
const res = await client.variationMigration('no-flag', { key: 'test-key' }, stage);

expect(res).toEqual(stage);
});

it('produces an error event for a migration flag with an incorrect value', async () => {
const flagKey = 'bad-migration';
td.update(td.flag(flagKey).valueForAll('potato'));
const res = await client.variationMigration(flagKey, { key: 'test-key' }, LDMigrationStage.Off);
expect(res).toEqual(LDMigrationStage.Off);
expect(errors.length).toEqual(1);
expect(errors[0].message).toEqual(
'Unrecognized MigrationState for "bad-migration"; returning default value.'
);
});

it('includes an error in the node callback', (done) => {
const flagKey = 'bad-migration';
td.update(td.flag(flagKey).valueForAll('potato'));
client.variationMigration(flagKey, { key: 'test-key' }, LDMigrationStage.Off, (err, value) => {
const error = err as Error;
expect(error.message).toEqual(
'Unrecognized MigrationState for "bad-migration"; returning default value.'
);
expect(value).toEqual(LDMigrationStage.Off);
done();
});
});
});
2 changes: 1 addition & 1 deletion packages/shared/sdk-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"build": "npx tsc",
"clean": "npx tsc --build --clean",
"lint": "npx eslint . --ext .ts",
"lint:fix": "yarn run lint -- --fix"
"lint:fix": "yarn run lint --fix"
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated minor fix to run lint.

},
"license": "Apache-2.0",
"dependencies": {
Expand Down
27 changes: 26 additions & 1 deletion packages/shared/sdk-server/src/LDClientImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,15 @@ import {
subsystem,
internal,
} from '@launchdarkly/js-sdk-common';
import { LDClient, LDFlagsStateOptions, LDOptions, LDStreamProcessor, LDFlagsState } from './api';
import {
LDClient,
LDFlagsStateOptions,
LDOptions,
LDStreamProcessor,
LDFlagsState,
LDMigrationStage,
IsMigrationStage,
} from './api';
import { BigSegmentStoreMembership } from './api/interfaces';
import BigSegmentsManager from './BigSegmentsManager';
import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl';
Expand Down Expand Up @@ -260,6 +268,23 @@ export default class LDClientImpl implements LDClient {
return res.detail;
}

async variationMigration(
key: string,
context: LDContext,
defaultValue: LDMigrationStage,
Copy link
Member Author

Choose a reason for hiding this comment

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

As long as they do not break the typing the default value is of type LDMigrationStage, so I am not validating the default value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Though if it was not the correct type, and it came back out as the default, it would still get validated.

callback?: (err: any, res: LDMigrationStage) => void
): Promise<LDMigrationStage> {
const stringValue = await this.variation(key, context, defaultValue as string);
if (!IsMigrationStage(stringValue)) {
const error = new Error(`Unrecognized MigrationState for "${key}"; returning default value.`);
this.onError(error);
callback?.(error, defaultValue);
Copy link
Member Author

Choose a reason for hiding this comment

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

Most of the implementation here is just to handle the compatibility we have with callbacks. Everything supports both a callback and a promise. The error handling is also a little unique, we raise error events or put errors in callbacks.

Copy link
Member Author

@kinyoklion kinyoklion Jul 24, 2023

Choose a reason for hiding this comment

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

I may consider removing callback support entirely for this method, but I will do that in another change after some discussion.

return defaultValue;
}
callback?.(null, stringValue as LDMigrationStage);
return stringValue as LDMigrationStage;
}

async allFlagsState(
context: LDContext,
options?: LDFlagsStateOptions,
Expand Down
23 changes: 23 additions & 0 deletions packages/shared/sdk-server/src/api/LDClient.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { LDContext, LDEvaluationDetail, LDFlagValue } from '@launchdarkly/js-sdk-common';
import { LDFlagsStateOptions } from './data/LDFlagsStateOptions';
import { LDFlagsState } from './data/LDFlagsState';
import { LDMigrationStage } from './data/LDMigrationStage';

/**
* The LaunchDarkly SDK client object.
Expand Down Expand Up @@ -119,6 +120,28 @@ export interface LDClient {
callback?: (err: any, res: LDEvaluationDetail) => void
): Promise<LDEvaluationDetail>;

/**
* TKTK: Should use a common description.
*
* If the evaluated value of the flag cannot be converted to an LDMigrationStage, then an error
* event will be raised.
*
* @param key The unique key of the feature flag.
* @param context The context requesting the flag. The client will generate an analytics event to
* register this context with LaunchDarkly if the context does not already exist.
* @param defaultValue The default value of the flag, to be used if the value is not available
* from LaunchDarkly.
* @param callback A Node-style callback to receive the result (as an {@link LDMigrationStage}).
* @returns
* A Promise which will be resolved with the result (as an{@link LDMigrationStage}).
*/
variationMigration(
key: string,
context: LDContext,
defaultValue: LDMigrationStage,
callback?: (err: any, res: LDMigrationStage) => void
): Promise<LDMigrationStage>;

/**
* Builds an object that encapsulates the state of all feature flags for a given context.
* This includes the flag values and also metadata that can be used on the front end. This
Expand Down
12 changes: 12 additions & 0 deletions packages/shared/sdk-server/src/api/data/LDMigrationStage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export enum LDMigrationStage {
Off = 'off',
DualWrite = 'dualwrite',
Shadow = 'shadow',
Live = 'live',
Rampdown = 'rampdown',
Complete = 'complete',
}

export function IsMigrationStage(value: string): boolean {
return Object.values(LDMigrationStage).includes(value as LDMigrationStage);
}
1 change: 1 addition & 0 deletions packages/shared/sdk-server/src/api/data/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './LDFlagsStateOptions';
export * from './LDFlagsState';
export * from './LDMigrationStage';