Skip to content

Move runtime check for to-many associations in mapped superclasses to a more suitable place#10398

Closed
mpdude wants to merge 1 commit intodoctrine:2.15.xfrom
mpdude:move-mapped-superclass-runtime-check
Closed

Move runtime check for to-many associations in mapped superclasses to a more suitable place#10398
mpdude wants to merge 1 commit intodoctrine:2.15.xfrom
mpdude:move-mapped-superclass-runtime-check

Conversation

@mpdude
Copy link
Copy Markdown
Contributor

@mpdude mpdude commented Jan 13, 2023

Closing this since #10473 was merged; it removed the check altogether.


This PR moves the check that a mapped superclass does not declare to-many associations to a more suitable (IMHO) place.

It is no longer checked when another class inherits the association from the mapped superclass, but at the final runtime validation stage when loading the metadata for the mapped superclass itself.

In addition to that, can we review whether the check is correct for the case where we have

  • a base entity class with a to-many (non-owning) association
  • a mapped superclass below it
  • possibly another entity class below that

In other words, only the mapped superclass may not define to-many associations, but it is fine if an entity class above it does and when an entity class below inherits all that? My feeling is that should be fine.

@mpdude mpdude force-pushed the move-mapped-superclass-runtime-check branch from 0a790fa to e34a185 Compare January 14, 2023 22:54
@mpdude mpdude changed the base branch from 2.14.x to 2.15.x January 14, 2023 22:54
@mpdude mpdude marked this pull request as draft January 18, 2023 09:02
@mpdude
Copy link
Copy Markdown
Contributor Author

mpdude commented Jan 18, 2023

Converted to draft for the time being. Not sure if it is working correctly – test coverage is unclear, and weird side effects come into play when trying to write a test (#10417, #10449, #10450)

@mpdude
Copy link
Copy Markdown
Contributor Author

mpdude commented Jan 19, 2023

X-Ref #9702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant