Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid Bidirectional Association Mapping with Mapped Superclass in Class Hierarchy #9516

Closed
bobdercole opened this issue Feb 15, 2022 · 6 comments · Fixed by #10453
Closed

Comments

@bobdercole
Copy link
Contributor

Bug Report

Q A
BC Break no
Version 2.11.1

Summary

I receive a mapping exception after defining an inverse-side association on an abstract entity. A mapped superclass extends the abstract entity. A concrete entity extends the mapped superclass.

Current behavior

ClassMetadataFactory is currently throwing an exception if there is an inverse-side association on a mapped superclass.

if ($mapping['type'] & ClassMetadata::TO_MANY && ! $mapping['isOwningSide']) {

This is correct, as stated in the documentation. However, I believe this limitation should be imposed only if the association is on the mapped superclass itself.

How to reproduce

  • Define an abstract entity (A).
  • Define a mapped superclass (B) extends A.
  • Define a concrete entity (C) extends B.
  • Define another entity (D) that has a many-to-one relationship with A.

I will follow-up with a failing test case.

Expected behavior

I think the above mapping should be valid since the association is not defined directly on the mapped superclass.

@bobdercole
Copy link
Contributor Author

Here is my failing test case: #9517.

My proposed fix is to add a condition to ClassMetadataFactory to only throw the exception if the association is directly on the mapped superclass. Maybe something like this?

if ($mapping['type'] & ClassMetadata::TO_MANY && ! $mapping['isOwningSide'] && $parentClass->name === $mapping['sourceEntity']) {
    throw MappingException::illegalToManyAssociationOnMappedSuperclass($parentClass->name, $field);
}

@mpdude
Copy link
Contributor

mpdude commented Feb 21, 2022

Related to #8415 (as per #9517).

@mpdude
Copy link
Contributor

mpdude commented Jan 23, 2023

Please check if this is already fixed in the current 2.14.x branch

@mpdude
Copy link
Contributor

mpdude commented Jan 23, 2023

You might also want to subscribe to #10398

mpdude added a commit to mpdude/doctrine2 that referenced this issue Jan 24, 2023
…ed superclass in the hierarchy

This picks the test case from doctrine#9517 and rebases it onto 2.14.x.

The problem has been covered in doctrine#8415, so this PR closes doctrine#9517 and fixes doctrine#9516.

Co-authored-by: Robert D'Ercole <bobdercole@gmail.com>
mpdude added a commit to mpdude/doctrine2 that referenced this issue Jan 24, 2023
…ed superclass in the hierarchy

This picks the test case from doctrine#9517 and rebases it onto 2.14.x.

The problem has been covered in doctrine#8415, so this PR closes doctrine#9517 and fixes doctrine#9516.

Co-authored-by: Robert D'Ercole <bobdercole@gmail.com>
@bobdercole
Copy link
Contributor Author

Please check if this is already fixed in the current 2.14.x branch

Yes! It works now. Thank you for all the work you've done to resolve it!

I suppose we should keep this open until the regression test PR is merged.

@mpdude
Copy link
Contributor

mpdude commented Jan 28, 2023

@greg0ire maybe a low hanging fruit:

Merge #10453 and then close the issues and PRs linked from it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants