From edaf129e9bb3c6edd8b8a2d37206e988bd619298 Mon Sep 17 00:00:00 2001 From: dvictory Date: Sun, 30 Apr 2023 11:31:46 -0500 Subject: [PATCH 1/3] Fix issue with mapped fields and defaults on update --- src/__tests__/entity.update.unit.test.ts | 15 +++++++++++---- src/classes/Entity/Entity.ts | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/__tests__/entity.update.unit.test.ts b/src/__tests__/entity.update.unit.test.ts index cf962080f..f55d1aede 100644 --- a/src/__tests__/entity.update.unit.test.ts +++ b/src/__tests__/entity.update.unit.test.ts @@ -25,6 +25,7 @@ const TestEntity = new Entity({ sort: { type: 'string', sortKey: true }, test_string: { type: 'string', coerce: false, default: 'default string' }, test_string_coerce: { type: 'string' }, + test_number_default_with_map: { type: 'number', map: 'test_mapped_number', default: 0, onUpdate: false }, test_number: { type: 'number', alias: 'count', coerce: false }, test_number_coerce: { type: 'number', default: 0 }, test_boolean: { type: 'boolean', coerce: false }, @@ -193,15 +194,19 @@ describe('update', () => { } = TestEntity.updateParams({ email: 'test-pk', sort: 'test-sk', - test_boolean_default: true + test_boolean_default: true, + test_number_default_with_map: 111 }) expect(ExpressionAttributeNames?.['#test_boolean_default']).toBe('test_boolean_default') expect(ExpressionAttributeValues?.[':test_boolean_default']).toBe(true) + expect(ExpressionAttributeNames?.['#test_mapped_number']).toBe('test_mapped_number') + expect(ExpressionAttributeValues?.[':test_mapped_number']).toBe(111) expect(UpdateExpression).toBe( - 'SET #test_string = if_not_exists(#test_string,:test_string), #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = :test_boolean_default, #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et)' + 'SET #test_string = if_not_exists(#test_string,:test_string), #test_mapped_number = :test_mapped_number, #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = :test_boolean_default, #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et)' ) + }) it('fails when removing fields with default values', () => { @@ -1172,16 +1177,17 @@ describe('update', () => { expect(ExpressionAttributeValues).toHaveProperty(':test_string') }) - it('should keep empty lists if removeNulls is true', () => { + it.only('should keep empty lists if removeNulls is true', () => { const params = TestEntity.updateParams( { email: 'x', sort: 'y', test_list: [] }, ) - expect(params.UpdateExpression).toBe('SET #test_string = if_not_exists(#test_string,:test_string), #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = if_not_exists(#test_boolean_default,:test_boolean_default), #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et), #test_list = :test_list') + expect(params.UpdateExpression).toBe("SET #test_string = if_not_exists(#test_string,:test_string), #test_mapped_number = if_not_exists(#test_mapped_number,:test_mapped_number), #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = if_not_exists(#test_boolean_default,:test_boolean_default), #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et), #test_list = :test_list") expect(params.ExpressionAttributeNames).toEqual({ '#_ct': '_ct', '#_et': '_et', '#_md': '_md', + '#test_mapped_number': 'test_mapped_number', '#test_boolean_default': 'test_boolean_default', '#test_list': 'test_list', '#test_number_coerce': 'test_number_coerce', @@ -1191,6 +1197,7 @@ describe('update', () => { ':_ct': expect.any(String), ':_et': 'TestEntity', ':_md': expect.any(String), + ':test_mapped_number': 0, ':test_boolean_default': false, ':test_list': [], ':test_number_coerce': 0, diff --git a/src/classes/Entity/Entity.ts b/src/classes/Entity/Entity.ts index 7d75333ec..a083a6141 100644 --- a/src/classes/Entity/Entity.ts +++ b/src/classes/Entity/Entity.ts @@ -1178,7 +1178,7 @@ class Entity Date: Sun, 30 Apr 2023 16:09:41 -0500 Subject: [PATCH 2/3] Update test suite to be more specific to the use case. --- src/__tests__/entity.update.unit.test.ts | 40 +++++++++++++++++++----- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/__tests__/entity.update.unit.test.ts b/src/__tests__/entity.update.unit.test.ts index f55d1aede..867bfbd77 100644 --- a/src/__tests__/entity.update.unit.test.ts +++ b/src/__tests__/entity.update.unit.test.ts @@ -25,7 +25,6 @@ const TestEntity = new Entity({ sort: { type: 'string', sortKey: true }, test_string: { type: 'string', coerce: false, default: 'default string' }, test_string_coerce: { type: 'string' }, - test_number_default_with_map: { type: 'number', map: 'test_mapped_number', default: 0, onUpdate: false }, test_number: { type: 'number', alias: 'count', coerce: false }, test_number_coerce: { type: 'number', default: 0 }, test_boolean: { type: 'boolean', coerce: false }, @@ -91,6 +90,17 @@ const TestEntity3 = new Entity({ table: TestTable3 } as const) +const TestEntity4 = new Entity({ + name: 'TestEntity4', + autoExecute: false, + attributes: { + email: { type: 'string', partitionKey: true }, + test_number_default_with_map: { type: 'number', map: 'test_mapped_number', default: 0, onUpdate: false }, + }, + timestamps: false, + table: TestTable3 +} as const) + const TestEntityGSI = new Entity({ name: 'TestEntityGSI', autoExecute: false, @@ -194,21 +204,37 @@ describe('update', () => { } = TestEntity.updateParams({ email: 'test-pk', sort: 'test-sk', - test_boolean_default: true, - test_number_default_with_map: 111 + test_boolean_default: true }) expect(ExpressionAttributeNames?.['#test_boolean_default']).toBe('test_boolean_default') expect(ExpressionAttributeValues?.[':test_boolean_default']).toBe(true) + + expect(UpdateExpression).toBe( + 'SET #test_string = if_not_exists(#test_string,:test_string), #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = :test_boolean_default, #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et)' + ) + }) + + it('allows overriding default field values that use mapping', () => { + const { + UpdateExpression, + ExpressionAttributeNames, + ExpressionAttributeValues + } = TestEntity4.updateParams({ + email: 'test-pk', + test_number_default_with_map: 111 + }) + expect(ExpressionAttributeNames?.['#test_mapped_number']).toBe('test_mapped_number') expect(ExpressionAttributeValues?.[':test_mapped_number']).toBe(111) expect(UpdateExpression).toBe( - 'SET #test_string = if_not_exists(#test_string,:test_string), #test_mapped_number = :test_mapped_number, #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = :test_boolean_default, #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et)' + 'SET #test_mapped_number = :test_mapped_number' ) }) + it('fails when removing fields with default values', () => { expect(() => TestEntity.updateParams({ email: 'test-pk', sort: 'test-sk', $remove: 'test_string' }) @@ -1177,17 +1203,16 @@ describe('update', () => { expect(ExpressionAttributeValues).toHaveProperty(':test_string') }) - it.only('should keep empty lists if removeNulls is true', () => { + it('should keep empty lists if removeNulls is true', () => { const params = TestEntity.updateParams( { email: 'x', sort: 'y', test_list: [] }, ) - expect(params.UpdateExpression).toBe("SET #test_string = if_not_exists(#test_string,:test_string), #test_mapped_number = if_not_exists(#test_mapped_number,:test_mapped_number), #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = if_not_exists(#test_boolean_default,:test_boolean_default), #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et), #test_list = :test_list") + expect(params.UpdateExpression).toBe('SET #test_string = if_not_exists(#test_string,:test_string), #test_number_coerce = if_not_exists(#test_number_coerce,:test_number_coerce), #test_boolean_default = if_not_exists(#test_boolean_default,:test_boolean_default), #_ct = if_not_exists(#_ct,:_ct), #_md = :_md, #_et = if_not_exists(#_et,:_et), #test_list = :test_list') expect(params.ExpressionAttributeNames).toEqual({ '#_ct': '_ct', '#_et': '_et', '#_md': '_md', - '#test_mapped_number': 'test_mapped_number', '#test_boolean_default': 'test_boolean_default', '#test_list': 'test_list', '#test_number_coerce': 'test_number_coerce', @@ -1197,7 +1222,6 @@ describe('update', () => { ':_ct': expect.any(String), ':_et': 'TestEntity', ':_md': expect.any(String), - ':test_mapped_number': 0, ':test_boolean_default': false, ':test_list': [], ':test_number_coerce': 0, From 46df289958760799a6a704f835fd912380c15bb0 Mon Sep 17 00:00:00 2001 From: dvictory Date: Sun, 30 Apr 2023 16:11:49 -0500 Subject: [PATCH 3/3] Use dedicated table object for test. --- src/__tests__/entity.update.unit.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/__tests__/entity.update.unit.test.ts b/src/__tests__/entity.update.unit.test.ts index 867bfbd77..d170c22e3 100644 --- a/src/__tests__/entity.update.unit.test.ts +++ b/src/__tests__/entity.update.unit.test.ts @@ -90,6 +90,13 @@ const TestEntity3 = new Entity({ table: TestTable3 } as const) +const TestTable4 = new Table({ + name: 'test-table4', + partitionKey: 'pk', + entityField: false, + DocumentClient +}) + const TestEntity4 = new Entity({ name: 'TestEntity4', autoExecute: false, @@ -98,7 +105,7 @@ const TestEntity4 = new Entity({ test_number_default_with_map: { type: 'number', map: 'test_mapped_number', default: 0, onUpdate: false }, }, timestamps: false, - table: TestTable3 + table: TestTable4 } as const) const TestEntityGSI = new Entity({