diff --git a/packages/core/data-source-manager/src/__tests__/database-data-source.test.ts b/packages/core/data-source-manager/src/__tests__/database-data-source.test.ts index bbae3282e44..9ef6290d1b7 100644 --- a/packages/core/data-source-manager/src/__tests__/database-data-source.test.ts +++ b/packages/core/data-source-manager/src/__tests__/database-data-source.test.ts @@ -10,6 +10,47 @@ import { DatabaseDataSource } from '@nocobase/data-source-manager'; describe('database data source', () => { + it('should preserve customized field display name during introspection merge', () => { + const dataSource = Object.create(DatabaseDataSource.prototype) as DatabaseDataSource; + + const [collection] = dataSource.mergeWithLoadedCollections( + [ + { + name: 'departments', + tableName: 'departments', + fields: [ + { + name: 'name', + field: 'name', + rawType: 'VARCHAR', + type: 'string', + uiSchema: { + title: 'name', + }, + }, + ], + }, + ], + { + departments: { + name: 'departments', + fields: [ + { + name: 'name', + field: 'name', + rawType: 'VARCHAR', + type: 'string', + uiSchema: { + title: 'Department name', + }, + }, + ], + }, + }, + ); + + expect(collection.fields[0].uiSchema.title).toBe('Department name'); + }); it('should preserve mapped fields regardless of loaded field order', () => { const dataSource = Object.create(DatabaseDataSource.prototype) as DatabaseDataSource; diff --git a/packages/core/data-source-manager/src/database-data-source.ts b/packages/core/data-source-manager/src/database-data-source.ts index de721dec0f4..39feeb4dc41 100644 --- a/packages/core/data-source-manager/src/database-data-source.ts +++ b/packages/core/data-source-manager/src/database-data-source.ts @@ -18,6 +18,18 @@ import { SequelizeCollectionManager } from './sequelize-collection-manager'; const PRESERVED_LOGICAL_FIELD_TYPES_ON_SYNC: readonly string[] = ['formula']; +export type LoadedCollectionOptions = { + name: string; + fields?: FieldOptions[]; + [key: string]: unknown; +}; + +export type LoadedCollections = Record; + +export type LoadTablesOptions = { + localData?: LoadedCollections; +}; + export abstract class DatabaseDataSource extends DataSource { declare introspector: T; @@ -41,13 +53,11 @@ export abstract class DatabaseDataSource; - abstract loadTables(ctx: Context, tables: string[]): Promise; + abstract loadTables(ctx: Context, tables: string[], options?: LoadTablesOptions): Promise; mergeWithLoadedCollections( collections: CollectionOptions[], - loadedCollections: { - [name: string]: { name: string; fields: FieldOptions[] }; - }, + loadedCollections: LoadedCollections, ): CollectionOptions[] { return collections.map((collection) => { const loadedCollection = loadedCollections[collection.name]; diff --git a/packages/plugins/@nocobase/plugin-data-source-manager/src/server/__tests__/load-tables.test.ts b/packages/plugins/@nocobase/plugin-data-source-manager/src/server/__tests__/load-tables.test.ts new file mode 100644 index 00000000000..9c894802770 --- /dev/null +++ b/packages/plugins/@nocobase/plugin-data-source-manager/src/server/__tests__/load-tables.test.ts @@ -0,0 +1,172 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +import { Context, Next } from '@nocobase/actions'; +import { DatabaseDataSource, LoadedCollections } from '@nocobase/data-source-manager'; +import { loadDataSourceTablesIntoCollections } from '../middlewares/load-tables'; +import { DataSourceModel } from '../models/data-source'; + +type ActionName = 'create' | 'update'; + +function createDatabaseDataSource() { + const dataSource = Object.create(DatabaseDataSource.prototype) as DatabaseDataSource; + const loadTables = vi.fn(async () => undefined); + Object.defineProperty(dataSource, 'loadTables', { + configurable: true, + value: loadTables, + }); + return { dataSource, loadTables }; +} + +function createDataSourceModel(options: Record, localData: LoadedCollections) { + const model = Object.create(DataSourceModel.prototype) as DataSourceModel; + const loadLocalData = vi.fn(async () => localData); + Object.defineProperties(model, { + get: { + configurable: true, + value: vi.fn((key: string) => (key === 'options' ? options : undefined)), + }, + loadLocalData: { + configurable: true, + value: loadLocalData, + }, + type: { + configurable: true, + value: 'external', + }, + }); + return { loadLocalData, model }; +} + +function createContext(options: { + actionName: ActionName; + connectionOptions: Record; + dataSource: DatabaseDataSource; + model?: DataSourceModel; +}) { + const { actionName, connectionOptions, dataSource, model } = options; + const dataSourcesRepo = { + findByTargetKey: vi.fn(async () => model), + }; + const create = vi.fn(() => dataSource); + const get = vi.fn(() => dataSource); + const values = { + collections: ['departments'], + key: 'external', + options: connectionOptions, + type: 'external', + }; + const ctx = { + action: { + actionName, + params: { + filterByTk: actionName === 'update' ? 'external' : undefined, + values, + }, + resourceName: 'dataSources', + }, + app: { + dataSourceManager: { + factory: { create }, + get, + }, + db: { + getRepository: vi.fn(() => dataSourcesRepo), + }, + }, + logger: {}, + } as unknown as Context; + + return { create, ctx, dataSourcesRepo, get }; +} + +describe('loadDataSourceTablesIntoCollections', () => { + it('should pass persisted local metadata when updating an on-demand data source', async () => { + const connectionOptions = { addAllCollections: false, database: 'external' }; + const localData: LoadedCollections = { + departments: { + name: 'departments', + fields: [ + { + name: 'name', + field: 'name', + rawType: 'VARCHAR', + type: 'string', + uiSchema: { + title: 'Department name', + }, + }, + ], + }, + }; + const { dataSource, loadTables } = createDatabaseDataSource(); + const { loadLocalData, model } = createDataSourceModel(connectionOptions, localData); + const { ctx, dataSourcesRepo, get } = createContext({ + actionName: 'update', + connectionOptions, + dataSource, + model, + }); + const next: Next = vi.fn(async () => undefined); + + await loadDataSourceTablesIntoCollections(ctx, next); + + expect(dataSourcesRepo.findByTargetKey).toHaveBeenCalledWith('external'); + expect(get).toHaveBeenCalledWith('external'); + expect(loadLocalData).toHaveBeenCalledTimes(1); + expect(loadTables).toHaveBeenCalledWith(ctx, ['departments'], { + localData, + }); + expect(ctx.action.params.values.collections).toBeUndefined(); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('should not read local metadata when an update omits collections', async () => { + const connectionOptions = { addAllCollections: false, database: 'external' }; + const localData: LoadedCollections = {}; + const { dataSource, loadTables } = createDatabaseDataSource(); + const { loadLocalData, model } = createDataSourceModel(connectionOptions, localData); + const { ctx } = createContext({ + actionName: 'update', + connectionOptions, + dataSource, + model, + }); + delete ctx.action.params.values.collections; + const next: Next = vi.fn(async () => undefined); + + await loadDataSourceTablesIntoCollections(ctx, next); + + expect(loadLocalData).not.toHaveBeenCalled(); + expect(loadTables).toHaveBeenCalledWith(ctx, undefined); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('should load selected collections on create without reading local metadata', async () => { + const connectionOptions = { addAllCollections: false, database: 'external' }; + const { dataSource, loadTables } = createDatabaseDataSource(); + const { create, ctx, dataSourcesRepo } = createContext({ + actionName: 'create', + connectionOptions, + dataSource, + }); + const next: Next = vi.fn(async () => undefined); + + await loadDataSourceTablesIntoCollections(ctx, next); + + expect(dataSourcesRepo.findByTargetKey).not.toHaveBeenCalled(); + expect(create).toHaveBeenCalledWith('external', { + name: 'external', + ...connectionOptions, + }); + expect(loadTables).toHaveBeenCalledWith(ctx, ['departments']); + expect(ctx.action.params.values.collections).toBeUndefined(); + expect(next).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/plugins/@nocobase/plugin-data-source-manager/src/server/middlewares/load-tables.ts b/packages/plugins/@nocobase/plugin-data-source-manager/src/server/middlewares/load-tables.ts index 67fd4c048e6..78e9545d3ad 100644 --- a/packages/plugins/@nocobase/plugin-data-source-manager/src/server/middlewares/load-tables.ts +++ b/packages/plugins/@nocobase/plugin-data-source-manager/src/server/middlewares/load-tables.ts @@ -18,6 +18,7 @@ export async function loadDataSourceTablesIntoCollections(ctx: Context, next: Ne if (resourceName === 'dataSources' && (actionName === 'create' || actionName === 'update')) { const dataSourcesRepo = ctx.app.db.getRepository('dataSources'); const { options, type, collections, key } = params.values || {}; + let model: DataSourceModel | undefined; const dataSourceProvider: { get: () => Promise; @@ -26,7 +27,7 @@ export async function loadDataSourceTablesIntoCollections(ctx: Context, next: Ne ? { get: async () => { const { filterByTk } = params; - const model: DataSourceModel = await dataSourcesRepo.findByTargetKey(filterByTk); + model = await dataSourcesRepo.findByTargetKey(filterByTk); if (_.isEqual(model.get('options'), options)) { return ctx.app.dataSourceManager.get(filterByTk); } else { @@ -57,6 +58,10 @@ export async function loadDataSourceTablesIntoCollections(ctx: Context, next: Ne `The number of collections exceeds the limit of ${ALLOW_MAX_COLLECTIONS_COUNT}. Please remove some collections before adding new ones.`, ); } + } else if (model && collections?.length) { + await dataSource.loadTables(ctx, collections, { + localData: await model.loadLocalData(), + }); } else { await dataSource.loadTables(ctx, collections); } diff --git a/packages/plugins/@nocobase/plugin-data-source-manager/src/server/models/data-source.ts b/packages/plugins/@nocobase/plugin-data-source-manager/src/server/models/data-source.ts index 88a0193f6c3..ceec212ca7e 100644 --- a/packages/plugins/@nocobase/plugin-data-source-manager/src/server/models/data-source.ts +++ b/packages/plugins/@nocobase/plugin-data-source-manager/src/server/models/data-source.ts @@ -9,7 +9,12 @@ import { ACL, AvailableActionOptions } from '@nocobase/acl'; import { Model, Transaction } from '@nocobase/database'; -import { SequelizeCollectionManager } from '@nocobase/data-source-manager'; +import { + FieldOptions, + LoadedCollectionOptions, + LoadedCollections, + SequelizeCollectionManager, +} from '@nocobase/data-source-manager'; import { setCurrentRole } from '@nocobase/plugin-acl'; import { Application } from '@nocobase/server'; import { storagePathJoin } from '@nocobase/utils'; @@ -161,7 +166,7 @@ export class DataSourceModel extends Model { pluginDataSourceManagerServer.dataSourceStatus[dataSourceKey] = 'loaded'; } - private async loadLocalData() { + async loadLocalData(): Promise { const dataSourceKey = this.get('key'); const remoteCollections = await this.db.getRepository('dataSourcesCollections').find({ @@ -176,29 +181,29 @@ export class DataSourceModel extends Model { }, }); - const localData = {}; + const localData: LoadedCollections = {}; for (const remoteCollection of remoteCollections) { - const remoteCollectionOptions = remoteCollection.toJSON(); + const remoteCollectionOptions = remoteCollection.toJSON() as LoadedCollectionOptions; localData[remoteCollectionOptions.name] = remoteCollectionOptions; } for (const remoteField of remoteFields) { - const remoteFieldOptions = remoteField.toJSON(); + const remoteFieldOptions = remoteField.toJSON() as FieldOptions & { collectionName: string }; const collectionName = remoteFieldOptions.collectionName; + let localCollection = localData[collectionName]; - if (!localData[collectionName]) { - localData[collectionName] = { + if (!localCollection) { + localCollection = { name: collectionName, fields: [], }; + localData[collectionName] = localCollection; } - if (!localData[collectionName].fields) { - localData[collectionName].fields = []; - } - - localData[collectionName].fields.push(remoteFieldOptions); + const fields = localCollection.fields || []; + fields.push(remoteFieldOptions); + localCollection.fields = fields; } return localData;