Skip to content

Commit

Permalink
refactor: rework fix for 5586
Browse files Browse the repository at this point in the history
Closes 5586
  • Loading branch information
B4nan committed May 20, 2024
1 parent a87568f commit a132036
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 25 deletions.
1 change: 1 addition & 0 deletions packages/core/src/EntityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ export class EntityManager<Driver extends IDatabaseDriver = IDatabaseDriver> {
return where;
}

// this method only handles the problem for mongo driver, SQL drivers have their implementation inside QueryBuilder
protected applyDiscriminatorCondition<Entity extends object>(entityName: string, where: FilterQuery<Entity>): FilterQuery<Entity> {
const meta = this.metadata.find<Entity>(entityName);

Expand Down
33 changes: 15 additions & 18 deletions packages/core/src/metadata/MetadataDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ import {
ArrayType,
BigIntType,
BlobType,
DoubleType,
DecimalType,
DoubleType,
EnumArrayType,
IntervalType,
JsonType,
t,
Type,
Uint8ArrayType,
UnknownType, IntervalType,
UnknownType,
} from '../types';
import { colors } from '../logging/colors';
import { raw, RawQueryFragment } from '../utils/RawQueryFragment';
Expand Down Expand Up @@ -494,7 +495,6 @@ export class MetadataDiscovery {
}

private initManyToManyFields(meta: EntityMetadata, prop: EntityProperty): void {
meta = meta.root;
const meta2 = this.metadata.get(prop.type);
Utils.defaultValue(prop, 'fixedOrder', !!prop.fixedOrderColumn);
const pivotMeta = this.metadata.find(prop.pivotEntity);
Expand All @@ -515,8 +515,8 @@ export class MetadataDiscovery {
if (pivotMeta && (pks.length === 2 || fks.length >= 2)) {
const owner = prop.mappedBy ? meta2.properties[prop.mappedBy] : prop;
const [first, second] = this.ensureCorrectFKOrderInPivotEntity(pivotMeta, owner);
prop.joinColumns = first!.fieldNames;
prop.inverseJoinColumns = second!.fieldNames;
prop.joinColumns ??= first!.fieldNames;
prop.inverseJoinColumns ??= second!.fieldNames;
}

if (!prop.pivotTable && prop.owner && this.platform.usesPivotTable()) {
Expand All @@ -534,18 +534,9 @@ export class MetadataDiscovery {
prop.inverseJoinColumns = prop2.joinColumns;
}

if (!prop.referencedColumnNames) {
prop.referencedColumnNames = Utils.flatten(meta.primaryKeys.map(primaryKey => meta.properties[primaryKey].fieldNames));
}

if (!prop.joinColumns) {
prop.joinColumns = prop.referencedColumnNames.map(referencedColumnName => this.namingStrategy.joinKeyColumnName(meta.className, referencedColumnName, meta.compositePK));
}

if (!prop.inverseJoinColumns) {
const meta2 = this.metadata.get(prop.type);
prop.inverseJoinColumns = this.initManyToOneFieldName(prop, meta2.className);
}
prop.referencedColumnNames ??= Utils.flatten(meta.primaryKeys.map(primaryKey => meta.properties[primaryKey].fieldNames));
prop.joinColumns ??= prop.referencedColumnNames.map(referencedColumnName => this.namingStrategy.joinKeyColumnName(meta.root.className, referencedColumnName, meta.compositePK));
prop.inverseJoinColumns ??= this.initManyToOneFieldName(prop, meta2.root.className);
}

private initManyToOneFields(prop: EntityProperty): void {
Expand Down Expand Up @@ -670,7 +661,6 @@ export class MetadataDiscovery {
}

private definePivotTableEntity(meta: EntityMetadata, prop: EntityProperty): EntityMetadata {
meta = meta.root;
const pivotMeta = this.metadata.find(prop.pivotEntity);

// ensure inverse side exists so we can join it when populating via pivot tables
Expand Down Expand Up @@ -698,6 +688,13 @@ export class MetadataDiscovery {
return pivotMeta;
}

const exists = this.metadata.find(prop.pivotTable);

if (exists) {
prop.pivotEntity = exists.className;
return exists;
}

let tableName = prop.pivotTable;
let schemaName: string | undefined;

Expand Down
2 changes: 1 addition & 1 deletion tests/features/single-table-inheritance/GH4351.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,6 @@ test('it should add discriminator to the query', async () => {

const qb3 = orm.em.qb(User, 'u').join('u.posts2', 'p');
const res3 = await qb3;
expect(qb3.getFormattedQuery()).toMatch("select `u`.* from `user` as `u` inner join `user_posts2` as `u1` on `u`.`id` = `u1`.`user_id` inner join `base_entity` as `p` on `u1`.`post_id` = `p`.`id` and `p`.`type` = 'post'");
expect(qb3.getFormattedQuery()).toMatch("select `u`.* from `user` as `u` inner join `user_posts2` as `u1` on `u`.`id` = `u1`.`user_id` inner join `base_entity` as `p` on `u1`.`base_entity_id` = `p`.`id` and `p`.`type` = 'post'");
expect(res3).toHaveLength(1);
});
4 changes: 2 additions & 2 deletions tests/features/single-table-inheritance/GH4422.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ describe('GH issue 4422', () => {
},
});
expect(mock.mock.calls[0][0]).toMatch(
"select `c0`.* from `company` as `c0` left join `tag_companies` as `t2` on `c0`.`id` = `t2`.`company_id` left join `tag` as `t1` on `t2`.`tag1_id` = `t1`.`id` and `t1`.`type` = 'tag1' where `t1`.`name` = 'tag1'",
"select `c0`.* from `company` as `c0` left join `tag_companies` as `t2` on `c0`.`id` = `t2`.`company_id` left join `tag` as `t1` on `t2`.`tag_id` = `t1`.`id` and `t1`.`type` = 'tag1' where `t1`.`name` = 'tag1'",
);
});

Expand All @@ -167,7 +167,7 @@ describe('GH issue 4422', () => {
},
});
expect(mock.mock.calls[0][0]).toMatch(
"select `c0`.* from `company` as `c0` left join `company_tags2` as `c2` on `c0`.`id` = `c2`.`company_id` left join `tag` as `c1` on `c2`.`tag2_id` = `c1`.`id` and `c1`.`type` = 'tag2' where `c1`.`name` = 'tag2'",
"select `c0`.* from `company` as `c0` left join `company_tags2` as `c2` on `c0`.`id` = `c2`.`company_id` left join `tag` as `c1` on `c2`.`tag_id` = `c1`.`id` and `c1`.`type` = 'tag2' where `c1`.`name` = 'tag2'",
);
});

Expand Down
4 changes: 2 additions & 2 deletions tests/features/single-table-inheritance/GH4423.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('GH issue 4423', () => {
await orm.em.findAll(Task, {
populate: ['managers'],
});
expect(mock.mock.calls[0][0]).toMatch("select `t0`.*, `m1`.`id` as `m1__id`, `m1`.`name` as `m1__name`, `m1`.`type` as `m1__type`, `m1`.`favorite_task_id` as `m1__favorite_task_id` from `task` as `t0` left join `task_managers` as `t2` on `t0`.`id` = `t2`.`task_id` left join `user` as `m1` on `t2`.`manager_id` = `m1`.`id` and `m1`.`type` = 'manager'");
expect(mock.mock.calls[0][0]).toMatch("select `t0`.*, `m1`.`id` as `m1__id`, `m1`.`name` as `m1__name`, `m1`.`type` as `m1__type`, `m1`.`favorite_task_id` as `m1__favorite_task_id` from `task` as `t0` left join `task_managers` as `t2` on `t0`.`id` = `t2`.`task_id` left join `user` as `m1` on `t2`.`user_id` = `m1`.`id` and `m1`.`type` = 'manager'");
});

test('The owning side is in the relation, This one works normally', async () => {
Expand All @@ -95,6 +95,6 @@ describe('GH issue 4423', () => {
populate: ['tasks'],
});

expect(mock.mock.calls[0][0]).toMatch("select `m0`.*, `t1`.`id` as `t1__id`, `t1`.`name` as `t1__name` from `user` as `m0` left join `task_managers` as `t2` on `m0`.`id` = `t2`.`manager_id` left join `task` as `t1` on `t2`.`task_id` = `t1`.`id` where `m0`.`type` = 'manager'");
expect(mock.mock.calls[0][0]).toMatch("select `m0`.*, `t1`.`id` as `t1__id`, `t1`.`name` as `t1__name` from `user` as `m0` left join `task_managers` as `t2` on `m0`.`id` = `t2`.`user_id` left join `task` as `t1` on `t2`.`task_id` = `t1`.`id` where `m0`.`type` = 'manager'");
});
});
2 changes: 0 additions & 2 deletions tests/features/single-table-inheritance/GH5586.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ class AdminUser extends User {}
let orm: MikroORM;

beforeAll(async () => {
// It should create a pivot table using privilege_id and user_id columns.
// Instead, it creates a pivot table using privilege_id and admin_user_id columns.
orm = await MikroORM.init({
dbName: ':memory:',
entities: [User, Privilege, PrivilegeGroup, SuperUser, AdminUser],
Expand Down

0 comments on commit a132036

Please sign in to comment.