From 0e0eb6432e9d64aef50f5cdcc4f81373cf1517e4 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Mon, 13 Feb 2023 21:38:47 +0800 Subject: [PATCH] feat: provide the underscored option for the database (#1366) * feat: underscored options * feat: underscored using hook * feat: database underscored options * feat: underscored env * fix: collectionExistsInDb * fix: test * fix: nocobase install * fix: test * fix: belongsTo association * fix: test of underscored * chore: console.log * fix: list action test * fix: dump test * chore: snakeCase algo * fix: underscored field create * fix: underscored env * fix(acl): custom appends merge strategy (#1416) * Update index.md * fix(plugin-workflow): use promise to request (#1426) * Update index.md * Update collection.md * Update index.md * Update index.md * Update collection.md * Update field.md * Update repository.md * Update has-one-repository.md * Update has-many-repository.md * Update belongs-to-many-repository.md * Update index.md * chore: translate 'Add tab' in page header (#1424) * fix: test * fix: workflow test * fix: underscored with inherits * fix: underscored test * fix: process.env.DB_UNDERSCORED * fix: process.env.DB_UNDERSCORED === 'true' * fix: test * fix: pg test * fix: underscored table name * feat: tableName & fieldName conflict check * fix: test * fix: underscored index * fix: update field unique index * fix: sync default value * fix: collection manager create field * chore: field sync * fix: pg test * chore: test * fix: test * chore: default constraint name * chore: syncUniqueIndex * feat: field destory before check * feat: field type check * fix: test * fix: test * fix: improve * fix: should destroy when fields refer to the same field * fix: acl meta with underscored --------- Co-authored-by: chenos --- .github/workflows/nocobase-test.yml | 94 ++++++++ packages/app/server/src/config/database.ts | 1 + .../database/src/__tests__/bigint.test.ts | 2 +- .../database/src/__tests__/collection.test.ts | 28 ++- .../inhertits/collection-inherits.test.ts | 12 +- .../src/__tests__/underscored-options.test.ts | 207 ++++++++++++++++++ packages/core/database/src/collection.ts | 83 +++++-- packages/core/database/src/database.ts | 48 +++- .../database/src/fields/belongs-to-field.ts | 4 +- .../src/fields/belongs-to-many-field.ts | 1 - packages/core/database/src/fields/field.ts | 53 ++++- .../core/database/src/fields/has-one-field.ts | 18 +- packages/core/database/src/index.ts | 1 + packages/core/database/src/mock-database.ts | 3 +- packages/core/database/src/model.ts | 4 +- packages/core/database/src/options-parser.ts | 5 + .../belongs-to-many-repository.ts | 6 +- packages/core/database/src/repository.ts | 11 +- packages/core/database/src/sync-runner.ts | 4 +- packages/core/database/src/update-guard.ts | 6 + packages/core/database/src/utils.ts | 9 + .../src/plugin-manager/PluginManager.ts | 2 + .../acl/src/__tests__/list-action.test.ts | 29 ++- packages/plugins/acl/src/server.ts | 51 ++++- .../__tests__/collections.repository.test.ts | 186 ++++++++++++++++ .../__tests__/field-options/indexes.test.ts | 2 + .../src/__tests__/fields/reverseField.test.ts | 1 + .../__tests__/http-api/collections.test.ts | 140 ++++++++++++ .../http-api/inherited-collection.test.ts | 4 +- .../migrations/update-id-to-bigint.test.ts | 6 + .../resources/collections.fields.test.ts | 6 + .../20221121111113-update-id-to-bigint.ts | 6 +- .../collection-manager/src/models/field.ts | 124 ++++++----- .../plugins/collection-manager/src/server.ts | 9 +- .../src/server/__tests__/dump.test.ts | 8 +- .../src/server/__tests__/attachment.test.ts | 50 +++++ .../src/server/__tests__/index.ts | 2 + .../workflow/src/server/__tests__/index.ts | 20 +- .../__tests__/instructions/delay.test.ts | 31 ++- .../__tests__/server/index.test.ts | 15 +- 40 files changed, 1124 insertions(+), 168 deletions(-) create mode 100644 packages/core/database/src/__tests__/underscored-options.test.ts create mode 100644 packages/plugins/file-manager/src/server/__tests__/attachment.test.ts diff --git a/.github/workflows/nocobase-test.yml b/.github/workflows/nocobase-test.yml index c1102272c..ab3c1b1a7 100644 --- a/.github/workflows/nocobase-test.yml +++ b/.github/workflows/nocobase-test.yml @@ -109,3 +109,97 @@ jobs: DB_USER: root DB_PASSWORD: password DB_DATABASE: nocobase + + sqlite-underscored-test: + strategy: + matrix: + node_version: ['16'] + runs-on: ubuntu-latest + container: node:${{ matrix.node_version }} + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node_version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node_version }} + cache: 'yarn' + - run: yarn install + - name: Test with Sqlite + run: yarn test + env: + DB_DIALECT: sqlite + DB_STORAGE: /tmp/db.sqlite + DB_UNDERSCORED: true + + postgres-underscored-test: + strategy: + matrix: + node_version: ['16'] + + runs-on: ubuntu-latest + container: node:${{ matrix.node_version }} + services: + # Label used to access the service container + postgres: + # Docker Hub image + image: postgres:10 + # Provide the password for postgres + env: + POSTGRES_USER: nocobase + POSTGRES_PASSWORD: password + # Set health checks to wait until postgres has started + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node_version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node_version }} + cache: 'yarn' + - run: yarn install + - name: Test with postgres + run: yarn test + env: + DB_DIALECT: postgres + DB_HOST: postgres + DB_PORT: 5432 + DB_USER: nocobase + DB_PASSWORD: password + DB_DATABASE: nocobase + DB_UNDERSCORED: true + + mysql-underscored-test: + strategy: + matrix: + node_version: ['16'] + runs-on: ubuntu-latest + container: node:${{ matrix.node_version }} + services: + mysql: + image: mysql:8 + env: + MYSQL_ROOT_PASSWORD: password + MYSQL_DATABASE: nocobase + options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node_version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node_version }} + cache: 'yarn' + - run: yarn install + - name: Test with MySQL + run: yarn test + env: + DB_DIALECT: mysql + DB_HOST: mysql + DB_PORT: 3306 + DB_USER: root + DB_PASSWORD: password + DB_DATABASE: nocobase + DB_UNDERSCORED: true diff --git a/packages/app/server/src/config/database.ts b/packages/app/server/src/config/database.ts index d44d8513a..b174ef4b9 100644 --- a/packages/app/server/src/config/database.ts +++ b/packages/app/server/src/config/database.ts @@ -12,6 +12,7 @@ export default { timezone: process.env.DB_TIMEZONE, tablePrefix: process.env.DB_TABLE_PREFIX, schema: process.env.DB_SCHEMA, + underscored: process.env.DB_UNDERSCORED === 'true', } as IDatabaseOptions; function customLogger(queryString, queryObject) { diff --git a/packages/core/database/src/__tests__/bigint.test.ts b/packages/core/database/src/__tests__/bigint.test.ts index 0e379e98d..130283e47 100644 --- a/packages/core/database/src/__tests__/bigint.test.ts +++ b/packages/core/database/src/__tests__/bigint.test.ts @@ -43,6 +43,6 @@ excludeSqlite()('collection', () => { const profileTableInfo = await db.sequelize.getQueryInterface().describeTable(profile.model.tableName); - expect(profileTableInfo['userId'].type).toBe('BIGINT'); + expect(profileTableInfo[profile.model.rawAttributes['userId'].field].type).toBe('BIGINT'); }); }); diff --git a/packages/core/database/src/__tests__/collection.test.ts b/packages/core/database/src/__tests__/collection.test.ts index 5dab3cdc4..80fe60836 100644 --- a/packages/core/database/src/__tests__/collection.test.ts +++ b/packages/core/database/src/__tests__/collection.test.ts @@ -8,9 +8,7 @@ describe('collection', () => { let db: Database; beforeEach(async () => { - db = mockDatabase({ - logging: console.log, - }); + db = mockDatabase(); await db.clean({ drop: true }); }); @@ -240,9 +238,15 @@ describe('collection sync', () => { await collection.sync(); const tableFields = await (collection.model).queryInterface.describeTable(`${db.getTablePrefix()}users`); - expect(tableFields).toHaveProperty('firstName'); - expect(tableFields).toHaveProperty('lastName'); - expect(tableFields).toHaveProperty('age'); + if (db.options.underscored) { + expect(tableFields).toHaveProperty('first_name'); + expect(tableFields).toHaveProperty('last_name'); + expect(tableFields).toHaveProperty('age'); + } else { + expect(tableFields).toHaveProperty('firstName'); + expect(tableFields).toHaveProperty('lastName'); + expect(tableFields).toHaveProperty('age'); + } }); test('sync with association not exists', async () => { @@ -290,9 +294,15 @@ describe('collection sync', () => { const model = collection.model; await collection.sync(); - const tableFields = await (model).queryInterface.describeTable(`${db.getTablePrefix()}postsTags`); - expect(tableFields['postId']).toBeDefined(); - expect(tableFields['tagId']).toBeDefined(); + if (db.options.underscored) { + const tableFields = await (model).queryInterface.describeTable(`${db.getTablePrefix()}posts_tags`); + expect(tableFields['post_id']).toBeDefined(); + expect(tableFields['tag_id']).toBeDefined(); + } else { + const tableFields = await (model).queryInterface.describeTable(`${db.getTablePrefix()}postsTags`); + expect(tableFields['postId']).toBeDefined(); + expect(tableFields['tagId']).toBeDefined(); + } }); test('limit table name length', async () => { 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 dcd93a8f4..ce52cf365 100644 --- a/packages/core/database/src/__tests__/inhertits/collection-inherits.test.ts +++ b/packages/core/database/src/__tests__/inhertits/collection-inherits.test.ts @@ -886,11 +886,13 @@ pgOnly()('collection inherits', () => { const studentTableInfo = await db.sequelize.getQueryInterface().describeTable(student.model.tableName); - expect(studentTableInfo.score).toBeDefined(); - expect(studentTableInfo.name).toBeDefined(); - expect(studentTableInfo.id).toBeDefined(); - expect(studentTableInfo.createdAt).toBeDefined(); - expect(studentTableInfo.updatedAt).toBeDefined(); + const getField = (name) => student.model.rawAttributes[name].field; + + expect(studentTableInfo[getField('score')]).toBeDefined(); + expect(studentTableInfo[getField('name')]).toBeDefined(); + expect(studentTableInfo[getField('id')]).toBeDefined(); + expect(studentTableInfo[getField('createdAt')]).toBeDefined(); + expect(studentTableInfo[getField('updatedAt')]).toBeDefined(); }); it('should get parent fields', async () => { diff --git a/packages/core/database/src/__tests__/underscored-options.test.ts b/packages/core/database/src/__tests__/underscored-options.test.ts new file mode 100644 index 000000000..4efa43708 --- /dev/null +++ b/packages/core/database/src/__tests__/underscored-options.test.ts @@ -0,0 +1,207 @@ +import { Database, mockDatabase } from '@nocobase/database'; + +describe('underscored options', () => { + let db: Database; + + beforeEach(async () => { + db = mockDatabase({ + underscored: true, + }); + + await db.clean({ drop: true }); + }); + + afterEach(async () => { + await db.close(); + }); + + it('should set two field with same type', async () => { + const collection = db.collection({ + name: 'test', + fields: [ + { + type: 'string', + name: 'test_field', + }, + { + type: 'string', + name: 'testField', + }, + ], + }); + + await db.sync(); + }); + + it('should not set two field with difference type but same field name', async () => { + const collection = db.collection({ + name: 'test', + fields: [ + { + type: 'string', + name: 'test_field', + }, + ], + }); + + expect(() => { + collection.addField('testField', { type: 'integer' }); + }).toThrowError(); + + expect(() => { + collection.addField('test123', { type: 'integer', field: 'test_field' }); + }).toThrowError(); + }); + + it('should create index', async () => { + const collectionA = db.collection({ + name: 'testCollection', + fields: [ + { + type: 'string', + name: 'aField', + }, + { + type: 'string', + name: 'bField', + }, + ], + indexes: [ + { + type: 'UNIQUE', + fields: ['aField', 'bField'], + }, + ], + }); + + await db.sync(); + }); + + it('should use underscored option', async () => { + const collectionA = db.collection({ + name: 'testCollection', + underscored: true, + fields: [ + { + type: 'string', + name: 'testField', + }, + ], + }); + + await db.sync(); + + const tableName = collectionA.model.tableName; + + expect(tableName.includes('test_collection')).toBeTruthy(); + + const repository = db.getRepository('testCollection'); + + await repository.create({ + values: { + testField: 'test', + }, + }); + + const record = await repository.findOne({}); + + expect(record.get('testField')).toBe('test'); + }); + + it('should use database options', async () => { + const collectionA = db.collection({ + name: 'testCollection', + fields: [ + { + type: 'string', + name: 'testField', + }, + ], + }); + + await db.sync(); + + const tableName = collectionA.model.tableName; + + expect(tableName.includes('test_collection')).toBeTruthy(); + }); + + test('through table', async () => { + db.collection({ + name: 'posts', + fields: [ + { + type: 'string', + name: 'name', + }, + { + type: 'belongsToMany', + name: 'tags', + through: 'collectionCategory', + target: 'posts', + sourceKey: 'name', + foreignKey: 'postsName', + targetKey: 'name', + otherKey: 'tagsName', + }, + ], + }); + + db.collection({ + name: 'tags', + fields: [ + { + type: 'string', + name: 'name', + }, + { + type: 'belongsToMany', + name: 'posts', + target: 'posts', + through: 'collectionCategory', + sourceKey: 'name', + foreignKey: 'tagsName', + targetKey: 'name', + otherKey: 'postsName', + }, + ], + }); + + await db.sync(); + + const through = db.getCollection('collectionCategory'); + + expect(through.model.tableName.includes('collection_category')).toBeTruthy(); + }); + + test('db collectionExists', async () => { + const collectionA = db.collection({ + name: 'testCollection', + underscored: true, + fields: [ + { + type: 'string', + name: 'testField', + }, + ], + }); + + expect(await db.collectionExistsInDb('testCollection')).toBeFalsy(); + + await db.sync(); + + expect(await db.collectionExistsInDb('testCollection')).toBeTruthy(); + }); + + it('should throw error when table names conflict', async () => { + db.collection({ + name: 'b1_z', + }); + + expect(() => { + db.collection({ + name: 'b1Z', + }); + }).toThrowError(); + }); +}); diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 86479b2c2..3c43342bf 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -13,7 +13,7 @@ import { Database } from './database'; import { Field, FieldOptions } from './fields'; import { Model } from './model'; import { Repository } from './repository'; -import { checkIdentifier, md5 } from './utils'; +import { checkIdentifier, md5, snakeCase } from './utils'; export type RepositoryType = typeof Repository; @@ -36,6 +36,7 @@ export interface CollectionOptions extends Omit * @default 'options' */ magicAttribute?: string; + [key: string]: any; } @@ -72,11 +73,11 @@ export class Collection< constructor(options: CollectionOptions, context: CollectionContext) { super(); - this.checkOptions(options); - this.context = context; this.options = options; + this.checkOptions(options); + this.bindFieldEventListener(); this.modelInit(); @@ -93,15 +94,31 @@ export class Collection< private checkOptions(options: CollectionOptions) { checkIdentifier(options.name); + this.checkTableName(); + } + + private checkTableName() { + const tableName = this.tableName(); + for (const [k, collection] of this.db.collections) { + if (collection.name != this.options.name && tableName === collection.tableName()) { + throw new Error(`collection ${collection.name} and ${this.name} have same tableName "${tableName}"`); + } + } + } + + tableName() { + const { name, tableName } = this.options; + const tName = tableName || name; + return this.db.options.underscored ? snakeCase(tName) : tName; } private sequelizeModelOptions() { - const { name, tableName } = this.options; + const { name } = this.options; return { ..._.omit(this.options, ['name', 'fields', 'model', 'targetKey']), modelName: name, sequelize: this.context.database.sequelize, - tableName: tableName || name, + tableName: this.tableName(), }; } @@ -112,8 +129,10 @@ export class Collection< if (this.model) { return; } + const { name, model, autoGenId = true } = this.options; let M: ModelStatic = Model; + if (this.context.database.sequelize.isDefined(name)) { const m = this.context.database.sequelize.model(name); if ((m as any).isThrough) { @@ -126,11 +145,13 @@ export class Collection< return; } } + if (typeof model === 'string') { M = this.context.database.models.get(model) || Model; } else if (model) { M = model; } + // @ts-ignore this.model = class extends M {}; this.model.init(null, this.sequelizeModelOptions()); @@ -183,8 +204,31 @@ export class Collection< return this.setField(name, options); } + checkFieldType(name: string, options: FieldOptions) { + if (!this.db.options.underscored) { + return; + } + const fieldName = options.field || snakeCase(name); + const field = this.findField((f) => { + if (f.name === name) { + return false; + } + if (f.field) { + return f.field === fieldName; + } + return snakeCase(f.name) === fieldName; + }); + if (!field) { + return; + } + if (options.type !== field.type) { + throw new Error(`fields with same column must be of the same type ${JSON.stringify(options)}`); + } + } + setField(name: string, options: FieldOptions): Field { checkIdentifier(name); + this.checkFieldType(name, options); const { database } = this.context; @@ -362,9 +406,13 @@ export class Collection< if (!index) { return; } + + // collection defined indexes let indexes: any = this.model.options.indexes || []; + let indexName = []; let indexItem; + if (typeof index === 'string') { indexItem = { fields: [index], @@ -379,13 +427,17 @@ export class Collection< indexItem = index; indexName = index.fields; } + if (lodash.isEqual(this.model.primaryKeyAttributes, indexName)) { return; } + const name: string = this.model.primaryKeyAttributes.join(','); + if (name.startsWith(`${indexName.join(',')},`)) { return; } + for (const item of indexes) { if (lodash.isEqual(item.fields, indexName)) { return; @@ -395,11 +447,13 @@ export class Collection< return; } } + if (!indexItem) { return; } + indexes.push(indexItem); - this.model.options.indexes = indexes; + const tableName = this.model.getTableName(); // @ts-ignore this.model._indexes = this.model.options.indexes @@ -411,6 +465,7 @@ export class Collection< } return item; }); + this.refreshIndexes(); } @@ -430,15 +485,17 @@ export class Collection< refreshIndexes() { // @ts-ignore const indexes: any[] = this.model._indexes; + // @ts-ignore - this.model._indexes = indexes.filter((item) => { - for (const field of item.fields) { - if (!this.model.rawAttributes[field]) { - return false; + this.model._indexes = lodash.uniqBy( + indexes.map((item) => { + if (this.options.underscored) { + item.fields = item.fields.map((field) => snakeCase(field)); } - } - return true; - }); + return item; + }), + 'name', + ); } async sync(syncOptions?: SyncOptions) { diff --git a/packages/core/database/src/database.ts b/packages/core/database/src/database.ts index be04dc2c7..90ffd7360 100644 --- a/packages/core/database/src/database.ts +++ b/packages/core/database/src/database.ts @@ -60,6 +60,7 @@ import { UpdateWithAssociationsListener, ValidateListener } from './types'; +import { snakeCase } from './utils'; export interface MergeOptions extends merge.Options {} @@ -76,6 +77,7 @@ export interface IDatabaseOptions extends Options { tablePrefix?: string; migrator?: any; usingBigIntForId?: boolean; + underscored?: boolean; } export type DatabaseOptions = IDatabaseOptions; @@ -167,7 +169,6 @@ export class Database extends EventEmitter implements AsyncEmitter { constructor(options: DatabaseOptions) { super(); - this.version = new DatabaseVersion(this); const opts = { @@ -265,6 +266,12 @@ export class Database extends EventEmitter implements AsyncEmitter { } initListener() { + this.on('beforeDefine', (model, options) => { + if (this.options.underscored) { + options.underscored = true; + } + }); + this.on('afterCreate', async (instance) => { instance?.toChangedWithAssociations?.(); }); @@ -294,6 +301,27 @@ export class Database extends EventEmitter implements AsyncEmitter { } } }); + + this.on('beforeDefineCollection', (options) => { + if (options.underscored) { + if (lodash.get(options, 'sortable.scopeKey')) { + options.sortable.scopeKey = snakeCase(options.sortable.scopeKey); + } + + if (lodash.get(options, 'indexes')) { + // change index fields to snake case + options.indexes = options.indexes.map((index) => { + if (index.fields) { + index.fields = index.fields.map((field) => { + return snakeCase(field); + }); + } + + return index; + }); + } + } + }); } addMigration(item: MigrationItem) { @@ -328,6 +356,10 @@ export class Database extends EventEmitter implements AsyncEmitter { collection( options: CollectionOptions, ): Collection { + if (this.options.underscored) { + options.underscored = true; + } + this.emit('beforeDefineCollection', options); const hasValidInheritsOptions = (() => { @@ -477,6 +509,10 @@ export class Database extends EventEmitter implements AsyncEmitter { throw Error(`unsupported field type ${type}`); } + if (options.field && this.options.underscored) { + options.field = snakeCase(options.field); + } + return new Field(options, context); } @@ -525,11 +561,17 @@ export class Database extends EventEmitter implements AsyncEmitter { await this.sequelize.getQueryInterface().dropAllTables(others); } - async collectionExistsInDb(name, options?: Transactionable) { + async collectionExistsInDb(name: string, options?: Transactionable) { + const collection = this.getCollection(name); + if (!collection) { + return false; + } + const tables = await this.sequelize.getQueryInterface().showAllTables({ transaction: options?.transaction, }); - return !!tables.find((table) => table === `${this.getTablePrefix()}${name}`); + + return tables.includes(this.getCollection(name).model.tableName); } public isSqliteMemory() { diff --git a/packages/core/database/src/fields/belongs-to-field.ts b/packages/core/database/src/fields/belongs-to-field.ts index f28007118..f4d1a8ea5 100644 --- a/packages/core/database/src/fields/belongs-to-field.ts +++ b/packages/core/database/src/fields/belongs-to-field.ts @@ -1,5 +1,5 @@ -import { omit } from 'lodash'; -import { BelongsTo, BelongsToOptions as SequelizeBelongsToOptions, Utils } from 'sequelize'; +import lodash, { omit } from 'lodash'; +import { BelongsToOptions as SequelizeBelongsToOptions, Utils } from 'sequelize'; import { Reference } from '../features/ReferencesMap'; import { checkIdentifier } from '../utils'; import { BaseRelationFieldOptions, RelationField } from './relation-field'; 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 bf49f5cbb..b0fc12ac2 100644 --- a/packages/core/database/src/fields/belongs-to-many-field.ts +++ b/packages/core/database/src/fields/belongs-to-many-field.ts @@ -57,7 +57,6 @@ export class BelongsToManyField extends RelationField { } else { Through = database.collection({ name: through, - // timestamps: false, }); Object.defineProperty(Through.model, 'isThrough', { value: true }); diff --git a/packages/core/database/src/fields/field.ts b/packages/core/database/src/fields/field.ts index 1bb9d7ad6..65fed3d0e 100644 --- a/packages/core/database/src/fields/field.ts +++ b/packages/core/database/src/fields/field.ts @@ -12,6 +12,7 @@ import { Collection } from '../collection'; import { Database } from '../database'; import { InheritedCollection } from '../inherited-collection'; import { ModelEventTypes } from '../types'; +import { snakeCase } from '../utils'; export interface FieldContext { database: Database; @@ -89,6 +90,18 @@ export abstract class Field { return this.collection.removeField(this.name); } + columnName() { + if (this.options.field) { + return this.options.field; + } + + if (this.database.options.underscored) { + return snakeCase(this.name); + } + + return this.name; + } + async removeFromDb(options?: QueryInterfaceOptions) { const attribute = this.collection.model.rawAttributes[this.name]; @@ -113,7 +126,12 @@ export abstract class Field { } if (this.collection.model.options.timestamps !== false) { // timestamps 相关字段不删除 - if (['createdAt', 'updatedAt', 'deletedAt'].includes(this.name)) { + let timestampsFields = ['createdAt', 'updatedAt', 'deletedAt']; + if (this.database.options.underscored) { + timestampsFields = timestampsFields.map((field) => snakeCase(field)); + } + if (timestampsFields.includes(this.columnName())) { + this.collection.fields.delete(this.name); return; } } @@ -132,19 +150,28 @@ export abstract class Field { return; } } - if (this.options.field && this.name !== this.options.field) { - // field 指向的是真实的字段名，如果与 name 不一样，说明字段只是引用 - this.remove(); - return; - } + + // if (this.options.field && this.name !== this.options.field) { + // // field 指向的是真实的字段名，如果与 name 不一样，说明字段只是引用 + // this.remove(); + // return; + // } + + const columnReferencesCount = _.filter( + this.collection.model.rawAttributes, + (attr) => attr.field == this.columnName(), + ).length; + if ( - await this.existsInDb({ + (await this.existsInDb({ transaction: options?.transaction, - }) + })) && + columnReferencesCount == 1 ) { const queryInterface = this.database.sequelize.getQueryInterface(); - await queryInterface.removeColumn(this.collection.model.tableName, this.name, options); + await queryInterface.removeColumn(this.collection.model.tableName, this.columnName(), options); } + this.remove(); } @@ -154,18 +181,20 @@ export abstract class Field { }; let sql; if (this.database.sequelize.getDialect() === 'sqlite') { - sql = `SELECT * from pragma_table_info('${this.collection.model.tableName}') WHERE name = '${this.name}'`; + sql = `SELECT * from pragma_table_info('${this.collection.model.tableName}') WHERE name = '${this.columnName()}'`; } else if (this.database.inDialect('mysql')) { sql = ` select column_name from INFORMATION_SCHEMA.COLUMNS - where TABLE_SCHEMA='${this.database.options.database}' AND TABLE_NAME='${this.collection.model.tableName}' AND column_name='${this.name}' + where TABLE_SCHEMA='${this.database.options.database}' AND TABLE_NAME='${ + this.collection.model.tableName + }' AND column_name='${this.columnName()}' `; } else { sql = ` select column_name from INFORMATION_SCHEMA.COLUMNS - where TABLE_NAME='${this.collection.model.tableName}' AND column_name='${this.name}' + where TABLE_NAME='${this.collection.model.tableName}' AND column_name='${this.columnName()}' `; } const [rows] = await this.database.sequelize.query(sql, opts); diff --git a/packages/core/database/src/fields/has-one-field.ts b/packages/core/database/src/fields/has-one-field.ts index f72de6018..799660398 100644 --- a/packages/core/database/src/fields/has-one-field.ts +++ b/packages/core/database/src/fields/has-one-field.ts @@ -1,4 +1,4 @@ -import { omit } from 'lodash'; +import lodash, { omit } from 'lodash'; import { AssociationScope, DataType, @@ -8,7 +8,7 @@ import { Utils, } from 'sequelize'; import { Collection } from '../collection'; -import { checkIdentifier } from '../utils'; +import { checkIdentifier, snakeCase } from '../utils'; import { BaseRelationFieldOptions, RelationField } from './relation-field'; import { Reference } from '../features/ReferencesMap'; @@ -84,11 +84,15 @@ export class HasOneField extends RelationField { } get foreignKey() { - if (this.options.foreignKey) { - return this.options.foreignKey; - } - const { model } = this.context.collection; - return Utils.camelize([model.options.name.singular, model.primaryKeyAttribute].join('_')); + const foreignKey = (() => { + if (this.options.foreignKey) { + return this.options.foreignKey; + } + const { model } = this.context.collection; + return Utils.camelize([model.options.name.singular, model.primaryKeyAttribute].join('_')); + })(); + + return foreignKey; } reference(association): Reference { diff --git a/packages/core/database/src/index.ts b/packages/core/database/src/index.ts index ba9a5144e..35f8a2bce 100644 --- a/packages/core/database/src/index.ts +++ b/packages/core/database/src/index.ts @@ -18,3 +18,4 @@ export * from './update-associations'; export * from './collection-importer'; export * from './filter-match'; export * from './field-repository/array-field-repository'; +export { snakeCase } from './utils'; diff --git a/packages/core/database/src/mock-database.ts b/packages/core/database/src/mock-database.ts index 6a993a97d..3e093d081 100644 --- a/packages/core/database/src/mock-database.ts +++ b/packages/core/database/src/mock-database.ts @@ -31,10 +31,11 @@ export function getConfigByEnv() { collate: 'utf8mb4_unicode_ci', }, timezone: process.env.DB_TIMEZONE, + underscored: process.env.DB_UNDERSCORED === 'true', }; } export function mockDatabase(options: IDatabaseOptions = {}): MockDatabase { - const dbOptions = merge(getConfigByEnv(), options); + const dbOptions = merge(getConfigByEnv(), options) as any; return new MockDatabase(dbOptions); } diff --git a/packages/core/database/src/model.ts b/packages/core/database/src/model.ts index 2e1c5af4f..38f189b6e 100644 --- a/packages/core/database/src/model.ts +++ b/packages/core/database/src/model.ts @@ -1,9 +1,8 @@ import lodash from 'lodash'; -import { DataTypes, Model as SequelizeModel, ModelStatic } from 'sequelize'; +import { Model as SequelizeModel, ModelStatic } from 'sequelize'; import { Collection } from './collection'; import { Database } from './database'; import { Field } from './fields'; -import type { InheritedCollection } from './inherited-collection'; import { SyncRunner } from './sync-runner'; const _ = lodash; @@ -28,6 +27,7 @@ export class Model = sortKey.replace('-', '').split('.'); + if (this.database.inDialect('postgres', 'sqlite')) { direction = `${direction} NULLS LAST`; } @@ -86,7 +87,11 @@ export class OptionsParser { sortField[i] = associationModel.associations[associationKey].target; associationModel = sortField[i]; } + } else { + const rawField = this.model.rawAttributes[sortField[0]]; + sortField[0] = rawField?.field || sortField[0]; } + sortField.push(direction); if (this.database.inDialect('mysql')) { orderParams.push([Sequelize.fn('ISNULL', Sequelize.col(`${this.model.name}.${sortField[0]}`))]); diff --git a/packages/core/database/src/relation-repository/belongs-to-many-repository.ts b/packages/core/database/src/relation-repository/belongs-to-many-repository.ts index 1d5438e93..2495036cf 100644 --- a/packages/core/database/src/relation-repository/belongs-to-many-repository.ts +++ b/packages/core/database/src/relation-repository/belongs-to-many-repository.ts @@ -62,6 +62,8 @@ export class BelongsToManyRepository extends MultipleRelationRepository implemen const transaction = await this.getTransaction(options); const association = this.association; + const throughModel = this.throughModel(); + const instancesToIds = (instances) => { return instances.map((instance) => instance.get(this.targetKey())); }; @@ -69,7 +71,7 @@ export class BelongsToManyRepository extends MultipleRelationRepository implemen // Through Table const throughTableWhere: Array = [ { - [association.foreignKey]: this.sourceKeyValue, + [throughModel.rawAttributes[association.foreignKey].field]: this.sourceKeyValue, }, ]; @@ -100,7 +102,7 @@ export class BelongsToManyRepository extends MultipleRelationRepository implemen } throughTableWhere.push({ - [association.otherKey]: { + [throughModel.rawAttributes[association.otherKey].field]: { [Op.in]: ids, }, }); diff --git a/packages/core/database/src/repository.ts b/packages/core/database/src/repository.ts index 5582652e5..fc26bbffb 100644 --- a/packages/core/database/src/repository.ts +++ b/packages/core/database/src/repository.ts @@ -11,7 +11,7 @@ import { Op, Transactionable, UpdateOptions as SequelizeUpdateOptions, - WhereOperators + WhereOperators, } from 'sequelize'; import { Collection } from './collection'; import { Database } from './database'; @@ -407,7 +407,12 @@ export class Repository(values, { @@ -476,7 +481,7 @@ export class Repository; action: UpdateAction; + underscored: boolean; private associationKeysToBeUpdate: AssociationKeysToBeUpdate; private blackList: BlackList; private whiteList: WhiteList; @@ -162,6 +163,11 @@ export class UpdateGuard { guard.setBlackList(options.blacklist); guard.setAction(lodash.get(options, 'action', 'update')); guard.setAssociationKeysToBeUpdate(options.updateAssociationValues); + + if (options.underscored) { + guard.underscored = options.underscored; + } + return guard; } } diff --git a/packages/core/database/src/utils.ts b/packages/core/database/src/utils.ts index 90a92ee42..173fc17b8 100644 --- a/packages/core/database/src/utils.ts +++ b/packages/core/database/src/utils.ts @@ -1,6 +1,7 @@ import crypto from 'crypto'; import { IdentifierError } from './errors/identifier-error'; import { Model } from './model'; +import lodash from 'lodash'; type HandleAppendsQueryOptions = { templateModel: any; @@ -73,3 +74,11 @@ export function checkIdentifier(value: string) { throw new IdentifierError(`Identifier ${value} is too long`); } } + +export function getTableName(collectionName: string, options) { + return options.underscored ? snakeCase(collectionName) : collectionName; +} + +export function snakeCase(name: string) { + return require('sequelize').Utils.underscore(name); +} diff --git a/packages/core/server/src/plugin-manager/PluginManager.ts b/packages/core/server/src/plugin-manager/PluginManager.ts index 559a2ba59..6e986682a 100644 --- a/packages/core/server/src/plugin-manager/PluginManager.ts +++ b/packages/core/server/src/plugin-manager/PluginManager.ts @@ -76,6 +76,7 @@ export class PluginManager { await this.repository.load(); } }); + this.app.on('beforeUpgrade', async () => { await this.collection.sync(); }); @@ -195,6 +196,7 @@ export class PluginManager { if (this.plugins.has(pluginName)) { throw new Error(`plugin name [${pluginName}] already exists`); } + this.plugins.set(pluginName, instance); return instance; } diff --git a/packages/plugins/acl/src/__tests__/list-action.test.ts b/packages/plugins/acl/src/__tests__/list-action.test.ts index cd65c4dcf..90330e056 100644 --- a/packages/plugins/acl/src/__tests__/list-action.test.ts +++ b/packages/plugins/acl/src/__tests__/list-action.test.ts @@ -32,12 +32,22 @@ describe('list action with acl', () => { role: 'user', }); + app.acl.addFixedParams('tests', 'destroy', () => { + return { + filter: { + $and: [{ 'name.$ne': 't1' }, { 'name.$ne': 't2' }], + }, + }; + }); + userRole.grantAction('tests:view', {}); userRole.grantAction('tests:update', { own: true, }); + userRole.grantAction('tests:destroy', {}); + const Test = app.db.collection({ name: 'tests', fields: [ @@ -74,13 +84,13 @@ describe('list action with acl', () => { before: 'acl', }, ); - + const response = await app.agent().set('X-With-ACL-Meta', true).resource('tests').list({}); const data = response.body; expect(data.meta.allowedActions.view).toEqual(['t1', 't2', 't3']); expect(data.meta.allowedActions.update).toEqual(['t1', 't2']); - expect(data.meta.allowedActions.destroy).toEqual([]); + expect(data.meta.allowedActions.destroy).toEqual(['t3']); }); it('should list items with meta permission', async () => { @@ -241,12 +251,15 @@ describe('list association action with acl', () => { }); const userPlugin = app.getPlugin('users'); - const userAgent = app.agent().set('X-With-ACL-Meta', true).auth( - userPlugin.jwtService.sign({ - userId: user.get('id'), - }), - { type: 'bearer' }, - ); + const userAgent = app + .agent() + .set('X-With-ACL-Meta', true) + .auth( + userPlugin.jwtService.sign({ + userId: user.get('id'), + }), + { type: 'bearer' }, + ); await userAgent.resource('posts').create({ values: { diff --git a/packages/plugins/acl/src/server.ts b/packages/plugins/acl/src/server.ts index 470588f42..dd512da09 100644 --- a/packages/plugins/acl/src/server.ts +++ b/packages/plugins/acl/src/server.ts @@ -1,6 +1,6 @@ import { NoPermissionError } from '@nocobase/acl'; import { Context, utils as actionUtils } from '@nocobase/actions'; -import { Collection, RelationField } from '@nocobase/database'; +import { Collection, RelationField, snakeCase } from '@nocobase/database'; import { Plugin } from '@nocobase/server'; import lodash from 'lodash'; import { resolve } from 'path'; @@ -262,6 +262,7 @@ export class PluginACL extends Plugin { this.app.db.on('rolesResources.afterDestroy', async (model, options) => { const role = this.acl.getRole(model.get('roleName')); + if (role) { role.revokeResource(model.get('name')); } @@ -281,6 +282,7 @@ export class PluginACL extends Plugin { const { transaction } = options; const collectionName = model.get('collectionName'); + const fieldName = model.get('name'); const resourceActions = (await this.app.db.getRepository('rolesResourcesActions').find({ @@ -695,16 +697,57 @@ export class PluginACL extends Plugin { const actionSql = ctx.db.sequelize.queryInterface.queryGenerator.selectQuery( Model.getTableName(), { - // ...queryParams, - where: queryParams.where, + where: (() => { + const filterObj = queryParams.where; + if (!this.db.options.underscored) { + return filterObj; + } + + const iterate = (rootObj, path = []) => { + const obj = path.length == 0 ? rootObj : lodash.get(rootObj, path); + + if (Array.isArray(obj)) { + for (let i = 0; i < obj.length; i++) { + if (obj[i] === null) { + continue; + } + + if (typeof obj[i] === 'object') { + iterate(rootObj, [...path, i]); + } + } + + return; + } + + Reflect.ownKeys(obj).forEach((key) => { + if (Array.isArray(obj) && key == 'length') { + return; + } + + if ((typeof obj[key] === 'object' && obj[key] !== null) || typeof obj[key] === 'symbol') { + iterate(rootObj, [...path, key]); + } + + if (typeof key === 'string' && key !== snakeCase(key)) { + lodash.set(rootObj, [...path, snakeCase(key)], lodash.cloneDeep(obj[key])); + lodash.unset(rootObj, [...path, key]); + } + }); + }; + + iterate(filterObj); + + return filterObj; + })(), attributes: [primaryKeyField], includeIgnoreAttributes: false, - // include: queryParams.include, }, Model, ); const whereCase = actionSql.match(/WHERE (.*?);/)[1]; + conditions.push({ whereCase, action, diff --git a/packages/plugins/collection-manager/src/__tests__/collections.repository.test.ts b/packages/plugins/collection-manager/src/__tests__/collections.repository.test.ts index 6fa9869c1..13309e424 100644 --- a/packages/plugins/collection-manager/src/__tests__/collections.repository.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/collections.repository.test.ts @@ -19,6 +19,32 @@ describe('collections repository', () => { await app.destroy(); }); + test('create underscored field', async () => { + if (process.env.DB_UNDERSCORED !== 'true') { + return; + } + + const collection = await Collection.repository.create({ + values: { + name: 'testCollection', + createdAt: true, + fields: [ + { + type: 'date', + field: 'createdAt', + name: 'createdAt', + }, + ], + }, + }); + + await collection.migrate(); + + const testCollection = db.getCollection('testCollection'); + + expect(testCollection.model.rawAttributes.createdAt.field).toEqual('created_at'); + }); + it('case 1', async () => { // 什么都没提供，随机 name 和 key const data = await Collection.repository.create({ @@ -157,4 +183,164 @@ describe('collections repository', () => { ], }); }); + + it('should destroy when fields refer to the same field', async () => { + await Collection.repository.create({ + context: {}, + values: { + name: 'tests', + timestamps: true, + fields: [ + { + type: 'date', + name: 'dateA', + }, + { + type: 'date', + name: 'date_a', + }, + ], + }, + }); + + const testCollection = db.getCollection('tests'); + const getTableInfo = async () => + await db.sequelize.getQueryInterface().describeTable(testCollection.model.tableName); + + const tableInfo0 = await getTableInfo(); + expect(tableInfo0['date_a']).toBeDefined(); + + await Field.repository.destroy({ + context: {}, + filter: { + name: ['dateA', 'date_a'], + }, + }); + + const count = await Field.repository.count(); + expect(count).toBe(0); + const tableInfo1 = await getTableInfo(); + expect(tableInfo1['dateA']).not.toBeDefined(); + expect(tableInfo1['date_a']).not.toBeDefined(); + }); + + it('should not destroy timestamps columns', async () => { + const createdAt = db.options.underscored ? 'created_at' : 'createdAt'; + await Collection.repository.create({ + context: {}, + values: { + name: 'tests', + timestamps: true, + fields: [ + { + type: 'date', + name: 'createdAt', + }, + ], + }, + }); + + const testCollection = db.getCollection('tests'); + const getTableInfo = async () => + await db.sequelize.getQueryInterface().describeTable(testCollection.model.tableName); + + const tableInfo0 = await getTableInfo(); + expect(tableInfo0[createdAt]).toBeDefined(); + + await Field.repository.destroy({ + context: {}, + filter: { + name: 'createdAt', + }, + }); + + const tableInfo1 = await getTableInfo(); + expect(tableInfo1[createdAt]).toBeDefined(); + expect(testCollection.hasField('createdAt')).toBeFalsy(); + expect(testCollection.model.rawAttributes['createdAt']).toBeDefined(); + }); + + it('should not destroy column when column belongs to a field', async () => { + if (db.options.underscored !== true) return; + + await Collection.repository.create({ + context: {}, + values: { + name: 'tests', + fields: [ + { + type: 'string', + name: 'test_field', + }, + { + type: 'string', + name: 'testField', + }, + { + type: 'string', + name: 'test123', + field: 'test_field', + }, + { + type: 'string', + name: 'otherField', + }, + ], + }, + }); + + const testCollection = db.getCollection('tests'); + + expect( + testCollection.model.rawAttributes.test_field.field === testCollection.model.rawAttributes.testField.field, + ).toBe(true); + const getTableInfo = async () => + await db.sequelize.getQueryInterface().describeTable(testCollection.model.tableName); + + const tableInfo0 = await getTableInfo(); + + expect(tableInfo0['other_field']).toBeDefined(); + + await Field.repository.destroy({ + context: {}, + filter: { + name: 'otherField', + }, + }); + + expect(testCollection.model.rawAttributes['otherField']).toBeUndefined(); + const tableInfo1 = await getTableInfo(); + expect(tableInfo1['other_field']).not.toBeDefined(); + + await Field.repository.destroy({ + context: {}, + filter: { + name: 'testField', + }, + }); + + expect(testCollection.model.rawAttributes['testField']).toBeUndefined(); + const tableInfo2 = await getTableInfo(); + expect(tableInfo2['test_field']).toBeDefined(); + + await Field.repository.destroy({ + context: {}, + filter: { + name: 'test_field', + }, + }); + + const tableInfo3 = await getTableInfo(); + expect(tableInfo3['test_field']).toBeDefined(); + + await Field.repository.destroy({ + context: {}, + filter: { + name: 'test123', + }, + }); + + const tableInfo4 = await getTableInfo(); + expect(tableInfo4['test_field']).not.toBeDefined(); + }); }); diff --git a/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts b/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts index 91e9f20f5..f87046628 100644 --- a/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts @@ -69,6 +69,8 @@ describe('field indexes', () => { }, }); + expect(field.status).toBe(200); + // create a record const response1 = await agent.resource(tableName).create({ values: { diff --git a/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts b/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts index bce332781..2998b738b 100644 --- a/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts @@ -38,6 +38,7 @@ describe('reverseField options', () => { reverseField: {}, }, }); + const json = JSON.parse(JSON.stringify(field.toJSON())); expect(json).toMatchObject({ type: 'hasMany', diff --git a/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts b/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts index 4d0652d06..208c4faac 100644 --- a/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts @@ -396,4 +396,144 @@ describe('collections repository', () => { }); expect(response1.body.data.length).toBe(2); }); + + it('should update field with default value', async () => { + const createCollectionResponse = await app + .agent() + .resource('collections') + .create({ + values: { + name: 'test', + fields: [ + { + name: 'testField', + type: 'string', + }, + ], + }, + }); + + const testField = await app.db.getRepository('fields').findOne({ + filter: { + name: 'testField', + }, + }); + + // update field with unique index + const addDefaultValueResponse = await app + .agent() + .resource('fields') + .update({ + values: { + defaultValue: '1231', + }, + filterByTk: testField.get('key'), + }); + + expect(addDefaultValueResponse.statusCode).toEqual(200); + }); + + it('should create collection field', async () => { + await app + .agent() + .resource('collections') + .create({ + values: { + name: 'test', + }, + }); + + const addFieldResponse = await app + .agent() + .resource('fields') + .create({ + values: { + name: 'testField', + collectionName: 'test', + type: 'string', + }, + }); + + expect(addFieldResponse.statusCode).toEqual(200); + + const collection = app.db.getCollection('test'); + + const columnName = collection.model.rawAttributes.testField.field; + + const tableInfo = await app.db.sequelize.getQueryInterface().describeTable(collection.model.tableName); + + expect(tableInfo[columnName]).toBeDefined(); + }); + + it('should update field with unique index', async () => { + const createCollectionResponse = await app + .agent() + .resource('collections') + .create({ + values: { + name: 'test', + fields: [ + { + name: 'testField', + type: 'string', + }, + ], + }, + }); + + expect(createCollectionResponse.statusCode).toEqual(200); + + const testField = await app.db.getRepository('fields').findOne({ + filter: { + name: 'testField', + }, + }); + + // update field with unique index + const addIndexResponse = await app + .agent() + .resource('fields') + .update({ + values: { + unique: true, + }, + filterByTk: testField.get('key'), + }); + + expect(addIndexResponse.statusCode).toEqual(200); + + const indexes = (await app.db.sequelize + .getQueryInterface() + .showIndex(app.db.getCollection('test').model.tableName)) as any; + + const columnName = app.db.getCollection('test').model.rawAttributes.testField.field; + + expect( + indexes.find( + (index) => index.unique == true && index.fields[0].attribute == columnName && index.fields.length === 1, + ), + ).toBeDefined(); + + const removeIndexResponse = await app + .agent() + .resource('fields') + .update({ + values: { + unique: false, + }, + filterByTk: testField.get('key'), + }); + + expect(removeIndexResponse.statusCode).toEqual(200); + + const afterIndexes = (await app.db.sequelize + .getQueryInterface() + .showIndex(app.db.getCollection('test').model.tableName)) as any; + + expect( + afterIndexes.find( + (index) => index.unique == true && index.fields[0].attribute == columnName && index.fields.length === 1, + ), + ).not.toBeDefined(); + }); }); diff --git a/packages/plugins/collection-manager/src/__tests__/http-api/inherited-collection.test.ts b/packages/plugins/collection-manager/src/__tests__/http-api/inherited-collection.test.ts index 6a4be7baf..b4a367bcc 100644 --- a/packages/plugins/collection-manager/src/__tests__/http-api/inherited-collection.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/http-api/inherited-collection.test.ts @@ -64,7 +64,7 @@ pgOnly()('Inherited Collection', () => { expect(response.statusCode).toBe(500); }); - it('can create relation with child table', async () => { + it('can create relation with child table', async () => { await agent.resource('collections').create({ values: { name: 'a', @@ -100,6 +100,8 @@ pgOnly()('Inherited Collection', () => { }, }); + const collectionB = app.db.getCollection('b'); + const res = await agent.resource('b').create({ values: { af: 'a1', diff --git a/packages/plugins/collection-manager/src/__tests__/migrations/update-id-to-bigint.test.ts b/packages/plugins/collection-manager/src/__tests__/migrations/update-id-to-bigint.test.ts index 546461f41..4e88826d7 100644 --- a/packages/plugins/collection-manager/src/__tests__/migrations/update-id-to-bigint.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/migrations/update-id-to-bigint.test.ts @@ -1,5 +1,6 @@ import { Database, MigrationContext } from '@nocobase/database'; import Migrator from '../../migrations/20221121111113-update-id-to-bigint'; +import lodash from 'lodash'; const excludeSqlite = () => (process.env.DB_DIALECT != 'sqlite' ? describe : describe.skip); @@ -72,6 +73,11 @@ excludeSqlite()('update id to bigint test', () => { .describeTable( db.getCollection(collectionName) ? db.getCollection(collectionName).model.tableName : collectionName, ); + + if (process.env.DB_UNDERSCORED) { + fieldName = lodash.snakeCase(fieldName); + } + console.log(`${collectionName}, ${fieldName}`, tableInfo[fieldName].type); expect(tableInfo[fieldName].type).toBe('BIGINT'); }; diff --git a/packages/plugins/collection-manager/src/__tests__/resources/collections.fields.test.ts b/packages/plugins/collection-manager/src/__tests__/resources/collections.fields.test.ts index 584be2cc3..9449c57e0 100644 --- a/packages/plugins/collection-manager/src/__tests__/resources/collections.fields.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/resources/collections.fields.test.ts @@ -27,15 +27,21 @@ describe('collections.fields', () => { ], }, }); + 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(); }); 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 1e7f2d397..48e7cabbe 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 @@ -30,6 +30,7 @@ export default class UpdateIdToBigIntMigrator extends Migration { const queryGenerator = queryInterface.queryGenerator as any; const updateToBigInt = async (model, fieldName) => { + const columnName = model.rawAttributes[fieldName].field; let sql; const tableName = model.tableName; @@ -51,7 +52,7 @@ export default class UpdateIdToBigIntMigrator extends Migration { if (model.rawAttributes[fieldName].type instanceof DataTypes.INTEGER) { if (db.inDialect('postgres')) { - sql = `ALTER TABLE "${tableName}" ALTER COLUMN "${fieldName}" SET DATA TYPE BIGINT;`; + sql = `ALTER TABLE "${tableName}" ALTER COLUMN "${columnName}" SET DATA TYPE BIGINT;`; } else if (db.inDialect('mysql')) { const dataTypeOrOptions = model.rawAttributes[fieldName]; const attributeName = fieldName; @@ -68,6 +69,7 @@ export default class UpdateIdToBigIntMigrator extends Migration { table: tableName, }, ); + sql = queryGenerator.changeColumnQuery(tableName, query); sql = sql.replace(' PRIMARY KEY;', ' ;'); @@ -83,7 +85,7 @@ export default class UpdateIdToBigIntMigrator extends Migration { } if (db.inDialect('postgres')) { - const sequenceQuery = `SELECT pg_get_serial_sequence('"${model.tableName}"', '${fieldName}');`; + const sequenceQuery = `SELECT pg_get_serial_sequence('"${model.tableName}"', '${columnName}');`; const [result] = await this.sequelize.query(sequenceQuery, {}); const sequenceName = result[0]['pg_get_serial_sequence']; diff --git a/packages/plugins/collection-manager/src/models/field.ts b/packages/plugins/collection-manager/src/models/field.ts index 1ab04ed48..239a989b2 100644 --- a/packages/plugins/collection-manager/src/models/field.ts +++ b/packages/plugins/collection-manager/src/models/field.ts @@ -1,5 +1,5 @@ -import Database, { Collection, Field, MagicAttributeModel } from '@nocobase/database'; -import { SyncOptions, Transactionable, UniqueConstraintError } from 'sequelize'; +import Database, { Collection, MagicAttributeModel, snakeCase } from '@nocobase/database'; +import { SyncOptions, Transactionable } from 'sequelize'; interface LoadOptions extends Transactionable { // TODO @@ -10,58 +10,6 @@ interface MigrateOptions extends SyncOptions, Transactionable { isNew?: boolean; } -async function migrate(field: Field, options: MigrateOptions): Promise { - const { unique } = field.options; - const { model } = field.collection; - - const ukName = `${model.tableName}_${field.name}_uk`; - const queryInterface = model.sequelize.getQueryInterface(); - const fieldAttribute = model.rawAttributes[field.name]; - - // @ts-ignore - const existedConstraints = (await queryInterface.showConstraint(model.tableName, ukName, { - transaction: options.transaction, - })) as any[]; - - const constraintBefore = existedConstraints.find((item) => item.constraintName === ukName); - - if (typeof fieldAttribute?.unique !== 'undefined') { - if (constraintBefore && !unique) { - await queryInterface.removeConstraint(model.tableName, ukName, { transaction: options.transaction }); - } - - fieldAttribute.unique = Boolean(constraintBefore); - } - - await field.sync(options); - - if (!constraintBefore && unique) { - await queryInterface.addConstraint(model.tableName, { - type: 'unique', - fields: [field.name], - name: ukName, - transaction: options.transaction, - }); - } - - if (typeof fieldAttribute?.unique !== 'undefined') { - fieldAttribute.unique = unique; - } - - // @ts-ignore - const updatedConstraints = (await queryInterface.showConstraint(model.tableName, ukName, { - transaction: options.transaction, - })) as any[]; - - const indexAfter = updatedConstraints.find((item) => item.constraintName === ukName); - - if (unique && !indexAfter) { - throw new UniqueConstraintError({ - fields: { [field.name]: undefined }, - }); - } -} - export class FieldModel extends MagicAttributeModel { get db(): Database { return (this.constructor).database; @@ -106,10 +54,24 @@ export class FieldModel extends MagicAttributeModel { field = await this.load({ transaction: options.transaction, }); + if (!field) { return; } - await migrate(field, options); + 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) { @@ -136,6 +98,56 @@ export class FieldModel extends MagicAttributeModel { }); } + async syncUniqueIndex(options: Transactionable) { + const unique = this.get('unique'); + const collection = this.getFieldCollection(); + const field = collection.getField(this.get('name')); + const columnName = collection.model.rawAttributes[this.get('name')].field; + const tableName = collection.model.tableName; + + const queryInterface = this.db.sequelize.getQueryInterface() as any; + + const existsIndexes = await queryInterface.showIndex(collection.model.tableName, { + transaction: options.transaction, + }); + + const existUniqueIndex = existsIndexes.find((item) => { + return item.unique && item.fields[0].attribute === columnName && item.fields.length === 1; + }); + + let existsUniqueConstraint; + + let constraintName = `${tableName}_${field.name}_uk`; + + if (existUniqueIndex) { + const existsUniqueConstraints = await queryInterface.showConstraint( + collection.model.tableName, + constraintName, + {}, + ); + + existsUniqueConstraint = existsUniqueConstraints[0]; + } + + if (unique && !existsUniqueConstraint) { + // @ts-ignore + await collection.sync({ ...options, force: false, alter: { drop: false } }); + + await queryInterface.addConstraint(tableName, { + type: 'unique', + fields: [columnName], + name: constraintName, + transaction: options.transaction, + }); + } + + if (!unique && existsUniqueConstraint) { + await queryInterface.removeConstraint(collection.model.tableName, constraintName, { + transaction: options.transaction, + }); + } + } + async syncDefaultValue( options: Transactionable & { defaultValue: any; @@ -153,7 +165,7 @@ export class FieldModel extends MagicAttributeModel { await queryInterface.changeColumn( collection.model.tableName, - this.get('name'), + collection.model.rawAttributes[this.get('name')].field, { type: field.dataType, defaultValue: options.defaultValue, diff --git a/packages/plugins/collection-manager/src/server.ts b/packages/plugins/collection-manager/src/server.ts index 4e7bc518b..0c064edd5 100644 --- a/packages/plugins/collection-manager/src/server.ts +++ b/packages/plugins/collection-manager/src/server.ts @@ -5,6 +5,7 @@ import { UniqueConstraintError } from 'sequelize'; import PluginErrorHandler from '@nocobase/plugin-error-handler'; import { Plugin } from '@nocobase/server'; +import { Mutex } from 'async-mutex'; import { CollectionRepository } from '.'; import { @@ -122,7 +123,7 @@ export class CollectionManagerPlugin extends Plugin { const next = currentOptions['unique']; if (Boolean(prev) !== Boolean(next)) { - await model.migrate({ transaction }); + await model.syncUniqueIndex({ transaction }); } } @@ -149,8 +150,12 @@ export class CollectionManagerPlugin extends Plugin { // before field remove this.app.db.on('fields.beforeDestroy', beforeDestroyForeignKey(this.app.db)); + + const mutex = new Mutex(); this.app.db.on('fields.beforeDestroy', async (model: FieldModel, options) => { - await model.remove(options); + await mutex.runExclusive(async () => { + await model.remove(options); + }); }); this.app.db.on('collections.beforeDestroy', async (model: CollectionModel, options) => { diff --git a/packages/plugins/duplicator/src/server/__tests__/dump.test.ts b/packages/plugins/duplicator/src/server/__tests__/dump.test.ts index b6c119b41..cb532bdb2 100644 --- a/packages/plugins/duplicator/src/server/__tests__/dump.test.ts +++ b/packages/plugins/duplicator/src/server/__tests__/dump.test.ts @@ -1,5 +1,5 @@ import { mockServer, MockServer } from '@nocobase/test'; -import { Database } from '@nocobase/database'; +import { Database, Model } from '@nocobase/database'; import * as os from 'os'; import path from 'path'; import lodash from 'lodash'; @@ -98,7 +98,11 @@ describe('dump', () => { const collectionMeta = JSON.parse(collectionMetaFile); expect(collectionMeta.count).toEqual(2); - expect(collectionMeta.columns).toEqual(Object.keys(db.getCollection('users').model.rawAttributes)); + expect(collectionMeta.columns).toEqual( + Object.keys(db.getCollection('users').model.rawAttributes).map( + (fieldName) => db.getCollection('users').model.rawAttributes[fieldName].field, + ), + ); const dataPath = path.resolve(testDir, 'collections', 'users', 'data'); diff --git a/packages/plugins/file-manager/src/server/__tests__/attachment.test.ts b/packages/plugins/file-manager/src/server/__tests__/attachment.test.ts new file mode 100644 index 000000000..8c529eb70 --- /dev/null +++ b/packages/plugins/file-manager/src/server/__tests__/attachment.test.ts @@ -0,0 +1,50 @@ +import { getApp } from '.'; + +describe('attachment', () => { + let db; + let app; + + beforeEach(async () => { + app = await getApp(); + db = app.db; + }); + + afterEach(async () => { + await app.destroy(); + }); + + it('should linked to a instance', async () => { + const testCollection = db.collection({ + name: 'test', + fields: [ + { + name: 'name', + type: 'string', + }, + { + type: 'belongsTo', + name: 'logo', + target: 'attachments', + }, + ], + }); + + await db.sync(); + + await testCollection.repository.create({ + values: { + name: 'test', + logo: { + title: 'nocobase-logo', + filename: '682e5ad037dd02a0fe4800a3e91c283b.png', + extname: '.png', + mimetype: 'image/png', + url: 'https://nocobase.oss-cn-beijing.aliyuncs.com/682e5ad037dd02a0fe4800a3e91c283b.png', + }, + }, + }); + + const item = await testCollection.repository.findOne({}); + expect(item.get('logoId')).toBeDefined(); + }); +}); diff --git a/packages/plugins/file-manager/src/server/__tests__/index.ts b/packages/plugins/file-manager/src/server/__tests__/index.ts index 67e49fed5..d7b1853b3 100644 --- a/packages/plugins/file-manager/src/server/__tests__/index.ts +++ b/packages/plugins/file-manager/src/server/__tests__/index.ts @@ -13,6 +13,8 @@ export async function getApp(options = {}): Promise { acl: false, }); + await app.cleanDb(); + app.plugin(plugin); app.db.import({ diff --git a/packages/plugins/workflow/src/server/__tests__/index.ts b/packages/plugins/workflow/src/server/__tests__/index.ts index eba067cb2..d23369c5f 100644 --- a/packages/plugins/workflow/src/server/__tests__/index.ts +++ b/packages/plugins/workflow/src/server/__tests__/index.ts @@ -7,7 +7,7 @@ import calculators from '../calculators'; import { JOB_STATUS } from '../constants'; export function sleep(ms: number) { - return new Promise(resolve => { + return new Promise((resolve) => { setTimeout(resolve, ms); }); } @@ -26,38 +26,40 @@ export async function getApp({ manual, ...options }: MockAppOptions = {}): Promi run(node, { result }, execution) { return { status: JOB_STATUS.RESOLVED, - result + result, }; - } + }, }, error: { run(node, input, execution) { throw new Error('definite error'); - } + }, }, 'prompt->error': { run(node, input, execution) { return { - status: JOB_STATUS.PENDING + status: JOB_STATUS.PENDING, }; }, resume(node, input, execution) { throw new Error('input failed'); - } - } - } + }, + }, + }, }); if (!calculators.get('no1')) { calculators.register('no1', () => 1); } + await app.db.clean({ drop: true }); + await app.load(); await app.db.import({ - directory: path.resolve(__dirname, './collections') + directory: path.resolve(__dirname, './collections'), }); try { diff --git a/packages/plugins/workflow/src/server/__tests__/instructions/delay.test.ts b/packages/plugins/workflow/src/server/__tests__/instructions/delay.test.ts index b87bfc2b2..3164f35e4 100644 --- a/packages/plugins/workflow/src/server/__tests__/instructions/delay.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/instructions/delay.test.ts @@ -3,8 +3,6 @@ import Database from '@nocobase/database'; import { getApp, sleep } from '..'; import { EXECUTION_STATUS, JOB_STATUS } from '../../constants'; - - describe('workflow > instructions > delay', () => { let app: Application; let db: Database; @@ -16,6 +14,7 @@ describe('workflow > instructions > delay', () => { app = await getApp(); db = app.db; + WorkflowModel = db.getCollection('workflows').model; PostRepo = db.getCollection('posts').repository; @@ -24,8 +23,8 @@ describe('workflow > instructions > delay', () => { type: 'collection', config: { mode: 1, - collection: 'posts' - } + collection: 'posts', + }, }); }); @@ -37,8 +36,8 @@ describe('workflow > instructions > delay', () => { type: 'delay', config: { duration: 2000, - endStatus: JOB_STATUS.RESOLVED - } + endStatus: JOB_STATUS.RESOLVED, + }, }); const post = await PostRepo.create({ values: { title: 't1' } }); @@ -63,8 +62,8 @@ describe('workflow > instructions > delay', () => { type: 'delay', config: { duration: 2000, - endStatus: JOB_STATUS.REJECTED - } + endStatus: JOB_STATUS.REJECTED, + }, }); const post = await PostRepo.create({ values: { title: 't1' } }); @@ -89,8 +88,8 @@ describe('workflow > instructions > delay', () => { type: 'delay', config: { duration: 2000, - endStatus: JOB_STATUS.RESOLVED - } + endStatus: JOB_STATUS.RESOLVED, + }, }); const n2 = await workflow.createNode({ type: 'create', @@ -98,11 +97,11 @@ describe('workflow > instructions > delay', () => { collection: 'comment', params: { values: { - status: 'should be number but use string to raise an error' - } - } + status: 'should be number but use string to raise an error', + }, + }, }, - upstreamId: n1.id + upstreamId: n1.id, }); await n1.setDownstream(n2); @@ -131,8 +130,8 @@ describe('workflow > instructions > delay', () => { type: 'delay', config: { duration: 2000, - endStatus: JOB_STATUS.RESOLVED - } + endStatus: JOB_STATUS.RESOLVED, + }, }); }); diff --git a/packages/samples/shop-actions/__tests__/server/index.test.ts b/packages/samples/shop-actions/__tests__/server/index.test.ts index 479562523..70fa3b91b 100644 --- a/packages/samples/shop-actions/__tests__/server/index.test.ts +++ b/packages/samples/shop-actions/__tests__/server/index.test.ts @@ -8,6 +8,8 @@ describe('shop actions', () => { beforeEach(async () => { app = mockServer(); + + await app.cleanDb(); app.plugin(Plugin); agent = app.agent(); db = app.db; @@ -26,15 +28,16 @@ describe('shop actions', () => { title: 'iPhone 14 Pro', price: 7999, enabled: true, - inventory: 1 - } + inventory: 1, + }, }); + expect(product.data.price).toEqual(7999); const { body: order } = await agent.resource('orders').create({ values: { - productId: product.data.id - } + productId: product.data.id, + }, }); expect(order.data.totalPrice).toEqual(7999); expect(order.data.status).toEqual(0); @@ -43,8 +46,8 @@ describe('shop actions', () => { filterByTk: order.data.id, values: { provider: 'SF', - trackingNumber: '123456789' - } + trackingNumber: '123456789', + }, }); expect(deliveredOrder.data.status).toBe(2); expect(deliveredOrder.data.delivery.trackingNumber).toBe('123456789');