From d1fb3c92d8a4e3dd84a1d99ee79912976919014d Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Sun, 5 Mar 2023 14:45:56 +0800 Subject: [PATCH] test: with collection_manager_schema env (#1532) * test: with collection_manager_schema env * fix: remove collection * fix: collection test * fix: collection exist in db with custom schema * fix: inherited with custom collection schema * fix: build error * fix: sync unique index & database logger --- .github/workflows/nocobase-test.yml | 2 + package.json | 4 ++ packages/core/database/package.json | 1 + packages/core/database/src/collection.ts | 8 +++- packages/core/database/src/database.ts | 24 +++++++---- .../query-interface/mysql-query-interface.ts | 20 +++++++++ .../postgres-query-interface.ts | 22 ++++++++++ .../query-interface-builder.ts | 14 +++++++ .../src/query-interface/query-interface.ts | 12 ++++++ .../query-interface/sqlite-query-interface.ts | 18 ++++++++ packages/core/database/src/sync-runner.ts | 41 +++++++++---------- packages/core/database/src/utils.ts | 7 ++-- packages/core/server/src/application.ts | 5 ++- .../__tests__/collections.repository.test.ts | 6 +-- .../field-options/default-value.test.ts | 2 +- .../__tests__/http-api/collections.test.ts | 2 +- .../__tests__/resources/collections.test.ts | 13 +++--- .../collection-manager/src/models/field.ts | 12 +++++- 18 files changed, 164 insertions(+), 49 deletions(-) create mode 100644 packages/core/database/src/query-interface/mysql-query-interface.ts create mode 100644 packages/core/database/src/query-interface/postgres-query-interface.ts create mode 100644 packages/core/database/src/query-interface/query-interface-builder.ts create mode 100644 packages/core/database/src/query-interface/query-interface.ts create mode 100644 packages/core/database/src/query-interface/sqlite-query-interface.ts diff --git a/.github/workflows/nocobase-test.yml b/.github/workflows/nocobase-test.yml index 378b57ad6b..04f9829b7a 100644 --- a/.github/workflows/nocobase-test.yml +++ b/.github/workflows/nocobase-test.yml @@ -56,6 +56,7 @@ jobs: node_version: ['18'] underscored: [true, false] schema: [public, nocobase] + collection_schema: [public, user_schema] runs-on: ubuntu-latest container: node:${{ matrix.node_version }} services: @@ -93,6 +94,7 @@ jobs: DB_DATABASE: nocobase DB_UNDERSCORED: ${{ matrix.underscored }} DB_SCHEMA: ${{ matrix.schema }} + COLLECTION_MANAGER_SCHEMA: ${{ matrix.collection_schema }} mysql-test: strategy: diff --git a/package.json b/package.json index 8f2b816c3e..0ba49ffd6e 100644 --- a/package.json +++ b/package.json @@ -44,5 +44,9 @@ "prettier": "^2.2.1", "pretty-format": "^24.0.0", "pretty-quick": "^3.1.0" + }, + "volta": { + "node": "18.14.2", + "yarn": "1.22.19" } } diff --git a/packages/core/database/package.json b/packages/core/database/package.json index 104836e570..a98cc44496 100644 --- a/packages/core/database/package.json +++ b/packages/core/database/package.json @@ -7,6 +7,7 @@ "license": "Apache-2.0", "dependencies": { "@nocobase/utils": "0.9.1-alpha.1", + "@nocobase/logger": "0.9.1-alpha.1", "async-mutex": "^0.3.2", "cron-parser": "4.4.0", "deepmerge": "^4.2.2", diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 98c1a68d05..2a36e207ef 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -308,13 +308,13 @@ export class Collection< }) ) { const queryInterface = this.db.sequelize.getQueryInterface(); - await queryInterface.dropTable(this.model.tableName, options); + await queryInterface.dropTable(this.addSchemaTableName(), options); } this.remove(); } async existsInDb(options?: Transactionable) { - return this.db.collectionExistsInDb(this.name, options); + return this.db.queryInterface.collectionTableExists(this, options); } removeField(name: string): void | Field { @@ -562,6 +562,10 @@ export class Collection< return this.db.options.schema; } + if (this.db.inDialect('postgres')) { + return 'public'; + } + return undefined; } } diff --git a/packages/core/database/src/database.ts b/packages/core/database/src/database.ts index 0dfb34c919..439b1f2691 100644 --- a/packages/core/database/src/database.ts +++ b/packages/core/database/src/database.ts @@ -15,7 +15,7 @@ import { Sequelize, SyncOptions, Transactionable, - Utils + Utils, } from 'sequelize'; import { SequelizeStorage, Umzug } from 'umzug'; import { Collection, CollectionOptions, RepositoryType } from './collection'; @@ -58,12 +58,15 @@ import { SyncListener, UpdateListener, UpdateWithAssociationsListener, - ValidateListener + ValidateListener, } from './types'; import { patchSequelizeQueryInterface, snakeCase } from './utils'; import DatabaseUtils from './database-utils'; import { BaseValueParser, registerFieldValueParsers } from './value-parsers'; +import buildQueryInterface from './query-interface/query-interface-builder'; +import QueryInterface from './query-interface/query-interface'; +import { Logger } from '@nocobase/logger'; export interface MergeOptions extends merge.Options {} @@ -166,6 +169,8 @@ export class Database extends EventEmitter implements AsyncEmitter { modelCollection = new Map, Collection>(); tableNameCollectionMap = new Map(); + queryInterface: QueryInterface; + utils = new DatabaseUtils(this); referenceMap = new ReferencesMap(); inheritanceMap = new InheritanceMap(); @@ -177,6 +182,8 @@ export class Database extends EventEmitter implements AsyncEmitter { delayCollectionExtend = new Map(); + logger: Logger; + constructor(options: DatabaseOptions) { super(); @@ -212,6 +219,8 @@ export class Database extends EventEmitter implements AsyncEmitter { this.sequelize = new Sequelize(opts); + this.queryInterface = buildQueryInterface(this); + this.collections = new Map(); this.modelHook = new ModelHook(this); @@ -280,6 +289,10 @@ export class Database extends EventEmitter implements AsyncEmitter { patchSequelizeQueryInterface(this); } + setLogger(logger: Logger) { + this.logger = logger; + } + initListener() { this.on('beforeDefine', (model, options) => { if (this.options.underscored) { @@ -631,15 +644,12 @@ export class Database extends EventEmitter implements AsyncEmitter { 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.includes(this.getCollection(name).model.tableName); + return await this.queryInterface.collectionTableExists(collection, options); } public isSqliteMemory() { diff --git a/packages/core/database/src/query-interface/mysql-query-interface.ts b/packages/core/database/src/query-interface/mysql-query-interface.ts new file mode 100644 index 0000000000..0f724ff8eb --- /dev/null +++ b/packages/core/database/src/query-interface/mysql-query-interface.ts @@ -0,0 +1,20 @@ +import QueryInterface from './query-interface'; +import { Collection } from '../collection'; +import { Transactionable } from 'sequelize'; + +export default class MysqlQueryInterface extends QueryInterface { + constructor(db) { + super(db); + } + + async collectionTableExists(collection: Collection, options?: Transactionable) { + const transaction = options?.transaction; + + const tableName = collection.model.tableName; + const databaseName = this.db.options.database; + const sql = `SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = '${databaseName}' AND TABLE_NAME = '${tableName}'`; + + const results = await this.db.sequelize.query(sql, { type: 'SELECT', transaction }); + return results.length > 0; + } +} diff --git a/packages/core/database/src/query-interface/postgres-query-interface.ts b/packages/core/database/src/query-interface/postgres-query-interface.ts new file mode 100644 index 0000000000..c01fe9509d --- /dev/null +++ b/packages/core/database/src/query-interface/postgres-query-interface.ts @@ -0,0 +1,22 @@ +import QueryInterface from './query-interface'; +import { Collection } from '../collection'; + +export default class PostgresQueryInterface extends QueryInterface { + constructor(db) { + super(db); + } + + async collectionTableExists(collection: Collection, options?) { + const transaction = options?.transaction; + + const tableName = collection.model.tableName; + const schema = collection.collectionSchema() || 'public'; + + const sql = `SELECT EXISTS(SELECT 1 FROM information_schema.tables + WHERE table_schema = '${schema}' + AND table_name = '${tableName}')`; + + const results = await this.db.sequelize.query(sql, { type: 'SELECT', transaction }); + return results[0]['exists']; + } +} diff --git a/packages/core/database/src/query-interface/query-interface-builder.ts b/packages/core/database/src/query-interface/query-interface-builder.ts new file mode 100644 index 0000000000..7944099ffd --- /dev/null +++ b/packages/core/database/src/query-interface/query-interface-builder.ts @@ -0,0 +1,14 @@ +import Database from '../database'; +import MysqlQueryInterface from './mysql-query-interface'; +import PostgresQueryInterface from './postgres-query-interface'; +import SqliteQueryInterface from './sqlite-query-interface'; + +export default function buildQueryInterface(db: Database) { + const map = { + mysql: MysqlQueryInterface, + postgres: PostgresQueryInterface, + sqlite: SqliteQueryInterface, + }; + + return new map[db.options.dialect](db); +} diff --git a/packages/core/database/src/query-interface/query-interface.ts b/packages/core/database/src/query-interface/query-interface.ts new file mode 100644 index 0000000000..311da005ab --- /dev/null +++ b/packages/core/database/src/query-interface/query-interface.ts @@ -0,0 +1,12 @@ +import Database from '../database'; +import { Collection } from '../collection'; +import { QueryInterface as SequelizeQueryInterface, Transactionable } from 'sequelize'; + +export default abstract class QueryInterface { + sequelizeQueryInterface: SequelizeQueryInterface; + protected constructor(public db: Database) { + this.sequelizeQueryInterface = db.sequelize.getQueryInterface(); + } + + abstract collectionTableExists(collection: Collection, options?: Transactionable): Promise; +} diff --git a/packages/core/database/src/query-interface/sqlite-query-interface.ts b/packages/core/database/src/query-interface/sqlite-query-interface.ts new file mode 100644 index 0000000000..9ac5e8a776 --- /dev/null +++ b/packages/core/database/src/query-interface/sqlite-query-interface.ts @@ -0,0 +1,18 @@ +import QueryInterface from './query-interface'; +import { Collection } from '../collection'; + +export default class SqliteQueryInterface extends QueryInterface { + constructor(db) { + super(db); + } + + async collectionTableExists(collection: Collection, options?) { + const transaction = options?.transaction; + + const tableName = collection.model.tableName; + + const sql = `SELECT name FROM sqlite_master WHERE type='table' AND name='${tableName}';`; + const results = await this.db.sequelize.query(sql, { type: 'SELECT', transaction }); + return results.length > 0; + } +} diff --git a/packages/core/database/src/sync-runner.ts b/packages/core/database/src/sync-runner.ts index 0e158c26fc..b561f2ec9d 100644 --- a/packages/core/database/src/sync-runner.ts +++ b/packages/core/database/src/sync-runner.ts @@ -7,7 +7,6 @@ export class SyncRunner { const inheritedCollection = model.collection as InheritedCollection; const db = inheritedCollection.context.database; - const schemaName = db.options.schema || 'public'; const dialect = db.sequelize.getDialect(); @@ -27,12 +26,7 @@ export class SyncRunner { ); } - const parentTables = parents.map((parent) => parent.model.tableName); - - const tableName = model.tableName; - - const schemaTableName = db.utils.addSchema(tableName); - const quoteTableName = db.utils.quoteTable(tableName); + const tableName = inheritedCollection.addSchemaTableName(); const attributes = model.tableAttributes; @@ -43,13 +37,14 @@ export class SyncRunner { let maxSequenceVal = 0; let maxSequenceName; + // find max sequence if (childAttributes.id && childAttributes.id.autoIncrement) { - for (const parent of parentTables) { + for (const parent of parents) { const sequenceNameResult = await queryInterface.sequelize.query( `SELECT column_default FROM information_schema.columns - WHERE table_name = '${parent}' - and table_schema = '${schemaName}' + WHERE table_name = '${parent.model.tableName}' + and table_schema = '${parent.collectionSchema()}' and "column_name" = 'id';`, { transaction, @@ -88,22 +83,24 @@ export class SyncRunner { } } - await this.createTable(schemaTableName, childAttributes, options, model, parentTables, db); + await this.createTable(tableName, childAttributes, options, model, parents); + // if we have max sequence, set it to child table if (maxSequenceName) { - const parentsDeep = Array.from(db.inheritanceMap.getParents(inheritedCollection.name)).map( - (parent) => db.getCollection(parent).model.tableName, + const parentsDeep = Array.from(db.inheritanceMap.getParents(inheritedCollection.name)).map((parent) => + db.getCollection(parent).addSchemaTableName(), ); - const sequenceTables = [...parentsDeep, tableName.toString()]; + const sequenceTables = [...parentsDeep, tableName]; for (const sequenceTable of sequenceTables) { - const queryName = - Boolean(sequenceTable.match(/[A-Z]/)) && !sequenceTable.includes(`"`) ? `"${sequenceTable}"` : sequenceTable; + const tableName = sequenceTable.tableName; + const schemaName = sequenceTable.schema; + + const queryName = Boolean(tableName.match(/[A-Z]/)) && !tableName.includes(`"`) ? `"${tableName}"` : tableName; const idColumnQuery = await queryInterface.sequelize.query( - ` - SELECT column_name + `SELECT column_name FROM information_schema.columns WHERE table_name='${queryName}' and column_name='id' and table_schema = '${schemaName}'; `, @@ -117,7 +114,7 @@ WHERE table_name='${queryName}' and column_name='id' and table_schema = '${schem } await queryInterface.sequelize.query( - `alter table "${schemaName}"."${sequenceTable}" + `alter table ${db.utils.quoteTable(sequenceTable)} alter column id set default nextval('${maxSequenceName}')`, { transaction, @@ -139,7 +136,7 @@ WHERE table_name='${queryName}' and column_name='id' and table_schema = '${schem } } - static async createTable(tableName, attributes, options, model, parentTables, db) { + static async createTable(tableName, attributes, options, model, parents) { let sql = ''; options = { ...options }; @@ -164,9 +161,9 @@ WHERE table_name='${queryName}' and column_name='id' and table_schema = '${schem sql = `${queryGenerator.createTableQuery(tableName, attributes, options)}`.replace( ';', - ` INHERITS (${parentTables + ` INHERITS (${parents .map((t) => { - return db.utils.quoteTable(db.utils.addSchema(t, db.options.schema)); + return t.addSchemaTableName(); }) .join(', ')});`, ); diff --git a/packages/core/database/src/utils.ts b/packages/core/database/src/utils.ts index fb1cb826cc..a44ea02e7b 100644 --- a/packages/core/database/src/utils.ts +++ b/packages/core/database/src/utils.ts @@ -2,6 +2,7 @@ import crypto from 'crypto'; import Database from './database'; import { IdentifierError } from './errors/identifier-error'; import { Model } from './model'; +import lodash from 'lodash'; type HandleAppendsQueryOptions = { templateModel: any; @@ -96,15 +97,15 @@ function patchShowConstraintsQuery(queryGenerator, db) { 'is_deferrable AS "isDeferrable",', 'initially_deferred AS "initiallyDeferred"', 'from INFORMATION_SCHEMA.table_constraints', - `WHERE table_name='${tableName}'`, + `WHERE table_name='${lodash.isPlainObject(tableName) ? tableName.tableName : tableName}'`, ]; if (!constraintName) { lines.push(`AND constraint_name='${constraintName}'`); } - if (db.options.schema && db.options.schema !== 'public') { - lines.push(`AND table_schema='${db.options.schema}'`); + if (lodash.isPlainObject(tableName) && tableName.schema) { + lines.push(`AND table_schema='${tableName.schema}'`); } return lines.join(' '); diff --git a/packages/core/server/src/application.ts b/packages/core/server/src/application.ts index 8be425b590..74fce85b89 100644 --- a/packages/core/server/src/application.ts +++ b/packages/core/server/src/application.ts @@ -283,12 +283,15 @@ export class Application exten } private createDatabase(options: ApplicationOptions) { - return new Database({ + const db = new Database({ ...(options.database instanceof Database ? options.database.options : options.database), migrator: { context: { app: this }, }, }); + + db.setLogger(this._logger); + return db; } getVersion() { 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 bbfff11b78..b207355ffd 100644 --- a/packages/plugins/collection-manager/src/__tests__/collections.repository.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/collections.repository.test.ts @@ -249,7 +249,7 @@ describe('collections repository', () => { const testCollection = db.getCollection('tests'); const getTableInfo = async () => - await db.sequelize.getQueryInterface().describeTable(testCollection.model.tableName); + await db.sequelize.getQueryInterface().describeTable(testCollection.addSchemaTableName()); const tableInfo0 = await getTableInfo(); expect(tableInfo0['date_a']).toBeDefined(); @@ -286,7 +286,7 @@ describe('collections repository', () => { const testCollection = db.getCollection('tests'); const getTableInfo = async () => - await db.sequelize.getQueryInterface().describeTable(testCollection.model.tableName); + await db.sequelize.getQueryInterface().describeTable(testCollection.addSchemaTableName()); const tableInfo0 = await getTableInfo(); expect(tableInfo0[createdAt]).toBeDefined(); @@ -339,7 +339,7 @@ describe('collections repository', () => { testCollection.model.rawAttributes.test_field.field === testCollection.model.rawAttributes.testField.field, ).toBe(true); const getTableInfo = async () => - await db.sequelize.getQueryInterface().describeTable(testCollection.model.tableName); + await db.sequelize.getQueryInterface().describeTable(testCollection.addSchemaTableName()); const tableInfo0 = await getTableInfo(); diff --git a/packages/plugins/collection-manager/src/__tests__/field-options/default-value.test.ts b/packages/plugins/collection-manager/src/__tests__/field-options/default-value.test.ts index be546ab3fc..bf02741111 100644 --- a/packages/plugins/collection-manager/src/__tests__/field-options/default-value.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/field-options/default-value.test.ts @@ -59,7 +59,7 @@ describe('field defaultValue', () => { const response2 = await app.agent().resource('test1').create(); expect(response2.body.data.field1).toBe('cba'); - const results = await app.db.sequelize.getQueryInterface().describeTable(TestCollection.model.tableName); + const results = await app.db.sequelize.getQueryInterface().describeTable(TestCollection.addSchemaTableName()); expect(results.field1.defaultValue).toBe('cba'); }); 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 d3d3f61fa8..9a68ea76c8 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 @@ -460,7 +460,7 @@ describe('collections repository', () => { const columnName = collection.model.rawAttributes.testField.field; - const tableInfo = await app.db.sequelize.getQueryInterface().describeTable(collection.model.tableName); + const tableInfo = await app.db.sequelize.getQueryInterface().describeTable(collection.addSchemaTableName()); expect(tableInfo[columnName]).toBeDefined(); }); diff --git a/packages/plugins/collection-manager/src/__tests__/resources/collections.test.ts b/packages/plugins/collection-manager/src/__tests__/resources/collections.test.ts index 2fe99c2cc9..10e74434ad 100644 --- a/packages/plugins/collection-manager/src/__tests__/resources/collections.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/resources/collections.test.ts @@ -13,7 +13,7 @@ describe('collections', () => { await app.destroy(); }); - test('remove collection', async () => { + test('remove collection 1', async () => { await app .agent() .resource('collections') @@ -23,16 +23,15 @@ describe('collections', () => { }, }); const collection = app.db.getCollection('test'); - const r1 = await collection.existsInDb(); - expect(r1).toBe(true); + expect(await collection.existsInDb()).toBeTruthy(); await app.agent().resource('collections').destroy({ filterByTk: 'test', }); - const r2 = await collection.existsInDb(); - expect(r2).toBe(false); + + expect(await collection.existsInDb()).toBeFalsy(); }); - test('remove collection', async () => { + test('remove collection 2', async () => { await app .agent() .resource('collections') @@ -77,7 +76,7 @@ describe('collections', () => { expect(count).toBe(0); }); - test('remove collection', async () => { + test('remove collection 3', async () => { await app .agent() .resource('collections') diff --git a/packages/plugins/collection-manager/src/models/field.ts b/packages/plugins/collection-manager/src/models/field.ts index 783ae522fc..97a475abbb 100644 --- a/packages/plugins/collection-manager/src/models/field.ts +++ b/packages/plugins/collection-manager/src/models/field.ts @@ -1,4 +1,4 @@ -import Database, { Collection, MagicAttributeModel, snakeCase } from '@nocobase/database'; +import Database, { Collection, MagicAttributeModel } from '@nocobase/database'; import { SyncOptions, Transactionable } from 'sequelize'; interface LoadOptions extends Transactionable { @@ -120,7 +120,11 @@ export class FieldModel extends MagicAttributeModel { let constraintName = `${tableName}_${field.name}_uk`; if (existUniqueIndex) { - const existsUniqueConstraints = await queryInterface.showConstraint(tableName, constraintName, {}); + const existsUniqueConstraints = await queryInterface.showConstraint( + collection.addSchemaTableName(), + constraintName, + {}, + ); existsUniqueConstraint = existsUniqueConstraints[0]; } @@ -135,12 +139,16 @@ export class FieldModel extends MagicAttributeModel { name: constraintName, transaction: options.transaction, }); + + this.db.logger.info(`add unique index ${constraintName}`); } if (!unique && existsUniqueConstraint) { await queryInterface.removeConstraint(collection.addSchemaTableName(), constraintName, { transaction: options.transaction, }); + + this.db.logger.info(`remove unique index ${constraintName}`); } }