Skip to content

Commit

Permalink
fix(sql): convert custom types at query builder level
Browse files Browse the repository at this point in the history
  • Loading branch information
B4nan committed Nov 9, 2020
1 parent bf8776c commit 83d3ab2
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 41 deletions.
8 changes: 4 additions & 4 deletions packages/core/src/drivers/DatabaseDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD

abstract async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions<T>, ctx?: Transaction): Promise<EntityData<T> | null>;

abstract async nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction): Promise<QueryResult>;
abstract async nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction, convertCustomTypes?: boolean): Promise<QueryResult>;

abstract async nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean): Promise<QueryResult>;
abstract async nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean, convertCustomTypes?: boolean): Promise<QueryResult>;

abstract async nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction): Promise<QueryResult>;
abstract async nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction, convertCustomTypes?: boolean): Promise<QueryResult>;

async nativeUpdateMany<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>[], data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean): Promise<QueryResult> {
async nativeUpdateMany<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>[], data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean, convertCustomTypes?: boolean): Promise<QueryResult> {
throw new Error(`Batch updates are not supported by ${this.constructor.name} driver`);
}

Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/drivers/IDatabaseDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ export interface IDatabaseDriver<C extends Connection = Connection> {
*/
findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions<T>, ctx?: Transaction): Promise<EntityData<T> | null>;

nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction): Promise<QueryResult>;
nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction, convertCustomTypes?: boolean): Promise<QueryResult>;

nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean): Promise<QueryResult>;
nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean, convertCustomTypes?: boolean): Promise<QueryResult>;

nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction): Promise<QueryResult>;
nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction, convertCustomTypes?: boolean): Promise<QueryResult>;

nativeUpdateMany<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>[], data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean): Promise<QueryResult>;
nativeUpdateMany<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>[], data: EntityData<T>[], ctx?: Transaction, processCollections?: boolean, convertCustomTypes?: boolean): Promise<QueryResult>;

nativeDelete<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, ctx?: Transaction): Promise<QueryResult>;

Expand Down
16 changes: 8 additions & 8 deletions packages/core/src/unit-of-work/ChangeSetPersister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class ChangeSetPersister {

private async persistNewEntity<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSet: ChangeSet<T>, ctx?: Transaction): Promise<void> {
const wrapped = changeSet.entity.__helper!;
const res = await this.driver.nativeInsert(changeSet.name, changeSet.payload, ctx);
const res = await this.driver.nativeInsert(changeSet.name, changeSet.payload, ctx, false);

if (!wrapped.hasPrimaryKey()) {
this.mapPrimaryKey(meta, res.insertId, changeSet);
Expand Down Expand Up @@ -93,7 +93,7 @@ export class ChangeSetPersister {
}

private async persistNewEntitiesBatch<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSets: ChangeSet<T>[], ctx?: Transaction): Promise<void> {
const res = await this.driver.nativeInsertMany(meta.className, changeSets.map(cs => cs.payload), ctx, false);
const res = await this.driver.nativeInsertMany(meta.className, changeSets.map(cs => cs.payload), ctx, false, false);

for (let i = 0; i < changeSets.length; i++) {
const changeSet = changeSets[i];
Expand Down Expand Up @@ -131,13 +131,13 @@ export class ChangeSetPersister {

private async persistManagedEntitiesBatch<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSets: ChangeSet<T>[], ctx?: Transaction): Promise<void> {
await this.checkOptimisticLocks(meta, changeSets, ctx);
await this.driver.nativeUpdateMany(meta.className, changeSets.map(cs => cs.entity.__helper!.getPrimaryKey() as Dictionary), changeSets.map(cs => cs.payload), ctx, false);
await this.driver.nativeUpdateMany(meta.className, changeSets.map(cs => cs.entity.__helper!.getPrimaryKey() as Dictionary), changeSets.map(cs => cs.payload), ctx, false, false);
changeSets.forEach(cs => cs.persisted = true);
}

private mapPrimaryKey<T extends AnyEntity<T>>(meta: EntityMetadata<T>, value: IPrimaryKey, changeSet: ChangeSet<T>): void {
const prop = meta.properties[meta.primaryKeys[0]];
const insertId = prop.customType ? prop.customType.convertToJSValue(value, this.driver.getPlatform()) : value;
const insertId = prop.customType ? prop.customType.convertToJSValue(value, this.platform) : value;
const wrapped = changeSet.entity.__helper!;

if (!wrapped.hasPrimaryKey()) {
Expand All @@ -147,7 +147,7 @@ export class ChangeSetPersister {
// some drivers might be returning bigint PKs as numbers when the number is small enough,
// but we need to have it as string so comparison works in change set tracking, so instead
// of using the raw value from db, we convert it back to the db value explicitly
value = prop.customType ? prop.customType.convertToDatabaseValue(insertId, this.driver.getPlatform()) : value;
value = prop.customType ? prop.customType.convertToDatabaseValue(insertId, this.platform) : value;
changeSet.payload[wrapped.__meta.primaryKeys[0]] = value;
wrapped.__identifier!.setValue(value);
}
Expand All @@ -174,15 +174,15 @@ export class ChangeSetPersister {

private async updateEntity<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSet: ChangeSet<T>, ctx?: Transaction): Promise<QueryResult> {
if (!meta.versionProperty || !changeSet.entity[meta.versionProperty]) {
return this.driver.nativeUpdate(changeSet.name, changeSet.entity.__helper!.getPrimaryKey() as Dictionary, changeSet.payload, ctx);
return this.driver.nativeUpdate(changeSet.name, changeSet.entity.__helper!.getPrimaryKey() as Dictionary, changeSet.payload, ctx, false);
}

const cond = {
...Utils.getPrimaryKeyCond<T>(changeSet.entity, meta.primaryKeys),
[meta.versionProperty]: this.platform.quoteVersionValue(changeSet.entity[meta.versionProperty] as unknown as Date, meta.properties[meta.versionProperty]),
} as FilterQuery<T>;

return this.driver.nativeUpdate(changeSet.name, cond, changeSet.payload, ctx);
return this.driver.nativeUpdate(changeSet.name, cond, changeSet.payload, ctx, false);
}

private async checkOptimisticLocks<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSets: ChangeSet<T>[], ctx?: Transaction): Promise<void> {
Expand Down Expand Up @@ -257,7 +257,7 @@ export class ChangeSetPersister {
}

if (changeSet.payload[prop.name] as unknown instanceof Date) {
changeSet.payload[prop.name] = this.driver.getPlatform().processDateProperty(changeSet.payload[prop.name]);
changeSet.payload[prop.name] = this.platform.processDateProperty(changeSet.payload[prop.name]);
}
}

Expand Down
39 changes: 21 additions & 18 deletions packages/knex/src/AbstractSqlDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
const meta = this.metadata.find<T>(entityName)!;
const populate = this.autoJoinOneToOneOwner(meta, options.populate as PopulateOptions<T>[], options.fields);
const joinedProps = this.joinedProps(meta, populate);
const qb = this.createQueryBuilder<T>(entityName, ctx, !!ctx).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES);
const qb = this.createQueryBuilder<T>(entityName, ctx, !!ctx, false);
const fields = this.buildFields(meta, populate, joinedProps, qb, options.fields);

if (Utils.isPrimaryKey(where, meta.compositePK)) {
Expand Down Expand Up @@ -166,8 +166,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra

async count<T extends AnyEntity<T>>(entityName: string, where: any, options: CountOptions<T> = {}, ctx?: Transaction<KnexTransaction>): Promise<number> {
const pks = this.metadata.find(entityName)!.primaryKeys;
const qb = this.createQueryBuilder(entityName, ctx, !!ctx)
.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES)
const qb = this.createQueryBuilder(entityName, ctx, !!ctx, false)
.count(pks, true)
.groupBy(options.groupBy!)
.having(options.having!)
Expand All @@ -178,11 +177,11 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
return +res.count;
}

async nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction<KnexTransaction>): Promise<QueryResult> {
async nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction<KnexTransaction>, convertCustomTypes = true): Promise<QueryResult> {
const meta = this.metadata.find<T>(entityName);
const collections = this.extractManyToMany(entityName, data);
const pks = this.getPrimaryKeyFields(entityName);
const qb = this.createQueryBuilder(entityName, ctx, true);
const qb = this.createQueryBuilder(entityName, ctx, true, convertCustomTypes);
const res = await this.rethrow(qb.insert(data).execute('run', false));
res.row = res.row || {};
let pk: any;
Expand All @@ -200,7 +199,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
return res;
}

async nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>[], ctx?: Transaction<KnexTransaction>, processCollections = true): Promise<QueryResult> {
async nativeInsertMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>[], ctx?: Transaction<KnexTransaction>, processCollections = true, convertCustomTypes = true): Promise<QueryResult> {
const meta = this.metadata.get<T>(entityName);
const collections = processCollections ? data.map(d => this.extractManyToMany(entityName, d)) : [];
const pks = this.getPrimaryKeyFields(entityName);
Expand All @@ -211,7 +210,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
let res: QueryResult;

if (fields.length === 0) {
const qb = this.createQueryBuilder(entityName, ctx, true);
const qb = this.createQueryBuilder(entityName, ctx, true, convertCustomTypes);
res = await this.rethrow(qb.insert(data).execute('run', false));
} else {
let sql = `insert into ${this.platform.quoteIdentifier(meta.collection)} `;
Expand Down Expand Up @@ -256,7 +255,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
return res;
}

async nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction<KnexTransaction>): Promise<QueryResult> {
async nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction<KnexTransaction>, convertCustomTypes = true): Promise<QueryResult> {
const meta = this.metadata.find<T>(entityName);
const pks = this.getPrimaryKeyFields(entityName);
const collections = this.extractManyToMany(entityName, data);
Expand All @@ -267,8 +266,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
}

if (Utils.hasObjectKeys(data)) {
const qb = this.createQueryBuilder<T>(entityName, ctx, true)
.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES)
const qb = this.createQueryBuilder<T>(entityName, ctx, true, convertCustomTypes)
.update(data)
.where(where);

Expand All @@ -281,7 +279,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
return res;
}

async nativeUpdateMany<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>[], data: EntityData<T>[], ctx?: Transaction<KnexTransaction>, processCollections = true): Promise<QueryResult> {
async nativeUpdateMany<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>[], data: EntityData<T>[], ctx?: Transaction<KnexTransaction>, processCollections = true, convertCustomTypes = true): Promise<QueryResult> {
const meta = this.metadata.get<T>(entityName);
const collections = processCollections ? data.map(d => this.extractManyToMany(entityName, d)) : [];
const keys = new Set<string>();
Expand Down Expand Up @@ -338,7 +336,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
where = { [pks[0]]: where };
}

const qb = this.createQueryBuilder(entityName, ctx, true).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES).delete(where);
const qb = this.createQueryBuilder(entityName, ctx, true, false).delete(where);

return this.rethrow(qb.execute('run', false));
}
Expand Down Expand Up @@ -366,7 +364,6 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
if (coll.property.reference === ReferenceType.ONE_TO_MANY) {
const cols = coll.property.referencedColumnNames;
const qb = this.createQueryBuilder(coll.property.type, ctx, true)
.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES)
.update({ [coll.property.mappedBy]: pks })
.getKnexQuery()
.whereIn(cols, insertDiff as string[][]);
Expand Down Expand Up @@ -490,8 +487,14 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
return prop.fieldNames;
}

protected createQueryBuilder<T extends AnyEntity<T>>(entityName: string, ctx?: Transaction<KnexTransaction>, write?: boolean): QueryBuilder<T> {
return new QueryBuilder(entityName, this.metadata, this, ctx, undefined, write ? 'write' : 'read');
protected createQueryBuilder<T extends AnyEntity<T>>(entityName: string, ctx?: Transaction<KnexTransaction>, write?: boolean, convertCustomTypes?: boolean): QueryBuilder<T> {
const qb = new QueryBuilder<T>(entityName, this.metadata, this, ctx, undefined, write ? 'write' : 'read');

if (!convertCustomTypes) {
qb.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES);
}

return qb;
}

protected extractManyToMany<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>): EntityData<T> {
Expand Down Expand Up @@ -531,7 +534,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
}

if (deleteDiff === true || deleteDiff.length > 0) {
const qb1 = this.createQueryBuilder(prop.pivotTable, ctx, true);
const qb1 = this.createQueryBuilder(prop.pivotTable, ctx, true).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES);
const knex = qb1.getKnex();

if (Array.isArray(deleteDiff)) {
Expand All @@ -558,12 +561,12 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
if (this.platform.allowsMultiInsert()) {
await this.nativeInsertMany(prop.pivotTable, items, ctx);
} else {
await Utils.runSerial(items, item => this.createQueryBuilder(prop.pivotTable, ctx, true).insert(item).execute('run', false));
await Utils.runSerial(items, item => this.createQueryBuilder(prop.pivotTable, ctx, true).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES).insert(item).execute('run', false));
}
}

async lockPessimistic<T extends AnyEntity<T>>(entity: T, mode: LockMode, ctx?: Transaction): Promise<void> {
const qb = this.createQueryBuilder(entity.constructor.name, ctx);
const qb = this.createQueryBuilder(entity.constructor.name, ctx).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES);
const meta = entity.__helper!.__meta;
const cond = Utils.getPrimaryKeyCond(entity, meta.primaryKeys);
qb.select('1').where(cond!).setLockMode(mode);
Expand Down
22 changes: 19 additions & 3 deletions packages/knex/src/query/QueryBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,23 @@
import { QueryBuilder as KnexQueryBuilder, Raw, Transaction, Value } from 'knex';
import {
AnyEntity, Dictionary, EntityMetadata, EntityProperty, FlatQueryOrderMap, GroupOperator, LockMode, MetadataStorage, EntityData,
PopulateOptions, QBFilterQuery, QueryFlag, QueryHelper, QueryOrderMap, ReferenceType, Utils, ValidationError, LoadStrategy,
AnyEntity,
Dictionary,
EntityData,
EntityMetadata,
EntityProperty,
FlatQueryOrderMap,
GroupOperator,
LoadStrategy,
LockMode,
MetadataStorage,
PopulateOptions,
QBFilterQuery,
QueryFlag,
QueryHelper,
QueryOrderMap,
ReferenceType,
Utils,
ValidationError,
} from '@mikro-orm/core';
import { QueryType } from './enums';
import { AbstractSqlDriver } from '../AbstractSqlDriver';
Expand Down Expand Up @@ -522,7 +538,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
}

if (data) {
this._data = this.helper.processData(data);
this._data = this.helper.processData(data, this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES));
}

if (cond) {
Expand Down
8 changes: 6 additions & 2 deletions packages/knex/src/query/QueryBuilderHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ export class QueryBuilderHelper {
return this.alias + '.' + ret;
}

processData(data: Dictionary, multi = false): any {
processData(data: Dictionary, convertCustomTypes: boolean, multi = false): any {
if (Array.isArray(data)) {
return data.map(d => this.processData(d, true));
return data.map(d => this.processData(d, convertCustomTypes, true));
}

data = Object.assign({}, data); // copy first
Expand All @@ -78,6 +78,10 @@ export class QueryBuilderHelper {
return;
}

if (prop.customType && convertCustomTypes) {
data[k] = prop.customType.convertToDatabaseValue(data[k], this.platform, true);
}

if (!prop.customType && (Array.isArray(data[k]) || Utils.isPlainObject(data[k]))) {
data[k] = JSON.stringify(data[k]);
}
Expand Down
3 changes: 3 additions & 0 deletions tests/EntityManager.mongo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,9 @@ describe('EntityManagerMongo', () => {
});

test('custom types', async () => {
await orm.em.nativeInsert(FooBar, { name: 'n1', array: [1, 2, 3] });
await orm.em.nativeInsert(FooBar, { name: 'n2', array: [] });

const bar = FooBar.create('b1');
bar.blob = Buffer.from([1, 2, 3, 4, 5]);
bar.array = [];
Expand Down
3 changes: 3 additions & 0 deletions tests/EntityManager.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2337,6 +2337,9 @@ describe('EntityManagerMySql', () => {
});

test('custom types', async () => {
await orm.em.nativeInsert(FooBar2, { id: 123, name: 'n1', array: [1, 2, 3] });
await orm.em.nativeInsert(FooBar2, { id: 456, name: 'n2', array: [] });

const bar = FooBar2.create('b1');
bar.blob = Buffer.from([1, 2, 3, 4, 5]);
bar.array = [];
Expand Down
3 changes: 3 additions & 0 deletions tests/EntityManager.postgre.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1456,6 +1456,9 @@ describe('EntityManagerPostgre', () => {
});

test('custom types', async () => {
await orm.em.nativeInsert(FooBar2, { id: 123, name: 'n1', array: [1, 2, 3] });
await orm.em.nativeInsert(FooBar2, { id: 456, name: 'n2', array: [] });

const bar = FooBar2.create('b1 "b" \'1\'');
bar.blob = Buffer.from([1, 2, 3, 4, 5]);
bar.array = [];
Expand Down
3 changes: 3 additions & 0 deletions tests/EntityManager.sqlite2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,9 @@ describe('EntityManagerSqlite2', () => {
});

test('custom types', async () => {
await orm.em.nativeInsert(FooBar4, { id: 123, name: 'n1', array: [1, 2, 3] });
await orm.em.nativeInsert(FooBar4, { id: 456, name: 'n2', array: [] });

const bar = orm.em.create(FooBar4, { name: 'b1 \'the bad\' lol' });
bar.blob = Buffer.from([1, 2, 3, 4, 5]);
bar.array = [];
Expand Down
Loading

0 comments on commit 83d3ab2

Please sign in to comment.