feat(core): respect naming strategy and explicit field names on embed…

…ded properties

Closes #4371
Closes #2165
Closes #2361

.github/workflows/tests.yml

@@ -150,35 +150,15 @@ jobs:
       - name: Set CC Required env vars
         run: export GIT_BRANCH=$GITHUB_HEAD_REF && export GIT_COMMIT_SHA=$(git rev-parse origin/$GITHUB_HEAD_REF)
 
-      - name: Check for changes
-        id: changed_packages
-        run: |
-          echo "::set-output name=changed_packages::$(node ./node_modules/.bin/lerna changed -p | wc -l | xargs)"
-
-      - name: Check for changes 2
-        if: steps.changed_packages.outputs.changed_packages == '0'
-        run: |
-          echo "no changes detected by lerna"
-
       - name: Test
-        if: steps.changed_packages.outputs.changed_packages != '0'
         run: |
           yarn coverage > COVERAGE_RESULT
           echo "$(cat COVERAGE_RESULT)"
           git status && git diff
 
       - name: Codecov
-        if: steps.changed_packages.outputs.changed_packages != '0'
         uses: codecov/codecov-action@v3
 
-#      - name: Codeclimate
-#        if: steps.changed_packages.outputs.changed_packages != '0'
-#        uses: paambaati/codeclimate-action@v4.0.0
-#        env:
-#          CC_TEST_REPORTER_ID: e2a39c5dc1a13674e97e94a467bacfaec953814982c7de89e9f0b55031e43bd8
-#        with:
-#          coverageCommand: echo "$(cat COVERAGE_RESULT)"
-
       - name: Teardown docker
         run: docker-compose down
 

packages/core/src/drivers/DatabaseDriver.ts

@@ -26,7 +26,7 @@ import type {
 } from '../typings';
 import type { MetadataStorage } from '../metadata';
 import type { Connection, QueryResult, Transaction } from '../connections';
-import { EntityComparator, Utils, type Configuration, type ConnectionOptions, Cursor } from '../utils';
+import { EntityComparator, Utils, type Configuration, type ConnectionOptions, Cursor, raw } from '../utils';
 import { type QueryOrder, ReferenceKind, type QueryOrderKeys, QueryOrderNumeric } from '../enums';
 import type { Platform } from '../platforms';
 import type { Collection } from '../entity/Collection';
@@ -260,7 +260,81 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
     } as FilterQuery<T>;
   }
 
+  /** @internal */
+  mapDataToFieldNames(data: Dictionary, stringifyJsonArrays: boolean, properties?: Record<string, EntityProperty>, convertCustomTypes?: boolean, object?: boolean) {
+    if (!properties || data == null) {
+      return data;
+    }
+
+    data = Object.assign({}, data); // copy first
+
+    Object.keys(data).forEach(k => {
+      const prop = properties[k];
+
+      if (!prop) {
+        return;
+      }
+
+      if (prop.embeddedProps && !prop.object && !object) {
+        const copy = data[k];
+        delete data[k];
+        Object.assign(data, this.mapDataToFieldNames(copy, stringifyJsonArrays, prop.embeddedProps, convertCustomTypes));
+
+        return;
+      }
+
+      if (prop.embeddedProps && (prop.object || object)) {
+        const copy = data[k];
+        delete data[k];
+
+        if (prop.array) {
+          data[prop.fieldNames[0]] = copy.map((item: Dictionary) => this.mapDataToFieldNames(item, stringifyJsonArrays, prop.embeddedProps, convertCustomTypes, true));
+        } else {
+          data[prop.fieldNames[0]] = this.mapDataToFieldNames(copy, stringifyJsonArrays, prop.embeddedProps, convertCustomTypes, true);
+        }
+
+        if (stringifyJsonArrays && prop.array) {
+          data[prop.fieldNames[0]] = this.platform.convertJsonToDatabaseValue(data[prop.fieldNames[0]]);
+        }
+
+        return;
+      }
+
+      if (prop.joinColumns && Array.isArray(data[k])) {
+        const copy = Utils.flatten(data[k]);
+        delete data[k];
+        prop.joinColumns.forEach((joinColumn, idx) => data[joinColumn] = copy[idx]);
+
+        return;
+      }
+
+      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' });
+      }
+
+      if (prop.hasConvertToDatabaseValueSQL && !this.platform.isRaw(data[k])) {
+        const quoted = this.platform.quoteValue(data[k]);
+        const sql = prop.customType.convertToDatabaseValueSQL!(quoted, this.platform);
+        data[k] = raw(sql.replace(/\?/g, '\\?'));
+      }
+
+      if (!prop.customType && (Array.isArray(data[k]) || Utils.isPlainObject(data[k]))) {
+        data[k] = JSON.stringify(data[k]);
+      }
+
+      if (prop.fieldNames) {
+        Utils.renameKey(data, k, prop.fieldNames[0]);
+      }
+    });
+
+    return data;
+  }
+
   protected inlineEmbeddables<T extends object>(meta: EntityMetadata<T>, data: T, where?: boolean): void {
+    if (data == null) {
+      return;
+    }
+
     Utils.keys(data).forEach(k => {
       if (Utils.isOperator(k as string)) {
         Utils.asArray(data[k]).forEach(payload => this.inlineEmbeddables(meta, payload as T, where));

packages/core/src/metadata/MetadataDiscovery.ts

@@ -425,7 +425,10 @@ export class MetadataDiscovery {
       return;
     }
 
-    if (prop.kind === ReferenceKind.SCALAR || (prop.kind === ReferenceKind.EMBEDDED && prop.object)) {
+    if (prop.kind === ReferenceKind.SCALAR || prop.kind === ReferenceKind.EMBEDDED) {
+      prop.fieldNames = [this.namingStrategy.propertyToColumnName(prop.name)];
+    } else if (prop.embedded && prop.object) {
+      // FIXME use different method that defaults to the prop name
       prop.fieldNames = [this.namingStrategy.propertyToColumnName(prop.name)];
     } else if ([ReferenceKind.MANY_TO_ONE, ReferenceKind.ONE_TO_ONE].includes(prop.kind)) {
       prop.fieldNames = this.initManyToOneFieldName(prop, prop.name);
@@ -896,6 +899,14 @@ export class MetadataDiscovery {
         meta.properties[name].nullable = true;
       }
 
+      if (prefix) {
+        if (meta.properties[name].fieldNames) {
+          meta.properties[name].fieldNames[0] = prefix + meta.properties[name].fieldNames[0];
+        } else {
+          this.initFieldName(meta.properties[name]);
+        }
+      }
+
       const isParentObject: (prop: EntityProperty) => boolean = (prop: EntityProperty) => {
         if (prop.object) {
           return true;
@@ -917,13 +928,17 @@ export class MetadataDiscovery {
         }
 
         if (tmp === rootProperty) {
-          path.unshift(this.namingStrategy.propertyToColumnName(rootProperty.name));
+          path.unshift(rootProperty.fieldNames[0]);
+        } else if (embeddedProp.embeddedPath) {
+          path = [...embeddedProp.embeddedPath];
         } else {
           path = [embeddedProp.fieldNames[0]];
         }
 
-        path.push(prop.name);
-        meta.properties[name].fieldNames = [path.join('.')]; // store path for ObjectHydrator
+        this.initFieldName(prop);
+        path.push(prop.fieldNames[0]);
+        meta.properties[name].fieldNames = prop.fieldNames;
+        meta.properties[name].embeddedPath = path;
         const fieldName = raw(this.platform.getSearchJsonPropertySQL(path.join('->'), prop.type, true));
         meta.properties[name].fieldNameRaw = fieldName.sql; // for querying in SQL drivers
         meta.properties[name].persist = false; // only virtual as we store the whole object

packages/core/src/typings.ts

@@ -343,6 +343,7 @@ export interface EntityProperty<Owner = any, Target = any> {
   formula?: (alias: string) => string;
   prefix?: string | boolean;
   embedded?: [EntityKey<Owner>, EntityKey<Owner>];
+  embeddedPath?: string[];
   embeddable: Constructor<Owner>;
   embeddedProps: Dictionary<EntityProperty>;
   discriminatorColumn?: string; // only for poly embeddables currently

packages/core/src/utils/EntityComparator.ts

@@ -319,7 +319,7 @@ export class EntityComparator {
         return;
       }
 
-      if (prop.embedded && meta.properties[prop.embedded[0]].object && prop.runtimeType !== 'Date') {
+      if (prop.embedded && (meta.embeddable || meta.properties[prop.embedded[0]].object)) {
         return;
       }
 
@@ -329,33 +329,23 @@ export class EntityComparator {
         lines.push(`    ${propName(prop.fieldNames[0], 'mapped')} = true;`);
         lines.push(`  }`);
       } else if (prop.runtimeType === 'Date') {
-        if (prop.embedded && meta.properties[prop.embedded[0]].array) {
-          const parentKey = 'ret.' + meta.properties[prop.embedded[0]].fieldNames[0];
-          const idx = this.tmpIndex++;
-          lines.push(`  if (Array.isArray(${parentKey.replace(/\./g, '?.')})) {`);
-          lines.push(`    ${parentKey}.forEach((item_${idx}, idx_${idx}) => {`);
-          const childProp = this.wrap(prop.embedded[1]);
-          lines.push(`      if (typeof item_${idx}${childProp} !== 'undefined') {`);
-          parseDate(`${parentKey}[idx_${idx}]${childProp}`, `${parentKey}[idx_${idx}]${childProp}`, '    ');
-          lines.push(`      }`);
-          lines.push(`    });`);
-          lines.push(`  }`);
-        } else if (prop.embedded && meta.properties[prop.embedded[0]].object) {
-          const entityKey = 'ret.' + prop.fieldNames[0];
-          const entityKeyOptional = 'ret.' + prop.fieldNames[0].replace(/\./g, '?.');
-          lines.push(`  if (typeof ${entityKeyOptional} !== 'undefined') {`);
-          parseDate('ret.' + prop.fieldNames[0], entityKey);
-          lines.push(`  }`);
-        } else {
-          lines.push(`  if (typeof ${propName(prop.fieldNames[0])} !== 'undefined') {`);
-          parseDate('ret' + this.wrap(prop.name), propName(prop.fieldNames[0]));
-          lines.push(`    ${propName(prop.fieldNames[0], 'mapped')} = true;`);
-          lines.push(`  }`);
-        }
-      } else if (prop.kind === ReferenceKind.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])} : parseJsonSafe(${propName(prop.fieldNames[0])});`);
+        parseDate('ret' + this.wrap(prop.name), propName(prop.fieldNames[0]));
+        lines.push(`    ${propName(prop.fieldNames[0], 'mapped')} = true;`);
+        lines.push(`  }`);
+      } else if (prop.kind === ReferenceKind.EMBEDDED && (prop.object || meta.embeddable)) {
+        const idx = this.tmpIndex++;
+        context.set(`mapEmbeddedResult_${idx}`, (data: Dictionary) => {
+          const item = parseJsonSafe(data);
+
+          if (Array.isArray(item)) {
+            return item.map(row => row == null ? row : this.getResultMapper(prop.type)(row));
+          }
+
+          return item == null ? item : this.getResultMapper(prop.type)(item);
+        });
+        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])} : mapEmbeddedResult_${idx}(${propName(prop.fieldNames[0])});`);
         lines.push(`    ${propName(prop.fieldNames[0], 'mapped')} = true;`);
         lines.push(`  }`);
       } else {
@@ -369,21 +359,22 @@ export class EntityComparator {
 
     const code = `// compiled mapper for entity ${meta.className}\n`
       + `return function(result) {\n  const ret = {};\n${lines.join('\n')}\n  return ret;\n}`;
-    const snapshotGenerator = Utils.createFunction(context, code);
-    this.mappers.set(entityName, snapshotGenerator);
+    const resultMapper = Utils.createFunction(context, code);
+    this.mappers.set(entityName, resultMapper);
 
-    return snapshotGenerator;
+    return resultMapper;
   }
 
-  private getPropertyCondition<T>(prop: EntityProperty<T>, entityKey: string, path: string[]): string {
+  private getPropertyCondition(path: string[]): string {
     const parts = path.slice(); // copy first
 
     if (parts.length > 1) {
       parts.pop();
     }
 
     let tail = '';
-    const ret = parts
+
+    return parts
       .map(k => {
         if (k.match(/^\[idx_\d+]$/)) {
           tail += k;
@@ -397,8 +388,6 @@ export class EntityComparator {
       })
       .filter(k => k)
       .join(' && ');
-
-    return ret;
   }
 
   private getEmbeddedArrayPropertySnapshot<T>(meta: EntityMetadata<T>, prop: EntityProperty<T>, context: Map<string, any>, level: number, path: string[], dataKey: string): string {
@@ -464,7 +453,7 @@ export class EntityComparator {
     }
 
     ret += meta.props.filter(p => p.embedded?.[0] === prop.name).map(childProp => {
-      const childDataKey = prop.object ? dataKey + this.wrap(childProp.embedded![1]) : this.wrap(childProp.name);
+      const childDataKey = meta.embeddable || prop.object ? dataKey + this.wrap(childProp.embedded![1]) : this.wrap(childProp.name);
       const childEntityKey = [...path, childProp.embedded![1]].map(k => this.wrap(k)).join('');
       const childCond = `typeof entity${childEntityKey} !== 'undefined'`;
 
@@ -501,7 +490,7 @@ export class EntityComparator {
   private getPropertySnapshot<T>(meta: EntityMetadata<T>, prop: EntityProperty<T>, context: Map<string, any>, dataKey: string, entityKey: string, path: string[], level = 1, object?: boolean): string {
     const convertorKey = this.safeKey(prop.name);
     const unwrap = prop.ref ? '?.unwrap()' : '';
-    let ret = `  if (${this.getPropertyCondition(prop, entityKey, path)}) {\n`;
+    let ret = `  if (${this.getPropertyCondition(path)}) {\n`;
 
     if (['number', 'string', 'boolean'].includes(prop.type.toLowerCase())) {
       return ret + `    ret${dataKey} = entity${entityKey}${unwrap};\n  }\n`;

packages/knex/src/AbstractSqlDriver.ts

@@ -408,12 +408,25 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
     }
 
     const addParams = (prop: EntityProperty<T>, row: Dictionary) => {
+      let value = row[prop.name];
+
+      if (prop.kind === ReferenceKind.EMBEDDED && prop.object) {
+        if (prop.array) {
+          for (let i = 0; i < (value as Dictionary[]).length; i++) {
+            const item = (value as Dictionary[])[i];
+            value[i] = this.mapDataToFieldNames(item, false, prop.embeddedProps, options.convertCustomTypes);
+          }
+        } else {
+          value = this.mapDataToFieldNames(value, false, prop.embeddedProps, options.convertCustomTypes);
+        }
+      }
+
       if (options.convertCustomTypes && prop.customType) {
-        params.push(prop.customType.convertToDatabaseValue(row[prop.name], this.platform, { key: prop.name, mode: 'query-data' }));
+        params.push(prop.customType.convertToDatabaseValue(value, this.platform, { key: prop.name, mode: 'query-data' }));
         return;
       }
 
-      params.push(row[prop.name]);
+      params.push(value);
     };
 
     if (fields.length > 0 || this.platform.usesDefaultKeyword()) {
@@ -564,10 +577,26 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
         }
       });
     });
+
     const pkCond = Utils.flatten(meta.primaryKeys.map(pk => meta.properties[pk].fieldNames)).map(pk => `${this.platform.quoteIdentifier(pk)} = ?`).join(' and ');
     const params: any[] = [];
     let sql = `update ${this.getTableName(meta, options)} set `;
 
+    const addParams = (prop: EntityProperty<T>, value: Dictionary) => {
+      if (prop.kind === ReferenceKind.EMBEDDED && prop.object) {
+        if (prop.array) {
+          for (let i = 0; i < (value as Dictionary[]).length; i++) {
+            const item = (value as Dictionary[])[i];
+            value[i] = this.mapDataToFieldNames(item, false, prop.embeddedProps, options.convertCustomTypes);
+          }
+        } else {
+          value = this.mapDataToFieldNames(value, false, prop.embeddedProps, options.convertCustomTypes);
+        }
+      }
+
+      params.push(value);
+    };
+
     keys.forEach(key => {
       const prop = meta.properties[key];
 
@@ -584,7 +613,8 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
               sql += '?';
             }
 
-            params.push(...pks, prop.fieldNames.length > 1 ? data[idx][key]?.[fieldNameIdx] : data[idx][key]);
+            params.push(...pks);
+            addParams(prop, prop.fieldNames.length > 1 ? data[idx][key]?.[fieldNameIdx] : data[idx][key]);
           }
         });
         sql += ` else ${this.platform.quoteIdentifier(fieldName)} end, `;
@@ -808,7 +838,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
     const map: Dictionary<T[]> = {};
     const pkProps = ownerMeta.getPrimaryProps();
     owners.forEach(owner => {
-      const key = Utils.getPrimaryKeyHash(prop.joinColumns.map((col, idx) => {
+      const key = Utils.getPrimaryKeyHash(prop.joinColumns.map((_col, idx) => {
         const pkProp = pkProps[idx];
         return pkProp.customType ? pkProp.customType.convertToJSValue(owner[idx], this.platform) : owner[idx];
       }));