From 04e943a0b2febf3b6c02a011c921f0484b20db24 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Tue, 1 Apr 2025 11:00:59 +0800 Subject: [PATCH] fix(m2m-array): error of filtering by fields in an association collection with a m2m array field (#6596) * fix(m2m-array): error of filtering by fields in an association collection with a m2m array field * test: add test * feat: generateJoinOnForJSONArray --- .../belongs-to-array-repository.ts | 13 ++- .../src/eager-loading/eager-loading-tree.ts | 37 ++++--- .../query-interface/mysql-query-interface.ts | 4 + .../postgres-query-interface.ts | 4 + .../src/query-interface/query-interface.ts | 5 + .../query-interface/sqlite-query-interface.ts | 4 + .../src/server/__tests__/issues.test.ts | 96 +++++++++++++++++++ .../__tests__/m2m-array-bigint-api.test.ts | 19 +--- .../__tests__/m2m-array-string-api.test.ts | 19 +--- 9 files changed, 153 insertions(+), 48 deletions(-) diff --git a/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts b/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts index 375e65829e..254baf3aa0 100644 --- a/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts +++ b/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts @@ -54,19 +54,18 @@ export class BelongsToArrayAssociation { return this.db.getModel(this.targetName); } - generateInclude() { - if (this.db.sequelize.getDialect() !== 'postgres') { - throw new Error('Filtering by many to many (array) associations is only supported on postgres'); - } + generateInclude(parentAs?: string) { const targetCollection = this.db.getCollection(this.targetName); const targetField = targetCollection.getField(this.targetKey); const sourceCollection = this.db.getCollection(this.source.name); const foreignField = sourceCollection.getField(this.foreignKey); const queryInterface = this.db.sequelize.getQueryInterface(); - const left = queryInterface.quoteIdentifiers(`${this.as}.${targetField.columnName()}`); - const right = queryInterface.quoteIdentifiers(`${this.source.collection.name}.${foreignField.columnName()}`); + const asLeft = parentAs ? `${parentAs}->${this.as}` : this.as; + const asRight = parentAs || this.source.collection.name; + const left = queryInterface.quoteIdentifiers(`${asLeft}.${targetField.columnName()}`); + const right = queryInterface.quoteIdentifiers(`${asRight}.${foreignField.columnName()}`); return { - on: this.db.sequelize.literal(`${left}=any(${right})`), + on: this.db.queryInterface.generateJoinOnForJSONArray(left, right), }; } diff --git a/packages/core/database/src/eager-loading/eager-loading-tree.ts b/packages/core/database/src/eager-loading/eager-loading-tree.ts index d0d9fb2eeb..c450eb816f 100644 --- a/packages/core/database/src/eager-loading/eager-loading-tree.ts +++ b/packages/core/database/src/eager-loading/eager-loading-tree.ts @@ -82,6 +82,31 @@ const queryParentSQL = (options: { SELECT ${q(targetKeyField)} AS ${q(targetKey)}, ${q(foreignKeyField)} AS ${q(foreignKey)} FROM cte`; }; +const processIncludes = (includes: any[], model: any, parentAs = '') => { + includes.forEach((include: { association: string; include?: any[] }, index: number) => { + // Process current level + const association = model.associations[include.association]; + if (association?.generateInclude) { + includes[index] = { + ...include, + ...association.generateInclude(parentAs), + }; + } + + // Recursively process nested includes if they exist + if (include.include && Array.isArray(include.include) && include.include.length > 0) { + // Get the associated model for the next level + const nextModel = association?.target; + if (!nextModel) { + return; + } + processIncludes(include.include, nextModel, parentAs ? `${parentAs}->${association.as}` : association.as); + } + }); + + return includes; +}; + export class EagerLoadingTree { public root: EagerLoadingNode; db: Database; @@ -252,16 +277,6 @@ export class EagerLoadingTree { throw new Error(`Model ${node.model.name} does not have primary key`); } - includeForFilter.forEach((include: { association: string }, index: number) => { - const association = node.model.associations[include.association]; - if (association?.associationType == 'BelongsToArray') { - includeForFilter[index] = { - ...include, - ...association.generateInclude(), - }; - } - }); - // find all ids const ids = ( await node.model.findAll({ @@ -270,7 +285,7 @@ export class EagerLoadingTree { attributes: [primaryKeyField], group: `${node.model.name}.${primaryKeyField}`, transaction, - include: includeForFilter, + include: processIncludes(includeForFilter, node.model), } as any) ).map((row) => { return { row, pk: row[primaryKeyField] }; diff --git a/packages/core/database/src/query-interface/mysql-query-interface.ts b/packages/core/database/src/query-interface/mysql-query-interface.ts index fff3afa528..62c2c7febd 100644 --- a/packages/core/database/src/query-interface/mysql-query-interface.ts +++ b/packages/core/database/src/query-interface/mysql-query-interface.ts @@ -141,4 +141,8 @@ export default class MysqlQueryInterface extends QueryInterface { await this.db.sequelize.query(sql, { transaction }); } } + + public generateJoinOnForJSONArray(left: string, right: string) { + return this.db.sequelize.literal(`JSON_CONTAINS(${right}, JSON_ARRAY(${left}))`); + } } diff --git a/packages/core/database/src/query-interface/postgres-query-interface.ts b/packages/core/database/src/query-interface/postgres-query-interface.ts index 2d251ddf51..0b38c7c098 100644 --- a/packages/core/database/src/query-interface/postgres-query-interface.ts +++ b/packages/core/database/src/query-interface/postgres-query-interface.ts @@ -232,4 +232,8 @@ $BODY$ return res[0]['show_create_table']; } + + public generateJoinOnForJSONArray(left: string, right: string) { + return this.db.sequelize.literal(`${left}=any(${right})`); + } } diff --git a/packages/core/database/src/query-interface/query-interface.ts b/packages/core/database/src/query-interface/query-interface.ts index c7d484ccb7..9d8ffe349a 100644 --- a/packages/core/database/src/query-interface/query-interface.ts +++ b/packages/core/database/src/query-interface/query-interface.ts @@ -83,4 +83,9 @@ export default abstract class QueryInterface { // @ts-ignore return this.db.sequelize.getQueryInterface().queryGenerator.quoteIdentifier(identifier); } + + public generateJoinOnForJSONArray(left: string, right: string) { + const dialect = this.db.sequelize.getDialect(); + throw new Error(`Filtering by many to many (array) associations is not supported on ${dialect}`); + } } diff --git a/packages/core/database/src/query-interface/sqlite-query-interface.ts b/packages/core/database/src/query-interface/sqlite-query-interface.ts index 2f6b93c877..ec77652c56 100644 --- a/packages/core/database/src/query-interface/sqlite-query-interface.ts +++ b/packages/core/database/src/query-interface/sqlite-query-interface.ts @@ -146,4 +146,8 @@ export default class SqliteQueryInterface extends QueryInterface { WHERE name = '${tableName}';`; await this.db.sequelize.query(sql, { transaction }); } + + public generateJoinOnForJSONArray(left: string, right: string) { + return this.db.sequelize.literal(`${left} in (SELECT value from json_each(${right}))`); + } } diff --git a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts index 536d72d1a5..94b2f559b3 100644 --- a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts +++ b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts @@ -324,4 +324,100 @@ describe('issues', () => { } expect(res.status).toBe(200); }); + + test('filtering by fields of a relation collection with m2m array field', async () => { + await db.getRepository('collections').create({ + values: { + name: 'tags', + fields: [ + { + name: 'id', + type: 'bigInt', + autoIncrement: true, + primaryKey: true, + allowNull: false, + }, + { + name: 'title', + type: 'string', + }, + ], + }, + }); + await db.getRepository('collections').create({ + values: { + name: 'users', + fields: [ + { + name: 'id', + type: 'bigInt', + autoIncrement: true, + primaryKey: true, + allowNull: false, + }, + { + name: 'username', + type: 'string', + }, + { + name: 'tags', + type: 'belongsToArray', + foreignKey: 'tag_ids', + target: 'tags', + targetKey: 'id', + }, + ], + }, + }); + await db.getRepository('collections').create({ + values: { + name: 'projects', + fields: [ + { + name: 'id', + type: 'bigInt', + autoIncrement: true, + primaryKey: true, + allowNull: false, + }, + { + name: 'title', + type: 'string', + }, + { + name: 'users', + type: 'belongsTo', + foreignKey: 'user_id', + target: 'users', + }, + ], + }, + }); + // @ts-ignore + await db.getRepository('collections').load(); + await db.sync(); + await db.getRepository('tags').create({ + values: [{ title: 'a' }, { title: 'b' }, { title: 'c' }], + }); + await db.getRepository('users').create({ + values: { id: 1, username: 'a' }, + }); + await db.getRepository('projects').create({ + values: { id: 1, title: 'p1', user_id: 1 }, + }); + await expect( + db.getRepository('projects').findOne({ + appends: ['users', 'users.tags'], + filter: { + $and: [ + { + users: { + username: 'a', + }, + }, + ], + }, + }), + ).resolves.toBeTruthy(); + }); }); diff --git a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts index e20b709e05..778c196582 100644 --- a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts +++ b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts @@ -207,15 +207,8 @@ describe('m2m array api, bigInt targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search; - expect(res.length).toBe(1); - } else { - expect(search).rejects.toThrowError(); - } - if (db.sequelize.getDialect() !== 'postgres') { - return; - } + const res1 = await search; + expect(res1.length).toBe(1); const search2 = db.getRepository('users').find({ filter: { 'tags.title': { @@ -223,12 +216,8 @@ describe('m2m array api, bigInt targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search2; - expect(res.length).toBe(2); - } else { - expect(search2).rejects.toThrowError(); - } + const res2 = await search2; + expect(res2.length).toBe(2); }); it('should create with belongsToArray', async () => { diff --git a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts index 3d1f1d8265..2c0cf26e4b 100644 --- a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts +++ b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts @@ -186,15 +186,8 @@ describe('m2m array api, string targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search; - expect(res.length).toBe(1); - } else { - expect(search).rejects.toThrowError(); - } - if (db.sequelize.getDialect() !== 'postgres') { - return; - } + const res1 = await search; + expect(res1.length).toBe(1); const search2 = db.getRepository('users').find({ filter: { 'tags.title': { @@ -202,12 +195,8 @@ describe('m2m array api, string targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search2; - expect(res.length).toBe(2); - } else { - expect(search2).rejects.toThrowError(); - } + const res2 = await search2; + expect(res2.length).toBe(2); }); it('should create with belongsToArray', async () => {