fix: remove collections & fields from db (#511)

* fix: remove collections & fields from db

* fix: cannot read property 'removeFromDb' of undefined

* test: add test cases

* test: add test cases

* fix: exclude non-deletable fields

packages/core/database/src/__tests__/collection.test.ts

@@ -13,6 +13,33 @@ describe('collection', () => {
     await db.close();
   });
 
+  test('removeFromDb', async () => {
+    await db.clean({ drop: true });
+    const collection = db.collection({
+      name: 'test',
+      fields: [
+        {
+          type: 'string',
+          name: 'name',
+        },
+      ],
+    });
+    await db.sync();
+
+    const field = collection.getField('name');
+    const r1 = await field.existsInDb();
+    expect(r1).toBe(true);
+    await field.removeFromDb();
+    const r2 = await field.existsInDb();
+    expect(r2).toBe(false);
+
+    const r3 = await collection.existsInDb();
+    expect(r3).toBe(true);
+    await collection.removeFromDb();
+    const r4 = await collection.existsInDb();
+    expect(r4).toBe(false);
+  });
+
   test('collection disable authGenId', async () => {
     const Test = db.collection({
       name: 'test',

packages/core/database/src/collection.ts

@@ -1,7 +1,7 @@
 import merge from 'deepmerge';
 import { EventEmitter } from 'events';
 import { default as lodash, default as _ } from 'lodash';
-import { ModelCtor, ModelOptions, SyncOptions } from 'sequelize';
+import { ModelCtor, ModelOptions, QueryInterfaceDropTableOptions, SyncOptions, Transactionable } from 'sequelize';
 import { Database } from './database';
 import { Field, FieldOptions } from './fields';
 import { Model } from './model';
@@ -53,6 +53,10 @@ export class Collection<
     return this.options.name;
   }
 
+  get db() {
+    return this.context.database;
+  }
+
   constructor(options: CollectionOptions, context?: CollectionContext) {
     super();
     this.context = context;
@@ -190,6 +194,22 @@ export class Collection<
     this.context.database.removeCollection(this.name);
   }
 
+  async removeFromDb(options?: QueryInterfaceDropTableOptions) {
+    if (
+      await this.existsInDb({
+        transaction: options?.transaction,
+      })
+    ) {
+      const queryInterface = this.db.sequelize.getQueryInterface();
+      await queryInterface.dropTable(this.model.tableName, options);
+    }
+    this.remove();
+  }
+
+  async existsInDb(options?: Transactionable) {
+    return this.db.collectionExistsInDb(this.name, options);
+  }
+
   removeField(name) {
     if (!this.fields.has(name)) {
       return;
@@ -199,7 +219,7 @@ export class Collection<
     if (bool) {
       this.emit('field.afterRemove', field);
     }
-    return bool;
+    return field as Field;
   }
 
   /**

packages/core/database/src/database.ts

@@ -12,6 +12,7 @@ import {
   QueryOptions,
   Sequelize,
   SyncOptions,
+  Transactionable,
   Utils
 } from 'sequelize';
 import { SequelizeStorage, Umzug } from 'umzug';
@@ -26,7 +27,6 @@ import extendOperators from './operators';
 import { RelationRepository } from './relation-repository/relation-repository';
 import { Repository } from './repository';
 
-
 export interface MergeOptions extends merge.Options {}
 
 export interface PendingOptions {
@@ -230,11 +230,13 @@ export class Database extends EventEmitter implements AsyncEmitter {
 
     const result = this.collections.delete(name);
 
+    this.sequelize.modelManager.removeModel(collection.model);
+
     if (result) {
       this.emit('afterRemoveCollection', collection);
     }
 
-    return result;
+    return collection;
   }
 
   getModel<M extends Model>(name: string) {
@@ -339,9 +341,11 @@ export class Database extends EventEmitter implements AsyncEmitter {
     }
   }
 
-  async doesCollectionExistInDb(name) {
+  async collectionExistsInDb(name, options?: Transactionable) {
-    const tables = await this.sequelize.getQueryInterface().showAllTables();
+    const tables = await this.sequelize.getQueryInterface().showAllTables({
-    return tables.find((table) => table === `${this.getTablePrefix()}${name}`);
+      transaction: options?.transaction,
+    });
+    return !!tables.find((table) => table === `${this.getTablePrefix()}${name}`);
   }
 
   public isSqliteMemory() {

packages/core/database/src/fields/field.ts

@@ -1,5 +1,12 @@
 import _ from 'lodash';
-import { DataType, ModelAttributeColumnOptions, ModelIndexesOptions, SyncOptions } from 'sequelize';
+import {
+  DataType,
+  ModelAttributeColumnOptions,
+  ModelIndexesOptions,
+  QueryInterfaceOptions,
+  SyncOptions,
+  Transactionable
+} from 'sequelize';
 import { Collection } from '../collection';
 import { Database } from '../database';
 
@@ -79,6 +86,76 @@ export abstract class Field {
     return this.collection.removeField(this.name);
   }
 
+  async removeFromDb(options?: QueryInterfaceOptions) {
+    if (!this.collection.model.rawAttributes[this.name]) {
+      this.remove();
+      // console.log('field is not attribute');
+      return;
+    }
+    if ((this.collection.model as any)._virtualAttributes.has(this.name)) {
+      this.remove();
+      // console.log('field is virtual attribute');
+      return;
+    }
+    if (this.collection.model.primaryKeyAttributes.includes(this.name)) {
+      // 主键不能删除
+      return;
+    }
+    if (this.collection.model.options.timestamps !== false) {
+      // timestamps 相关字段不删除
+      if (['createdAt', 'updatedAt', 'deletedAt'].includes(this.name)) {
+        return;
+      }
+    }
+    // 排序字段通过 sortable 控制
+    const sortable = this.collection.options.sortable;
+    if (sortable) {
+      let sortField: string;
+      if (sortable === true) {
+        sortField = 'sort';
+      } else if (typeof sortable === 'string') {
+        sortField = sortable;
+      } else if (sortable.name) {
+        sortField = sortable.name || 'sort';
+      }
+      if (this.name === sortField) {
+        return;
+      }
+    }
+    if (this.options.field && this.name !== this.options.field) {
+      // field 指向的是真实的字段名，如果与 name 不一样，说明字段只是引用
+      this.remove();
+      return;
+    }
+    if (
+      await this.existsInDb({
+        transaction: options?.transaction,
+      })
+    ) {
+      const queryInterface = this.database.sequelize.getQueryInterface();
+      await queryInterface.removeColumn(this.collection.model.tableName, this.name, options);
+    }
+    this.remove();
+  }
+
+  async existsInDb(options?: Transactionable) {
+    const opts = {
+      transaction: options?.transaction,
+    };
+    let sql;
+    if (this.database.sequelize.getDialect() === 'sqlite') {
+      sql = `SELECT * from pragma_table_info('${this.collection.model.tableName}') WHERE name = '${this.name}'`;
+    } else {
+      sql = `
+        select column_name
+        from INFORMATION_SCHEMA.COLUMNS
+        where TABLE_NAME='${this.collection.model.tableName}' AND column_name='${this.name}'
+      `;
+    }
+    const [rows] = await this.database.sequelize.query(sql, opts);
+    return rows.length > 0;
+  }
+
   merge(obj: any) {
     Object.assign(this.options, obj);
   }

packages/core/server/src/application.ts

@@ -97,7 +97,7 @@ export class ApplicationVersion {
   }
 
   async get() {
-    if (await this.app.db.doesCollectionExistInDb('applicationVersion')) {
+    if (await this.app.db.collectionExistsInDb('applicationVersion')) {
       const model = await this.collection.model.findOne();
       return model.get('value') as any;
     }
@@ -115,7 +115,7 @@ export class ApplicationVersion {
   }
 
   async satisfies(range: string) {
-    if (await this.app.db.doesCollectionExistInDb('applicationVersion')) {
+    if (await this.app.db.collectionExistsInDb('applicationVersion')) {
       const model = await this.collection.model.findOne();
       const version = model.get('value') as any;
       return semver.satisfies(version, range);

packages/core/server/src/commands/install.ts

@@ -20,7 +20,7 @@ export default (app: Application) => {
       }
 
       if (!opts?.clean && !opts?.force) {
-        if (app.db.doesCollectionExistInDb('applicationVersion')) {
+        if (app.db.collectionExistsInDb('applicationVersion')) {
           installed = true;
           if (!opts.silent) {
             console.log('NocoBase is already installed. To reinstall, please execute:');

packages/plugins/collection-manager/src/__tests__/index.ts

@@ -1,8 +1,7 @@
-import { mockServer } from '@nocobase/test';
 import PluginUiSchema from '@nocobase/plugin-ui-schema-storage';
-
+import { mockServer } from '@nocobase/test';
-import CollectionManagerPlugin from '..';
 import lodash from 'lodash';
+import CollectionManagerPlugin from '../';
 
 export async function createApp(options = {}) {
   const app = mockServer();

packages/plugins/collection-manager/src/__tests__/remove-collection.test.ts

@@ -25,7 +25,7 @@ describe('collections repository', () => {
       context: {},
       values: {
         name: 'posts',
-        fields: [{ type: 'hasMany', name: 'comments', options: { target: 'comments' } }],
+        fields: [{ type: 'hasMany', name: 'comments', target: 'comments' }],
       },
     });
 

packages/plugins/collection-manager/src/__tests__/resources/collections.fields.test.ts

@@ -0,0 +1,123 @@
+import { MockServer } from '@nocobase/test';
+import { createApp } from '..';
+
+describe('collections.fields', () => {
+  let app: MockServer;
+
+  beforeEach(async () => {
+    app = await createApp();
+    await app.install({ clean: true });
+  });
+
+  afterEach(async () => {
+    await app.destroy();
+  });
+
+  test('destroy field', async () => {
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test1',
+          fields: [
+            {
+              type: 'string',
+              name: 'name',
+            },
+          ],
+        },
+      });
+    const collection = app.db.getCollection('test1');
+    const field = collection.getField('name');
+    expect(collection.hasField('name')).toBeTruthy();
+    const r1 = await field.existsInDb();
+    expect(r1).toBeTruthy();
+    await app.agent().resource('collections.fields', 'test1').destroy({
+      filterByTk: 'name',
+    });
+    expect(collection.hasField('name')).toBeFalsy();
+    const r2 = await field.existsInDb();
+    expect(r2).toBeFalsy();
+  });
+
+  test('destroy field', async () => {
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test1',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test2',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections.fields', 'test1')
+      .create({
+        values: {
+          type: 'string',
+          name: 'name',
+        },
+      });
+    const collection = app.db.getCollection('test1');
+    const field = collection.getField('name');
+    expect(collection.hasField('name')).toBeTruthy();
+    const r1 = await field.existsInDb();
+    expect(r1).toBeTruthy();
+    await app.agent().resource('collections.fields', 'test1').destroy({
+      filterByTk: 'name',
+    });
+    expect(collection.hasField('name')).toBeFalsy();
+    const r2 = await field.existsInDb();
+    expect(r2).toBeFalsy();
+  });
+
+  test('remove association field', async () => {
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test1',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test2',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections.fields', 'test1')
+      .create({
+        values: {
+          type: 'belongsTo',
+          name: 'test2',
+          target: 'test2',
+          reverseField: {
+            name: 'test1',
+          },
+        },
+      });
+    const collection = app.db.getCollection('test1');
+    const collection2 = app.db.getCollection('test2');
+    expect(collection.hasField('test2')).toBeTruthy();
+    expect(collection2.hasField('test1')).toBeTruthy();
+    await app.agent().resource('collections.fields', 'test1').destroy({
+      filterByTk: 'test2',
+    });
+    expect(collection.hasField('test2')).toBeFalsy();
+    expect(collection2.hasField('test1')).toBeTruthy();
+  });
+});

packages/plugins/collection-manager/src/__tests__/resources/collections.test.ts

@@ -0,0 +1,130 @@
+import { HasManyRepository } from '@nocobase/database';
+import { MockServer } from '@nocobase/test';
+import { createApp } from '..';
+
+describe('collections', () => {
+  let app: MockServer;
+
+  beforeEach(async () => {
+    app = await createApp();
+    await app.install({ clean: true });
+  });
+
+  afterEach(async () => {
+    await app.destroy();
+  });
+
+  test('remove collection', async () => {
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test',
+        },
+      });
+    const collection = app.db.getCollection('test');
+    const r1 = await collection.existsInDb();
+    expect(r1).toBe(true);
+    await app.agent().resource('collections').destroy({
+      filterByTk: 'test',
+    });
+    const r2 = await collection.existsInDb();
+    expect(r2).toBe(false);
+  });
+
+  test('remove collection', async () => {
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test1',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test2',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections.fields', 'test1')
+      .create({
+        values: {
+          type: 'belongsTo',
+          name: 'test2',
+          target: 'test2',
+          reverseField: {
+            name: 'test1',
+          },
+        },
+      });
+    await app.agent().resource('collections').destroy({
+      filterByTk: 'test1',
+    });
+    expect(app.db.hasCollection('test1')).toBeFalsy();
+    expect(!!app.db.sequelize.modelManager.getModel('test1')).toBeFalsy();
+    const collection2 = app.db.getCollection('test2');
+    expect(collection2.hasField('test2')).toBeFalsy();
+    const count = await app.db.getRepository<HasManyRepository>('collections.fields', 'test2').count({
+      filter: {
+        name: 'test2',
+      },
+    });
+    expect(count).toBe(0);
+  });
+
+  test('remove collection', async () => {
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test1',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test2',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections')
+      .create({
+        values: {
+          name: 'test3',
+        },
+      });
+    await app
+      .agent()
+      .resource('collections.fields', 'test1')
+      .create({
+        values: {
+          type: 'belongsToMany',
+          name: 'test2',
+          target: 'test2',
+          through: 'test3',
+          reverseField: {
+            name: 'test1',
+          },
+        },
+      });
+    await app.agent().resource('collections').destroy({
+      filterByTk: 'test3',
+    });
+    expect(app.db.hasCollection('test3')).toBeFalsy();
+    expect(!!app.db.sequelize.modelManager.getModel('test3')).toBeFalsy();
+    const collection1 = app.db.getCollection('test1');
+    expect(collection1.hasField('test2')).toBeFalsy();
+    const collection2 = app.db.getCollection('test2');
+    expect(collection2.hasField('test1')).toBeFalsy();
+  });
+});

packages/plugins/collection-manager/src/models/collection.ts

@@ -50,19 +50,29 @@ export class CollectionModel extends MagicAttributeModel {
     }
   }
 
-  /**
-   * TODO: drop table from the database
-   * 
-   * @param options 
-   * @returns 
-   */
   async remove(options?: any) {
+    const { transaction } = options || {};
     const name = this.get('name');
-    // delete from memory
+    const collection = this.db.getCollection(name);
-    const result = this.db.removeCollection(name);
+    if (!collection) {
-    // TODO: drop table from the database
+      return;
-    // this.db.sequelize.getQueryInterface().dropTable(this.get('name'));
+    }
-    return result;
+    const fields = await this.db.getRepository('fields').find({
+      filter: {
+        'type.$in': ['belongsToMany', 'belongsTo', 'hasMany', 'hasOne'],
+      },
+      transaction,
+    });
+    for (const field of fields) {
+      if (field.get('target') && field.get('target') === name) {
+        await field.destroy({ transaction });
+      } else if (field.get('through') && field.get('through') === name) {
+        await field.destroy({ transaction });
+      }
+    }
+    await collection.removeFromDb({
+      transaction,
+    });
   }
 
   async migrate(options?: SyncOptions & Transactionable) {

packages/plugins/collection-manager/src/models/field.ts

@@ -50,23 +50,18 @@ export class FieldModel extends MagicAttributeModel {
     }
   }
 
-  /**
-   * TODO: drop column from the database
-   * 
-   * @param options 
-   * @returns 
-   */
   async remove(options?: any) {
     const collectionName = this.get('collectionName');
-    const fieldName = this.get('name');
     if (!this.db.hasCollection(collectionName)) {
       return;
     }
     const collection = this.db.getCollection(collectionName);
-    // delete from memory
+    const field = collection.getField(this.get('name'));
-    const result = collection.removeField(this.get('name'));
+    if (!field) {
-    // TODO: drop column from the database
+      return;
-    // this.db.sequelize.getQueryInterface().removeColumn(collectionName, fieldName);
+    }
-    return result;
+    return field.removeFromDb({
+      transaction: options.transaction,
+    });
   }
 }

packages/plugins/collection-manager/src/server.ts

@@ -65,28 +65,6 @@ export class CollectionManagerPlugin extends Plugin {
       }
     });
 
-    this.app.db.on('collections.afterDestroy', async (model, { transaction }) => {
-      const name = model.get('name');
-
-      const fields = await this.app.db.getRepository('fields').find({
-        filter: {
-          'type.$in': ['belongsToMany', 'belongsTo', 'hasMany', 'hasOne'],
-        },
-        transaction,
-      });
-
-      const deleteFieldsKey = fields
-        .filter((field) => (field.get('options') as any)?.target === name)
-        .map((field) => field.get('key') as string);
-
-      await this.app.db.getRepository('fields').destroy({
-        filter: {
-          'key.$in': deleteFieldsKey,
-        },
-        transaction,
-      });
-    });
-
     this.app.db.on('fields.afterCreate', async (model, { context, transaction }) => {
       if (context) {
         await model.migrate({ transaction });
@@ -99,126 +77,6 @@ export class CollectionManagerPlugin extends Plugin {
       }
     });
 
-    // this.app.db.on('fields.afterCreateWithAssociations', async (model, { context, transaction }) => {
-    //   return;
-    //   if (!context) {
-    //     return;
-    //   }
-    //   if (!model.get('through')) {
-    //     return;
-    //   }
-    //   const [throughName, sourceName, targetName] = [
-    //     model.get('through'),
-    //     model.get('collectionName'),
-    //     model.get('target'),
-    //   ];
-    //   const db = this.app.db;
-    //   const through = await db.getRepository('collections').findOne({
-    //     filter: {
-    //       name: throughName,
-    //     },
-    //     transaction,
-    //   });
-    //   if (!through) {
-    //     return;
-    //   }
-    //   const repository = db.getRepository('collections.fields', throughName);
-    //   await repository.create({
-    //     transaction,
-    //     values: {
-    //       name: `f_${uid()}`,
-    //       type: 'belongsTo',
-    //       target: sourceName,
-    //       targetKey: model.get('sourceKey'),
-    //       foreignKey: model.get('foreignKey'),
-    //       interface: 'linkTo',
-    //       reverseField: {
-    //         interface: 'subTable',
-    //         uiSchema: {
-    //           type: 'void',
-    //           title: through.get('title'),
-    //           'x-component': 'TableField',
-    //           'x-component-props': {},
-    //         },
-    //         // uiSchema: {
-    //         //   title: through.get('title'),
-    //         //   'x-component': 'RecordPicker',
-    //         //   'x-component-props': {
-    //         //     // mode: 'tags',
-    //         //     multiple: true,
-    //         //     fieldNames: {
-    //         //       label: 'id',
-    //         //       value: 'id',
-    //         //     },
-    //         //   },
-    //         // },
-    //       },
-    //       uiSchema: {
-    //         title: db.getCollection(sourceName)?.options?.title || sourceName,
-    //         'x-component': 'RecordPicker',
-    //         'x-component-props': {
-    //           // mode: 'tags',
-    //           multiple: false,
-    //           fieldNames: {
-    //             label: 'id',
-    //             value: 'id',
-    //           },
-    //         },
-    //       },
-    //     },
-    //   });
-    //   await repository.create({
-    //     transaction,
-    //     values: {
-    //       name: `f_${uid()}`,
-    //       type: 'belongsTo',
-    //       target: targetName,
-    //       targetKey: model.get('targetKey'),
-    //       foreignKey: model.get('otherKey'),
-    //       interface: 'linkTo',
-    //       reverseField: {
-    //         interface: 'subTable',
-    //         uiSchema: {
-    //           type: 'void',
-    //           title: through.get('title'),
-    //           'x-component': 'TableField',
-    //           'x-component-props': {},
-    //         },
-    //         // interface: 'linkTo',
-    //         // uiSchema: {
-    //         //   title: through.get('title'),
-    //         //   'x-component': 'RecordPicker',
-    //         //   'x-component-props': {
-    //         //     // mode: 'tags',
-    //         //     multiple: true,
-    //         //     fieldNames: {
-    //         //       label: 'id',
-    //         //       value: 'id',
-    //         //     },
-    //         //   },
-    //         // },
-    //       },
-    //       uiSchema: {
-    //         title: db.getCollection(targetName)?.options?.title || targetName,
-    //         'x-component': 'RecordPicker',
-    //         'x-component-props': {
-    //           // mode: 'tags',
-    //           multiple: false,
-    //           fieldNames: {
-    //             label: 'id',
-    //             value: 'id',
-    //           },
-    //         },
-    //       },
-    //     },
-    //   });
-    //   await db.getRepository<CollectionRepository>('collections').load({
-    //     filter: {
-    //       'name.$in': [throughName, sourceName, targetName],
-    //     },
-    //   });
-    // });
-
     this.app.db.on('fields.afterDestroy', async (model, options) => {
       await model.remove(options);
     });