fix(query-builder): fix mapping of 1:1 inverse sides

Closes #849
mikro-orm · Sep 18, 2020 · a46281e · a46281e
1 parent 1089c86
commit a46281e
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 18 deletions.
diff --git a/packages/knex/src/query/CriteriaNode.ts b/packages/knex/src/query/CriteriaNode.ts
@@ -54,7 +54,7 @@ export class CriteriaNode {
 
     switch (type) {
       case ReferenceType.MANY_TO_ONE: return false;
-      case ReferenceType.ONE_TO_ONE: return /* istanbul ignore next */ !this.prop!.owner && !this.parent?.parent;
+      case ReferenceType.ONE_TO_ONE: return operator || (/* istanbul ignore next */ !this.prop!.owner && !this.parent?.parent);
       case ReferenceType.ONE_TO_MANY: return scalar || operator;
       case ReferenceType.MANY_TO_MANY: return scalar || operator;
       default: return false;
@@ -71,24 +71,24 @@ export class CriteriaNode {
       return Utils.getPrimaryKeyHash(this.prop!.joinColumns);
     }
 
-    const meta = this.metadata.find(this.prop!.type)!;
     const alias = qb.getAliasForJoinPath(this.getPath());
-    const pks = Utils.flatten(meta.primaryKeys.map(primaryKey => meta.properties[primaryKey].fieldNames));
-
-    return Utils.getPrimaryKeyHash(pks.map(col => `${alias}.${col}`));
+    return Utils.getPrimaryKeyHash(this.prop!.referencedColumnNames.map(col => `${alias}.${col}`));
   }
 
   getPath(): string {
+    const parentPath = this.parent?.getPath();
     let ret = this.parent && this.prop ? this.prop.name : this.entityName;
 
+    if (parentPath && this.prop?.reference === ReferenceType.SCALAR) {
+      return parentPath;
+    }
+
     if (this.parent && Array.isArray(this.parent.payload) && this.parent.parent && !this.key) {
       ret = this.parent.parent.key!;
     }
 
-    const parentPath = this.parent?.getPath();
-
     if (parentPath) {
-      ret = this.parent!.getPath() + '.' + ret;
+      ret = parentPath + '.' + ret;
     } else if (this.parent?.entityName && ret) {
       ret = this.parent.entityName + '.' + ret;
     }

diff --git a/packages/knex/src/query/ObjectCriteriaNode.ts b/packages/knex/src/query/ObjectCriteriaNode.ts
@@ -1,4 +1,4 @@
-import { ReferenceType, Utils } from '@mikro-orm/core';
+import { Dictionary, ReferenceType, Utils } from '@mikro-orm/core';
 import { CriteriaNode } from './CriteriaNode';
 import { IQueryBuilder } from '../typings';
 import { QueryType } from './enums';
@@ -25,13 +25,8 @@ export class ObjectCriteriaNode extends CriteriaNode {
       const virtual = childNode.prop?.persist === false;
 
       if (childNode.shouldInline(payload)) {
-        const operators = Object.keys(payload).filter(k => Utils.isOperator(k, false));
-        operators.forEach(op => {
-          const tmp = payload[op];
-          delete payload[op];
-          payload[`${alias}.${field}`] = { [op]: tmp, ...(payload[`${alias}.${field}`] || {}) };
-        });
-        Object.assign(o, payload);
+        const childAlias = qb.getAliasForJoinPath(childNode.getPath());
+        this.inlineChildPayload(o, payload, field, alias, childAlias);
       } else if (childNode.shouldRename(payload)) {
         o[childNode.renameFieldToPK(qb)] = payload;
       } else if (virtual || operator || customExpression || field.includes('.') || ![QueryType.SELECT, QueryType.COUNT].includes(qb.type)) {
@@ -70,6 +65,20 @@ export class ObjectCriteriaNode extends CriteriaNode {
     return !!this.prop && this.prop.reference !== ReferenceType.SCALAR && !scalar && !operator;
   }
 
+  private inlineChildPayload<T>(o: Dictionary, payload: Dictionary, field: string, alias?: string, childAlias?: string) {
+    for (const k of Object.keys(payload)) {
+      if (Utils.isOperator(k, false)) {
+        const tmp = payload[k];
+        delete payload[k];
+        o[`${alias}.${field}`] = { [k]: tmp, ...(o[`${alias}.${field}`] || {}) };
+      } else if (this.isPrefixed(k) || Utils.isOperator(k)) {
+        o[k] = payload[k];
+      } else {
+        o[`${childAlias}.${k}`] = payload[k];
+      }
+    }
+  }
+
   private shouldAutoJoin(nestedAlias: string | undefined): boolean {
     if (!this.prop || !this.parent) {
       return false;
@@ -92,12 +101,16 @@ export class ObjectCriteriaNode extends CriteriaNode {
     if (this.prop!.reference === ReferenceType.MANY_TO_MANY && (scalar || operator)) {
       qb.join(field, nestedAlias, undefined, 'pivotJoin', this.getPath());
     } else {
-      const prev = qb._fields!.slice();
+      const prev = qb._fields?.slice();
       qb.join(field, nestedAlias, undefined, 'leftJoin', this.getPath());
       qb._fields = prev;
     }
 
     return nestedAlias;
   }
 
+  private isPrefixed(field: string): boolean {
+    return !!field.match(/\w+\./);
+  }
+
 }
diff --git a/tests/QueryBuilder.test.ts b/tests/QueryBuilder.test.ts
@@ -102,7 +102,7 @@ describe('QueryBuilder', () => {
 
     const qb2 = orm.em.createQueryBuilder(Book2);
     qb2.select('*').where({ author: { $ne: null, name: 'Jon Snow' } });
-    expect(qb2.getQuery()).toEqual('select `e0`.*, `e0`.price * 1.19 as `price_taxed` from `book2` as `e0` left join `author2` as `e1` on `e0`.`author_id` = `e1`.`id` where `e1`.`name` = ? and `e0`.`author_id` is not null');
+    expect(qb2.getQuery()).toEqual('select `e0`.*, `e0`.price * 1.19 as `price_taxed` from `book2` as `e0` left join `author2` as `e1` on `e0`.`author_id` = `e1`.`id` where `e0`.`author_id` is not null and `e1`.`name` = ?');
     expect(qb2.getParams()).toEqual(['Jon Snow']);
   });
 
@@ -1633,6 +1633,22 @@ describe('QueryBuilder', () => {
     }
   });
 
+  test('joining 1:1 inverse inside $and condition (GH issue 849)', async () => {
+    const expected = 'select `e0`.* from `foo_baz2` as `e0` left join `foo_bar2` as `e1` on `e0`.`id` = `e1`.`baz_id` where `e1`.`id` in (?)';
+    const sql1 = orm.em.createQueryBuilder(FooBaz2).where({ bar: [123] }).getQuery();
+    expect(sql1).toBe(expected);
+    const sql2 = orm.em.createQueryBuilder(FooBaz2).where({ bar: { id: [123] } }).getQuery();
+    expect(sql2).toBe(expected);
+    const sql3 = orm.em.createQueryBuilder(FooBaz2).where({ bar: { id: { $in: [123] } } }).getQuery();
+    expect(sql3).toBe(expected);
+    const sql4 = orm.em.createQueryBuilder(FooBaz2).where({ $and: [{ bar: { id: { $in: [123] } } }] }).getQuery();
+    expect(sql4).toBe(expected);
+    const sql5 = orm.em.createQueryBuilder(FooBaz2).where({ $and: [{ bar: [123] }] }).getQuery();
+    expect(sql5).toBe(expected);
+    const sql6 = orm.em.createQueryBuilder(FooBaz2).where({ $and: [{ bar: { id: [123] } }] }).getQuery();
+    expect(sql6).toBe(expected);
+  });
+
   afterAll(async () => orm.close(true));
 
 });