fix(core): fix extra updates for composite FKs that share a column

packages/core/src/EntityManager.ts

@@ -1267,6 +1267,38 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
    * parameter matches a property name, its value will be extracted from `data`. If no matching property exists,
    * the whole `data` parameter will be passed. This means we can also define `constructor(data: Partial<T>)` and
    * `em.create()` will pass the data into it (unless we have a property named `data` too).
+   *
+   * The parameters are strictly checked, you need to provide all required properties. You can use `OptionalProps`
+   * symbol to omit some properties from this check without making them optional. Alternatively, use `partial: true`
+   * in the options to disable the strict checks for required properties. This option has no effect on runtime.
+   */
+  create<Entity extends object>(entityName: EntityName<Entity>, data: RequiredEntityData<Entity>, options?: CreateOptions): Entity;
+
+  /**
+   * Creates new instance of given entity and populates it with given data.
+   * The entity constructor will be used unless you provide `{ managed: true }` in the options parameter.
+   * The constructor will be given parameters based on the defined constructor of the entity. If the constructor
+   * parameter matches a property name, its value will be extracted from `data`. If no matching property exists,
+   * the whole `data` parameter will be passed. This means we can also define `constructor(data: Partial<T>)` and
+   * `em.create()` will pass the data into it (unless we have a property named `data` too).
+   *
+   * The parameters are strictly checked, you need to provide all required properties. You can use `OptionalProps`
+   * symbol to omit some properties from this check without making them optional. Alternatively, use `partial: true`
+   * in the options to disable the strict checks for required properties. This option has no effect on runtime.
+   */
+  create<Entity extends object>(entityName: EntityName<Entity>, data: EntityData<Entity>, options: CreateOptions & { partial: true }): Entity;
+
+  /**
+   * Creates new instance of given entity and populates it with given data.
+   * The entity constructor will be used unless you provide `{ managed: true }` in the options parameter.
+   * The constructor will be given parameters based on the defined constructor of the entity. If the constructor
+   * parameter matches a property name, its value will be extracted from `data`. If no matching property exists,
+   * the whole `data` parameter will be passed. This means we can also define `constructor(data: Partial<T>)` and
+   * `em.create()` will pass the data into it (unless we have a property named `data` too).
+   *
+   The parameters are strictly checked, you need to provide all required properties. You can use `OptionalProps`
+   symbol to omit some properties from this check without making them optional. Alternatively, use `partial: true`
+   in the options to disable the strict checks for required properties. This option has no effect on runtime.
    */
   create<Entity extends object>(entityName: EntityName<Entity>, data: RequiredEntityData<Entity>, options: CreateOptions = {}): Entity {
     const em = this.getContext();
@@ -1902,9 +1934,14 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
 }
 
 export interface CreateOptions {
+  /** creates a managed entity instance instead, bypassing the constructor call */
   managed?: boolean;
+  /** create entity in a specific schema - alternatively, use `wrap(entity).setSchema()` */
   schema?: string;
+  /** persist the entity automatically - this is the default behavior and is also configurable globally via `persistOnCreate` option */
   persist?: boolean;
+  /** this option disables the strict typing which requires all mandatory properties to have value, it has no effect on runtime */
+  partial?: boolean;
 }
 
 export interface MergeOptions {

packages/core/src/entity/EntityHelper.ts

@@ -104,13 +104,13 @@ export class EntityHelper {
               },
               set(val) {
                 this.__helper.__data[prop.name] = val;
-                this.__helper.__touched = true;
+                this.__helper.__touched = !this.__helper.hydrator.isRunning();
               },
               enumerable: true,
               configurable: true,
             });
             this.__helper.__data[prop.name] = val;
-            this.__helper.__touched = true;
+            this.__helper.__touched = !this.__helper.hydrator.isRunning();
           },
         });
       });
@@ -156,7 +156,7 @@ export class EntityHelper {
         if (val && hydrator.isRunning() && wrapped.__originalEntityData && prop.owner) {
           wrapped.__originalEntityData[prop.name as string] = helper(wrapped.__data[prop.name]).getPrimaryKey(true);
         } else {
-          wrapped.__touched = true;
+          wrapped.__touched = !hydrator.isRunning();
         }
 
         EntityHelper.propagate(meta, entity, this, prop, Reference.unwrapReference(val), old);

packages/core/src/typings.ts

@@ -450,7 +450,7 @@ export class EntityMetadata<T = any> {
             if (val && hydrator.isRunning() && wrapped.__originalEntityData && prop.owner) {
               wrapped.__originalEntityData[prop.name as string] = val.__helper.getPrimaryKey(true);
             } else {
-              wrapped.__touched = true;
+              wrapped.__touched = !hydrator.isRunning();
             }
 
             EntityHelper.propagate(meta, entity, this, prop, Reference.unwrapReference(val), old);
@@ -470,7 +470,7 @@ export class EntityMetadata<T = any> {
         },
         set(val: unknown) {
           this.__helper.__data[prop.name] = val;
-          this.__helper.__touched = true;
+          this.__helper.__touched = !this.__helper.hydrator.isRunning();
         },
         enumerable: true,
         configurable: true,

packages/core/src/utils/Utils.ts

@@ -945,6 +945,28 @@ export class Utils {
     return false;
   }
 
+  /**
+   * Maps nested FKs from `[1, 2, 3]` to `[1, [2, 3]]`.
+   */
+  static mapFlatCompositePrimaryKey(fk: Primary<any>[], prop: EntityProperty, fieldNames = prop.fieldNames, idx = 0): Primary<any> | Primary<any>[] {
+    if (!prop.targetMeta) {
+      return fk[idx++];
+    }
+
+    const parts: Primary<any>[] = [];
+
+    for (const pk of prop.targetMeta.getPrimaryProps()) {
+      parts.push(this.mapFlatCompositePrimaryKey(fk, pk, fieldNames, idx));
+      idx += pk.fieldNames.length;
+    }
+
+    if (parts.length < 2) {
+      return parts[0];
+    }
+
+    return parts;
+  }
+
   static getGlobalStorage(namespace: string): Dictionary {
     const key = `mikro-orm-${namespace}`;
     global[key] = global[key] || {};

packages/knex/src/AbstractSqlDriver.ts

@@ -269,18 +269,22 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
           }
         });
 
-      meta2.props
-        .filter(prop => this.platform.shouldHaveColumn(prop, p.children || []))
-        .forEach(prop => {
-          if (prop.fieldNames.length > 1) { // composite keys
-            relationPojo[prop.name] = prop.fieldNames.map(name => root![`${relationAlias}__${name}`]);
-            prop.fieldNames.map(name => delete root![`${relationAlias}__${name}`]);
-          } else {
-            const alias = `${relationAlias}__${prop.fieldNames[0]}`;
-            relationPojo[prop.name] = root![alias];
-            delete root![alias];
-          }
-        });
+      const props = meta2.props.filter(prop => this.platform.shouldHaveColumn(prop, p.children || []));
+
+      for (const prop of props) {
+        if (prop.fieldNames.length > 1) { // composite keys
+          relationPojo[prop.name] = prop.fieldNames.map(name => root![`${relationAlias}__${name}`]);
+        } else {
+          const alias = `${relationAlias}__${prop.fieldNames[0]}`;
+          relationPojo[prop.name] = root![alias];
+        }
+      }
+
+      // properties can be mapped to multiple places, e.g. when sharing a column in multiple FKs,
+      // so we need to delete them after everything is mapped from given level
+      for (const prop of props) {
+        prop.fieldNames.map(name => delete root![`${relationAlias}__${name}`]);
+      }
 
       if ([ReferenceType.MANY_TO_MANY, ReferenceType.ONE_TO_MANY].includes(relation.reference)) {
         result[relation.name] = result[relation.name] || [] as unknown as T[keyof T & string];

tests/issues/GHx9.test.ts

@@ -0,0 +1,107 @@
+import { Entity, LoadStrategy, ManyToOne, OneToOne, PrimaryKey, Property, Ref, wrap } from '@mikro-orm/core';
+import { MikroORM } from '@mikro-orm/sqlite';
+import { v4 } from 'uuid';
+
+@Entity()
+class Organization {
+
+  @PrimaryKey({ columnType: 'uuid' })
+  id = v4();
+
+}
+
+@Entity()
+class Project {
+
+  @PrimaryKey({ columnType: 'uuid' })
+  id = v4();
+
+  @ManyToOne({ entity: () => Organization, ref: true, primary: true })
+  organization!: Ref<Organization>;
+
+  @Property({ length: 255 })
+  name!: string;
+
+  @OneToOne({
+    entity: () => ProjectUpdate,
+    mappedBy: 'project',
+    ref: true,
+  })
+  projectUpdate?: Ref<ProjectUpdate>;
+
+}
+
+@Entity()
+class ProjectUpdate {
+
+  @PrimaryKey({ columnType: 'uuid' })
+  id = v4();
+
+  @ManyToOne({ entity: () => Organization, ref: true, primary: true })
+  organization!: Ref<Organization>;
+
+  @OneToOne({
+    entity: () => Project,
+    ref: true,
+    joinColumns: ['project_id', 'organization_id'],
+  })
+  project!: Ref<Project>;
+
+}
+
+let orm: MikroORM;
+let org: Organization;
+let project: Project;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    dbName: ':memory:',
+    entities: [Project, Organization, ProjectUpdate],
+  });
+
+  await orm.schema.refreshDatabase();
+
+  org = new Organization();
+  project = orm.em.create(Project, {
+    organization: org,
+    name: 'init',
+  });
+
+  orm.em.create(ProjectUpdate, {
+    organization: org.id,
+    project,
+  });
+
+  await orm.em.flush();
+  orm.em.clear();
+});
+
+afterAll(async () => {
+  await orm.close();
+});
+
+test('extra updates with 1:1 relations (joined)', async () => {
+  const result = await orm.em.findOneOrFail(Project, { id: project.id, organization: org.id }, {
+    populate: ['projectUpdate'],
+    strategy: LoadStrategy.JOINED,
+  });
+
+  expect(wrap(result.projectUpdate!.$).isTouched()).toBe(false);
+  expect(wrap(result).isTouched()).toBe(false);
+
+  orm.em.getUnitOfWork().computeChangeSets();
+  expect(orm.em.getUnitOfWork().getChangeSets()).toHaveLength(0);
+});
+
+test('extra updates with 1:1 relations (select-in)', async () => {
+  const result = await orm.em.findOneOrFail(Project, { id: project.id, organization: org.id }, {
+    populate: ['projectUpdate'],
+    strategy: LoadStrategy.SELECT_IN,
+  });
+
+  expect(wrap(result.projectUpdate!.$).isTouched()).toBe(false);
+  expect(wrap(result).isTouched()).toBe(false);
+
+  orm.em.getUnitOfWork().computeChangeSets();
+  expect(orm.em.getUnitOfWork().getChangeSets()).toHaveLength(0);
+});