From ccc135f2e2874ae323453d72ca51e203a48bc7b8 Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Thu, 10 Oct 2019 21:37:18 +0200 Subject: [PATCH 1/8] feat(logger): new loglevel 'verbose' and some more log statements new log statements for: - toDb - fromDb other: - include failing value in mapping errors --- src/logger/log-level.type.ts | 1 + src/logger/logger.spec.ts | 96 ++++++++++++++++++- src/logger/logger.ts | 45 +++++++++ src/mapper/for-type/boolean.mapper.ts | 10 +- src/mapper/for-type/collection.mapper.spec.ts | 50 ++++++++++ src/mapper/for-type/collection.mapper.ts | 26 +++-- src/mapper/for-type/enum.mapper.spec.ts | 24 +++-- src/mapper/for-type/enum.mapper.ts | 17 +++- src/mapper/for-type/null.mapper.ts | 8 +- src/mapper/for-type/number.mapper.ts | 4 +- src/mapper/for-type/object.mapper.ts | 1 + src/mapper/for-type/string.mapper.spec.ts | 3 + src/mapper/for-type/string.mapper.ts | 2 +- src/mapper/mapper.ts | 19 +++- src/mapper/util.ts | 10 +- .../model-without-partition-key.model.ts | 0 tsconfig.jest.json | 3 +- 17 files changed, 279 insertions(+), 40 deletions(-) create mode 100644 test/models/model-without-partition-key.model.ts diff --git a/src/logger/log-level.type.ts b/src/logger/log-level.type.ts index 704fb241d..83438a77b 100644 --- a/src/logger/log-level.type.ts +++ b/src/logger/log-level.type.ts @@ -9,4 +9,5 @@ export enum LogLevel { WARNING = 2, INFO = 3, DEBUG = 4, + VERBOSE = 5, } diff --git a/src/logger/logger.spec.ts b/src/logger/logger.spec.ts index 2d92d3dc7..96488108a 100644 --- a/src/logger/logger.spec.ts +++ b/src/logger/logger.spec.ts @@ -1,8 +1,10 @@ -import { Employee } from '../../test/models' +import { Employee, SimpleModel } from '../../test/models' import { updateDynamoEasyConfig } from '../config/update-config.function' import { DynamoStore } from '../dynamo/dynamo-store' import { LogInfo } from './log-info.type' +import { LogLevel } from './log-level.type' import { LogReceiver } from './log-receiver.type' +import { createLogger, createOptModelLogger, Logger, OptModelLogger } from './logger' describe('log receiver', () => { let logs: LogInfo[] = [] @@ -21,3 +23,95 @@ describe('log receiver', () => { expect(logs[0].modelConstructor).toBe(Employee.name) }) }) + +describe('createLogger', () => { + let logReceiver: jest.Mock + let logger: Logger + beforeEach(() => { + logReceiver = jest.fn() + updateDynamoEasyConfig({ logReceiver }) + logger = createLogger('MyClass', SimpleModel) + }) + it('creates correct Logger instance with working warn function', () => { + logger.warn('warn') + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('warn') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.WARNING) + }) + it('creates correct Logger instance with working info function', () => { + logger.info('info') + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('info') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.INFO) + }) + it('creates correct Logger instance with working debug function', () => { + logger.debug('debug') + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('debug') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.DEBUG) + }) + it('creates correct Logger instance with working debug function', () => { + logger.verbose('verbose') + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('verbose') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.VERBOSE) + }) +}) + +describe('createOptModelLogger', () => { + let logReceiver: jest.Mock + let logger: OptModelLogger + beforeEach(() => { + logReceiver = jest.fn() + updateDynamoEasyConfig({ logReceiver }) + logger = createOptModelLogger('MyClass') + }) + it('creates correct OptModelLogger instance with working warn function', () => { + logger.warn('warn', SimpleModel) + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('warn') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.WARNING) + }) + it('creates correct OptModelLogger instance with working info function', () => { + logger.info('info', SimpleModel) + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('info') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.INFO) + }) + it('creates correct OptModelLogger instance with working debug function', () => { + logger.debug('debug', SimpleModel) + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('debug') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.DEBUG) + }) + it('creates correct OptModelLogger instance with working verbose function', () => { + logger.verbose('verbose', SimpleModel) + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0]).toBeDefined() + expect(logReceiver.mock.calls[0][0].className).toEqual('MyClass') + expect(logReceiver.mock.calls[0][0].modelConstructor).toEqual('SimpleModel') + expect(logReceiver.mock.calls[0][0].message).toEqual('verbose') + expect(logReceiver.mock.calls[0][0].level).toEqual(LogLevel.VERBOSE) + }) +}) diff --git a/src/logger/logger.ts b/src/logger/logger.ts index d5d98ed9a..edd47badb 100644 --- a/src/logger/logger.ts +++ b/src/logger/logger.ts @@ -13,6 +13,12 @@ import { LogLevel } from './log-level.type' */ export type LogFn = (message: string, data?: any) => void +export type OptModelLogFn = ( + message: string, + modelConstructor: ModelConstructor | undefined | null, + data?: any, +) => void + /** * @hidden */ @@ -20,6 +26,17 @@ export interface Logger { warn: LogFn info: LogFn debug: LogFn + verbose: LogFn +} + +/** + * @hidden + */ +export interface OptModelLogger { + warn: OptModelLogFn + info: OptModelLogFn + debug: OptModelLogFn + verbose: OptModelLogFn } /** @@ -38,6 +55,22 @@ function getLogFn(className: string, modelConstructor: string, level: LogLevel): } } +/** + * @hidden + */ +function getOptModelLogFn(className: string, level: LogLevel): OptModelLogFn { + return (message: string, modelConstructor: ModelConstructor | undefined | null, data?: any) => { + dynamoEasyConfig.logReceiver({ + className, + modelConstructor: (modelConstructor && modelConstructor.name) || 'NO_MODEL', + level, + message, + data, + timestamp: Date.now(), + }) + } +} + /** * @hidden */ @@ -46,5 +79,17 @@ export function createLogger(className: string, modelConstructor: ModelConstruct warn: getLogFn(className, modelConstructor.name, LogLevel.WARNING), info: getLogFn(className, modelConstructor.name, LogLevel.INFO), debug: getLogFn(className, modelConstructor.name, LogLevel.DEBUG), + verbose: getLogFn(className, modelConstructor.name, LogLevel.VERBOSE), + } +} +/** + * @hidden + */ +export function createOptModelLogger(className: string): OptModelLogger { + return { + warn: getOptModelLogFn(className, LogLevel.WARNING), + info: getOptModelLogFn(className, LogLevel.INFO), + debug: getOptModelLogFn(className, LogLevel.DEBUG), + verbose: getOptModelLogFn(className, LogLevel.VERBOSE), } } diff --git a/src/mapper/for-type/boolean.mapper.ts b/src/mapper/for-type/boolean.mapper.ts index 8f65ec6ad..587c9b2e4 100644 --- a/src/mapper/for-type/boolean.mapper.ts +++ b/src/mapper/for-type/boolean.mapper.ts @@ -4,16 +4,16 @@ import { BooleanAttribute } from '../type/attribute.type' import { MapperForType } from './base.mapper' -function booleanFromDb(dbValue: BooleanAttribute): boolean { - if (dbValue.BOOL === undefined) { - throw new Error('only attribute values with BOOL value can be mapped to a boolean') +function booleanFromDb(attributeValue: BooleanAttribute): boolean { + if (attributeValue.BOOL === undefined) { + throw new Error(`there is no BOOL(ean) value defined on given attribute value: ${JSON.stringify(attributeValue)}`) } - return dbValue.BOOL === true + return attributeValue.BOOL === true } function booleanToDb(modelValue: boolean): BooleanAttribute { if (!(modelValue === true || modelValue === false)) { - throw new Error('only boolean values are mapped to a BOOl attribute') + throw new Error(`only boolean values are mapped to a BOOl attribute, given: ${JSON.stringify(modelValue)}`) } return { BOOL: modelValue } } diff --git a/src/mapper/for-type/collection.mapper.spec.ts b/src/mapper/for-type/collection.mapper.spec.ts index 37a503aa7..638771c63 100644 --- a/src/mapper/for-type/collection.mapper.spec.ts +++ b/src/mapper/for-type/collection.mapper.spec.ts @@ -64,6 +64,16 @@ describe('collection mapper', () => { it('heterogeneous set should throw', () => { expect(() => CollectionMapper.toDb(new Set(['value1', 10]))).toThrow() }) + + /* + * neither set nor arr or not primitive in set + */ + it('should throw if neither array nor set', () => { + expect(() => CollectionMapper.toDb({ aValue: true })).toThrow() + }) + it('should throw if set of non primitives', () => { + expect(() => CollectionMapper.toDb(new Set([{ aValue: true }]))).toThrow() + }) }) describe('with metadata', () => { @@ -117,6 +127,42 @@ describe('collection mapper', () => { expect(attributeValue.L.length).toBe(2) expect(attributeValue.L).toEqual([{ N: '5' }, { N: '10' }]) }) + + it('set with generic number type', () => { + const meta: PropertyMetadata = { + name: 'aName', + nameDb: 'aName', + typeInfo: { + type: Set, + genericType: Number, + }, + } + const r = CollectionMapper.toDb(new Set([1, 2, 3, 5]), meta) + expect(r).toEqual({ NS: ['1', '2', '3', '5'] }) + }) + + it('without generic type but actually set of numbers', () => { + const meta: PropertyMetadata = { + name: 'aName', + nameDb: 'aName', + typeInfo: { + type: Set, + }, + } + const r = CollectionMapper.toDb(new Set([1, 2, 3, 5]), meta) + expect(r).toEqual({ NS: ['1', '2', '3', '5'] }) + }) + + it('throws if explicit type is neither array nor set', () => { + const meta: PropertyMetadata = { + name: 'aName', + nameDb: 'aName', + typeInfo: { + type: Number, + }, + } + expect(() => CollectionMapper.toDb(new Set([0, 1, 1, 2, 3, 5]), meta)).toThrow() + }) }) describe('with CollectionProperty decorator', () => { @@ -195,6 +241,10 @@ describe('collection mapper', () => { expect(numberSet.size).toBe(2) expect(typeof Array.from(numberSet)[0]).toBe('number') }) + + it('throws if not a L|SS|NS|BS', () => { + expect(() => CollectionMapper.fromDb({ S: 'not a list' })).toThrow() + }) }) }) }) diff --git a/src/mapper/for-type/collection.mapper.ts b/src/mapper/for-type/collection.mapper.ts index 19ba72e3f..55117e736 100644 --- a/src/mapper/for-type/collection.mapper.ts +++ b/src/mapper/for-type/collection.mapper.ts @@ -4,7 +4,7 @@ import { hasGenericType, PropertyMetadata } from '../../decorator/metadata/property-metadata.model' import { notNull } from '../../helper/not-null.function' import { fromDb, fromDbOne, toDb, toDbOne } from '../mapper' -import { AttributeCollectionType, AttributeType } from '../type/attribute-type.type' +import { AttributeCollectionType } from '../type/attribute-type.type' import { BinarySetAttribute, ListAttribute, @@ -43,14 +43,18 @@ function collectionFromDb( } // if [(N|S|B)S]et - if ('SS' in attributeValue) { + else if ('SS' in attributeValue) { arr = attributeValue.SS } else if ('NS' in attributeValue) { arr = attributeValue.NS.map(parseFloat) } else if ('BS' in attributeValue) { arr = attributeValue.BS } else { - throw new Error('No Collection Data (SS | NS | BS | L) was found in attribute data') + throw new Error( + `No Collection Data (SS | NS | BS | L) was found in attribute data, given attributeValue: ${JSON.stringify( + attributeValue, + )}`, + ) } return explicitType && explicitType === Array ? arr : new Set(arr) } @@ -60,16 +64,17 @@ function collectionToDb( propertyMetadata?: PropertyMetadata, ): CollectionAttributeTypes | null { if (!(Array.isArray(propertyValue) || isSet(propertyValue))) { - throw new Error(`Given value must be either Array or Set ${propertyValue}`) + throw new Error(`Given value must be either Array or Set ${JSON.stringify(propertyValue)}`) } - let collectionType: AttributeType + let collectionType: AttributeCollectionType // detect collection type if (propertyMetadata) { // based on metadata collectionType = detectCollectionTypeFromMetadata(propertyMetadata, propertyValue) } else { // based on value + // or throw if not a collectionType collectionType = detectCollectionTypeFromValue(propertyValue) } @@ -77,7 +82,7 @@ function collectionToDb( propertyValue = isSet(propertyValue) ? Array.from(propertyValue) : propertyValue // empty values are not allowed for S(et) types only for L(ist) - if ((collectionType === 'SS' || collectionType === 'NS' || collectionType === 'BS') && propertyValue.length === 0) { + if (collectionType !== 'L' && propertyValue.length === 0) { return null } @@ -104,8 +109,7 @@ function collectionToDb( .filter(notNull), } } - default: - throw new Error(`Collection type must be one of SS | NS | BS | L found type ${collectionType}`) + // no 'default' necessary, all possible cases caught } } @@ -121,7 +125,11 @@ function detectCollectionTypeFromMetadata( const explicitType = propertyMetadata && propertyMetadata.typeInfo ? propertyMetadata.typeInfo.type : null if (!(explicitType === Array || explicitType === Set)) { - throw new Error(`only 'Array' and 'Set' are valid values for explicit type, found ${explicitType}`) + throw new Error( + `only 'Array' and 'Set' are valid values for explicit type, found ${explicitType} on value ${JSON.stringify( + propertyValue, + )}`, + ) } if (propertyMetadata.isSortedCollection) { diff --git a/src/mapper/for-type/enum.mapper.spec.ts b/src/mapper/for-type/enum.mapper.spec.ts index 978edf502..1e71f4230 100644 --- a/src/mapper/for-type/enum.mapper.spec.ts +++ b/src/mapper/for-type/enum.mapper.spec.ts @@ -51,15 +51,27 @@ describe('enum mapper', () => { }) it('should throw', () => { - expect(() => { - EnumMapper.fromDb({ S: '2' }, propertyMetadata) - }).toThrowError() + expect(() => EnumMapper.fromDb({ S: '2' }, propertyMetadata)).toThrow() }) it('should throw', () => { - expect(() => { - EnumMapper.fromDb({ S: '2' }) - }).toThrowError() + expect(() => EnumMapper.fromDb({ S: '2' })).toThrow() + }) + + it('should throw', () => { + enum anEnum { + OK, + NOK, + } + const meta: PropertyMetadata = { + name: 'aName', + nameDb: 'sameName', + typeInfo: { + type: String, + genericType: anEnum, + }, + } + expect(() => EnumMapper.fromDb({ N: '2' }, meta)).toThrow() }) }) }) diff --git a/src/mapper/for-type/enum.mapper.ts b/src/mapper/for-type/enum.mapper.ts index 47e254630..e3c052a64 100644 --- a/src/mapper/for-type/enum.mapper.ts +++ b/src/mapper/for-type/enum.mapper.ts @@ -8,11 +8,11 @@ import { MapperForType } from './base.mapper' function enumToDb(value: string | number, propertyMetadata?: PropertyMetadata): NumberAttribute { if (Number.isInteger(value)) { if (hasGenericType(propertyMetadata) && (propertyMetadata.typeInfo.genericType)[value] === undefined) { - throw new Error(`${value} is not a valid value for enum ${propertyMetadata.typeInfo.genericType}`) + throw new Error(`${JSON.stringify(value)} is not a valid value for enum ${propertyMetadata.typeInfo.genericType}`) } return { N: value.toString() } } else { - throw new Error('only integer is a supported value for an enum') + throw new Error(`only integer is a supported value for an enum, given value: ${JSON.stringify(value)}`) } } @@ -24,18 +24,25 @@ function enumFromDb( const enumValue = parseInt(attributeValue.N, 10) if (propertyMetadata && propertyMetadata.typeInfo && propertyMetadata.typeInfo.genericType) { if ((propertyMetadata.typeInfo.genericType)[enumValue] === undefined) { - throw new Error(`${enumValue} is not a valid value for enum ${propertyMetadata.typeInfo.genericType}`) + throw new Error( + `${enumValue} is not a valid value for enum ${JSON.stringify(propertyMetadata.typeInfo.genericType)}`, + ) } } return enumValue } else { - throw new Error('make sure the value is a N(umber), which is the only supported for EnumMapper right now') + throw new Error( + `make sure the value is a N(umber), which is the only supported for EnumMapper right now, given attributeValue: ${JSON.stringify( + attributeValue, + )}`, + ) } } /** - * Enums are mapped to numbers by default + * Enums are mapped to numbers by default. + * ensures given value is from enum, if enum was specified as generic type */ export const EnumMapper: MapperForType = { fromDb: enumFromDb, diff --git a/src/mapper/for-type/null.mapper.ts b/src/mapper/for-type/null.mapper.ts index 65544a7c6..71baad283 100644 --- a/src/mapper/for-type/null.mapper.ts +++ b/src/mapper/for-type/null.mapper.ts @@ -4,17 +4,17 @@ import { NullAttribute } from '../type/attribute.type' import { MapperForType } from './base.mapper' -function nullFromDb(value: NullAttribute): null { - if (value.NULL) { +function nullFromDb(attributeValue: NullAttribute): null { + if (attributeValue.NULL) { return null } else { - throw new Error(`there is no NULL value defined on given attribute value ${value}`) + throw new Error(`there is no NULL value defined on given attribute value: ${JSON.stringify(attributeValue)}`) } } function nullToDb(value: null): NullAttribute { if (value !== null) { - throw new Error(`null mapper only supports null value, got ${value}`) + throw new Error(`null mapper only supports null value, got ${JSON.stringify(value)}`) } return { NULL: true } diff --git a/src/mapper/for-type/number.mapper.ts b/src/mapper/for-type/number.mapper.ts index 2a20522f6..baa83e6d6 100644 --- a/src/mapper/for-type/number.mapper.ts +++ b/src/mapper/for-type/number.mapper.ts @@ -13,13 +13,13 @@ function numberFromDb(attributeValue: NumberAttribute): number { } return numberValue } else { - throw new Error('there is no N(umber) value defiend on given attribute value') + throw new Error(`there is no N(umber) value defined on given attribute value: ${JSON.stringify(attributeValue)}`) } } function numberToDb(modelValue: number): NumberAttribute | null { if (!isNumber(modelValue)) { - throw new Error('this mapper only support values of type number') + throw new Error(`this mapper only support values of type number, value given: ${JSON.stringify(modelValue)}`) } if (isNaN(modelValue)) { diff --git a/src/mapper/for-type/object.mapper.ts b/src/mapper/for-type/object.mapper.ts index 5e5ccfada..cc706917e 100644 --- a/src/mapper/for-type/object.mapper.ts +++ b/src/mapper/for-type/object.mapper.ts @@ -7,6 +7,7 @@ import { Attributes, MapAttribute } from '../type/attribute.type' import { MapperForType } from './base.mapper' function objectFromDb(val: MapAttribute, propertyMetadata?: PropertyMetadata): any { + // todo: shouldn't we check for existence off 'M' here? (and throw if undefined) if (hasType(propertyMetadata)) { return fromDb(val.M, propertyMetadata.typeInfo.type) } else { diff --git a/src/mapper/for-type/string.mapper.spec.ts b/src/mapper/for-type/string.mapper.spec.ts index 122b03f96..8ef713d1b 100644 --- a/src/mapper/for-type/string.mapper.spec.ts +++ b/src/mapper/for-type/string.mapper.spec.ts @@ -28,5 +28,8 @@ describe('string mapper', () => { const stringValue = StringMapper.fromDb({ S: 'myStringValue' }) expect(stringValue).toBe('myStringValue') }) + it('should throw if not a string attribute', () => { + expect(() => StringMapper.fromDb({ N: '8' })).toThrow() + }) }) }) diff --git a/src/mapper/for-type/string.mapper.ts b/src/mapper/for-type/string.mapper.ts index 38446269a..3e86a251b 100644 --- a/src/mapper/for-type/string.mapper.ts +++ b/src/mapper/for-type/string.mapper.ts @@ -8,7 +8,7 @@ function stringFromDb(attributeValue: StringAttribute): string { if (attributeValue.S) { return attributeValue.S } else { - throw new Error('there is no S(tring) value defiend on given attribute value') + throw new Error(`there is no S(tring) value defined on given attribute value: ${JSON.stringify(attributeValue)}`) } } diff --git a/src/mapper/mapper.ts b/src/mapper/mapper.ts index b6f091ff7..b672b2f28 100644 --- a/src/mapper/mapper.ts +++ b/src/mapper/mapper.ts @@ -5,6 +5,7 @@ import { v4 as uuidv4 } from 'uuid' import { hasSortKey, Metadata } from '../decorator/metadata/metadata' import { metadataForModel } from '../decorator/metadata/metadata-for-model.function' import { hasType, Key, PropertyMetadata } from '../decorator/metadata/property-metadata.model' +import { createOptModelLogger } from '../logger/logger' import { ModelConstructor } from '../model/model-constructor' import { MapperForType } from './for-type/base.mapper' import { BooleanMapper } from './for-type/boolean.mapper' @@ -25,10 +26,16 @@ import { getPropertyPath, typeOf, typeOfFromDb } from './util' */ const mapperForType: Map> = new Map() +/** + * @hidden + */ +const logger = createOptModelLogger('dynamo.mapper.mapper') + /** * mapps an item according to given model constructor [its meta data] to attributes */ export function toDb(item: T, modelConstructor?: ModelConstructor): Attributes { + logger.verbose('map toDb', modelConstructor, { item }) const mapped = >{} if (modelConstructor) { @@ -117,6 +124,7 @@ export function toDbOne( propertyPathOrMetadata?: string | PropertyMetadata, propertyMetadata?: PropertyMetadata, ): Attribute | null { + logger.verbose('map toDbOne', null, { propertyValue, propertyPathOrMetadata, propertyMetadata }) const propertyPath = propertyPathOrMetadata && typeof propertyPathOrMetadata === 'string' ? propertyPathOrMetadata : null propertyMetadata = @@ -168,8 +176,9 @@ export function createToKeyFn(modelConstructor: ModelConstructor): (item: const keyProperties = properties.filter(testForKey) - return (item: Partial) => - keyProperties.reduce( + return (item: Partial) => { + logger.verbose('create key', null, { item, propertyMeta: keyProperties }) + return keyProperties.reduce( (key, propMeta) => { if (item[propMeta.name] === null || item[propMeta.name] === undefined) { throw new Error(`there is no value for property ${propMeta.name.toString()} but is ${propMeta.key.type} key`) @@ -180,6 +189,7 @@ export function createToKeyFn(modelConstructor: ModelConstructor): (item: }, >{}, ) + } } /** @@ -230,6 +240,7 @@ export function createKeyAttributes( * parses attributes to a js item according to the given model constructor [its meta data] */ export function fromDb(attributeMap: Attributes, modelConstructor?: ModelConstructor): T { + logger.verbose('parse fromDb', modelConstructor, { attributeMap }) const model: T = {} Object.getOwnPropertyNames(attributeMap).forEach(attributeName => { @@ -277,6 +288,7 @@ export function fromDb(attributeMap: Attributes, modelConstructor?: ModelC * parses an attribute to a js value according to the given property metadata */ export function fromDbOne(attributeValue: Attribute, propertyMetadata?: PropertyMetadata): T { + logger.verbose('parse fromDbOne', null, { attributeValue, propertyMetadata }) const explicitType: AttributeValueType | null = hasType(propertyMetadata) ? propertyMetadata.typeInfo.type : null const type: AttributeValueType = explicitType || typeOfFromDb(attributeValue) @@ -318,6 +330,9 @@ export function forType(type: AttributeValueType): MapperForType mapper = NullMapper break case Binary: + // The applications must encode binary values in base64-encoded format before sending them to DynamoDB. + // Upon receipt of these values, + // DynamoDB decodes the data into an unsigned byte array and uses that as the length of the binary attribute. throw new Error('no mapper for binary type implemented yet') case UndefinedType: mapper = ObjectMapper diff --git a/src/mapper/util.ts b/src/mapper/util.ts index d0ca50f28..56cf23b2c 100644 --- a/src/mapper/util.ts +++ b/src/mapper/util.ts @@ -97,6 +97,7 @@ export function detectCollectionTypeFromValue(collection: any[] | Set): Att return 'SS' } } else { + // basically can't happen since collectionmapper already checks for arr/set or throws throw new Error('given collection was neither array nor Set -> type could not be detected') } } @@ -145,11 +146,10 @@ export function isSet(value: any): value is Set { * @hidden */ export function detectType(value: any): AttributeType { - if (isCollection(value)) { - return detectCollectionTypeFromValue(value) - } else if (isString(value)) { + if (isString(value)) { return 'S' } else if (isNumber(value)) { + // TODO LOW: we should probably use _.isFinite --> otherwise Infinity & NaN are numbers as well return 'N' } else if (isBinary(value)) { return 'B' @@ -157,11 +157,13 @@ export function detectType(value: any): AttributeType { return 'NULL' } else if (typeof value === 'boolean') { return 'BOOL' + } else if (isCollection(value)) { + return detectCollectionTypeFromValue(value) } else if (typeof value === 'object') { return 'M' } - throw new Error(`the type for value ${value} could not be detected`) + throw new Error(`the type for value ${value} could not be detected.`) } /** diff --git a/test/models/model-without-partition-key.model.ts b/test/models/model-without-partition-key.model.ts new file mode 100644 index 000000000..e69de29bb diff --git a/tsconfig.jest.json b/tsconfig.jest.json index 1577af47b..0d1f467ee 100644 --- a/tsconfig.jest.json +++ b/tsconfig.jest.json @@ -11,5 +11,6 @@ "include": [ "src/**/*.spec.ts", "test/**/*" - ] + ], + "exclude": [] } From eeeca9282c00d60e5398f7b8d4d51e6bbdd89932 Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Thu, 10 Oct 2019 22:56:50 +0200 Subject: [PATCH 2/8] refactor(decorator): keep ModelMetadata properties and indexes defined --- src/decorator/decorators.spec.ts | 2 +- src/decorator/impl/index/util.ts | 1 + src/decorator/impl/model/errors.const.ts | 17 ++ src/decorator/impl/model/model.decorator.ts | 150 ++++++++---------- src/decorator/metadata/metadata.ts | 31 ++-- .../metadata/model-metadata.model.ts | 4 +- src/decorator/util.ts | 10 +- src/mapper/mapper.ts | 4 +- .../helper/get-meta-data-property.function.ts | 2 +- 9 files changed, 103 insertions(+), 118 deletions(-) create mode 100644 src/decorator/impl/model/errors.const.ts diff --git a/src/decorator/decorators.spec.ts b/src/decorator/decorators.spec.ts index a5b37236e..4d996ba2b 100644 --- a/src/decorator/decorators.spec.ts +++ b/src/decorator/decorators.spec.ts @@ -65,7 +65,7 @@ describe('Decorators should add correct metadata', () => { }) it('with no properties', () => { - expect(modelOptions.properties).toBeUndefined() + expect(modelOptions.properties).toEqual([]) }) }) diff --git a/src/decorator/impl/index/util.ts b/src/decorator/impl/index/util.ts index 8cd4280a9..85f668f29 100644 --- a/src/decorator/impl/index/util.ts +++ b/src/decorator/impl/index/util.ts @@ -36,6 +36,7 @@ export function initOrUpdateIndex(indexType: IndexType, indexData: IndexData, ta indexData, ) break + // `default` is actually unnecessary - but could only be removed by cast or nonNullAssertion of `propertyMetadata` default: throw new Error(`unsupported index type ${indexType}`) } diff --git a/src/decorator/impl/model/errors.const.ts b/src/decorator/impl/model/errors.const.ts new file mode 100644 index 000000000..488859492 --- /dev/null +++ b/src/decorator/impl/model/errors.const.ts @@ -0,0 +1,17 @@ +/** + * @module decorators + */ + +/** + * @hidden + */ +export const modelErrors = { + gsiMultiplePk: (indexName: string, propDbName: string) => + `there is already a partition key defined for global secondary index ${indexName} (property name: ${propDbName})`, + gsiMultipleSk: (indexName: string, propDbName: string) => + `there is already a sort key defined for global secondary index ${indexName} (property name: ${propDbName})`, + lsiMultipleSk: (indexName: string, propDbName: string) => + `only one sort key can be defined for the same local secondary index, ${propDbName} is already defined as sort key for index ${indexName}`, + lsiRequiresPk: (indexName: string, propDbName: string) => + `the local secondary index ${indexName} requires the partition key to be defined`, +} diff --git a/src/decorator/impl/model/model.decorator.ts b/src/decorator/impl/model/model.decorator.ts index dcfd66765..5685983cc 100644 --- a/src/decorator/impl/model/model.decorator.ts +++ b/src/decorator/impl/model/model.decorator.ts @@ -7,6 +7,7 @@ import { ModelMetadata } from '../../metadata/model-metadata.model' import { PropertyMetadata } from '../../metadata/property-metadata.model' import { SecondaryIndex } from '../index/secondary-index' import { KEY_PROPERTY } from '../property/key-property.const' +import { modelErrors } from './errors.const' import { KEY_MODEL } from './key-model.const' import { ModelData } from './model-data.model' @@ -21,27 +22,26 @@ export function Model(opts: ModelData = {}): ClassDecorator { // get all the properties with @Property() annotation (or @PartitionKey(),...) // if given class has own properties, all inherited properties are already set and we can get the properties with 'getOwnMetadata'. // otherwise when the given class does not have own properties, there's no 'ownMetadata' but we need to get them from the class it extends with 'getMetadata' - const properties: Array> = Reflect.hasOwnMetadata(KEY_PROPERTY, constructor) - ? Reflect.getOwnMetadata(KEY_PROPERTY, constructor) - : Reflect.getMetadata(KEY_PROPERTY, constructor) + const properties: Array> = + (Reflect.hasOwnMetadata(KEY_PROPERTY, constructor) + ? Reflect.getOwnMetadata(KEY_PROPERTY, constructor) + : Reflect.getMetadata(KEY_PROPERTY, constructor)) || [] // get partition key - const partitionKeys = properties - ? properties.filter(property => property.key && property.key.type === 'HASH') - : null - const partitionKeyName: string | null = partitionKeys && partitionKeys.length ? partitionKeys[0].nameDb : null + const partitionKeys = properties.filter(property => property.key && property.key.type === 'HASH') + + const partitionKeyName: string | null = partitionKeys.length ? partitionKeys[0].nameDb : null /* * get the local and global secondary indexes */ - const globalSecondaryIndexes: any = getGlobalSecondaryIndexes(properties) || [] - const localSecondaryIndexes: any = getLocalSecondaryIndexes(partitionKeyName, properties) || [] + const globalSecondaryIndexes: any = getGlobalSecondaryIndexes(properties) + const localSecondaryIndexes: any = getLocalSecondaryIndexes(partitionKeyName, properties) const indexes: Map> = new Map([...globalSecondaryIndexes, ...localSecondaryIndexes]) - const transientProperties = - properties && properties.length - ? properties.filter(property => property.transient === true).map(property => property.name) - : [] + const transientProperties = properties.length + ? properties.filter(property => property.transient === true).map(property => property.name) + : [] const metaData: ModelMetadata = { clazz: constructor, @@ -77,46 +77,36 @@ function testForLSI(property: PropertyMetadata): property is PropertyMetad /** * @hidden */ -function getGlobalSecondaryIndexes(properties: Array>): Map> | null { - if (properties && properties.length) { - return properties.filter(testForGSI).reduce((map, property): Map> => { - let gsi: SecondaryIndex - Object.keys(property.keyForGSI).forEach(indexName => { - if (map.has(indexName)) { - gsi = map.get(indexName) - } else { - gsi = >{} - } - - switch (property.keyForGSI[indexName]) { - case 'HASH': - if (gsi.partitionKey) { - throw new Error( - `there is already a partition key defined for global secondary index ${indexName} (property name: ${property.nameDb})`, - ) - } - - gsi.partitionKey = property.nameDb - break - case 'RANGE': - if (gsi.sortKey) { - throw new Error( - `there is already a sort key defined for global secondary index ${indexName} (property name: ${property.nameDb})`, - ) - } - - gsi.sortKey = property.nameDb - break - } - - map.set(indexName, gsi) - }) - - return map - }, new Map()) - } else { - return null - } +function getGlobalSecondaryIndexes(properties: Array>): Map> { + return properties.filter(testForGSI).reduce((map, property): Map> => { + let gsi: SecondaryIndex + Object.keys(property.keyForGSI).forEach(indexName => { + if (map.has(indexName)) { + gsi = map.get(indexName) + } else { + gsi = >{} + } + + switch (property.keyForGSI[indexName]) { + case 'HASH': + if (gsi.partitionKey) { + throw new Error(modelErrors.gsiMultiplePk(indexName, property.nameDb)) + } + gsi.partitionKey = property.nameDb + break + case 'RANGE': + if (gsi.sortKey) { + throw new Error(modelErrors.gsiMultipleSk(indexName, property.nameDb)) + } + gsi.sortKey = property.nameDb + break + } + + map.set(indexName, gsi) + }) + + return map + }, new Map()) } /** @@ -125,35 +115,27 @@ function getGlobalSecondaryIndexes(properties: Array>): Ma function getLocalSecondaryIndexes( basePartitionKey: string | null, properties: Array>, -): Map> | null { - if (properties && properties.length) { - return properties.filter(testForLSI).reduce((map, property): Map> => { - let lsi: SecondaryIndex - - property.sortKeyForLSI.forEach(indexName => { - if (map.has(indexName)) { - throw new Error( - `only one sort key can be defined for the same local secondary index, ${property.nameDb} is already defined as sort key for index ${indexName}`, - ) - } - - if (!basePartitionKey) { - throw new Error( - 'a local secondary index requires the partition key to be defined, use the @PartitionKey decorator', - ) - } - - lsi = { - partitionKey: basePartitionKey, - sortKey: property.nameDb, - } - - map.set(indexName, lsi) - }) - - return map - }, new Map()) - } else { - return null - } +): Map> { + return properties.filter(testForLSI).reduce((map, property): Map> => { + let lsi: SecondaryIndex + + property.sortKeyForLSI.forEach(indexName => { + if (map.has(indexName)) { + throw new Error(modelErrors.lsiMultipleSk(indexName, property.nameDb)) + } + + if (!basePartitionKey) { + throw new Error(modelErrors.lsiRequiresPk(indexName, property.nameDb)) + } + + lsi = { + partitionKey: basePartitionKey, + sortKey: property.nameDb, + } + + map.set(indexName, lsi) + }) + + return map + }, new Map()) } diff --git a/src/decorator/metadata/metadata.ts b/src/decorator/metadata/metadata.ts index 9faa2a511..452dd223b 100644 --- a/src/decorator/metadata/metadata.ts +++ b/src/decorator/metadata/metadata.ts @@ -32,11 +32,7 @@ export class Metadata { modelOpts: ModelMetadata, propertyName: keyof M, ): PropertyMetadata | undefined { - return ( - (modelOpts.properties && - modelOpts.properties.find(property => property.name === propertyName || property.nameDb === propertyName)) || - undefined - ) + return modelOpts.properties.find(property => property.name === propertyName || property.nameDb === propertyName) } constructor(modelConstructor: ModelConstructor) { @@ -44,7 +40,7 @@ export class Metadata { } forProperty(propertyKey: keyof T | string): PropertyMetadata | undefined { - if (!this.modelOptions.properties) { + if (this.modelOptions.properties.length === 0) { return } if (typeof propertyKey === 'string' && NESTED_ATTR_PATH_REGEX.test(propertyKey)) { @@ -90,7 +86,11 @@ export class Metadata { if (indexName) { const index = this.getIndex(indexName) if (index) { - return index.partitionKey + if (index.partitionKey) { + return index.partitionKey + } else { + throw new Error('the index exists but no partition key for it was defined. use @GSIPartitionKey(indexName)') + } } else { throw new Error(`there is no index defined for name ${indexName}`) } @@ -133,11 +133,7 @@ export class Metadata { * @returns {SecondaryIndex[]} Returns all the secondary indexes if exists or an empty array if none is defined */ getIndexes(): Array> { - if (this.modelOptions.indexes && this.modelOptions.indexes.size) { - return Array.from(this.modelOptions.indexes.values()) - } else { - return [] - } + return Array.from(this.modelOptions.indexes.values()) } /** @@ -145,12 +141,7 @@ export class Metadata { * @returns {SecondaryIndex} Returns the index if one with given name exists, null otherwise */ getIndex(indexName: string): SecondaryIndex | null { - if (this.modelOptions.indexes) { - const index = this.modelOptions.indexes.get(indexName) - return index ? index : null - } - - return null + return this.modelOptions.indexes.get(indexName) || null } } @@ -162,9 +153,9 @@ function filterBy( predicate: (property: PropertyMetadata) => boolean, defaultValue: R, ): Array> | R { - if (modelOptions && modelOptions.properties) { + if (modelOptions) { const properties = modelOptions.properties.filter(predicate) - if (properties && properties.length) { + if (properties.length) { return properties } } diff --git a/src/decorator/metadata/model-metadata.model.ts b/src/decorator/metadata/model-metadata.model.ts index 612d35d17..4d08ea6c1 100644 --- a/src/decorator/metadata/model-metadata.model.ts +++ b/src/decorator/metadata/model-metadata.model.ts @@ -11,9 +11,9 @@ export interface ModelMetadata { clazzName: string clazz: any tableName: string - properties?: Array> + properties: Array> transientProperties?: Array // local and global secondary indexes maps the name to the index definition (partition and optional sort key depending on index type) - indexes?: Map> + indexes: Map> } diff --git a/src/decorator/util.ts b/src/decorator/util.ts index 529778f57..c26f11087 100644 --- a/src/decorator/util.ts +++ b/src/decorator/util.ts @@ -27,12 +27,6 @@ export const getMetadataType = makeMetadataGetter>(KEY_TYP /** * @hidden */ -export function makeMetadataGetter(metadataKey: string): (target: any, targetKey?: string) => T { - return (target: any, targetKey?: string) => { - if (targetKey) { - return Reflect.getMetadata(metadataKey, target, targetKey) - } else { - return Reflect.getMetadata(metadataKey, target) - } - } +export function makeMetadataGetter(metadataKey: string): (target: any, targetKey: string) => T { + return (target: any, targetKey: string) => Reflect.getMetadata(metadataKey, target, targetKey) } diff --git a/src/mapper/mapper.ts b/src/mapper/mapper.ts index b672b2f28..a81d4b54e 100644 --- a/src/mapper/mapper.ts +++ b/src/mapper/mapper.ts @@ -170,8 +170,8 @@ function testForKey(p: PropertyMetadata): p is PropertyMetadata & { key export function createToKeyFn(modelConstructor: ModelConstructor): (item: Partial) => Attributes { const metadata = metadataForModel(modelConstructor) const properties = metadata.modelOptions.properties - if (!properties) { - throw new Error('metadata properties is not defined') + if (!properties.length) { + throw new Error('no properties defined on metadata') } const keyProperties = properties.filter(testForKey) diff --git a/test/helper/get-meta-data-property.function.ts b/test/helper/get-meta-data-property.function.ts index 785cfb974..38dc0e16d 100644 --- a/test/helper/get-meta-data-property.function.ts +++ b/test/helper/get-meta-data-property.function.ts @@ -4,5 +4,5 @@ export function getMetaDataProperty( modelOptions: ModelMetadata, propertyKey: K, ): PropertyMetadata | undefined { - return (modelOptions.properties || []).find(property => property.name === propertyKey) + return modelOptions.properties.find(property => property.name === propertyKey) } From a27e8e7f5f065bdae2e7db615d62f884d5016acb Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Thu, 10 Oct 2019 22:57:53 +0200 Subject: [PATCH 3/8] test(decorator): add tests for model decorator --- .../impl/model/model.decorator.spec.ts | 70 +++++++++++++++++++ src/decorator/metadata/metadata.spec.ts | 30 +++++++- test/models/index.ts | 1 + .../model-without-partition-key.model.ts | 14 ++++ 4 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 src/decorator/impl/model/model.decorator.spec.ts diff --git a/src/decorator/impl/model/model.decorator.spec.ts b/src/decorator/impl/model/model.decorator.spec.ts new file mode 100644 index 000000000..a7bdeaa3f --- /dev/null +++ b/src/decorator/impl/model/model.decorator.spec.ts @@ -0,0 +1,70 @@ +// tslint:disable:max-classes-per-file +import { GSIPartitionKey } from '../index/gsi-partition-key.decorator' +import { GSISortKey } from '../index/gsi-sort-key.decorator' +import { LSISortKey } from '../index/lsi-sort-key.decorator' +import { PartitionKey } from '../key/partition-key.decorator' +import { modelErrors } from './errors.const' +import { Model } from './model.decorator' + +const IX_NAME = 'anIndexName' + +describe('@model decorator', () => { + describe('getGlobalSecondaryIndexes', () => { + // throws on applying decorator + + it('throws when defining multiple partitionKeys for same gsi', () => { + expect(() => { + // @ts-ignore + @Model() + class FailModel { + @GSIPartitionKey(IX_NAME) + pk1: string + @GSIPartitionKey(IX_NAME) + pk2: string + @GSISortKey(IX_NAME) + sk1: string + } + }).toThrow(modelErrors.gsiMultiplePk(IX_NAME, 'pk2')) + }) + it('throws when defining multiple sortKeys for same gsi', () => { + expect(() => { + // @ts-ignore + @Model() + class FailModel { + @GSIPartitionKey(IX_NAME) + pk1: string + @GSISortKey(IX_NAME) + sk1: string + @GSISortKey(IX_NAME) + sk2: string + } + }).toThrow(modelErrors.gsiMultipleSk(IX_NAME, 'sk2')) + }) + }) + describe('getLocalSecondaryIndexes', () => { + it('throws when defining LSI sortKey but no PartitionKey', () => { + expect(() => { + // @ts-ignore + @Model() + class FailModel { + @LSISortKey(IX_NAME) + sk1: string + } + }).toThrow(modelErrors.lsiRequiresPk(IX_NAME, 'sk1')) + }) + it('throws when defining multiple sortKeys for same lsi', () => { + expect(() => { + // @ts-ignore + @Model() + class FailModel { + @PartitionKey() + pk1: string + @LSISortKey(IX_NAME) + sk1: string + @LSISortKey(IX_NAME) + sk2: string + } + }).toThrow(modelErrors.lsiMultipleSk(IX_NAME, 'sk2')) + }) + }) +}) diff --git a/src/decorator/metadata/metadata.spec.ts b/src/decorator/metadata/metadata.spec.ts index 466b0da2a..df7127a14 100644 --- a/src/decorator/metadata/metadata.spec.ts +++ b/src/decorator/metadata/metadata.spec.ts @@ -1,18 +1,23 @@ // tslint:disable:no-non-null-assertion import { ComplexModel, + FAIL_MODEL_GSI, + INDEX_ACTIVE, + INDEX_ACTIVE_CREATED_AT, + INDEX_COUNT, ModelWithABunchOfIndexes, ModelWithAutogeneratedId, ModelWithGSI, ModelWithLSI, + ModelWithoutPartitionKeyModel, SimpleWithCompositePartitionKeyModel, SimpleWithPartitionKeyModel, } from '../../../test/models' -import { INDEX_ACTIVE, INDEX_ACTIVE_CREATED_AT, INDEX_COUNT } from '../../../test/models/model-with-indexes.model' import { Metadata } from './metadata' describe('metadata', () => { let metaDataPartitionKey: Metadata + let metaDataNoPartitionKey: Metadata let metaDataComposite: Metadata let metaDataLsi: Metadata let metaDataGsi: Metadata @@ -22,6 +27,7 @@ describe('metadata', () => { beforeEach(() => { metaDataPartitionKey = new Metadata(SimpleWithPartitionKeyModel) + metaDataNoPartitionKey = new Metadata(ModelWithoutPartitionKeyModel) metaDataComposite = new Metadata(SimpleWithCompositePartitionKeyModel) metaDataLsi = new Metadata(ModelWithLSI) metaDataGsi = new Metadata(ModelWithGSI) @@ -70,6 +76,16 @@ describe('metadata', () => { expect(metaDataIndexes.getPartitionKey(INDEX_ACTIVE_CREATED_AT)).toEqual('active') }) + it('getPartitionKey throws if no partitionKey defined [no index]', () => { + expect(() => metaDataNoPartitionKey.getPartitionKey()).toThrow() + }) + it('getPartitionKey throws if no partitionKey defined [GSI]', () => { + expect(() => metaDataNoPartitionKey.getPartitionKey(FAIL_MODEL_GSI)).toThrow() + }) + it('getPartitionKey throws if given index is not defined', () => { + expect(() => metaDataNoPartitionKey.getPartitionKey('not-existing-index')).toThrow() + }) + it('getSortKey', () => { expect(metaDataPartitionKey.getSortKey()).toBe(null) expect(metaDataComposite.getSortKey()).toBe('creationDate') @@ -77,6 +93,9 @@ describe('metadata', () => { expect(() => metaDataGsi.getSortKey(INDEX_ACTIVE)).toThrow() expect(metaDataIndexes.getSortKey(INDEX_ACTIVE_CREATED_AT)).toBe('createdAt') }) + it('getSortKey throws if given index is not defined', () => { + expect(() => metaDataNoPartitionKey.getSortKey('non-existent-index-name')).toThrow() + }) it('getIndexes', () => { expect(metaDataLsi.getIndexes()).toEqual([{ partitionKey: 'id', sortKey: 'active' }]) @@ -95,4 +114,13 @@ describe('metadata', () => { sortKey: 'createdAt', }) }) + it('getIndex returns null if not existent', () => { + // no indexes at all --> should always be defined + expect(metaDataNoPartitionKey.modelOptions).toBeDefined() + expect(metaDataNoPartitionKey.modelOptions.indexes).toBeInstanceOf(Map) + // no indexes at all + expect(metaDataPartitionKey.getIndex('non-existent-index')).toBeNull() + // indexes defined, but not the one requesting + expect(metaDataIndexes.getIndex('non-existent-index')).toBeNull() + }) }) diff --git a/test/models/index.ts b/test/models/index.ts index 98f8779a2..3f17cb0a7 100644 --- a/test/models/index.ts +++ b/test/models/index.ts @@ -9,6 +9,7 @@ export * from './model-with-enum.model' export * from './model-with-indexes.model' export * from './model-with-date-as-key.model' export * from './model-without-custom-mapper.model' +export * from './model-without-partition-key.model' export * from './nested-complex.model' export * from './nested-object.model' export * from './organization.model' diff --git a/test/models/model-without-partition-key.model.ts b/test/models/model-without-partition-key.model.ts index e69de29bb..e2fb960aa 100644 --- a/test/models/model-without-partition-key.model.ts +++ b/test/models/model-without-partition-key.model.ts @@ -0,0 +1,14 @@ +import { GSISortKey } from '../../src/decorator/impl/index/gsi-sort-key.decorator' +import { Model } from '../../src/decorator/impl/model/model.decorator' +import { Property } from '../../src/decorator/impl/property/property.decorator' + +export const FAIL_MODEL_GSI = 'failModelGsi' + +@Model() +export class ModelWithoutPartitionKeyModel { + @Property() + name: string + + @GSISortKey(FAIL_MODEL_GSI) + gsiRange: string +} From 1759adef6a9acbcc34dd38b10a4c1dff810196a5 Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Thu, 10 Oct 2019 22:58:39 +0200 Subject: [PATCH 4/8] test(mapper): tests for mapper/util `detectType(val)` --- src/mapper/util.spec.ts | 45 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/mapper/util.spec.ts b/src/mapper/util.spec.ts index dffbe58df..a5d8f68f3 100644 --- a/src/mapper/util.spec.ts +++ b/src/mapper/util.spec.ts @@ -3,8 +3,10 @@ import { NullType } from './type/null.type' import { UndefinedType } from './type/undefined.type' import { detectCollectionTypeFromValue, + detectType, isCollection, isHomogeneous, + isNode, isSet, typeName, typeOf, @@ -88,6 +90,49 @@ describe('Util', () => { }) }) + describe('detect type', () => { + it('detects string', () => { + expect(detectType('aString')).toBe('S') + expect(detectType(String('aString'))).toBe('S') + // tslint:disable-next-line:no-construct + expect(detectType(new String('aString'))).toBe('S') + }) + it('detects number', () => { + expect(detectType(3)).toBe('N') + expect(detectType(Number(-5))).toBe('N') + // tslint:disable-next-line:no-construct + expect(detectType(new Number(83))).toBe('N') + }) + it('detects binary', () => { + let buffer: any + if (isNode()) { + buffer = Buffer.alloc(5) + } else { + buffer = new ArrayBuffer(8) + } + expect(detectType(buffer)).toBe('B') + }) + it('detects null', () => { + expect(detectType(null)).toBe('NULL') + }) + it('detects bool', () => { + expect(detectType(true)).toBe('BOOL') + expect(detectType(false)).toBe('BOOL') + }) + it('detects collection', () => { + expect(detectType(new Set(['a']))).toBe('SS') + expect(detectType(new Set([2]))).toBe('NS') + expect(detectType([0, 1, 1, 2, 3, 5])).toBe('L') + }) + it('detects object', () => { + expect(detectType({})).toBe('M') + expect(detectType({ foo: 'bar' })).toBe('M') + }) + it('throws if not such a type', () => { + expect(() => detectType(undefined)).toThrow() + }) + }) + describe('type name', () => { it('String', () => { expect(typeName(String)).toBe('String') From c7009efab95f861435e3598b08b36f1ac9a5bda1 Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Thu, 10 Oct 2019 23:33:36 +0200 Subject: [PATCH 5/8] test(mapper): tests for wrap-mapper-for-collection --- ...rap-mapper-for-collection.function.spec.ts | 233 ++++++++++++------ .../wrap-mapper-for-collection.function.ts | 1 + 2 files changed, 160 insertions(+), 74 deletions(-) diff --git a/src/mapper/wrap-mapper-for-collection.function.spec.ts b/src/mapper/wrap-mapper-for-collection.function.spec.ts index 0629596b1..b92c1506c 100644 --- a/src/mapper/wrap-mapper-for-collection.function.spec.ts +++ b/src/mapper/wrap-mapper-for-collection.function.spec.ts @@ -10,6 +10,10 @@ import { arrayToSetAttribute, listAttributeToArray, setAttributeToArray, + wrapMapperForDynamoListJsArray, + wrapMapperForDynamoListJsSet, + wrapMapperForDynamoSetJsArray, + wrapMapperForDynamoSetJsSet, } from './wrap-mapper-for-collection.function' class MyNumber { @@ -28,108 +32,189 @@ const myCharToNumberAttrMapper: MapperForType = { toDb: propertyValue => ({ N: `${propertyValue.value.charCodeAt(0)}` }), fromDb: attributeValue => ({ value: String.fromCharCode(parseInt(attributeValue.N, 10)) }), } - -describe('arrayToListAttribute', () => { - it('should map empty array to empty (L)ist', () => { - expect(arrayToListAttribute(myNumberToStringAttrMapper)([])).toEqual({ L: [] }) - }) - it('should map array to list with given mapper', () => { - expect(arrayToListAttribute(myNumberToStringAttrMapper)([{ value: 7 }])).toEqual({ L: [{ S: '7' }] }) +describe('wrap mapper for collection', () => { + describe('arrayToListAttribute', () => { + it('should map empty array to empty (L)ist', () => { + expect(arrayToListAttribute(myNumberToStringAttrMapper)([])).toEqual({ L: [] }) + }) + it('should map array to list with given mapper', () => { + expect(arrayToListAttribute(myNumberToStringAttrMapper)([{ value: 7 }])).toEqual({ L: [{ S: '7' }] }) + }) }) -}) -describe('listAttributeToArray', () => { - it('should parse empty list to empty array', () => { - expect(listAttributeToArray(myNumberToStringAttrMapper)({ L: [] })).toEqual([]) - }) - it('should parse list to array with given mapper', () => { - expect(listAttributeToArray(myNumberToStringAttrMapper)({ L: [{ S: '7' }] })).toEqual([{ value: 7 }]) + describe('listAttributeToArray', () => { + it('should parse empty list to empty array', () => { + expect(listAttributeToArray(myNumberToStringAttrMapper)({ L: [] })).toEqual([]) + }) + it('should parse list to array with given mapper', () => { + expect(listAttributeToArray(myNumberToStringAttrMapper)({ L: [{ S: '7' }] })).toEqual([{ value: 7 }]) + }) }) -}) -describe('arrayToSetAttribute', () => { - it('should map empty array to null', () => { - expect(arrayToSetAttribute(myNumberToStringAttrMapper)([])).toEqual(null) - }) - it('should map array to (S)et', () => { - expect(arrayToSetAttribute(myNumberToStringAttrMapper)([{ value: 7 }])).toEqual({ SS: ['7'] }) - expect(arrayToSetAttribute(myCharToNumberAttrMapper)([{ value: 'A' }])).toEqual({ NS: ['65'] }) + describe('arrayToSetAttribute', () => { + it('should map empty array to null', () => { + expect(arrayToSetAttribute(myNumberToStringAttrMapper)([])).toEqual(null) + }) + it('should map array to (S)et', () => { + expect(arrayToSetAttribute(myNumberToStringAttrMapper)([{ value: 7 }])).toEqual({ SS: ['7'] }) + expect(arrayToSetAttribute(myCharToNumberAttrMapper)([{ value: 'A' }])).toEqual({ NS: ['65'] }) + }) }) -}) -describe('setAttributeToArray', () => { - it('should parse (S)et to array', () => { - expect(setAttributeToArray(myNumberToStringAttrMapper)({ SS: ['7'] })).toEqual([{ value: 7 }]) - expect(setAttributeToArray(myCharToNumberAttrMapper)({ NS: ['65'] })).toEqual([{ value: 'A' }]) + describe('setAttributeToArray', () => { + it('should parse (S)et to array', () => { + expect(setAttributeToArray(myNumberToStringAttrMapper)({ SS: ['7'] })).toEqual([{ value: 7 }]) + expect(setAttributeToArray(myCharToNumberAttrMapper)({ NS: ['65'] })).toEqual([{ value: 'A' }]) + }) }) -}) -describe('for collection wrapped mappers', () => { - describe('fromDb', () => { - let aFormId: FormId - beforeEach(() => { - aFormId = new FormId(FormType.REQUEST, 55, 2020) + describe('wrapMapperForDynamoSetJsArray', () => { + const wrappedMapper = wrapMapperForDynamoSetJsArray(myNumberToStringAttrMapper) + it('maps correctly toDb', () => { + const dbVal = wrappedMapper.toDb([{ value: 5 }]) + expect(dbVal).toEqual({ SS: ['5'] }) + }) + it('toDb throws if not an array is given', () => { + expect(() => wrappedMapper.toDb(new Set([{ value: 5 }]))).toThrow() + }) + it('maps correctly fromDb', () => { + const jsVal = wrappedMapper.fromDb({ SS: ['5', '1'] }) + expect(jsVal).toEqual([{ value: 5 }, { value: 1 }]) }) + it('fromDb throws if not a Set was given', () => { + // it does not throw, if it is a wrong set --> this should do the single item mapper + // it only throws if it is not a set at all + expect(() => wrappedMapper.fromDb({ S: '5' })).toThrow() + }) + }) - it('array to (L)ist (itemMapper, sorted)', () => { - const dbObj: Attributes = { - arrayOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, - } - expect(fromDb(dbObj, ModelWithCollections)).toEqual({ arrayOfFormIdToListWithStrings: [aFormId] }) + describe('wrapMapperForDynamoSetJsSet', () => { + const wrappedMapper = wrapMapperForDynamoSetJsSet(myNumberToStringAttrMapper) + it('maps correctly toDb', () => { + const dbVal = wrappedMapper.toDb(new Set([{ value: 5 }])) + expect(dbVal).toEqual({ SS: ['5'] }) + }) + it('toDb throws if not a set is given', () => { + expect(() => wrappedMapper.toDb([{ value: 5 }])).toThrow() }) - it('set to (L)ist (itemMapper, sorted)', () => { - const dbObj: Attributes = { - setOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, - } - expect(fromDb(dbObj, ModelWithCollections)).toEqual({ setOfFormIdToListWithStrings: new Set([aFormId]) }) + it('maps correctly fromDb', () => { + const jsVal = wrappedMapper.fromDb({ SS: ['5', '1'] }) + expect(jsVal).toEqual(new Set([{ value: 5 }, { value: 1 }])) }) + it('fromDb throws if not a Set was given', () => { + // it does not throw, if it is a wrong set --> this should do the single item mapper + // it only throws if it is not a set at all + expect(() => wrappedMapper.fromDb({ S: '5' })).toThrow() + }) + }) - it('array to (S)et (itemMapper)', () => { - const dbObj: Attributes = { arrayOfFormIdToSet: { SS: [FormId.unparse(aFormId)] } } - expect(fromDb(dbObj, ModelWithCollections)).toEqual({ arrayOfFormIdToSet: [aFormId] }) + describe('wrapMapperForDynamoListJsArray', () => { + const wrappedMapper = wrapMapperForDynamoListJsArray(myNumberToStringAttrMapper) + it('maps correctly toDb', () => { + const dbVal = wrappedMapper.toDb([{ value: 5 }]) + expect(dbVal).toEqual({ L: [{ S: '5' }] }) }) - it('set to (S)et (itemMapper)', () => { - const dbObj: Attributes = { setOfFormIdToSet: { SS: [FormId.unparse(aFormId)] } } - expect(fromDb(dbObj, ModelWithCollections)).toEqual({ setOfFormIdToSet: new Set([aFormId]) }) + it('toDb throws if not an array is given', () => { + expect(() => wrappedMapper.toDb(new Set([{ value: 5 }]))).toThrow() }) + it('maps correctly fromDb', () => { + const jsVal = wrappedMapper.fromDb({ L: [{ S: '5' }, { S: '1' }] }) + expect(jsVal).toEqual([{ value: 5 }, { value: 1 }]) + }) + it('fromDb throws if not a List was given', () => { + expect(() => wrappedMapper.fromDb({ SS: ['5'] })).toThrow() + expect(() => wrappedMapper.fromDb({ NS: ['5'] })).toThrow() + expect(() => wrappedMapper.fromDb({ M: { S: '5' } })).toThrow() + }) + }) - it('should throw when not a (S)et attribute', () => { - const dbObj: Attributes = { myFail: { M: { id: { S: '42' } } } } - expect(() => fromDb(dbObj, FailModel)).toThrow() + describe('wrapMapperForDynamoListJsSet', () => { + const wrappedMapper = wrapMapperForDynamoListJsSet(myNumberToStringAttrMapper) + it('maps correctly toDb', () => { + const dbVal = wrappedMapper.toDb(new Set([{ value: 5 }])) + expect(dbVal).toEqual({ L: [{ S: '5' }] }) + }) + it('toDb throws if not a set is given', () => { + expect(() => wrappedMapper.toDb([{ value: 5 }])).toThrow() + }) + it('maps correctly fromDb', () => { + const jsVal = wrappedMapper.fromDb({ L: [{ S: '5' }, { S: '1' }] }) + expect(jsVal).toEqual(new Set([{ value: 5 }, { value: 1 }])) + }) + it('fromDb throws if not a List was given', () => { + expect(() => wrappedMapper.fromDb({ SS: ['5'] })).toThrow() + expect(() => wrappedMapper.fromDb({ NS: ['5'] })).toThrow() + expect(() => wrappedMapper.fromDb({ M: { S: '5' } })).toThrow() }) }) - describe('toDb', () => { - let aFormId: FormId + describe('for collection wrapped mappers', () => { + describe('fromDb', () => { + let aFormId: FormId + beforeEach(() => { + aFormId = new FormId(FormType.REQUEST, 55, 2020) + }) - beforeEach(() => { - aFormId = new FormId(FormType.REQUEST, 55, 2020) - }) + it('array to (L)ist (itemMapper, sorted)', () => { + const dbObj: Attributes = { + arrayOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, + } + expect(fromDb(dbObj, ModelWithCollections)).toEqual({ arrayOfFormIdToListWithStrings: [aFormId] }) + }) + it('set to (L)ist (itemMapper, sorted)', () => { + const dbObj: Attributes = { + setOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, + } + expect(fromDb(dbObj, ModelWithCollections)).toEqual({ setOfFormIdToListWithStrings: new Set([aFormId]) }) + }) - it('array to (L)ist (itemMapper, sorted)', () => { - expect(toDb({ arrayOfFormIdToListWithStrings: [aFormId] }, ModelWithCollections)).toEqual({ - arrayOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, + it('array to (S)et (itemMapper)', () => { + const dbObj: Attributes = { arrayOfFormIdToSet: { SS: [FormId.unparse(aFormId)] } } + expect(fromDb(dbObj, ModelWithCollections)).toEqual({ arrayOfFormIdToSet: [aFormId] }) }) - }) - it('set to (L)ist (itemMapper, sorted)', () => { - expect(toDb({ setOfFormIdToListWithStrings: new Set([aFormId]) }, ModelWithCollections)).toEqual({ - setOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, + it('set to (S)et (itemMapper)', () => { + const dbObj: Attributes = { setOfFormIdToSet: { SS: [FormId.unparse(aFormId)] } } + expect(fromDb(dbObj, ModelWithCollections)).toEqual({ setOfFormIdToSet: new Set([aFormId]) }) }) - }) - it('array to (S)et (itemMapper)', () => { - expect(toDb({ arrayOfFormIdToSet: [aFormId] }, ModelWithCollections)).toEqual({ - arrayOfFormIdToSet: { SS: [FormId.unparse(aFormId)] }, + it('should throw when not a (S)et attribute', () => { + const dbObj: Attributes = { myFail: { M: { id: { S: '42' } } } } + expect(() => fromDb(dbObj, FailModel)).toThrow() }) }) - it('set to (S)et (itemMapper)', () => { - expect(toDb({ setOfFormIdToSet: new Set([aFormId]) }, ModelWithCollections)).toEqual({ - setOfFormIdToSet: { SS: [FormId.unparse(aFormId)] }, + + describe('toDb', () => { + let aFormId: FormId + + beforeEach(() => { + aFormId = new FormId(FormType.REQUEST, 55, 2020) }) - }) - it('should throw when wrong mapper', () => { - expect(() => toDb({ myFail: [{ id: 42 }] }, FailModel)).toThrow() + it('array to (L)ist (itemMapper, sorted)', () => { + expect(toDb({ arrayOfFormIdToListWithStrings: [aFormId] }, ModelWithCollections)).toEqual({ + arrayOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, + }) + }) + it('set to (L)ist (itemMapper, sorted)', () => { + expect(toDb({ setOfFormIdToListWithStrings: new Set([aFormId]) }, ModelWithCollections)).toEqual({ + setOfFormIdToListWithStrings: { L: [formIdMapper.toDb(aFormId)] }, + }) + }) + + it('array to (S)et (itemMapper)', () => { + expect(toDb({ arrayOfFormIdToSet: [aFormId] }, ModelWithCollections)).toEqual({ + arrayOfFormIdToSet: { SS: [FormId.unparse(aFormId)] }, + }) + }) + it('set to (S)et (itemMapper)', () => { + expect(toDb({ setOfFormIdToSet: new Set([aFormId]) }, ModelWithCollections)).toEqual({ + setOfFormIdToSet: { SS: [FormId.unparse(aFormId)] }, + }) + }) + + it('should throw when wrong mapper', () => { + expect(() => toDb({ myFail: [{ id: 42 }] }, FailModel)).toThrow() + }) }) }) }) diff --git a/src/mapper/wrap-mapper-for-collection.function.ts b/src/mapper/wrap-mapper-for-collection.function.ts index cd5d9f391..0f57089e7 100644 --- a/src/mapper/wrap-mapper-for-collection.function.ts +++ b/src/mapper/wrap-mapper-for-collection.function.ts @@ -120,6 +120,7 @@ export function wrapMapperForDynamoSetJsArray Date: Fri, 11 Oct 2019 01:48:13 +0200 Subject: [PATCH 6/8] fix(partition-key.decorator): use logger instead of console --- src/decorator/impl/key/partition-key.decorator.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/decorator/impl/key/partition-key.decorator.ts b/src/decorator/impl/key/partition-key.decorator.ts index c47e0edc1..de26ff21f 100644 --- a/src/decorator/impl/key/partition-key.decorator.ts +++ b/src/decorator/impl/key/partition-key.decorator.ts @@ -1,10 +1,13 @@ /** * @module decorators */ +import { createOptModelLogger } from '../../../logger/logger' import { PropertyMetadata } from '../../metadata/property-metadata.model' import { initOrUpdateProperty } from '../property/init-or-update-property.function' import { KEY_PROPERTY } from '../property/key-property.const' +const logger = createOptModelLogger('@PartitionKey') + export function PartitionKey(): PropertyDecorator { return (target: any, propertyKey: string | symbol) => { if (typeof propertyKey === 'string') { @@ -14,9 +17,11 @@ export function PartitionKey(): PropertyDecorator { const existingPartitionKeys = properties.filter(property => property.key && property.key.type === 'HASH') if (existingPartitionKeys.length) { if (properties.find(property => property.name === propertyKey)) { - // just ignore this and go on, somehow the partition key gets defined - // tslint:disable-next-line:no-console - console.warn(`this is the second execution to define the partitionKey for property ${propertyKey}`) + // just ignore this and go on, somehow the partition key gets defined two times + logger.warn( + `this is the second execution to define the partitionKey for property ${propertyKey}`, + target.constructor, + ) } else { throw new Error( 'only one partition key is allowed per model, if you want to define key for indexes use one of these decorators: ' + From 6a833af32fc3ae1cc5b3d3a6abb2ba420858be5b Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Fri, 11 Oct 2019 01:49:47 +0200 Subject: [PATCH 7/8] test(decorator): add tests for multiple gsi/lsi/partiton key decorators --- src/decorator/decorators.spec.ts | 113 +++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 7 deletions(-) diff --git a/src/decorator/decorators.spec.ts b/src/decorator/decorators.spec.ts index 4d996ba2b..782b3bc7f 100644 --- a/src/decorator/decorators.spec.ts +++ b/src/decorator/decorators.spec.ts @@ -19,6 +19,8 @@ import { SimpleModel, } from '../../test/models' import { Form } from '../../test/models/real-world' +import { updateDynamoEasyConfig } from '../config/update-config.function' +import { LogLevel } from '../logger/log-level.type' import { CollectionProperty } from './impl/collection/collection-property.decorator' import { GSIPartitionKey } from './impl/index/gsi-partition-key.decorator' import { GSISortKey } from './impl/index/gsi-sort-key.decorator' @@ -317,6 +319,77 @@ describe('Decorators should add correct metadata', () => { }) }) + describe('multiple property decorators', () => { + const REVERSE_INDEX = 'reverse-index' + const OTHER_INDEX = 'other-index' + const LSI_1 = 'lsi-1' + const LSI_2 = 'lsi-2' + + @Model() + class ABC { + @PartitionKey() + @Property({ name: 'pk' }) + @GSISortKey(REVERSE_INDEX) + id: string + + @SortKey() + @Property({ name: 'sk' }) + @GSIPartitionKey(REVERSE_INDEX) + @GSISortKey(OTHER_INDEX) + timestamp: number + + @GSIPartitionKey(OTHER_INDEX) + @LSISortKey(LSI_1) + @LSISortKey(LSI_2) + otherId: string + } + + let metaData: Metadata + + beforeEach(() => (metaData = metadataForModel(ABC))) + + it('PartitionKey & Property & GSISortKey should combine the data', () => { + const propData = metaData.forProperty('id') + expect(propData).toEqual({ + key: { type: 'HASH' }, + name: 'id', + nameDb: 'pk', + typeInfo: { type: String }, + keyForGSI: { [REVERSE_INDEX]: 'RANGE' }, + }) + }) + it('SortKey & Property & GSIPartitionKey & GSISortKey should combine the data', () => { + const propData = metaData.forProperty('timestamp') + expect(propData).toEqual({ + key: { type: 'RANGE' }, + name: 'timestamp', + nameDb: 'sk', + typeInfo: { type: Number }, + keyForGSI: { [REVERSE_INDEX]: 'HASH', [OTHER_INDEX]: 'RANGE' }, + }) + }) + it('GSIPartitionKey & multiple LSISortkey should combine the data', () => { + const propData = metaData.forProperty('otherId') + expect(propData).toBeDefined() + expect(propData!.name).toEqual('otherId') + expect(propData!.nameDb).toEqual('otherId') + expect(propData!.typeInfo).toEqual({ type: String }) + expect(propData!.keyForGSI).toEqual({ [OTHER_INDEX]: 'HASH' }) + expect(propData!.sortKeyForLSI).toContain(LSI_1) + expect(propData!.sortKeyForLSI).toContain(LSI_2) + }) + it('correctly defines the indexes', () => { + const reverseIndex = metaData.getIndex(REVERSE_INDEX) + const otherIndex = metaData.getIndex(OTHER_INDEX) + const lsi1 = metaData.getIndex(LSI_1) + const lsi2 = metaData.getIndex(LSI_2) + expect(reverseIndex).toEqual({ partitionKey: 'sk', sortKey: 'pk' }) + expect(otherIndex).toEqual({ partitionKey: 'otherId', sortKey: 'sk' }) + expect(lsi1).toEqual({ partitionKey: 'pk', sortKey: 'otherId' }) + expect(lsi2).toEqual({ partitionKey: 'pk', sortKey: 'otherId' }) + }) + }) + describe('enum (no Enum decorator)', () => { let metadata: Metadata @@ -544,17 +617,43 @@ describe('Decorators should add correct metadata', () => { }) describe('should throw when more than one partitionKey was defined in a model', () => { - expect(() => { + it('does so', () => { + expect(() => { + @Model() + class InvalidModel { + @PartitionKey() + partKeyA: string + + @PartitionKey() + partKeyB: string + } + + return new InvalidModel() + }).toThrow() + }) + }) + + describe('decorate property multiple times identically', () => { + let logReceiver: jest.Mock + + beforeEach(() => { + logReceiver = jest.fn() + updateDynamoEasyConfig({ logReceiver }) + }) + + it('should not throw but warn, if the PartitionKey is two times annotated', () => { @Model() - class InvalidModel { + class NotCoolButOkModel { @PartitionKey() - partKeyA: string - @PartitionKey() - partKeyB: string + doppeltGemoppelt: string } - return new InvalidModel() - }).toThrow() + const propertyMetaData = metadataForModel(NotCoolButOkModel).forProperty('doppeltGemoppelt') + expect(propertyMetaData).toBeDefined() + expect(propertyMetaData!.key).toEqual({ type: 'HASH' }) + expect(logReceiver).toBeCalledTimes(1) + expect(logReceiver.mock.calls[0][0].level).toBe(LogLevel.WARNING) + }) }) }) From 4de844bb028cf1b775be709e7968173b25244370 Mon Sep 17 00:00:00 2001 From: Simon Mumenthaler Date: Fri, 11 Oct 2019 01:50:50 +0200 Subject: [PATCH 8/8] test(request): add tests for Put- and TransactGetSingleTableRequest --- src/dynamo/request/put/put.request.spec.ts | 20 +++++++++++--- src/dynamo/request/put/put.request.ts | 1 + .../transact-get-single-table.request.spec.ts | 26 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/dynamo/request/put/put.request.spec.ts b/src/dynamo/request/put/put.request.spec.ts index 6769a0919..c58e6cfe2 100644 --- a/src/dynamo/request/put/put.request.spec.ts +++ b/src/dynamo/request/put/put.request.spec.ts @@ -6,9 +6,13 @@ import { PutRequest } from './put.request' describe('put request', () => { describe('params', () => { - const request = new PutRequest(null, SimpleWithPartitionKeyModel, { - id: 'myId', - age: 45, + let request: PutRequest + + beforeEach(() => { + request = new PutRequest(null, SimpleWithPartitionKeyModel, { + id: 'myId', + age: 45, + }) }) it('constructor', () => { @@ -28,6 +32,16 @@ describe('put request', () => { expect(params.ExpressionAttributeValues).toBeUndefined() }) + it('ifNotExists with false does add the predicate', () => { + // but it also does not remove it. it actually does nothing with false but returning the request instance + request.ifNotExists(false) + + const params: DynamoDB.PutItemInput = request.params + expect(params.ConditionExpression).toBeUndefined() + expect(params.ExpressionAttributeNames).toBeUndefined() + expect(params.ExpressionAttributeValues).toBeUndefined() + }) + it('returnValues', () => { const req = request.returnValues('ALL_OLD') expect(req.params.ReturnValues).toEqual('ALL_OLD') diff --git a/src/dynamo/request/put/put.request.ts b/src/dynamo/request/put/put.request.ts index 36804e55c..23b504a62 100644 --- a/src/dynamo/request/put/put.request.ts +++ b/src/dynamo/request/put/put.request.ts @@ -30,6 +30,7 @@ export class PutRequest extends WriteRequest { if (predicate) { diff --git a/src/dynamo/request/transactgetsingletable/transact-get-single-table.request.spec.ts b/src/dynamo/request/transactgetsingletable/transact-get-single-table.request.spec.ts index 237151c70..6dd97d2ea 100644 --- a/src/dynamo/request/transactgetsingletable/transact-get-single-table.request.spec.ts +++ b/src/dynamo/request/transactgetsingletable/transact-get-single-table.request.spec.ts @@ -1,3 +1,4 @@ +// tslint:disable:no-non-null-assertion import * as DynamoDB from 'aws-sdk/clients/dynamodb' import { SimpleWithPartitionKeyModel } from '../../../../test/models' import { metadataForModel } from '../../../decorator/metadata/metadata-for-model.function' @@ -49,6 +50,10 @@ describe('TransactGetSingleTableRequest', () => { }) }) + it('execNoMap should return the raw value', async () => { + expect(await req.execNoMap()).toEqual(response) + }) + it('execFullResponse should map items and potentially return consumed capacity', async () => { const resp = await req.execFullResponse() expect(resp).toBeDefined() @@ -61,4 +66,25 @@ describe('TransactGetSingleTableRequest', () => { }) }) }) + + describe('with empty response', () => { + const response: DynamoDB.TransactGetItemsOutput = {} + const transactGetItemsSpy = jasmine.createSpy().and.returnValue(Promise.resolve(response)) + beforeEach(() => { + const dynamoDBWrapperMock: DynamoDbWrapper = { transactGetItems: transactGetItemsSpy } + req = new TransactGetSingleTableRequest(dynamoDBWrapperMock, SimpleWithPartitionKeyModel, []) + }) + it('exec returns empty array', async () => { + expect(await req.exec()).toEqual([]) + }) + it('execNoMap returns original response (empty object)', async () => { + expect(await req.execNoMap()).toEqual({}) + }) + it('execFullResponse returns the response with empty items array', async () => { + expect(await req.execFullResponse()).toEqual({ + ConsumedCapacity: undefined, + Items: [], + }) + }) + }) })