diff --git a/packages/core/database/src/__tests__/sync/primary-key.test.ts b/packages/core/database/src/__tests__/sync/primary-key.test.ts index a43caae11..ea7e0c710 100644 --- a/packages/core/database/src/__tests__/sync/primary-key.test.ts +++ b/packages/core/database/src/__tests__/sync/primary-key.test.ts @@ -75,6 +75,7 @@ describe('primary key', () => { await assertPrimaryKey('someField', true); }); }); + describe.skipIf(process.env['DB_DIALECT'] === 'sqlite')('primary key not in sqlite', () => { let db: Database; diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 9d9506463..39430e401 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -357,10 +357,6 @@ export class Collection< ); } - if (this.options.autoGenId !== false && options.primaryKey) { - this.model.removeAttribute('id'); - } - this.removeField(name); this.fields.set(name, field); this.emit('field.afterAdd', field); diff --git a/packages/core/database/src/sync-runner.ts b/packages/core/database/src/sync-runner.ts index b681f8a96..acb5a4830 100644 --- a/packages/core/database/src/sync-runner.ts +++ b/packages/core/database/src/sync-runner.ts @@ -12,6 +12,8 @@ export class SyncRunner { private readonly database: Database; private tableDescMap = {}; + private uniqueAttributes: string[] = []; + constructor(private model: typeof Model) { this.collection = model.collection; this.database = model.database; @@ -63,6 +65,16 @@ export class SyncRunner { throw e; } + try { + const beforeColumns = await this.queryInterface.describeTable(this.tableName, options); + await this.handlePrimaryKeyBeforeSync(beforeColumns, options); + await this.handleUniqueFieldBeforeSync(beforeColumns, options); + } catch (e) { + if (!e.message.includes('No description found')) { + throw e; + } + } + const syncResult = await this.performSync(options); const columns = await this.queryInterface.describeTable(this.tableName, options); @@ -73,11 +85,48 @@ export class SyncRunner { return syncResult; } - async handlePrimaryKey(columns, options) { - if (!this.database.inDialect('postgres')) { + async handleUniqueFieldBeforeSync(beforeColumns, options) { + if (!this.database.inDialect('sqlite')) { return; } + // find new attributes with unique true + const newAttributes = Object.keys(this.rawAttributes).filter((key) => { + return !Object.keys(beforeColumns).includes(this.rawAttributes[key].field) && this.rawAttributes[key].unique; + }); + this.uniqueAttributes = newAttributes; + + // set unique false for new attributes to skip sequelize sync error + for (const newAttribute of newAttributes) { + this.rawAttributes[newAttribute].unique = false; + } + } + + async handlePrimaryKeyBeforeSync(columns, options) { + const columnsBePrimaryKey = Object.keys(columns) + .filter((key) => { + return columns[key].primaryKey == true; + }) + .sort(); + + const columnsWillBePrimaryKey = Object.keys(this.rawAttributes) + .filter((key) => { + return this.rawAttributes[key].primaryKey == true; + }) + .map((key) => { + return this.rawAttributes[key].field; + }) + .sort(); + + if (columnsBePrimaryKey.length == 1 && !columnsWillBePrimaryKey.includes(columnsBePrimaryKey[0])) { + // remove primary key + if (this.database.inDialect('mariadb', 'mysql')) { + await this.sequelize.query(`ALTER TABLE ${this.collection.quotedTableName()} DROP PRIMARY KEY;`, options); + } + } + } + + async handlePrimaryKey(columns, options) { try { const columnsBePrimaryKey = Object.keys(columns) .filter((key) => { @@ -95,22 +144,32 @@ export class SyncRunner { .sort(); if (columnsWillBePrimaryKey.length == 0) { - // skip if no primary key return; } - if (JSON.stringify(columnsBePrimaryKey) != JSON.stringify(columnsWillBePrimaryKey)) { - await this.queryInterface.addConstraint(this.tableName, { - type: 'primary key', - fields: columnsWillBePrimaryKey, - name: `${this.collection.tableName()}_${columnsWillBePrimaryKey.join('_')}_pk`, - transaction: options?.transaction, - }); + if ( + columnsWillBePrimaryKey.length == 1 && + JSON.stringify(columnsBePrimaryKey) != JSON.stringify(columnsWillBePrimaryKey) + ) { + if (this.database.inDialect('mariadb', 'mysql')) { + await this.sequelize.query( + `ALTER TABLE ${this.collection.quotedTableName()} ADD PRIMARY KEY (${columnsWillBePrimaryKey[0]});`, + options, + ); + } else { + await this.queryInterface.addConstraint(this.tableName, { + type: 'primary key', + fields: columnsWillBePrimaryKey, + name: `${this.collection.tableName()}_${columnsWillBePrimaryKey.join('_')}_pk`, + transaction: options?.transaction, + }); + } } } catch (e) { if (e.message.includes('No description found')) { return; } + throw e; } } @@ -176,6 +235,10 @@ export class SyncRunner { } async handleUniqueIndex(options) { + for (const uniqueAttribute of this.uniqueAttributes) { + this.rawAttributes[uniqueAttribute].unique = true; + } + const existsIndexes: any = await this.queryInterface.showIndex(this.collection.getTableNameWithSchema(), options); const existsUniqueIndexes = existsIndexes.filter((index) => index.unique); @@ -225,6 +288,7 @@ export class SyncRunner { await this.queryInterface.addIndex(this.tableName, [this.rawAttributes[uniqueAttribute].field], { unique: true, transaction: options?.transaction, + name: `${this.collection.tableName()}_${this.rawAttributes[uniqueAttribute].field}_uk`, }); } } diff --git a/packages/core/server/src/__tests__/gateway.test.ts b/packages/core/server/src/__tests__/gateway.test.ts index 5aabc0288..630323d8b 100644 --- a/packages/core/server/src/__tests__/gateway.test.ts +++ b/packages/core/server/src/__tests__/gateway.test.ts @@ -196,12 +196,16 @@ describe('gateway', () => { plugins: ['nocobase'], }); await waitSecond(); + await app.runAsCLI(['install'], { from: 'user', + throwError: true, }); + await app.runAsCLI(['start'], { from: 'user', }); + await waitSecond(); clearMessages(); }); diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/primary-key.test.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/primary-key.test.ts index 4984eefed..4f3f8efb6 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/primary-key.test.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/primary-key.test.ts @@ -1,4 +1,4 @@ -import { Database } from '@nocobase/database'; +import { Database, HasManyRepository } from '@nocobase/database'; import { MockServer } from '@nocobase/test'; import { createApp } from './index'; @@ -45,4 +45,161 @@ describe('primary key test', function () { const errorMessage = response.body.errors[0].message; expect(errorMessage).toContain('already has primary key'); }); + + it('should throw error when update field in collection that already has primary key', async () => { + await db.getRepository('collections').create({ + values: { + name: 'posts', + fields: [ + { + name: 'id', + type: 'bigInt', + primaryKey: true, + }, + { + name: 'title', + type: 'string', + }, + { + name: 'name', + type: 'string', + }, + ], + }, + context: {}, + }); + + let err; + try { + await db.getRepository('collections.fields', 'posts').update({ + filterByTk: 'title', + values: { + primaryKey: true, + }, + context: {}, + }); + } catch (e) { + err = e; + } + + expect(err).toBeDefined(); + }); + + it.skipIf(process.env['DB_DIALECT'] === 'sqlite')('should add new primary key', async () => { + await db.getRepository('collections').create({ + values: { + name: 'posts', + fields: [ + { + name: 'id', + type: 'bigInt', + primaryKey: true, + }, + { + name: 'name', + type: 'string', + }, + ], + }, + context: {}, + }); + + const response1 = await app + .agent() + .resource('collections.fields', 'posts') + .update({ + filterByTk: 'id', + values: { + primaryKey: false, + }, + }); + + expect(response1.statusCode).toBe(200); + + const model = db.getCollection('posts').model; + expect(model.rawAttributes['id'].primaryKey).toBe(false); + + const response2 = await app + .agent() + .resource('collections.fields', 'posts') + .create({ + values: { + primaryKey: true, + name: 'title', + type: 'string', + }, + }); + + expect(response2.statusCode).toBe(200); + + expect(model.rawAttributes['title'].primaryKey).toBe(true); + expect(model.rawAttributes['id'].primaryKey).toBe(false); + }); + + it.skipIf(process.env['DB_DIALECT'] === 'sqlite')('should update new primary key', async () => { + await db.getRepository('collections').create({ + values: { + name: 'posts', + fields: [ + { + name: 'id', + type: 'bigInt', + primaryKey: true, + }, + { + name: 'name', + type: 'string', + }, + ], + }, + context: {}, + }); + + const response1 = await app + .agent() + .resource('collections.fields', 'posts') + .update({ + filterByTk: 'id', + values: { + primaryKey: false, + }, + }); + + expect(response1.statusCode).toBe(200); + + const model = db.getCollection('posts').model; + expect(model.rawAttributes['id'].primaryKey).toBe(false); + + const response2 = await app + .agent() + .resource('collections.fields', 'posts') + .create({ + values: { + name: 'title', + type: 'string', + }, + }); + + expect(response2.statusCode).toBe(200); + + const response3 = await app + .agent() + .resource('collections.fields', 'posts') + .update({ + filterByTk: 'title', + values: { + primaryKey: true, + }, + }); + + expect(response3.statusCode).toBe(200); + + expect(model.rawAttributes['title'].primaryKey).toBe(true); + + const tableInfo = await db.sequelize + .getQueryInterface() + .describeTable(db.getCollection('posts').getTableNameWithSchema()); + + expect(tableInfo.title.primaryKey).toBe(true); + }); }); diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/resources/collections.test.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/resources/collections.test.ts index cb1cd8da8..fbd64b9d7 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/resources/collections.test.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/resources/collections.test.ts @@ -101,7 +101,7 @@ describe('collections', () => { expect(count).toBe(0); }); - test('remove collection 3', async () => { + test.skipIf(process.env['DB_DIALECT'] === 'sqlite')('remove collection 3', async () => { await app .agent() .resource('collections') diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForForeignKeyField.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForForeignKeyField.ts index c2c69ce0b..00c542758 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForForeignKeyField.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForForeignKeyField.ts @@ -189,6 +189,8 @@ export function afterCreateForForeignKeyField(db: Database) { return async (model, options) => { try { await hook(model, options); - } catch (error) {} + } catch (error) { + console.log(error); + } }; } diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForReverseField.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForReverseField.ts index 61005a8fb..b156e84f2 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForReverseField.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/afterCreateForReverseField.ts @@ -8,6 +8,7 @@ export function afterCreateForReverseField(db: Database) { if (!reverseKey) { return; } + const reverse = await Field.model.findByPk(reverseKey, { transaction }); await reverse.update({ reverseKey: model.get('key') }, { hooks: false, transaction }); }; diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/beforeCreateForValidateField.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/beforeCreateForValidateField.ts index f9f5a1080..f727c3a82 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/beforeCreateForValidateField.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/hooks/beforeCreateForValidateField.ts @@ -25,3 +25,25 @@ export function beforeCreateForValidateField(db: Database) { } }; } + +export function beforeUpdateForValidateField(db: Database) { + return async (model, { transaction }) => { + const isPrimaryKey = model.get('primaryKey'); + if (isPrimaryKey) { + const collection = db.getCollection(model.get('collectionName')); + if (!collection) { + return; + } + + const primaryKey = collection.model.primaryKeyAttribute; + + if (primaryKey !== model.get('name') && collection.model.rawAttributes[primaryKey]) { + throw new Error( + `update field ${model.get('name')} failed, collection ${ + collection.name + } already has primary key ${primaryKey}`, + ); + } + } + }; +} diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/models/field.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/models/field.ts index 462fb62ae..d668b4d1d 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/models/field.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/models/field.ts @@ -62,40 +62,6 @@ export class FieldModel extends MagicAttributeModel { }); } - async migrate({ isNew, ...options }: MigrateOptions = {}) { - let field; - try { - field = await this.load({ - transaction: options.transaction, - }); - - if (!field) { - return; - } - const collection = this.getFieldCollection(); - - if (isNew && collection.model.rawAttributes[this.get('name')] && this.get('unique')) { - // trick: set unique to false to avoid auto sync unique index - collection.model.rawAttributes[this.get('name')].unique = false; - } - - await field.sync(options); - - if (isNew && this.get('unique')) { - await this.syncUniqueIndex({ - transaction: options.transaction, - }); - } - } catch (error) { - // field sync failed, delete from memory - if (isNew && field) { - // update field should not remove field from memory - field.remove(); - } - throw error; - } - } - async remove(options?: any) { const collection = this.getFieldCollection(); diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/server.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/server.ts index 2fea0b6ad..9cbabd7db 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/server.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/server.ts @@ -13,7 +13,7 @@ import { beforeDestroyForeignKey, beforeInitOptions, } from './hooks'; -import { beforeCreateForValidateField } from './hooks/beforeCreateForValidateField'; +import { beforeCreateForValidateField, beforeUpdateForValidateField } from './hooks/beforeCreateForValidateField'; import { beforeCreateForViewCollection } from './hooks/beforeCreateForViewCollection'; import { CollectionModel, FieldModel } from './models'; import collectionActions from './resourcers/collections'; @@ -120,21 +120,11 @@ export class CollectionManagerPlugin extends Plugin { this.app.db.on('fields.beforeCreate', beforeCreateForValidateField(this.app.db)); this.app.db.on('fields.afterCreate', afterCreateForReverseField(this.app.db)); - - this.app.db.on('fields.afterCreate', async (model: FieldModel, { context, transaction }) => { - if (context) { - await model.migrate({ - isNew: true, - transaction, - }); - } - }); - - // after migrate - this.app.db.on('fields.afterCreate', afterCreateForForeignKeyField(this.app.db)); + this.app.db.on('fields.beforeUpdate', beforeUpdateForValidateField(this.app.db)); this.app.db.on('fields.beforeUpdate', async (model, options) => { const newValue = options.values; + if ( model.get('reverseKey') && lodash.get(newValue, 'reverseField') && @@ -147,6 +137,7 @@ export class CollectionManagerPlugin extends Plugin { throw new Error('cant update field without a reverseField key'); } } + // todo: 目前只支持一对多 if (model.get('sortable') && model.get('type') === 'hasMany') { model.set('sortBy', model.get('foreignKey') + 'Sort'); @@ -185,9 +176,36 @@ export class CollectionManagerPlugin extends Plugin { } }); - this.app.db.on('fields.afterSaveWithAssociations', async (model: FieldModel, { context, transaction }) => { + const afterCreateForForeignKeyFieldHook = afterCreateForForeignKeyField(this.app.db); + + this.app.db.on('fields.afterCreate', async (model: FieldModel, options) => { + const { context, transaction } = options; if (context) { await model.load({ transaction }); + await afterCreateForForeignKeyFieldHook(model, options); + } + }); + + this.app.db.on('fields.afterUpdate', async (model: FieldModel, options) => { + const { context, transaction } = options; + if (context) { + await model.load({ transaction }); + } + }); + + this.app.db.on('fields.afterSaveWithAssociations', async (model: FieldModel, options) => { + const { context, transaction } = options; + if (context) { + const collection = this.app.db.getCollection(model.get('collectionName')); + const syncOptions = { + transaction, + force: false, + alter: { + drop: false, + }, + }; + + await collection.sync(syncOptions); } }); diff --git a/packages/plugins/@nocobase/plugin-custom-request/src/server/collections/customRequest.ts b/packages/plugins/@nocobase/plugin-custom-request/src/server/collections/customRequest.ts index 3e14cd9df..9fccf28fd 100644 --- a/packages/plugins/@nocobase/plugin-custom-request/src/server/collections/customRequest.ts +++ b/packages/plugins/@nocobase/plugin-custom-request/src/server/collections/customRequest.ts @@ -3,6 +3,7 @@ import { defineCollection } from '@nocobase/database'; export default defineCollection({ dumpRules: 'required', name: 'customRequests', + autoGenId: false, fields: [ { type: 'uid',