-
Notifications
You must be signed in to change notification settings - Fork 1
feat: inspect format field to determine whether config is obfuscated #218
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
Changes from 3 commits
cb062eb
8aee608
e01f3b3
5ebe36e
f067c3b
04d8a43
0786d71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,12 +4,12 @@ import * as td from 'testdouble'; | |
|
||
import { | ||
ASSIGNMENT_TEST_DATA_DIR, | ||
getTestAssignments, | ||
IAssignmentTestCase, | ||
MOCK_UFC_RESPONSE_FILE, | ||
OBFUSCATED_MOCK_UFC_RESPONSE_FILE, | ||
SubjectTestCase, | ||
getTestAssignments, | ||
readMockUFCResponse, | ||
SubjectTestCase, | ||
testCasesByFileName, | ||
validateTestAssignments, | ||
} from '../../test/testHelpers'; | ||
|
@@ -21,13 +21,13 @@ import { | |
} from '../configuration'; | ||
import { IConfigurationStore } from '../configuration-store/configuration-store'; | ||
import { MemoryOnlyConfigurationStore } from '../configuration-store/memory.store'; | ||
import { MAX_EVENT_QUEUE_SIZE, DEFAULT_POLL_INTERVAL_MS, POLL_JITTER_PCT } from '../constants'; | ||
import { DEFAULT_POLL_INTERVAL_MS, MAX_EVENT_QUEUE_SIZE, POLL_JITTER_PCT } from '../constants'; | ||
import { decodePrecomputedFlag } from '../decoding'; | ||
import { Flag, ObfuscatedFlag, VariationType } from '../interfaces'; | ||
import { Flag, FormatEnum, ObfuscatedFlag, VariationType } from '../interfaces'; | ||
import { getMD5Hash } from '../obfuscation'; | ||
import { AttributeType } from '../types'; | ||
|
||
import EppoClient, { FlagConfigurationRequestParameters, checkTypeMatch } from './eppo-client'; | ||
import EppoClient, { checkTypeMatch, FlagConfigurationRequestParameters } from './eppo-client'; | ||
import { initConfiguration } from './test-utils'; | ||
|
||
// Use a known salt to produce deterministic hashes | ||
|
@@ -45,6 +45,18 @@ describe('EppoClient E2E test', () => { | |
}) as jest.Mock; | ||
const storage = new MemoryOnlyConfigurationStore<Flag | ObfuscatedFlag>(); | ||
|
||
/** | ||
* Use this helper instead of directly setting entries on the `storage` ConfigurationStore. | ||
* This method ensures the format field is set as it is required for parsing. | ||
* @param entries | ||
*/ | ||
function setUnobfuscatedFlagEntries( | ||
typotter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
entries: Record<string, Flag | ObfuscatedFlag>, | ||
): Promise<boolean> { | ||
storage.setFormat(FormatEnum.SERVER); | ||
return storage.setEntries(entries); | ||
} | ||
|
||
beforeAll(async () => { | ||
await initConfiguration(storage); | ||
}); | ||
|
@@ -88,7 +100,7 @@ describe('EppoClient E2E test', () => { | |
let client: EppoClient; | ||
|
||
beforeAll(() => { | ||
storage.setEntries({ [flagKey]: mockFlag }); | ||
setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
client = new EppoClient({ flagConfigurationStore: storage }); | ||
|
||
td.replace(EppoClient.prototype, 'getAssignmentDetail', function () { | ||
|
@@ -144,7 +156,7 @@ describe('EppoClient E2E test', () => { | |
|
||
describe('setLogger', () => { | ||
beforeAll(() => { | ||
storage.setEntries({ [flagKey]: mockFlag }); | ||
setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
}); | ||
|
||
it('Invokes logger for queued events', () => { | ||
|
@@ -192,7 +204,7 @@ describe('EppoClient E2E test', () => { | |
|
||
describe('precomputed flags', () => { | ||
beforeAll(() => { | ||
storage.setEntries({ | ||
setUnobfuscatedFlagEntries({ | ||
[flagKey]: mockFlag, | ||
disabledFlag: { ...mockFlag, enabled: false }, | ||
anotherFlag: { | ||
|
@@ -424,10 +436,10 @@ describe('EppoClient E2E test', () => { | |
); | ||
}); | ||
|
||
it('logs variation assignment and experiment key', () => { | ||
it('logs variation assignment and experiment key', async () => { | ||
const mockLogger = td.object<IAssignmentLogger>(); | ||
|
||
storage.setEntries({ [flagKey]: mockFlag }); | ||
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setAssignmentLogger(mockLogger); | ||
|
||
|
@@ -449,11 +461,11 @@ describe('EppoClient E2E test', () => { | |
expect(loggedAssignmentEvent.allocation).toEqual(mockFlag.allocations[0].key); | ||
}); | ||
|
||
it('handles logging exception', () => { | ||
it('handles logging exception', async () => { | ||
const mockLogger = td.object<IAssignmentLogger>(); | ||
td.when(mockLogger.logAssignment(td.matchers.anything())).thenThrow(new Error('logging error')); | ||
|
||
storage.setEntries({ [flagKey]: mockFlag }); | ||
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setAssignmentLogger(mockLogger); | ||
|
||
|
@@ -468,8 +480,8 @@ describe('EppoClient E2E test', () => { | |
expect(assignment).toEqual('variation-a'); | ||
}); | ||
|
||
it('exports flag configuration', () => { | ||
storage.setEntries({ [flagKey]: mockFlag }); | ||
it('exports flag configuration', async () => { | ||
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
expect(client.getFlagConfigurations()).toEqual({ [flagKey]: mockFlag }); | ||
}); | ||
|
@@ -478,10 +490,10 @@ describe('EppoClient E2E test', () => { | |
let client: EppoClient; | ||
let mockLogger: IAssignmentLogger; | ||
|
||
beforeEach(() => { | ||
beforeEach(async () => { | ||
mockLogger = td.object<IAssignmentLogger>(); | ||
|
||
storage.setEntries({ [flagKey]: mockFlag }); | ||
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setAssignmentLogger(mockLogger); | ||
}); | ||
|
@@ -536,7 +548,7 @@ describe('EppoClient E2E test', () => { | |
}); | ||
|
||
it('logs for each unique flag', async () => { | ||
await storage.setEntries({ | ||
await setUnobfuscatedFlagEntries({ | ||
[flagKey]: mockFlag, | ||
'flag-2': { | ||
...mockFlag, | ||
|
@@ -563,10 +575,10 @@ describe('EppoClient E2E test', () => { | |
expect(td.explain(mockLogger.logAssignment).callCount).toEqual(3); | ||
}); | ||
|
||
it('logs twice for the same flag when allocations change', () => { | ||
it('logs twice for the same flag when allocations change', async () => { | ||
client.useNonExpiringInMemoryAssignmentCache(); | ||
|
||
storage.setEntries({ | ||
setUnobfuscatedFlagEntries({ | ||
[flagKey]: { | ||
...mockFlag, | ||
|
||
|
@@ -587,7 +599,7 @@ describe('EppoClient E2E test', () => { | |
}); | ||
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); | ||
|
||
storage.setEntries({ | ||
await setUnobfuscatedFlagEntries({ | ||
[flagKey]: { | ||
...mockFlag, | ||
allocations: [ | ||
|
@@ -613,13 +625,13 @@ describe('EppoClient E2E test', () => { | |
client.useNonExpiringInMemoryAssignmentCache(); | ||
|
||
// original configuration version | ||
storage.setEntries({ [flagKey]: mockFlag }); | ||
setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
|
||
|
||
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // log this assignment | ||
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // cache hit, don't log | ||
|
||
// change the variation | ||
storage.setEntries({ | ||
setUnobfuscatedFlagEntries({ | ||
[flagKey]: { | ||
...mockFlag, | ||
allocations: [ | ||
|
@@ -642,13 +654,13 @@ describe('EppoClient E2E test', () => { | |
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // cache hit, don't log | ||
|
||
// change the flag again, back to the original | ||
storage.setEntries({ [flagKey]: mockFlag }); | ||
setUnobfuscatedFlagEntries({ [flagKey]: mockFlag }); | ||
|
||
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // important: log this assignment | ||
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // cache hit, don't log | ||
|
||
// change the allocation | ||
storage.setEntries({ | ||
setUnobfuscatedFlagEntries({ | ||
[flagKey]: { | ||
...mockFlag, | ||
allocations: [ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,8 @@ import { LRUInMemoryAssignmentCache } from '../cache/lru-in-memory-assignment-ca | |
import { NonExpiringInMemoryAssignmentCache } from '../cache/non-expiring-in-memory-cache-assignment'; | ||
import { TLRUInMemoryAssignmentCache } from '../cache/tlru-in-memory-assignment-cache'; | ||
import { | ||
IConfigurationWire, | ||
ConfigurationWireV1, | ||
IConfigurationWire, | ||
IPrecomputedConfiguration, | ||
PrecomputedConfiguration, | ||
} from '../configuration'; | ||
|
@@ -27,6 +27,7 @@ import { | |
DEFAULT_POLL_CONFIG_REQUEST_RETRIES, | ||
DEFAULT_POLL_INTERVAL_MS, | ||
DEFAULT_REQUEST_TIMEOUT_MS, | ||
OBFUSCATED_FORMATS, | ||
} from '../constants'; | ||
import { decodeFlag } from '../decoding'; | ||
import { EppoValue } from '../eppo_value'; | ||
|
@@ -46,6 +47,7 @@ import { | |
BanditVariation, | ||
ConfigDetails, | ||
Flag, | ||
getFormatFromString, | ||
IPrecomputedBandit, | ||
ObfuscatedFlag, | ||
PrecomputedFlag, | ||
|
@@ -101,6 +103,9 @@ export type EppoClientParameters = { | |
banditVariationConfigurationStore?: IConfigurationStore<BanditVariation[]>; | ||
banditModelConfigurationStore?: IConfigurationStore<BanditParameters>; | ||
configurationRequestParameters?: FlagConfigurationRequestParameters; | ||
/** | ||
* @deprecated obfuscation is determined by inspecting the `format` field of the UFC response. | ||
*/ | ||
typotter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
isObfuscated?: boolean; | ||
}; | ||
|
||
|
@@ -121,7 +126,7 @@ export default class EppoClient { | |
private assignmentCache?: AssignmentCache; | ||
// whether to suppress any errors and return default values instead | ||
private isGracefulFailureMode = true; | ||
private isObfuscated: boolean; | ||
private expectObfuscated: boolean; | ||
private requestPoller?: IPoller; | ||
private readonly evaluator = new Evaluator(); | ||
|
||
|
@@ -138,7 +143,18 @@ export default class EppoClient { | |
this.banditVariationConfigurationStore = banditVariationConfigurationStore; | ||
this.banditModelConfigurationStore = banditModelConfigurationStore; | ||
this.configurationRequestParameters = configurationRequestParameters; | ||
this.isObfuscated = isObfuscated; | ||
this.expectObfuscated = isObfuscated; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐛 This looks like it sets |
||
} | ||
|
||
private isObfuscated() { | ||
const configFormat = getFormatFromString(this.flagConfigurationStore.getFormat()); | ||
const configObfuscated = OBFUSCATED_FORMATS.includes(configFormat); | ||
|
||
if (configObfuscated !== this.expectObfuscated) { | ||
logger.warn( | ||
`[Eppo SDK] configuration obfuscation [${configObfuscated}] does not match expected [${this.expectObfuscated}]`, | ||
); | ||
} | ||
return configObfuscated; | ||
} | ||
|
||
setConfigurationRequestParameters( | ||
|
@@ -188,8 +204,12 @@ export default class EppoClient { | |
} | ||
|
||
// noinspection JSUnusedGlobalSymbols | ||
/** | ||
* @deprecated The client determines whether the configuration is obfuscated by inspection | ||
* @param isObfuscated | ||
typotter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
setIsObfuscated(isObfuscated: boolean) { | ||
this.isObfuscated = isObfuscated; | ||
this.expectObfuscated = isObfuscated; | ||
} | ||
|
||
async fetchFlagConfigurations() { | ||
|
@@ -854,7 +874,7 @@ export default class EppoClient { | |
configDetails, | ||
subjectKey, | ||
subjectAttributes, | ||
this.isObfuscated, | ||
this.isObfuscated(), | ||
); | ||
|
||
// allocationKey is set along with variation when there is a result. this check appeases typescript below | ||
|
@@ -993,15 +1013,16 @@ export default class EppoClient { | |
); | ||
} | ||
|
||
const isObfuscated = this.isObfuscated(); | ||
const result = this.evaluator.evaluateFlag( | ||
flag, | ||
configDetails, | ||
subjectKey, | ||
subjectAttributes, | ||
this.isObfuscated, | ||
isObfuscated, | ||
expectedVariationType, | ||
); | ||
if (this.isObfuscated) { | ||
if (isObfuscated) { | ||
// flag.key is obfuscated, replace with requested flag key | ||
result.flagKey = flagKey; | ||
} | ||
|
@@ -1052,7 +1073,7 @@ export default class EppoClient { | |
} | ||
|
||
private getFlag(flagKey: string): Flag | null { | ||
return this.isObfuscated | ||
return this.isObfuscated() | ||
typotter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
? this.getObfuscatedFlag(flagKey) | ||
: this.flagConfigurationStore.get(flagKey); | ||
} | ||
|
@@ -1220,7 +1241,7 @@ export default class EppoClient { | |
|
||
private buildLoggerMetadata(): Record<string, unknown> { | ||
return { | ||
obfuscated: this.isObfuscated, | ||
obfuscated: this.isObfuscated(), | ||
sdkLanguage: 'javascript', | ||
sdkLibVersion: LIB_VERSION, | ||
}; | ||
|
Uh oh!
There was an error while loading. Please reload this page.