Skip to content

Commit

Permalink
fix(core): consider star populates on nested positions only for one l…
Browse files Browse the repository at this point in the history
…evel

Previously, using `populate: ['books.*']` would be ignored, now it will mean "populate all properties of books relation",
as opposed to using `populate: ['*']` which still means "populate all relations recursively".

Related: #5213
  • Loading branch information
B4nan committed Feb 8, 2024
1 parent 760ec77 commit 65d1575
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/EntityManager.ts
Expand Up @@ -2076,7 +2076,7 @@ export class EntityManager<Driver extends IDatabaseDriver = IDatabaseDriver> {
options.populate = Utils.asArray(options.populate).map(field => {
/* istanbul ignore next */
if (typeof field === 'boolean' || field === '*') {
return [{ field: meta.primaryKeys[0], strategy: options.strategy, all: !!field }];
return [{ field: meta.primaryKeys[0], strategy: options.strategy, all: !!field }]; //
}

// will be handled in QueryBuilder when processing the where condition via CriteriaNode
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/entity/EntityLoader.ts
Expand Up @@ -133,14 +133,16 @@ export class EntityLoader {
return;
}

const [f, ...parts] = p.field.split('.');
let [f, ...parts] = p.field.split('.');

Check failure on line 136 in packages/core/src/entity/EntityLoader.ts

View workflow job for this annotation

GitHub Actions / Lint

'f' is never reassigned. Use 'const' instead

Check failure on line 136 in packages/core/src/entity/EntityLoader.ts

View workflow job for this annotation

GitHub Actions / Lint

'parts' is never reassigned. Use 'const' instead
p.field = f as EntityKey<Entity>;
p.children = p.children || [];
p.children ??= [];
const prop = meta.properties[f];
p.strategy ??= prop.strategy;

if (parts[0] === '*') {
p.all = true;
prop.targetMeta!.props
.filter(prop => prop.lazy || prop.kind !== ReferenceKind.SCALAR)
.forEach(prop => p.children!.push({ field: prop.name as EntityKey, strategy: p.strategy }));
} else {
p.children.push(this.expandNestedPopulate(prop.type, parts, p.strategy, p.all));
}
Expand Down
13 changes: 11 additions & 2 deletions tests/issues/GH5213.test.ts
Expand Up @@ -9,6 +9,7 @@ import {
Property,
Ref,
} from '@mikro-orm/sqlite';
import { mockLogger } from '../helpers';

@Entity()
class A {
Expand Down Expand Up @@ -45,7 +46,7 @@ class C {
id!: number;

@ManyToOne({ entity: () => D, deleteRule: 'cascade', updateRule: 'cascade', eager: true })
e!: Ref<D>;
d!: Ref<D>;

@OneToOne(() => B, b => b.c1, { nullable: true })
b1!: B | null;
Expand Down Expand Up @@ -84,13 +85,14 @@ test('GH #5213', async () => {
orm.em.create(A, {
b: [{
c1: {
e: { name: 'test' },
d: { name: 'test' },
},
}],
});
await orm.em.flush();
orm.em.clear();

const mock = mockLogger(orm);
const [a] = await orm.em.findAll(A, { populate: ['b.*'] });

a.b.getItems().forEach(b => orm.em.assign(b, {
Expand All @@ -105,4 +107,11 @@ test('GH #5213', async () => {

// expecting that count is 0, because the only existing C is not referenced in any B anymore
expect(count).toBe(0);

expect(mock.mock.calls[0][0]).toMatch('select `a0`.*, `b1`.`id` as `b1__id`, `b1`.`a_id` as `b1__a_id`, `b1`.`c1_id` as `b1__c1_id`, `b1`.`c2_id` as `b1__c2_id`, `a2`.`id` as `a2__id`, `c3`.`id` as `c3__id`, `c3`.`d_id` as `c3__d_id`, `d4`.`id` as `d4__id`, `d4`.`name` as `d4__name`, `c5`.`id` as `c5__id`, `c5`.`d_id` as `c5__d_id`, `d6`.`id` as `d6__id`, `d6`.`name` as `d6__name` from `a` as `a0` left join `b` as `b1` on `a0`.`id` = `b1`.`a_id` left join `a` as `a2` on `b1`.`a_id` = `a2`.`id` left join `c` as `c3` on `b1`.`c1_id` = `c3`.`id` left join `d` as `d4` on `c3`.`d_id` = `d4`.`id` left join `c` as `c5` on `b1`.`c2_id` = `c5`.`id` left join `d` as `d6` on `c5`.`d_id` = `d6`.`id`');
expect(mock.mock.calls[1][0]).toMatch('begin');
expect(mock.mock.calls[2][0]).toMatch('update `b` set `c1_id` = NULL where `id` = 1');
expect(mock.mock.calls[3][0]).toMatch('delete from `c` where `id` in (1)');
expect(mock.mock.calls[4][0]).toMatch('commit');
expect(mock.mock.calls[5][0]).toMatch('select count(*) as `count` from `c` as `c0`');
});

0 comments on commit 65d1575

Please sign in to comment.