Skip to content

Commit

Permalink
fix(core): fix M:N relations with custom type PKs
Browse files Browse the repository at this point in the history
Closes #1930
  • Loading branch information
B4nan committed Jun 19, 2021
1 parent d6d56f5 commit 3cdc786
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 11 deletions.
25 changes: 25 additions & 0 deletions packages/core/src/entity/WrappedEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,31 @@ export class WrappedEntity<T extends AnyEntity<T>, PK extends keyof T> {
return this.pkGetter(this.entity);
}

getPrimaryKeys(convertCustomTypes = false): Primary<T>[] | null {
const pk = this.getPrimaryKey(convertCustomTypes);

if (!pk) {
return null;
}

if (this.__meta.compositePK) {
return this.__meta.primaryKeys.reduce((ret, pk) => {
const child = this.entity[pk] as AnyEntity<T> | Primary<unknown>;

if (Utils.isEntity(child, true)) {
const childPk = child.__helper!.getPrimaryKeys(convertCustomTypes);
ret.push(...childPk!);
} else {
ret.push(child as Primary<unknown>);
}

return ret;
}, [] as Primary<T>[]);
}

return [pk];
}

setPrimaryKey(id: Primary<T> | null) {
this.entity[this.entity.__meta!.primaryKeys[0] as string] = id;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ export interface IWrappedEntity<T extends AnyEntity<T>, PK extends keyof T | unk

export interface IWrappedEntityInternal<T, PK extends keyof T | unknown = PrimaryProperty<T>, P = keyof T> extends IWrappedEntity<T, PK, P> {
hasPrimaryKey(): boolean;
getPrimaryKey(convertCustomTypes?: boolean): Primary<T>;
getPrimaryKey(convertCustomTypes?: boolean): Primary<T> | null;
getPrimaryKeys(convertCustomTypes?: boolean): Primary<T>[] | null;
setPrimaryKey(val: Primary<T>): void;
getSerializedPrimaryKey(): string & keyof T;
__meta: EntityMetadata<T>;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/unit-of-work/ChangeSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { EntityData, AnyEntity, EntityMetadata, EntityDictionary, Primary } from

export class ChangeSet<T extends AnyEntity<T>> {

private primaryKey?: Primary<T> | Primary<T>[];
private primaryKey?: Primary<T> | Primary<T>[] | null;

constructor(public entity: T,
public type: ChangeSetType,
Expand All @@ -13,7 +13,7 @@ export class ChangeSet<T extends AnyEntity<T>> {
this.collection = meta.root.collection;
}

getPrimaryKey(): Primary<T> | Primary<T>[] {
getPrimaryKey(): Primary<T> | Primary<T>[] | null {
this.primaryKey = this.primaryKey ?? this.entity.__helper!.getPrimaryKey(true);
return this.primaryKey;
}
Expand Down
25 changes: 18 additions & 7 deletions packages/knex/src/AbstractSqlDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,11 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
async syncCollection<T extends AnyEntity<T>, O extends AnyEntity<O>>(coll: Collection<T, O>, ctx?: Transaction): Promise<void> {
const wrapped = coll.owner.__helper!;
const meta = wrapped.__meta;
const pks = wrapped.__primaryKeys;
const pks = wrapped.getPrimaryKeys(true);
const snap = coll.getSnapshot();
const includes = <T>(arr: T[], item: T) => !!arr.find(i => Utils.equals(i, item));
const snapshot = snap ? snap.map(item => item.__helper!.__primaryKeys) : [];
const current = coll.getItems(false).map(item => item.__helper!.__primaryKeys);
const snapshot = snap ? snap.map(item => item.__helper!.getPrimaryKeys(true)) : [];
const current = coll.getItems(false).map(item => item.__helper!.getPrimaryKeys(true));
const deleteDiff = snap ? snapshot.filter(item => !includes(current, item)) : true;
const insertDiff = current.filter(item => !includes(snapshot, item));
const target = snapshot.filter(item => includes(current, item)).concat(...insertDiff);
Expand All @@ -414,7 +414,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
return this.rethrow(this.execute<any>(qb));
}

return this.rethrow(this.updateCollectionDiff<T, O>(meta, coll.property, pks, deleteDiff, insertDiff, ctx));
return this.rethrow(this.updateCollectionDiff<T, O>(meta, coll.property, pks as any, deleteDiff as any, insertDiff as any, ctx));
}

async loadFromPivotTable<T extends AnyEntity<T>, O extends AnyEntity<O>>(prop: EntityProperty, owners: Primary<O>[][], where: FilterQuery<T> = {}, orderBy?: QueryOrderMap, ctx?: Transaction, options?: FindOptions<T>): Promise<Dictionary<T[]>> {
Expand Down Expand Up @@ -443,9 +443,20 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
const items = owners.length ? await this.rethrow(qb.execute('all')) : [];

const map: Dictionary<T[]> = {};
owners.forEach(owner => map['' + Utils.getPrimaryKeyHash(owner as string[])] = []);
const pkProps = ownerMeta.getPrimaryProps();
owners.forEach(owner => {
const key = Utils.getPrimaryKeyHash(prop.joinColumns.map((col, idx) => {
const pkProp = pkProps[idx];
return pkProp.customType ? pkProp.customType.convertToJSValue(owner[idx], this.platform) : owner[idx];
}));

return map[key] = [];
});
items.forEach((item: any) => {
const key = Utils.getPrimaryKeyHash(prop.joinColumns.map(col => item[`fk__${col}`]));
const key = Utils.getPrimaryKeyHash(prop.joinColumns.map((col, idx) => {
const pkProp = pkProps[idx];
return pkProp.customType ? pkProp.customType.convertToJSValue(item[`fk__${col}`], this.platform) : item[`fk__${col}`];
}));
map[key].push(item);
prop.joinColumns.forEach(col => delete item[`fk__${col}`]);
prop.inverseJoinColumns.forEach(col => delete item[`fk__${col}`]);
Expand Down Expand Up @@ -632,7 +643,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra

/* istanbul ignore else */
if (this.platform.allowsMultiInsert()) {
await this.nativeInsertMany<T>(prop.pivotTable, items as EntityData<T>[], ctx);
await this.nativeInsertMany<T>(prop.pivotTable, items as EntityData<T>[], ctx, false, false);
} else {
await Utils.runSerial(items, item => this.createQueryBuilder(prop.pivotTable, ctx, true).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES).insert(item).execute('run', false));
}
Expand Down
4 changes: 3 additions & 1 deletion tests/features/composite-keys/GH1079.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Entity, PrimaryKey, MikroORM, ManyToOne, Enum, PrimaryKeyType, Property, BigIntType, Logger } from '@mikro-orm/core';
import { Entity, PrimaryKey, MikroORM, ManyToOne, Enum, PrimaryKeyType, Property, BigIntType, Logger, wrap } from '@mikro-orm/core';
import { PostgreSqlDriver } from '@mikro-orm/postgresql';
import { v4 } from 'uuid';

Expand Down Expand Up @@ -90,6 +90,7 @@ describe('GH issue 1079', () => {
const user = new User();
const wallet = new Wallet();
const deposit = new Deposit();
expect(wrap(deposit, true).getPrimaryKeys()).toBeNull();
user._id = '1';
wallet.currencyRef = 'USD';
wallet.owner = user;
Expand All @@ -98,6 +99,7 @@ describe('GH issue 1079', () => {
deposit.amount = '123';
deposit.txRef = '456';
deposit.gatewayKey = '789';
expect(wrap(deposit, true).getPrimaryKeys()).toEqual(['456', 'USD', '1']);

const mock = jest.fn();
const logger = new Logger(mock, ['query']);
Expand Down
96 changes: 96 additions & 0 deletions tests/features/custom-types/GH1930.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { v4, parse, stringify } from 'uuid';
import { Collection, Entity, ManyToMany, MikroORM, PrimaryKey, Property, Type } from '@mikro-orm/core';
import { MySqlDriver, SchemaGenerator } from '@mikro-orm/mysql';

export class UuidBinaryType extends Type<string, Buffer> {

convertToDatabaseValue(value: string): Buffer {
return Buffer.from(parse(value) as number[]);
}

convertToJSValue(value: Buffer): string {
return stringify(value);
}

getColumnType(): string {
return 'binary(16)';
}

}

@Entity()
class B {

@PrimaryKey({ type: UuidBinaryType, name: 'uuid' })
id: string = v4();

@Property()
name: string;

constructor(name: string) {
this.name = name;
}

}

@Entity()
class A {

@PrimaryKey({ type: UuidBinaryType, name: 'uuid' })
id: string = v4();

@Property()
name: string;

@ManyToMany(() => B)
fields = new Collection<B>(this);

constructor(name: string) {
this.name = name;
}

}

describe('GH issue 1930', () => {

let orm: MikroORM<MySqlDriver>;

beforeAll(async () => {
orm = await MikroORM.init({
entities: [A, B],
dbName: `mikro_orm_test_gh_1930`,
type: 'mysql',
port: 3307,
});
await new SchemaGenerator(orm.em).ensureDatabase();
await new SchemaGenerator(orm.em).dropSchema();
await new SchemaGenerator(orm.em).createSchema();
});

afterAll(async () => {
await orm.close(true);
});

afterEach(async () => {
await orm.em.nativeDelete(B, {});
await orm.em.nativeDelete(A, {});
});

test(`M:N with custom type PKs`, async () => {
const a = new A('a1');
a.fields.add(new B('b1'), new B('b2'), new B('b3'));
await orm.em.persistAndFlush(a);
orm.em.clear();

const a1 = await orm.em.findOneOrFail(A, a.id, {
populate: ['fields'],
orderBy: { fields: { name: 'asc' } },
});
expect(a1.id).toBe(a.id);
expect(a1.fields.length).toBe(3);
expect(a1.fields[0].id).toBe(a.fields[0].id);
expect(a1.fields[1].id).toBe(a.fields[1].id);
expect(a1.fields[2].id).toBe(a.fields[2].id);
});

});

0 comments on commit 3cdc786

Please sign in to comment.