Skip to content

Commit

Permalink
fix(core): fix propagation of locking option with select-in population
Browse files Browse the repository at this point in the history
Closes #1670
  • Loading branch information
B4nan committed Nov 22, 2021
1 parent a9af269 commit f3990d0
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 11 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/EntityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
const cached = await this.tryCache<T, Loaded<T, P>>(entityName, options.cache, [entityName, 'em.findOne', options, where], options.refresh, true);

if (cached?.data) {
await this.entityLoader.populate<T, P>(entityName, [cached.data as T], options.populate as unknown as PopulateOptions<T>[], { ...options, where, convertCustomTypes: false, lookup: false });
await this.entityLoader.populate<T, P>(entityName, [cached.data as T], options.populate as unknown as PopulateOptions<T>[], { ...options as Dictionary, where, convertCustomTypes: false, lookup: false });
return cached.data;
}

Expand Down Expand Up @@ -848,7 +848,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
}

const preparedPopulate = this.preparePopulate<T>(entityName, options.populate, options.strategy);
await this.entityLoader.populate(entityName, [entity], preparedPopulate, { ...options, where, convertCustomTypes: false, lookup: false });
await this.entityLoader.populate(entityName, [entity], preparedPopulate, { ...options as Dictionary, where, convertCustomTypes: false, lookup: false });

return entity as Loaded<T, P>;
}
Expand Down
19 changes: 11 additions & 8 deletions packages/core/src/entity/EntityLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { QueryHelper } from '../utils/QueryHelper';
import { Utils } from '../utils/Utils';
import { ValidationError } from '../errors';
import type { Collection } from './Collection';
import type { QueryOrderMap } from '../enums';
import type { QueryOrderMap , LockMode } from '../enums';
import { LoadStrategy, QueryOrder, ReferenceType } from '../enums';
import { Reference } from './Reference';
import type { EntityField, FindOptions } from '../drivers/IDatabaseDriver';
Expand All @@ -19,6 +19,7 @@ export type EntityLoaderOptions<T, P extends string = never> = {
convertCustomTypes?: boolean;
filters?: Dictionary<boolean | Dictionary> | string[] | boolean;
strategy?: LoadStrategy;
lockMode?: Exclude<LockMode, LockMode.OPTIMISTIC>;
};

export class EntityLoader {
Expand Down Expand Up @@ -219,14 +220,17 @@ export class EntityLoader {
}

const ids = Utils.unique(children.map(e => Utils.getPrimaryKeyValues(e, e.__meta!.primaryKeys, true)));
const where = { ...QueryHelper.processWhere({ [fk]: { $in: ids } }, meta.name!, this.metadata, this.driver.getPlatform(), !options.convertCustomTypes), ...(options.where as Dictionary) } as FilterQuery<T>;
const cond1 = QueryHelper.processWhere({ [fk]: { $in: ids } }, meta.name!, this.metadata, this.driver.getPlatform(), !options.convertCustomTypes);

const where = options.where[fk]
? { $and: [cond1, options.where] }
: { ...cond1, ...(options.where as Dictionary) };
const fields = this.buildFields<T>(prop, options);
const { refresh, filters, convertCustomTypes, lockMode, strategy } = options;

return this.em.find<T>(prop.type, where, {
return this.em.find<T>(prop.type, where as FilterQuery<T>, {
refresh, filters, convertCustomTypes, lockMode, strategy,
orderBy: [...Utils.asArray(options.orderBy), ...Utils.asArray(prop.orderBy), { [fk]: QueryOrder.ASC }] as QueryOrderMap<T>[],
refresh: options.refresh,
filters: options.filters,
convertCustomTypes: options.convertCustomTypes,
populate: populate.children as never,
fields: fields.length > 0 ? fields : undefined,
}) as Promise<T[]>;
Expand Down Expand Up @@ -335,8 +339,7 @@ export class EntityLoader {
}

if (filters) {
/* istanbul ignore next */
return this.em.applyFilters(prop.type, subCond, options.filters ?? {}, 'read');
return this.em.applyFilters(prop.type, subCond, options.filters, 'read');
}

return subCond;
Expand Down
2 changes: 1 addition & 1 deletion packages/knex/src/AbstractSqlDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
const qb = this.createQueryBuilder<T>(prop.type, ctx, !!ctx).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES).withSchema(options?.schema);
const populate = this.autoJoinOneToOneOwner(targetMeta, [{ field: prop.pivotTable }]);
const fields = this.buildFields(targetMeta, (options?.populate ?? []) as unknown as PopulateOptions<T>[], [], qb, options?.fields as Field<T>[]);
qb.select(fields).populate(populate).where(where).orderBy(orderBy!);
qb.select(fields).populate(populate).where(where).orderBy(orderBy!).setLockMode(options?.lockMode, options?.lockTableAliases);

if (owners.length === 1 && (options?.offset != null || options?.limit != null)) {
qb.limit(options.limit, options.offset);
Expand Down
19 changes: 19 additions & 0 deletions tests/EntityManager.postgre.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,25 @@ describe('EntityManagerPostgre', () => {
expect(mock.mock.calls[2][0]).toMatch('commit');
});

test('locking and select-in population (GH #1670)', async () => {
await createBooksWithTags();
const mock = mockLogger(orm, ['query']);

await orm.em.transactional(async em => {
await em.find(Book2, {}, {
lockMode: LockMode.PESSIMISTIC_PARTIAL_WRITE,
populate: ['author', 'tags'],
strategy: LoadStrategy.SELECT_IN,
});
});
expect(mock.mock.calls.length).toBe(5);
expect(mock.mock.calls[0][0]).toMatch('begin');
expect(mock.mock.calls[1][0]).toMatch(`select "b0"."uuid_pk", "b0"."created_at", "b0"."title", "b0"."price", "b0"."double", "b0"."meta", "b0"."author_id", "b0"."publisher_id", "b0".price * 1.19 as "price_taxed" from "book2" as "b0" where "b0"."author_id" is not null for update skip locked`);
expect(mock.mock.calls[2][0]).toMatch(`select "a0".* from "author2" as "a0" where "a0"."id" in ($1) and "a0"."id" is not null order by "a0"."id" asc for update skip locked`);
expect(mock.mock.calls[3][0]).toMatch(`select "b0".*, "b1"."book_tag2_id" as "fk__book_tag2_id", "b1"."book2_uuid_pk" as "fk__book2_uuid_pk" from "book_tag2" as "b0" left join "book2_tags" as "b1" on "b0"."id" = "b1"."book_tag2_id" where "b1"."book2_uuid_pk" in ($1, $2, $3) order by "b1"."order" asc for update skip locked`);
expect(mock.mock.calls[4][0]).toMatch('commit');
});

test('stable results of serialization', async () => {
const god = new Author2('God', 'hello@heaven.god');
const bible = new Book2('Bible', god);
Expand Down

0 comments on commit f3990d0

Please sign in to comment.