refactor: add WrappedEntity.hasPrimaryKey() method

packages/core/src/EntityManager.ts

@@ -23,9 +23,9 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
   private readonly validator = new EntityValidator(this.config.get('strict'));
   private readonly repositoryMap: Dictionary<EntityRepository<AnyEntity>> = {};
   private readonly entityLoader: EntityLoader = new EntityLoader(this);
+  private readonly comparator = new EntityComparator(this.metadata, this.driver.getPlatform());
   private readonly unitOfWork = new UnitOfWork(this);
   private readonly entityFactory = new EntityFactory(this.unitOfWork, this);
-  private readonly comparator = new EntityComparator(this.metadata, this.driver.getPlatform());
   private filters: Dictionary<FilterDef<any>> = {};
   private filterParams: Dictionary<Dictionary> = {};
   private transactionContext?: Transaction;

packages/core/src/entity/ArrayCollection.ts

@@ -102,7 +102,7 @@ export class ArrayCollection<T, O> {
 
     return !!this.items.find((i: AnyEntity<T>) => {
       const objectIdentity = i === entity;
-      const primaryKeyIdentity = i.__helper!.__primaryKey && entity.__helper!.__primaryKey && i.__helper!.__serializedPrimaryKey === entity.__helper!.__serializedPrimaryKey;
+      const primaryKeyIdentity = i.__helper!.hasPrimaryKey() && entity.__helper!.hasPrimaryKey() && i.__helper!.__serializedPrimaryKey === entity.__helper!.__serializedPrimaryKey;
 
       return objectIdentity || primaryKeyIdentity;
     });

packages/core/src/entity/EntityTransformer.ts

@@ -31,7 +31,7 @@ export class EntityTransformer {
       })
       .forEach(([pk, value]) => ret[this.propertyName(meta, pk, platform)] = value as unknown as T[keyof T]);
 
-    if ((!wrapped.isInitialized() && Utils.isDefined(wrapped.__primaryKey, true)) || visited.has(entity)) {
+    if ((!wrapped.isInitialized() && wrapped.hasPrimaryKey()) || visited.has(entity)) {
       return ret;
     }
 

packages/core/src/entity/WrappedEntity.ts

@@ -70,6 +70,13 @@ export class WrappedEntity<T extends AnyEntity<T>, PK extends keyof T> {
     return this.entity;
   }
 
+  hasPrimaryKey(): boolean {
+    return this.__meta.primaryKeys.every(pk => {
+      const val = Utils.extractPK(this.entity[pk]);
+      return val !== undefined && val !== null;
+    });
+  }
+
   get __primaryKey(): Primary<T> {
     return Utils.getPrimaryKeyValue(this.entity, this.__meta.primaryKeys);
   }

packages/core/src/typings.ts

@@ -70,6 +70,7 @@ export interface IWrappedEntity<T extends AnyEntity<T>, PK extends keyof T, P =
 }
 
 export interface IWrappedEntityInternal<T extends AnyEntity<T>, PK extends keyof T, P = keyof T> extends IWrappedEntity<T, PK, P> {
+  hasPrimaryKey(): boolean;
   __meta: EntityMetadata<T>;
   __data: Dictionary;
   __em?: any; // we cannot have `EntityManager` here as that causes a cycle

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

@@ -64,9 +64,8 @@ export class ChangeSetPersister {
     const meta = this.metadata.find(changeSet.name)!;
     const wrapped = changeSet.entity.__helper!;
     const res = await this.driver.nativeInsert(changeSet.name, changeSet.payload, ctx);
-    const hasPrimaryKey = Utils.isDefined(wrapped.__primaryKey, true);
 
-    if (!hasPrimaryKey) {
+    if (!wrapped.hasPrimaryKey()) {
       this.mapPrimaryKey(meta, res.insertId, changeSet);
     }
 
@@ -90,7 +89,11 @@ export class ChangeSetPersister {
     const prop = meta.properties[meta.primaryKeys[0]];
     const insertId = prop.customType ? prop.customType.convertToJSValue(value, this.driver.getPlatform()) : value;
     const wrapped = changeSet.entity.__helper!;
-    wrapped.__primaryKey = Utils.isDefined(wrapped.__primaryKey, true) ? wrapped.__primaryKey : insertId;
+
+    if (!wrapped.hasPrimaryKey()) {
+      wrapped.__primaryKey = insertId;
+    }
+
     changeSet.payload[wrapped.__meta.primaryKeys[0]] = value;
     wrapped.__identifier!.setValue(changeSet.entity[prop.name] as unknown as IPrimaryKey);
   }

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

@@ -23,17 +23,19 @@ export class UnitOfWork {
   private readonly extraUpdates = new Set<[AnyEntity, string, AnyEntity | Reference<AnyEntity>]>();
   private readonly metadata = this.em.getMetadata();
   private readonly platform = this.em.getDriver().getPlatform();
+  private readonly eventManager = this.em.getEventManager();
+  private readonly comparator = this.em.getComparator();
   private readonly changeSetComputer = new ChangeSetComputer(this.em.getValidator(), this.collectionUpdates, this.removeStack, this.metadata, this.platform, this.em.config);
   private readonly changeSetPersister = new ChangeSetPersister(this.em.getDriver(), this.metadata, this.em.config.getHydrator(this.em.getEntityFactory(), this.em), this.em.config);
   private working = false;
 
   constructor(private readonly em: EntityManager) { }
 
-  merge<T extends AnyEntity<T>>(entity: T, visited = new Set<AnyEntity>(), mergeData = true): void {
+  merge<T extends AnyEntity<T>>(entity: T, visited = new WeakSet<AnyEntity>(), mergeData = true): void {
     const wrapped = entity.__helper!;
     wrapped.__em = this.em;
 
-    if (!Utils.isDefined(wrapped.__primaryKey, true)) {
+    if (!wrapped.hasPrimaryKey()) {
       return;
     }
 
@@ -46,7 +48,7 @@ export class UnitOfWork {
     this.identityMap.set(`${root.name}-${wrapped.__serializedPrimaryKey}`, entity);
 
     if (mergeData || !entity.__helper!.__originalEntityData) {
-      entity.__helper!.__originalEntityData = this.em.getComparator().prepareEntity(entity);
+      entity.__helper!.__originalEntityData = this.comparator.prepareEntity(entity);
     }
 
     this.cascade(entity, Cascade.MERGE, visited, { mergeData: false });
@@ -153,7 +155,7 @@ export class UnitOfWork {
     this.initIdentifier(entity);
     this.changeSets.push(cs);
     this.persistStack.delete(entity);
-    entity.__helper!.__originalEntityData = this.em.getComparator().prepareEntity(entity);
+    entity.__helper!.__originalEntityData = this.comparator.prepareEntity(entity);
   }
 
   recomputeSingleChangeSet<T extends AnyEntity<T>>(entity: T): void {
@@ -167,11 +169,11 @@ export class UnitOfWork {
 
     if (cs) {
       Object.assign(this.changeSets[idx].payload, cs.payload);
-      entity.__helper!.__originalEntityData = this.em.getComparator().prepareEntity(entity);
+      entity.__helper!.__originalEntityData = this.comparator.prepareEntity(entity);
     }
   }
 
-  persist<T extends AnyEntity<T>>(entity: T, visited = new Set<AnyEntity>(), checkRemoveStack = false): void {
+  persist<T extends AnyEntity<T>>(entity: T, visited = new WeakSet<AnyEntity>(), checkRemoveStack = false): void {
     if (this.persistStack.has(entity)) {
       return;
     }
@@ -180,21 +182,17 @@ export class UnitOfWork {
       return;
     }
 
-    if (!Utils.isDefined(entity.__helper!.__primaryKey, true)) {
-      entity.__helper!.__identifier = new EntityIdentifier();
-    }
-
     this.persistStack.add(entity);
     this.removeStack.delete(entity);
     this.cascade(entity, Cascade.PERSIST, visited, { checkRemoveStack });
   }
 
-  remove(entity: AnyEntity, visited = new Set<AnyEntity>()): void {
+  remove(entity: AnyEntity, visited = new WeakSet<AnyEntity>()): void {
     if (this.removeStack.has(entity)) {
       return;
     }
 
-    if (entity.__helper!.__primaryKey) {
+    if (entity.__helper!.hasPrimaryKey()) {
       this.removeStack.add(entity);
     }
 
@@ -208,14 +206,14 @@ export class UnitOfWork {
       throw ValidationError.cannotCommit();
     }
 
-    await this.em.getEventManager().dispatchEvent(EventType.beforeFlush, { em: this.em, uow: this });
+    await this.eventManager.dispatchEvent(EventType.beforeFlush, { em: this.em, uow: this });
     this.working = true;
     this.computeChangeSets();
-    await this.em.getEventManager().dispatchEvent(EventType.onFlush, { em: this.em, uow: this });
+    await this.eventManager.dispatchEvent(EventType.onFlush, { em: this.em, uow: this });
 
     // nothing to do, do not start transaction
     if (this.changeSets.length === 0 && this.collectionUpdates.length === 0 && this.extraUpdates.size === 0) {
-      await this.em.getEventManager().dispatchEvent(EventType.afterFlush, { em: this.em, uow: this });
+      await this.eventManager.dispatchEvent(EventType.afterFlush, { em: this.em, uow: this });
       this.postCommitCleanup();
 
       return;
@@ -231,7 +229,7 @@ export class UnitOfWork {
       await this.persistToDatabase(groups, this.em.getTransactionContext());
     }
 
-    await this.em.getEventManager().dispatchEvent(EventType.afterFlush, { em: this.em, uow: this });
+    await this.eventManager.dispatchEvent(EventType.afterFlush, { em: this.em, uow: this });
     this.postCommitCleanup();
   }
 
@@ -267,7 +265,8 @@ export class UnitOfWork {
 
     for (const entity of this.identityMap.values()) {
       if (!this.removeStack.has(entity) && !this.orphanRemoveStack.has(entity)) {
-        this.persist(entity, undefined, true);
+        this.persistStack.add(entity);
+        this.cascade(entity, Cascade.PERSIST, new WeakSet<AnyEntity>(), { checkRemoveStack: true });
       }
     }
 
@@ -293,7 +292,7 @@ export class UnitOfWork {
     this.orphanRemoveStack.delete(entity);
   }
 
-  private findNewEntities<T extends AnyEntity<T>>(entity: T, visited = new Set<AnyEntity>()): void {
+  private findNewEntities<T extends AnyEntity<T>>(entity: T, visited = new WeakSet<AnyEntity>()): void {
     if (visited.has(entity)) {
       return;
     }
@@ -308,7 +307,7 @@ export class UnitOfWork {
     this.initIdentifier(entity);
 
     for (const prop of Object.values<EntityProperty>(wrapped.__meta.properties)) {
-      const reference = this.unwrapReference(entity, prop);
+      const reference = Reference.unwrapReference(entity[prop.name]);
       this.processReference(entity, prop, reference, visited);
     }
 
@@ -317,21 +316,21 @@ export class UnitOfWork {
     if (changeSet) {
       this.changeSets.push(changeSet);
       this.persistStack.delete(entity);
-      wrapped.__originalEntityData = this.em.getComparator().prepareEntity(entity);
+      wrapped.__originalEntityData = changeSet.payload;
     }
   }
 
   private initIdentifier<T extends AnyEntity<T>>(entity: T): void {
     const wrapped = entity.__helper!;
 
-    if (Utils.isDefined(wrapped.__primaryKey, true) || wrapped.__identifier) {
+    if (wrapped.__identifier || wrapped.hasPrimaryKey()) {
       return;
     }
 
     wrapped.__identifier = new EntityIdentifier();
   }
 
-  private processReference<T extends AnyEntity<T>>(parent: T, prop: EntityProperty<T>, reference: any, visited: Set<AnyEntity>): void {
+  private processReference<T extends AnyEntity<T>>(parent: T, prop: EntityProperty<T>, reference: any, visited: WeakSet<AnyEntity>): void {
     const isToOne = prop.reference === ReferenceType.MANY_TO_ONE || prop.reference === ReferenceType.ONE_TO_ONE;
 
     if (isToOne && reference) {
@@ -343,13 +342,13 @@ export class UnitOfWork {
     }
   }
 
-  private processToOneReference<T extends AnyEntity<T>>(reference: any, visited: Set<AnyEntity>): void {
+  private processToOneReference<T extends AnyEntity<T>>(reference: any, visited: WeakSet<AnyEntity>): void {
     if (!reference.__helper!.__originalEntityData) {
       this.findNewEntities(reference, visited);
     }
   }
 
-  private processToManyReference<T extends AnyEntity<T>>(reference: Collection<AnyEntity>, visited: Set<AnyEntity>, parent: T, prop: EntityProperty<T>): void {
+  private processToManyReference<T extends AnyEntity<T>>(reference: Collection<AnyEntity>, visited: WeakSet<AnyEntity>, parent: T, prop: EntityProperty<T>): void {
     if (this.isCollectionSelfReferenced(reference, visited)) {
       this.extraUpdates.add([parent, prop.name, reference]);
       parent[prop.name as keyof T] = new Collection<AnyEntity>(parent) as unknown as T[keyof T];
@@ -363,19 +362,19 @@ export class UnitOfWork {
   }
 
   private async runHooks<T extends AnyEntity<T>>(type: EventType, changeSet: ChangeSet<T>, sync = false): Promise<unknown> {
-    const hasListeners = this.em.getEventManager().hasListeners(type, changeSet.entity);
+    const hasListeners = this.eventManager.hasListeners(type, changeSet.entity);
 
     if (!hasListeners) {
       return;
     }
 
     if (!sync) {
-      return this.em.getEventManager().dispatchEvent(type, { entity: changeSet.entity, em: this.em, changeSet });
+      return this.eventManager.dispatchEvent(type, { entity: changeSet.entity, em: this.em, changeSet });
     }
 
-    const copy = this.em.getComparator().prepareEntity(changeSet.entity) as T;
-    await this.em.getEventManager().dispatchEvent(type, { entity: changeSet.entity, em: this.em, changeSet });
-    Object.assign(changeSet.payload, this.em.getComparator().diffEntities<T>(copy, changeSet.entity));
+    const copy = this.comparator.prepareEntity(changeSet.entity) as T;
+    await this.eventManager.dispatchEvent(type, { entity: changeSet.entity, em: this.em, changeSet });
+    Object.assign(changeSet.payload, this.comparator.diffEntities<T>(copy, changeSet.entity));
   }
 
   private postCommitCleanup(): void {
@@ -388,7 +387,7 @@ export class UnitOfWork {
     this.working = false;
   }
 
-  private cascade<T extends AnyEntity<T>>(entity: T, type: Cascade, visited: Set<AnyEntity>, options: { checkRemoveStack?: boolean; mergeData?: boolean } = {}): void {
+  private cascade<T extends AnyEntity<T>>(entity: T, type: Cascade, visited: WeakSet<AnyEntity>, options: { checkRemoveStack?: boolean; mergeData?: boolean } = {}): void {
     if (visited.has(entity)) {
       return;
     }
@@ -408,14 +407,14 @@ export class UnitOfWork {
     }
   }
 
-  private cascadeReference<T extends AnyEntity<T>>(entity: T, prop: EntityProperty<T>, type: Cascade, visited: Set<AnyEntity>, options: { checkRemoveStack?: boolean; mergeData?: boolean }): void {
+  private cascadeReference<T extends AnyEntity<T>>(entity: T, prop: EntityProperty<T>, type: Cascade, visited: WeakSet<AnyEntity>, options: { checkRemoveStack?: boolean; mergeData?: boolean }): void {
     this.fixMissingReference(entity, prop);
 
     if (!this.shouldCascade(prop, type)) {
       return;
     }
 
-    const reference = this.unwrapReference(entity, prop);
+    const reference = Reference.unwrapReference(entity[prop.name]) as unknown as T | Collection<AnyEntity>;
 
     if ([ReferenceType.MANY_TO_ONE, ReferenceType.ONE_TO_ONE].includes(prop.reference) && reference) {
       return this.cascade(reference as T, type, visited, options);
@@ -432,7 +431,7 @@ export class UnitOfWork {
     }
   }
 
-  private isCollectionSelfReferenced(collection: Collection<AnyEntity>, visited: Set<AnyEntity>): boolean {
+  private isCollectionSelfReferenced(collection: Collection<AnyEntity>, visited: WeakSet<AnyEntity>): boolean {
     const filtered = collection.getItems(false).filter(item => !item.__helper!.__originalEntityData);
     return filtered.some(items => visited.has(items));
   }
@@ -476,7 +475,7 @@ export class UnitOfWork {
   }
 
   private fixMissingReference<T extends AnyEntity<T>>(entity: T, prop: EntityProperty<T>): void {
-    const reference = this.unwrapReference(entity, prop);
+    const reference = Reference.unwrapReference(entity[prop.name]);
 
     if ([ReferenceType.MANY_TO_ONE, ReferenceType.ONE_TO_ONE].includes(prop.reference) && reference && !Utils.isEntity(reference)) {
       entity[prop.name] = this.em.getReference(prop.type, reference as Primary<T[string & keyof T]>, !!prop.wrappedReference) as T[string & keyof T];
@@ -491,16 +490,6 @@ export class UnitOfWork {
     }
   }
 
-  private unwrapReference<T extends AnyEntity<T>, U extends AnyEntity | Reference<T> | Collection<AnyEntity> | Primary<T> | (AnyEntity | Primary<T>)[]>(entity: T, prop: EntityProperty<T>): U {
-    const reference = entity[prop.name] as U;
-
-    if (Reference.isReference(reference)) {
-      return reference.unwrap() as U;
-    }
-
-    return reference;
-  }
-
   private async persistToDatabase(groups: { [K in ChangeSetType]: Map<string, ChangeSet<any>[]> }, tx?: Transaction): Promise<void> {
     const commitOrder = this.getCommitOrder();
     const commitOrderReversed = [...commitOrder].reverse();

packages/core/src/utils/EntityComparator.ts

@@ -81,7 +81,7 @@ export class EntityComparator {
 
     const value = entity[prop.name];
     const collection = Utils.isCollection(value);
-    const noPkRef = Utils.isEntity<T>(value, true) && !value.__helper!.__primaryKeys.every(pk => Utils.isDefined(pk, true));
+    const noPkRef = Utils.isEntity<T>(value, true) && !value.__helper!.hasPrimaryKey();
     const noPkProp = prop.primary && !Utils.isDefined(value, true);
     const inverse = prop.reference === ReferenceType.ONE_TO_ONE && !prop.owner;
     const discriminator = prop.name === root.discriminatorColumn;

tests/Utils.test.ts

@@ -370,6 +370,10 @@ describe('Utils', () => {
     expect(name).toEqual('mikro-orm-root');
   });
 
+  test('getPrimaryKeyCond', () => {
+    expect(Utils.getPrimaryKeyCond({ a: null }, ['a'])).toBe(null);
+  });
+
   afterAll(async () => orm.close(true));
 
 });