fix: do not rely on model loading order to find an implicit owner (#10360)

.changeset/cold-dragons-lie.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/utils": patch
+---
+
+fix: do not rely on model loading order to find an implicit owner

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

@@ -5149,9 +5149,9 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "teams",
           entity: "Team",
-          owner: true,
+          owner: false,
           pivotTable: "team_users",
-          inversedBy: "users",
+          mappedBy: "users",
         },
         created_at: {
           reference: "scalar",
@@ -5217,9 +5217,9 @@ describe("Entity builder", () => {
         users: {
           reference: "m:n",
           name: "users",
-          mappedBy: "teams",
+          inversedBy: "teams",
           entity: "User",
-          owner: false,
+          owner: true,
           pivotTable: "team_users",
         },
         created_at: {
@@ -5330,9 +5330,9 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "teams",
           entity: "Team",
-          owner: true,
+          owner: false,
           pivotTable: "team_users",
-          inversedBy: "users",
+          mappedBy: "users",
         },
         created_at: {
           reference: "scalar",
@@ -5399,8 +5399,8 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "users",
           entity: "User",
-          owner: false,
+          owner: true,
-          mappedBy: "teams",
+          inversedBy: "teams",
           pivotTable: "team_users",
         },
         created_at: {
@@ -5545,9 +5545,9 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "teams",
           entity: "Team",
-          owner: true,
+          owner: false,
           pivotTable: "team_users",
-          inversedBy: "users",
+          mappedBy: "users",
         },
         created_at: {
           reference: "scalar",
@@ -5614,9 +5614,9 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "users",
           entity: "User",
-          owner: false,
+          owner: true,
           pivotTable: "team_users",
-          mappedBy: "teams",
+          inversedBy: "teams",
         },
         created_at: {
           reference: "scalar",
@@ -6125,9 +6125,9 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "teams",
           entity: "Team",
-          owner: true,
+          owner: false,
           pivotTable: "platform.team_users",
-          inversedBy: "users",
+          mappedBy: "users",
         },
         created_at: {
           reference: "scalar",
@@ -6195,8 +6195,8 @@ describe("Entity builder", () => {
           reference: "m:n",
           name: "users",
           entity: "User",
-          owner: false,
+          owner: true,
-          mappedBy: "teams",
+          inversedBy: "teams",
           pivotTable: "platform.team_users",
         },
         created_at: {
@@ -6719,10 +6719,10 @@ describe("Entity builder", () => {
         },
       })
 
-      const metaData = MetadataStorage.getMetadataFromDecorator(User)
+      const teamMetaData = MetadataStorage.getMetadataFromDecorator(Team)
-      expect(metaData.className).toEqual("User")
+      expect(teamMetaData.className).toEqual("Team")
-      expect(metaData.path).toEqual("User")
+      expect(teamMetaData.path).toEqual("Team")
-      expect(metaData.properties).toEqual({
+      expect(teamMetaData.properties).toEqual({
         id: {
           reference: "scalar",
           type: "number",
@@ -6733,23 +6733,23 @@ describe("Entity builder", () => {
           getter: false,
           setter: false,
         },
-        username: {
+        name: {
           reference: "scalar",
           type: "string",
           columnType: "text",
-          name: "username",
+          name: "name",
-          fieldName: "username",
+          fieldName: "name",
           nullable: false,
           getter: false,
           setter: false,
         },
-        teams: {
+        users: {
           reference: "m:n",
-          name: "teams",
+          name: "users",
-          entity: "Team",
+          entity: "User",
           owner: true,
+          inversedBy: "teams",
           pivotEntity: "TeamUsers",
-          inversedBy: "users",
         },
         created_at: {
           reference: "scalar",
@@ -6788,10 +6788,10 @@ describe("Entity builder", () => {
         },
       })
 
-      const teamMetaData = MetadataStorage.getMetadataFromDecorator(Team)
+      const metaData = MetadataStorage.getMetadataFromDecorator(User)
-      expect(teamMetaData.className).toEqual("Team")
+      expect(metaData.className).toEqual("User")
-      expect(teamMetaData.path).toEqual("Team")
+      expect(metaData.path).toEqual("User")
-      expect(teamMetaData.properties).toEqual({
+      expect(metaData.properties).toEqual({
         id: {
           reference: "scalar",
           type: "number",
@@ -6802,23 +6802,23 @@ describe("Entity builder", () => {
           getter: false,
           setter: false,
         },
-        name: {
+        username: {
           reference: "scalar",
           type: "string",
           columnType: "text",
-          name: "name",
+          name: "username",
-          fieldName: "name",
+          fieldName: "username",
           nullable: false,
           getter: false,
           setter: false,
         },
-        users: {
+        teams: {
           reference: "m:n",
-          name: "users",
+          name: "teams",
-          entity: "User",
+          entity: "Team",
           owner: false,
-          mappedBy: "teams",
           pivotEntity: "TeamUsers",
+          mappedBy: "users",
         },
         created_at: {
           reference: "scalar",
@@ -6857,5 +6857,28 @@ describe("Entity builder", () => {
         },
       })
     })
+
+    test("throw error when both sides of relationship defines the pivot table", () => {
+      const team = model.define("team", {
+        id: model.number(),
+        name: model.text(),
+        users: model.manyToMany(() => user, {
+          pivotTable: "user_teams",
+        }),
+      })
+
+      const user = model.define("user", {
+        id: model.number(),
+        username: model.text(),
+        teams: model.manyToMany(() => team, {
+          pivotTable: "team_users",
+          mappedBy: "users",
+        }),
+      })
+
+      expect(() => toMikroORMEntity(user)).toThrow(
+        `Invalid relationship reference for "User.teams". Define "pivotTable", "joinColumn", or "inverseJoinColumn" on only one side of the relationship`
+      )
+    })
   })
 })

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

@@ -15,13 +15,13 @@ import {
   Property,
   rel,
 } from "@mikro-orm/core"
-import { camelToSnakeCase, pluralize } from "../../../common"
 import { DmlEntity } from "../../entity"
-import { HasMany } from "../../relations/has-many"
 import { HasOne } from "../../relations/has-one"
-import { ManyToMany as DmlManyToMany } from "../../relations/many-to-many"
+import { HasMany } from "../../relations/has-many"
-import { applyEntityIndexes } from "../mikro-orm/apply-indexes"
 import { parseEntityName } from "./parse-entity-name"
+import { camelToSnakeCase, pluralize } from "../../../common"
+import { applyEntityIndexes } from "../mikro-orm/apply-indexes"
+import { ManyToMany as DmlManyToMany } from "../../relations/many-to-many"
 
 type Context = {
   MANY_TO_MANY_TRACKED_RELATIONS: Record<string, boolean>
@@ -375,12 +375,10 @@ export function defineManyToManyRelationship(
   >,
   {
     relatedModelName,
-    relatedTableName,
     pgSchema,
   }: {
     relatedModelName: string
     pgSchema: string | undefined
-    relatedTableName: string
   },
   { MANY_TO_MANY_TRACKED_RELATIONS }: Context
 ) {
@@ -450,11 +448,12 @@ export function defineManyToManyRelationship(
     pivotEntityName = parseEntityName(pivotEntity).modelName
   }
 
-  if (!pivotEntityName) {
+  const tableName = parseEntityName(entity).tableNameWithoutSchema
-    const { tableName } = parseEntityName(entity)
+  const relatedTableName = parseEntityName(relatedEntity).tableNameWithoutSchema
-    let tableNameWithoutSchema: string
+  const sortedTableNames = [tableName, relatedTableName].sort()
-    let relatedTableNameWithoutSchema: string
+  const otherSideRelationOptions = otherSideRelationship.parse("").options
 
+  if (!pivotEntityName) {
     /**
      * Pivot table name is created as follows (when not explicitly provided)
      *
@@ -464,25 +463,10 @@ export function defineManyToManyRelationship(
      * - And finally pluralizing the second entity name.
      */
 
-    let [schema, ...tableTokens] = tableName.split(".")
-    if (!tableTokens.length) {
-      tableNameWithoutSchema = schema
-    } else {
-      tableNameWithoutSchema = tableTokens.join(".")
-    }
-
-    const [relatedSchema, ...relatedTableTokens] = relatedTableName.split(".")
-    if (!relatedTableTokens.length) {
-      relatedTableNameWithoutSchema = relatedSchema
-    } else {
-      relatedTableNameWithoutSchema = relatedTableTokens.join(".")
-    }
-
     pivotTableName =
       relationship.options.pivotTable ??
       otherSideRelationship.parse("").options.pivotTable ??
-      [tableNameWithoutSchema, relatedTableNameWithoutSchema]
+      sortedTableNames
-        .sort()
         .map((token, index) => {
           if (index === 1) {
             return pluralize(token)
@@ -492,22 +476,51 @@ export function defineManyToManyRelationship(
         .join("_")
   }
 
-  const otherSideRelationOptions = otherSideRelationship.parse("").options
+  let isOwner: boolean | undefined = undefined
+
+  const configuresRelationship = !!(
+    joinColumn ||
+    inverseJoinColumn ||
+    relationship.options.pivotTable
+  )
+  const relatedOneConfiguresRelationship = !!(
+    otherSideRelationOptions.pivotTable ||
+    otherSideRelationOptions.joinColumn ||
+    otherSideRelationOptions.inverseJoinColumn
+  )
 
-  const isOwner =
-    !!joinColumn ||
-    !!inverseJoinColumn ||
-    !!relationship.options.pivotTable ||
   /**
-     * We can't infer it from the current entity so lets
+   * Both sides are configuring the properties that must be on one
-     * look at the otherside configuration as well to make a choice
+   * side only
    */
-    (!otherSideRelationOptions.pivotTable &&
+  if (configuresRelationship && relatedOneConfiguresRelationship) {
-      !otherSideRelationOptions.joinColumn &&
+    throw new Error(
-      !otherSideRelationOptions.inverseJoinColumn &&
+      `Invalid relationship reference for "${MikroORMEntity.name}.${relationship.name}". Define "pivotTable", "joinColumn", or "inverseJoinColumn" on only one side of the relationship`
-      !MANY_TO_MANY_TRACKED_RELATIONS[
+    )
-        `${relatedModelName}.${otherSideRelationshipProperty}`
+  }
-      ])
+
+  /**
+   * If any of the following properties are provided, we consider
+   * the current side to be the owner
+   */
+  if (configuresRelationship) {
+    isOwner = true
+  }
+
+  /**
+   * If any of the properties are provided on the other side,
+   * then we do not expect the current side to be the owner
+   */
+  if (isOwner === undefined && relatedOneConfiguresRelationship) {
+    isOwner = false
+  }
+
+  /**
+   * Finally, we consider the current side as owner, if it is
+   * the first one in alphabetical order. The same logic is
+   * applied to pivot table name as well.
+   */
+  isOwner ??= sortedTableNames[0] === tableName
 
   const mappedByProp = isOwner ? "inversedBy" : "mappedBy"
   const mappedByPropValue =

packages/core/utils/src/dml/helpers/entity-builder/parse-entity-name.ts

@@ -25,5 +25,6 @@ export function parseEntityName(entity: DmlEntity<any, any>) {
     tableName,
     modelName: upperCaseFirst(toCamelCase(parsedEntity.name)),
     pgSchema: rest.length ? pgSchema : undefined,
+    tableNameWithoutSchema: rest.length ? rest.join(".") : pgSchema,
   }
 }

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

@@ -217,13 +217,13 @@ describe("manyToMany - manyToMany", () => {
     ;[User, Team] = toMikroOrmEntities([user, team])
 
     const teamMetaData = MetadataStorage.getMetadataFromDecorator(Team)
-    expect((teamMetaData.properties as any).users.mappedBy).toBe("squads")
+    expect((teamMetaData.properties as any).users.inversedBy).toBe("squads")
-    expect((teamMetaData.properties as any).users.owner).toBe(false)
+    expect((teamMetaData.properties as any).users.owner).toBe(true)
 
     const userMetaData = MetadataStorage.getMetadataFromDecorator(User)
-    expect((userMetaData.properties as any).squads.mappedBy).not.toBeDefined()
+    expect((userMetaData.properties as any).squads.inversedBy).not.toBeDefined()
-    expect((userMetaData.properties as any).squads.inversedBy).toBe("users")
+    expect((userMetaData.properties as any).squads.mappedBy).toBe("users")
-    expect((userMetaData.properties as any).squads.owner).toBe(true)
+    expect((userMetaData.properties as any).squads.owner).toBe(false)
   })
 
   it(`should load the dml's correclty when both side of the relation are specifying the mappedBy options without pivot table`, () => {
@@ -248,14 +248,14 @@ describe("manyToMany - manyToMany", () => {
     let [User, Team] = toMikroOrmEntities([user, team])
 
     const teamMetaData = MetadataStorage.getMetadataFromDecorator(Team)
-    expect((teamMetaData.properties as any).users.mappedBy).toBe("squads")
+    expect((teamMetaData.properties as any).users.inversedBy).toBe("squads")
-    expect((teamMetaData.properties as any).users.owner).toBe(false)
+    expect((teamMetaData.properties as any).users.owner).toBe(true)
     expect((teamMetaData.properties as any).users.pivotTable).toBe("team_users")
 
     const userMetaData = MetadataStorage.getMetadataFromDecorator(User)
-    expect((userMetaData.properties as any).squads.mappedBy).not.toBeDefined()
+    expect((userMetaData.properties as any).squads.inversedBy).not.toBeDefined()
-    expect((userMetaData.properties as any).squads.inversedBy).toBe("users")
+    expect((userMetaData.properties as any).squads.mappedBy).toBe("users")
-    expect((userMetaData.properties as any).squads.owner).toBe(true)
+    expect((userMetaData.properties as any).squads.owner).toBe(false)
     expect((userMetaData.properties as any).squads.pivotTable).toBe(
       "team_users"
     )