fix: Handle multiple id prop generation (#8097)

* fix: Handle multiple id prop generation

* throw on duplicate id + cleanup

* fix mistakenly removed files

* fix

* fix

* passzord string

* passzord string

packages/core/utils/package.json

@@ -48,6 +48,6 @@
     "build": "rimraf dist && tsc --build",
     "watch": "tsc --build --watch",
     "test": "jest --silent --bail --maxWorkers=50% --forceExit --testPathIgnorePatterns='/integration-tests/' -- src/**/__tests__/**/*.ts",
-    "test:integration": "jest --silent --bail --maxWorkers=50% --forceExit -- src/**/integration-tests/__tests__/**/*.ts"
+    "test:integration": "jest --silent --bail --runInBand --forceExit -- src/**/integration-tests/__tests__/**/*.ts"
   }
 }

packages/core/utils/src/dml/__tests__/entity-builder.spec.ts

@@ -7,6 +7,8 @@ import {
   toMikroOrmEntities,
   toMikroORMEntity,
 } from "../helpers/create-mikro-orm-entity"
+import { User } from "@medusajs/icons"
+import { DuplicateIdPropertyError } from "../errors"
 
 describe("Entity builder", () => {
   beforeEach(() => {
@@ -548,6 +550,26 @@ describe("Entity builder", () => {
       })
     })
 
+    test("should throw on duplicate id definition", () => {
+      const user = model.define("user", {
+        id: model.id().primaryKey(),
+        uuid: model.id(),
+        name: model.text(),
+      })
+
+      let err
+      try {
+        toMikroORMEntity(user)
+      } catch (e) {
+        err = e
+      }
+
+      expect(err).toBeInstanceOf(DuplicateIdPropertyError)
+      expect(err.message).toBe(
+        "The model User can only have one usage of the id() property"
+      )
+    })
+
     test("should mark a property as searchable", () => {
       const user = model.define("user", {
         id: model.number(),

packages/core/utils/src/dml/errors.ts

@@ -0,0 +1,5 @@
+export class DuplicateIdPropertyError extends Error {
+  constructor(modelName: string) {
+    super(`The model ${modelName} can only have one usage of the id() property`)
+  }
+}

packages/core/utils/src/dml/helpers/create-mikro-orm-entity.ts

@@ -13,6 +13,8 @@ import { defineProperty } from "./entity-builder/define-property"
 import { defineRelationship } from "./entity-builder/define-relationship"
 import { parseEntityName } from "./entity-builder/parse-entity-name"
 import { applyEntityIndexes, applyIndexes } from "./mikro-orm/apply-indexes"
+import { IdProperty } from "../properties/id"
+import { DuplicateIdPropertyError } from "../errors"
 
 /**
  * Factory function to create the mikro orm entity builder. The return
@@ -62,6 +64,8 @@ export function createMikrORMEntity() {
       MANY_TO_MANY_TRACKED_REALTIONS,
     }
 
+    let hasIdAlreadyDefined = false
+
     /**
      * Processing schema fields
      */
@@ -69,6 +73,13 @@ export function createMikrORMEntity() {
       const field = property.parse(name)
 
       if ("fieldName" in field) {
+        if (IdProperty.isIdProperty(field)) {
+          if (hasIdAlreadyDefined) {
+            throw new DuplicateIdPropertyError(modelName)
+          }
+          hasIdAlreadyDefined = true
+        }
+
         defineProperty(MikroORMEntity, name, property as PropertyType<any>)
         applyIndexes(MikroORMEntity, tableName, field)
         applySearchable(MikroORMEntity, field)

packages/core/utils/src/dml/helpers/entity-builder/define-property.ts

@@ -190,7 +190,8 @@ export function defineProperty(
     /**
      * Hook to generate entity within the code
      */
-    MikroORMEntity.prototype.generateId = function () {
+    const generateIdMethodName = `generateId`
+    MikroORMEntity.prototype[generateIdMethodName] = function () {
       this[field.fieldName] = generateEntityId(
         this[field.fieldName],
         field.dataType.options?.prefix
@@ -200,8 +201,8 @@ export function defineProperty(
     /**
      * Execute hook via lifecycle decorators
      */
-    BeforeCreate()(MikroORMEntity.prototype, "generateId")
+    BeforeCreate()(MikroORMEntity.prototype, generateIdMethodName)
-    OnInit()(MikroORMEntity.prototype, "generateId")
+    OnInit()(MikroORMEntity.prototype, generateIdMethodName)
 
     return
   }

packages/core/utils/src/dml/integration-tests/__tests__/enum.spec.ts

@@ -3,41 +3,34 @@ import {
   MetadataStorage,
   MikroORM,
 } from "@mikro-orm/core"
-import { model } from "../entity-builder"
+import { model } from "../../entity-builder"
-import { toMikroOrmEntities } from "../helpers/create-mikro-orm-entity"
+import { toMikroOrmEntities } from "../../helpers/create-mikro-orm-entity"
 import { createDatabase, dropDatabase } from "pg-god"
-import { mikroOrmSerializer } from "../../dal"
+import { CustomTsMigrationGenerator, mikroOrmSerializer } from "../../../dal"
-import { FileSystem } from "../../common"
-import { join } from "path"
 import { EntityConstructor } from "@medusajs/types"
+import { pgGodCredentials } from "../utils"
+import { FileSystem } from "../../../common"
+import { join } from "path"
 
-const DB_HOST = process.env.DB_HOST
+export const fileSystem = new FileSystem(
-const DB_USERNAME = process.env.DB_USERNAME
+  join(__dirname, "../../integration-tests-migrations-enum")
-const DB_PASSWORD = process.env.DB_PASSWORD
+)
-
-const pgGodCredentials = {
-  user: DB_USERNAME,
-  password: DB_PASSWORD,
-  host: DB_HOST,
-}
-
-const fileSystem = new FileSystem(join(__dirname, "../../migrations"))
 
 describe("EntityBuilder | enum", () => {
   const dbName = "EntityBuilder-enum"
 
   let orm!: MikroORM
-  let Team: EntityConstructor<any>, User: EntityConstructor<any>
+  let User: EntityConstructor<any>
 
-  afterAll(() => {
+  afterAll(async () => {
-    fileSystem.cleanup()
+    await fileSystem.cleanup()
   })
 
   beforeEach(async () => {
     MetadataStorage.clear()
 
     const user = model.define("user", {
-      id: model.id(),
+      id: model.id().primaryKey(),
       username: model.text(),
       role: model.enum(["admin", "moderator", "editor"]),
     })
@@ -50,8 +43,14 @@ describe("EntityBuilder | enum", () => {
       entities: [User],
       tsNode: true,
       dbName,
-      debug: true,
+      password: pgGodCredentials.password,
+      host: pgGodCredentials.host,
+      user: pgGodCredentials.user,
       type: "postgresql",
+      migrations: {
+        generator: CustomTsMigrationGenerator,
+        path: fileSystem.basePath,
+      },
     })
 
     const migrator = orm.getMigrator()

packages/core/utils/src/dml/integration-tests/__tests__/many-to-many.spec.ts

@@ -1,23 +1,16 @@
 import { MetadataStorage, MikroORM } from "@mikro-orm/core"
-import { model } from "../entity-builder"
+import { model } from "../../entity-builder"
-import { toMikroOrmEntities } from "../helpers/create-mikro-orm-entity"
+import { toMikroOrmEntities } from "../../helpers/create-mikro-orm-entity"
 import { createDatabase, dropDatabase } from "pg-god"
-import { CustomTsMigrationGenerator, mikroOrmSerializer } from "../../dal"
+import { CustomTsMigrationGenerator, mikroOrmSerializer } from "../../../dal"
-import { FileSystem } from "../../common"
-import { join } from "path"
 import { EntityConstructor } from "@medusajs/types"
+import { pgGodCredentials } from "../utils"
+import { FileSystem } from "../../../common"
+import { join } from "path"
 
-const DB_HOST = process.env.DB_HOST
+export const fileSystem = new FileSystem(
-const DB_USERNAME = process.env.DB_USERNAME
+  join(__dirname, "../../integration-tests-migrations-many-to-many")
-const DB_PASSWORD = process.env.DB_PASSWORD
+)
-
-const pgGodCredentials = {
-  user: DB_USERNAME,
-  password: DB_PASSWORD,
-  host: DB_HOST,
-}
-
-const fileSystem = new FileSystem(join(__dirname, "../../migrations"))
 
 describe("manyToMany - manyToMany", () => {
   const dbName = "EntityBuilder-ManyToMany"
@@ -28,15 +21,15 @@ describe("manyToMany - manyToMany", () => {
     User: EntityConstructor<any>,
     Squad: EntityConstructor<any>
 
-  afterAll(() => {
+  afterAll(async () => {
-    fileSystem.cleanup()
+    await fileSystem.cleanup()
   })
 
   beforeEach(async () => {
     MetadataStorage.clear()
 
     const team = model.define("team", {
-      id: model.id(),
+      id: model.id().primaryKey(),
       name: model.text(),
       users: model.manyToMany(() => user, {
         pivotEntity: () => squad,
@@ -45,13 +38,13 @@ describe("manyToMany - manyToMany", () => {
     })
 
     const squad = model.define("teamUsers", {
-      id: model.id(),
+      id: model.id().primaryKey(),
       user: model.belongsTo(() => user, { mappedBy: "squads" }),
       squad: model.belongsTo(() => team, { mappedBy: "users" }),
     })
 
     const user = model.define("user", {
-      id: model.id(),
+      id: model.id().primaryKey(),
       username: model.text(),
       squads: model.manyToMany(() => team, {
         pivotEntity: () => squad,
@@ -67,10 +60,13 @@ describe("manyToMany - manyToMany", () => {
       entities: [Team, User, Squad],
       tsNode: true,
       dbName,
-      debug: true,
+      password: pgGodCredentials.password,
+      host: pgGodCredentials.host,
+      user: pgGodCredentials.user,
       type: "postgresql",
       migrations: {
         generator: CustomTsMigrationGenerator,
+        path: fileSystem.basePath,
       },
     })
 

packages/core/utils/src/dml/integration-tests/__tests__/many-to-one.spec.ts

@@ -1,23 +1,16 @@
 import { MetadataStorage, MikroORM } from "@mikro-orm/core"
-import { model } from "../entity-builder"
+import { model } from "../../entity-builder"
-import { toMikroOrmEntities } from "../helpers/create-mikro-orm-entity"
+import { toMikroOrmEntities } from "../../helpers/create-mikro-orm-entity"
 import { createDatabase, dropDatabase } from "pg-god"
-import { mikroOrmSerializer } from "../../dal"
+import { CustomTsMigrationGenerator, mikroOrmSerializer } from "../../../dal"
-import { FileSystem } from "../../common"
-import { join } from "path"
 import { EntityConstructor } from "@medusajs/types"
+import { pgGodCredentials } from "../utils"
+import { FileSystem } from "../../../common"
+import { join } from "path"
 
-const DB_HOST = process.env.DB_HOST
+export const fileSystem = new FileSystem(
-const DB_USERNAME = process.env.DB_USERNAME
+  join(__dirname, "../../integration-tests-migrations-many-to-one")
-const DB_PASSWORD = process.env.DB_PASSWORD
+)
-
-const pgGodCredentials = {
-  user: DB_USERNAME,
-  password: DB_PASSWORD,
-  host: DB_HOST,
-}
-
-const fileSystem = new FileSystem(join(__dirname, "../../migrations"))
 
 describe("manyToOne - belongTo", () => {
   const dbName = "EntityBuilder-ManyToOne"
@@ -25,21 +18,21 @@ describe("manyToOne - belongTo", () => {
   let orm!: MikroORM
   let Team: EntityConstructor<any>, User: EntityConstructor<any>
 
-  afterAll(() => {
+  afterAll(async () => {
-    fileSystem.cleanup()
+    await fileSystem.cleanup()
   })
 
   beforeEach(async () => {
     MetadataStorage.clear()
 
     const team = model.define("team", {
-      id: model.id(),
+      id: model.id().primaryKey(),
       name: model.text(),
       user: model.belongsTo(() => user, { mappedBy: "teams" }),
     })
 
     const user = model.define("user", {
-      id: model.id(),
+      id: model.id().primaryKey(),
       username: model.text(),
       teams: model.hasMany(() => team, { mappedBy: "user" }),
     })
@@ -52,8 +45,14 @@ describe("manyToOne - belongTo", () => {
       entities: [Team, User],
       tsNode: true,
       dbName,
-      debug: true,
+      password: pgGodCredentials.password,
+      host: pgGodCredentials.host,
+      user: pgGodCredentials.user,
       type: "postgresql",
+      migrations: {
+        generator: CustomTsMigrationGenerator,
+        path: fileSystem.basePath,
+      },
     })
 
     const migrator = orm.getMigrator()

packages/core/utils/src/dml/integration-tests/utils.ts

@@ -0,0 +1,9 @@
+const DB_HOST = process.env.DB_HOST ?? "localhost"
+const DB_USERNAME = process.env.DB_USERNAME ?? ""
+const DB_PASSWORD = process.env.DB_PASSWORD ?? ""
+
+export const pgGodCredentials = {
+  user: DB_USERNAME,
+  password: DB_PASSWORD,
+  host: DB_HOST,
+}

packages/core/utils/src/dml/properties/id.ts

@@ -11,7 +11,7 @@ export class IdProperty extends BaseProperty<string> {
   [IsIdProperty] = true
 
   static isIdProperty(value: any): value is IdProperty {
-    return !!value?.[IsIdProperty]
+    return !!value?.[IsIdProperty] || value?.dataType?.name === "id"
   }
 
   protected dataType: {