jdaugherty commented on code in PR #16532:
URL: https://github.com/apache/grails-core/pull/16532#discussion_r4289727619
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CollectionBinder.java:
##########
@@ -212,6 +219,37 @@ private void bindCollectionTable(HibernateToManyProperty
property, Collection co
collection.setCollectionTable(
mappings.addTable(schemaName, catalogName, tableName, null,
false, metadataBuildingContext, false));
collection.setInverse(property.isBidirectional() &&
!property.isOwningSide());
+ if (collection.isInverse()) {
+ warnIfManyToManyHasNoOwningSide(property);
+ }
+ }
+
+ /**
+ * A bidirectional many-to-many is written by its owning side, which
{@code belongsTo} designates. When neither
+ * side declares {@code belongsTo}, both sides are inverse and the
relationship is never stored, so warn about it
+ * once per relationship. A Map-valued side is never inverse once its
second pass has run, because
+ * {@code MapSecondPassBinder} makes it write the join table, so a
relationship with a Map on either side is
+ * stored and gets no warning.
+ */
+ private void warnIfManyToManyHasNoOwningSide(HibernateToManyProperty
property) {
+ if (!(property instanceof HibernateManyToManyProperty manyToMany) ||
+ !manyToMany.isBidirectional() || manyToMany.isCircular()) {
Review Comment:
Non-blocking. A circular bidirectional many-to-many has the problem this
warning targets and is excluded here. I probed this on the branch with
```groovy
@Entity
class ProbeUser {
String name
Set<ProbeUser> following
Set<ProbeUser> followers
static hasMany = [following: ProbeUser, followers: ProbeUser]
static mappedBy = [following: 'followers', followers: 'following']
}
```
Both properties are `HibernateManyToManyProperty`, bidirectional,
`owningSide == false`. Two join tables are created (`probe_user_following`,
`probe_user_followers`), and after `a.addToFollowing(b); a.save(flush: true)`
both `a.following` and `b.followers` read back empty. No warning is logged.
I can see why it is excluded: "declare belongsTo on the owned side" does not
fit this shape, since `static belongsTo = ProbeUser` on the class makes both
sides owning. But the data loss then stays silent, which is what the warning
exists to prevent. Either warn for the circular shape with its own wording (the
once-per-relationship check would need to compare property names, since the
owner names tie), or keep the exclusion and say why in this Javadoc. #16542 has
the same exclusion on H5, so whichever way, both modules should match.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
