From 43bca57d0d0aa645f5ff6bad919123558942b68c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=A9?= Date: Sun, 27 Nov 2022 22:43:41 +0100 Subject: [PATCH] feat: add `@Unique` decorator, support specifying multiple unique indexes on the same attribute (#15342) --- src/associations/belongs-to-many.ts | 4 +- src/associations/belongs-to.ts | 1 + src/decorators/legacy/attribute.ts | 58 ++++++++----- src/decorators/legacy/decorator-utils.ts | 44 ++++++++++ src/decorators/shared/model.ts | 17 ++++ src/model.d.ts | 8 +- src/model.js | 56 ++++++++----- src/utils/format.ts | 3 +- .../associations/belongs-to-many.test.js | 36 -------- test/integration/model/create.test.js | 14 ---- .../unit/associations/belongs-to-many.test.ts | 8 +- test/unit/decorators/attribute.test.ts | 57 ++++++++++++- test/unit/model/define.test.js | 84 ++++++++++++++++++- 13 files changed, 281 insertions(+), 109 deletions(-) create mode 100644 src/decorators/legacy/decorator-utils.ts diff --git a/src/associations/belongs-to-many.ts b/src/associations/belongs-to-many.ts index 382899b7fd63..6113d9bc2d4d 100644 --- a/src/associations/belongs-to-many.ts +++ b/src/associations/belongs-to-many.ts @@ -376,8 +376,8 @@ Add your own primary key to the through model, on different attributes than the uniqueKey = [this.through.model.tableName, ...keys, 'unique'].join('_'); } - this.throughModel.rawAttributes[this.foreignKey].unique = uniqueKey; - this.throughModel.rawAttributes[this.otherKey].unique = uniqueKey; + this.throughModel.rawAttributes[this.foreignKey].unique = [{ name: uniqueKey }]; + this.throughModel.rawAttributes[this.otherKey].unique = [{ name: uniqueKey }]; } this.throughModel.refreshAttributes(); diff --git a/src/associations/belongs-to.ts b/src/associations/belongs-to.ts index 6075ecd6bfa7..ce01a223691e 100644 --- a/src/associations/belongs-to.ts +++ b/src/associations/belongs-to.ts @@ -117,6 +117,7 @@ export class BelongsTo< // for non primary columns. if (target.sequelize.options.dialect === 'db2' && this.target.getAttributes()[this.targetKey].primaryKey !== true) { // TODO: throw instead + // @ts-expect-error this.target.getAttributes()[this.targetKey].unique = true; } diff --git a/src/decorators/legacy/attribute.ts b/src/decorators/legacy/attribute.ts index b23e2f4445c9..24db0e690e3d 100644 --- a/src/decorators/legacy/attribute.ts +++ b/src/decorators/legacy/attribute.ts @@ -4,38 +4,58 @@ import type { ModelAttributeColumnOptions, ModelStatic } from '../../model.js'; import { Model } from '../../model.js'; import { columnToAttribute } from '../../utils/deprecations.js'; import { registerModelAttributeOptions } from '../shared/model.js'; +import type { PropertyOrGetterDescriptor } from './decorator-utils.js'; +import { makeParameterizedPropertyDecorator } from './decorator-utils.js'; -export function Attribute(optionsOrDataType: DataType | ModelAttributeColumnOptions): PropertyDecorator { - return (target: Object, propertyName: string | symbol, propertyDescriptor?: PropertyDescriptor) => { - if (typeof propertyName === 'symbol') { - throw new TypeError('Symbol Model Attributes are not currently supported. We welcome a PR that implements this feature.'); - } +type AttributeDecoratorOption = DataType | Partial | undefined; - annotate( - target, - propertyName, - propertyDescriptor ?? Object.getOwnPropertyDescriptor(target, propertyName), - optionsOrDataType, - ); - }; -} +export const Attribute = makeParameterizedPropertyDecorator(undefined, ( + option: AttributeDecoratorOption, + target: Object, + propertyName: string | symbol, + propertyDescriptor?: PropertyDescriptor, +) => { + if (!option) { + throw new Error('Decorator @Attribute requires an argument'); + } + + annotate(target, propertyName, propertyDescriptor, option); +}); /** * @param optionsOrDataType * @deprecated use {@link Attribute} instead. */ -export function Column(optionsOrDataType: DataType | ModelAttributeColumnOptions): PropertyDecorator { +export function Column(optionsOrDataType: DataType | ModelAttributeColumnOptions): PropertyOrGetterDescriptor { columnToAttribute(); return Attribute(optionsOrDataType); } +type UniqueOptions = NonNullable; + +/** + * Sets the unique option true for annotated property + */ +export const Unique = makeParameterizedPropertyDecorator(true, ( + option: UniqueOptions, + target: Object, + propertyName: string | symbol, + propertyDescriptor?: PropertyDescriptor, +) => { + annotate(target, propertyName, propertyDescriptor, { unique: option }); +}); + function annotate( target: Object, - propertyName: string, + propertyName: string | symbol, propertyDescriptor: PropertyDescriptor | undefined, - optionsOrDataType: ModelAttributeColumnOptions | DataType, + optionsOrDataType: Partial | DataType, ): void { + if (typeof propertyName === 'symbol') { + throw new TypeError('Symbol Model Attributes are not currently supported. We welcome a PR that implements this feature.'); + } + if (typeof target === 'function') { throw new TypeError( `Decorator @Attribute has been used on "${target.name}.${String(propertyName)}", which is static. This decorator can only be used on instance properties, setters and getters.`, @@ -48,7 +68,7 @@ function annotate( ); } - let options: ModelAttributeColumnOptions; + let options: Partial; if (isDataType(optionsOrDataType)) { options = { @@ -58,10 +78,6 @@ function annotate( options = { ...optionsOrDataType }; } - if (!options.type) { - throw new Error(`Decorator @Attribute has been used on "${target.constructor.name}.${String(propertyName)}" but does not specify the data type of the attribute. Please specify a data type.`); - } - if (propertyDescriptor) { if (propertyDescriptor.get) { options.get = propertyDescriptor.get; diff --git a/src/decorators/legacy/decorator-utils.ts b/src/decorators/legacy/decorator-utils.ts new file mode 100644 index 000000000000..862b6c6edd04 --- /dev/null +++ b/src/decorators/legacy/decorator-utils.ts @@ -0,0 +1,44 @@ +export type PropertyOrGetterDescriptor = ( + target: Object, + propertyName: string | symbol, + propertyDescriptor?: PropertyDescriptor, +) => void; + +export interface ParameterizedPropertyDecorator { + (options: T): PropertyOrGetterDescriptor; + + (target: Object, propertyName: string | symbol, propertyDescriptor?: PropertyDescriptor): void; +} + +/** + * Makes a decorator that can optionally receive a parameter + * + * @param defaultValue The value to use if no parameter is provided. + * @param callback The callback that will be executed once the decorator is applied. + */ +export function makeParameterizedPropertyDecorator( + defaultValue: T, + callback: ( + option: T, + target: Object, + propertyName: string | symbol, + propertyDescriptor: PropertyDescriptor | undefined, + ) => void, +): ParameterizedPropertyDecorator { + return function decorator(...args: [options: T] | Parameters) { + if (args.length === 1) { + return function parameterizedDecorator( + target: Object, + propertyName: string | symbol, + propertyDescriptor?: PropertyDescriptor | undefined, + ) { + callback(args[0], target, propertyName, propertyDescriptor ?? Object.getOwnPropertyDescriptor(target, propertyName)); + }; + } + + callback(defaultValue, args[0], args[1], args[2] ?? Object.getOwnPropertyDescriptor(args[0], args[1])); + + // this method only returns something if args.length === 1, but typescript doesn't understand it + return undefined as unknown as PropertyOrGetterDescriptor; + }; +} diff --git a/src/decorators/shared/model.ts b/src/decorators/shared/model.ts index 3088063775d9..0ddbe2d0aaab 100644 --- a/src/decorators/shared/model.ts +++ b/src/decorators/shared/model.ts @@ -121,6 +121,23 @@ export function registerModelAttributeOptions( continue; } + if (optionName === 'unique') { + if (!existingOptions.unique) { + existingOptions.unique = []; + } else if (!Array.isArray(existingOptions.unique)) { + existingOptions.unique = [existingOptions.unique]; + } + + if (Array.isArray(optionValue)) { + existingOptions.unique = [...existingOptions.unique, ...optionValue]; + } else { + // @ts-expect-error -- runtime type checking is enforced by model + existingOptions.unique = [...existingOptions.unique, optionValue]; + } + + continue; + } + // @ts-expect-error if (optionValue === existingOptions[optionName]) { continue; diff --git a/src/model.d.ts b/src/model.d.ts index 3dba8118feed..4270800c7cf2 100644 --- a/src/model.d.ts +++ b/src/model.d.ts @@ -1696,9 +1696,7 @@ export interface ModelAttributeColumnOptions { * composite unique index. If multiple columns have the same string, they will be part of the same unique * index */ - // TODO: unique should accept an array so multiple uniques can be registered - // https://github.com/sequelize/sequelize/issues/15334 - unique?: boolean | string | { name: string, msg: string }; + unique?: AllowArray; /** * If true, this attribute will be marked as primary key @@ -1764,7 +1762,7 @@ export interface ModelAttributeColumnOptions { set?(this: M, val: unknown): void; } -export interface BuiltModelAttributeColumnOptions extends Omit, 'type'> { +export interface BuiltModelAttributeColumnOptions extends Omit, 'type' | 'unique'> { /** * The name of the attribute (JS side). */ @@ -1776,6 +1774,8 @@ export interface BuiltModelAttributeColumnOptions exten type: string | AbstractDataType; references?: ModelAttributeColumnReferencesOptions; + unique?: Array<{ name: string, msg?: string }>; + /** * This attribute was added by sequelize. Do not use! * diff --git a/src/model.js b/src/model.js index 4c0b994e11e7..546fb2b85f6c 100644 --- a/src/model.js +++ b/src/model.js @@ -988,7 +988,7 @@ Specify a different name for either index to resolve this issue.`); } if (attribute.type === undefined) { - throw new Error(`Unrecognized datatype for attribute "${this.name}.${name}"`); + throw new Error(`Attribute "${this.name}.${name}" does not specify its DataType.`); } if (attribute.allowNull !== false && _.get(attribute, 'validate.notNull')) { @@ -1172,36 +1172,46 @@ Specify a different name for either index to resolve this issue.`); } if (Object.prototype.hasOwnProperty.call(definition, 'unique') && definition.unique) { - if (typeof definition.unique === 'string') { - definition.unique = { - name: definition.unique, - }; - } else if (definition.unique === true) { - definition.unique = {}; + if (!Array.isArray(definition.unique)) { + definition.unique = [definition.unique]; } - const index = definition.unique.name && this.uniqueKeys[definition.unique.name] - ? this.uniqueKeys[definition.unique.name] - : { fields: [] }; + for (let i = 0; i < definition.unique.length; i++) { + let unique = definition.unique[i]; - index.fields.push(definition.field); - index.msg = index.msg || definition.unique.msg || null; + if (typeof unique === 'string') { + unique = { + name: unique, + }; + } else if (unique === true) { + unique = {}; + } - // TODO: remove this 'column'? It does not work with composite indexes, and is only used by db2 which should use fields instead. - index.column = name; + definition.unique[i] = unique; - index.customIndex = definition.unique !== true; - index.unique = true; + const index = unique.name && this.uniqueKeys[unique.name] + ? this.uniqueKeys[unique.name] + : { fields: [] }; - if (definition.unique.name) { - index.name = definition.unique.name; - } else { - this._nameIndex(index); - } + index.fields.push(definition.field); + index.msg = index.msg || unique.msg || null; + + // TODO: remove this 'column'? It does not work with composite indexes, and is only used by db2 which should use fields instead. + index.column = name; - definition.unique.name ??= index.name; + index.customIndex = unique !== true; + index.unique = true; - this.uniqueKeys[index.name] = index; + if (unique.name) { + index.name = unique.name; + } else { + this._nameIndex(index); + } + + unique.name ??= index.name; + + this.uniqueKeys[index.name] = index; + } } if (Object.prototype.hasOwnProperty.call(definition, 'validate')) { diff --git a/src/utils/format.ts b/src/utils/format.ts index f3894802c5f7..81958f5d4cc2 100644 --- a/src/utils/format.ts +++ b/src/utils/format.ts @@ -5,7 +5,6 @@ import type { Attributes, BuiltModelAttributeColumnOptions, Model, - ModelAttributeColumnOptions, ModelStatic, WhereOptions, } from '..'; @@ -103,7 +102,7 @@ export function mapWhereFieldNames(where: Record, Model: Model const newWhere: Record = Object.create(null); // TODO: note on 'as any[]'; removing the cast causes the following error on attributeNameOrOperator "TS2538: Type 'symbol' cannot be used as an index type." for (const attributeNameOrOperator of getComplexKeys(where) as any[]) { - const rawAttribute: ModelAttributeColumnOptions | undefined = Model.rawAttributes[attributeNameOrOperator]; + const rawAttribute: BuiltModelAttributeColumnOptions | undefined = Model.rawAttributes[attributeNameOrOperator]; const columnNameOrOperator: PropertyKey = rawAttribute?.field ?? attributeNameOrOperator; diff --git a/test/integration/associations/belongs-to-many.test.js b/test/integration/associations/belongs-to-many.test.js index b6028cc763a5..20f01deff4a1 100644 --- a/test/integration/associations/belongs-to-many.test.js +++ b/test/integration/associations/belongs-to-many.test.js @@ -3320,42 +3320,6 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), () => { expect(ut1).to.have.length(1); expect(ut2).to.have.length(1); }); - - it('create custom unique identifier', async function () { - const UserTasksLong = this.sequelize.define('table_user_task_with_very_long_name', { - id: { - type: DataTypes.INTEGER, - primaryKey: true, - autoIncrement: true, - }, - id_user_very_long_field: { - type: DataTypes.INTEGER(1), - }, - id_task_very_long_field: { - type: DataTypes.INTEGER(1), - }, - }, { - tableName: 'table_user_task_with_very_long_name', - }); - - this.User.belongsToMany(this.Task, { - as: 'MyTasks', - through: { - model: UserTasksLong, - unique: 'custom_user_group_unique', - }, - foreignKey: 'id_user_very_long_field', - otherKey: 'id_task_very_long_field', - inverse: { - as: 'MyUsers', - }, - }); - - await this.sequelize.sync({ force: true }); - - expect(UserTasksLong.rawAttributes.id_user_very_long_field.unique).to.deep.equal({ name: 'custom_user_group_unique' }); - expect(UserTasksLong.rawAttributes.id_task_very_long_field.unique).to.deep.equal({ name: 'custom_user_group_unique' }); - }); }); describe('Association options', () => { diff --git a/test/integration/model/create.test.js b/test/integration/model/create.test.js index 165b78a0c6dc..490f8af7006a 100644 --- a/test/integration/model/create.test.js +++ b/test/integration/model/create.test.js @@ -1175,20 +1175,6 @@ describe(Support.getTestDialectTeaser('Model'), () => { } }); - it('raises an error if you mess up the datatype', function () { - expect(() => { - this.sequelize.define('UserBadDataType', { - activity_date: DataTypes.DATe, - }); - }).to.throw(Error, 'Unrecognized datatype for attribute "UserBadDataType.activity_date"'); - - expect(() => { - this.sequelize.define('UserBadDataType', { - activity_date: { type: DataTypes.DATe }, - }); - }).to.throw(Error, 'Unrecognized datatype for attribute "UserBadDataType.activity_date"'); - }); - it('sets a 64 bit int in bigint', async function () { const User = this.sequelize.define('UserWithBigIntFields', { big: DataTypes.BIGINT, diff --git a/test/unit/associations/belongs-to-many.test.ts b/test/unit/associations/belongs-to-many.test.ts index 7f486b0b1ebe..39b0d719edba 100644 --- a/test/unit/associations/belongs-to-many.test.ts +++ b/test/unit/associations/belongs-to-many.test.ts @@ -968,8 +968,8 @@ describe(getTestDialectTeaser('belongsToMany'), () => { expect(Through === MyGroups.through.model); expect(Object.keys(Through.rawAttributes).sort()).to.deep.equal(['id', 'createdAt', 'updatedAt', 'id_user_very_long_field', 'id_group_very_long_field'].sort()); - expect(Through.rawAttributes.id_user_very_long_field.unique).to.deep.equal({ name: 'table_user_group_with_very_long_name_id_group_very_long_field_id_user_very_long_field_unique' }); - expect(Through.rawAttributes.id_group_very_long_field.unique).to.deep.equal({ name: 'table_user_group_with_very_long_name_id_group_very_long_field_id_user_very_long_field_unique' }); + expect(Through.rawAttributes.id_user_very_long_field.unique).to.deep.equal([{ name: 'table_user_group_with_very_long_name_id_group_very_long_field_id_user_very_long_field_unique' }]); + expect(Through.rawAttributes.id_group_very_long_field.unique).to.deep.equal([{ name: 'table_user_group_with_very_long_name_id_group_very_long_field_id_user_very_long_field_unique' }]); }); it('generates unique identifier with custom name', () => { @@ -1010,8 +1010,8 @@ describe(getTestDialectTeaser('belongsToMany'), () => { expect(MyUsers.through.model === UserGroup); expect(MyGroups.through.model === UserGroup); - expect(UserGroup.rawAttributes.id_user_very_long_field.unique).to.deep.equal({ name: 'custom_user_group_unique' }); - expect(UserGroup.rawAttributes.id_group_very_long_field.unique).to.deep.equal({ name: 'custom_user_group_unique' }); + expect(UserGroup.rawAttributes.id_user_very_long_field.unique).to.deep.equal([{ name: 'custom_user_group_unique' }]); + expect(UserGroup.rawAttributes.id_group_very_long_field.unique).to.deep.equal([{ name: 'custom_user_group_unique' }]); }); }); diff --git a/test/unit/decorators/attribute.test.ts b/test/unit/decorators/attribute.test.ts index 15f55f6c9efa..62abe4389658 100644 --- a/test/unit/decorators/attribute.test.ts +++ b/test/unit/decorators/attribute.test.ts @@ -1,6 +1,6 @@ import type { InferAttributes } from '@sequelize/core'; import { Model, DataTypes } from '@sequelize/core'; -import { Attribute } from '@sequelize/core/decorators-legacy'; +import { Attribute, Unique } from '@sequelize/core/decorators-legacy'; import { expect } from 'chai'; import { sequelize } from '../../support'; @@ -131,7 +131,7 @@ describe(`@Attribute legacy decorator`, () => { expect(User.getAttributes().pk.validate).to.deep.equal({ not: 'abc', is: 'abc' }); }); - it('rejects conflicting getterMethods', () => { + it('rejects conflicting validates', () => { expect(() => { class User extends Model> { @Attribute({ @@ -152,4 +152,57 @@ describe(`@Attribute legacy decorator`, () => { return User; }).to.throw(); }); + + it('merges "unique"', () => { + class User extends Model> { + @Attribute({ + type: DataTypes.STRING, + unique: true, + }) + @Attribute({ + unique: 'firstName-lastName', + }) + @Unique(['firstName-country']) + declare firstName: string; + + @Attribute({ + type: DataTypes.STRING, + unique: 'firstName-lastName', + }) + declare lastName: string; + + @Attribute(DataTypes.STRING) + @Unique('firstName-country') + declare country: string; + } + + sequelize.addModels([User]); + + expect(User.getIndexes()).to.deep.equal([ + { + fields: ['firstName', 'country'], + msg: null, + column: 'country', + customIndex: true, + unique: true, + name: 'firstName-country', + }, + { + fields: ['firstName', 'lastName'], + msg: null, + column: 'lastName', + customIndex: true, + unique: true, + name: 'firstName-lastName', + }, + { + fields: ['firstName'], + msg: null, + column: 'firstName', + customIndex: true, + unique: true, + name: 'users_first_name_unique', + }, + ]); + }); }); diff --git a/test/unit/model/define.test.js b/test/unit/model/define.test.js index e8d40a138fbb..517e927d6d19 100644 --- a/test/unit/model/define.test.js +++ b/test/unit/model/define.test.js @@ -100,6 +100,88 @@ describe(Support.getTestDialectTeaser('Model'), () => { expect(User.options.noPrimaryKey).to.equal(true); }); + it('supports marking an attribute as unique', () => { + const User = current.define('User', { + firstName: { + type: DataTypes.STRING, + unique: true, + }, + }); + + expect(User.getIndexes()).to.deep.equal([{ + fields: ['firstName'], + msg: null, + column: 'firstName', + customIndex: true, + unique: true, + name: 'users_first_name_unique', + }]); + }); + + it('supports marking multiple attributes as composite unique', () => { + const User = current.define('User', { + firstName: { + type: DataTypes.STRING, + unique: 'firstName-lastName', + }, + lastName: { + type: DataTypes.STRING, + unique: 'firstName-lastName', + }, + }); + + expect(User.getIndexes()).to.deep.equal([{ + fields: ['firstName', 'lastName'], + msg: null, + column: 'lastName', + customIndex: true, + unique: true, + name: 'firstName-lastName', + }]); + }); + + it('supports using the same attribute in multiple uniques', () => { + const User = current.define('User', { + firstName: { + type: DataTypes.STRING, + unique: [true, 'firstName-lastName', 'firstName-country'], + }, + lastName: { + type: DataTypes.STRING, + unique: 'firstName-lastName', + }, + country: { + type: DataTypes.STRING, + unique: 'firstName-country', + }, + }); + + expect(User.getIndexes()).to.deep.equal([ + { + fields: ['firstName'], + msg: null, + column: 'firstName', + customIndex: true, + unique: true, + name: 'users_first_name_unique', + }, { + fields: ['firstName', 'lastName'], + msg: null, + column: 'lastName', + customIndex: true, + unique: true, + name: 'firstName-lastName', + }, { + fields: ['firstName', 'country'], + msg: null, + column: 'country', + customIndex: true, + unique: true, + name: 'firstName-country', + }, + ]); + }); + it('should throw when the attribute name is ambiguous with $nested.attribute$ syntax', () => { expect(() => { current.define('foo', { @@ -163,7 +245,7 @@ describe(Support.getTestDialectTeaser('Model'), () => { type: DataTypes.MY_UNKNOWN_TYPE, }, }); - }).to.throw('Unrecognized datatype for attribute "bar.name"'); + }).to.throw('Attribute "bar.name" does not specify its DataType.'); }); it('should throw for notNull validator without allowNull', () => {