test(misc): remove TODOs and update tests

Author: michaelwittwer

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

@@ -103,8 +103,6 @@ function createNewProperty(
   let propertyType: AttributeModelType = getMetadataType(target, propertyKey)
   let customType = isCustomType(propertyType)
 
-  // FIXME model metadata is not accessible here, need to know the model data for metadatintorspection
-  // let modelMetadata: ModelMetadata = Reflect.getMetadata(KEY_MODEL, target.constructor);
   const typeByConvention = Util.typeByConvention(propertyKey)
   if (typeByConvention) {
     customType = true
@@ -147,8 +145,7 @@ function createNewProperty(
 }
 
 /**
- * FIXME fix typing
- * FIXME make sure to implement the context dependant details of Binary (Buffer vs. Uint8Array)
+ * TODO 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,7 @@ import { ModelConstructor } from '../../model/model-constructor'
 
 export interface TypeInfo {
   type: ModelConstructor<any>
-  // TODO define what custom means
+  // 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

@@ -4,7 +4,7 @@
 //
 // MomentJs locales
 //
-// FIXME we should import other locals (should we just import all locales for now?)
+// TODO MOMENT we should import other locals (should we just import all locales for now?)
 import 'moment/locale/de-ch'
 import 'reflect-metadata'
 //

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

@@ -8,7 +8,6 @@ import { ConditionExpressionBuilder } from './condition-expression-builder'
 
 @Model()
 class MyModel {
-  // FIXME TEST if the db name is used for requests
   @Property({ name: 'myId' })
   @PartitionKey()
   id: string
@@ -260,7 +259,7 @@ describe('expressions', () => {
       )
       expect(condition.statement).toBe('#myCollection IN (:myCollection)')
       expect(condition.attributeNames).toEqual({ '#myCollection': 'myCollection' })
-      // TODO review not sure if the attribute should be L(ist) or S(et) or if both are supported, they both executed successfuly against dynamodb when testing but with wrong result
+      // TODO review not sure if the attribute should be L(ist) or S(et) or if both are supported, they both executed successfuly against dynamodb when testing but with unexpected result
       // expect(condition.attributeValues).toEqual({
       //   ':myCollection': { L: [{ S: 'myCollection' }, { S: 'myOtherValue' }] },
       // })

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

@@ -14,7 +14,6 @@ import { ConditionOperator } from './type/condition-operator.type'
 import { Expression } from './type/expression.type'
 
 /**
- * TODO complete doc
  * see http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ConditionExpressions.html
  */
 export class ConditionExpressionBuilder {
@@ -38,6 +37,7 @@ export class ConditionExpressionBuilder {
   ): Expression {
     // TODO 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)
 
     // check if provided values are valid for given operator

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

@@ -104,7 +104,6 @@ export class RequestExpressionBuilder {
     return RequestExpressionBuilder.addSortKeyCondition(keyName, request, metadata).equals(keyValue)
   }
 
-  // FIXME attributePath could also be a simple string for nested paths
   static updateDefinitionFunction(attributePath: string): UpdateExpressionDefinitionChain
   static updateDefinitionFunction<T>(attributePath: keyof T): UpdateExpressionDefinitionChain
 
@@ -147,7 +146,7 @@ export class RequestExpressionBuilder {
    * for all the values included in operators
    *
    * @param {(operator: ConditionOperator) => any} impl The function which is called with the operator and returns a function which expects the value
-   * for the condition. when executed the implementation defines what todo with the condition, just return it for example or add the condition to the request
+   * for the condition. when executed the implementation defines what to do with the condition, just return it for example or add the condition to the request
    * parameters as another example
    */
   private static createUpdateFunctions<T>(impl: (operation: UpdateActionDef) => any): T {
@@ -166,7 +165,7 @@ export class RequestExpressionBuilder {
    * for all the values included in operators
    *
    * @param {(operator: ConditionOperator) => any} impl The function which is called with the operator and returns a function which expects the value
-   * for the condition. when executed the implementation defines what todo with the condition, just return it for example or add the condition to the request
+   * for the condition. when executed the implementation defines what to do with the condition, just return it for example or add the condition to the request
    * parameters as another example
    */
   private static createConditionFunctions<T>(

src/dynamo/request/put/put.request.ts

@@ -23,7 +23,6 @@ export class PutRequest<T> extends BaseRequest<T, any> {
    */
   ifNotExists(predicate?: boolean): PutRequest<T> {
     // FIXME should we check for sort key too?
-    // FIXME add test for predicate
     if (predicate === undefined || (predicate !== undefined && predicate === true)) {
       this.whereAttribute(this.metaData.getPartitionKey()).null()
     }

src/dynamo/request/query/query.request.ts

@@ -77,32 +77,6 @@ export class QueryRequest<T> extends Request<T, QueryRequest<T>, QueryInput, Que
     return this
   }
 
-  /**
-   * multiple conditions will be combined using the AND operator by default
-   * @param {Expression[]} conditions
-   * @returns {QueryRequest<T>}
-   *
-   * TODO remove after investigation
-   */
-  // implementation with overload won't work perfectly with ide support, so we add two different methods
-  // property(keyName: keyof T): RequestConditionFunction<QueryRequest<T>>
-  // property(...conditionDefFns: ConditionExpressionDefinitionFunction[]): QueryRequest<T>
-  //
-  // // (keyof T)[] | ConditionExpressionDefinitionFunction[]
-  // property(...args: any[]): RequestConditionFunction<QueryRequest<T>> | QueryRequest<T> {
-  //   if (args.length === 1 && typeof args[0] === 'string') {
-  //
-  //   } else {
-  //     const conditions: Expression[] = args.map((conditionDefFn: ConditionExpressionDefinitionFunction) => {
-  //       return conditionDefFn(undefined, this.metaData)
-  //     })
-  //
-  //     const condition = and(...conditions)
-  //     ParamUtil.addFilterExpression(condition, this.params)
-  //     return this
-  //   }
-  // }
-
   ascending(): QueryRequest<T> {
     this.params.ScanIndexForward = true
     return this
@@ -130,12 +104,9 @@ export class QueryRequest<T> extends Request<T, QueryRequest<T>, QueryInput, Que
   }
 
   exec(): Observable<T[]> {
-    return (
-      this.dynamoRx
-        .query(this.params)
-        // TODO check if Items is always defined
-        .map(response => response.Items!.map(item => Mapper.fromDb(item, this.modelClazz)))
-    )
+    return this.dynamoRx
+      .query(this.params)
+      .map(response => response.Items!.map(item => Mapper.fromDb(item, this.modelClazz)))
   }
 
   execSingle(): Observable<T | null> {

src/dynamo/request/update/update.request.ts

@@ -22,7 +22,6 @@ import { BaseRequest } from '../base.request'
 
 export type Bla = { [key in UpdateActionKeyword]: Expression[] }
 
-// TODO add if no operations are defined, don't execute
 export class UpdateRequest<T> extends BaseRequest<T, any> {
   constructor(
     dynamoRx: DynamoRx,
@@ -74,33 +73,6 @@ export class UpdateRequest<T> extends BaseRequest<T, any> {
     return this
   }
 
-  /*
-   * update()
-   * //////////// SET ACTION ////////////
-   *  - add one or more attributes to an item
-   *  .set('attrPath', value)
-   *  - increment / decrement number value (incrementBy(), decrementBy())
-   *  .set('attr', value) SET Attr = Attr +- :value
-   *  - add a new list / map
-   *  .set('attrPath', collectionValue)
-   *  - add elements to a list
-   *  .set('listAttrPath', itemValue)
-   *  - add nested map attributes
-   *  .set('', )
-   * ///////////  /////////////
-   *
-   *  //
-   *  .
-   *  //
-   *  .
-   *  //
-   *  .
-   *
-   */
-
-  /**
-   *
-   */
   operations(...updateDefFns: UpdateExpressionDefinitionFunction[]): UpdateRequest<T> {
     if (updateDefFns && updateDefFns.length) {
       const sortedByActionKeyWord: Bla = updateDefFns
@@ -119,11 +91,6 @@ export class UpdateRequest<T> extends BaseRequest<T, any> {
           <Bla>{}
         )
 
-      /*
-       * {
-       *  SET: [updateExpression, updateExpression]
-       * }
-       */
       const actionStatements: string[] = []
       let attributeValues: AttributeMap = {}
       let attributeNames: { [key: string]: string } = {}
@@ -177,7 +144,6 @@ export class UpdateRequest<T> extends BaseRequest<T, any> {
   }
 
   exec(): Observable<void> {
-    // TODO maybe we should map the returned Attributes to the given model type, needs some more investigation
     return this.dynamoRx.updateItem(this.params).map(response => {
       return
     })

src/mapper/for-type/collection.mapper.ts

@@ -95,7 +95,7 @@ export class CollectionMapper implements MapperForType<any[] | Set<any>> {
         propertyValue = Array.from(propertyValue)
       }
 
-      // empty value is not allowed for Set and List (TODO check if true for lists)
+      // empty value is not allowed for S(et) (supported for L(ist))
       if (propertyValue.length === 0) {
         return null
       } else {