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.

Sign up for GitHub

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 support for client-side prerequisite events. #593

Closed
wants to merge 5 commits into from
Closed

feat: Add support for client-side prerequisite events. #593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb42068
feat: Add support for server-side prerequisite details.
kinyoklion Sep 25, 2024
95a656d
Add client-side feature event and allFlags test.
kinyoklion Sep 25, 2024
20af513
Fix client-side tests.
kinyoklion Sep 25, 2024
bf8a0d8
Merge branch 'main' into rlamb/client-prereq-proto
kinyoklion Oct 3, 2024
da7ce51
Test dep update.
kinyoklion Oct 3, 2024
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
10 changes: 10 additions & 0 deletions packages/shared/common/src/api/data/LDEvaluationDetail.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export interface LDEvaluationDetail {
* An object describing the main factor that influenced the flag evaluation value.
*/
reason: LDEvaluationReason;

/**
* An ordered list of prerequisite flag keys evaluated while determining the flags value.
*/
prerequisites?: string[];
}

export interface LDEvaluationDetailTyped<TFlag> {
Expand All @@ -47,4 +52,9 @@ export interface LDEvaluationDetailTyped<TFlag> {
* An object describing the main factor that influenced the flag evaluation value.
*/
reason: LDEvaluationReason;

/**
* An ordered list of prerequisite flag keys evaluated while determining the flags value.
*/
prerequisites?: string[];
}
41 changes: 41 additions & 0 deletions packages/shared/sdk-client/__tests__/LDClientImpl.events.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,45 @@ describe('sdk-client object', () => {
expect.stringMatching(/was called with a non-numeric/),
);
});

it('sends events for prerequisite flags', async () => {
await ldc.identify({ kind: 'user', key: 'bob' });
ldc.variation('has-prereq-depth-1', false);
ldc.flush();

// Prerequisite evaluation event should be emitted before the evaluation event for the flag
// being evaluated.
expect(mockedSendEvent).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
context: expect.anything(),
creationDate: expect.any(Number),
default: undefined,
key: 'is-prereq',
kind: 'feature',
samplingRatio: 1,
trackEvents: true,
value: true,
variation: 0,
version: 1,
withReasons: false,
}),
);
expect(mockedSendEvent).toHaveBeenNthCalledWith(
3,
expect.objectContaining({
context: expect.anything(),
creationDate: expect.any(Number),
default: false,
key: 'has-prereq-depth-1',
kind: 'feature',
samplingRatio: 1,
trackEvents: true,
value: true,
variation: 0,
version: 4,
withReasons: false,
}),
);
});
});
10 changes: 10 additions & 0 deletions packages/shared/sdk-client/__tests__/LDClientImpl.storage.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ describe('sdk-client storage', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down Expand Up @@ -156,6 +158,8 @@ describe('sdk-client storage', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down Expand Up @@ -218,6 +222,8 @@ describe('sdk-client storage', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down Expand Up @@ -388,6 +394,8 @@ describe('sdk-client storage', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down Expand Up @@ -517,6 +525,8 @@ describe('sdk-client storage', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/sdk-client/__tests__/LDClientImpl.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ describe('sdk-client object', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/sdk-client/__tests__/LDClientImpl.timeout.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ describe('sdk-client identify timeout', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand All @@ -112,6 +114,8 @@ describe('sdk-client identify timeout', () => {
'easter-i-tunes-special': false,
'easter-specials': 'no specials',
fdsafdsafdsafdsa: true,
'has-prereq-depth-1': true,
'is-prereq': true,
'log-level': 'warn',
'moonshot-demo': true,
test1: 's1',
Expand Down
19 changes: 19 additions & 0 deletions packages/shared/sdk-client/__tests__/evaluation/mockResponse.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,24 @@
"value": true,
"variation": 0,
"trackEvents": false
},
"is-prereq": {
"value": true,
"variation": 0,
"reason": {
"kind": "FALLTHROUGH"
},
"version": 1,
"trackEvents": true
},
"has-prereq-depth-1": {
"value": true,
"variation": 0,
"prerequisites": ["is-prereq"],
"reason": {
"kind": "FALLTHROUGH"
},
"version": 4,
"trackEvents": true
}
}
20 changes: 18 additions & 2 deletions packages/shared/sdk-client/src/LDClientImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export default class LDClientImpl implements LDClient {
return createErrorEvaluationDetail(ErrorKinds.FlagNotFound, defaultValue);
}

const { reason, value, variation } = foundItem.flag;
const { reason, value, variation, prerequisites } = foundItem.flag;

if (typeChecker) {
const [matched, type] = typeChecker(value);
Expand All @@ -324,11 +324,27 @@ export default class LDClientImpl implements LDClient {
}
}

const successDetail = createSuccessEvaluationDetail(value, variation, reason);
const successDetail = createSuccessEvaluationDetail(value, variation, reason, prerequisites);
if (value === undefined || value === null) {
this.logger.debug('Result value is null. Providing default value.');
successDetail.value = defaultValue;
}

successDetail.prerequisites?.forEach((prereqKey) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Send prerequisite events in order before the event for the variation. This matches server behavior.

const prereqFlag = this.flagManager.get(prereqKey);
if (prereqFlag) {
this.eventProcessor?.sendEvent(
eventFactory.evalEventClient(
prereqKey,
prereqFlag.flag.value,
undefined,
prereqFlag.flag,
evalContext,
prereqFlag.flag.reason,
),
);
}
});
this.eventProcessor?.sendEvent(
eventFactory.evalEventClient(
flagKey,
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/sdk-client/src/evaluation/evaluationDetail.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ export function createSuccessEvaluationDetail(
value: LDFlagValue,
variationIndex?: number,
reason?: LDEvaluationReason,
prerequisites?: string[],
): LDEvaluationDetail {
return {
value,
variationIndex: variationIndex ?? null,
reason: reason ?? null,
prerequisites,
};
}
1 change: 1 addition & 0 deletions packages/shared/sdk-client/src/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface Flag {
reason?: LDEvaluationReason;
debugEventsUntilDate?: number;
deleted?: boolean;
prerequisites?: string[];
}

export interface PatchFlag extends Flag {
Expand Down
54 changes: 54 additions & 0 deletions packages/shared/sdk-server/__tests__/LDClient.allFlags.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,60 @@ describe('given an LDClient with test data', () => {
done();
});
});

it('includes prerequisites in flag meta', async () => {
await td.update(td.flag('is-prereq').valueForAll(true));
await td.usePreconfiguredFlag({
key: 'has-prereq-depth-1',
on: true,
prerequisites: [
{
key: 'is-prereq',
variation: 0,
},
],
fallthrough: {
variation: 0,
},
offVariation: 1,
variations: [true, false],
clientSideAvailability: {
usingMobileKey: true,
usingEnvironmentId: true,
},
clientSide: true,
version: 4,
});

const state = await client.allFlagsState(defaultUser, {
withReasons: true,
detailsOnlyForTrackedFlags: false,
});
expect(state.valid).toEqual(true);
expect(state.allValues()).toEqual({ 'is-prereq': true, 'has-prereq-depth-1': true });
expect(state.toJSON()).toEqual({
'is-prereq': true,
'has-prereq-depth-1': true,
$flagsState: {
'is-prereq': {
variation: 0,
reason: {
kind: 'FALLTHROUGH',
},
version: 1,
},
'has-prereq-depth-1': {
variation: 0,
prerequisites: ['is-prereq'],
reason: {
kind: 'FALLTHROUGH',
},
version: 4,
},
},
$valid: true,
});
});
});

describe('given an offline client', () => {
Expand Down
69 changes: 69 additions & 0 deletions packages/shared/sdk-server/__tests__/LDClient.evaluation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,75 @@ describe('given an LDClient with test data', () => {
expect(res.reason.kind).toEqual('ERROR');
expect(res.reason.errorKind).toEqual('WRONG_TYPE');
});

it('includes prerequisite information for flags with prerequisites', async () => {
await td.update(td.flag('is-prereq').valueForAll(true));
await td.usePreconfiguredFlag({
key: 'has-prereq-depth-1',
on: true,
prerequisites: [
{
key: 'is-prereq',
variation: 0,
},
],
fallthrough: {
variation: 0,
},
offVariation: 1,
variations: [true, false],
clientSideAvailability: {
usingMobileKey: true,
usingEnvironmentId: true,
},
clientSide: true,
version: 4,
});

const res = await client.variationDetail('has-prereq-depth-1', defaultUser, false);
expect(res.value).toEqual(true);
expect(res.reason.kind).toEqual('FALLTHROUGH');
expect(res.prerequisites).toEqual(['is-prereq']);
});

it.each([
['boolVariationDetail', true, false],
['numberVariationDetail', 42, 3.14],
['stringVariationDetail', 'value', 'default'],
['jsonVariationDetail', { value: 'value' }, { value: 'default' }],
])(
'includes prerequisite information for typed evals',
async (method: string, value: any, def: any) => {
await td.update(td.flag('is-prereq').valueForAll(true));
await td.usePreconfiguredFlag({
key: 'has-prereq-depth-1',
on: true,
prerequisites: [
{
key: 'is-prereq',
variation: 0,
},
],
fallthrough: {
variation: 0,
},
offVariation: 1,
variations: [value, def],
clientSideAvailability: {
usingMobileKey: true,
usingEnvironmentId: true,
},
clientSide: true,
version: 4,
});

// @ts-ignore Typescript cannot infer the matching method types.
const res = await client[method]('has-prereq-depth-1', defaultUser, def);
expect(res.value).toEqual(value);
expect(res.reason.kind).toEqual('FALLTHROUGH');
expect(res.prerequisites).toEqual(['is-prereq']);
},
);
});

describe('given an offline client', () => {
Expand Down
Loading
Loading