-
Notifications
You must be signed in to change notification settings - Fork 6
[WIP] Fallback treatment #442
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
850b074
Implement FallbackSanitizer and add fallback to config
ZamoraEmmanuel 7371aad
Merge branch 'development' into fme-10504
ZamoraEmmanuel e39f22b
Merge pull request #437 from splitio/fme-10504
ZamoraEmmanuel 1a05cee
[FME-10566] Create FallbackTreatmentsCalculator
ZamoraEmmanuel 381b458
Merge pull request #441 from splitio/fme-10566
ZamoraEmmanuel 8911132
[FME-10567] Add fallbackTreatmentCalculator to client
ZamoraEmmanuel d9c4cff
Update src/sdkClient/clientInputValidation.ts
ZamoraEmmanuel c88d787
Merge pull request #444 from splitio/fme-10567-refactor
ZamoraEmmanuel c60c4ce
review changes and add fallbacklabel to avoid impression
ZamoraEmmanuel eeb8073
Prepare release v2.8.0
ZamoraEmmanuel 6cf13c9
remove unnecessary validation
ZamoraEmmanuel e52c45e
Merge pull request #446 from splitio/review-changes
ZamoraEmmanuel 2b3fac0
Merge branch 'development' into fallback-treatment
EmilianoSanchez dcbef71
Merge branch 'fallback-treatment' into prepare-release
EmilianoSanchez ad8d66a
rc
EmilianoSanchez d58f162
Merge branch 'development' into fallback-treatment
EmilianoSanchez 79df832
Merge branch 'fallback-treatment' into prepare-release
EmilianoSanchez f4145a9
stable version
EmilianoSanchez bd5abe3
Merge pull request #447 from splitio/prepare-release
ZamoraEmmanuel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
66 changes: 66 additions & 0 deletions
66
src/evaluator/fallbackTreatmentsCalculator/__tests__/fallback-calculator.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import { FallbackTreatmentsCalculator } from '../'; | ||
| import type { FallbackTreatmentConfiguration } from '../../../../types/splitio'; // adjust path if needed | ||
|
|
||
| describe('FallbackTreatmentsCalculator', () => { | ||
|
|
||
| test('returns specific fallback if flag exists', () => { | ||
| const config: FallbackTreatmentConfiguration = { | ||
| byFlag: { | ||
| 'featureA': { treatment: 'TREATMENT_A', config: '{ value: 1 }' }, | ||
| }, | ||
| }; | ||
| const calculator = new FallbackTreatmentsCalculator(config); | ||
| const result = calculator.resolve('featureA', 'label by flag'); | ||
|
|
||
| expect(result).toEqual({ | ||
| treatment: 'TREATMENT_A', | ||
| config: '{ value: 1 }', | ||
| label: 'fallback - label by flag', | ||
| }); | ||
| }); | ||
|
|
||
| test('returns global fallback if flag is missing and global exists', () => { | ||
| const config: FallbackTreatmentConfiguration = { | ||
| byFlag: {}, | ||
| global: { treatment: 'GLOBAL_TREATMENT', config: '{ global: true }' }, | ||
| }; | ||
| const calculator = new FallbackTreatmentsCalculator(config); | ||
| const result = calculator.resolve('missingFlag', 'label by global'); | ||
|
|
||
| expect(result).toEqual({ | ||
| treatment: 'GLOBAL_TREATMENT', | ||
| config: '{ global: true }', | ||
| label: 'fallback - label by global', | ||
| }); | ||
| }); | ||
|
|
||
| test('returns control fallback if flag and global are missing', () => { | ||
| const config: FallbackTreatmentConfiguration = { | ||
| byFlag: {}, | ||
| }; | ||
| const calculator = new FallbackTreatmentsCalculator(config); | ||
| const result = calculator.resolve('missingFlag', 'label by noFallback'); | ||
|
|
||
| expect(result).toEqual({ | ||
| treatment: 'CONTROL', | ||
| config: null, | ||
| label: 'fallback - label by noFallback', | ||
| }); | ||
| }); | ||
|
|
||
| test('returns undefined label if no label provided', () => { | ||
| const config: FallbackTreatmentConfiguration = { | ||
| byFlag: { | ||
| 'featureB': { treatment: 'TREATMENT_B' }, | ||
| }, | ||
| }; | ||
| const calculator = new FallbackTreatmentsCalculator(config); | ||
| const result = calculator.resolve('featureB'); | ||
|
|
||
| expect(result).toEqual({ | ||
| treatment: 'TREATMENT_B', | ||
| config: undefined, | ||
| label: undefined, | ||
| }); | ||
| }); | ||
| }); |
103 changes: 103 additions & 0 deletions
103
src/evaluator/fallbackTreatmentsCalculator/__tests__/fallback-sanitizer.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import { FallbacksSanitizer } from '../fallbackSanitizer'; | ||
| import { TreatmentWithConfig } from '../../../../types/splitio'; | ||
|
|
||
| describe('FallbacksSanitizer', () => { | ||
| const validTreatment: TreatmentWithConfig = { treatment: 'on', config: '{"color":"blue"}' }; | ||
| const invalidTreatment: TreatmentWithConfig = { treatment: ' ', config: null }; | ||
|
|
||
| beforeEach(() => { | ||
| jest.spyOn(console, 'error').mockImplementation(() => {}); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| (console.error as jest.Mock).mockRestore(); | ||
| }); | ||
|
|
||
| describe('isValidFlagName', () => { | ||
| test('returns true for a valid flag name', () => { | ||
| // @ts-expect-private-access | ||
| expect((FallbacksSanitizer as any).isValidFlagName('my_flag')).toBe(true); | ||
| }); | ||
|
|
||
| test('returns false for a name longer than 100 chars', () => { | ||
| const longName = 'a'.repeat(101); | ||
| expect((FallbacksSanitizer as any).isValidFlagName(longName)).toBe(false); | ||
| }); | ||
|
|
||
| test('returns false if the name contains spaces', () => { | ||
| expect((FallbacksSanitizer as any).isValidFlagName('invalid flag')).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isValidTreatment', () => { | ||
| test('returns true for a valid treatment string', () => { | ||
| expect((FallbacksSanitizer as any).isValidTreatment(validTreatment)).toBe(true); | ||
| }); | ||
|
|
||
| test('returns false for null or undefined', () => { | ||
| expect((FallbacksSanitizer as any).isValidTreatment(null)).toBe(false); | ||
| expect((FallbacksSanitizer as any).isValidTreatment(undefined)).toBe(false); | ||
| }); | ||
|
|
||
| test('returns false for a treatment longer than 100 chars', () => { | ||
| const long = { treatment: 'a'.repeat(101) }; | ||
| expect((FallbacksSanitizer as any).isValidTreatment(long)).toBe(false); | ||
| }); | ||
|
|
||
| test('returns false if treatment does not match regex pattern', () => { | ||
| const invalid = { treatment: 'invalid treatment!' }; | ||
| expect((FallbacksSanitizer as any).isValidTreatment(invalid)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('sanitizeGlobal', () => { | ||
| test('returns the treatment if valid', () => { | ||
| expect(FallbacksSanitizer.sanitizeGlobal(validTreatment)).toEqual(validTreatment); | ||
| expect(console.error).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('returns undefined and logs error if invalid', () => { | ||
| const result = FallbacksSanitizer.sanitizeGlobal(invalidTreatment); | ||
| expect(result).toBeUndefined(); | ||
| expect(console.error).toHaveBeenCalledWith( | ||
| expect.stringContaining('Fallback treatments - Discarded fallback') | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('sanitizeByFlag', () => { | ||
| test('returns a sanitized map with valid entries only', () => { | ||
| const input = { | ||
| valid_flag: validTreatment, | ||
| 'invalid flag': validTreatment, | ||
| bad_treatment: invalidTreatment, | ||
| }; | ||
|
|
||
| const result = FallbacksSanitizer.sanitizeByFlag(input); | ||
|
|
||
| expect(result).toEqual({ valid_flag: validTreatment }); | ||
| expect(console.error).toHaveBeenCalledTimes(2); // invalid flag + bad_treatment | ||
| }); | ||
|
|
||
| test('returns empty object if all invalid', () => { | ||
| const input = { | ||
| 'invalid flag': invalidTreatment, | ||
| }; | ||
|
|
||
| const result = FallbacksSanitizer.sanitizeByFlag(input); | ||
| expect(result).toEqual({}); | ||
| expect(console.error).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('returns same object if all valid', () => { | ||
| const input = { | ||
| flag_one: validTreatment, | ||
| flag_two: { treatment: 'valid_2', config: null }, | ||
| }; | ||
|
|
||
| const result = FallbacksSanitizer.sanitizeByFlag(input); | ||
| expect(result).toEqual(input); | ||
| expect(console.error).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export enum FallbackDiscardReason { | ||
| FlagName = 'Invalid flag name (max 100 chars, no spaces)', | ||
| Treatment = 'Invalid treatment (max 100 chars and must match pattern)', | ||
| } |
68 changes: 68 additions & 0 deletions
68
src/evaluator/fallbackTreatmentsCalculator/fallbackSanitizer/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { FallbackTreatment } from '../../../../types/splitio'; | ||
| import { FallbackDiscardReason } from '../constants'; | ||
|
|
||
|
|
||
| export class FallbacksSanitizer { | ||
|
|
||
| private static readonly pattern = /^[0-9]+[.a-zA-Z0-9_-]*$|^[a-zA-Z]+[a-zA-Z0-9_-]*$/; | ||
|
|
||
| private static isValidFlagName(name: string): boolean { | ||
| return name.length <= 100 && !name.includes(' '); | ||
| } | ||
|
|
||
| private static isValidTreatment(t?: FallbackTreatment): boolean { | ||
| if (!t) { | ||
| return false; | ||
| } | ||
|
|
||
| if (typeof t === 'string') { | ||
| if (t.length > 100) { | ||
| return false; | ||
| } | ||
| return FallbacksSanitizer.pattern.test(t); | ||
| } | ||
|
|
||
| const { treatment } = t; | ||
| if (!treatment || treatment.length > 100) { | ||
| return false; | ||
| } | ||
| return FallbacksSanitizer.pattern.test(treatment); | ||
| } | ||
|
|
||
| static sanitizeGlobal(treatment?: FallbackTreatment): FallbackTreatment | undefined { | ||
| if (!this.isValidTreatment(treatment)) { | ||
| console.error( | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| `Fallback treatments - Discarded fallback: ${FallbackDiscardReason.Treatment}` | ||
| ); | ||
| return undefined; | ||
| } | ||
| return treatment!; | ||
| } | ||
|
|
||
| static sanitizeByFlag( | ||
| byFlagFallbacks: Record<string, FallbackTreatment> | ||
| ): Record<string, FallbackTreatment> { | ||
| const sanitizedByFlag: Record<string, FallbackTreatment> = {}; | ||
|
|
||
| const entries = Object.entries(byFlagFallbacks); | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| entries.forEach(([flag, t]) => { | ||
| if (!this.isValidFlagName(flag)) { | ||
| console.error( | ||
| `Fallback treatments - Discarded flag '${flag}': ${FallbackDiscardReason.FlagName}` | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (!this.isValidTreatment(t)) { | ||
| console.error( | ||
| `Fallback treatments - Discarded treatment for flag '${flag}': ${FallbackDiscardReason.Treatment}` | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| sanitizedByFlag[flag] = t; | ||
| }); | ||
|
|
||
| return sanitizedByFlag; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { FallbackTreatmentConfiguration, FallbackTreatment, IFallbackTreatmentsCalculator} from '../../../types/splitio'; | ||
|
|
||
| export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculator { | ||
| private readonly labelPrefix = 'fallback - '; | ||
| private readonly control = 'CONTROL'; | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| private readonly fallbacks: FallbackTreatmentConfiguration; | ||
|
|
||
| constructor(fallbacks: FallbackTreatmentConfiguration) { | ||
| this.fallbacks = fallbacks; | ||
| } | ||
|
|
||
| resolve(flagName: string, label?: string | undefined): FallbackTreatment { | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const treatment = this.fallbacks.byFlag[flagName]; | ||
| if (treatment) { | ||
| return this.copyWithLabel(treatment, label); | ||
| } | ||
|
|
||
| if (this.fallbacks.global) { | ||
| return this.copyWithLabel(this.fallbacks.global, label); | ||
| } | ||
|
|
||
| return { | ||
| treatment: this.control, | ||
| config: null, | ||
| label: this.resolveLabel(label), | ||
| }; | ||
| } | ||
|
|
||
| private copyWithLabel(fallback: FallbackTreatment, label: string | undefined): FallbackTreatment { | ||
| if (typeof fallback === 'string') { | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return { | ||
| treatment: fallback, | ||
| config: null, | ||
| label: this.resolveLabel(label), | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }; | ||
| } | ||
|
|
||
| return { | ||
| treatment: fallback.treatment, | ||
| config: fallback.config, | ||
| label: this.resolveLabel(label), | ||
ZamoraEmmanuel marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }; | ||
| } | ||
|
|
||
| private resolveLabel(label?: string | undefined): string | undefined { | ||
| return label ? `${this.labelPrefix}${label}` : undefined; | ||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.