Skip to content

Commit

Permalink
fix(dml): Throw when many to many are not configured properly (#9085)
Browse files Browse the repository at this point in the history
**What**
When both sides of a many to many relationship do not define the mapped by option, it leads to misconfigured relations and a malformed SQL query. (ref. #9075)

- When both mapped by are not defined, infer look up to try to identify the missing configuration if any
- Disallow defining many to many only on one side
  • Loading branch information
adrien2p authored Sep 12, 2024
1 parent 41fb2aa commit a9b559a
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/core/types/src/dml/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export type PropertyType<T> = {
export type RelationshipOptions = {
/**
* The name of the relationship as defined in the other
* data model. This is only required by the `belongsTo`
* data model. This is only required by the `belongsTo` and `manyToMany`
* relationship method.
*/
mappedBy?: string
Expand Down
23 changes: 19 additions & 4 deletions packages/core/utils/src/dml/__tests__/entity-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2971,6 +2971,7 @@ describe("Entity builder", () => {
})
.indexes([
{
// @ts-expect-error
on: ["email", "account", "doesnotexist", "anotherdoesnotexist"],
},
])
Expand Down Expand Up @@ -4714,7 +4715,9 @@ describe("Entity builder", () => {
const user = model.define("user", {
id: model.number(),
username: model.text(),
teams: model.manyToMany(() => team),
teams: model.manyToMany(() => team, {
mappedBy: "users",
}),
})

const User = toMikroORMEntity(user)
Expand Down Expand Up @@ -4775,6 +4778,7 @@ describe("Entity builder", () => {
name: "teams",
entity: "Team",
pivotTable: "team_users",
mappedBy: "users",
},
created_at: {
reference: "scalar",
Expand Down Expand Up @@ -5686,7 +5690,9 @@ describe("Entity builder", () => {
const user = model.define("platform.user", {
id: model.number(),
username: model.text(),
teams: model.manyToMany(() => team),
teams: model.manyToMany(() => team, {
mappedBy: "users",
}),
})

const User = toMikroORMEntity(user)
Expand Down Expand Up @@ -5748,6 +5754,7 @@ describe("Entity builder", () => {
name: "teams",
entity: "Team",
pivotTable: "platform.team_users",
mappedBy: "users",
},
created_at: {
reference: "scalar",
Expand Down Expand Up @@ -5867,7 +5874,10 @@ describe("Entity builder", () => {
const user = model.define("user", {
id: model.number(),
username: model.text(),
teams: model.manyToMany(() => team, { pivotTable: "users_teams" }),
teams: model.manyToMany(() => team, {
pivotTable: "users_teams",
mappedBy: "users",
}),
})

const User = toMikroORMEntity(user)
Expand Down Expand Up @@ -5928,6 +5938,7 @@ describe("Entity builder", () => {
name: "teams",
entity: "Team",
pivotTable: "users_teams",
mappedBy: "users",
},
created_at: {
reference: "scalar",
Expand Down Expand Up @@ -6052,7 +6063,10 @@ describe("Entity builder", () => {
const user = model.define("user", {
id: model.number(),
username: model.text(),
teams: model.manyToMany(() => team, { pivotEntity: () => squad }),
teams: model.manyToMany(() => team, {
pivotEntity: () => squad,
mappedBy: "users",
}),
})

const [User, Team, Squad] = toMikroOrmEntities([user, team, squad])
Expand Down Expand Up @@ -6203,6 +6217,7 @@ describe("Entity builder", () => {
name: "teams",
entity: "Team",
pivotEntity: "TeamUsers",
mappedBy: "users",
},
created_at: {
reference: "scalar",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
BeforeCreate,
ManyToMany,
ManyToOne,
OnInit,
OneToMany,
OneToOne,
OnInit,
Property,
rel,
} from "@mikro-orm/core"
Expand All @@ -27,6 +27,66 @@ type Context = {
MANY_TO_MANY_TRACKED_RELATIONS: Record<string, boolean>
}

/**
* Validates a many to many relationship without mappedBy and checks if the other side of the relationship is defined and possesses mappedBy.
* @param MikroORMEntity
* @param relationship
* @param relatedEntity
* @param relatedModelName
*/
function validateManyToManyRelationshipWithoutMappedBy({
MikroORMEntity,
relationship,
relatedEntity,
relatedModelName,
}: {
MikroORMEntity: EntityConstructor<any>
relationship: RelationshipMetadata
relatedEntity: DmlEntity<
Record<string, PropertyType<any> | RelationshipType<any>>,
any
>
relatedModelName: string
}) {
/**
* Since we don't have the information about the other side of the
* relationship, we will try to find all the other side many to many that refers to the current entity.
* If there is any, we will try to find if at least one of them has a mappedBy.
*/
const potentialOtherSides = Object.entries(relatedEntity.schema)
.filter(([, propConfig]) => DmlManyToMany.isManyToMany(propConfig))
.filter(([prop, propConfig]) => {
const parsedProp = propConfig.parse(prop) as RelationshipMetadata
const relatedEntity =
typeof parsedProp.entity === "function"
? parsedProp.entity()
: undefined

if (!relatedEntity) {
throw new Error(
`Invalid relationship reference for "${relatedModelName}.${prop}". Make sure to define the relationship using a factory function`
)
}

return parseEntityName(relatedEntity).modelName === MikroORMEntity.name
}) as unknown as [string, RelationshipType<any>][]

if (potentialOtherSides.length) {
const hasMappedBy = potentialOtherSides.some(
([, propConfig]) => !!propConfig.parse("").mappedBy
)
if (!hasMappedBy) {
throw new Error(
`Invalid relationship reference for "${MikroORMEntity.name}.${relationship.name}". "mappedBy" should be defined on one side or the other.`
)
}
} else {
throw new Error(
`Invalid relationship reference for "${MikroORMEntity.name}.${relationship.name}". The other side of the relationship is missing.`
)
}
}

/**
* Defines has one relationship on the Mikro ORM entity.
*/
Expand Down Expand Up @@ -280,6 +340,13 @@ export function defineManyToManyRelationship(
`${MikroORMEntity.name}.${relationship.name}`
] = true
}
} else {
validateManyToManyRelationshipWithoutMappedBy({
MikroORMEntity,
relationship,
relatedEntity,
relatedModelName,
})
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,65 @@ describe("manyToMany - manyToMany", () => {
]),
})
})

it(`should fail to load the dml's if both side of the relation are missing the mappedBy options`, () => {
const team = model.define("team", {
id: model.id().primaryKey(),
name: model.text(),
users: model.manyToMany(() => user, {
pivotEntity: () => squad,
}),
})

const squad = model.define("teamUsers", {
id: model.id().primaryKey(),
user: model.belongsTo(() => user, { mappedBy: "squads" }),
squad: model.belongsTo(() => team, { mappedBy: "users" }),
})

const user = model.define("user", {
id: model.id().primaryKey(),
username: model.text(),
squads: model.manyToMany(() => team, {
pivotEntity: () => squad,
}),
})

let error!: Error
try {
;[User, Squad, Team] = toMikroOrmEntities([user, squad, team])
} catch (e) {
error = e
}

expect(error).toBeTruthy()
expect(error.message).toEqual(
'Invalid relationship reference for "User.squads". "mappedBy" should be defined on one side or the other.'
)
})

it(`should fail to load the dml's if the relation is defined only on one side`, () => {
const team = model.define("team", {
id: model.id().primaryKey(),
name: model.text(),
users: model.manyToMany(() => user),
})

const user = model.define("user", {
id: model.id().primaryKey(),
username: model.text(),
})

let error!: Error
try {
;[User, Team] = toMikroOrmEntities([user, team])
} catch (e) {
error = e
}

expect(error).toBeTruthy()
expect(error.message).toEqual(
'Invalid relationship reference for "Team.users". The other side of the relationship is missing.'
)
})
})

0 comments on commit a9b559a

Please sign in to comment.