Merge pull request #137 from shiftcode/#136-improvements

#136 improvements

Author: michaelwittwer

src/config/config.type.ts

@@ -3,9 +3,24 @@ import { SessionValidityEnsurer } from '../dynamo/session-validity-ensurer.type'
 import { LogReceiver } from '../logger'
 import { MapperForType } from '../mapper'
 
+/**
+ * the global config object
+ */
 export interface Config {
+  /**
+   * function receiving all the log statements
+   */
   logReceiver: LogReceiver
+  /**
+   * mapper used for {@link DateProperty} decorated properties
+   */
   dateMapper: MapperForType<any, any>,
+  /**
+   * function used to create the table names
+   */
   tableNameResolver: TableNameResolver,
+  /**
+   * function called before calling dynamoDB api
+   */
   sessionValidityEnsurer: SessionValidityEnsurer,
 }

src/config/update-config.function.spec.ts

@@ -8,14 +8,37 @@ import { updateDynamoEasyConfig } from './update-config.function'
 describe('updateDynamoEasyConfig', () => {
   afterEach(resetDynamoEasyConfig)
 
-  it('should throw when providing invalid stuff', () => {
+  it('should have defaults', () => {
+    expect(dynamoEasyConfig.logReceiver).toBeDefined()
+    expect(dynamoEasyConfig.dateMapper).toBeDefined()
+    expect(dynamoEasyConfig.tableNameResolver).toBeDefined()
+    expect(dynamoEasyConfig.sessionValidityEnsurer).toBeDefined()
+  })
+
+  it('should throw when providing explicitly undefined, null, or a non-function value', () => {
+    expect(() => updateDynamoEasyConfig({ logReceiver: undefined })).toThrow()
     expect(() => updateDynamoEasyConfig({ logReceiver: <any>null })).toThrow()
+    expect(() => updateDynamoEasyConfig({ logReceiver: <any>'foo' })).toThrow()
+
+    expect(() => updateDynamoEasyConfig({ dateMapper: undefined })).toThrow()
     expect(() => updateDynamoEasyConfig({ dateMapper: <any>null })).toThrow()
+    expect(() => updateDynamoEasyConfig({ dateMapper: <any>'foo' })).toThrow()
+
+    expect(() => updateDynamoEasyConfig({ tableNameResolver: undefined })).toThrow()
+    expect(() => updateDynamoEasyConfig({ tableNameResolver: <any>null })).toThrow()
+    expect(() => updateDynamoEasyConfig({ tableNameResolver: <any>'foo' })).toThrow()
+
+    expect(() => updateDynamoEasyConfig({ sessionValidityEnsurer: undefined })).toThrow()
+    expect(() => updateDynamoEasyConfig({ sessionValidityEnsurer: <any>null })).toThrow()
+    expect(() => updateDynamoEasyConfig({ sessionValidityEnsurer: <any>'foo' })).toThrow()
   })
 
-  it('should have defaults', () => {
+  it('should not override default when updating but not providing a value', () => {
+    updateDynamoEasyConfig({})
     expect(dynamoEasyConfig.logReceiver).toBeDefined()
     expect(dynamoEasyConfig.dateMapper).toBeDefined()
+    expect(dynamoEasyConfig.tableNameResolver).toBeDefined()
+    expect(dynamoEasyConfig.sessionValidityEnsurer).toBeDefined()
   })
 
   it('should work when providing valid stuff', () => {

src/config/update-config.function.ts

@@ -1,17 +1,22 @@
 import { Config } from './config.type'
 import { dynamoEasyConfig } from './dynamo-easy-config'
 
+/**
+ * update the global dynamoEasy configuration.
+ * @see {@link Config}
+ * @param config
+ */
 export function updateDynamoEasyConfig(config: Partial<Config>): void {
-  if (config.logReceiver !== undefined && typeof config.logReceiver !== 'function') {
+  if ('logReceiver' in config && typeof config.logReceiver !== 'function') {
     throw new Error('Config.logReceiver has to be a function')
   }
-  if (config.dateMapper !== undefined && (config.dateMapper === null || typeof config.dateMapper.toDb !== 'function' || typeof config.dateMapper.fromDb !== 'function')) {
+  if ('dateMapper' in config && (!config.dateMapper || typeof config.dateMapper.toDb !== 'function' || typeof config.dateMapper.fromDb !== 'function')) {
     throw new Error('Config.dateMapper must be an object of type MapperForType')
   }
-  if (config.tableNameResolver !== undefined && (config.tableNameResolver === null || typeof  config.tableNameResolver !== 'function')) {
+  if ('tableNameResolver' in config && typeof  config.tableNameResolver !== 'function') {
     throw new Error('Config.tableNameResolver must be function')
   }
-  if (config.sessionValidityEnsurer !== undefined && (config.sessionValidityEnsurer === null || typeof config.sessionValidityEnsurer !== 'function')) {
+  if ('sessionValidityEnsurer' in config && typeof config.sessionValidityEnsurer !== 'function') {
     throw new Error('Config.sessionValidityEnsurer must be function')
   }
   Object.assign(dynamoEasyConfig, config)

src/decorator/decorators.spec.ts

@@ -1,6 +1,7 @@
 // tslint:disable:max-classes-per-file
 // tslint:disable:no-unnecessary-class
 // tslint:disable:no-non-null-assertion
+// tslint:disable:no-unused-variable
 import { getMetaDataProperty } from '../../test/helper'
 import {
   ComplexModel,
@@ -523,4 +524,19 @@ describe('Decorators should add correct metadata', () => {
       expect(meta).toBeDefined()
     })
   })
+
+  describe('should throw when more than one partitionKey was defined in a model', () => {
+    expect(() => {
+      @Model()
+      class InvalidModel {
+        @PartitionKey()
+        partKeyA: string
+
+        @PartitionKey()
+        partKeyB: string
+      }
+
+      return new InvalidModel()
+    }).toThrow()
+  })
 })

src/decorator/impl/key/partition-key.decorator.ts

@@ -13,7 +13,7 @@ export function PartitionKey(): PropertyDecorator {
           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 paritionKey for propety ${propertyKey}`)
+            console.warn(`this is the second execution to define the partitionKey for property ${propertyKey}`)
           } 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: ' +

src/decorator/impl/property/init-or-update-property.function.ts

@@ -46,7 +46,6 @@ function createNewProperty(
     name: propertyKey,
     nameDb: propertyKey,
     typeInfo,
-    // ...mapperOpts,
     ...propertyOptions,
   }
 

src/dynamo/batchget/batch-get-utils.ts

@@ -4,6 +4,14 @@ import { Observable, of } from 'rxjs'
 import { delay, map, mergeMap } from 'rxjs/operators'
 import { DynamoRx } from '../dynamo-rx'
 
+/**
+ * Function which executes batchGetItem operations until all given items (as params) are processed (fetched).
+ * Between each follow-up request (in case of unprocessed items) a delay is interposed calculated by the given backoffTime and throttleTimeSlot.
+ * @param dynamoRx
+ * @param params containing the keys per table to create the batchGet operation
+ * @param backoffTimer used to determine how many time slots the follow-up request should be delayed
+ * @param throttleTimeSlot used to calculate the effective wait time
+ */
 export function batchGetItemsFetchAll(
   dynamoRx: DynamoRx,
   params: DynamoDB.BatchGetItemInput,
@@ -14,16 +22,21 @@ export function batchGetItemsFetchAll(
     .pipe(
       mergeMap(response => {
         if (hasUnprocessedKeys(response)) {
+          // in case of unprocessedItems do a follow-up requests
           return of(response.UnprocessedKeys)
             .pipe(
+              // delay before doing the follow-up request
               delay(backoffTimer.next().value * throttleTimeSlot),
+
               mergeMap((UnprocessedKeys: DynamoDB.BatchGetRequestMap) => {
                 const nextParams = { ...params, RequestItems: UnprocessedKeys }
+                // call recursively batchGetItemsFetchAll with the returned UnprocessedItems params
                 return batchGetItemsFetchAll(dynamoRx, nextParams, backoffTimer, throttleTimeSlot)
               }),
               map(combineBatchGetResponses(response)),
             )
         }
+        // no follow-up request necessary, return result
         return of(response)
       }),
     )

src/dynamo/batchwrite/batch-write-utils.ts

@@ -4,6 +4,14 @@ import { delay, mergeMap } from 'rxjs/operators'
 import { DynamoRx } from '../dynamo-rx'
 
 
+/**
+ * Function which executes batchWriteItem operations until all given items (as params) are processed (written).
+ * Between each follow-up request (in case of unprocessed items) a delay is interposed calculated by the given backoffTime and throttleTimeSlot.
+ * @param dynamoRx
+ * @param params containing the items per table to create the batchWrite operation
+ * @param backoffTimer used to determine how many time slots the follow-up request should be delayed
+ * @param throttleTimeSlot used to calculate the effective wait time
+ */
 export function batchWriteItemsWriteAll(
   dynamoRx: DynamoRx,
   params: DynamoDB.BatchWriteItemInput,
@@ -14,15 +22,21 @@ export function batchWriteItemsWriteAll(
     .pipe(
       mergeMap(response => {
         if (hasUnprocessedItems(response)) {
+          // in case of unprocessedItems do a follow-up requests
           return of(response.UnprocessedItems)
             .pipe(
+              // delay before doing the follow-up request
               delay(backoffTimer.next().value * throttleTimeSlot),
+
               mergeMap((unprocessedKeys: DynamoDB.BatchWriteItemRequestMap) => {
                 const nextParams: DynamoDB.BatchWriteItemInput = { ...params, RequestItems: unprocessedKeys }
+                // call recursively batchWriteItemsWriteAll with the returned UnprocessedItems params
                 return batchWriteItemsWriteAll(dynamoRx, nextParams, backoffTimer, throttleTimeSlot)
               }),
+              // no combining of responses necessary, only the last response is returned
             )
         }
+        // no follow-up request necessary, return result
         return of(response)
       }),
     )

src/dynamo/dynamo-rx.spec.ts

@@ -1,73 +1,115 @@
 // tslint:disable:no-empty
 // tslint:disable:no-unnecessary-callback-wrapper
 
-import { Config, Credentials, DynamoDB } from 'aws-sdk'
-import { EMPTY, Observable } from 'rxjs'
+import { Config, Credentials } from 'aws-sdk'
+import { of } from 'rxjs'
 import { resetDynamoEasyConfig } from '../../test/helper/resetDynamoEasyConfig.function'
 import { updateDynamoEasyConfig } from '../config'
 import { DynamoRx } from './dynamo-rx'
-import { SessionValidityEnsurer } from './session-validity-ensurer.type'
 
 describe('dynamo rx', () => {
-  describe('should call the validity ensurer before each call and return an observable', () => {
+  describe('should call the validity ensurer before each call and call the correct dynamoDB method', () => {
     let dynamoRx: DynamoRx
-    let spyValidityEnsurer: SessionValidityEnsurer
+    let sessionValidityEnsurerSpy: jasmine.Spy
+    let dynamoDbSpy: jasmine.Spy
+    let pseudoParams: any
 
     beforeEach(() => {
-      spyValidityEnsurer = jasmine.createSpy().and.returnValue(EMPTY)
-      updateDynamoEasyConfig({ sessionValidityEnsurer: spyValidityEnsurer })
+      pseudoParams = { TableName: 'tableName', KeyConditionExpression: 'blub' }
+      sessionValidityEnsurerSpy = jasmine.createSpy().and.returnValue(of(true))
+      updateDynamoEasyConfig({ sessionValidityEnsurer: sessionValidityEnsurerSpy })
       dynamoRx = new DynamoRx()
     })
 
-    afterEach(resetDynamoEasyConfig)
+    afterEach(() => {
+      resetDynamoEasyConfig()
+      expect(sessionValidityEnsurerSpy).toHaveBeenCalled()
+      expect(dynamoDbSpy).toHaveBeenCalledTimes(1)
+      expect(dynamoDbSpy).toHaveBeenCalledWith(pseudoParams)
+    })
 
-    it('putItem', () => {
-      expect(dynamoRx.putItem(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+    it('putItem', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'putItem').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.putItem(pseudoParams).toPromise()
     })
-    it('getItem', () => {
-      expect(dynamoRx.getItem(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('getItem', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'getItem').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.getItem(pseudoParams).toPromise()
     })
-    it('updateItem', () => {
-      expect(dynamoRx.updateItem(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('updateItem', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'updateItem').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.updateItem(pseudoParams).toPromise()
     })
-    it('deleteItem', () => {
-      expect(dynamoRx.deleteItem(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('deleteItem', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'deleteItem').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.deleteItem(pseudoParams).toPromise()
     })
-    it('batchWriteItem', () => {
-      expect(dynamoRx.batchWriteItem(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('batchWriteItem', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'batchWriteItem').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.batchWriteItem(pseudoParams).toPromise()
     })
-    it('batchGetItems', () => {
-      expect(dynamoRx.batchGetItems(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('batchGetItems', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'batchGetItem').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.batchGetItems(pseudoParams).toPromise()
     })
-    it('transactWriteItem', () => {
-      expect(dynamoRx.transactWriteItems(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('transactWriteItems', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'transactWriteItems').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.transactWriteItems(pseudoParams).toPromise()
     })
-    it('transactGetItems', () => {
-      expect(dynamoRx.transactGetItems(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('transactGetItems', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'transactGetItems').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.transactGetItems(pseudoParams).toPromise()
     })
-    it('scan', () => {
-      expect(dynamoRx.scan(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+
+    it('scan', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'scan').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.scan(pseudoParams).toPromise()
     })
-    it('query', () => {
-      const params: DynamoDB.QueryInput = {
-        TableName: 'tableName',
-      }
-      expect(dynamoRx.query({ ...params, KeyConditionExpression: 'blub' }) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
-      expect(() => dynamoRx.query(params)).toThrow()
+
+    it('query', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'query').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.query(pseudoParams).toPromise()
     })
-    it('makeRequest', () => {
-      expect(dynamoRx.makeRequest(<any>null) instanceof Observable).toBeTruthy()
-      expect(spyValidityEnsurer).toHaveBeenCalled()
+  })
+
+  describe('makeRequest', async () => {
+    let dynamoRx: DynamoRx
+    let sessionValidityEnsurerSpy: jasmine.Spy
+    let dynamoDbSpy: jasmine.Spy
+    let pseudoParams: any
+
+    beforeEach(() => {
+      pseudoParams = { TableName: 'tableName', KeyConditionExpression: 'blub' }
+      sessionValidityEnsurerSpy = jasmine.createSpy().and.returnValue(of(true))
+      updateDynamoEasyConfig({ sessionValidityEnsurer: sessionValidityEnsurerSpy })
+      dynamoRx = new DynamoRx()
+    })
+
+    afterEach(() => {
+      resetDynamoEasyConfig()
+      expect(sessionValidityEnsurerSpy).toHaveBeenCalled()
+      expect(dynamoDbSpy).toHaveBeenCalledTimes(1)
+      expect(dynamoDbSpy).toHaveBeenCalledWith('pseudoOperation', pseudoParams)
+    })
+
+    it('should call the validity ensurer before each call and call the correct dynamoDB method', async () => {
+      dynamoDbSpy = spyOn(dynamoRx.dynamoDb, 'makeRequest').and.returnValue({ promise: () => Promise.resolve() })
+      await dynamoRx.makeRequest('pseudoOperation', pseudoParams).toPromise()
+    })
+  })
+
+  describe('query', () => {
+    beforeEach(() => {})
+    it('should throw when no KeyConditionExpression was given', () => {
+      const dynamoRx = new DynamoRx()
+      expect(() => dynamoRx.query({ TableName: 'tableName' })).toThrow()
     })
   })
 

src/dynamo/expression/condition-expression-builder.ts

@@ -7,7 +7,7 @@ import { resolveAttributeNames } from './functions/attribute-names.function'
 import { isFunctionOperator } from './functions/is-function-operator.function'
 import { isNoParamFunctionOperator } from './functions/is-no-param-function-operator.function'
 import { operatorParameterArity } from './functions/operator-parameter-arity.function'
-import { uniqAttributeValueName } from './functions/unique-attribute-value-name.function'
+import { uniqueAttributeValueName } from './functions/unique-attribute-value-name.function'
 import { ConditionOperator } from './type/condition-operator.type'
 import { Expression } from './type/expression.type'
 import { validateAttributeType } from './update-expression-builder'
@@ -114,7 +114,7 @@ export function buildFilterExpression(
    * person.age
    */
   const resolvedAttributeNames = resolveAttributeNames(attributePath, propertyMetadata)
-  const valuePlaceholder = uniqAttributeValueName(attributePath, existingValueNames)
+  const valuePlaceholder = uniqueAttributeValueName(attributePath, existingValueNames)
 
   /*
    * build the statement
@@ -204,7 +204,7 @@ function buildBetweenConditionExpression(
   [mappedValue1, mappedValue2]
     .forEach(mv => validateAttributeType('between', mv, 'S', 'N', 'B'))
 
-  const value2Placeholder = uniqAttributeValueName(attributePath, [valuePlaceholder].concat(existingValueNames || []))
+  const value2Placeholder = uniqueAttributeValueName(attributePath, [valuePlaceholder].concat(existingValueNames || []))
 
   const statement = `${namePlaceholder} BETWEEN ${valuePlaceholder} AND ${value2Placeholder}`
   attributeValues[valuePlaceholder] = mappedValue1