SDKS-5410. Create attributes cache in memory and add it to browser cl… - #66
Conversation
| /** | ||
| * Remove all the stored attributes in the client's in memory attribute storage | ||
| */ | ||
| clearAttributes(): any |
There was a problem hiding this comment.
clearAttributes(): any --> clearAttributes(): boolean
| expect(cache.getAttribute('attributeName3')).toEqual('attributeValue3'); | ||
| expect(cache.getAttribute('attributeName4')).toEqual('attributeValue4'); | ||
|
|
||
| cache.clear(); |
There was a problem hiding this comment.
expect(cache.clear()).toEqual(true);
| import { AttributesCacheInMemory } from '../AttributesCacheInMemory'; | ||
|
|
||
|
|
||
| describe('ATTRIBUTES CACHE', () => { |
There was a problem hiding this comment.
I would remove the describe call if there is only one test inside.
|
|
||
|
|
||
| test('INPUT VALIDATION for Attribute', () => { | ||
| expect(validateAttribute(loggerMock, '', 'empty', 'some_method_attrs')).toEqual(false); // It should be invalid if the attribute key is not a string |
There was a problem hiding this comment.
Emma, it seems that there are more asserts here that we can also migrate.
| test('Should return false and log error if attributes map is invalid', () => { | ||
|
|
||
| expect(validateAttributesDeep(loggerMock, { '': 'empty' }, 'some_method_attrs')).toEqual(false); // It should be invalid if the attribute key is not a string | ||
| expect(validateAttributesDeep(loggerMock, { 'attributeKey': new Date() }, 'some_method_attrs')).toEqual(false); // It should be invalid if the attribute value is not a String, Number, Boolean or Lists. |
There was a problem hiding this comment.
migrate the tape assert assert.equals(validateAttributesDeep({'attrKey': null}, 'some_method_attrs'), false, 'should be invalid if the attribute value is not a String, Number, Boolean or Lists.');
| /** | ||
| * Add in memory attributes storage methods and combine them with any attribute received from the getTreatment/s call | ||
| */ | ||
| export function ClientAttributesDecoration<TClient extends SplitIO.IClient | SplitIO.IAsyncClient>(log: ILogger, client: TClient): any { |
There was a problem hiding this comment.
Rename to lower case: clientAttributesDecoration. For convention, we use lowercase for factory functions (E.g.: = myFactory()) and uppercase for classes -a.k.a constructor functions- (E.g., = new MyClass())
| * @param {string, number, boolean, list} attributeValue Attribute value | ||
| * @returns {boolean} true if the attribute was stored and false otherways | ||
| */ | ||
| function setAttribute(attributeName: string, attributeValue: Object): boolean { |
There was a problem hiding this comment.
Let's try to use the Typescript automatic type inference as much as possible. For example:
- Do
let x = 10, instead oflet x: number = 10 - In this case, remove the return type of the method, since if should be inferred automatically:
function setAttribute(attributeName: string, attributeValue: Object) {
| setAttribute: setAttribute, | ||
| getAttribute: getAttribute, | ||
| setAttributes: setAttributes, | ||
| getAttributes: getAttributes, | ||
| removeAttribute: removeAttribute, | ||
| clearAttributes: clearAttributes |
There was a problem hiding this comment.
these methods can be defined here directly, to reduce some size. For example, replace
setAttributes: setAttributes,
with
/**
* Add an attribute to client's in memory attributes storage
*
* @param {string} attributeName Attrinute name
* @param {string, number, boolean, list} attributeValue Attribute value
* @returns {boolean} true if the attribute was stored and false otherways
*/
setAttribute(attributeName: string, attributeValue: Object) {
const attribute: Record<string, Object> = {};
attribute[attributeName] = attributeValue;
if (!validateAttributesDeep(log, attribute, 'setAttribute')) return false;
log.debug(`stored ${attributeValue} for attribute ${attributeName}`);
return attributeStorage.setAttribute(attributeName, attributeValue);
},
| expect(client.getAttribute('attributeName3')).toEqual('attributeValue3'); | ||
| expect(client.getAttribute('attributeName4')).toEqual('attributeValue4'); | ||
|
|
||
| client.clearAttributes(); |
There was a problem hiding this comment.
expect(client.clearAttributes()).toBe(true);
| expect(emmanuelClient.getAttributes()).toEqual({ name: 'Emmanuel', email: 'emmanuel@split.io' }); | ||
| expect(emilianoClient.getAttributes()).toEqual({ name: 'Emiliano', email: 'emiliano@split.io' }); | ||
|
|
||
| emmanuelClient.clearAttributes(); |
There was a problem hiding this comment.
expect(emmanuelClient.clearAttributes()).toBe(true);
3 participants
Javascript commons library
What did you accomplish?
created attributes in memory cache and attributes decorator for browser client
How do we test the changes introduced in this PR?
Added unit tests
Extra Notes