fix: update association with a non-primary key table (#5495)

* fix: update association with non primaryKey table

* fix: test

* fix: test

* fix: get primary key attribute with multi filter target keys

* fix: update has one associations

* fix: test

* fix: test

* chore: test

* chore: test

* chore: test

* chore: test

* chore: middleware

* chore: error condition

* chore: test

* fix: test

.github/workflows/nocobase-test-backend.yml

@@ -39,8 +39,8 @@ jobs:
   sqlite-test:
     strategy:
       matrix:
-        node_version: ['20']
+        node_version: [ '20' ]
-        underscored: [true, false]
+        underscored: [ true, false ]
     runs-on: ubuntu-latest
     container: node:${{ matrix.node_version }}
     services:
@@ -70,10 +70,10 @@ jobs:
   postgres-test:
     strategy:
       matrix:
-        node_version: ['20']
+        node_version: [ '20' ]
-        underscored: [true, false]
+        underscored: [ true, false ]
-        schema: [public, nocobase]
+        schema: [ public, nocobase ]
-        collection_schema: [public, user_schema]
+        collection_schema: [ public, user_schema ]
     runs-on: ubuntu-latest
     container: node:${{ matrix.node_version }}
     services:
@@ -129,8 +129,8 @@ jobs:
   mysql-test:
     strategy:
       matrix:
-        node_version: ['20']
+        node_version: [ '20' ]
-        underscored: [true, false]
+        underscored: [ true, false ]
     runs-on: ubuntu-latest
     container: node:${{ matrix.node_version }}
     services:
@@ -175,8 +175,8 @@ jobs:
   mariadb-test:
     strategy:
       matrix:
-        node_version: ['20']
+        node_version: [ '20' ]
-        underscored: [true, false]
+        underscored: [ true, false ]
     runs-on: ubuntu-latest
     container: node:${{ matrix.node_version }}
     services:

packages/core/database/src/__tests__/collection.test.ts

@@ -346,6 +346,7 @@ describe('collection sync', () => {
 
     const model = collection.model;
     await collection.sync();
+
     if (db.options.underscored) {
       const tableFields = await (<any>model).queryInterface.describeTable(`${db.getTablePrefix()}posts_tags`);
       expect(tableFields['post_id']).toBeDefined();

packages/core/database/src/__tests__/target-key.test.ts

@@ -75,18 +75,23 @@ describe('targetKey', () => {
       ],
     });
     await db.sync();
+
     const r1 = db.getRepository('a1');
     const r2 = db.getRepository('b1');
+
     const b1 = await r2.create({
       values: {},
     });
+
     await r1.create({
       values: {
         name: 'a1',
         b1: [b1.toJSON()],
       },
     });
+
     const b1r = await b1.reload();
+
     expect(b1r.a1Id).toBe(b1.id);
   });
 

packages/core/database/src/__tests__/update-association-values.test.ts

@@ -21,6 +21,72 @@ describe('update associations', () => {
     await db.close();
   });
 
+  it('should update associations with target key', async () => {
+    const T1 = db.collection({
+      name: 'test1',
+      autoGenId: false,
+      timestamps: false,
+      filterTargetKey: 'id_',
+      fields: [
+        {
+          name: 'id_',
+          type: 'string',
+        },
+        {
+          type: 'hasMany',
+          name: 't2',
+          foreignKey: 'nvarchar2',
+          targetKey: 'varchar_',
+          sourceKey: 'id_',
+          target: 'test2',
+        },
+      ],
+    });
+
+    const T2 = db.collection({
+      name: 'test2',
+      autoGenId: false,
+      timestamps: false,
+      filterTargetKey: 'varchar_',
+      fields: [
+        {
+          name: 'varchar_',
+          type: 'string',
+          unique: true,
+        },
+        {
+          name: 'nvarchar2',
+          type: 'string',
+        },
+      ],
+    });
+
+    await db.sync();
+
+    const t2 = await T2.repository.create({
+      values: {
+        varchar_: '1',
+      },
+    });
+
+    await T1.repository.create({
+      values: {
+        id_: 1,
+        t2: [
+          {
+            varchar_: '1',
+          },
+        ],
+      },
+    });
+
+    const t1 = await T1.repository.findOne({
+      appends: ['t2'],
+    });
+
+    expect(t1['t2'][0]['varchar_']).toBe('1');
+  });
+
   it('hasOne', async () => {
     db.collection({
       name: 'a',

packages/core/database/src/collection.ts

@@ -126,6 +126,7 @@ export interface CollectionOptions extends Omit<ModelOptions, 'name' | 'hooks'>
    */
   origin?: string;
   asStrategyResource?: boolean;
+
   [key: string]: any;
 }
 
@@ -155,6 +156,7 @@ export class Collection<
     this.modelInit();
 
     this.db.modelCollection.set(this.model, this);
+    this.db.modelNameCollectionMap.set(this.model.name, this);
 
     // set tableName to collection map
     // the form of key is `${schema}.${tableName}` if schema exists
@@ -259,8 +261,58 @@ export class Collection<
       M = model;
     }
 
+    const collection = this;
+
     // @ts-ignore
     this.model = class extends M {};
+
+    Object.defineProperty(this.model, 'primaryKeyAttribute', {
+      get: function () {
+        const singleFilterTargetKey: string = (() => {
+          if (!collection.options.filterTargetKey) {
+            return null;
+          }
+
+          if (Array.isArray(collection.options.filterTargetKey) && collection.options.filterTargetKey.length === 1) {
+            return collection.options.filterTargetKey[0];
+          }
+
+          return collection.options.filterTargetKey as string;
+        })();
+
+        if (!this._primaryKeyAttribute && singleFilterTargetKey && collection.getField(singleFilterTargetKey)) {
+          return singleFilterTargetKey;
+        }
+
+        return this._primaryKeyAttribute;
+      }.bind(this.model),
+
+      set(value) {
+        this._primaryKeyAttribute = value;
+      },
+    });
+
+    Object.defineProperty(this.model, 'primaryKeyAttributes', {
+      get: function () {
+        if (Array.isArray(this._primaryKeyAttributes) && this._primaryKeyAttributes.length) {
+          return this._primaryKeyAttributes;
+        }
+
+        if (collection.options.filterTargetKey) {
+          const fields = lodash.castArray(collection.options.filterTargetKey);
+          if (fields.every((field) => collection.getField(field))) {
+            return fields;
+          }
+        }
+
+        return this._primaryKeyAttributes;
+      }.bind(this.model),
+
+      set(value) {
+        this._primaryKeyAttributes = value;
+      },
+    });
+
     this.model.init(null, this.sequelizeModelOptions());
 
     this.model.options.modelName = this.options.name;
@@ -856,12 +908,15 @@ export class Collection<
 
   protected sequelizeModelOptions() {
     const { name } = this.options;
-    return {
+
+    const attr = {
       ..._.omit(this.options, ['name', 'fields', 'model', 'targetKey']),
       modelName: name,
       sequelize: this.context.database.sequelize,
       tableName: this.tableName(),
     };
+
+    return attr;
   }
 
   protected bindFieldEventListener() {

packages/core/database/src/database.ts

@@ -144,6 +144,7 @@ export class Database extends EventEmitter implements AsyncEmitter {
   collections = new Map<string, Collection>();
   pendingFields = new Map<string, RelationField[]>();
   modelCollection = new Map<ModelStatic<any>, Collection>();
+  modelNameCollectionMap = new Map<string, Collection>();
   tableNameCollectionMap = new Map<string, Collection>();
   context: any = {};
   queryInterface: QueryInterface;
@@ -566,6 +567,10 @@ export class Database extends EventEmitter implements AsyncEmitter {
     return field;
   }
 
+  getCollectionByModelName(name: string) {
+    return this.modelNameCollectionMap.get(name);
+  }
+
   /**
    * get exists collection by its name
    * @param name

packages/core/database/src/features/referential-integrity-check.ts

@@ -18,8 +18,7 @@ interface ReferentialIntegrityCheckOptions extends Transactionable {
 export async function referentialIntegrityCheck(options: ReferentialIntegrityCheckOptions) {
   const { referencedInstance, db, transaction } = options;
 
-  // @ts-ignore
+  const collection = db.getCollectionByModelName(referencedInstance.constructor.name);
-  const collection = db.modelCollection.get(referencedInstance.constructor);
 
   const collectionName = collection.name;
   const references = db.referenceMap.getReferences(collectionName);

packages/core/database/src/update-associations.ts

@@ -18,10 +18,10 @@ import {
   ModelStatic,
   Transactionable,
 } from 'sequelize';
-import Database from './database';
 import { Model } from './model';
 import { UpdateGuard } from './update-guard';
 import { TargetKey } from './repository';
+import Database from './database';
 
 function isUndefinedOrNull(value: any) {
   return typeof value === 'undefined' || value === null;
@@ -449,7 +449,8 @@ export async function updateMultipleAssociation(
     } else if (item.sequelize) {
       setItems.push(item);
     } else if (typeof item === 'object') {
-      const targetKey = (association as any).targetKey || 'id';
+      // @ts-ignore
+      const targetKey = (association as any).targetKey || association.options.targetKey || 'id';
 
       if (item[targetKey]) {
         const attributes = {
@@ -468,16 +469,19 @@ export async function updateMultipleAssociation(
   await model[setAccessor](setItems, { transaction, context, individualHooks: true });
 
   const newItems = [];
+
   const pk = association.target.primaryKeyAttribute;
-  const tmpKey = association['options']?.['targetKey'];
   let targetKey = pk;
   const db = model.constructor['database'] as Database;
+
+  const tmpKey = association['options']?.['targetKey'];
   if (tmpKey !== pk) {
     const targetKeyFieldOptions = db.getFieldByPath(`${association.target.name}.${tmpKey}`)?.options;
     if (targetKeyFieldOptions?.unique) {
       targetKey = tmpKey;
     }
   }
+
   for (const item of objectItems) {
     const through = (<any>association).through ? (<any>association).through.model.name : null;
 
@@ -550,7 +554,10 @@ export async function updateMultipleAssociation(
   }
 
   for (const newItem of newItems) {
-    const existIndexInSetItems = setItems.findIndex((setItem) => setItem[targetKey] === newItem[targetKey]);
+    // @ts-ignore
+    const findTargetKey = (association as any).targetKey || association.options.targetKey || targetKey;
+
+    const existIndexInSetItems = setItems.findIndex((setItem) => setItem[findTargetKey] === newItem[findTargetKey]);
 
     if (existIndexInSetItems !== -1) {
       setItems[existIndexInSetItems] = newItem;

packages/core/server/src/helper.ts

@@ -40,10 +40,13 @@ export function createResourcer(options: ApplicationOptions) {
 }
 
 export function registerMiddlewares(app: Application, options: ApplicationOptions) {
-  app.use(async (ctx, next) => {
+  app.use(
+    async function generateReqId(ctx, next) {
       app.context.reqId = randomUUID();
       await next();
-  });
+    },
+    { tag: 'generateReqId' },
+  );
 
   app.use(requestLogger(app.name, app.requestLogger, options.logger?.request), { tag: 'logger' });
 
@@ -82,10 +85,10 @@ export function registerMiddlewares(app: Application, options: ApplicationOption
     await next();
   });
 
-  app.use(i18n, { tag: 'i18n', after: 'cors' });
+  app.use(i18n, { tag: 'i18n', before: 'cors' });
 
   if (options.dataWrapping !== false) {
-    app.use(dataWrapping(), { tag: 'dataWrapping', after: 'i18n' });
+    app.use(dataWrapping(), { tag: 'dataWrapping', after: 'cors' });
   }
 
   app.use(app.dataSourceManager.middleware(), { tag: 'dataSource', after: 'dataWrapping' });

packages/core/server/src/middlewares/i18n.ts

@@ -19,14 +19,17 @@ export async function i18n(ctx, next) {
       'en-US';
     return lng;
   };
+
   const lng = ctx.getCurrentLocale();
   const localeManager = ctx.app.localeManager as Locale;
   const i18n = await localeManager.getI18nInstance(lng);
   ctx.i18n = i18n;
   ctx.t = i18n.t.bind(i18n);
+
   if (lng !== '*' && lng) {
-    i18n.changeLanguage(lng);
+    await i18n.changeLanguage(lng);
     await localeManager.loadResourcesByLang(lng);
   }
+
   await next();
 }

packages/plugins/@nocobase/plugin-error-handler/src/server/__tests__/render-error.test.ts

@@ -112,13 +112,7 @@ describe('create with exception', () => {
 
     expect(response.statusCode).toEqual(400);
 
-    expect(response.body).toEqual({
+    expect(response.body['errors'][0]['message']).toBe('name must be unique');
-      errors: [
-        {
-          message: 'name must be unique',
-        },
-      ],
-    });
   });
 
   it('should render error with field title', async () => {

packages/plugins/@nocobase/plugin-error-handler/src/server/server.ts

@@ -65,6 +65,6 @@ export class PluginErrorHandlerServer extends Plugin {
   }
 
   async load() {
-    this.app.use(this.errorHandler.middleware(), { before: 'cors', tag: 'errorHandler' });
+    this.app.use(this.errorHandler.middleware(), { after: 'i18n', tag: 'errorHandler', before: 'cors' });
   }
 }