feat(expression-builder): cleanup validation and other

- split validation into two functions one for arity check and one for values, to be ready to add more rules in a clearer scope
- add util to build templates on runtime, to build error messages with ${varName} syntax
- remove unused comparator-operators.const.ts

Author: michaelwittwer

src/dynamo/expression/comparator-operators.const.ts

@@ -2,7 +2,18 @@ import { has } from 'lodash'
 import { ComplexModel } from '../../../test/models'
 import { Model, PartitionKey, Property } from '../../decorator/impl'
 import { metadataForClass } from '../../decorator/metadata'
-import { buildFilterExpression, deepFilter } from './condition-expression-builder'
+import { typeOf } from '../../mapper'
+import {
+  buildFilterExpression,
+  deepFilter,
+  ERR_ARITY_DEFAULT,
+  ERR_ARITY_IN,
+  ERR_VALUES_BETWEEN_TYPE,
+  ERR_VALUES_IN,
+} from './condition-expression-builder'
+import { operatorParameterArity } from './functions/operator-parameter-arity.function'
+import { ConditionOperator } from './type'
+import { dynamicTemplate } from './util'
 
 @Model()
 class MyModel {
@@ -264,24 +275,6 @@ describe('expressions', () => {
       expect(has(condition.attributeValues, ':creationDate_2')).toBeTruthy()
       expect(condition.attributeValues[':creationDate_2']).toEqual({ S: date2.toISOString() })
     })
-
-    it('should throw error for wrong value arity', () => {
-      expect(() => buildFilterExpression('age', 'attribute_type', [], undefined, undefined)).toThrowError(
-        'expected 1 value(s) for operator attribute_type, this is not the right amount of method parameters for this operator',
-      )
-    })
-
-    it('should throw error for wrong value arity', () => {
-      expect(() => buildFilterExpression('age', 'attribute_type', [undefined], undefined, undefined)).toThrowError(
-        'expected 1 value(s) for operator attribute_type, this is not the right amount of method parameters for this operator',
-      )
-    })
-
-    it('should throw error for wrong value type', () => {
-      expect(() => buildFilterExpression('age', 'IN', ['myValue', 'mySecondValue'], undefined, undefined)).toThrowError(
-        'expected 1 value(s) for operator IN, this is not the right amount of method parameters for this operator (IN operator requires one value of array type)',
-      )
-    })
   })
 
   describe('operator nested attributes', () => {
@@ -312,4 +305,38 @@ describe('expressions', () => {
       expect(condition.attributeValues).toEqual({ ':person__birthdays_at_5__year': { N: '2016' } })
     })
   })
+
+  describe('validation', () => {
+    describe('arity', () => {
+      it('should throw default error for wrong arity', () => {
+        const operator: ConditionOperator = 'attribute_type'
+        expect(() => buildFilterExpression('age', operator, [], undefined, undefined)).toThrow(
+          dynamicTemplate(ERR_ARITY_DEFAULT, { parameterArity: operatorParameterArity(operator), operator }),
+        )
+      })
+
+      it('should throw error for wrong IN arity', () => {
+        const operator: ConditionOperator = 'IN'
+        expect(() =>
+          buildFilterExpression('age', operator, ['myValue', 'mySecondValue'], undefined, undefined),
+        ).toThrowError(dynamicTemplate(ERR_ARITY_IN, { parameterArity: operatorParameterArity(operator), operator }))
+      })
+    })
+
+    describe('operator values', () => {
+      it('should throw error for wrong IN values', () => {
+        const operator: ConditionOperator = 'IN'
+        expect(() => buildFilterExpression('age', operator, ['myValue'], undefined, undefined)).toThrowError(
+          ERR_VALUES_IN,
+        )
+      })
+
+      it('should throw error for wrong value type', () => {
+        const operator: ConditionOperator = 'BETWEEN'
+        expect(() => buildFilterExpression('age', operator, ['myValue', 2], undefined, undefined)).toThrowError(
+          dynamicTemplate(ERR_VALUES_BETWEEN_TYPE, { value1: typeOf('myValue'), value2: typeOf(2) }),
+        )
+      })
+    })
+  })
 })

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

@@ -10,7 +10,8 @@ import { operatorParameterArity } from './functions/operator-parameter-arity.fun
 import { uniqAttributeValueName } from './functions/unique-attribute-value-name.function'
 import { ConditionOperator } from './type/condition-operator.type'
 import { Expression } from './type/expression.type'
-import { validateAttributeValue } from './update-expression-builder'
+import { validateAttributeType } from './update-expression-builder'
+import { dynamicTemplate } from './util'
 
 type BuildFilterFn = (
   attributePath: string,
@@ -96,7 +97,7 @@ export function buildFilterExpression(
   values = deepFilter(values, value => value !== undefined)
 
   // check if provided values are valid for given operator
-  validateValues(operator, values)
+  validateForOperator(operator, values)
 
   // load property metadata if model metadata was provided
   let propertyMetadata: PropertyMetadata<any> | undefined
@@ -165,7 +166,7 @@ function buildInConditionExpression(
     .reduce(
       (result, mappedValue: Attribute | null, index: number) => {
         if (mappedValue !== null) {
-          validateAttributeValue('IN condition', mappedValue, 'S', 'N', 'B')
+          validateAttributeType('IN condition', mappedValue, 'S', 'N', 'B')
           result[`${valuePlaceholder}_${index}`] = mappedValue
         }
         return result
@@ -201,7 +202,7 @@ function buildBetweenConditionExpression(
     throw new Error('make sure to provide an actual value for te BETWEEN operator')
   }
   [mappedValue1, mappedValue2]
-    .forEach(mv => validateAttributeValue('between', mv, 'S', 'N', 'B'))
+    .forEach(mv => validateAttributeType('between', mv, 'S', 'N', 'B'))
 
   const value2Placeholder = uniqAttributeValueName(attributePath, [valuePlaceholder].concat(existingValueNames || []))
 
@@ -244,14 +245,14 @@ function buildDefaultConditionExpression(
     const attribute: Attribute | null = toDbOne(values[0], propertyMetadata)
     switch (operator) {
       case 'begins_with':
-        validateAttributeValue(`${operator} condition`, attribute, 'S', 'B')
+        validateAttributeType(`${operator} condition`, attribute, 'S', 'B')
         break
       case 'contains':
       case '<':
       case '<=':
       case '>':
       case '>=':
-        validateAttributeValue(`${operator} condition`, attribute, 'N', 'S', 'B')
+        validateAttributeType(`${operator} condition`, attribute, 'N', 'S', 'B')
         break
     }
 
@@ -271,51 +272,85 @@ function buildDefaultConditionExpression(
  * Every operator requires a predefined arity of parameters, this method checks for the correct arity and throws an Error
  * if not correct
  *
- * @param {any[]} values The values which will be applied to the operator function implementation
+ * @param operator
+ * @param values The values which will be applied to the operator function implementation, not every operator requires values
  * @throws {Error} error Throws an error if the amount of values won't match the operator function parameter arity or
  * the given values is not an array
  */
-function validateValues(operator: ConditionOperator, values?: any[]) {
-  const parameterArity = operatorParameterArity(operator)
+function validateForOperator(operator: ConditionOperator, values?: any[]) {
+  validateArity(operator, values)
+
+  /*
+   * validate values if operator supports values
+   */
+  if (!isFunctionOperator(operator) || isFunctionOperator(operator) && !isNoParamFunctionOperator(operator)) {
+    if (values && Array.isArray(values) && values.length) {
+      validateValues(operator, values)
+    } else {
+      // TODO
+      throw new Error('blub')
+    }
+  }
+}
+
+// tslint:disable:no-invalid-template-strings
+/*
+ * error messages for arity issues
+ */
+export const ERR_ARITY_IN = 'expected ${parameterArity} value(s) for operator ${operator}, this is not the right amount of method parameters for this operator (IN operator requires one value of array type)'
+export const ERR_ARITY_DEFAULT = 'expected ${parameterArity} value(s) for operator ${operator}, this is not the right amount of method parameters for this operator'
+
+// tslint:enable:no-invalid-template-strings
+
+function validateArity(operator: ConditionOperator, values?: any[]) {
   if (values === null || values === undefined) {
     if (isFunctionOperator(operator) && !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`,
-      )
+      throw new Error(dynamicTemplate(ERR_ARITY_DEFAULT, {parameterArity: operatorParameterArity(operator), operator}))
     }
   } else if (values && Array.isArray(values)) {
+    const parameterArity = operatorParameterArity(operator)
     // check for correct amount of values
     if (values.length !== parameterArity) {
       switch (operator) {
         case 'IN':
-          throw new Error(
-            `expected ${parameterArity} value(s) for operator ${operator}, this is not the right amount of method parameters for this operator (IN operator requires one value of array type)`,
-          )
+          throw new Error(dynamicTemplate(ERR_ARITY_IN, { parameterArity, operator }))
         default:
-          throw new Error(
-            `expected ${parameterArity} value(s) for operator ${operator}, this is not the right amount of method parameters for this operator`,
-          )
+          throw new Error(dynamicTemplate(ERR_ARITY_DEFAULT, { parameterArity, operator }))
       }
     }
+  }
+}
 
-    // some additional operator dependent validation
-    switch (operator) {
-      case 'BETWEEN':
-        // values must be the same type
-        if (typeOf(values[0]) !== typeOf(values[1])) {
-          throw new Error(
-            `both values for operator BETWEEN must have the same type, got ${typeOf(values[0])} and ${typeOf(
-              values[1],
-            )}`,
-          )
-        }
-        break
-      case 'IN':
-        if (!Array.isArray(values[0])) {
-          throw new Error('the provided value for IN operator must be an array')
-        }
-    }
+
+/*
+ * error message for wrong operator values
+ */
+// tslint:disable:no-invalid-template-strings
+export const ERR_VALUES_BETWEEN_TYPE = 'both values for operator BETWEEN must have the same type, got ${value1} and ${value2}'
+export const ERR_VALUES_IN = 'the provided value for IN operator must be an array'
+// tslint:enable:no-invalid-template-strings
+
+/**
+ * Every operator has some constraints about the values it supports, this method makes sure everything is fine for given
+ * operator and values
+ */
+function validateValues(operator: ConditionOperator, values: any[]) {
+  // some additional operator dependent validation
+  switch (operator) {
+    case 'BETWEEN':
+      // values must be the same type
+      if (typeOf(values[0]) !== typeOf(values[1])) {
+        throw new Error(dynamicTemplate(
+          ERR_VALUES_BETWEEN_TYPE,
+          { value1: typeOf(values[0]), value2: typeOf(values[1]) }
+        ))
+      }
+      break
+    case 'IN':
+      if (!Array.isArray(values[0])) {
+        throw new Error(ERR_VALUES_IN)
+      }
   }
 }
 

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

@@ -3,7 +3,7 @@ import { isFunctionOperator } from './is-function-operator.function'
 import { isNoParamFunctionOperator } from './is-no-param-function-operator.function'
 
 /**
- * Every expression condition operator has a predefined arity (amount) of function paramers, this method
+ * Every expression condition operator has a predefined arity (amount) of function parameters, this method
  * returns this value
  *
  * @returns {number} The amount of required method parameters when calling an operator function
@@ -27,7 +27,7 @@ export function operatorParameterArity(operator: ConditionOperator): number {
       case 'BETWEEN':
         return 2
       default:
-        throw new Error(`no parameter arity defined for opererator ${operator}`)
+        throw new Error(`no parameter arity defined for operator ${operator}`)
     }
   }
 }

src/dynamo/expression/functions/operator-parameter-arity.function.ts

@@ -1,2 +1,3 @@
 export * from './logical-operator'
 export * from './type'
+export * from './util'

src/dynamo/expression/index.ts

@@ -2,11 +2,6 @@ import { UpdateActionKeyword } from './update-action-keyword.type'
 import { UpdateAction } from './update-action.type'
 
 export class UpdateActionDef {
-  actionKeyword: UpdateActionKeyword
-  action: UpdateAction
-
-  constructor(actionKeyWord: UpdateActionKeyword, action: UpdateAction) {
-    this.actionKeyword = actionKeyWord
-    this.action = action
+  constructor(public actionKeyword: UpdateActionKeyword, public action: UpdateAction) {
   }
 }

src/dynamo/expression/type/update-action-def.ts

@@ -1,6 +1,7 @@
 /**
  *
  * update expressions support these 4 base operations:
+ * https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.UpdateExpressions.html
  *
  * update-expression ::=
  * [ SET action [, action] ... ]

src/dynamo/expression/type/update-action.type.ts

@@ -25,11 +25,13 @@ export interface UpdateFunctions<T, R> {
    * samples:
    * - persons.age
    * - places[0].address.street
+   *
+   * specify ifNotExists to only execute if the property does not exist
    */
   set: (value: T, ifNotExists?: boolean) => R
 
   /**
-   * appends one or more values to the start or end of a list, value must be of type L(ist)
+   * appends one or more values to the start or end of a list, value must map to ListAttribute
    */
   appendToList: (value: T | Array<ExtractListType<T>> | Set<ExtractListType<T>>, position?: 'START' | 'END') => R
 
@@ -45,7 +47,7 @@ export interface UpdateFunctions<T, R> {
 
   /**
    * adds or manipulates a value to an attribute of type N(umber) or S(et), manipulation behaviour differs based on attribute type
-   * for numbers AWS generally recommends to use SET rather than ADD. See incrementBy and decrementBy
+   * for numbers AWS generally recommends to use SET rather than ADD. See incrementBy and decrementBy.
    *
    * @param values {multiple values as Array | Set}
    *

src/dynamo/expression/type/update-expression-definition-chain.ts

@@ -30,6 +30,7 @@ describe('buildUpdateExpression', () => {
       expect(() => buildUpdateExpression('age', op, ['notANumber'], [], metaDataS)).toThrow()
     })
   })
+
   describe('decrementBy', () => {
     const op = new UpdateActionDef('SET', 'decrementBy')
     it('should build expression', () => {
@@ -80,6 +81,7 @@ describe('buildUpdateExpression', () => {
     it('should throw when not number or a set value', () => {
       expect(() => buildUpdateExpression('age', op, ['notANumber'], [], metaDataS)).toThrow()
     })
+
     it('should throw when no value for attributeValue was given', () => {
       expect(() => buildUpdateExpression('age', op, [], [], metaDataS)).toThrow()
     })

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

@@ -81,14 +81,15 @@ function buildDefaultExpression(
     }
   }
 
+  // see update-expression-definition-chain.ts for action definitions
   let statement: string
   switch (operator.action) {
     case 'incrementBy':
-      validateAttributeValue(operator.action, attribute, 'N')
+      validateAttributeType(operator.action, attribute, 'N')
       statement = `${namePlaceholder} = ${namePlaceholder} + ${valuePlaceholder}`
       break
     case 'decrementBy':
-      validateAttributeValue(operator.action, attribute, 'N')
+      validateAttributeType(operator.action, attribute, 'N')
       statement = `${namePlaceholder} = ${namePlaceholder} - ${valuePlaceholder}`
       break
     case 'set':
@@ -112,11 +113,11 @@ function buildDefaultExpression(
       statement = values.map(pos => `${namePlaceholder}[${pos}]`).join(', ')
       break
     case 'add':
-      validateAttributeValue(operator.action, attribute, 'N', 'SS', 'NS', 'BS')
+      validateAttributeType(operator.action, attribute, 'N', 'SS', 'NS', 'BS')
       statement = `${namePlaceholder} ${valuePlaceholder}`
       break
     case 'removeFromSet':
-      validateAttributeValue(operator.action, attribute, 'SS', 'NS', 'BS')
+      validateAttributeType(operator.action, attribute, 'SS', 'NS', 'BS')
       statement = `${namePlaceholder} ${valuePlaceholder}`
       break
     default:
@@ -139,12 +140,12 @@ function isNoAttributeValueAction(action: UpdateAction) {
   )
 }
 
-export function validateAttributeValue(name: string, attributeValue: Attribute | null, ...allowedTypes: AttributeType[]) {
-  if (attributeValue === null || attributeValue === undefined) {
+export function validateAttributeType(name: string, attribute: Attribute | null, ...allowedTypes: AttributeType[]) {
+  if (attribute === null || attribute === undefined) {
     throw new Error(`${name} requires an attributeValue of ${allowedTypes.join(', ')} but non was given`)
   }
-  const key = <AttributeType>Object.keys(attributeValue)[0]
+  const key = <AttributeType>Object.keys(attribute)[0]
   if (!allowedTypes.includes(key)) {
-    throw new Error(`Type ${key} of ${JSON.stringify(attributeValue)} is not allowed for ${name}. Valid types are: ${allowedTypes.join('. ')}`)
+    throw new Error(`Type ${key} of ${JSON.stringify(attribute)} is not allowed for ${name}. Valid types are: ${allowedTypes.join('. ')}`)
   }
 }