Skip to content

Commit

Permalink
fix(core): fix populating relations in parallel via Promise.all (#4415
Browse files Browse the repository at this point in the history
)

This reworks the fix for #4213 which broke populating in parallel.

Closes #4343
  • Loading branch information
B4nan committed Jun 1, 2023
1 parent ef14581 commit f4127a7
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 8 deletions.
13 changes: 7 additions & 6 deletions packages/core/src/entity/EntityLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export class EntityLoader {
throw ValidationError.notDiscoveredEntity(entity, meta, 'populate');
}

const visited = (options as Dictionary).visited ??= new Set<AnyEntity>();
options.where ??= {} as FilterQuery<T>;
options.orderBy ??= {};
options.filters ??= {};
Expand All @@ -65,6 +66,8 @@ export class EntityLoader {
throw ValidationError.invalidPropertyName(entityName, invalid.field);
}

entities = entities.filter(e => !visited.has(e));
entities.forEach(e => visited.add(e));
entities.forEach(e => helper(e).__serializationContext.populate ??= populate as PopulateOptions<T>[]);

for (const pop of populate) {
Expand Down Expand Up @@ -257,10 +260,6 @@ export class EntityLoader {
}

private async findChildren<T extends object>(entities: T[], prop: EntityProperty<T>, populate: PopulateOptions<T>, options: Required<EntityLoaderOptions<T>>): Promise<AnyEntity[]> {
if (!options.refresh) {
entities = entities.filter(e => !helper(e).__loadedRelations.has(prop.name));
}

const children = this.getChildReferences<T>(entities, prop, options.refresh);
const meta = this.metadata.find(prop.type)!;
let fk = Utils.getPrimaryKeyHash(meta.primaryKeys);
Expand Down Expand Up @@ -289,13 +288,13 @@ export class EntityLoader {
const fields = this.buildFields(options.fields, prop);
const { refresh, filters, convertCustomTypes, lockMode, strategy, populateWhere, connectionType } = options;

entities.forEach(e => helper(e).__loadedRelations.add(prop.name));

return this.em.find(prop.type, where, {
refresh, filters, convertCustomTypes, lockMode, populateWhere,
orderBy: [...Utils.asArray(options.orderBy), ...Utils.asArray(prop.orderBy), { [fk]: QueryOrder.ASC }] as QueryOrderMap<T>[],
populate: populate.children as never ?? populate.all ?? [],
strategy, fields, schema, connectionType,
// @ts-ignore not a public option, will be propagated to the populate call
visited: options.visited,
});
}

Expand Down Expand Up @@ -357,6 +356,8 @@ export class EntityLoader {
ignoreLazyScalarProperties,
populateWhere,
connectionType,
// @ts-ignore not a public option, will be propagated to the populate call
visited: options.visited,
});
}

Expand Down
1 change: 0 additions & 1 deletion packages/core/src/entity/WrappedEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export class WrappedEntity<T extends object, PK extends keyof T> {
__em?: EntityManager;
__serializationContext: { root?: SerializationContext<T>; populate?: PopulateOptions<T>[] } = {};
__loadedProperties = new Set<string>();
__loadedRelations = new Set<string>();
__data: Dictionary = {};
__processing = false;

Expand Down
1 change: 0 additions & 1 deletion packages/core/src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ export interface IWrappedEntityInternal<
__touched: boolean;
__originalEntityData?: EntityData<T>;
__loadedProperties: Set<string>;
__loadedRelations: Set<string>;
__identifier?: EntityIdentifier;
__managed: boolean;
__processing: boolean;
Expand Down
70 changes: 70 additions & 0 deletions tests/issues/GH4343.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { MikroORM } from '@mikro-orm/sqlite';
import { Entity, ManyToOne, PrimaryKey, Property, ref, Ref } from '@mikro-orm/core';
import { v4 } from 'uuid';

@Entity()
export class LocalizedString {

@PrimaryKey({ type: 'uuid' })
id = v4();

@Property()
de_DE: string;

@Property({ nullable: true })
en_US?: string;

constructor(de: string) {
this.de_DE = de;
}

}

@Entity()
export class Book {

@PrimaryKey({ type: 'uuid' })
id = v4();

@ManyToOne(() => LocalizedString, { ref: true })
title: Ref<LocalizedString>;

@ManyToOne(() => LocalizedString, { ref: true, nullable: true })
description?: Ref<LocalizedString>;

constructor(title: string, description: string) {
this.title = ref(new LocalizedString(title));
this.description = ref(new LocalizedString(description));
}

}

let orm: MikroORM;

beforeAll(async () => {
orm = await MikroORM.init({
entities: [Book],
dbName: `:memory:`,
});

await orm.schema.refreshDatabase();
});

afterAll(async () => {
await orm.close(true);
});

it('GH #4343', async () => {
async function request(id: number) {
const books = await orm.em.find(Book, {}, {
populate: ['description', 'title'],
});
expect(books[0].title.isInitialized()).toBe(true);
expect(books[0].description?.isInitialized()).toBe(true);
}

const book = new Book('mikro-orm', 'Book about mikro-orm');
await orm.em.fork().persistAndFlush(book);

await Promise.all([request(1), request(2)]);
});

0 comments on commit f4127a7

Please sign in to comment.