fix(core): refactor mapping of Date properties (#4391)

Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476

docs/docs/upgrading-v5-to-v6.md

@@ -359,3 +359,9 @@ const dtos = serialize([user1, user, ...], { exclude: ['id', 'email'], forceObje
 const [dto1] = serialize(user, { exclude: ['id', 'email'], forceObject: true });
 const dto2 = wrap(user).serialize({ exclude: ['id', 'email'], forceObject: true });
 ```
+
+## Changes in `Date` property mapping 
+
+Previously, mapping of datetime columns to JS `Date` objects was dependent on the driver, while SQLite didn't have this out of box support and required manual conversion on various places. All drivers now have disabled `Date` conversion and this is now handled explicitly, in the same way for all of them.
+
+Moreover, the `date` type was previously seen as a `datetime`, while now only `Date` (with uppercase `D`) will be considered as `datetime`, while `date` is just a `date`.

packages/better-sqlite/src/BetterSqlitePlatform.ts

@@ -81,7 +81,7 @@ export class BetterSqlitePlatform extends AbstractSqlPlatform {
   }
 
   override quoteVersionValue(value: Date | number, prop: EntityProperty): Date | string | number {
-    if (prop.type.toLowerCase() === 'date') {
+    if (prop.runtimeType === 'Date') {
       return escape(value, true, this.timezone).replace(/^'|\.\d{3}'$/g, '');
     }
 

packages/core/src/EntityManager.ts

@@ -1293,7 +1293,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
     Object.keys(data).forEach(k => {
       const prop = meta.properties[k];
 
-      if (prop && prop.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.type.toLowerCase()) && (prop.setter || !prop.getter)) {
+      if (prop && prop.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.runtimeType) && (prop.setter || !prop.getter)) {
         data[k] = this.validator.validateProperty(prop, data[k], data);
       }
     });

packages/core/src/entity/EntityAssigner.ts

@@ -105,11 +105,11 @@ export class EntityAssigner {
       return EntityAssigner.assignReference<T>(entity, value, prop, options.em, options);
     }
 
-    if (prop?.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.type.toLowerCase()) && (prop.setter || !prop.getter)) {
+    if (prop.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.runtimeType) && (prop.setter || !prop.getter)) {
       return entity[propName as keyof T] = validator.validateProperty(prop, value, entity);
     }
 
-    if (prop?.kind === ReferenceKind.EMBEDDED && EntityAssigner.validateEM(options.em)) {
+    if (prop.kind === ReferenceKind.EMBEDDED && EntityAssigner.validateEM(options.em)) {
       return EntityAssigner.assignEmbeddable(entity, value, prop, options.em, options);
     }
 

packages/core/src/entity/EntityValidator.ts

@@ -18,9 +18,9 @@ export class EntityValidator {
         this.validateCollection(entity, prop);
       }
 
-      const SCALAR_TYPES = ['string', 'number', 'boolean', 'date'];
+      const SCALAR_TYPES = ['string', 'number', 'boolean', 'Date'];
 
-      if (prop.kind !== ReferenceKind.SCALAR || !SCALAR_TYPES.includes(prop.type.toLowerCase())) {
+      if (prop.kind !== ReferenceKind.SCALAR || !SCALAR_TYPES.includes(prop.type)) {
         return;
       }
 
@@ -66,7 +66,7 @@ export class EntityValidator {
       return givenValue;
     }
 
-    const expectedType = prop.type.toLowerCase();
+    const expectedType = prop.runtimeType;
     let givenType = Utils.getObjectType(givenValue);
     let ret = givenValue;
 
@@ -136,7 +136,7 @@ export class EntityValidator {
   }
 
   private fixTypes(expectedType: string, givenType: string, givenValue: any): any {
-    if (expectedType === 'date' && ['string', 'number'].includes(givenType)) {
+    if (expectedType === 'Date' && ['string', 'number'].includes(givenType)) {
       givenValue = this.fixDateType(givenValue);
     }
 

packages/core/src/enums.ts

@@ -96,7 +96,7 @@ export enum QueryFlag {
   AUTO_JOIN_ONE_TO_ONE_OWNER = 'AUTO_JOIN_ONE_TO_ONE_OWNER',
 }
 
-export const SCALAR_TYPES = ['string', 'number', 'boolean', 'date', 'buffer', 'regexp'];
+export const SCALAR_TYPES = ['string', 'number', 'boolean', 'Date', 'Buffer', 'RegExp'];
 
 export enum ReferenceKind {
   SCALAR = 'scalar',

packages/core/src/hydration/ObjectHydrator.ts

@@ -81,7 +81,7 @@ export class ObjectHydrator extends Hydrator {
 
     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 tz = this.platform.getTimezone();
       const convertorKey = path.filter(k => !k.match(/\[idx_\d+]/)).map(k => this.safeKey(k)).join('_');
       const ret: string[] = [];
       const idx = this.tmpIndex++;
@@ -96,9 +96,7 @@ export class ObjectHydrator extends Hydrator {
       ret.push(`    entity${entityKey} = ${nullVal};`);
       ret.push(`  } else if (typeof data${dataKey} !== 'undefined') {`);
 
-      if (prop.type.toLowerCase() === 'date') {
-        ret.push(`    entity${entityKey} = new Date(data${dataKey});`);
-      } else if (prop.customType) {
+      if (prop.customType) {
         context.set(`convertToJSValue_${convertorKey}`, (val: any) => prop.customType.convertToJSValue(val, this.platform));
         context.set(`convertToDatabaseValue_${convertorKey}`, (val: any) => prop.customType.convertToDatabaseValue(val, this.platform, { mode: 'hydration' }));
 
@@ -117,8 +115,23 @@ export class ObjectHydrator extends Hydrator {
           `      entity${entityKey} = data${dataKey};`,
           `    }`,
         );
-      } else if (prop.type.toLowerCase() === 'boolean') {
-        ret.push(`    entity${entityKey} = data${dataKey} === null ? ${nullVal} : !!data${dataKey};`);
+      } else if (prop.runtimeType === 'boolean') {
+        ret.push(`    entity${entityKey} = !!data${dataKey};`);
+      } else if (prop.runtimeType === 'Date') {
+        ret.push(`    if (data${dataKey} instanceof Date) {`);
+        ret.push(`      entity${entityKey} = data${dataKey};`);
+
+        if (!tz || tz === 'local') {
+          ret.push(`    } else {`);
+          ret.push(`      entity${entityKey} = new Date(data${dataKey});`);
+        } else {
+          ret.push(`    } else if (typeof data${dataKey} === 'number' || data${dataKey}.includes('+')) {`);
+          ret.push(`      entity${entityKey} = new Date(data${dataKey});`);
+          ret.push(`    } else {`);
+          ret.push(`      entity${entityKey} = new Date(data${dataKey} + '${tz}');`);
+        }
+
+        ret.push(`    }`);
       } else {
         ret.push(`    entity${entityKey} = data${dataKey};`);
       }

packages/core/src/metadata/MetadataDiscovery.ts

@@ -1024,8 +1024,7 @@ export class MetadataDiscovery {
 
       // but still use object values for type inference if not explicitly set, e.g. `createdAt = new Date()`
       if (prop.kind === ReferenceKind.SCALAR && prop.type == null && entity1[prop.name] != null) {
-        const type = Utils.getObjectType(entity1[prop.name]);
-        prop.type = type === 'object' ? 'string' : type;
+        prop.type = Utils.getObjectType(entity1[prop.name]);
       }
     } catch {
       // ignore
@@ -1101,10 +1100,27 @@ export class MetadataDiscovery {
       prop.customType = new Uint8ArrayType();
     }
 
-    if (!prop.customType && this.getMappedType(prop) instanceof BigIntType) {
+    if (!prop.customType && ['json', 'jsonb'].includes(prop.type)) {
+      prop.customType = new JsonType();
+    }
+
+    if (prop.kind === ReferenceKind.SCALAR && !prop.customType && prop.columnTypes && ['json', 'jsonb'].includes(prop.columnTypes[0])) {
+      prop.customType = new JsonType();
+    }
+
+    const mappedType = this.getMappedType(prop);
+
+    if (!prop.customType && mappedType instanceof BigIntType) {
       prop.customType = new BigIntType();
     }
 
+    if (prop.customType && !prop.columnTypes) {
+      const mappedType = this.getMappedType({ columnTypes: [prop.customType.getColumnType(prop, this.platform)] } as EntityProperty);
+      prop.runtimeType ??= mappedType.runtimeType as typeof prop.runtimeType;
+    } else {
+      prop.runtimeType ??= mappedType.runtimeType as typeof prop.runtimeType;
+    }
+
     if (prop.customType) {
       prop.customType.platform = this.platform;
       prop.customType.meta = meta;
@@ -1115,7 +1131,7 @@ export class MetadataDiscovery {
     }
 
     if (Type.isMappedType(prop.customType) && prop.kind === ReferenceKind.SCALAR && !prop.type?.toString().endsWith('[]')) {
-      prop.type = prop.customType.constructor.name;
+      prop.type = prop.customType.name;
     }
 
     if (!prop.customType && [ReferenceKind.ONE_TO_ONE, ReferenceKind.MANY_TO_ONE].includes(prop.kind) && this.metadata.get(prop.type).compositePK) {
@@ -1131,7 +1147,6 @@ export class MetadataDiscovery {
     }
 
     if (prop.kind === ReferenceKind.SCALAR) {
-      const mappedType = this.getMappedType(prop);
       prop.columnTypes ??= [mappedType.getColumnType(prop, this.platform)];
 
       // use only custom types provided by user, we don't need to use the ones provided by ORM,
@@ -1203,19 +1218,23 @@ export class MetadataDiscovery {
   }
 
   private getMappedType(prop: EntityProperty): Type<unknown> {
-    let t = prop.columnTypes?.[0] ?? prop.type?.toLowerCase();
+    if (prop.customType) {
+      return prop.customType;
+    }
+
+    let t = prop.columnTypes?.[0] ?? prop.type;
 
     if (prop.nativeEnumName) {
       t = 'enum';
     } else if (prop.enum) {
       t = prop.items?.every(item => Utils.isString(item)) ? 'enum' : 'tinyint';
     }
 
-    if (t === 'date') {
+    if (t === 'Date') {
       t = 'datetime';
     }
 
-    return prop.customType ?? this.platform.getMappedType(t);
+    return this.platform.getMappedType(t);
   }
 
   private initUnsigned(prop: EntityProperty): void {

packages/core/src/metadata/MetadataValidator.ts

@@ -222,9 +222,9 @@ export class MetadataValidator {
     }
 
     const prop = meta.properties[meta.versionProperty];
-    const type = prop.type.toLowerCase();
+    const type = prop.runtimeType ?? prop.columnTypes?.[0] ?? prop.type;
 
-    if (type !== 'number' && type !== 'date' && !type.startsWith('timestamp') && !type.startsWith('datetime')) {
+    if (type !== 'number' && type !== 'Date' && !type.startsWith('timestamp') && !type.startsWith('datetime')) {
       throw MetadataError.invalidVersionFieldType(meta);
     }
   }

packages/core/src/metadata/ReflectMetadataProvider.ts

@@ -25,26 +25,28 @@ export class ReflectMetadataProvider extends MetadataProvider {
   }
 
   protected initPropertyType(meta: EntityMetadata, prop: EntityProperty) {
-    let type = Reflect.getMetadata('design:type', meta.prototype, prop.name);
+    const type = Reflect.getMetadata('design:type', meta.prototype, prop.name);
 
     if (!type || (type === Object && prop.kind !== ReferenceKind.SCALAR)) {
       throw new Error(`Please provide either 'type' or 'entity' attribute in ${meta.className}.${prop.name}. If you are using decorators, ensure you have 'emitDecoratorMetadata' enabled in your tsconfig.json.`);
     }
 
-    // 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.
+    // Force mapping to UnknownType which is a string when we see just `Object`, as that often means failed inference.
+    // 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 there are explicitly provided `columnTypes`, we use those instead for the inference, this way
     // we can have things like `columnType: 'timestamp'` be respected as `type: 'Date'`.
     if (prop.kind === ReferenceKind.SCALAR && type === Object && !prop.columnTypes) {
-      type = String;
+      prop.type = 'any';
+      return;
     }
 
     prop.type = type.name;
 
     if (prop.type && ['string', 'number', 'boolean', 'array', 'object'].includes(prop.type.toLowerCase())) {
       prop.type = prop.type.toLowerCase();
     }
+
+    prop.runtimeType = prop.type as 'string';
   }
 
 }

packages/core/src/platforms/Platform.ts

@@ -397,6 +397,10 @@ export abstract class Platform {
     return this.config;
   }
 
+  getTimezone() {
+    return this.timezone;
+  }
+
   isNumericColumn(mappedType: Type<unknown>): boolean {
     return [IntegerType, SmallIntType, BigIntType].some(t => mappedType instanceof t);
   }

packages/core/src/types/ArrayType.ts

@@ -10,7 +10,7 @@ export class ArrayType<T extends string | number = string> extends Type<T[] | nu
     super();
   }
 
-  override convertToDatabaseValue(value: T[] | null, platform: Platform, context?: TransformContext | boolean): string | null {
+  override convertToDatabaseValue(value: T[] | null, platform: Platform, context?: TransformContext): string | null {
     if (!value) {
       return value as null;
     }
@@ -20,7 +20,7 @@ export class ArrayType<T extends string | number = string> extends Type<T[] | nu
     }
 
     /* istanbul ignore next */
-    if (typeof context === 'boolean' ? context : context?.fromQuery) {
+    if (context?.fromQuery) {
       return value;
     }
 

packages/core/src/types/EnumArrayType.ts

@@ -20,7 +20,7 @@ export class EnumArrayType<T extends string | number = string> extends ArrayType
     super(mapHydrator(items, hydrate));
   }
 
-  override convertToDatabaseValue(value: T[] | null, platform: Platform, context: TransformContext | boolean): string | null {
+  override convertToDatabaseValue(value: T[] | null, platform: Platform, context?: TransformContext): string | null {
     /* istanbul ignore else */
     if (Array.isArray(value) && Array.isArray(this.items)) {
       const invalid = value.filter(v => !this.items!.includes(v));

packages/core/src/types/JsonType.ts

@@ -32,4 +32,12 @@ export class JsonType extends Type<unknown, string | null> {
     return !prop.embedded || !meta.properties[prop.embedded[0]].object;
   }
 
+  override compareAsType(): string {
+    return 'any';
+  }
+
+  override get runtimeType(): string {
+    return 'object';
+  }
+
 }

packages/core/src/types/Type.ts

@@ -19,7 +19,7 @@ export abstract class Type<JSType = string, DBType = JSType> {
   /**
    * Converts a value from its JS representation to its database representation of this type.
    */
-  convertToDatabaseValue(value: JSType | DBType, platform: Platform, context?: TransformContext | boolean): DBType {
+  convertToDatabaseValue(value: JSType | DBType, platform: Platform, context?: TransformContext): DBType {
     return value as DBType;
   }
 
@@ -48,6 +48,15 @@ export abstract class Type<JSType = string, DBType = JSType> {
     return 'any';
   }
 
+  get runtimeType(): string {
+    const compareType = this.compareAsType();
+    return compareType === 'any' ? 'string' : compareType;
+  }
+
+  get name(): string {
+    return this.constructor.name;
+  }
+
   /**
    * When a value is hydrated, we convert it back to the database value to ensure comparability,
    * as often the raw database response is not the same as the `convertToDatabaseValue` result.

packages/core/src/typings.ts

@@ -268,6 +268,7 @@ export interface EntityProperty<Owner = any, Target = any> {
   name: EntityKey<Owner>;
   entity: () => EntityName<Owner>;
   type: keyof typeof types | AnyString;
+  runtimeType: 'number' | 'string' | 'boolean' | 'bigint' | 'Buffer' | 'Date';
   targetMeta?: EntityMetadata<Target>;
   columnTypes: string[];
   customType: Type<any>;

packages/core/src/unit-of-work/ChangeSetPersister.ts

@@ -11,6 +11,7 @@ import { ReferenceKind } from '../enums';
 export class ChangeSetPersister {
 
   private readonly platform = this.driver.getPlatform();
+  private readonly comparator = this.config.getComparator(this.metadata);
 
   constructor(private readonly driver: IDatabaseDriver,
               private readonly metadata: MetadataStorage,
@@ -419,18 +420,22 @@ export class ChangeSetPersister {
    */
   mapReturnedValues<T extends object>(entity: T, payload: EntityDictionary<T>, row: Dictionary | undefined, meta: EntityMetadata<T>, override = false): void {
     if (this.platform.usesReturningStatement() && row && Utils.hasObjectKeys(row)) {
-      const data = meta.props.reduce((ret, prop) => {
-        if (prop.fieldNames && row[prop.fieldNames[0]] != null && (override || entity[prop.name] == null || Utils.isRawSql(entity[prop.name]))) {
-          ret[prop.name] = row[prop.fieldNames[0]];
-        }
-
-        return ret;
-      }, {} as Dictionary);
-
-      if (Utils.hasObjectKeys(data)) {
-        this.hydrator.hydrate(entity, meta, data as EntityData<T>, this.factory, 'full', false, true);
-        Object.assign(payload, data); // merge to the changeset payload, so it gets saved to the entity snapshot
-      }
+      // FIXME
+      // const data = meta.props.reduce((ret, prop) => {
+      //   if (prop.fieldNames && row[prop.fieldNames[0]] != null && (override || entity[prop.name] == null || Utils.isRawSql(entity[prop.name]))) {
+      //     ret[prop.name] = row[prop.fieldNames[0]];
+      //   }
+      //
+      //   return ret;
+      // }, {} as Dictionary);
+      //
+      // if (Utils.hasObjectKeys(data)) {
+      //   this.hydrator.hydrate(entity, meta, data as EntityData<T>, this.factory, 'full', false, true);
+      //   Object.assign(payload, data); // merge to the changeset payload, so it gets saved to the entity snapshot
+      // }
+      const mapped = this.comparator.mapResult<T>(meta.className, row as EntityDictionary<T>);
+      this.hydrator.hydrate(entity, meta, mapped, this.factory, 'full', false, true);
+      Object.assign(payload, mapped); // merge to the changeset payload, so it gets saved to the entity snapshot
     }
   }