fix(core): rework JSON value processing (mikro-orm#4194)

Every driver behaves a bit differently when it comes to handling JSON
columns. While SQLite is not processing then anyhow, and MongoDB
supports them natively, all the others have rather quirky and not very
configurable JSON parsing and stringification implemented. It is crucial
to have the JSON values properly parsed, as well as normalized in the
entity snapshot, so we can correctly detect updates.

This PR makes the JSON parsing fail-safe, returning the value directly
if it is not a valid JSON string, and ensures the entity data are in the
right shape.

Closes mikro-orm#4193

packages/core/src/entity/EntityFactory.ts

@@ -105,7 +105,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);

packages/core/src/hydration/Hydrator.ts

@@ -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);
 
@@ -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);
@@ -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]);
     }

packages/core/src/hydration/ObjectHydrator.ts

@@ -6,7 +6,7 @@ import { Utils } from '../utils/Utils';
 import { ReferenceKind } 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 {
 
@@ -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) {
@@ -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('_');
@@ -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);`);
         }
 
@@ -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) {`);
@@ -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});`,
           `  }`,
         );
       }
@@ -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[] = [];
 

packages/core/src/platforms/Platform.ts

@@ -14,6 +14,7 @@ import {
 import { Utils } from '../utils/Utils';
 import { ReferenceKind } from '../enums';
 import type { MikroORM } from '../MikroORM';
+import type { TransformContext } from '../types/Type';
 
 export const JsonProperty = Symbol('JsonProperty');
 
@@ -321,8 +322,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>> {

packages/core/src/types/JsonType.ts

@@ -1,7 +1,7 @@
+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> {
 
@@ -10,7 +10,7 @@ export class JsonType extends Type<unknown, string | null> {
       return value as string;
     }
 
-    return JSON.stringify(value);
+    return platform.convertJsonToDatabaseValue(value, typeof context === 'boolean' ? { fromQuery: context } : context) as string;
   }
 
   override convertToJSValue(value: string | unknown, platform: Platform): unknown {
@@ -25,4 +25,8 @@ export class JsonType extends Type<unknown, string | null> {
     return platform.getJsonDeclarationSQL();
   }
 
+  ensureComparable<T extends object>(meta: EntityMetadata<T>, prop: EntityProperty<T>): boolean {
+    return !prop.embedded || !meta.properties[prop.embedded[0]].object;
+  }
+
 }

packages/core/src/types/Type.ts

@@ -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> {
@@ -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;
   }
 

packages/core/src/utils/EntityComparator.ts

@@ -11,7 +11,7 @@ import type {
 } from '../typings';
 import { ReferenceKind } 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>;
@@ -320,7 +320,7 @@ export class EntityComparator {
         }
       } else if (prop.kind === ReferenceKind.EMBEDDED && prop.object && !this.platform.convertsJsonAutomatically()) {
         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 {

packages/core/src/utils/Utils.ts

@@ -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 = '~~~';

packages/knex/src/query/QueryBuilderHelper.ts

@@ -821,7 +821,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])) {

packages/postgresql/src/PostgreSqlConnection.ts

@@ -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';
@@ -17,11 +17,13 @@ export class PostgreSqlConnection extends AbstractSqlConnection {
 
   override 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;

tests/EntityManager.mariadb.test.ts

@@ -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',

tests/EntityManager.mysql.test.ts

@@ -67,7 +67,7 @@ describe('EntityManagerMySql', () => {
     } as any, false);
     config.reset('debug');
     const driver = new MySqlDriver(config);
-    expect(driver.getConnection().getConnectionOptions()).toEqual({
+    expect(driver.getConnection().getConnectionOptions()).toMatchObject({
       database: 'db_name',
       host: '127.0.0.10',
       password: 'secret',
@@ -827,24 +827,30 @@ describe('EntityManagerMySql', () => {
     await orm.em.persistAndFlush(bible);
     orm.em.clear();
 
-    const qb1 = orm.em.createQueryBuilder(Book2);
+    const qb1 = orm.em.fork().createQueryBuilder(Book2);
     const res1 = await qb1.select('*').where({ 'JSON_CONTAINS(`b0`.`meta`, ?)': [{ foo: 'bar' }, false] }).execute('get');
     expect(res1.createdAt).toBeDefined();
     // @ts-expect-error
     expect(res1.created_at).not.toBeDefined();
     expect(res1.meta).toEqual({ category: 'foo', items: 1 });
 
-    const qb2 = orm.em.createQueryBuilder(Book2);
+    const qb2 = orm.em.fork().createQueryBuilder(Book2);
     const res2 = await qb2.select('*').where({ 'JSON_CONTAINS(meta, ?)': [{ category: 'foo' }, true] }).execute('get', false);
     expect(res2.createdAt).not.toBeDefined();
     // @ts-expect-error
     expect(res2.created_at).toBeDefined();
     expect(res2.meta).toEqual({ category: 'foo', items: 1 });
 
-    const res3 = await orm.em.findOneOrFail(Book2, { [expr('JSON_CONTAINS(meta, ?)')]: [{ items: 1 }, true] });
+    const qb3 = orm.em.fork().createQueryBuilder(Book2);
+    const res3 = await qb3.select('*').where({ 'JSON_CONTAINS(meta, ?)': [{ category: 'foo' }, true] }).getSingleResult();
     expect(res3).toBeInstanceOf(Book2);
-    expect(res3.createdAt).toBeDefined();
-    expect(res3.meta).toEqual({ category: 'foo', items: 1 });
+    expect(res3!.createdAt).toBeDefined();
+    expect(res3!.meta).toEqual({ category: 'foo', items: 1 });
+
+    const res4 = await orm.em.fork().findOneOrFail(Book2, { [expr('JSON_CONTAINS(meta, ?)')]: [{ items: 1 }, true] });
+    expect(res4).toBeInstanceOf(Book2);
+    expect(res4.createdAt).toBeDefined();
+    expect(res4.meta).toEqual({ category: 'foo', items: 1 });
   });
 
   test('tuple comparison', async () => {

tests/EntityManager.postgre.test.ts

@@ -79,7 +79,7 @@ describe('EntityManagerPostgre', () => {
       forceUtcTimezone: true,
     } as any, false);
     const driver = new PostgreSqlDriver(config);
-    expect(driver.getConnection().getConnectionOptions()).toEqual({
+    expect(driver.getConnection().getConnectionOptions()).toMatchObject({
       database: 'db_name',
       host: '127.0.0.10',
       password: 'secret',
@@ -1943,14 +1943,14 @@ describe('EntityManagerPostgre', () => {
       ],
     } as any, false);
     const driver = new PostgreSqlDriver(config);
-    expect(driver.getConnection('write').getConnectionOptions()).toEqual({
+    expect(driver.getConnection('write').getConnectionOptions()).toMatchObject({
       database: 'db_name',
       host: '127.0.0.10',
       password: 'secret',
       user: 'user',
       port: 1234,
     });
-    expect(driver.getConnection('read').getConnectionOptions()).toEqual({
+    expect(driver.getConnection('read').getConnectionOptions()).toMatchObject({
       database: 'db_name',
       host: 'read_host_1',
       password: 'secret',

tests/features/custom-types/GH4193.test.ts

@@ -0,0 +1,36 @@
+import { MikroORM } from '@mikro-orm/mysql';
+import { Entity, PrimaryKey, Property } from '@mikro-orm/core';
+
+@Entity()
+class User {
+
+  @PrimaryKey({ type: 'number' })
+  id?: number;
+
+  @Property({ type: 'json' })
+  value!: string;
+
+}
+
+let orm: MikroORM;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    dbName: 'mo-test',
+    port: 3308,
+    entities: [User],
+  });
+  await orm.schema.refreshDatabase();
+});
+
+afterAll(() => orm.close());
+
+test('It should fetch record matching by json column', async () => {
+  const user = new User();
+  user.id = 1;
+  user.value = 'test';
+  await orm.em.fork().persistAndFlush(user);
+
+  const c = await orm.em.findOne(User, { value: 'test' });
+  expect(c).not.toBeNull();
+});