feat(expressionBuilder): update filtering to be deep and walk the object tree to filter out undefine

Author: michaelwittwer

README.md

@@ -1,6 +1,5 @@
 # Dynamo-Easy
 [![Travis](https://img.shields.io/travis/shiftcode/dynamo-easy.svg)](https://travis-ci.org/shiftcode/dynamo-easy)
-[![Coverage Status](https://img.shields.io/coveralls/jekyll/jekyll.svg)](https://coveralls.io/github/shiftcode/dynamo-easy?branch=master)
 [![Coverage Status](https://coveralls.io/repos/github/shiftcode/dynamo-easy/badge.svg?branch=master)](https://coveralls.io/github/shiftcode/dynamo-easy?branch=master)
 [![Dev Dependencies](https://img.shields.io/david/expressjs/express.svg)](https://david-dm.org/michaelwittwer/dynamo-easy?type=dev)
 [![Greenkeeper badge](https://badges.greenkeeper.io/alexjoverm/typescript-library-starter.svg)](https://greenkeeper.io/)
@@ -153,7 +152,7 @@ Enum values are persisted as Numbers (index of enum).
 
 # Request API
 To start making requests create an instance of [DynamoStore](https://shiftcode.github.io/dynamo-easy/classes/_dynamo_dynamo_store_.dynamostore.html) and execute the desired operation using the provided api.
-We support all the common dynamodb operations:
+We support the following dynamodb operations with a fluent api:
 
 - Put
 - Get

src/decorator/impl/property/property.decorator.ts

@@ -55,7 +55,7 @@ export function initOrUpdateIndex(indexType: IndexType, indexData: IndexData, ta
 
 function initOrUpdateGSI(indexes: { [key: string]: KeyType }, indexData: IndexData): Partial<PropertyMetadata<any>> {
   if (indexes[indexData.name]) {
-    // TODO when we throw an error we have a problem where multiple different classes extend one base class, this will be executed by multiple times
+    // TODO LOW:INVESTIGATE when we throw an error we have a problem where multiple different classes extend one base class, this will be executed by multiple times
     // throw new Error(
     //   'the property with name is already registered as key for index - one property can only define one key per index'
     // )
@@ -145,7 +145,7 @@ function createNewProperty(
 }
 
 /**
- * TODO BINARY make sure to implement the context dependant details of Binary (Buffer vs. Uint8Array)
+ * TODO LOW:BINARY make sure to implement the context dependant details of Binary (Buffer vs. Uint8Array)
  * @returns {boolean} true if the type cannot be mapped by dynamo document client
  */
 function isCustomType(type: AttributeModelTypes): boolean {

src/decorator/metadata/property-metadata.model.ts

@@ -5,7 +5,6 @@ import { ModelConstructor } from '../../model/model-constructor'
 
 export interface TypeInfo {
   type: ModelConstructor<any>
-  // TODO define what custom means, maybe remove it
   // true if we use a non native type for dynamo document client
   isCustom?: boolean
   genericType?: ModelConstructor<any>

src/dynamo-easy.ts

@@ -1,11 +1,6 @@
 //
 // Reflect Metadata
 //
-//
-// MomentJs locales
-//
-// TODO MOMENT we should import other locals (should we just import all locales for now?)
-import 'moment/locale/de-ch'
 import 'reflect-metadata'
 //
 // RxJs

src/dynamo/dynamo-store.spec.ts

@@ -10,28 +10,30 @@ class DynamoStoreModel {}
 class DynamoStoreModel2 {}
 
 describe('dynamo store', () => {
-  it('correct table name (default)', () => {
-    const dynamoStore = new DynamoStore(DynamoStoreModel)
+  describe('table name', () => {
+    it('correct table name (default)', () => {
+      const dynamoStore = new DynamoStore(DynamoStoreModel)
 
-    expect(dynamoStore.tableName).toBe('dynamo-store-models')
-  })
+      expect(dynamoStore.tableName).toBe('dynamo-store-models')
+    })
 
-  it('correct table name ()', () => {
-    const dynamoStore = new DynamoStore(DynamoStoreModel2)
+    it('correct table name ()', () => {
+      const dynamoStore = new DynamoStore(DynamoStoreModel2)
 
-    expect(dynamoStore.tableName).toBe('myTableName')
-  })
+      expect(dynamoStore.tableName).toBe('myTableName')
+    })
 
-  it('correct table name ()', () => {
-    const dynamoStore = new DynamoStore(DynamoStoreModel2, tableName => `${tableName}-with-special-thing`)
+    it('correct table name ()', () => {
+      const dynamoStore = new DynamoStore(DynamoStoreModel2, tableName => `${tableName}-with-special-thing`)
 
-    expect(dynamoStore.tableName).toBe('myTableName-with-special-thing')
-  })
+      expect(dynamoStore.tableName).toBe('myTableName-with-special-thing')
+    })
 
-  it('throw error because table name is invalid', () => {
-    expect(() => {
-      // tslint:disable-next-line:no-unused-expression
-      new DynamoStore(DynamoStoreModel2, tableName => `${tableName}$`)
-    }).toThrowError()
+    it('throw error because table name is invalid', () => {
+      expect(() => {
+        // tslint:disable-next-line:no-unused-expression
+        new DynamoStore(DynamoStoreModel2, tableName => `${tableName}$`)
+      }).toThrowError()
+    })
   })
 })

src/dynamo/dynamo-store.ts

@@ -73,53 +73,37 @@ export class DynamoStore<T> {
     return this.dynamoRx.makeRequest(operation, params)
   }
 
-  /*
-   * some methods which simplify calls which are usually often used
-   * TODO review the methods
-   */
-  /**
-   * executes a dynamoDB.batchGetItem for multiple keys or a operation
-   */
-  byKeys(keys: any[]): Observable<T[]> {
-    return this.findByMultipleKeys(keys)
-  }
-
-  findAll(): Observable<T[]> {
-    return this.scan().exec()
-  }
-
-  // TODO how does this work when we work with composite primary key
-  private findByMultipleKeys(keys: any[]): Observable<T[]> {
-    const requestItems: { [nameDb: string]: { Keys: DynamoDB.AttributeMap[] } } = {}
-    const attributeMaps: DynamoDB.AttributeMap[] = []
-    keys.forEach(id => {
-      // TODO add support for secondary index
-      const idOb: DynamoDB.AttributeMap = {}
-      const value = Mapper.toDbOne(id)
-      if (value === null) {
-        throw Error('please provide an actual value for partition key')
-      }
-
-      idOb[MetadataHelper.get(this.modelClazz).getPartitionKey()] = value
-      attributeMaps.push(idOb)
-    })
-
-    requestItems[this.tableName] = {
-      Keys: attributeMaps,
-    }
-
-    const params: DynamoDB.BatchGetItemInput = {
-      RequestItems: requestItems,
-    }
-
-    return this.dynamoRx.batchGetItems(params).map(response => {
-      if (response.Responses && Object.keys(response.Responses).length) {
-        return response.Responses[this.tableName].map(attributeMap => Mapper.fromDb(attributeMap, this.modelClazz))
-      } else {
-        return []
-      }
-    })
-  }
+  // TODO implement BatchGetItem request (think about support for secondary indexes)
+  // batchGetItems(keys: any[]): Observable<T[]> {
+  //   const requestItems: { [nameDb: string]: { Keys: DynamoDB.AttributeMap[] } } = {}
+  //   const attributeMaps: DynamoDB.AttributeMap[] = []
+  //   keys.forEach(id => {
+  //     const idOb: DynamoDB.AttributeMap = {}
+  //     const value = Mapper.toDbOne(id)
+  //     if (value === null) {
+  //       throw Error('please provide an actual value for partition key')
+  //     }
+  //
+  //     idOb[MetadataHelper.get(this.modelClazz).getPartitionKey()] = value
+  //     attributeMaps.push(idOb)
+  //   })
+  //
+  //   requestItems[this.tableName] = {
+  //     Keys: attributeMaps,
+  //   }
+  //
+  //   const params: DynamoDB.BatchGetItemInput = {
+  //     RequestItems: requestItems,
+  //   }
+  //
+  //   return this.dynamoRx.batchGetItems(params).map(response => {
+  //     if (response.Responses && Object.keys(response.Responses).length) {
+  //       return response.Responses[this.tableName].map(attributeMap => Mapper.fromDb(attributeMap, this.modelClazz))
+  //     } else {
+  //       return []
+  //     }
+  //   })
+  // }
 
   private createBaseParams(): { TableName: string } {
     const params: { TableName: string } = {

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

@@ -19,6 +19,23 @@ class MyModel {
 }
 
 describe('expressions', () => {
+  it('deep filter', () => {
+    const arr = [5, 'bla', undefined]
+    const obj = [
+      { street: 'street', zip: 1524 },
+      undefined,
+      [undefined, { name: undefined, age: 25 }],
+      [undefined, undefined, {}],
+      {},
+      [],
+      { blub: undefined, other: undefined },
+      new Set(arr),
+    ]
+
+    const filteredObj = ConditionExpressionBuilder.deepFilter(obj, item => item !== undefined)
+    expect(filteredObj).toEqual([{ street: 'street', zip: 1524 }, [{ age: 25 }], new Set([arr[0], arr[1]])])
+  })
+
   it('use property metadata', () => {
     const condition = ConditionExpressionBuilder.buildFilterExpression(
       'prop',

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

@@ -1,5 +1,5 @@
 import { AttributeMap, AttributeValue } from 'aws-sdk/clients/dynamodb'
-import { curryRight } from 'lodash'
+import { curryRight, forEach, isPlainObject } from 'lodash'
 import { Metadata } from '../../decorator/metadata/metadata'
 import { PropertyMetadata } from '../../decorator/metadata/property-metadata.model'
 import { Mapper } from '../../mapper/mapper'
@@ -17,6 +17,53 @@ import { Expression } from './type/expression.type'
  * see http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ConditionExpressions.html
  */
 export class ConditionExpressionBuilder {
+  /**
+   * Will walk the object tree recursively and removes all items which do not satisfy the filterFn
+   * @param obj
+   * @param {(value: any) => boolean} filterFn
+   * @returns {any}
+   */
+  static deepFilter(obj: any, filterFn: (value: any) => boolean): any | null {
+    if (Array.isArray(obj)) {
+      const returnArr: any[] = []
+      obj.forEach(i => {
+        const item = ConditionExpressionBuilder.deepFilter(i, filterFn)
+        if (item !== null) {
+          returnArr.push(item)
+        }
+      })
+
+      return returnArr.length ? returnArr : null
+    } else if (obj instanceof Set) {
+      const returnArr: any[] = []
+      Array.from(<Set<any>>obj).forEach(i => {
+        const item = ConditionExpressionBuilder.deepFilter(i, filterFn)
+        if (item !== null) {
+          returnArr.push(item)
+        }
+      })
+
+      return returnArr.length ? new Set(returnArr) : null
+    } else if (isPlainObject(obj)) {
+      const returnObj: { [key: string]: any } = {}
+
+      forEach(obj, (value: any, key: string) => {
+        const item = ConditionExpressionBuilder.deepFilter(value, filterFn)
+        if (item !== null) {
+          returnObj[key] = item
+        }
+      })
+
+      return Object.keys(returnObj).length ? returnObj : null
+    } else {
+      if (filterFn(obj)) {
+        return obj
+      } else {
+        return null
+      }
+    }
+  }
+
   /**
    * Will create a condition which can be added to a request using the param object.
    * It will create the expression statement and the attribute names and values.
@@ -35,10 +82,9 @@ export class ConditionExpressionBuilder {
     existingValueNames: string[] | undefined,
     metadata: Metadata<any> | undefined
   ): Expression {
-    // TODO investigate is there a use case for undefined desired to be a value
+    // TODO LOW:INVESTIGATE is there a use case for undefined desired to be a value
     // get rid of undefined values
-    // TODO should this not be a deep filter?
-    values = values.filter(value => value !== undefined)
+    values = ConditionExpressionBuilder.deepFilter(values, value => value !== undefined)
 
     // check if provided values are valid for given operator
     ConditionExpressionBuilder.validateValues(operator, values)
@@ -202,9 +248,16 @@ export class ConditionExpressionBuilder {
    * the given values is not an array
    */
   private static validateValues(operator: ConditionOperator, values?: any[]) {
-    if (values && Array.isArray(values)) {
+    const parameterArity = operatorParameterArity(operator)
+    if (values === null || values === undefined) {
+      if (!isNoParamFunctionOperator(operator)) {
+        // the operator needs some values to work
+        throw new Error(
+          `expected ${parameterArity} value(s) for operator ${operator}, this is not the right amount of method parameters for this operator`
+        )
+      }
+    } else if (values && Array.isArray(values)) {
       // check for correct amount of values
-      const parameterArity = operatorParameterArity(operator)
       if (values.length !== parameterArity) {
         switch (operator) {
           case 'IN':
@@ -231,8 +284,6 @@ export class ConditionExpressionBuilder {
           }
           break
       }
-    } else {
-      throw new Error('values must be of type Array')
     }
   }
 

src/dynamo/expression/logical-operator/update.function.ts

@@ -1,9 +1,9 @@
-/**
- * Use this method when accesing a top level attribute of a model
- */
 import { RequestExpressionBuilder } from '../request-expression-builder'
 import { UpdateExpressionDefinitionChain } from '../type/update-expression-definition-chain'
 
+/**
+ * Use this method when accesing a top level attribute of a model
+ */
 export function update<T>(attributePath: keyof T): UpdateExpressionDefinitionChain
 
 /**