jdaugherty commented on code in PR #16617:
URL: https://github.com/apache/grails-core/pull/16617#discussion_r4257572281
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CollectionBinder.java:
##########
@@ -216,11 +216,11 @@ private void bindCollectionTable(HibernateToManyProperty
property, Collection co
private void registerSecondPass(HibernateToManyProperty property,
Collection collection) {
Review Comment:
On the limitation in the description:
`GormMappingConfigurationStrategy.getOrCreateEmbeddedEntity` returns the same
`PersistentEntity` when the embedded type is itself an `@Entity`, so an
embedded entity with a `hasMany` is always also bound standalone. Both bindings
then resolve the same join table name (`TableForManyCalculator` names it from
the property's owner) and `mappings.addTable` hands back the same `Table`,
which is why the standalone collection's key ends up with the holder's foreign
key. Agreed that is out of scope here, but could you open an issue for it and
link it from the PR description so it is tracked now that the session factory
builds?
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/EmbeddedEntityWithHasManySpec.groovy:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package grails.gorm.tests
+
+import grails.gorm.annotation.Entity
+import org.apache.grails.data.hibernate7.core.GrailsDataHibernate7TckManager
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+
+/**
+ * An entity that is also embedded in another can declare a hasMany. The
collection is bound once for the
+ * embedded component and once for the entity itself, and each needs its own
element and key.
+ */
Review Comment:
This covers the same set case as `EmbeddedEntityCollectionKindsSpec`, on a
different base class. Consider folding it into that spec so the embedded
`hasMany` scenario lives in one place.
--
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]
