Skip to content

Commit

Permalink
fix(core): rework JSON value processing
Browse files Browse the repository at this point in the history
Closes #4193
  • Loading branch information
B4nan committed Apr 6, 2023
1 parent ca96acc commit 8261825
Show file tree
Hide file tree
Showing 22 changed files with 327 additions and 187 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/entity/EntityFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class EntityFactory {

mergeData<T extends object>(meta: EntityMetadata<T>, entity: T, data: EntityData<T>, options: FactoryOptions = {}): void {
// merge unchanged properties automatically
data = QueryHelper.processParams({ ...data });
data = QueryHelper.processParams(Utils.copy(data));
const existsData = this.comparator.prepareEntity(entity);
const originalEntityData = helper(entity).__originalEntityData ?? {} as EntityData<T>;
const diff = this.comparator.diffEntities(meta.className, originalEntityData, existsData);
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/hydration/Hydrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export abstract class Hydrator implements IHydrator {
/**
* @inheritDoc
*/
hydrate<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, type: 'full' | 'returning' | 'reference', newEntity = false, convertCustomTypes = false, schema?: string): void {
hydrate<T extends object>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, type: 'full' | 'returning' | 'reference', newEntity = false, convertCustomTypes = false, schema?: string): void {
this.running = true;
const props = this.getProperties(meta, type);

Expand All @@ -29,7 +29,7 @@ export abstract class Hydrator implements IHydrator {
/**
* @inheritDoc
*/
hydrateReference<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes?: boolean, schema?: string): void {
hydrateReference<T extends object>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes?: boolean, schema?: string): void {
this.running = true;
meta.primaryKeys.forEach(pk => {
this.hydrateProperty<T>(entity, meta.properties[pk], data, factory, false, convertCustomTypes);
Expand All @@ -41,7 +41,7 @@ export abstract class Hydrator implements IHydrator {
return this.running;
}

protected getProperties<T>(meta: EntityMetadata<T>, type: 'full' | 'returning' | 'reference'): EntityProperty<T>[] {
protected getProperties<T extends object>(meta: EntityMetadata<T>, type: 'full' | 'returning' | 'reference'): EntityProperty<T>[] {
if (type === 'reference') {
return meta.primaryKeys.map(pk => meta.properties[pk]);
}
Expand All @@ -53,7 +53,7 @@ export abstract class Hydrator implements IHydrator {
return meta.hydrateProps;
}

protected hydrateProperty<T>(entity: T, prop: EntityProperty, data: EntityData<T>, factory: EntityFactory, newEntity?: boolean, convertCustomTypes?: boolean): void {
protected hydrateProperty<T extends object>(entity: T, prop: EntityProperty, data: EntityData<T>, factory: EntityFactory, newEntity?: boolean, convertCustomTypes?: boolean): void {
entity[prop.name] = data[prop.name];
}

Expand Down
21 changes: 11 additions & 10 deletions packages/core/src/hydration/ObjectHydrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import type { EntityData, EntityMetadata, EntityProperty } from '../typings';
import { Hydrator } from './Hydrator';
import { Collection } from '../entity/Collection';
import { Reference } from '../entity/Reference';
import { Utils } from '../utils/Utils';
import { parseJsonSafe, Utils } from '../utils/Utils';
import { ReferenceType } from '../enums';
import type { EntityFactory } from '../entity/EntityFactory';

type EntityHydrator<T> = (entity: T, data: EntityData<T>, factory: EntityFactory, newEntity: boolean, convertCustomTypes: boolean, schema?: string) => void;
type EntityHydrator<T extends object> = (entity: T, data: EntityData<T>, factory: EntityFactory, newEntity: boolean, convertCustomTypes: boolean, schema?: string) => void;

export class ObjectHydrator extends Hydrator {

Expand All @@ -21,7 +21,7 @@ export class ObjectHydrator extends Hydrator {
/**
* @inheritDoc
*/
hydrate<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, type: 'full' | 'returning' | 'reference', newEntity = false, convertCustomTypes = false, schema?: string): void {
hydrate<T extends object>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, type: 'full' | 'returning' | 'reference', newEntity = false, convertCustomTypes = false, schema?: string): void {
const hydrate = this.getEntityHydrator(meta, type);
const running = this.running;
this.running = true;
Expand All @@ -32,7 +32,7 @@ export class ObjectHydrator extends Hydrator {
/**
* @inheritDoc
*/
hydrateReference<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes = false, schema?: string): void {
hydrateReference<T extends object>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes = false, schema?: string): void {
const hydrate = this.getEntityHydrator(meta, 'reference');
const running = this.running;
this.running = true;
Expand All @@ -43,7 +43,7 @@ export class ObjectHydrator extends Hydrator {
/**
* @internal Highly performance-sensitive method.
*/
getEntityHydrator<T>(meta: EntityMetadata<T>, type: 'full' | 'returning' | 'reference'): EntityHydrator<T> {
getEntityHydrator<T extends object>(meta: EntityMetadata<T>, type: 'full' | 'returning' | 'reference'): EntityHydrator<T> {
const exists = this.hydrators[type].get(meta.className);

if (exists) {
Expand Down Expand Up @@ -78,7 +78,7 @@ export class ObjectHydrator extends Hydrator {
return ret;
};

const hydrateScalar = <T, U>(prop: EntityProperty<T>, object: boolean | undefined, path: string[], dataKey: string): string[] => {
const hydrateScalar = (prop: EntityProperty<T>, object: boolean | undefined, path: string[], dataKey: string): string[] => {
const entityKey = path.map(k => this.wrap(k)).join('');
const preCond = preCondition(dataKey);
const convertorKey = path.filter(k => !k.match(/\[idx_\d+]/)).map(k => this.safeKey(k)).join('_');
Expand All @@ -99,7 +99,7 @@ export class ObjectHydrator extends Hydrator {
` const value = convertToJSValue_${convertorKey}(data${dataKey});`,
);

if (prop.customType.ensureComparable()) {
if (prop.customType.ensureComparable(meta, prop)) {
ret.push(` data${dataKey} = convertToDatabaseValue_${convertorKey}(value);`);
}

Expand Down Expand Up @@ -158,7 +158,7 @@ export class ObjectHydrator extends Hydrator {
}
}

if (prop.customType?.ensureComparable()) {
if (prop.customType?.ensureComparable(meta, prop)) {
context.set(`convertToDatabaseValue_${this.safeKey(prop.name)}`, (val: any) => prop.customType.convertToDatabaseValue(val, this.platform, { mode: 'hydration' }));

ret.push(` if (data${dataKey} != null && convertCustomTypes) {`);
Expand Down Expand Up @@ -210,9 +210,10 @@ export class ObjectHydrator extends Hydrator {

const parseObjectEmbeddable = (prop: EntityProperty, dataKey: string, ret: string[]): void => {
if (!this.platform.convertsJsonAutomatically() && (prop.object || prop.array)) {
context.set('parseJsonSafe', parseJsonSafe);
ret.push(
` if (typeof data${dataKey} === 'string') {`,
` data${dataKey} = JSON.parse(data${dataKey});`,
` data${dataKey} = parseJsonSafe(data${dataKey});`,
` }`,
);
}
Expand Down Expand Up @@ -341,7 +342,7 @@ export class ObjectHydrator extends Hydrator {
return hydrator;
}

private createCollectionItemMapper<T>(prop: EntityProperty): string[] {
private createCollectionItemMapper<T extends object>(prop: EntityProperty): string[] {
const meta = this.metadata.get(prop.type);
const lines: string[] = [];

Expand Down
14 changes: 12 additions & 2 deletions packages/core/src/platforms/Platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ import {
ArrayType, BigIntType, BlobType, BooleanType, DateType, DecimalType, DoubleType, JsonType, SmallIntType, TimeType,
TinyIntType, Type, UuidType, StringType, IntegerType, FloatType, DateTimeType, TextType, EnumType, UnknownType, MediumIntType,
} from '../types';
import { Utils } from '../utils/Utils';
import { parseJsonSafe, Utils } from '../utils/Utils';
import { ReferenceType } from '../enums';
import type { MikroORM } from '../MikroORM';
import type { TransformContext } from '../types/Type';

export const JsonProperty = Symbol('JsonProperty');

Expand Down Expand Up @@ -320,8 +321,17 @@ export abstract class Platform {
throw new Error('Full text searching is not supported by this driver.');
}

// TODO v6: remove the `marshall` parameter
convertsJsonAutomatically(marshall = false): boolean {
return !marshall;
return true;
}

convertJsonToDatabaseValue(value: unknown, context?: TransformContext): unknown {
return JSON.stringify(value);
}

convertJsonToJSValue(value: unknown): unknown {
return parseJsonSafe(value);
}

getRepositoryClass<T extends object>(): Constructor<EntityRepository<T>> {
Expand Down
23 changes: 12 additions & 11 deletions packages/core/src/types/JsonType.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,29 @@
import type { TransformContext } from './Type';
import { Type } from './Type';
import type { Platform } from '../platforms';
import type { EntityProperty } from '../typings';
import { Utils } from '../utils';
import type { EntityMetadata, EntityProperty } from '../typings';

export class JsonType extends Type<unknown, string | null> {

convertToDatabaseValue(value: unknown, platform: Platform): string | null {
if (platform.convertsJsonAutomatically(true) || value === null) {
return value as string;
// TODO v6: remove the boolean variant
convertToDatabaseValue(value: unknown, platform: Platform, context?: TransformContext | boolean): string | null {
if (value == null) {
return value as null;
}

return JSON.stringify(value);
return platform.convertJsonToDatabaseValue(value, typeof context === 'boolean' ? { fromQuery: context } : context) as string;
}

convertToJSValue(value: string | unknown, platform: Platform): unknown {
if (!platform.convertsJsonAutomatically() && Utils.isString(value)) {
return JSON.parse(value);
}

return value;
return platform.convertJsonToJSValue(value);
}

getColumnType(prop: EntityProperty, platform: Platform): string {
return platform.getJsonDeclarationSQL();
}

ensureComparable<T extends object>(meta: EntityMetadata<T>, prop: EntityProperty<T>): boolean {
return !prop.embedded || !meta.properties[prop.embedded[0]].object;
}

}
4 changes: 2 additions & 2 deletions packages/core/src/types/Type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Constructor, EntityMetadata, EntityProperty } from '../typings';
export interface TransformContext {
fromQuery?: boolean;
key?: string;
mode?: 'hydration' | 'query' | 'discovery' | 'serialization';
mode?: 'hydration' | 'query' | 'query-data' | 'discovery' | 'serialization';
}

export abstract class Type<JSType = string, DBType = JSType> {
Expand Down Expand Up @@ -53,7 +53,7 @@ export abstract class Type<JSType = string, DBType = JSType> {
* as often the raw database response is not the same as the `convertToDatabaseValue` result.
* This allows to disable the additional conversion in case you know it is not needed.
*/
ensureComparable(): boolean {
ensureComparable<T extends object>(meta: EntityMetadata<T>, prop: EntityProperty<T>): boolean {
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/types/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Type } from './Type';
import { Type, TransformContext } from './Type';
import { DateType } from './DateType';
import { TimeType } from './TimeType';
import { DateTimeType } from './DateTimeType';
Expand All @@ -24,7 +24,7 @@ import { UnknownType } from './UnknownType';
export {
Type, DateType, TimeType, DateTimeType, BigIntType, BlobType, ArrayType, EnumArrayType, EnumType,
JsonType, IntegerType, SmallIntType, TinyIntType, MediumIntType, FloatType, DoubleType, BooleanType, DecimalType,
StringType, UuidType, TextType, UnknownType,
StringType, UuidType, TextType, UnknownType, TransformContext,
};

export const types = {
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/utils/EntityComparator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {
} from '../typings';
import { ReferenceType } from '../enums';
import type { Platform } from '../platforms';
import { compareArrays, compareBooleans, compareBuffers, compareObjects, equals, Utils } from './Utils';
import { compareArrays, compareBooleans, compareBuffers, compareObjects, equals, parseJsonSafe, Utils } from './Utils';
import { JsonType } from '../types/JsonType';

type Comparator<T> = (a: T, b: T) => EntityData<T>;
Expand Down Expand Up @@ -318,8 +318,9 @@ export class EntityComparator {
lines.push(` }`);
}
} else if (prop.reference === ReferenceType.EMBEDDED && prop.object && !this.platform.convertsJsonAutomatically()) {
context.set('parseJsonSafe', parseJsonSafe);
lines.push(` if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') {`);
lines.push(` ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])} == null ? ${propName(prop.fieldNames[0])} : JSON.parse(${propName(prop.fieldNames[0])});`);
lines.push(` ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])} == null ? ${propName(prop.fieldNames[0])} : parseJsonSafe(${propName(prop.fieldNames[0])});`);
lines.push(` ${propName(prop.fieldNames[0], 'mapped')} = true;`);
lines.push(` }`);
} else {
Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/utils/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,19 @@ export function equals(a: any, b: any): boolean {

const equalsFn = equals;

export function parseJsonSafe<T = unknown>(value: unknown): T {
if (typeof value === 'string') {
try {
return JSON.parse(value);
} catch {
// ignore and return the value, as sometimes we get the parsed value,
// e.g. when it is a string value in JSON column
}
}

return value as T;
}

export class Utils {

static readonly PK_SEPARATOR = '~~~';
Expand Down
2 changes: 1 addition & 1 deletion packages/knex/src/AbstractSqlDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection

const addParams = (prop: EntityProperty<T>, row: Dictionary) => {
if (options.convertCustomTypes && prop.customType) {
return params.push(prop.customType.convertToDatabaseValue(row[prop.name], this.platform, { key: prop.name, mode: 'query' }));
return params.push(prop.customType.convertToDatabaseValue(row[prop.name], this.platform, { key: prop.name, mode: 'query-data' }));
}

params.push(row[prop.name]);
Expand Down
2 changes: 1 addition & 1 deletion packages/knex/src/query/QueryBuilderHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ export class QueryBuilderHelper {
}

if (prop.customType && convertCustomTypes && !this.platform.isRaw(data[k])) {
data[k] = prop.customType.convertToDatabaseValue(data[k], this.platform, { fromQuery: true, key: k, mode: 'query' });
data[k] = prop.customType.convertToDatabaseValue(data[k], this.platform, { fromQuery: true, key: k, mode: 'query-data' });
}

if (prop.customType && 'convertToDatabaseValueSQL' in prop.customType && !this.platform.isRaw(data[k])) {
Expand Down
8 changes: 8 additions & 0 deletions packages/mongodb/src/MongoPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ export class MongoPlatform extends Platform {
return true;
}

convertJsonToDatabaseValue(value: unknown): unknown {
return value;
}

convertJsonToJSValue(value: unknown): unknown {
return value;
}

marshallArray(values: string[]): string {
return values as unknown as string;
}
Expand Down
10 changes: 9 additions & 1 deletion packages/mysql/src/MySqlPlatform.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AbstractSqlPlatform } from '@mikro-orm/knex';
import { MySqlSchemaHelper } from './MySqlSchemaHelper';
import { MySqlExceptionConverter } from './MySqlExceptionConverter';
import type { SimpleColumnMeta, Type } from '@mikro-orm/core';
import type { SimpleColumnMeta, Type, TransformContext } from '@mikro-orm/core';
import { expr, Utils } from '@mikro-orm/core';

export class MySqlPlatform extends AbstractSqlPlatform {
Expand All @@ -13,6 +13,14 @@ export class MySqlPlatform extends AbstractSqlPlatform {
return 'utf8mb4';
}

convertJsonToDatabaseValue(value: unknown, context?: TransformContext): unknown {
if (context?.mode === 'query') {
return value;
}

return JSON.stringify(value);
}

getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
const [a, ...b] = path;

Expand Down
6 changes: 4 additions & 2 deletions packages/postgresql/src/PostgreSqlConnection.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { types, defaults } from 'pg';
import TypeOverrides from 'pg/lib/type-overrides';
import type { Dictionary } from '@mikro-orm/core';
import type { Knex } from '@mikro-orm/knex';
import { AbstractSqlConnection, MonkeyPatchable } from '@mikro-orm/knex';
Expand All @@ -16,11 +16,13 @@ export class PostgreSqlConnection extends AbstractSqlConnection {

getConnectionOptions(): Knex.PgConnectionConfig {
const ret = super.getConnectionOptions() as Knex.PgConnectionConfig;
const types = new TypeOverrides();
[1082].forEach(oid => types.setTypeParser(oid, str => str)); // date type

if (this.config.get('forceUtcTimezone')) {
[1114].forEach(oid => types.setTypeParser(oid, str => new Date(str + 'Z'))); // timestamp w/o TZ type
(defaults as any).parseInputDatesAsUTC = true;
ret.parseInputDatesAsUTC = true;
ret.types = types as any;
}

return ret;
Expand Down
2 changes: 1 addition & 1 deletion tests/EntityManager.mariadb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('EntityManagerMariaDb', () => {
forceUtcTimezone: true,
} as any, false);
const driver = new MariaDbDriver(config);
expect(driver.getConnection().getConnectionOptions()).toEqual({
expect(driver.getConnection().getConnectionOptions()).toMatchObject({
database: 'db_name',
host: '127.0.0.10',
password: 'secret',
Expand Down

0 comments on commit 8261825

Please sign in to comment.