Skip to content

Commit

Permalink
Removed the function, which handled the logic to bound an optional tr…
Browse files Browse the repository at this point in the history
…affic type to SDK clients
  • Loading branch information
EmilianoSanchez committed Oct 8, 2024
1 parent 4c979f7 commit 66210e6
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 201 deletions.
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- Removed the deprecated `GOOGLE_ANALYTICS_TO_SPLIT` and `SPLIT_TO_GOOGLE_ANALYTICS` integrations.
- Removed internal ponyfills for `Map`, `Set` and `Array.from` global objects, dropping support for IE and other outdated browsers. The SDK now requires the runtime environment to support these features natively or to provide a polyfill.
- Removed the migration logic for the old format of MySegments keys in LocalStorage introduced in JavaScript SDK v10.17.3.
- Removed the `sdkClientMethodCSWithTTFactory` function, which handled the logic to bound an optional traffic type to SDK clients. Client-side SDK implementations must use `sdkClientMethodCSWithTT` which, unlike the previous function, does not allow passing a traffic type but simplifies the SDK API.

1.17.0 (September 6, 2024)
- Added `sync.requestOptions.getHeaderOverrides` configuration option to enhance SDK HTTP request Headers for Authorization Frameworks.
Expand Down
69 changes: 23 additions & 46 deletions src/sdkClient/__tests__/sdkClientMethodCS.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { sdkClientMethodCSFactory as sdkClientMethodCSWithTTFactory } from '../sdkClientMethodCSWithTT';
import { sdkClientMethodCSFactory } from '../sdkClientMethodCS';
import { assertClientApi } from './testUtils';
import { telemetryTrackerFactory } from '../../trackers/telemetryTracker';
import { settingsWithKey, settingsWithKeyAndTT, settingsWithKeyObject } from '../../utils/settingsValidation/__tests__/settings.mocks';
import { settingsWithKey, settingsWithKeyObject } from '../../utils/settingsValidation/__tests__/settings.mocks';

const partialStorages: { destroy: jest.Mock }[] = [];

Expand Down Expand Up @@ -75,13 +74,7 @@ describe('sdkClientMethodCSFactory', () => {
params.clients = {};
});

// list of factory functions and their types (whether it ignores TT or not)
const testTargets = [
[sdkClientMethodCSWithTTFactory, false],
[sdkClientMethodCSFactory, true]
];

test.each(testTargets)('main client', (sdkClientMethodCSFactory) => {
test('main client', () => {
// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);

Expand All @@ -106,21 +99,20 @@ describe('sdkClientMethodCSFactory', () => {

});

test.each(testTargets)('multiple clients', async (sdkClientMethodCSFactory, ignoresTT) => {
test('multiple clients', async () => {

// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);

// calling the function with a diferent key than settings, should return a new client instance
// calling the function with a different key than settings, should return a new client instance
const newClients = new Set([
sdkClientMethod('other-key'), // new client
sdkClientMethod('other-key', 'other-tt'), // new client
sdkClientMethod({ matchingKey: 'other-key', bucketingKey: 'buck' }) // new client
sdkClientMethod('other-key'), // @ts-expect-error
sdkClientMethod('other-key', 'ignored-tt'),
sdkClientMethod({ matchingKey: 'other-key', bucketingKey: 'buck' })
]);
if (ignoresTT) expect(newClients.size).toBe(2);
else expect(newClients.size).toBe(3);
expect(newClients.size).toBe(2);

// each new client must follog the Client API
// each new client must follow the Client API
newClients.forEach(newClient => {
assertClientApi(newClient);
expect(newClient).not.toBe(sdkClientMethod());
Expand Down Expand Up @@ -150,7 +142,7 @@ describe('sdkClientMethodCSFactory', () => {

});

test.each(testTargets)('return main client instance if called with same key', (sdkClientMethodCSFactory) => {
test('returns main client instance if called with same key', () => {

params.settings = settingsWithKey;
// @ts-expect-error
Expand All @@ -163,20 +155,7 @@ describe('sdkClientMethodCSFactory', () => {
expect(params.syncManager.shared).not.toBeCalled();
});

test.each(testTargets)('return main client instance if called with same key and TT', (sdkClientMethodCSFactory) => {

params.settings = settingsWithKeyAndTT;
// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);

expect(sdkClientMethod()).toBe(sdkClientMethod(settingsWithKeyAndTT.core.key, settingsWithKeyAndTT.core.trafficType));

expect(params.storage.shared).not.toBeCalled();
expect(params.sdkReadinessManager.shared).not.toBeCalled();
expect(params.syncManager.shared).not.toBeCalled();
});

test.each(testTargets)('return main client instance if called with same key object', (sdkClientMethodCSFactory) => {
test('returns main client instance if called with same key object', () => {
// @ts-expect-error
params.settings = settingsWithKeyObject;
// @ts-expect-error
Expand All @@ -189,39 +168,37 @@ describe('sdkClientMethodCSFactory', () => {
expect(params.syncManager.shared).not.toBeCalled();
});

test.each(testTargets)('return same client instance if called with same key or traffic type (input validation)', (sdkClientMethodCSFactory, ignoresTT) => {
test('returns same client instance if called with same key (input validation)', () => {
// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);

const clientInstance = sdkClientMethod('key', 'tt');
const clientInstance = sdkClientMethod('key');

expect(sdkClientMethod('key', 'tT')).toBe(clientInstance); // No new client created: TT is lowercased / ignored
expect(sdkClientMethod(' key ', 'tt')).toBe(clientInstance); // No new client created: key is trimmed
expect(sdkClientMethod({ matchingKey: 'key ', bucketingKey: ' key' }, 'TT')).toBe(clientInstance); // No new client created: key object is equivalent to 'key' string
expect(sdkClientMethod('key')).toBe(clientInstance); // No new client created: same key
expect(sdkClientMethod(' key ')).toBe(clientInstance); // No new client created: key is trimmed
expect(sdkClientMethod({ matchingKey: 'key ', bucketingKey: ' key' })).toBe(clientInstance); // No new client created: key object is equivalent to 'key' string

expect(params.storage.shared).toBeCalledTimes(1);
expect(params.sdkReadinessManager.shared).toBeCalledTimes(1);
expect(params.syncManager.shared).toBeCalledTimes(1);

expect(sdkClientMethod('KEY', 'tt')).not.toBe(clientInstance); // New client created: key is case-sensitive
if (!ignoresTT) expect(sdkClientMethod('key', 'TT ')).not.toBe(clientInstance); // New client created: TT is not trimmed
expect(sdkClientMethod('KEY')).not.toBe(clientInstance); // New client created: key is case-sensitive

const clientCount = ignoresTT ? 2 : 3;
const clientCount = 2;
expect(params.storage.shared).toBeCalledTimes(clientCount);
expect(params.sdkReadinessManager.shared).toBeCalledTimes(clientCount);
expect(params.syncManager.shared).toBeCalledTimes(clientCount);
});

test.each(testTargets)('invalid calls throw an error', (sdkClientMethodCSFactory, ignoresTT) => {
test('invalid calls throw an error', () => {
// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);
const sdkClientMethod = sdkClientMethodCSFactory(params); // @ts-expect-error
expect(() => sdkClientMethod({ matchingKey: settingsWithKey.core.key, bucketingKey: undefined })).toThrow('Shared Client needs a valid key.');
if (!ignoresTT) expect(() => sdkClientMethod('valid-key', ['invalid-TT'])).toThrow('Shared Client needs a valid traffic type or no traffic type at all.');
});

test.each(testTargets)('attributes binding - main client', (sdkClientMethodCSFactory) => {
test('attributes binding - main client', () => {
// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);
const sdkClientMethod = sdkClientMethodCSFactory(params) as any;

// should return a function
expect(typeof sdkClientMethod).toBe('function');
Expand Down Expand Up @@ -273,7 +250,7 @@ describe('sdkClientMethodCSFactory', () => {
});


test.each(testTargets)('attributes binding - shared clients', (sdkClientMethodCSFactory) => {
test('attributes binding - shared clients', () => {
// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);

Expand Down
13 changes: 5 additions & 8 deletions src/sdkClient/clientCS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,17 @@ import { clientAttributesDecoration } from './clientAttributesDecoration';


/**
* Decorator that binds a key and (optionally) a traffic type to client methods
* Decorator that binds a key to client methods
*
* @param client sync client instance
* @param key validated split key
* @param trafficType validated traffic type
*/
export function clientCSDecorator(log: ILogger, client: SplitIO.IClient, key: SplitIO.SplitKey, trafficType?: string): SplitIO.ICsClient {
export function clientCSDecorator(log: ILogger, client: SplitIO.IClient, key: SplitIO.SplitKey): SplitIO.ICsClient {

let clientCS = clientAttributesDecoration(log, client);

return objectAssign(clientCS, {
// In the client-side API, we bind a key to the client `getTreatment*` methods
// In the client-side API, we bind a key to the client `getTreatment*` and `track` methods
getTreatment: clientCS.getTreatment.bind(clientCS, key),
getTreatmentWithConfig: clientCS.getTreatmentWithConfig.bind(clientCS, key),
getTreatments: clientCS.getTreatments.bind(clientCS, key),
Expand All @@ -26,12 +25,10 @@ export function clientCSDecorator(log: ILogger, client: SplitIO.IClient, key: Sp
getTreatmentsByFlagSet: clientCS.getTreatmentsByFlagSet.bind(clientCS, key),
getTreatmentsWithConfigByFlagSet: clientCS.getTreatmentsWithConfigByFlagSet.bind(clientCS, key),

// Key is bound to the `track` method. Same thing happens with trafficType but only if provided
track: trafficType ? clientCS.track.bind(clientCS, key, trafficType) : clientCS.track.bind(clientCS, key),
track: clientCS.track.bind(clientCS, key),

// Not part of the public API. These properties are used to support other modules (e.g., Split Suite)
isClientSide: true,
key,
trafficType
key
}) as SplitIO.ICsClient;
}
2 changes: 1 addition & 1 deletion src/sdkClient/sdkClientMethodCS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
return mainClientInstance;
}

// Validate the key value. The trafficType (2nd argument) is ignored
// Validate the key value
const validKey = validateKey(log, key, LOG_PREFIX_CLIENT_INSTANTIATION);
if (validKey === false) {
throw new Error('Shared Client needs a valid key.');
Expand Down
98 changes: 0 additions & 98 deletions src/sdkClient/sdkClientMethodCSWithTT.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/sdkFactory/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export interface ISdkFactoryParams {

// Sdk client method factory (ISDK::client method).
// It Allows to distinguish SDK clients with the client-side API (`ICsSDK`) or server-side API (`ISDK` or `IAsyncSDK`).
sdkClientMethodFactory: (params: ISdkFactoryContext) => ({ (): SplitIO.ICsClient; (key: SplitIO.SplitKey, trafficType?: string | undefined): SplitIO.ICsClient; } | (() => SplitIO.IClient) | (() => SplitIO.IAsyncClient))
sdkClientMethodFactory: (params: ISdkFactoryContext) => ({ (): SplitIO.ICsClient; (key: SplitIO.SplitKey): SplitIO.ICsClient; } | (() => SplitIO.IClient) | (() => SplitIO.IAsyncClient))

// Impression observer factory.
impressionsObserverFactory: () => IImpressionObserver
Expand Down
10 changes: 1 addition & 9 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ export interface ISettings {
readonly core: {
authorizationKey: string,
key: SplitIO.SplitKey,
trafficType?: string,
labelsEnabled: boolean,
IPAddressesEnabled: boolean
},
Expand Down Expand Up @@ -923,12 +922,6 @@ export namespace SplitIO {
* @property {SplitKey} key
*/
key: SplitKey,
/**
* Traffic type associated with the customer identifier. @see {@link https://help.split.io/hc/en-us/articles/360019916311-Traffic-type}
* If no provided as a setting it will be required on the client.track() calls.
* @property {string} trafficType
*/
trafficType?: string,
/**
* Disable labels from being sent to Split backend. Labels may contain sensitive information.
* @property {boolean} labelsEnabled
Expand Down Expand Up @@ -1037,10 +1030,9 @@ export namespace SplitIO {
* Returns a shared client of the SDK, with the given key and optional traffic type.
* @function client
* @param {SplitKey} key The key for the new client instance.
* @param {string=} trafficType The traffic type of the provided key.
* @returns {ICsClient} The client instance.
*/
client(key: SplitKey, trafficType?: string): ICsClient,
client(key: SplitKey): ICsClient,
/**
* Returns a manager instance of the SDK to explore available information.
* @function manager
Expand Down
21 changes: 6 additions & 15 deletions src/utils/settingsValidation/__tests__/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,40 +233,32 @@ describe('settingsValidation', () => {
expect(settings.core.key).toBe(undefined);
});

test('validates and sanitizes key and traffic type in client-side', () => {
const clientSideValidationParams = { ...minimalSettingsParams, acceptKey: true, acceptTT: true };
test('validates and sanitizes key in client-side', () => {
const clientSideValidationParams = { ...minimalSettingsParams, acceptKey: true };

const samples = [{
key: ' valid-key ', settingsKey: 'valid-key', // key string is trimmed
trafficType: 'VALID-TT', settingsTrafficType: 'valid-tt', // TT is converted to lowercase
}, {
key: undefined, settingsKey: false, // undefined key is not valid in client-side
trafficType: undefined, settingsTrafficType: undefined,
}, {
key: null, settingsKey: false,
trafficType: null, settingsTrafficType: false,
key: {}, settingsKey: false,
}, {
key: true, settingsKey: false,
trafficType: true, settingsTrafficType: false,
}, {
key: 1.5, settingsKey: '1.5', // finite number as key is parsed
trafficType: 100, settingsTrafficType: false,
}, {
key: { matchingKey: 100, bucketingKey: ' BUCK ' }, settingsKey: { matchingKey: '100', bucketingKey: 'BUCK' },
trafficType: {}, settingsTrafficType: false,
}];

samples.forEach(({ key, trafficType, settingsKey, settingsTrafficType }) => {
samples.forEach(({ key, settingsKey }) => {
const settings = settingsValidation({
core: {
authorizationKey: 'dummy token',
key,
trafficType
key
}
}, clientSideValidationParams);

expect(settings.core.key).toEqual(settingsKey);
expect(settings.core.trafficType).toEqual(settingsTrafficType);
});
});

Expand All @@ -275,12 +267,11 @@ describe('settingsValidation', () => {
core: {
authorizationKey: 'dummy token',
key: true,
trafficType: true
trafficType: 'ignored'
}
}, { ...minimalSettingsParams, acceptKey: true });

expect(settings.core.key).toEqual(false); // key is validated
expect(settings.core.trafficType).toEqual(true); // traffic type is ignored
});

// Not implemented yet
Expand Down
Loading

0 comments on commit 66210e6

Please sign in to comment.