fix(core): ensure propagation during hydration dont produce extra upd…

…ates Closes #3941
mikro-orm · Jan 13, 2023 · 88595bd · 88595bd
1 parent f3dbd43
commit 88595bd
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 7 deletions.
diff --git a/packages/core/src/entity/EntityHelper.ts b/packages/core/src/entity/EntityHelper.ts
@@ -1,10 +1,9 @@
 import { inspect } from 'util';
 
 import type { EntityManager } from '../EntityManager';
-import type { AnyEntity, Dictionary, EntityMetadata, EntityProperty } from '../typings';
+import type { AnyEntity, Dictionary, EntityMetadata, EntityProperty, IHydrator } from '../typings';
 import { EntityTransformer } from '../serialization/EntityTransformer';
 import { Reference } from './Reference';
-import type { Platform } from '../platforms';
 import { Utils } from '../utils/Utils';
 import { WrappedEntity } from './WrappedEntity';
 import { ReferenceType } from '../enums';
@@ -31,7 +30,7 @@ export class EntityHelper {
     EntityHelper.defineBaseProperties(meta, meta.prototype, fork);
 
     if (!meta.embeddable && !meta.virtual) {
-      EntityHelper.defineProperties(meta);
+      EntityHelper.defineProperties(meta, fork);
     }
 
     const prototype = meta.prototype as Dictionary;
@@ -78,7 +77,8 @@ export class EntityHelper {
    * First defines a setter on the prototype, once called, actual get/set handlers are registered on the instance rather
    * than on its prototype. Thanks to this we still have those properties enumerable (e.g. part of `Object.keys(entity)`).
    */
-  private static defineProperties<T>(meta: EntityMetadata<T>): void {
+  private static defineProperties<T>(meta: EntityMetadata<T>, em: EntityManager): void {
+    const hydrator = em.config.getHydrator(em.getMetadata());
     Object
       .values<EntityProperty<T>>(meta.properties)
       .forEach(prop => {
@@ -88,7 +88,7 @@ export class EntityHelper {
         if (isReference) {
           return Object.defineProperty(meta.prototype, prop.name, {
             set(val: AnyEntity) {
-              EntityHelper.defineReferenceProperty(meta, prop, this);
+              EntityHelper.defineReferenceProperty(meta, prop, this, hydrator);
               this[prop.name] = val;
             },
           });
@@ -133,7 +133,7 @@ export class EntityHelper {
     };
   }
 
-  static defineReferenceProperty<T extends object>(meta: EntityMetadata<T>, prop: EntityProperty<T>, ref: T): void {
+  static defineReferenceProperty<T extends object>(meta: EntityMetadata<T>, prop: EntityProperty<T>, ref: T, hydrator: IHydrator): void {
     Object.defineProperty(ref, prop.name, {
       get() {
         return helper(ref).__data[prop.name];
@@ -143,7 +143,14 @@ export class EntityHelper {
         const entity = Reference.unwrapReference(val ?? wrapped.__data[prop.name]);
         const old = Reference.unwrapReference(wrapped.__data[prop.name]);
         wrapped.__data[prop.name] = Reference.wrapReference(val as T, prop);
-        wrapped.__touched = true;
+
+        // when propagation from inside hydration, we set the FK to the entity data immediately
+        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;
+        }
+
         EntityHelper.propagate(meta, entity, this, prop, Reference.unwrapReference(val), old);
       },
       enumerable: true,

diff --git a/packages/core/src/hydration/Hydrator.ts b/packages/core/src/hydration/Hydrator.ts
@@ -7,6 +7,8 @@ import type { Configuration } from '../utils/Configuration';
 /* istanbul ignore next */
 export abstract class Hydrator implements IHydrator {
 
+  protected running = false;
+
   constructor(protected readonly metadata: MetadataStorage,
               protected readonly platform: Platform,
               protected readonly config: Configuration) { }
@@ -15,20 +17,28 @@ export abstract class Hydrator implements IHydrator {
    * @inheritDoc
    */
   hydrate<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, type: 'full' | 'returning' | 'reference', newEntity = false, convertCustomTypes = false, schema?: string): void {
+    this.running = true;
     const props = this.getProperties(meta, type);
 
     for (const prop of props) {
       this.hydrateProperty(entity, prop, data, factory, newEntity, convertCustomTypes);
     }
+    this.running = false;
   }
 
   /**
    * @inheritDoc
    */
   hydrateReference<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes?: boolean, schema?: string): void {
+    this.running = true;
     meta.primaryKeys.forEach(pk => {
       this.hydrateProperty<T>(entity, meta.properties[pk], data, factory, false, convertCustomTypes);
     });
+    this.running = false;
+  }
+
+  isRunning(): boolean {
+    return this.running;
   }
 
   protected getProperties<T>(meta: EntityMetadata<T>, type: 'full' | 'returning' | 'reference'): EntityProperty<T>[] {

diff --git a/packages/core/src/hydration/ObjectHydrator.ts b/packages/core/src/hydration/ObjectHydrator.ts
@@ -23,15 +23,19 @@ export class ObjectHydrator extends Hydrator {
    */
   hydrate<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, type: 'full' | 'returning' | 'reference', newEntity = false, convertCustomTypes = false, schema?: string): void {
     const hydrate = this.getEntityHydrator(meta, type);
+    this.running = true;
     Utils.callCompiledFunction(hydrate, entity, data, factory, newEntity, convertCustomTypes, schema);
+    this.running = false;
   }
 
   /**
    * @inheritDoc
    */
   hydrateReference<T>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes = false, schema?: string): void {
     const hydrate = this.getEntityHydrator(meta, 'reference');
+    this.running = true;
     Utils.callCompiledFunction(hydrate, entity, data, factory, false, convertCustomTypes, schema);
+    this.running = false;
   }
 
   /**

diff --git a/packages/core/src/typings.ts b/packages/core/src/typings.ts
@@ -758,6 +758,8 @@ export interface IHydrator {
    */
   hydrateReference<T extends object>(entity: T, meta: EntityMetadata<T>, data: EntityData<T>, factory: EntityFactory, convertCustomTypes?: boolean, schema?: string): void;
 
+  isRunning(): boolean;
+
 }
 
 export interface HydratorConstructor {

diff --git a/tests/issues/GH3941.test.ts b/tests/issues/GH3941.test.ts
@@ -0,0 +1,85 @@
+import { Collection, Entity, helper, ManyToOne, OneToMany, OneToOne, PrimaryKey, Property, Ref } from '@mikro-orm/core';
+import { MikroORM } from '@mikro-orm/sqlite';
+
+@Entity()
+class Organization {
+
+  @PrimaryKey()
+  id!: number;
+
+  @OneToOne({
+    entity: () => License,
+    ref: true,
+  })
+  license!: Ref<License>;
+
+  @OneToMany(() => Workspace, workspace => workspace.organization)
+  workspaces = new Collection<Workspace>(this);
+
+  @Property()
+  name!: string;
+
+}
+
+@Entity()
+class License {
+
+  @PrimaryKey()
+  id!: number;
+
+  @OneToOne(() => Organization, organization => organization.license)
+  organization!: Ref<Organization>;
+
+  @Property()
+  name!: string;
+
+}
+
+@Entity()
+class Workspace {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  name!: string;
+
+  @ManyToOne(() => Organization, { ref: true })
+  organization!: Ref<Organization>;
+
+}
+
+let orm: MikroORM;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    entities: [Organization, License, Workspace],
+    dbName: ':memory:',
+  });
+  await orm.schema.refreshDatabase();
+});
+
+afterAll(async () => {
+  await orm.close();
+});
+
+test('3941', async () => {
+  const organization = orm.em.create(Organization, {
+    name: 'Organization',
+    license: {
+      name: 'License',
+    },
+    workspaces: [
+      { name: 'Workspace' },
+    ],
+  });
+  await orm.em.flush();
+  orm.em.clear();
+
+  const workspace = await orm.em.findOneOrFail(Workspace, organization.workspaces[0].id);
+  const license = await orm.em.findOneOrFail(License, { organization: { workspaces: workspace } });
+
+  orm.em.getUnitOfWork().computeChangeSets();
+  expect(orm.em.getUnitOfWork().getChangeSets()).toHaveLength(0);
+  expect(helper(license.organization).__originalEntityData).toEqual({ id: 1, license: 1 });
+});