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

@@ -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),
           },
         });
       }

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' }],

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);
 
+    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.afterAdd', (field: Field) => {
-    this.on('field.afterRemove', (field: Field) => field.unbind());
+      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;
   }
 

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() {

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 ||

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() {
+  abstract get dataType();
-    return this.options.dataType;
-  }
 
   async sync(syncOptions: SyncOptions) {
     await this.collection.sync({
@@ -206,4 +205,8 @@ export abstract class Field {
   isSqlite() {
     return this.database.sequelize.getDialect() === 'sqlite';
   }
+
+  typeToString() {
+    return this.dataType.toString();
+  }
 }

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;

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;
       });
 

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,15 +53,13 @@ export class InheritedCollection extends Collection {
   }
 
   protected setParentFields() {
-    for (const [name, field] of this.parentFields()) {
+    for (const [name, fieldOptions] of this.parentFields()) {
-      if (!this.hasField(name)) {
       this.setField(name, {
-          ...field.options,
+        ...fieldOptions,
         inherit: true,
       });
     }
   }
-  }
 
   getParents() {
     return this.parents;
@@ -71,15 +70,16 @@ export class InheritedCollection extends Collection {
     for (const parent of this.parents) {
       if (parent.isInherited()) {
         for (const [name, field] of (<InheritedCollection>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;
   }
 

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);

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);
       }
     };

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<CollectionRepository>('collections').load();
         } catch (error) {
+          this.app.logger.warn(error);
           await this.app.db.sync();
+
           try {
             await this.app.db.getRepository<CollectionRepository>('collections').load();
           } catch (error) {