From 1ebb70e4c5ad0bcc58d85e57a1af33298bcd1423 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Sat, 26 Nov 2022 09:24:53 +0800 Subject: [PATCH] fix/inherits issues(#1146) * chore: sync inherits command * fix: inherit field type conflit * fix: merge * fix: test * fix: test * chore: app command * chore: create inheritance map * fix: test * fix: test * fix: test --- packages/core/cli/src/commands/dev.js | 12 ++- .../inhertits/collection-inherits.test.ts | 80 +++++++++++++++++-- packages/core/database/src/collection.ts | 21 +++-- .../database/src/fields/belongs-to-field.ts | 4 + .../src/fields/belongs-to-many-field.ts | 4 + packages/core/database/src/fields/field.ts | 15 ++-- .../database/src/fields/has-many-field.ts | 6 +- .../core/database/src/fields/has-one-field.ts | 7 +- .../core/database/src/inherited-collection.ts | 22 ++--- packages/core/database/src/sync-runner.ts | 6 +- .../20221121111113-update-id-to-bigint.ts | 9 ++- .../plugins/collection-manager/src/server.ts | 4 +- 12 files changed, 152 insertions(+), 38 deletions(-) diff --git a/packages/core/cli/src/commands/dev.js b/packages/core/cli/src/commands/dev.js index 480567d18..1a293d477 100644 --- a/packages/core/cli/src/commands/dev.js +++ b/packages/core/cli/src/commands/dev.js @@ -18,6 +18,7 @@ module.exports = (cli) => { .allowUnknownOption() .action(async (opts) => { promptForTs(); + if (process.argv.includes('-h') || process.argv.includes('--help')) { run('ts-node', [ '-P', @@ -29,15 +30,22 @@ module.exports = (cli) => { ]); return; } + const { port, client, server } = opts; + if (port) { process.env.APP_PORT = opts.port; } + const { APP_PORT } = process.env; + let clientPort = APP_PORT; let serverPort; + nodeCheck(); + await postCheck(opts); + if (server) { serverPort = APP_PORT; } else if (!server && !client) { @@ -45,6 +53,7 @@ module.exports = (cli) => { port: 1 * clientPort + 1, }); } + await runAppCommand('install', ['--silent']); // if (opts.dbSync) { // await runAppCommand('db:sync'); @@ -76,7 +85,8 @@ module.exports = (cli) => { env: { PORT: clientPort, APP_ROOT: `packages/${APP_PACKAGE_ROOT}/client`, - PROXY_TARGET_URL: process.env.PROXY_TARGET_URL || (serverPort ? `http://127.0.0.1:${serverPort}` : undefined), + PROXY_TARGET_URL: + process.env.PROXY_TARGET_URL || (serverPort ? `http://127.0.0.1:${serverPort}` : undefined), }, }); } diff --git a/packages/core/database/src/__tests__/inhertits/collection-inherits.test.ts b/packages/core/database/src/__tests__/inhertits/collection-inherits.test.ts index d1a22708e..6ceb71125 100644 --- a/packages/core/database/src/__tests__/inhertits/collection-inherits.test.ts +++ b/packages/core/database/src/__tests__/inhertits/collection-inherits.test.ts @@ -15,6 +15,69 @@ pgOnly()('collection inherits', () => { await db.close(); }); + it('should not throw error when fields have same type with parent', async () => { + db.collection({ + name: 'parent', + fields: [{ name: 'field1', type: 'date' }], + }); + + expect(() => { + db.collection({ + name: 'child', + inherits: ['parent'], + fields: [ + { + name: 'field1', + type: 'date', + otherOptions: true, + }, + ], + }); + }).not.toThrowError(); + }); + + it('should throw error when fields conflict with parent ', async () => { + db.collection({ + name: 'parent', + fields: [{ name: 'field1', type: 'string' }], + }); + + expect(() => { + db.collection({ + name: 'child', + inherits: ['parent'], + fields: [{ name: 'field1', type: 'integer' }], + }); + }).toThrowError(); + + await db.sync(); + }); + + it('should not conflict when fields have same DateType', async () => { + db.collection({ + name: 'parent', + fields: [{ name: 'field1', type: 'string' }], + }); + + expect(() => { + db.collection({ + name: 'child', + inherits: ['parent'], + fields: [ + { + name: 'field1', + type: 'sequence', + patterns: [ + { + type: 'integer', + }, + ], + }, + ], + }); + }).not.toThrowError(); + }); + it('should create inherits with lazy parents', async () => { const child = db.collection({ name: 'child', @@ -523,27 +586,22 @@ pgOnly()('collection inherits', () => { name: 'person', fields: [ { name: 'name', type: 'string' }, - { type: 'hasOne', name: 'profile' }, + { type: 'hasOne', name: 'profile', foreignKey: 'personId' }, ], }); - db.collection({ + const Profile = db.collection({ name: 'profiles', fields: [ { name: 'age', type: 'integer' }, { type: 'belongsTo', name: 'person', + foreignKey: 'personId', }, ], }); - db.collection({ - name: 'teachers', - inherits: 'person', - fields: [{ name: 'salary', type: 'integer' }], - }); - db.collection({ name: 'students', inherits: 'person', @@ -557,6 +615,12 @@ pgOnly()('collection inherits', () => { ], }); + db.collection({ + name: 'teachers', + inherits: 'person', + fields: [{ name: 'salary', type: 'integer' }], + }); + db.collection({ name: 'studentProfiles', fields: [{ name: 'grade', type: 'string' }], diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 3e3e033f2..827396754 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -7,7 +7,7 @@ import { QueryInterfaceDropTableOptions, SyncOptions, Transactionable, - Utils + Utils, } from 'sequelize'; import { Database } from './database'; import { Field, FieldOptions } from './fields'; @@ -79,7 +79,10 @@ export class Collection< this.db.modelCollection.set(this.model, this); this.db.tableNameCollectionMap.set(this.model.tableName, this); - this.setFields(options.fields); + if (!options.inherits) { + this.setFields(options.fields); + } + this.setRepository(options.repository); this.setSortable(options.sortable); } @@ -147,8 +150,13 @@ export class Collection< } private bindFieldEventListener() { - this.on('field.afterAdd', (field: Field) => field.bind()); - this.on('field.afterRemove', (field: Field) => field.unbind()); + this.on('field.afterAdd', (field: Field) => { + field.bind(); + }); + + this.on('field.afterRemove', (field: Field) => { + field.unbind(); + }); } forEachField(callback: (field: Field) => void) { @@ -186,7 +194,7 @@ export class Collection< const oldField = this.fields.get(name); - if (oldField && oldField.options.inherit && options.type != oldField.options.type) { + if (oldField && oldField.options.inherit && field.typeToString() != oldField.typeToString()) { throw new Error( `Field type conflict: cannot set "${name}" on "${this.name}" to ${options.type}, parent "${name}" type is ${oldField.options.type}`, ); @@ -196,6 +204,7 @@ export class Collection< this.fields.set(name, field); this.emit('field.afterAdd', field); + // refresh children models if (this.isParent()) { for (const child of this.context.database.inheritanceMap.getChildren(this.name, { deep: false, @@ -264,9 +273,11 @@ export class Collection< const field = this.fields.get(name); const bool = this.fields.delete(name); + if (bool) { this.emit('field.afterRemove', field); } + return field as Field; } diff --git a/packages/core/database/src/fields/belongs-to-field.ts b/packages/core/database/src/fields/belongs-to-field.ts index dababfece..7da6196ca 100644 --- a/packages/core/database/src/fields/belongs-to-field.ts +++ b/packages/core/database/src/fields/belongs-to-field.ts @@ -5,6 +5,10 @@ import { checkIdentifier } from '../utils'; import { BaseRelationFieldOptions, RelationField } from './relation-field'; export class BelongsToField extends RelationField { + get dataType() { + return 'BelongsTo'; + } + static type = 'belongsTo'; get target() { diff --git a/packages/core/database/src/fields/belongs-to-many-field.ts b/packages/core/database/src/fields/belongs-to-many-field.ts index a7dac5189..34501ffc5 100644 --- a/packages/core/database/src/fields/belongs-to-many-field.ts +++ b/packages/core/database/src/fields/belongs-to-many-field.ts @@ -5,6 +5,10 @@ import { checkIdentifier } from '../utils'; import { MultipleRelationFieldOptions, RelationField } from './relation-field'; export class BelongsToManyField extends RelationField { + get dataType() { + return 'BelongsToMany'; + } + get through() { return ( this.options.through || diff --git a/packages/core/database/src/fields/field.ts b/packages/core/database/src/fields/field.ts index a5ffd1d31..af07e8d14 100644 --- a/packages/core/database/src/fields/field.ts +++ b/packages/core/database/src/fields/field.ts @@ -1,4 +1,5 @@ import _ from 'lodash'; + import { DataType, ModelAttributeColumnOptions, @@ -53,9 +54,7 @@ export abstract class Field { return this.options.type; } - get dataType() { - return this.options.dataType; - } + abstract get dataType(); async sync(syncOptions: SyncOptions) { await this.collection.sync({ @@ -178,7 +177,7 @@ export abstract class Field { } bind() { - const {model} = this.context.collection; + const { model } = this.context.collection; model.rawAttributes[this.name] = this.toSequelize(); // @ts-ignore model.refreshAttributes(); @@ -188,7 +187,7 @@ export abstract class Field { } unbind() { - const {model} = this.context.collection; + const { model } = this.context.collection; model.removeAttribute(this.name); if (this.options.index || this.options.unique) { this.context.collection.removeIndex([this.name]); @@ -198,7 +197,7 @@ export abstract class Field { toSequelize(): any { const opts = _.omit(this.options, ['name']); if (this.dataType) { - Object.assign(opts, {type: this.dataType}); + Object.assign(opts, { type: this.dataType }); } return opts; } @@ -206,4 +205,8 @@ export abstract class Field { isSqlite() { return this.database.sequelize.getDialect() === 'sqlite'; } + + typeToString() { + return this.dataType.toString(); + } } diff --git a/packages/core/database/src/fields/has-many-field.ts b/packages/core/database/src/fields/has-many-field.ts index 5bea5d4f2..2b00815d8 100644 --- a/packages/core/database/src/fields/has-many-field.ts +++ b/packages/core/database/src/fields/has-many-field.ts @@ -5,7 +5,7 @@ import { ForeignKeyOptions, HasManyOptions, HasManyOptions as SequelizeHasManyOptions, - Utils + Utils, } from 'sequelize'; import { Collection } from '../collection'; import { Reference } from '../features/ReferencesMap'; @@ -74,6 +74,10 @@ export interface HasManyFieldOptions extends HasManyOptions { } export class HasManyField extends RelationField { + get dataType(): any { + return 'HasMany'; + } + get foreignKey() { if (this.options.foreignKey) { return this.options.foreignKey; diff --git a/packages/core/database/src/fields/has-one-field.ts b/packages/core/database/src/fields/has-one-field.ts index da2be12b5..f72de6018 100644 --- a/packages/core/database/src/fields/has-one-field.ts +++ b/packages/core/database/src/fields/has-one-field.ts @@ -74,6 +74,10 @@ export interface HasOneFieldOptions extends HasOneOptions { } export class HasOneField extends RelationField { + get dataType(): any { + return 'HasOne'; + } + get target() { const { target, name } = this.options; return target || Utils.pluralize(name); @@ -155,13 +159,14 @@ export class HasOneField extends RelationField { // 如果关系表内没有显式的创建外键字段，删除关系时，外键也删除掉 const tcoll = database.collections.get(this.target); - if (tcoll) { + if (tcoll && !this.options.inherit) { const foreignKey = this.options.foreignKey; const field = tcoll.findField((field) => { if (field.name === foreignKey) { return true; } + return field.type === 'belongsTo' && field.foreignKey === foreignKey; }); diff --git a/packages/core/database/src/inherited-collection.ts b/packages/core/database/src/inherited-collection.ts index a7066df9b..cb7d5a878 100644 --- a/packages/core/database/src/inherited-collection.ts +++ b/packages/core/database/src/inherited-collection.ts @@ -36,13 +36,14 @@ export class InheritedCollection extends Collection { protected bindParents() { this.setParents(this.options.inherits); - this.context.database.inheritanceMap.setInheritance(this.name, this.options.inherits); this.setParentFields(); + this.setFields(this.options.fields, false); + this.db.inheritanceMap.setInheritance(this.name, this.options.inherits); } protected setParents(inherits: string | string[]) { this.parents = lodash.castArray(inherits).map((name) => { - const existCollection = this.context.database.collections.get(name); + const existCollection = this.db.collections.get(name); if (!existCollection) { throw new ParentCollectionNotFound(name); } @@ -52,13 +53,11 @@ export class InheritedCollection extends Collection { } protected setParentFields() { - for (const [name, field] of this.parentFields()) { - if (!this.hasField(name)) { - this.setField(name, { - ...field.options, - inherit: true, - }); - } + for (const [name, fieldOptions] of this.parentFields()) { + this.setField(name, { + ...fieldOptions, + inherit: true, + }); } } @@ -71,15 +70,16 @@ export class InheritedCollection extends Collection { for (const parent of this.parents) { if (parent.isInherited()) { for (const [name, field] of (parent).parentFields()) { - fields.set(name, field); + fields.set(name, field.options); } } const parentFields = parent.fields; for (const [name, field] of parentFields) { - fields.set(name, field); + fields.set(name, field.options); } } + return fields; } diff --git a/packages/core/database/src/sync-runner.ts b/packages/core/database/src/sync-runner.ts index 57340403e..1c7ddceb4 100644 --- a/packages/core/database/src/sync-runner.ts +++ b/packages/core/database/src/sync-runner.ts @@ -19,7 +19,11 @@ export class SyncRunner { const parents = inheritedCollection.parents; if (!parents) { - throw new Error("Inherit model can't be created without parents"); + throw new Error( + `Inherit model ${inheritedCollection.name} can't be created without parents, parents option is ${lodash + .castArray(inheritedCollection.options.inherits) + .join(', ')}`, + ); } const parentTables = parents.map((parent) => parent.model.tableName); diff --git a/packages/plugins/collection-manager/src/migrations/20221121111113-update-id-to-bigint.ts b/packages/plugins/collection-manager/src/migrations/20221121111113-update-id-to-bigint.ts index e91c8b8b9..1e7f2d397 100644 --- a/packages/plugins/collection-manager/src/migrations/20221121111113-update-id-to-bigint.ts +++ b/packages/plugins/collection-manager/src/migrations/20221121111113-update-id-to-bigint.ts @@ -32,9 +32,13 @@ export default class UpdateIdToBigIntMigrator extends Migration { const updateToBigInt = async (model, fieldName) => { let sql; + const tableName = model.tableName; + + const collection = db.modelCollection.get(model); + const fieldRecord = await db.getCollection('fields').repository.findOne({ filter: { - collectionName: model.name, + collectionName: collection.name, name: fieldName, type: 'integer', }, @@ -45,8 +49,6 @@ export default class UpdateIdToBigIntMigrator extends Migration { await fieldRecord.save(); } - const tableName = model.tableName; - if (model.rawAttributes[fieldName].type instanceof DataTypes.INTEGER) { if (db.inDialect('postgres')) { sql = `ALTER TABLE "${tableName}" ALTER COLUMN "${fieldName}" SET DATA TYPE BIGINT;`; @@ -89,6 +91,7 @@ export default class UpdateIdToBigIntMigrator extends Migration { await this.sequelize.query(`ALTER SEQUENCE ${sequenceName} AS BIGINT;`, {}); } } + this.app.log.info(`updated ${tableName}.${fieldName} to BIGINT`, tableName, fieldName); } }; diff --git a/packages/plugins/collection-manager/src/server.ts b/packages/plugins/collection-manager/src/server.ts index 5545c6db6..b73d724ce 100644 --- a/packages/plugins/collection-manager/src/server.ts +++ b/packages/plugins/collection-manager/src/server.ts @@ -13,7 +13,7 @@ import { beforeCreateForChildrenCollection, beforeCreateForReverseField, beforeDestroyForeignKey, - beforeInitOptions + beforeInitOptions, } from './hooks'; import { CollectionModel, FieldModel } from './models'; @@ -188,7 +188,9 @@ export class CollectionManagerPlugin extends Plugin { try { await this.app.db.getRepository('collections').load(); } catch (error) { + this.app.logger.warn(error); await this.app.db.sync(); + try { await this.app.db.getRepository('collections').load(); } catch (error) {