feat(mongo): allow using different primary key types than ObjectId

packages/core/src/entity/EntityFactory.ts

@@ -1,5 +1,5 @@
 import { Utils } from '../utils';
-import { AnyEntity, EntityData, EntityMetadata, EntityName, Primary } from '../typings';
+import { AnyEntity, EntityData, EntityMetadata, EntityName, EntityProperty, Primary } from '../typings';
 import { UnitOfWork } from '../unit-of-work';
 import { ReferenceType } from './enums';
 import { EntityManager, wrap } from '..';
@@ -23,7 +23,7 @@ export class EntityFactory {
 
     entityName = Utils.className(entityName);
     const meta = this.metadata.get(entityName);
-    meta.primaryKeys.forEach(pk => this.denormalizePrimaryKey(data, pk));
+    meta.primaryKeys.forEach(pk => this.denormalizePrimaryKey(data, pk, meta.properties[pk]));
     const entity = this.createEntity(data, meta);
     const wrapped = wrap(entity, true);
 
@@ -103,12 +103,17 @@ export class EntityFactory {
   /**
    * denormalize PK to value required by driver (e.g. ObjectId)
    */
-  private denormalizePrimaryKey<T extends AnyEntity<T>>(data: EntityData<T>, primaryKey: string): void {
+  private denormalizePrimaryKey<T extends AnyEntity<T>>(data: EntityData<T>, primaryKey: string, prop: EntityProperty<T>): void {
     const platform = this.driver.getPlatform();
     const pk = platform.getSerializedPrimaryKeyField(primaryKey);
 
     if (Utils.isDefined(data[pk], true) || Utils.isDefined(data[primaryKey], true)) {
-      const id = platform.denormalizePrimaryKey(data[pk] || data[primaryKey]);
+      let id = data[pk] || data[primaryKey];
+
+      if (prop.type.toLowerCase() === 'objectid') {
+        id = platform.denormalizePrimaryKey(id);
+      }
+
       delete data[pk];
       data[primaryKey as keyof T] = id as Primary<T> & T[keyof T];
     }

packages/mongodb/src/MongoDriver.ts

@@ -1,7 +1,23 @@
 import { ClientSession, ObjectId } from 'mongodb';
 import {
-  DatabaseDriver, EntityData, AnyEntity, FilterQuery, EntityMetadata, EntityProperty, Configuration, Utils, ReferenceType,
-  FindOneOptions, FindOptions, QueryResult, Transaction, IDatabaseDriver, EntityManager, EntityManagerType, Dictionary, ValidationError,
+  DatabaseDriver,
+  EntityData,
+  AnyEntity,
+  FilterQuery,
+  EntityMetadata,
+  EntityProperty,
+  Configuration,
+  Utils,
+  ReferenceType,
+  FindOneOptions,
+  FindOptions,
+  QueryResult,
+  Transaction,
+  IDatabaseDriver,
+  EntityManager,
+  EntityManagerType,
+  Dictionary,
+  ValidationError,
 } from '@mikro-orm/core';
 import { MongoConnection } from './MongoConnection';
 import { MongoPlatform } from './MongoPlatform';
@@ -31,7 +47,7 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {
 
   async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options: FindOneOptions<T> = { populate: [], orderBy: {} }, ctx?: Transaction<ClientSession>): Promise<T | null> {
     if (Utils.isPrimaryKey(where)) {
-      where = { _id: new ObjectId(where as string) } as FilterQuery<T>;
+      where = this.buildFilterById(entityName, where as string);
     }
 
     where = this.renameFields(entityName, where);
@@ -52,7 +68,7 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {
 
   async nativeUpdate<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, data: EntityData<T>, ctx?: Transaction<ClientSession>): Promise<QueryResult> {
     if (Utils.isPrimaryKey(where)) {
-      where = { _id: new ObjectId(where as string) } as FilterQuery<T>;
+      where = this.buildFilterById(entityName, where as string);
     }
 
     where = this.renameFields(entityName, where);
@@ -63,7 +79,7 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {
 
   async nativeDelete<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, ctx?: Transaction<ClientSession>): Promise<QueryResult> {
     if (Utils.isPrimaryKey(where)) {
-      where = { _id: new ObjectId(where as string) } as FilterQuery<T>;
+      where = this.buildFilterById(entityName, where as string);
     }
 
     where = this.renameFields(entityName, where);
@@ -243,4 +259,16 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {
     return data;
   }
 
+  private buildFilterById<T extends AnyEntity<T>>(entityName: string, id: string): FilterQuery<T> {
+    const meta = this.metadata.get(entityName);
+
+
+    if (meta.properties[meta.primaryKeys[0]].type.toLowerCase() === 'objectid') {
+      return { _id: new ObjectId(id) } as FilterQuery<T>;
+    }
+
+
+    return { _id: id } as FilterQuery<T>;
+  }
+
 }

tests/issues/GH349.test.ts

@@ -0,0 +1,127 @@
+import {
+  Entity,
+  PrimaryKey,
+  Property,
+  MikroORM,
+  ReflectMetadataProvider,
+  SerializedPrimaryKey,
+} from '@mikro-orm/core';
+import { ObjectId } from 'mongodb';
+import { MongoDriver } from '@mikro-orm/mongodb';
+
+@Entity()
+class A {
+
+  @PrimaryKey()
+  _id!: string;
+
+  @SerializedPrimaryKey()
+  id!: string;
+
+  @Property()
+  name: string;
+
+  constructor(name: string) {
+    this.name = name;
+  }
+
+}
+
+@Entity()
+class B {
+
+  @PrimaryKey()
+  _id!: ObjectId;
+
+  @SerializedPrimaryKey()
+  id!: string;
+
+  @Property()
+  name: string;
+
+  constructor(name: string) {
+    this.name = name;
+  }
+
+}
+
+describe('GH issue 349', () => {
+
+  let orm: MikroORM<MongoDriver>;
+
+  beforeAll(async () => {
+    orm = await MikroORM.init({
+      entities: [A, B],
+      clientUrl: 'mongodb://localhost:27017,localhost:27018,localhost:27019/mikro-orm-test?replicaSet=rs0',
+      type: 'mongo',
+      metadataProvider: ReflectMetadataProvider,
+      cache: { enabled: false },
+    });
+  });
+
+  afterEach(async () => {
+    await orm.em.remove(A, {});
+  });
+
+  afterAll(async () => {
+    await orm.close(true);
+  });
+
+  test(`should fetch document with uuid id type`, async () => {
+    const name = 'test';
+    const a = new A(name);
+    const uuid = '67f66459-63c5-4f27-8c59-abc382a1e5f6';
+    a._id = uuid;
+    expect(a._id).toBe(uuid);
+    await orm.em.persistAndFlush(a);
+    expect(a._id).not.toBeInstanceOf(ObjectId);
+    orm.em.clear();
+
+    const getA = await orm.em.findOneOrFail<A>(A,  a._id);
+    expect(getA!._id).not.toBeInstanceOf(ObjectId);
+    expect(getA!._id).toBe(uuid);
+    expect(getA!.id).toBe(uuid);
+
+    orm.em.clear();
+  });
+
+  test(`should fetch all documents with uuid _id type`, async () => {
+    const a1 = new A('test1');
+    const uuid1 = '67f66459-63c5-4f27-8c59-abc382a1e5f6';
+    a1._id = uuid1;
+    expect(a1._id).toBe(uuid1);
+    const a2 = new A('test2');
+    const uuid2 = 'b567730f-060f-4457-ae92-41bd25d26384';
+    a2._id = uuid2;
+    expect(a2._id).toBe(uuid2);
+    await orm.em.persistAndFlush([a1, a2]);
+    orm.em.clear();
+    const getAll = await orm.em.find<A>(A, {});
+    expect(getAll[0]._id).not.toBeInstanceOf(ObjectId);
+    expect(getAll[1]._id).not.toBeInstanceOf(ObjectId);
+    orm.em.clear();
+  });
+
+  test(`should not convert to objectId even if it can`, async () => {
+    const a1 = new A('test1');
+    const id = '5ea32a539c36ba7c62a99d60';
+    a1._id = id;
+    expect(a1._id).toBe(id);
+    await orm.em.persistAndFlush(a1);
+    orm.em.clear();
+    const getA = await orm.em.findOneOrFail(A, a1._id);
+    expect(getA._id).not.toBeInstanceOf(ObjectId);
+    orm.em.clear();
+  });
+
+  test(`should convert to objectId if type is ObjectId`, async () => {
+    const b = new B('test1');
+    await orm.em.persistAndFlush(b);
+    expect(b._id).toBeInstanceOf(ObjectId);
+    orm.em.clear();
+    const getB = await orm.em.findOneOrFail(B, b._id);
+    expect(getB._id).toBeInstanceOf(ObjectId);
+    orm.em.clear();
+  });
+
+});