Fix many-to-many with property-ref #1873
Conversation
fredericDelaporte
force-pushed
the
many-to-many-prop-ref
branch
from
0a1ada8 to
93237c3
Compare
| @@ -131,7 +131,7 @@ bei System.ThrowHelper.ThrowKeyNotFoundException() | |||
| bei NHibernate.Type.EntityType.LoadByUniqueKey(String entityName, String uniqueKeyPropertyName, Object key, ISessionImplementor session) in C:\Users\Armin\Projects\NHibernate\branches\2.1.x\nhibernate\src\NHibernate\Type\EntityType.cs:Zeile 552. | |||
| */ | |||
|
|
|||
| Assert.That(loadedManyA.ManyBs.Count, Is.EqualTo(3)); | |||
| Assert.That(loadedManyA.ManyBs, Has.Count.EqualTo(3).And.None.Null); | |||
There was a problem hiding this comment.
And.None.Null is required because there was two bugs: one causing an exception (the reported one) and another one, prevented by the failure, causing the children to be null with this test case.
| { | ||
| manyToMany.ReferencedPropertyName = manyToManyMapping.propertyref; | ||
| mappings.AddUniquePropertyReference(manyToMany.ReferencedEntityName, manyToMany.ReferencedPropertyName); |
There was a problem hiding this comment.
Root cause of the reported bug: a many-to-many rhs is like a one-to-many and like an actual one-to-many, it needs to promote the property-ref as a unique property reference for allowing the loader to eager load it.
| // 6.0 TODO: consider breaking again that IJoinable.SelectFragment interface for transmitting | ||
| // the OuterJoinableAssociation instead of its Joinable property. This would allow to get the | ||
| // adequate columns directly instead of re-computing them. | ||
| : ((IPropertyMapping) rhs).ToColumns(elementType.RHSUniqueKeyPropertyName); |
There was a problem hiding this comment.
Fix of the additional bug, see all the comments added in code for the rational.
fredericDelaporte
force-pushed
the
many-to-many-prop-ref
branch
from
93237c3 to
855996d
Compare
|
The other PR on which it was relying is merged, and this PR has been rebased, and is ready for review. |
|
This one is not in the 5.2 milestone yet, but I consider this as a follow up to #1872, and would rather have it in. So, As proposed on the development group, and since no continued grounded disagreement has been expressed there since one week after raised concerns being addressed, I intend to merge this PR next week (likely on Wednesday). (I am issuing this notice on PR issued before starting to apply this handling of merges. I will not do it for new PR.) |
|
Reviewing a PR which is built on top of (includes) another is really painful as it is not clear what has changed. I'll review it now. |
Fixes the second bug causing #1214.