From 6de932714bf4fa1f6fa112b5e143a892d16f069c Mon Sep 17 00:00:00 2001 From: TJ Higgins Date: Mon, 2 Aug 2021 13:27:15 -0400 Subject: [PATCH] Simplify check and update tests --- src/query-builder/QueryBuilder.ts | 85 +++++-------------- .../select/query-builder-select.ts | 57 ++++++++----- 2 files changed, 56 insertions(+), 86 deletions(-) diff --git a/src/query-builder/QueryBuilder.ts b/src/query-builder/QueryBuilder.ts index cff9c9e985..3f8cebaed1 100644 --- a/src/query-builder/QueryBuilder.ts +++ b/src/query-builder/QueryBuilder.ts @@ -994,21 +994,20 @@ export abstract class QueryBuilder { if (metadata.hasRelationWithPropertyPath(path)) { const relation = metadata.findRelationWithPropertyPath(path)!; const isEntityTarget = relation.inverseEntityMetadata.target === entity[key].constructor; - const entityKeysLength = Object.keys(entity[key]).length; - // There's also cases where we don't want to return back all of the properties. - // These handles the situation where someone passes the model & we don't need to make - // a HUGE `where` to uniquely look up the entity. - - // In the case of a *-to-one, there's only ever one possible entity on the other side - // so if the join columns are all defined we can return just the relation itself - // because it will fetch only the join columns and do the lookup. - if (relation.relationType === "one-to-one" || relation.relationType === "many-to-one") { - const joinColumns = relation.joinColumns - .map(j => j.referencedColumn) - .filter((j): j is ColumnMetadata => !!j); + if (isEntityTarget) { + // There's also cases where we don't want to return back all of the properties. + // These handles the situation where someone passes the model & we don't need to make + // a HUGE `where` to uniquely look up the entity. + + // In the case of a *-to-one, there's only ever one possible entity on the other side + // so if the join columns are all defined we can return just the relation itself + // because it will fetch only the join columns and do the lookup. + if (relation.relationType === "one-to-one" || relation.relationType === "many-to-one") { + const joinColumns = relation.joinColumns + .map(j => j.referencedColumn) + .filter((j): j is ColumnMetadata => !!j); - if (isEntityTarget) { const hasAllJoinColumns = joinColumns.length > 0 && joinColumns.every( column => column.getEntityValue(entity[key], false) ); @@ -1017,40 +1016,19 @@ export abstract class QueryBuilder { paths.push(path); continue; } - } else { - const seenJoinColumns = new Set(); - for (const joinColumn of joinColumns) { - const entityValueMap = joinColumn.getEntityValueMap(entity[key]); - for (const [columnName, value] of Object.entries(entityValueMap || {})) { - if (value) { - seenJoinColumns.add(columnName); - } else { - break; - } - } - } - - const hasOnlyJoinColumns = joinColumns.length > 0 && seenJoinColumns.size === entityKeysLength; - - if (hasOnlyJoinColumns) { - paths.push(path); - continue; - } } - } - if (relation.relationType === "one-to-many" || relation.relationType === "many-to-many") { - throw new Error(`Cannot query across ${relation.relationType} for property ${path}`); - } + if (relation.relationType === "one-to-many" || relation.relationType === "many-to-many") { + throw new Error(`Cannot query across ${relation.relationType} for property ${path}`); + } - // For any other case, if the `entity[key]` contains all of the primary keys we can do a - // lookup via these. We don't need to look up via any other values 'cause these are - // the unique primary keys. - // This handles the situation where someone passes the model & we don't need to make - // a HUGE where. - const primaryColumns = relation.inverseEntityMetadata.primaryColumns; + // For any other case, if the `entity[key]` contains all of the primary keys we can do a + // lookup via these. We don't need to look up via any other values 'cause these are + // the unique primary keys. + // This handles the situation where someone passes the model & we don't need to make + // a HUGE where. + const primaryColumns = relation.inverseEntityMetadata.primaryColumns; - if (isEntityTarget) { const hasAllPrimaryKeys = primaryColumns.length > 0 && primaryColumns.every( column => column.getEntityValue(entity[key], false) ); @@ -1062,27 +1040,6 @@ export abstract class QueryBuilder { paths.push(...subPaths); continue; } - } else { - const seenPrimaryColumns = new Set(); - for (const primaryColumn of primaryColumns) { - const entityValueMap = primaryColumn.getEntityValueMap(entity[key]); - for (const [columnName, value] of Object.entries(entityValueMap || {})) { - if (value) { - seenPrimaryColumns.add(columnName); - } else { - break; - } - } - } - const hasOnlyPrimaryKeys = primaryColumns.length > 0 && seenPrimaryColumns.size === entityKeysLength; - - if (hasOnlyPrimaryKeys) { - const subPaths = primaryColumns.map( - column => `${path}.${column.propertyPath}` - ); - paths.push(...subPaths); - continue; - } } // If nothing else, just return every property that's being passed to us. diff --git a/test/functional/query-builder/select/query-builder-select.ts b/test/functional/query-builder/select/query-builder-select.ts index 92e737b752..fb10fcab95 100644 --- a/test/functional/query-builder/select/query-builder-select.ts +++ b/test/functional/query-builder/select/query-builder-select.ts @@ -203,17 +203,23 @@ describe("query builder > select", () => { it("should craft query with FindOperator", () => Promise.all(connections.map(async connection => { // For github issue #6647 - expect(() => { - connection.createQueryBuilder(Category, "category") - .select("category.id") - .leftJoin("category.posts", "posts") - .where({ - posts: { - id: IsNull() - } - }) - .getQueryAndParameters(); - }).to.throw(); + const [sql, params] = connection.createQueryBuilder(Category, "category") + .select("category.id") + .leftJoin("category.posts", "posts") + .where({ + posts: { + id: IsNull() + } + }) + .getQueryAndParameters(); + + expect(sql).to.equal( + 'SELECT "category"."id" AS "category_id" FROM "category" "category" ' + + 'LEFT JOIN "post" "posts" ON "posts"."categoryId"="category"."id" ' + + 'WHERE "posts"."id" IS NULL' + ); + + expect(params).to.eql([]); }))); }); @@ -234,17 +240,24 @@ describe("query builder > select", () => { }))); it("should craft query with FindOperator", () => Promise.all(connections.map(async connection => { - expect(() => { - connection.createQueryBuilder(Post, "post") - .select("post.id") - .leftJoin("post.tags", "tags_join") - .where({ - "tags": { - "name": IsNull() - } - }) - .getQueryAndParameters(); - }).to.throw(); + const [sql, params] = connection.createQueryBuilder(Post, "post") + .select("post.id") + .leftJoin("post.tags", "tags_join") + .where({ + "tags": { + "name": IsNull() + } + }) + .getQueryAndParameters(); + + expect(sql).to.equal( + 'SELECT "post"."id" AS "post_id" FROM "post" "post" ' + + 'LEFT JOIN "post_tags_tag" "post_tags_join" ON "post_tags_join"."postId"="post"."id" ' + + 'LEFT JOIN "tag" "tags_join" ON "tags_join"."id"="post_tags_join"."tagId" ' + + 'WHERE "tags_join"."name" IS NULL' + ); + + expect(params).to.eql([]); }))); });