Skip to content

Fix issue with Unique Records not being removed when the unique fields are value fields #510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 33 additions & 21 deletions src/Model.js
Original file line number Diff line number Diff line change
Expand Up @@ -797,8 +797,16 @@ export class Model {
/* eslint-disable-next-line */
;({properties, params} = this.checkArgs(properties, params, {exists: true, parse: true, high: true}))
if (this.hasUniqueFields) {
let hasUniqueProperties = Object.entries(properties).find((pair) => {
return this.block.fields[pair[0]] && this.block.fields[pair[0]].unique
const newParams = {...params}
let newProperties = {...properties}
newProperties = this.prepareProperties('update', newProperties, newParams)
const fieldNames = Object.keys(newProperties)
if (newParams.remove) {
fieldNames.push(...newParams.remove)
}

let hasUniqueProperties = fieldNames.find((fieldName) => {
return this.block.fields[fieldName] && this.block.fields[fieldName].unique
})
if (hasUniqueProperties) {
return await this.updateUnique(properties, params)
Expand All @@ -825,12 +833,15 @@ export class Model {
let index = this.indexes.primary
let {hash, sort} = index

params.prepared = properties = this.prepareProperties('update', properties, params)

const newParams = {...params}
let newProperties = {...properties}
newProperties = this.prepareProperties('update', newProperties, newParams)
let keys = {
[index.hash]: properties[index.hash],
[index.hash]: newProperties[index.hash],
}
if (index.sort) {
keys[index.sort] = properties[index.sort]
keys[index.sort] = newProperties[index.sort]
}
/*
Get the prior item so we know the previous unique property values so they can be removed.
Expand All @@ -839,8 +850,8 @@ export class Model {
let prior = await this.get(keys, {hidden: true})
if (prior) {
prior = this.prepareProperties('update', prior)
} else if (params.exists === undefined || params.exists == true) {
throw new OneTableError('Cannot find existing item to update', {properties, code: 'NotFoundError'})
} else if (newParams.exists === undefined || newParams.exists == true) {
throw new OneTableError('Cannot find existing item to update', {newProperties, code: 'NotFoundError'})
}
/*
Create all required unique properties. Remove prior unique properties if they have changed.
Expand All @@ -850,19 +861,19 @@ export class Model {
)

for (let field of fields) {
let toBeRemoved = params.remove && params.remove.includes(field.name)
let isUnchanged = prior && properties[field.name] === prior[field.name]
let toBeRemoved = newParams.remove && newParams.remove.includes(field.name)
let isUnchanged = prior && newProperties[field.name] === prior[field.name]
if (isUnchanged) {
continue
}
let scope = ''
if (field.scope) {
scope = this.runTemplate(null, null, field, properties, params, field.scope) + '#'
scope = this.runTemplate(null, null, field, newProperties, newParams, field.scope) + '#'
}
let pk = `_unique#${scope}${this.name}#${field.attribute}#${properties[field.name]}`
let pk = `_unique#${scope}${this.name}#${field.attribute}#${newProperties[field.name]}`
let sk = `_unique#`
// If we had a prior value AND value is changing or being removed, remove old value
if (prior && prior[field.name] && (properties[field.name] !== undefined || toBeRemoved)) {
if (prior && prior[field.name] && (newProperties[field.name] !== undefined || toBeRemoved)) {
/*
Remove prior unique properties if they have changed and create new unique property.
*/
Expand All @@ -876,21 +887,21 @@ export class Model {
{
transaction,
exists: null,
execute: params.execute,
log: params.log,
execute: newParams.execute,
log: newParams.log,
}
)
}
// If value is changing, add new unique value
if (properties[field.name] !== undefined) {
if (newProperties[field.name] !== undefined) {
await this.schema.uniqueModel.create(
{[this.hash]: pk, [this.sort]: sk},
{
transaction,
exists: false,
return: 'NONE',
log: params.log,
execute: params.execute,
log: newParams.log,
execute: newParams.execute,
}
)
}
Expand Down Expand Up @@ -925,10 +936,10 @@ export class Model {
}
if (params.return == 'get') {
return await this.get(keys, {
hidden: params.hidden,
log: params.log,
parse: params.parse,
execute: params.execute,
hidden: newParams.hidden,
log: newParams.log,
parse: newParams.parse,
execute: newParams.execute,
})
}
if (this.table.warn !== false) {
Expand Down Expand Up @@ -1038,6 +1049,7 @@ export class Model {
}
}
}

properties = this.prepareProperties('update', properties, params)
let expression = new Expression(this, 'update', properties, params)
return await this.run('update', expression)
Expand Down
5 changes: 5 additions & 0 deletions test/schemas/uniqueSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ export default {
sk: {type: String, value: '${_type}#'},
name: {type: String},
email: {type: String, unique: true, required: true},
otherEmail: {type: String},
phone: {type: String, unique: true},
age: {type: Number},
code: {type: String},
deletedAt: {type: Date},
interpolated: {type: String, value: '${name}#${email}', unique: true},
uniqueValueFunction: {type: String, value: true, unique: true},
uniqueValueTemplate: {type: String, value: '${code}', unique: true}
},
} as const,
params: {},
Expand Down
112 changes: 97 additions & 15 deletions test/unique.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,35 @@ import {OneTableError} from '../src'

// jest.setTimeout(7200 * 1000)

function valueGenerator(
model,
fieldName,
properties,
) {
// Unique Email
if (fieldName === 'uniqueValueFunction') {
// If the item is deleted then remove the uniqueEmail
if (properties.deletedAt) {
return null;
}

// If no email then there is no change to uniqueEmail field
if (!properties.otherEmail) {
return undefined;
}

// Set uniqueEmail as the email address
return properties.otherEmail;
}
}

const table = new Table({
name: 'UniqueTestTable',
client: Client,
partial: false,
schema: UniqueSchema,
logger: true,
value: valueGenerator,
})

type UserType = Entity<typeof UniqueSchema.models.User>
Expand All @@ -29,18 +52,22 @@ test('Create Table', async () => {
expect(await table.exists()).toBe(true)
}
User = table.getModel('User')

let items = await table.scanItems()
expect(items.length).toBe(0)
})

test('Create user 1', async () => {
const props = {
name: 'Peter Smith',
email: '[email protected]',
otherEmail: '[email protected]',
}
user = await User.create(props)
expect(user).toMatchObject(props)

let items = await table.scanItems()
expect(items.length).toBe(3)
expect(items.length).toBe(4)

let pk = (() => {
if (isV3()) {
Expand All @@ -67,7 +94,7 @@ test('Create user 2', async () => {
expect(user).toMatchObject(props)

let items = await table.scanItems()
expect(items.length).toBe(7)
expect(items.length).toBe(8)
})

test('Update user 2 with the same email', async () => {
Expand All @@ -79,7 +106,7 @@ test('Update user 2 with the same email', async () => {
expect(user).toMatchObject(props)

let items = await table.scanItems()
expect(items.length).toBe(7)
expect(items.length).toBe(8)
})

test('Update user 2 with unique email', async () => {
Expand All @@ -91,7 +118,7 @@ test('Update user 2 with unique email', async () => {
expect(user).toMatchObject(props)

let items = await table.scanItems()
expect(items.length).toBe(7)
expect(items.length).toBe(8)
})

test('Update non-unique property', async () => {
Expand All @@ -103,7 +130,7 @@ test('Update non-unique property', async () => {
expect(user).toMatchObject(props)

let items = await table.scanItems()
expect(items.length).toBe(7)
expect(items.length).toBe(8)
})

test('Update with unknown property', async () => {
Expand All @@ -117,7 +144,7 @@ test('Update with unknown property', async () => {
expect(user).toMatchObject(expectedProps)

let items = await table.scanItems()
expect(items.length).toBe(7)
expect(items.length).toBe(8)
})

test('Update to remove optional unique property', async () => {
Expand All @@ -131,7 +158,7 @@ test('Update to remove optional unique property', async () => {
expect(user.phone).toBeUndefined()

let items = await table.scanItems()
expect(items.length).toBe(6)
expect(items.length).toBe(7)
})

test('Create non-unique email', async () => {
Expand All @@ -143,52 +170,107 @@ test('Create non-unique email', async () => {
user = await User.create(props)
}).rejects.toThrow(
new OneTableError(
`Cannot create unique attributes "email, phone, interpolated" for "User". An item of the same name already exists.`,
`Cannot create unique attributes "email, phone, interpolated, uniqueValueFunction, uniqueValueTemplate" for "User". An item of the same name already exists.`,
{
code: 'UniqueError',
}
)
)

let items = await table.scanItems()
expect(items.length).toBe(6)
expect(items.length).toBe(7)
})

test('Update non-unique email', async () => {
const props = {
let props = {
name: 'Judy Smith',
email: '[email protected]',
}
await expect(async () => {
await User.update(props, {return: 'none'})
}).rejects.toThrow(
new OneTableError(
`Cannot update unique attributes "email, phone, interpolated" for "User". An item of the same name already exists.`,
`Cannot update unique attributes "email, phone, interpolated, uniqueValueFunction, uniqueValueTemplate" for "User". An item of the same name already exists.`,
{
code: 'UniqueError',
}
)
)

let items = await table.scanItems()
expect(items.length).toBe(7)
})

test('Update to remove uniqueValueFunction unique record', async () => {
// Update the user to trigger the uniqueValueFunction to be removed
let props = {
name: 'Peter Smith',
deletedAt: new Date(),
}
await User.update(props, {return: 'none'})

let items = await table.scanItems()
expect(items.length).toBe(6)

// Create a new user with the same emails
const createProps = {
name: 'Another Peter Smith',
email: '[email protected]',
otherEmail: '[email protected]'
}
user = await User.create(createProps)

items = await table.scanItems()
expect(items.length).toBe(10)
})

test('Update to remove uniqueValueTemplate unique record', async () => {
// Create a user with a "code" that will set the uniqueValueTemplate
let props = {
name: 'John Smith',
email: '[email protected]',
code: '12345678',
}
await User.create(props, {return: 'none'})

let items = await table.scanItems()
expect(items.length).toBe(14)

// Update the user's code
let updateProps = {
name: 'John Smith',
code: '87654321',
}
await User.update(updateProps, {return: 'none'})
items = await table.scanItems()

// Create a new user with the same code
const createProps = {
name: 'Jane Doe',
email: '[email protected]',
code: '12345678',
}
user = await User.create(createProps)

items = await table.scanItems()
expect(items.length).toBe(18)
})

test('Remove user 1', async () => {
users = await User.scan()
expect(users.length).toBe(2)
expect(users.length).toBe(5)

await User.remove(users[0])
users = await User.scan()
expect(users.length).toBe(1)
expect(users.length).toBe(4)

let items = await table.scanItems()
expect(items.length).toBe(3)
expect(items.length).toBe(15)
})

test('Remove all users', async () => {
users = await User.scan({})
expect(users.length).toBe(1)
expect(users.length).toBe(4)
for (let user of users) {
await User.remove(user)
}
Expand Down