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

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

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

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

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

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<ModelStatic<any>, Collection>();
   tableNameCollectionMap = new Map<string, Collection>();
 
+  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<string, { collectionOptions: CollectionOptions; mergeOptions?: any }[]>();
 
+  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({
+    return await this.queryInterface.collectionTableExists(collection, options);
-      transaction: options?.transaction,
-    });
-
-    return tables.includes(this.getCollection(name).model.tableName);
   }
 
   public isSqliteMemory() {

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

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'];
+  }
+}

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

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<boolean>;
+}

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

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 = inheritedCollection.addSchemaTableName();
-
-    const tableName = model.tableName;
-
-    const schemaTableName = db.utils.addSchema(tableName);
-    const quoteTableName = db.utils.quoteTable(tableName);
 
     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}'
+           WHERE table_name = '${parent.model.tableName}'
-            and table_schema = '${schemaName}'
+            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(
+      const parentsDeep = Array.from(db.inheritanceMap.getParents(inheritedCollection.name)).map((parent) =>
-        (parent) => db.getCollection(parent).model.tableName,
+        db.getCollection(parent).addSchemaTableName(),
       );
 
-      const sequenceTables = [...parentsDeep, tableName.toString()];
+      const sequenceTables = [...parentsDeep, tableName];
 
       for (const sequenceTable of sequenceTables) {
-        const queryName =
+        const tableName = sequenceTable.tableName;
-          Boolean(sequenceTable.match(/[A-Z]/)) && !sequenceTable.includes(`"`) ? `"${sequenceTable}"` : sequenceTable;
+        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(', ')});`,
     );

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') {
+    if (lodash.isPlainObject(tableName) && tableName.schema) {
-      lines.push(`AND table_schema='${db.options.schema}'`);
+      lines.push(`AND table_schema='${tableName.schema}'`);
     }
 
     return lines.join(' ');

packages/core/server/src/application.ts

@@ -283,12 +283,15 @@ export class Application<StateT = DefaultState, ContextT = DefaultContext> 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() {

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

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

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

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(await collection.existsInDb()).toBeTruthy();
-    expect(r1).toBe(true);
     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')

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