Skip to content

Commit

Permalink
Simplify check and update tests
Browse files Browse the repository at this point in the history
  • Loading branch information
TJ Higgins committed Aug 2, 2021
1 parent b56813c commit 6de9327
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 86 deletions.
85 changes: 21 additions & 64 deletions src/query-builder/QueryBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,21 +994,20 @@ export abstract class QueryBuilder<Entity> {
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)
);
Expand All @@ -1017,40 +1016,19 @@ export abstract class QueryBuilder<Entity> {
paths.push(path);
continue;
}
} else {
const seenJoinColumns = new Set<string>();
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)
);
Expand All @@ -1062,27 +1040,6 @@ export abstract class QueryBuilder<Entity> {
paths.push(...subPaths);
continue;
}
} else {
const seenPrimaryColumns = new Set<string>();
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.
Expand Down
57 changes: 35 additions & 22 deletions test/functional/query-builder/select/query-builder-select.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
})));
});

Expand All @@ -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([]);
})));
});

Expand Down

0 comments on commit 6de9327

Please sign in to comment.