From e1f82bc5b63ea7547685244fa42a7f612250b909 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Ad=C3=A1mek?= <banan23@gmail.com>
Date: Sat, 4 Feb 2023 01:02:46 +0100
Subject: [PATCH] fix(core): ensure json type is recognized with `type:
 'jsonb'`

Also supports inference from `columnType` and improves the overall logic
for internal custom type mapping that is needed for JSON columns.

Closes #3998
---
 packages/core/src/metadata/ReflectMetadataProvider.ts    | 9 ++++++++-
 packages/core/src/utils/EntityComparator.ts              | 2 +-
 tests/entities-sql/BaseEntity2.ts                        | 2 +-
 tests/entities/BaseEntity.ts                             | 2 +-
 tests/features/composite-keys/GH1157.test.ts             | 6 +++---
 .../custom-types/json-hydration.postgres.test.ts         | 4 ++--
 tests/issues/GH1958.test.ts                              | 4 ++--
 7 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/packages/core/src/metadata/ReflectMetadataProvider.ts b/packages/core/src/metadata/ReflectMetadataProvider.ts
index a49c09b45c48..e8d90a8b1ded 100644
--- a/packages/core/src/metadata/ReflectMetadataProvider.ts
+++ b/packages/core/src/metadata/ReflectMetadataProvider.ts
@@ -10,12 +10,19 @@ export class ReflectMetadataProvider extends MetadataProvider {
   }
 
   protected initPropertyType(meta: EntityMetadata, prop: EntityProperty) {
-    const type = Reflect.getMetadata('design:type', meta.prototype, prop.name);
+    let type = Reflect.getMetadata('design:type', meta.prototype, prop.name);
 
     if (!type || (type === Object && prop.reference !== ReferenceType.SCALAR)) {
       throw new Error(`Please provide either 'type' or 'entity' attribute in ${meta.className}.${prop.name}`);
     }
 
+    // Instead of requiring the type everywhere, we default to string, which maintains the behaviour,
+    // as we were mapping it to UnknownType which is a string. This is to prevent defaulting to JSON
+    // column type, which can be often hard to revert and cause hard to understand issues with PKs.
+    if (prop.reference === ReferenceType.SCALAR && type === Object) {
+      type = String;
+    }
+
     prop.type = type.name;
 
     if (prop.type && ['string', 'number', 'boolean', 'array', 'object'].includes(prop.type.toLowerCase())) {
diff --git a/packages/core/src/utils/EntityComparator.ts b/packages/core/src/utils/EntityComparator.ts
index ba73daf41648..34e00ba44351 100644
--- a/packages/core/src/utils/EntityComparator.ts
+++ b/packages/core/src/utils/EntityComparator.ts
@@ -285,7 +285,7 @@ export class EntityComparator {
       }
 
       if (prop.type === 'boolean') {
-        lines.push(`  if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') { ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])} == null ? ${propName(prop.fieldNames[0])} : !!${propName(prop.fieldNames[0])}; mapped.${prop.fieldNames[0]} = true; }`);
+        lines.push(`  if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') { ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])} == null ? ${propName(prop.fieldNames[0])} : !!${propName(prop.fieldNames[0])}; ${propName(prop.fieldNames[0], 'mapped')} = true; }`);
       } else {
         lines.push(`  if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') { ret${this.wrap(prop.name)} = ${propName(prop.fieldNames[0])}; ${propName(prop.fieldNames[0], 'mapped')} = true; }`);
       }
diff --git a/tests/entities-sql/BaseEntity2.ts b/tests/entities-sql/BaseEntity2.ts
index 2ad2d997e85e..82ff24e17651 100644
--- a/tests/entities-sql/BaseEntity2.ts
+++ b/tests/entities-sql/BaseEntity2.ts
@@ -6,7 +6,7 @@ export abstract class BaseEntity2 {
   id!: number;
 
   @Property({ persist: false })
-  hookTest = false;
+  hookTest: boolean = false;
 
   protected constructor() {
     const props = wrap(this, true).__meta.properties;
diff --git a/tests/entities/BaseEntity.ts b/tests/entities/BaseEntity.ts
index 72b140a7ff9d..6dc4def80998 100644
--- a/tests/entities/BaseEntity.ts
+++ b/tests/entities/BaseEntity.ts
@@ -23,7 +23,7 @@ export abstract class BaseEntity<T extends { id: unknown; _id: unknown }, Option
   foo?: string;
 
   @Property({ persist: false })
-  hookTest = false;
+  hookTest: boolean = false;
 
   @BeforeCreate()
   baseBeforeCreate() {
diff --git a/tests/features/composite-keys/GH1157.test.ts b/tests/features/composite-keys/GH1157.test.ts
index a10f1fa23bd5..4147a1d2efb9 100644
--- a/tests/features/composite-keys/GH1157.test.ts
+++ b/tests/features/composite-keys/GH1157.test.ts
@@ -7,7 +7,7 @@ import { SqliteDriver } from '@mikro-orm/sqlite';
 export class D {
 
   @PrimaryKey()
-  id = v4();
+  id: string = v4();
 
   @ManyToOne({ entity: 'A' })
   a!: any;
@@ -18,7 +18,7 @@ export class D {
 export class C {
 
   @PrimaryKey()
-  id = v4();
+  id: string = v4();
 
 }
 
@@ -26,7 +26,7 @@ export class C {
 export class B {
 
   @PrimaryKey()
-  id = v4();
+  id: string = v4();
 
 }
 
diff --git a/tests/features/custom-types/json-hydration.postgres.test.ts b/tests/features/custom-types/json-hydration.postgres.test.ts
index 2913faacd9cf..73064025526f 100644
--- a/tests/features/custom-types/json-hydration.postgres.test.ts
+++ b/tests/features/custom-types/json-hydration.postgres.test.ts
@@ -1,5 +1,5 @@
 import { MikroORM } from '@mikro-orm/postgresql';
-import { Embedded, Entity, PrimaryKey, Embeddable, OneToOne, Property, t } from '@mikro-orm/core';
+import { Embedded, Entity, PrimaryKey, Embeddable, OneToOne, Property } from '@mikro-orm/core';
 
 @Embeddable()
 export class Page {
@@ -7,7 +7,7 @@ export class Page {
   private _attestations!: string[];
   static log: unknown[] = [];
 
-  @Property({ type: t.json })
+  @Property({ type: 'jsonb' })
   get attestations(): string[] {
     return this._attestations;
   }
diff --git a/tests/issues/GH1958.test.ts b/tests/issues/GH1958.test.ts
index b46b1d393d6c..e481367e8f95 100644
--- a/tests/issues/GH1958.test.ts
+++ b/tests/issues/GH1958.test.ts
@@ -5,10 +5,10 @@ import { SqliteDriver } from '@mikro-orm/sqlite';
 class LoopOptions {
 
   @Property()
-  'enabled-prop' = false;
+  'enabled-prop': boolean = false;
 
   @Property()
-  'type-prop' = 'a';
+  'type-prop': string = 'a';
 
 }