From 0782d9d28b0786fba11bd76891d997037006a540 Mon Sep 17 00:00:00 2001 From: Harold Howe Date: Sat, 11 Mar 2023 13:41:56 -0600 Subject: [PATCH 1/2] Make assertion errors in build.test fail the test if they occur. Treat single column unique indexes as a primary key if no other primary key was found. Fixes #638 and implement #639. --- src/auto-builder.ts | 26 ++++++++++++++++++++------ test/build.test.js | 6 +++++- test/helpers.js | 3 ++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/auto-builder.ts b/src/auto-builder.ts index 831f602e..ab8dcda6 100644 --- a/src/auto-builder.ts +++ b/src/auto-builder.ts @@ -1,5 +1,5 @@ import _ from "lodash"; -import { Dialect, QueryInterface, QueryTypes, Sequelize } from "sequelize"; +import { ColumnsDescription, Dialect, QueryInterface, QueryTypes, Sequelize } from 'sequelize'; import { AutoOptions } from "."; import { ColumnElementType, ColumnPrecision, DialectOptions, FKRow, FKSpec, TriggerCount } from "./dialects/dialect-options"; import { dialects } from "./dialects/dialects"; @@ -187,12 +187,11 @@ export class AutoBuilder { this.tableData.indexes[makeTableQName(table)] = await this.queryInterface.showIndex( { tableName: table.table_name, schema: table.table_schema }) as IndexSpec[]; - // if there is no primaryKey, and `id` field exists, then make id the primaryKey (#480) + // if there is no primaryKey, and a single column unique index exists, or an `id` field exists, then make that field the primaryKey (#480) if (!_.some(fields, { primaryKey: true })) { - const idname = _.keys(fields).find(f => f.toLowerCase() === 'id'); - const idfield = idname && fields[idname]; - if (idfield) { - idfield.primaryKey = true; + const pkField = this.getUniqueIndexField(fields, this.tableData.indexes[makeTableQName(table)]) || this.getIdField(fields); + if (pkField) { + pkField.primaryKey = true; } } @@ -207,6 +206,21 @@ export class AutoBuilder { } } + private getUniqueIndexField(fields: ColumnsDescription, tableIndexes: IndexSpec[]) { + const uniqueIndex = tableIndexes.find(index => index.unique && index.fields.length === 1 ); + if(uniqueIndex) { + const indexColumnName = uniqueIndex.fields[0].attribute.toLowerCase(); + const fieldName = _.keys(fields).find(f => f.toLowerCase() === indexColumnName); + return fieldName && fields[fieldName]; + } + } + + + private getIdField(fields: ColumnsDescription) { + const idname = _.keys(fields).find(f => f.toLowerCase() === 'id'); + return idname && fields[idname]; + } + private executeQuery(query: string): Promise { return this.sequelize.query(query, { type: QueryTypes.SELECT, diff --git a/test/build.test.js b/test/build.test.js index a1e8280c..b5fa67bb 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -37,8 +37,9 @@ describe(helpers.getTestDialectTeaser("sequelize-auto build"), function() { auto.build().then(tableData => { callback(tableData); - }).finally(() => { done(); + }).catch((e) => { + done(e); }) } @@ -71,6 +72,9 @@ describe(helpers.getTestDialectTeaser("sequelize-auto build"), function() { const fkParents = foreignKeys[isSnakeTables ? 'parents': 'Parents']; const fkKids = foreignKeys[isSnakeTables ? 'kids': 'Kids']; + const paranoidPkColumn = tables[isSnakeTables ? 'paranoid_users': 'ParanoidUsers'].username; + expect(paranoidPkColumn.primaryKey).to.equal(true); + if (helpers.getTestDialect() === "sqlite") { expect(foreignKeys).to.have.keys(['Kids', 'Parents', 'ParanoidUsers']); expect(fkParanoidUsers).to.include.keys(['UserId']); diff --git a/test/helpers.js b/test/helpers.js index 2250d352..77bb1d3f 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -66,7 +66,7 @@ module.exports = { }); test.ParanoidUser = test.sequelize.define('ParanoidUser', { - username: { type: Sequelize.STRING } + username: { type: Sequelize.STRING, unique: true } }, { paranoid: true, @@ -74,6 +74,7 @@ module.exports = { } ); + test.ParanoidUser.removeAttribute('id'); test.ParanoidUser.belongsTo(test.User); // test data for relationships across schemas From 83ea216763840d4cf6edd393e62feb54cfb2bf84 Mon Sep 17 00:00:00 2001 From: Harold Howe Date: Sat, 11 Mar 2023 15:12:07 -0600 Subject: [PATCH 2/2] Require that the unique column not allow nulls #639. --- src/auto-builder.ts | 5 ++++- test/helpers.js | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/auto-builder.ts b/src/auto-builder.ts index ab8dcda6..a077dd16 100644 --- a/src/auto-builder.ts +++ b/src/auto-builder.ts @@ -211,7 +211,10 @@ export class AutoBuilder { if(uniqueIndex) { const indexColumnName = uniqueIndex.fields[0].attribute.toLowerCase(); const fieldName = _.keys(fields).find(f => f.toLowerCase() === indexColumnName); - return fieldName && fields[fieldName]; + const field = fieldName && fields[fieldName]; + if(field && !field.allowNull) { + return field; + } } } diff --git a/test/helpers.js b/test/helpers.js index 77bb1d3f..a7e98782 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -66,7 +66,7 @@ module.exports = { }); test.ParanoidUser = test.sequelize.define('ParanoidUser', { - username: { type: Sequelize.STRING, unique: true } + username: { type: Sequelize.STRING, unique: true, allowNull: false } }, { paranoid: true,