Skip to content

Commit

Permalink
fix(core): alias pivot fields when loading m:n relations
Browse files Browse the repository at this point in the history
Closes #1346
Closes #1349
  • Loading branch information
B4nan committed Jan 26, 2021
1 parent 6dac1a8 commit 56682be
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 20 deletions.
6 changes: 3 additions & 3 deletions packages/knex/src/AbstractSqlDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,10 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
const map: Dictionary<T[]> = {};
owners.forEach(owner => map['' + Utils.getPrimaryKeyHash(owner as string[])] = []);
items.forEach((item: any) => {
const key = Utils.getPrimaryKeyHash(prop.joinColumns.map(col => item[col]));
const key = Utils.getPrimaryKeyHash(prop.joinColumns.map(col => item[`fk__${col}`]));
map[key].push(item);
prop.joinColumns.forEach(col => delete item[col]);
prop.inverseJoinColumns.forEach(col => delete item[col]);
prop.joinColumns.forEach(col => delete item[`fk__${col}`]);
prop.inverseJoinColumns.forEach(col => delete item[`fk__${col}`]);
});

return map;
Expand Down
4 changes: 2 additions & 2 deletions packages/knex/src/query/QueryBuilderHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ export class QueryBuilderHelper {
}

return [
...join.joinColumns!.map(col => this.mapper(`${join.alias}.${col}`, type)),
...join.inverseJoinColumns!.map(col => this.mapper(`${join.alias}.${col}`, type)),
...join.joinColumns!.map(col => this.mapper(`${join.alias}.${col}`, type, undefined, `fk__${col}`)),
...join.inverseJoinColumns!.map(col => this.mapper(`${join.alias}.${col}`, type, undefined, `fk__${col}`)),
];
}

Expand Down
6 changes: 3 additions & 3 deletions tests/EntityManager.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,7 @@ describe('EntityManagerMySql', () => {
orm.em.clear();
mock.mock.calls.length = 0;
const tags = await orm.em.find(BookTag2, { booksUnordered: { title: { $ne: 'My Life on The Wall, part 3' } } }, ['booksUnordered.perex'], { name: QueryOrder.ASC });
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e0`.price * 1.19 as `price_taxed`, `e1`.`book2_uuid_pk`, `e1`.`book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` ' +
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e0`.price * 1.19 as `price_taxed`, `e1`.`book2_uuid_pk` as `fk__book2_uuid_pk`, `e1`.`book_tag2_id` as `fk__book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` ' +
'left join `book_to_tag_unordered` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` ' +
'left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` ' +
'where `e0`.`title` != ? and `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?)');
Expand Down Expand Up @@ -1635,7 +1635,7 @@ describe('EntityManagerMySql', () => {
'where `e0`.`id` = ? ' +
'order by `e1`.`name` asc ' +
'limit ?');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`author2_2_id`, `e1`.`author2_1_id`, `e2`.`author_id` as `address_author_id` ' +
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`author2_2_id` as `fk__author2_2_id`, `e1`.`author2_1_id` as `fk__author2_1_id`, `e2`.`author_id` as `address_author_id` ' +
'from `author2` as `e0` ' +
'left join `author_to_friend` as `e1` on `e0`.`id` = `e1`.`author2_2_id` ' +
'left join `address2` as `e2` on `e0`.`id` = `e2`.`author_id` ' +
Expand All @@ -1652,7 +1652,7 @@ describe('EntityManagerMySql', () => {
'where `e1`.`author2_2_id` = ? ' +
'order by `e2`.`name` asc ' +
'limit ?');
expect(mock.mock.calls[3][0]).toMatch('select `e0`.*, `e1`.`author2_2_id`, `e1`.`author2_1_id`, `e2`.`author_id` as `address_author_id` ' +
expect(mock.mock.calls[3][0]).toMatch('select `e0`.*, `e1`.`author2_2_id` as `fk__author2_2_id`, `e1`.`author2_1_id` as `fk__author2_1_id`, `e2`.`author_id` as `address_author_id` ' +
'from `author2` as `e0` ' +
'left join `author_to_friend` as `e1` on `e0`.`id` = `e1`.`author2_2_id` ' +
'left join `address2` as `e2` on `e0`.`id` = `e2`.`author_id` ' +
Expand Down
8 changes: 4 additions & 4 deletions tests/QueryBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ describe('QueryBuilder', () => {
test('select by m:n inverse side (that is not defined as property) via populate', async () => {
const qb = orm.em.createQueryBuilder(Test2);
qb.select('*').populate([{ field: 'publisher2_tests' }]).where({ 'publisher2_tests.Publisher2_owner': { $in: [ 1, 2 ] } }).orderBy({ 'publisher2_tests.id': QueryOrder.ASC });
let sql = 'select `e0`.*, `e1`.`test2_id`, `e1`.`publisher2_id` from `test2` as `e0` ';
let sql = 'select `e0`.*, `e1`.`test2_id` as `fk__test2_id`, `e1`.`publisher2_id` as `fk__publisher2_id` from `test2` as `e0` ';
sql += 'left join `publisher2_tests` as `e1` on `e0`.`id` = `e1`.`test2_id` ';
sql += 'where `e1`.`publisher2_id` in (?, ?) ';
sql += 'order by `e1`.`id` asc';
Expand All @@ -523,7 +523,7 @@ describe('QueryBuilder', () => {
test('select by m:n self reference owner', async () => {
const qb = orm.em.createQueryBuilder(Author2);
qb.select('*').populate([{ field: 'author2_following' }]).where({ 'author2_following.Author2_owner': { $in: [ 1, 2 ] } });
let sql = 'select `e0`.*, `e1`.`author2_2_id`, `e1`.`author2_1_id` from `author2` as `e0` ';
let sql = 'select `e0`.*, `e1`.`author2_2_id` as `fk__author2_2_id`, `e1`.`author2_1_id` as `fk__author2_1_id` from `author2` as `e0` ';
sql += 'left join `author2_following` as `e1` on `e0`.`id` = `e1`.`author2_2_id` ';
sql += 'where `e1`.`author2_1_id` in (?, ?)';
expect(qb.getQuery()).toEqual(sql);
Expand All @@ -533,7 +533,7 @@ describe('QueryBuilder', () => {
test('select by m:n self reference inverse', async () => {
const qb = orm.em.createQueryBuilder(Author2);
qb.select('*').populate([{ field: 'author2_following' }]).where({ 'author2_following.Author2_inverse': { $in: [ 1, 2 ] } });
let sql = 'select `e0`.*, `e1`.`author2_1_id`, `e1`.`author2_2_id` from `author2` as `e0` ';
let sql = 'select `e0`.*, `e1`.`author2_1_id` as `fk__author2_1_id`, `e1`.`author2_2_id` as `fk__author2_2_id` from `author2` as `e0` ';
sql += 'left join `author2_following` as `e1` on `e0`.`id` = `e1`.`author2_1_id` ';
sql += 'where `e1`.`author2_2_id` in (?, ?)';
expect(qb.getQuery()).toEqual(sql);
Expand All @@ -543,7 +543,7 @@ describe('QueryBuilder', () => {
test('select by m:n with composite keys', async () => {
const qb = orm.em.createQueryBuilder(User2);
qb.select('*').populate([{ field: 'user2_cars' }]).where({ 'user2_cars.Car2_inverse': { $in: [ [1, 2], [3, 4] ] } });
const sql = 'select `e0`.*, `e1`.`user2_first_name`, `e1`.`user2_last_name`, `e1`.`car2_name`, `e1`.`car2_year` ' +
const sql = 'select `e0`.*, `e1`.`user2_first_name` as `fk__user2_first_name`, `e1`.`user2_last_name` as `fk__user2_last_name`, `e1`.`car2_name` as `fk__car2_name`, `e1`.`car2_year` as `fk__car2_year` ' +
'from `user2` as `e0` left join `user2_cars` as `e1` on `e0`.`first_name` = `e1`.`user2_first_name` and `e0`.`last_name` = `e1`.`user2_last_name` ' +
'where (`e1`.`car2_name`, `e1`.`car2_year`) in ((?, ?), (?, ?))';
expect(qb.getQuery()).toEqual(sql);
Expand Down
4 changes: 2 additions & 2 deletions tests/issues/GH1041.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('GH issue 1041, 1043', () => {
const findPromise = orm.em.findOne(User, { apps: 1 }, { populate: { apps: LoadStrategy.SELECT_IN } });
await expect(findPromise).resolves.toBeInstanceOf(User);
expect(log.mock.calls[0][0]).toMatch('select `e0`.* from `user` as `e0` left join `user_apps` as `e1` on `e0`.`id` = `e1`.`user_id` where `e1`.`app_id` = 1 limit 1');
expect(log.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`app_id`, `e1`.`user_id` from `app` as `e0` left join `user_apps` as `e1` on `e0`.`id` = `e1`.`app_id` where `e1`.`user_id` in (123)');
expect(log.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`app_id` as `fk__app_id`, `e1`.`user_id` as `fk__user_id` from `app` as `e0` left join `user_apps` as `e1` on `e0`.`id` = `e1`.`app_id` where `e1`.`user_id` in (123)');
});

test('joined strategy: find by many-to-many relation ID', async () => {
Expand All @@ -104,7 +104,7 @@ describe('GH issue 1041, 1043', () => {
const findPromise = orm.em.findOne(User, { apps: [1, 2, 3] }, { populate: { apps: LoadStrategy.SELECT_IN } });
await expect(findPromise).resolves.toBeInstanceOf(User);
expect(log.mock.calls[0][0]).toMatch('select `e0`.* from `user` as `e0` left join `user_apps` as `e1` on `e0`.`id` = `e1`.`user_id` where `e1`.`app_id` in (1, 2, 3) limit 1');
expect(log.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`app_id`, `e1`.`user_id` from `app` as `e0` left join `user_apps` as `e1` on `e0`.`id` = `e1`.`app_id` where `e1`.`user_id` in (123)');
expect(log.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`app_id` as `fk__app_id`, `e1`.`user_id` as `fk__user_id` from `app` as `e0` left join `user_apps` as `e1` on `e0`.`id` = `e1`.`app_id` where `e1`.`user_id` in (123)');
});

test('joined strategy: find by many-to-many relation IDs', async () => {
Expand Down
65 changes: 65 additions & 0 deletions tests/issues/GH1346.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { Collection, Entity, ManyToMany, MikroORM, PrimaryKey, Property } from '@mikro-orm/core';
import { AbstractSqlDriver, SchemaGenerator } from '@mikro-orm/knex';

@Entity({ tableName: 'name' })
class Name {

@PrimaryKey()
id!: number;

@Property()
name!: string;

@ManyToMany({ entity: 'User', pivotTable: 'userNames', mappedBy: 'names' })
users = new Collection<User>(this);

constructor(name: string) {
this.name = name;
}

}

@Entity({ tableName: 'user' })
class User {

@PrimaryKey()
id!: number;

@ManyToMany({ entity: 'Name', pivotTable: 'userNames', joinColumn: 'name', inverseJoinColumn: 'user' })
names = new Collection<Name>(this);

}

describe('GH issue 1346', () => {

let orm: MikroORM<AbstractSqlDriver>;

beforeAll(async () => {
orm = await MikroORM.init({
entities: [User, Name],
dbName: `mikro_orm_test_pivot_fields`,
type: 'postgresql',
});

await new SchemaGenerator(orm.em).ensureDatabase();
});

beforeEach(async () => {
await orm.getSchemaGenerator().dropSchema();
await orm.getSchemaGenerator().createSchema();
});

afterAll(() => orm.close(true));

test('preserve data fields that match pivot field', async () => {
const user = orm.em.create<User>(User, {});
const name = orm.em.create<Name>(Name, { name: 'this is my name' });
user.names.add(name);
await orm.em.persistAndFlush([user, name]);
orm.em.clear();

const entity = await orm.em.findOneOrFail<User>(User, user, { populate: ['names'], refresh: true });
expect(entity.names[0].name).toEqual('this is my name');
});

});
4 changes: 2 additions & 2 deletions tests/issues/GH234.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ describe('GH issue 234', () => {
Object.assign(orm.config, { logger });
const res1 = await orm.em.find<B>(B, { aCollection: [1, 2, 3] }, ['aCollection']);
expect(mock.mock.calls[0][0]).toMatch('select `e0`.* from `b` as `e0` left join `b_a_collection` as `e1` on `e0`.`id` = `e1`.`b_id` where `e1`.`a_id` in (?, ?, ?)');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`a_id`, `e1`.`b_id` from `a` as `e0` left join `b_a_collection` as `e1` on `e0`.`id` = `e1`.`a_id` where `e1`.`b_id` in (?) order by `e1`.`id` asc');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`a_id` as `fk__a_id`, `e1`.`b_id` as `fk__b_id` from `a` as `e0` left join `b_a_collection` as `e1` on `e0`.`id` = `e1`.`a_id` where `e1`.`b_id` in (?) order by `e1`.`id` asc');
expect(res1.map(b => b.id)).toEqual([b.id]);

orm.em.clear();
mock.mock.calls.length = 0;
const res2 = await orm.em.find<A>(A, { bCollection: [1, 2, 3] }, ['bCollection']);
expect(mock.mock.calls[0][0]).toMatch('select `e0`.* from `a` as `e0` left join `b_a_collection` as `e1` on `e0`.`id` = `e1`.`a_id` where `e1`.`b_id` in (?, ?, ?)');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`b_id`, `e1`.`a_id` from `b` as `e0` left join `b_a_collection` as `e1` on `e0`.`id` = `e1`.`b_id` where `e1`.`a_id` in (?, ?, ?) order by `e1`.`id` asc');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.*, `e1`.`b_id` as `fk__b_id`, `e1`.`a_id` as `fk__a_id` from `b` as `e0` left join `b_a_collection` as `e1` on `e0`.`id` = `e1`.`b_id` where `e1`.`a_id` in (?, ?, ?) order by `e1`.`id` asc');
expect(res2.map(a => a.id)).toEqual([a1.id, a2.id, a3.id]);
});

Expand Down
8 changes: 4 additions & 4 deletions tests/partial-loading.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('partial loading (mysql)', () => {
expect(r1[0].books[0].price).toBeUndefined();
expect(r1[0].books[0].author).toBeUndefined();
expect(mock.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name` from `book_tag2` as `e0`');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e1`.`book2_uuid_pk`, `e1`.`book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?) order by `e1`.`order` asc');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e1`.`book2_uuid_pk` as `fk__book2_uuid_pk`, `e1`.`book_tag2_id` as `fk__book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?) order by `e1`.`order` asc');
orm.em.clear();
mock.mock.calls.length = 0;

Expand All @@ -141,7 +141,7 @@ describe('partial loading (mysql)', () => {
expect(r2[0].books[0].price).toBeUndefined();
expect(r2[0].books[0].author).toBeUndefined();
expect(mock.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name` from `book_tag2` as `e0` where `e0`.`name` = ?');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e1`.`book2_uuid_pk`, `e1`.`book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?) order by `e1`.`order` asc');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e1`.`book2_uuid_pk` as `fk__book2_uuid_pk`, `e1`.`book_tag2_id` as `fk__book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?) order by `e1`.`order` asc');
});

test('partial nested loading (mixed)', async () => {
Expand Down Expand Up @@ -172,7 +172,7 @@ describe('partial loading (mysql)', () => {
expect(r1[0].books[0].author.name).toBeUndefined();
expect(r1[0].books[0].author.email).toBe(god.email);
expect(mock.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name` from `book_tag2` as `e0`');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e0`.`author_id`, `e1`.`book2_uuid_pk`, `e1`.`book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?) order by `e1`.`order` asc');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e0`.`author_id`, `e1`.`book2_uuid_pk` as `fk__book2_uuid_pk`, `e1`.`book_tag2_id` as `fk__book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?) order by `e1`.`order` asc');
expect(mock.mock.calls[2][0]).toMatch('select `e0`.`id`, `e0`.`email` from `author2` as `e0` where `e0`.`id` in (?) order by `e0`.`id` asc');
orm.em.clear();
mock.mock.calls.length = 0;
Expand All @@ -187,7 +187,7 @@ describe('partial loading (mysql)', () => {
expect(r2[0].books[0].author.name).toBeUndefined();
expect(r2[0].books[0].author.email).toBe(god.email);
expect(mock.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name` from `book_tag2` as `e0`');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e0`.`author_id`, `e1`.`book2_uuid_pk`, `e1`.`book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?) order by `e1`.`order` asc');
expect(mock.mock.calls[1][0]).toMatch('select `e0`.`uuid_pk`, `e0`.`title`, `e0`.`author_id`, `e1`.`book2_uuid_pk` as `fk__book2_uuid_pk`, `e1`.`book_tag2_id` as `fk__book_tag2_id`, `e2`.`id` as `test_id` from `book2` as `e0` left join `book2_tags` as `e1` on `e0`.`uuid_pk` = `e1`.`book2_uuid_pk` left join `test2` as `e2` on `e0`.`uuid_pk` = `e2`.`book_uuid_pk` where `e1`.`book_tag2_id` in (?, ?, ?, ?, ?, ?) order by `e1`.`order` asc');
expect(mock.mock.calls[2][0]).toMatch('select `e0`.`id`, `e0`.`email` from `author2` as `e0` where `e0`.`id` in (?) order by `e0`.`id` asc');
});

Expand Down

0 comments on commit 56682be

Please sign in to comment.