fix(core): detect JsonType based on columnType (#4252)

Closes #4229
mikro-orm · Apr 21, 2023 · 2e01622 · 2e01622
1 parent 9132e23
commit 2e01622
Show file tree

Hide file tree

Showing 18 changed files with 55 additions and 17 deletions.
diff --git a/packages/core/src/metadata/EntitySchema.ts b/packages/core/src/metadata/EntitySchema.ts
@@ -264,7 +264,7 @@ export class EntitySchema<T = any, U = never> {
 
   private initProperties(): void {
     Object.entries<Property<T, unknown>>(this._meta.properties as Dictionary).forEach(([name, options]) => {
-      options.type = options.customType != null ? options.customType.constructor.name : options.type;
+      options.type ??= options.customType != null ? options.customType.constructor.name : options.type;
 
       switch ((options as EntityProperty).reference) {
         case ReferenceType.ONE_TO_ONE:

diff --git a/packages/core/src/metadata/MetadataDiscovery.ts b/packages/core/src/metadata/MetadataDiscovery.ts
@@ -1020,6 +1020,10 @@ export class MetadataDiscovery {
       prop.customType = new JsonType();
     }
 
+    if (prop.reference === ReferenceType.SCALAR && !prop.customType && prop.columnTypes && ['json', 'jsonb'].includes(prop.columnTypes[0])) {
+      prop.customType = new JsonType();
+    }
+
     if (!prop.customType && this.getMappedType(prop) instanceof BigIntType) {
       prop.customType = new BigIntType();
     }
@@ -1029,6 +1033,8 @@ export class MetadataDiscovery {
       prop.customType.meta = meta;
       prop.customType.prop = prop;
       prop.columnTypes ??= [prop.customType.getColumnType(prop, this.platform)];
+      prop.hasConvertToJSValueSQL = !!prop.customType.convertToJSValueSQL && prop.customType.convertToJSValueSQL('', this.platform) !== '';
+      prop.hasConvertToDatabaseValueSQL = !!prop.customType.convertToDatabaseValueSQL && prop.customType.convertToDatabaseValueSQL('', this.platform) !== '';
     }
 
     if (Type.isMappedType(prop.customType) && prop.reference === ReferenceType.SCALAR && !prop.type?.toString().endsWith('[]')) {

diff --git a/packages/core/src/platforms/Platform.ts b/packages/core/src/platforms/Platform.ts
@@ -470,7 +470,14 @@ export abstract class Platform {
   /**
    * @internal
    */
-  castColumn(prop?: EntityProperty): string {
+  castColumn(prop?: { columnTypes?: string[] }): string {
+    return '';
+  }
+
+  /**
+   * @internal
+   */
+  castJsonValue(prop?: { columnTypes?: string[] }): string {
     return '';
   }
 

diff --git a/packages/core/src/types/JsonType.ts b/packages/core/src/types/JsonType.ts
@@ -14,6 +14,14 @@ export class JsonType extends Type<unknown, string | null> {
     return platform.convertJsonToDatabaseValue(value, typeof context === 'boolean' ? { fromQuery: context } : context) as string;
   }
 
+  convertToJSValueSQL(key: string, platform: Platform): string {
+    return key + platform.castJsonValue(this.prop);
+  }
+
+  convertToDatabaseValueSQL(key: string, platform: Platform): string {
+    return key + platform.castColumn(this.prop);
+  }
+
   convertToJSValue(value: string | unknown, platform: Platform): unknown {
     return platform.convertJsonToJSValue(value);
   }

diff --git a/packages/core/src/typings.ts b/packages/core/src/typings.ts
@@ -253,6 +253,8 @@ export interface EntityProperty<T = any> {
   targetMeta?: EntityMetadata;
   columnTypes: string[];
   customType: Type<any>;
+  hasConvertToJSValueSQL: boolean;
+  hasConvertToDatabaseValueSQL: boolean;
   autoincrement?: boolean;
   primary?: boolean;
   serializedPrimaryKey: boolean;

diff --git a/packages/knex/src/AbstractSqlDriver.ts b/packages/knex/src/AbstractSqlDriver.ts
@@ -1001,7 +1001,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
         });
 
       meta.props
-        .filter(prop => prop.customType?.convertToDatabaseValueSQL || prop.customType?.convertToJSValueSQL)
+        .filter(prop => prop.hasConvertToDatabaseValueSQL || prop.hasConvertToJSValueSQL)
         .forEach(prop => ret.push(prop.name));
     }
 

diff --git a/packages/knex/src/query/CriteriaNode.ts b/packages/knex/src/query/CriteriaNode.ts
@@ -124,7 +124,7 @@ export class CriteriaNode implements ICriteriaNode {
   [inspect.custom]() {
     const o: Dictionary = {};
     ['entityName', 'key', 'index', 'payload']
-      .filter(k => this[k] != null)
+      .filter(k => this[k] !== undefined)
       .forEach(k => o[k] = this[k]);
 
     return `${this.constructor.name} ${inspect(o)}`;

diff --git a/packages/knex/src/query/CriteriaNodeFactory.ts b/packages/knex/src/query/CriteriaNodeFactory.ts
@@ -48,10 +48,6 @@ export class CriteriaNodeFactory {
   static createObjectNode(metadata: MetadataStorage, entityName: string, payload: Dictionary, parent?: ICriteriaNode, key?: string): ICriteriaNode {
     const meta = metadata.find(entityName);
 
-    if (!parent && Object.keys(payload).every(k => meta?.properties[k]?.reference === ReferenceType.SCALAR)) {
-      return this.createScalarNode(metadata, entityName, payload, parent, key);
-    }
-
     const node = new ObjectCriteriaNode(metadata, entityName, parent, key);
     node.payload = Object.keys(payload).reduce((o, item) => {
       o[item] = this.createObjectItemNode(metadata, entityName, node, payload, item, meta);

diff --git a/packages/knex/src/query/QueryBuilder.ts b/packages/knex/src/query/QueryBuilder.ts
@@ -887,7 +887,7 @@ export class QueryBuilder<T extends object = AnyEntity> {
 
     const meta = this.mainAlias.metadata;
     /* istanbul ignore next */
-    const requiresSQLConversion = meta?.props.filter(p => p.customType?.convertToJSValueSQL) ?? [];
+    const requiresSQLConversion = meta?.props.filter(p => p.hasConvertToJSValueSQL) ?? [];
 
     if (this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES) && (fields.includes('*') || fields.includes(`${this.mainAlias.aliasName}.*`)) && requiresSQLConversion.length > 0) {
       requiresSQLConversion.forEach(p => ret.push(this.helper.mapper(p.name, this.type)));

diff --git a/packages/knex/src/query/QueryBuilderHelper.ts b/packages/knex/src/query/QueryBuilderHelper.ts
@@ -84,7 +84,7 @@ export class QueryBuilderHelper {
       return this.knex.raw(`${prop.formula(alias2)}${as}`);
     }
 
-    if (prop?.customType?.convertToJSValueSQL) {
+    if (prop?.hasConvertToJSValueSQL) {
       const prefixed = this.prefix(field, isTableNameAliasRequired, true);
       const valueSQL = prop.customType.convertToJSValueSQL!(prefixed, this.platform);
 
@@ -789,9 +789,10 @@ export class QueryBuilderHelper {
         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])) {
+      if (prop.hasConvertToDatabaseValueSQL && !this.platform.isRaw(data[k])) {
         const quoted = this.platform.quoteValue(data[k]);
-        data[k] = this.knex.raw(prop.customType.convertToDatabaseValueSQL!(quoted, this.platform).replace(/\?/, '\\?'));
+        const sql = prop.customType.convertToDatabaseValueSQL!(quoted, this.platform);
+        data[k] = this.knex.raw(sql.replace(/\?/, '\\?'));
       }
 
       if (!prop.customType && (Array.isArray(data[k]) || Utils.isPlainObject(data[k]))) {

diff --git a/packages/knex/src/typings.ts b/packages/knex/src/typings.ts
@@ -145,6 +145,7 @@ export interface IQueryBuilder<T> {
   having(cond?: QBFilterQuery | string, params?: any[]): this;
   getAliasForJoinPath(path: string): string | undefined;
   getNextAlias(entityName?: string): string;
+  raw(field: string): any;
 }
 
 export interface ICriteriaNode {

diff --git a/packages/mariadb/src/MariaDbConnection.ts b/packages/mariadb/src/MariaDbConnection.ts
@@ -38,6 +38,8 @@ export class MariaDbConnection extends AbstractSqlConnection {
 
     ret.bigNumberStrings = true;
     ret.supportBigNumbers = true;
+    // @ts-ignore
+    ret.checkDuplicate = false;
 
     return ret;
   }

diff --git a/packages/postgresql/src/PostgreSqlPlatform.ts b/packages/postgresql/src/PostgreSqlPlatform.ts
@@ -269,12 +269,24 @@ export class PostgreSqlPlatform extends AbstractSqlPlatform {
   /**
    * @inheritDoc
    */
-  castColumn(prop?: EntityProperty): string {
+  castColumn(prop?: { columnTypes?: string[] }): string {
     switch (prop?.columnTypes?.[0]) {
       case this.getUuidTypeDeclarationSQL({}): return '::text';
       case this.getBooleanTypeDeclarationSQL(): return '::int';
+      case 'json': return '::jsonb';
       default: return '';
     }
   }
 
+  /**
+   * @inheritDoc
+   */
+  castJsonValue(prop?: { columnTypes?: string[] }): string {
+    if (prop?.columnTypes?.[0] === 'json') {
+      return '::text';
+    }
+
+    return '';
+  }
+
 }
diff --git a/tests/EntityManager.sqlite2.test.ts b/tests/EntityManager.sqlite2.test.ts
@@ -525,8 +525,9 @@ describe.each(['sqlite', 'better-sqlite'] as const)('EntityManager (%s)', driver
     expect(b1).toBe(b5);
     expect(b1).toBe(b6);
     expect(b1).toBe(b7);
+  });
 
-    // complex condition for json property with update query (GH #2839)
+  test('complex condition for json property with update query (GH #2839)', async () => {
     const qb141 = orm.em.createQueryBuilder(Book4).update({ meta: { items: 3 } }).where({
       $and: [
         { id: 123 },

diff --git a/tests/features/custom-types/json-properties.test.ts b/tests/features/custom-types/json-properties.test.ts
@@ -9,7 +9,7 @@ export class User {
   @PrimaryKey({ name: '_id' })
   id: number = User.id++;
 
-  @Property({ type: 'json' })
+  @Property({ columnType: 'json' })
   value: any;
 
 }

diff --git a/tests/features/embeddables/embedded-entities.postgres.test.ts b/tests/features/embeddables/embedded-entities.postgres.test.ts
@@ -281,7 +281,7 @@ describe('embedded entities in postgresql', () => {
     expect(mock.mock.calls[2][0]).toMatch('select "u0"."id", "u0"."addr_street", "u0"."addr_city" from "user" as "u0"');
   });
 
-  test('partial loading', async () => {
+  test('partial loading 2', async () => {
     const mock = mockLogger(orm, ['query']);
 
     await orm.em.fork().qb(User).select('address1.city').where({ address1: { city: 'London 1' } }).execute();

diff --git a/tests/features/embeddables/entities-in-embeddables.postgres.test.ts b/tests/features/embeddables/entities-in-embeddables.postgres.test.ts
@@ -368,7 +368,9 @@ describe('embedded entities in postgres', () => {
     const u5 = await orm.em.findOneOrFail(User, { $or: [{ profile1: { identity: { meta: { foo: 'foooooooo' } } } }, { profile2: { identity: { meta: { bar: 'bababar' } } } }] });
     expect(mock.mock.calls[0][0]).toMatch(`select "u0".* from "user" as "u0" where ("u0"."profile1_identity_meta_foo" = 'foooooooo' or "u0"."profile2"->'identity'->'meta'->>'bar' = 'bababar') limit 1`);
     expect(u5.id).toEqual(u1.id);
+  });
 
+  test('invalid embedded property query', async () => {
     const err1 = `Invalid query for entity 'User', property 'city' does not exist in embeddable 'Identity'`;
     await expect(orm.em.findOneOrFail(User, { profile1: { identity: { city: 'London 1' } as any } })).rejects.toThrowError(err1);
 

diff --git a/tests/features/embeddables/nested-embeddables.postgres.test.ts b/tests/features/embeddables/nested-embeddables.postgres.test.ts
@@ -348,7 +348,7 @@ describe('embedded entities in postgres', () => {
     expect(jon.profile1.identity.meta).toBeUndefined();
   });
 
-  test('partial loading', async () => {
+  test('partial loading 2', async () => {
     const mock = mockLogger(orm, ['query']);
 
     await orm.em.fork().qb(User).select('profile1.identity.email').where({ profile1: { identity: { email: 'foo@bar.baz' } } }).execute();