Merge pull request #218 from shiftcode/#216-condition-builder

fix(merge-conditions): no redundant parentheses

Author: Simon Mumenthaler

src/dynamo/expression/create-if-not-exists-condition.function.spec.ts

@@ -35,7 +35,7 @@ describe('create ifNotExistsCondition', () => {
     const condition = and(...res)(undefined, metaSimple)
     addExpression('ConditionExpression', condition, paramsSimple)
 
-    expect(paramsSimple.ConditionExpression).toBe('(attribute_not_exists (#id))')
+    expect(paramsSimple.ConditionExpression).toBe('attribute_not_exists (#id)')
     expect(paramsSimple.ExpressionAttributeNames).toEqual({ '#id': 'id' })
     expect(paramsSimple.ExpressionAttributeValues).toBeUndefined()
   })

src/dynamo/expression/logical-operator/merge-conditions.function.spec.ts

@@ -0,0 +1,30 @@
+import { attribute } from './attribute.function'
+import { mergeConditions } from './merge-conditions.function'
+
+describe('mergeCondition statements', () => {
+  test('no redundant parentheses for single condition', () => {
+    const conditionDefinitionFns = [attribute('name').beginsWith('sample')]
+
+    const conditions = mergeConditions('OR', conditionDefinitionFns)
+    const expression = conditions(undefined, undefined)
+    expect(expression.statement).toEqual('begins_with (#name, :name)')
+  })
+
+  test('no redundant parentheses for multiple condition', () => {
+    const conditionDefinitionFns = [attribute('name').beginsWith('sample'), attribute('fullName').beginsWith('sample')]
+
+    const conditions = mergeConditions('OR', conditionDefinitionFns)
+    const expression = conditions(undefined, undefined)
+
+    expect(expression.statement).toEqual('(begins_with (#name, :name) OR begins_with (#fullName, :fullName))')
+  })
+
+  test('no redundant parentheses for single condition combined', () => {
+    const conditionDefinitionFns = [attribute('name').beginsWith('sample'), attribute('fullName').beginsWith('sample')]
+
+    const conditions = mergeConditions('OR', conditionDefinitionFns)
+    const andConditions = mergeConditions('AND', [conditions])
+    const expression = andConditions(undefined, undefined)
+    expect(expression.statement).toEqual('(begins_with (#name, :name) OR begins_with (#fullName, :fullName))')
+  })
+})

src/dynamo/expression/logical-operator/merge-conditions.function.ts

@@ -57,7 +57,8 @@ export function mergeConditions(
       statements.push(condition.statement)
     })
 
-    mergedCondition.statement = `(${statements.join(' ' + operator + ' ')})`
+
+    mergedCondition.statement = statements.length === 1 ? statements[0] : `(${statements.join(' ' + operator + ' ')})`
     return mergedCondition
   }
 }

src/dynamo/expression/param-util.spec.ts

@@ -39,7 +39,7 @@ describe('ParamUtils', () => {
       ':c': { N: '3' },
       ':c_2': { N: '5' },
     })
-    expect(params.ConditionExpression).toBe('((#a = :a AND (#b = :b OR #c BETWEEN :c AND :c_2)))')
+    expect(params.ConditionExpression).toBe('(#a = :a AND (#b = :b OR #c BETWEEN :c AND :c_2))')
   })
 
   it('should build correct UpdateExpression', () => {

src/dynamo/expression/prepare-and-add-update-expressions.function.spec.ts

@@ -354,7 +354,7 @@ describe('PrepareExpressions function', () => {
           ':name': { S: 'newName' },
           ':topics': { S: 'otherTopic' },
         })
-        expect(params.ConditionExpression).toBe('(NOT contains (#topics, :topics))')
+        expect(params.ConditionExpression).toBe('NOT contains (#topics, :topics)')
       })
 
       it('with name conflicting where clause', () => {
@@ -378,7 +378,7 @@ describe('PrepareExpressions function', () => {
           ':topics': { SS: ['myTopic'] },
           ':topics_2': { S: 'otherTopic' },
         })
-        expect(params.ConditionExpression).toBe('(NOT contains (#topics, :topics_2))')
+        expect(params.ConditionExpression).toBe('NOT contains (#topics, :topics_2)')
       })
     })
 

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

@@ -23,7 +23,7 @@ describe('put request', () => {
       request.ifNotExists()
 
       const params: DynamoDB.PutItemInput = request.params
-      expect(params.ConditionExpression).toBe('(attribute_not_exists (#id))')
+      expect(params.ConditionExpression).toBe('attribute_not_exists (#id)')
       expect(params.ExpressionAttributeNames).toEqual({ '#id': 'id' })
       expect(params.ExpressionAttributeValues).toBeUndefined()
     })

src/dynamo/request/read-many.request.spec.ts

@@ -119,14 +119,14 @@ describe('ReadManyRequest', () => {
       expect(request.params.ExpressionAttributeNames).toEqual({ '#age': 'age' })
       expect(request.params.ExpressionAttributeValues).toEqual({ ':age': { N: '20' } })
     })
+
     it('where', () => {
-      request.where(or(attribute('age').lt(10), attribute('age').gt(20)))
-      expect(request.params.FilterExpression).toEqual('((#age < :age OR #age > :age_2))')
-      expect(request.params.ExpressionAttributeNames).toEqual({ '#age': 'age' })
-      expect(request.params.ExpressionAttributeValues).toEqual({
-        ':age': { N: '10' },
-        ':age_2': { N: '20' },
-      })
+      const conditions = or(attribute('age').lt(10), attribute('age').gt(20))
+      const expression = conditions(undefined, undefined)
+      request.where(conditions)
+      expect(request.params.FilterExpression).toEqual(expression.statement)
+      expect(request.params.ExpressionAttributeNames).toEqual(expression.attributeNames)
+      expect(request.params.ExpressionAttributeValues).toEqual(expression.attributeValues)
     })
   })
 

src/dynamo/request/write.request.spec.ts

@@ -81,7 +81,7 @@ describe('write request', () => {
 
     it('[onlyIf] should set condition', () => {
       req.onlyIf(or(attribute('age').lt(10), attribute('age').gt(20)))
-      expect(req.params.ConditionExpression).toEqual('((#age < :age OR #age > :age_2))')
+      expect(req.params.ConditionExpression).toEqual('(#age < :age OR #age > :age_2)')
       expect(req.params.ExpressionAttributeNames).toEqual({ '#age': 'age' })
       expect(req.params.ExpressionAttributeValues).toEqual({
         ':age': { N: '10' },

src/dynamo/transactwrite/transact-base-operation.spec.ts

@@ -43,7 +43,7 @@ describe('TransactBaseOperation', () => {
       op.onlyIf(attribute('age').gt(20))
       expect(op.params).toEqual({
         TableName: 'simple-with-partition-key-models',
-        ConditionExpression: '(#age > :age)',
+        ConditionExpression: '#age > :age',
         ExpressionAttributeNames: { '#age': 'age' },
         ExpressionAttributeValues: { ':age': { N: '20' } },
       })

src/dynamo/transactwrite/transact-put.spec.ts

@@ -46,7 +46,7 @@ describe('TransactPut', () => {
     expect(op.params).toEqual({
       TableName: getTableName(UpdateModel),
       Item: toDb(item, UpdateModel),
-      ConditionExpression: '(attribute_not_exists (#id))',
+      ConditionExpression: 'attribute_not_exists (#id)',
       ExpressionAttributeNames: { '#id': 'id' },
     })
   })