Skip to content

Commit

Permalink
add test, add return types, move a util func
Browse files Browse the repository at this point in the history
  • Loading branch information
zhukaihan committed Jul 25, 2024
1 parent ffc30a6 commit b60bb56
Show file tree
Hide file tree
Showing 6 changed files with 373 additions and 211 deletions.
29 changes: 10 additions & 19 deletions packages/node/src/local/updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,11 @@ export class FlagConfigUpdaterBase {
}
}

private async downloadNewCohorts(
protected async downloadNewCohorts(
cohortIds: Set<string>,
): Promise<Set<string>> {
const oldCohortIds = this.cohortStorage?.getAllCohortIds();
const newCohortIds = FlagConfigUpdaterBase.setSubtract(
cohortIds,
oldCohortIds,
);
const newCohortIds = CohortUtils.setSubtract(cohortIds, oldCohortIds);
const failedCohortIds = new Set<string>();
const cohortDownloadPromises = [...newCohortIds].map((cohortId) =>
this.cohortFetcher
Expand All @@ -126,10 +123,11 @@ export class FlagConfigUpdaterBase {
return failedCohortIds;
}

private async filterFlagConfigsWithFullCohorts(
protected async filterFlagConfigsWithFullCohorts(
flagConfigs: Record<string, FlagConfig>,
) {
): Promise<Record<string, FlagConfig>> {
const newFlagConfigs = {};
const availableCohortIds = this.cohortStorage.getAllCohortIds();
for (const flagKey in flagConfigs) {
// Get cohorts for this flag.
const cohortIds = CohortUtils.extractCohortIdsFromFlag(
Expand All @@ -140,9 +138,7 @@ export class FlagConfigUpdaterBase {
// If any cohort failed, don't use the new flag.
const updateFlag =
cohortIds.size === 0 ||
[...cohortIds]
.map((id) => this.cohortStorage.getCohort(id))
.reduce((acc, cur) => acc && cur);
CohortUtils.setSubtract(cohortIds, availableCohortIds).size === 0;

if (updateFlag) {
newFlagConfigs[flagKey] = flagConfigs[flagKey];
Expand All @@ -160,20 +156,15 @@ export class FlagConfigUpdaterBase {
return newFlagConfigs;
}

private async removeUnusedCohorts(validCohortIds: Set<string>) {
const cohortIdsToBeRemoved = FlagConfigUpdaterBase.setSubtract(
protected async removeUnusedCohorts(
validCohortIds: Set<string>,
): Promise<void> {
const cohortIdsToBeRemoved = CohortUtils.setSubtract(
this.cohortStorage.getAllCohortIds(),
validCohortIds,
);
cohortIdsToBeRemoved.forEach((id) => {
this.cohortStorage.delete(id);
});
}

private static setSubtract(one: Set<string>, other: Set<string>) {
const result = new Set<string>(one);
other.forEach((v) => result.delete(v));

return result;
}
}
15 changes: 12 additions & 3 deletions packages/node/src/util/cohort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ export class CohortUtils {
return cohortIdsByGroup;
}

private static mergeBIntoA(
public static mergeBIntoA(
a: Record<string, Set<string>>,
b: Record<string, Set<string>>,
) {
): void {
for (const groupType in b) {
if (!(groupType in a)) {
a[groupType] = new Set<string>();
Expand All @@ -98,11 +98,20 @@ export class CohortUtils {
}
}

private static mergeAllValues(a: Record<string, Set<string>>) {
public static mergeAllValues(a: Record<string, Set<string>>): Set<string> {
const merged = new Set<string>();
for (const key in a) {
a[key].forEach(merged.add, merged);
}
return merged;
}

public static setSubtract(one: Set<string>, other: Set<string>): Set<string> {
const result = new Set<string>();
one.forEach((v) => {
if (!other.has(v)) result.add(v);
});

return result;
}
}
184 changes: 10 additions & 174 deletions packages/node/test/local/flagConfigPoller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,173 +7,9 @@ import { SdkCohortApi } from 'src/local/cohort/cohort-api';
import { CohortFetcher } from 'src/local/cohort/fetcher';
import { InMemoryCohortStorage } from 'src/local/cohort/storage';

import { FLAGS, NEW_FLAGS } from './util/flags';
import { MockHttpClient } from './util/mockHttpClient';

const FLAG = [
{
key: 'flag1',
segments: [
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'user', 'cohort_ids'],
values: ['hahahaha1'],
},
],
],
},
{
metadata: {
segmentName: 'All Other Users',
},
variant: 'off',
},
],
variants: {},
},
{
key: 'flag2',
segments: [
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'user', 'cohort_ids'],
values: ['hahahaha2'],
},
],
],
metadata: {
segmentName: 'Segment 1',
},
variant: 'off',
},
{
metadata: {
segmentName: 'All Other Users',
},
variant: 'off',
},
],
variants: {},
},
{
key: 'flag3',
metadata: {
deployed: true,
evaluationMode: 'local',
experimentKey: 'exp-1',
flagType: 'experiment',
flagVersion: 6,
},
segments: [
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'user', 'cohort_ids'],
values: ['hahahaha3'],
},
],
],
variant: 'off',
},
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'user', 'cocoids'],
values: ['nohaha'],
},
],
],
variant: 'off',
},
{
metadata: {
segmentName: 'All Other Users',
},
variant: 'off',
},
],
variants: {},
},
{
key: 'flag5',
segments: [
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'user', 'cohort_ids'],
values: ['hahahaha3', 'hahahaha4'],
},
],
],
},
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'groups', 'org name', 'cohort_ids'],
values: ['hahaorgname1'],
},
],
],
metadata: {
segmentName: 'Segment 1',
},
},
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'gg', 'org name', 'cohort_ids'],
values: ['nohahaorgname'],
},
],
],
metadata: {
segmentName: 'Segment 1',
},
},
],
variants: {},
},
].reduce((acc, flag) => {
acc[flag.key] = flag;
return acc;
}, {});

const NEW_FLAGS = {
...FLAG,
flag6: {
key: 'flag6',
segments: [
{
conditions: [
[
{
op: 'set contains any',
selector: ['context', 'user', 'cohort_ids'],
values: ['anewcohortid'],
},
],
],
},
],
variants: {},
},
};

afterEach(() => {
// Note that if a test failed, and the poller has not stopped,
// the test will hang and this won't be called.
Expand Down Expand Up @@ -203,7 +39,7 @@ test('flagConfig poller success', async () => {
.spyOn(FlagConfigFetcher.prototype, 'fetch')
.mockImplementation(async () => {
++flagPolled;
if (flagPolled == 1) return { ...FLAG, flagPolled: { key: flagPolled } };
if (flagPolled == 1) return { ...FLAGS, flagPolled: { key: flagPolled } };
return { ...NEW_FLAGS, flagPolled: { key: flagPolled } };
});
// Return cohort with their own cohortId.
Expand All @@ -224,7 +60,7 @@ test('flagConfig poller success', async () => {
await poller.start();
expect(flagPolled).toBe(1);
expect(await poller.cache.getAll()).toStrictEqual({
...FLAG,
...FLAGS,
flagPolled: { key: flagPolled },
});
expect(cohortStorage.getCohort('hahahaha1').cohortId).toBe('hahahaha1');
Expand Down Expand Up @@ -276,18 +112,18 @@ test('flagConfig poller initial error', async () => {
),
10,
);
// Fetch returns FLAG, but cohort fails.
// Fetch returns FLAGS, but cohort fails.
jest
.spyOn(FlagConfigFetcher.prototype, 'fetch')
.mockImplementation(async () => {
return FLAG;
return FLAGS;
});
jest
.spyOn(SdkCohortApi.prototype, 'getCohort')
.mockImplementation(async () => {
throw new Error();
});
// FLAG should be empty, as cohort failed. Poller should be stopped immediately and test exists cleanly.
// FLAGS should be empty, as cohort failed. Poller should be stopped immediately and test exists cleanly.
try {
// Should throw when init failed.
await poller.start();
Expand Down Expand Up @@ -318,7 +154,7 @@ test('flagConfig poller initial success, polling error and use old flags', async
jest
.spyOn(FlagConfigFetcher.prototype, 'fetch')
.mockImplementation(async () => {
if (++flagPolled === 1) return FLAG;
if (++flagPolled === 1) return FLAGS;
return NEW_FLAGS;
});
// Only success on first poll and fail on all later ones.
Expand All @@ -339,16 +175,16 @@ test('flagConfig poller initial success, polling error and use old flags', async
throw new Error();
});

// First poll should return FLAG.
// First poll should return FLAGS.
await poller.start();
expect(await poller.cache.getAll()).toStrictEqual(FLAG);
expect(await poller.cache.getAll()).toStrictEqual(FLAGS);
expect(flagPolled).toBe(1);

// Second poll flags with new cohort should fail when new cohort download failed.
// The different flag should not be updated.
await new Promise((f) => setTimeout(f, 2000));
expect(flagPolled).toBeGreaterThanOrEqual(2);
expect(await poller.cache.getAll()).toStrictEqual(FLAG);
expect(await poller.cache.getAll()).toStrictEqual(FLAGS);

poller.stop();
});
Loading

0 comments on commit b60bb56

Please sign in to comment.