Skip to content

Commit

Permalink
fix(core): make FilterQuery strict again!
Browse files Browse the repository at this point in the history 
The inference fix from fcb1739 introduced an issue with `FilterQuery`
that started accepting any keys. This was because of the extension, which
was not a proper way to fix this. The original issue was in fact caused by
something else than the PK type - it was more about the usage of naked `T`
type in the `FilterQuery` union. We now define it as `T & {}` which helps
with getting the right inference priorities. The fix from fcb1739 is now
removed and the types are simplified (the `Query` generic is removed).

Related: fcb1739
  • Loading branch information
@B4nan
B4nan committed Jan 10, 2023
1 parent 8bab7f6 commit 5427097
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 20 deletions.
21 changes: 8 additions & 13 deletions packages/core/src/EntityManager.ts
View file
Expand Up @@ -141,8 +141,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
async find<
Entity extends object,
Hint extends string = never,
Query extends FilterQuery<Entity> = FilterQuery<Entity>,
>(entityName: EntityName<Entity>, where: Query, options: FindOptions<Entity, Hint> = {}): Promise<Loaded<Entity, Hint>[]> {
>(entityName: EntityName<Entity>, where: FilterQuery<Entity>, options: FindOptions<Entity, Hint> = {}): Promise<Loaded<Entity, Hint>[]> {
if (options.disableIdentityMap) {
const em = this.getContext(false);
const fork = em.fork();
Expand All @@ -155,7 +154,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
const em = this.getContext();
await em.tryFlush(entityName, options);
entityName = Utils.className(entityName);
where = await em.processWhere(entityName, where as FilterQuery<Entity>, options, 'read') as Query;
where = await em.processWhere(entityName, where, options, 'read') as FilterQuery<Entity>;
em.validator.validateParams(where);
options.orderBy = options.orderBy || {};
options.populate = em.preparePopulate<Entity, Hint>(entityName, options) as unknown as Populate<Entity, Hint>;
Expand Down Expand Up @@ -390,8 +389,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
async findAndCount<
Entity extends object,
Hint extends string = never,
Query extends FilterQuery<Entity> = FilterQuery<Entity>,
>(entityName: EntityName<Entity>, where: Query, options: FindOptions<Entity, Hint> = {}): Promise<[Loaded<Entity, Hint>[], number]> {
>(entityName: EntityName<Entity>, where: FilterQuery<Entity>, options: FindOptions<Entity, Hint> = {}): Promise<[Loaded<Entity, Hint>[], number]> {
const [entities, count] = await Promise.all([
this.find<Entity, Hint>(entityName, where, options),
this.count(entityName, where, options),
Expand Down Expand Up @@ -430,8 +428,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
async findOne<
Entity extends object,
Hint extends string = never,
Query extends FilterQuery<Entity> = FilterQuery<Entity>,
>(entityName: EntityName<Entity>, where: Query, options: FindOneOptions<Entity, Hint> = {}): Promise<Loaded<Entity, Hint> | null> {
>(entityName: EntityName<Entity>, where: FilterQuery<Entity>, options: FindOneOptions<Entity, Hint> = {}): Promise<Loaded<Entity, Hint> | null> {
if (options.disableIdentityMap) {
const em = this.getContext(false);
const fork = em.fork();
Expand All @@ -445,7 +442,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
await em.tryFlush(entityName, options);
entityName = Utils.className(entityName);
const meta = em.metadata.get<Entity>(entityName);
where = await em.processWhere(entityName, where as FilterQuery<Entity>, options, 'read') as Query;
where = await em.processWhere(entityName, where, options, 'read');
em.validator.validateEmptyWhere(where);
em.checkLockRequirements(options.lockMode, meta);
let entity = em.unitOfWork.tryGetById<Entity>(entityName, where, options.schema);
Expand Down Expand Up @@ -506,8 +503,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
async findOneOrFail<
Entity extends object,
Hint extends string = never,
Query extends FilterQuery<Entity> = FilterQuery<Entity>,
>(entityName: EntityName<Entity>, where: Query, options: FindOneOrFailOptions<Entity, Hint> = {}): Promise<Loaded<Entity, Hint>> {
>(entityName: EntityName<Entity>, where: FilterQuery<Entity>, options: FindOneOrFailOptions<Entity, Hint> = {}): Promise<Loaded<Entity, Hint>> {
let entity: Loaded<Entity, Hint> | null;
let isStrictViolation = false;

Expand Down Expand Up @@ -1138,11 +1134,10 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
async count<
Entity extends object,
Hint extends string = never,
Query extends FilterQuery<Entity> = FilterQuery<Entity>,
>(entityName: EntityName<Entity>, where: Query = {} as Query, options: CountOptions<Entity, Hint> = {}): Promise<number> {
>(entityName: EntityName<Entity>, where: FilterQuery<Entity> = {} as FilterQuery<Entity>, options: CountOptions<Entity, Hint> = {}): Promise<number> {
const em = this.getContext(false);
entityName = Utils.className(entityName);
where = await em.processWhere(entityName, where as FilterQuery<Entity>, options as FindOptions<Entity, Hint>, 'read') as Query;
where = await em.processWhere(entityName, where, options as FindOptions<Entity, Hint>, 'read') as FilterQuery<Entity>;
options.populate = em.preparePopulate(entityName, options) as unknown as Populate<Entity>;
em.validator.validateParams(where);

Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/typings.ts
View file
Expand Up @@ -93,7 +93,8 @@ export type Query<T> = T extends object
: FilterValue<T>;

export type ObjectQuery<T> = ExpandObject<T> & OperatorMap<T>;
export type FilterQuery<T> = ObjectQuery<T> | NonNullable<ExpandScalar<Primary<T>>> | T | FilterQuery<T>[];
// eslint-disable-next-line @typescript-eslint/ban-types
export type FilterQuery<T> = ObjectQuery<T> | NonNullable<ExpandScalar<Primary<T>>> | (T & {}) | FilterQuery<T>[];
export type QBFilterQuery<T = any> = FilterQuery<T> | Dictionary;

export interface IWrappedEntity<
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/unit-of-work/ChangeSetPersister.ts
View file
Expand Up @@ -208,7 +208,7 @@ export class ChangeSetPersister {
convertCustomTypes: false,
processCollections: false,
});
const cond = changeSets.map(cs => cs.getPrimaryKey(true) as Dictionary);
const cond = changeSets.map(cs => cs.getPrimaryKey(true) as FilterQuery<T>);

changeSets.forEach((changeSet, idx) => {
this.checkConcurrencyKeys(meta, changeSet, cond[idx]);
Expand Down Expand Up @@ -263,7 +263,7 @@ export class ChangeSetPersister {
});

if (meta.concurrencyCheckKeys.size === 0 && (!meta.versionProperty || changeSet.entity[meta.versionProperty] == null)) {
return this.driver.nativeUpdate(changeSet.name, changeSet.getPrimaryKey() as Dictionary, changeSet.payload, options);
return this.driver.nativeUpdate(changeSet.name, changeSet.getPrimaryKey() as FilterQuery<T>, changeSet.payload, options);
}

const cond = changeSet.getPrimaryKey(true) as Dictionary;
Expand Down
2 changes: 1 addition & 1 deletion packages/knex/src/query/QueryBuilder.ts
View file
Expand Up @@ -258,7 +258,7 @@ export class QueryBuilder<T extends object = AnyEntity> {
operator = operator || '$and';
} else {
cond = QueryHelper.processWhere({
where: cond,
where: cond as FilterQuery<T>,
entityName: this.mainAlias.entityName,
metadata: this.metadata,
platform: this.platform,
Expand Down
6 changes: 3 additions & 3 deletions packages/mongodb/src/MongoDriver.ts
View file
Expand Up @@ -33,7 +33,7 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {

const fields = this.buildFields(entityName, options.populate as unknown as PopulateOptions<T>[] || [], options.fields);
where = this.renameFields(entityName, where, true);
const res = await this.rethrow(this.getConnection('read').find(entityName, where as Dictionary, options.orderBy, options.limit, options.offset, fields, options.ctx));
const res = await this.rethrow(this.getConnection('read').find(entityName, where, options.orderBy, options.limit, options.offset, fields, options.ctx));

return res.map(r => this.mapResult<T>(r, this.metadata.find<T>(entityName))!);
}
Expand All @@ -51,7 +51,7 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {

const fields = this.buildFields(entityName, options.populate as unknown as PopulateOptions<T>[] || [], options.fields);
where = this.renameFields(entityName, where, true);
const res = await this.rethrow(this.getConnection('read').find(entityName, where as Dictionary, options.orderBy, 1, undefined, fields, options.ctx));
const res = await this.rethrow(this.getConnection('read').find(entityName, where, options.orderBy, 1, undefined, fields, options.ctx));

return this.mapResult<T>(res[0], this.metadata.find(entityName)!);
}
Expand Down Expand Up @@ -221,7 +221,7 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {
return data;
}

private buildFilterById<T>(entityName: string, id: string): FilterQuery<T> {
private buildFilterById<T extends { _id: any }>(entityName: string, id: string): FilterQuery<T> {
const meta = this.metadata.find(entityName)!;

if (meta.properties[meta.primaryKeys[0]].type.toLowerCase() === 'objectid') {
Expand Down
14 changes: 14 additions & 0 deletions tests/types.test.ts
View file
Expand Up @@ -530,4 +530,18 @@ describe('check typings', () => {
const test: MemberNotification = {} as Loaded<MemberNotification, 'notification'>;
});

test('inference of entity type', async () => {
interface MemberNotification {
id: string;
notification?: Ref<Notification>;
}

interface Notification {
id: string;
}

const em = { findOne: jest.fn() as any } as EntityManager;
const res: Loaded<MemberNotification> | null = await em.findOne('MemberNotification' as EntityName<MemberNotification>, {} as MemberNotification | string);
});

});

0 comments on commit 5427097

Please sign in to comment.