fix(data-source-manager): preserve field metadata when reloading tables (#10387)

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

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<string, LoadedCollectionOptions>;
+
+export type LoadTablesOptions = {
+  localData?: LoadedCollections;
+};
+
 export abstract class DatabaseDataSource<T extends DatabaseIntrospector = DatabaseIntrospector> extends DataSource {
   declare introspector: T;
 
@@ -41,13 +53,11 @@ export abstract class DatabaseDataSource<T extends DatabaseIntrospector = Databa
   }
 
   abstract readTables(): Promise<any>;
-  abstract loadTables(ctx: Context, tables: string[]): Promise<any>;
+  abstract loadTables(ctx: Context, tables: string[], options?: LoadTablesOptions): Promise<unknown>;
 
   mergeWithLoadedCollections(
     collections: CollectionOptions[],
-    loadedCollections: {
-      [name: string]: { name: string; fields: FieldOptions[] };
-    },
+    loadedCollections: LoadedCollections,
   ): CollectionOptions[] {
     return collections.map((collection) => {
       const loadedCollection = loadedCollections[collection.name];

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

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<DatabaseDataSource>;
@@ -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);
     }

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<LoadedCollections> {
     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;