fix(core): ignore collection operators in populateWhere conditions

mikro-orm · Mar 2, 2024 · 7b6b363 · 7b6b363
1 parent b6efd44
commit 7b6b363
Show file tree

Hide file tree

Showing 9 changed files with 147 additions and 11 deletions.
diff --git a/packages/knex/src/query/ArrayCriteriaNode.ts b/packages/knex/src/query/ArrayCriteriaNode.ts
@@ -18,9 +18,9 @@ export class ArrayCriteriaNode<T extends object> extends CriteriaNode<T> {
     });
   }
 
-  override willAutoJoin(qb: IQueryBuilder<T>, alias?: string) {
+  override willAutoJoin(qb: IQueryBuilder<T>, alias?: string, options?: ICriteriaNodeProcessOptions) {
     return this.payload.some((node: CriteriaNode<T>) => {
-      return node.willAutoJoin(qb, alias);
+      return node.willAutoJoin(qb, alias, options);
     });
   }
 

diff --git a/packages/knex/src/query/CriteriaNode.ts b/packages/knex/src/query/CriteriaNode.ts
@@ -59,7 +59,7 @@ export class CriteriaNode<T extends object> implements ICriteriaNode<T> {
     return false;
   }
 
-  willAutoJoin(qb: IQueryBuilder<T>, alias?: string) {
+  willAutoJoin(qb: IQueryBuilder<T>, alias?: string, options?: ICriteriaNodeProcessOptions) {
     return false;
   }
 

diff --git a/packages/knex/src/query/ObjectCriteriaNode.ts b/packages/knex/src/query/ObjectCriteriaNode.ts
@@ -29,9 +29,16 @@ export class ObjectCriteriaNode<T extends object> extends CriteriaNode<T> {
 
     if (this.shouldAutoJoin(qb, nestedAlias)) {
       if (keys.some(k => ['$some', '$none', '$every'].includes(k))) {
+        // ignore collection operators when used on a non-relational property - this can happen when they get into
+        // populateWhere via `infer` on m:n properties with select-in strategy
+        if (!this.prop?.targetMeta) {
+          return {};
+        }
+
         const $and: Dictionary[] = [];
+        const knownKey = [ReferenceKind.SCALAR, ReferenceKind.MANY_TO_ONE, ReferenceKind.EMBEDDED].includes(this.prop!.kind) || (this.prop!.kind === ReferenceKind.ONE_TO_ONE && this.prop!.owner);
         const primaryKeys = this.metadata.find(this.entityName)!.primaryKeys.map(pk => {
-          return [QueryType.SELECT, QueryType.COUNT].includes(qb.type!) ? `${alias}.${pk}` : pk;
+          return [QueryType.SELECT, QueryType.COUNT].includes(qb.type!) ? `${knownKey ? alias : ownerAlias}.${pk}` : pk;
         });
 
         for (const key of keys) {
@@ -100,8 +107,8 @@ export class ObjectCriteriaNode<T extends object> extends CriteriaNode<T> {
     }, {} as Dictionary);
   }
 
-  override willAutoJoin(qb: IQueryBuilder<T>, alias?: string) {
-    const nestedAlias = qb.getAliasForJoinPath(this.getPath());
+  override willAutoJoin(qb: IQueryBuilder<T>, alias?: string, options?: ICriteriaNodeProcessOptions) {
+    const nestedAlias = qb.getAliasForJoinPath(this.getPath(), options);
     const ownerAlias = alias || qb.alias;
     const keys = Object.keys(this.payload);
 
@@ -115,7 +122,7 @@ export class ObjectCriteriaNode<T extends object> extends CriteriaNode<T> {
 
     return keys.some(field => {
       const childNode = this.payload[field] as CriteriaNode<T>;
-      return childNode.willAutoJoin(qb, this.prop ? alias : ownerAlias);
+      return childNode.willAutoJoin(qb, this.prop ? alias : ownerAlias, options);
     });
   }
 
@@ -173,6 +180,10 @@ export class ObjectCriteriaNode<T extends object> extends CriteriaNode<T> {
       return false;
     }
 
+    if (keys.some(k => ['$some', '$none', '$every'].includes(k))) {
+      return true;
+    }
+
     const meta = this.metadata.find(this.entityName)!;
     const embeddable = this.prop.kind === ReferenceKind.EMBEDDED;
     const knownKey = [ReferenceKind.SCALAR, ReferenceKind.MANY_TO_ONE, ReferenceKind.EMBEDDED].includes(this.prop.kind) || (this.prop.kind === ReferenceKind.ONE_TO_ONE && this.prop.owner);

diff --git a/packages/knex/src/query/QueryBuilder.ts b/packages/knex/src/query/QueryBuilder.ts
@@ -354,7 +354,7 @@ export class QueryBuilder<T extends object = AnyEntity> {
     const criteriaNode = CriteriaNodeFactory.createNode<T>(this.metadata, this.mainAlias.entityName, cond);
     const ignoreBranching = this.__populateWhere === 'infer';
 
-    if ([QueryType.UPDATE, QueryType.DELETE].includes(this.type!) && criteriaNode.willAutoJoin(this)) {
+    if ([QueryType.UPDATE, QueryType.DELETE].includes(this.type!) && criteriaNode.willAutoJoin(this, undefined, { ignoreBranching })) {
       // use sub-query to support joining
       this.setFlag(this.type === QueryType.UPDATE ? QueryFlag.UPDATE_SUB_QUERY : QueryFlag.DELETE_SUB_QUERY);
       this.select(this.mainAlias.metadata!.primaryKeys, true);

diff --git a/packages/knex/src/query/QueryBuilderHelper.ts b/packages/knex/src/query/QueryBuilderHelper.ts
@@ -553,7 +553,7 @@ export class QueryBuilderHelper {
 
     /* istanbul ignore next */
     if (!op) {
-      throw new Error(`Invalid query condition: ${inspect(cond)}`);
+      throw new Error(`Invalid query condition: ${inspect(cond, { depth: 5 })}`);
     }
 
     const replacement = this.getOperatorReplacement(op, value);

diff --git a/packages/knex/src/query/ScalarCriteriaNode.ts b/packages/knex/src/query/ScalarCriteriaNode.ts
@@ -34,7 +34,7 @@ export class ScalarCriteriaNode<T extends object> extends CriteriaNode<T> {
     return this.payload;
   }
 
-  override willAutoJoin<T>(qb: IQueryBuilder<T>, alias?: string) {
+  override willAutoJoin<T>(qb: IQueryBuilder<T>, alias?: string, options?: ICriteriaNodeProcessOptions) {
     return this.shouldJoin();
   }
 

diff --git a/packages/knex/src/typings.ts b/packages/knex/src/typings.ts
@@ -190,7 +190,7 @@ export interface ICriteriaNode<T extends object> {
   index?: number;
   process(qb: IQueryBuilder<T>, options?: ICriteriaNodeProcessOptions): any;
   shouldInline(payload: any): boolean;
-  willAutoJoin(qb: IQueryBuilder<T>, alias?: string): boolean;
+  willAutoJoin(qb: IQueryBuilder<T>, alias?: string, options?: ICriteriaNodeProcessOptions): boolean;
   shouldRename(payload: any): boolean;
   renameFieldToPK<T>(qb: IQueryBuilder<T>): string;
   getPath(addIndex?: boolean): string;

diff --git a/tests/features/collection/collection-operators.test.ts b/tests/features/collection/collection-operators.test.ts
@@ -67,6 +67,7 @@ beforeAll(async () => {
     entities: [Author],
     dbName: ':memory:',
     loggerFactory: options => new SimpleLogger(options),
+    populateWhere: 'infer',
   });
 
   await orm.schema.createSchema();

diff --git a/tests/features/collection/populate-where-infer.test.ts b/tests/features/collection/populate-where-infer.test.ts
@@ -0,0 +1,124 @@
+import { Collection, Entity, ManyToOne, MikroORM, OneToMany, PrimaryKey, Property, Ref } from '@mikro-orm/sqlite';
+import { mockLogger } from '../../helpers';
+
+@Entity()
+class User {
+
+  @PrimaryKey()
+  id!: number;
+
+  @ManyToOne(() => Location, { ref: true })
+  location!: Ref<Location>;
+
+  @OneToMany(() => Server, x => x.user)
+  servers = new Collection<Server>(this);
+
+}
+
+@Entity()
+class Server {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  name!: string;
+
+  @ManyToOne(() => User, { ref: true })
+  user!: Ref<User>;
+
+}
+
+@Entity()
+class Location {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  location!: string;
+
+  @OneToMany(() => User, x => x.location)
+  users = new Collection<User>(this);
+
+}
+
+let orm: MikroORM;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    dbName: ':memory:',
+    entities: [User],
+  });
+  await orm.schema.createSchema();
+
+  orm.em.create(User, {
+    id: 1,
+    location: {
+      location: 'loc name',
+    },
+    servers: [
+      { name: 's1' },
+      { name: 's2' },
+      { name: 'test' },
+    ],
+  });
+
+  await orm.em.flush();
+  orm.em.clear();
+});
+
+afterAll(async () => {
+  await orm.close(true);
+});
+
+test('$every without populateWhere', async () => {
+  const mock = mockLogger(orm);
+
+  const res = await orm.em.fork().find(
+    User,
+    {
+      servers: {
+        $every: {
+          name: {
+            $ne: 'test',
+          },
+        },
+      },
+    },
+    {
+      populate: ['servers'],
+    },
+  );
+  expect(res).toHaveLength(0);
+  expect(mock.mock.calls[0][0]).toMatch('select `u0`.*, `s1`.`id` as `s1__id`, `s1`.`name` as `s1__name`, `s1`.`user_id` as `s1__user_id` ' +
+    'from `user` as `u0` ' +
+    'left join `server` as `s1` on `u0`.`id` = `s1`.`user_id` ' +
+    'where `u0`.`id` not in (select `u0`.`id` from `user` as `u0` inner join `server` as `s1` on `u0`.`id` = `s1`.`user_id` where not (`s1`.`name` != \'test\'))');
+});
+
+test('$every with populateWhere: infer', async () => {
+  const mock = mockLogger(orm);
+
+  const res = await orm.em.fork().find(
+    User,
+    {
+      servers: {
+        $every: {
+          name: {
+            $ne: 'test',
+          },
+        },
+      },
+    },
+    {
+      populate: ['servers'],
+      populateWhere: 'infer',
+    },
+  );
+  expect(res).toHaveLength(0);
+  expect(mock.mock.calls[0][0]).toMatch('select `u0`.*, `s1`.`id` as `s1__id`, `s1`.`name` as `s1__name`, `s1`.`user_id` as `s1__user_id` ' +
+    'from `user` as `u0` ' +
+    'left join `server` as `s1` on `u0`.`id` = `s1`.`user_id` ' +
+    'where `u0`.`id` not in (select `u0`.`id` from `user` as `u0` inner join `server` as `s1` on `u0`.`id` = `s1`.`user_id` where not (`s1`.`name` != \'test\'))');
+});