Skip to content

Commit

Permalink
fix: eager relation respects children relations (#5685)
Browse files Browse the repository at this point in the history
  • Loading branch information
satanTime committed Jul 11, 2021
1 parent eb680f9 commit e7e887a
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 12 deletions.
13 changes: 8 additions & 5 deletions src/entity-manager/EntityManager.ts
Expand Up @@ -674,11 +674,12 @@ export class EntityManager {
async find<Entity>(entityClass: EntityTarget<Entity>, optionsOrConditions?: FindManyOptions<Entity>|FindConditions<Entity>): Promise<Entity[]> {
const metadata = this.connection.getMetadata(entityClass);
const qb = this.createQueryBuilder<Entity>(entityClass as any, FindOptionsUtils.extractFindManyOptionsAlias(optionsOrConditions) || metadata.name);
FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, optionsOrConditions);

if (!FindOptionsUtils.isFindManyOptions(optionsOrConditions) || optionsOrConditions.loadEagerRelations !== false)
FindOptionsUtils.joinEagerRelations(qb, qb.alias, metadata);

return FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, optionsOrConditions).getMany();
return qb.getMany();
}

/**
Expand All @@ -703,11 +704,12 @@ export class EntityManager {
async findAndCount<Entity>(entityClass: EntityTarget<Entity>, optionsOrConditions?: FindConditions<Entity>|FindManyOptions<Entity>): Promise<[Entity[], number]> {
const metadata = this.connection.getMetadata(entityClass);
const qb = this.createQueryBuilder<Entity>(entityClass as any, FindOptionsUtils.extractFindManyOptionsAlias(optionsOrConditions) || metadata.name);
FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, optionsOrConditions);

if (!FindOptionsUtils.isFindManyOptions(optionsOrConditions) || optionsOrConditions.loadEagerRelations !== false)
FindOptionsUtils.joinEagerRelations(qb, qb.alias, metadata);

return FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, optionsOrConditions).getManyAndCount();
return qb.getManyAndCount();
}

/**
Expand Down Expand Up @@ -782,9 +784,6 @@ export class EntityManager {
}
const qb = this.createQueryBuilder<Entity>(entityClass as any, alias);

if (!findOptions || findOptions.loadEagerRelations !== false)
FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);

const passedId = typeof idOrOptionsOrConditions === "string" || typeof idOrOptionsOrConditions === "number" || (idOrOptionsOrConditions as any) instanceof Date;

if (!passedId) {
Expand All @@ -796,6 +795,10 @@ export class EntityManager {

FindOptionsUtils.applyOptionsToQueryBuilder(qb, findOptions);

if (!findOptions || findOptions.loadEagerRelations !== false) {
FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);
}

if (options) {
qb.where(options);

Expand Down
48 changes: 41 additions & 7 deletions src/find-options/FindOptionsUtils.ts
Expand Up @@ -245,18 +245,18 @@ export class FindOptionsUtils {
const selection = alias + "." + relation;
qb.leftJoinAndSelect(selection, relationAlias);

// join the eager relations of the found relation
const relMetadata = metadata.relations.find(metadata => metadata.propertyName === relation);
if (relMetadata) {
this.joinEagerRelations(qb, relationAlias, relMetadata.inverseEntityMetadata);
}

// remove added relations from the allRelations array, this is needed to find all not found relations at the end
allRelations.splice(allRelations.indexOf(prefix ? prefix + "." + relation : relation), 1);

// try to find sub-relations
const join = qb.expressionMap.joinAttributes.find(join => join.entityOrProperty === selection);
this.applyRelationsRecursively(qb, allRelations, join!.alias.name, join!.metadata!, prefix ? prefix + "." + relation : relation);

// join the eager relations of the found relation
const relMetadata = metadata.relations.find(metadata => metadata.propertyName === relation);
if (relMetadata) {
this.joinEagerRelations(qb, relationAlias, relMetadata.inverseEntityMetadata);
}
});
}

Expand All @@ -267,7 +267,41 @@ export class FindOptionsUtils {
let relationAlias = DriverUtils.buildAlias(qb.connection.driver, { shorten: true }, qb.connection.namingStrategy.eagerJoinRelationAlias(alias, relation.propertyPath));

// add a join for the relation
qb.leftJoinAndSelect(alias + "." + relation.propertyPath, relationAlias);
// Checking whether the relation wasn't joined yet.
let addJoin = true;
for (const join of qb.expressionMap.joinAttributes) {
if (
join.condition !== "" ||
join.mapToProperty !== undefined ||
join.isMappingMany !== undefined ||
join.direction !== "LEFT" ||
join.entityOrProperty !== `${alias}.${relation.propertyPath}`
) {
continue;
}
addJoin = false;
relationAlias = join.alias.name;
break;
}

if (addJoin) {
qb.leftJoin(alias + "." + relation.propertyPath, relationAlias);
}

// Checking whether the relation wasn't selected yet.
// This check shall be after the join check to detect relationAlias.
let addSelect = true;
for (const select of qb.expressionMap.selects) {
if (select.aliasName !== undefined || select.virtual !== undefined || select.selection !== relationAlias) {
continue;
}
addSelect = false;
break;
}

if (addSelect) {
qb.addSelect(relationAlias);
}

// (recursive) join the eager relations
this.joinEagerRelations(qb, relationAlias, relation.inverseEntityMetadata);
Expand Down
24 changes: 24 additions & 0 deletions test/github-issues/5684/entity/Company.ts
@@ -0,0 +1,24 @@
import {Column, Entity, JoinColumn, OneToMany, OneToOne, PrimaryGeneratedColumn} from "../../../../src";
import {User} from "./User";

@Entity()
export class Company {

@PrimaryGeneratedColumn()
id: number;

@Column()
name: string;

@OneToOne(
() => User,
)
@JoinColumn()
admin: User;

@OneToMany(
() => User,
user => user.company,
)
public staff?: Array<User>;
}
22 changes: 22 additions & 0 deletions test/github-issues/5684/entity/User.ts
@@ -0,0 +1,22 @@
import {Column, Entity, JoinColumn, ManyToOne, PrimaryGeneratedColumn} from "../../../../src";
import {Company} from "./Company";

@Entity()
export class User {

@PrimaryGeneratedColumn()
id: number;

@Column()
name: string;

@ManyToOne(
() => Company,
company => company.staff,
{
eager: true, // <- this cases the bug.
},
)
@JoinColumn()
company: Company;
}
85 changes: 85 additions & 0 deletions test/github-issues/5684/issue-5684.ts
@@ -0,0 +1,85 @@
import {expect} from "chai";
import "reflect-metadata";
import {Connection} from "../../../src";
import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
import {Company} from "./entity/Company";
import {User} from "./entity/User";

describe("github issues > #5684 eager relation skips children relations", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [User, Company],
schemaCreate: true,
dropSchema: true,
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should select children of an eager relation", () => Promise.all(connections.map(async connection => {
const company = new Company();
company.name = "company";
await connection.getRepository(Company).save(company);

const userAdmin = new User();
userAdmin.name = "admin";
userAdmin.company = company;
await connection.getRepository(User).save(userAdmin);

const userNormal = new User();
userNormal.name = "normal";
userNormal.company = company;
await connection.getRepository(User).save(userNormal);

company.admin = userAdmin;
await connection.getRepository(Company).save(company);

const assert = (user?: User): void => {
expect(user && user.company && user.company.admin)
.to.be.a.instanceOf(User, "loads nested relation of an eager relation");
expect(user && user.company && user.company.staff)
.to.have.length(2, "loads nested relation of an eager relation");
for (const member of user && user.company.staff || []) {
expect(member).to.be.a.instanceOf(User, "loads nested relation of an eager relation");
expect(member.company).to.be.a.instanceOf(Company, "loads nested relation of an eager relation");
expect(member.company.admin).to.be.a.instanceOf(User, "loads nested relation of an eager relation");
expect(member.company.admin.company).to.be.a.instanceOf(Company, "still loads an eager relation");
}
};

const relations = [
"company",
"company.admin", // <-- can't be loaded without the fix.
"company.staff", // <-- can't be loaded without the fix.
"company.staff.company", // <-- can't be loaded without the fix.
"company.staff.company.admin", // <-- can't be loaded without the fix.
];

const user1 = await connection.getRepository(User).findOne(userAdmin.id, {
relations: [...relations],
});
assert(user1);
const user2 = await connection.getRepository(User).findOneOrFail(userAdmin.id, {
relations: [...relations],
});
assert(user2);
const users3 = await connection.getRepository(User).find({
where: {
id: userAdmin.id,
},
relations: [...relations],
});
assert(users3.pop());
const [users4] = await connection.getRepository(User).findAndCount({
where: {
id: userAdmin.id,
},
relations: [...relations],
});
assert(users4.pop());
const users5 = await connection.getRepository(User).findByIds([userAdmin.id], {
relations: [...relations],
});
assert(users5.pop());
})));
});

0 comments on commit e7e887a

Please sign in to comment.