borinquenkid commented on code in PR #16574:
URL: https://github.com/apache/grails-core/pull/16574#discussion_r4330542052
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinderSpec.groovy:
##########
@@ -18,6 +18,7 @@
*/
package org.grails.orm.hibernate.cfg.domainbinding.secondpass
+import
org.grails.orm.hibernate.cfg.domainbinding.binder.CompositeIdentifierToManyToOneBinder
Review Comment:
Removed in cad1bf8a7f.
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinderSpec.groovy:
##########
@@ -20,6 +20,7 @@
package org.grails.orm.hibernate.cfg.domainbinding.secondpass
+import
org.grails.orm.hibernate.cfg.domainbinding.binder.CompositeIdentifierToManyToOneBinder
Review Comment:
Removed in cad1bf8a7f.
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/BidirectionalOneToManyLinkerSpec.groovy:
##########
@@ -80,4 +87,72 @@ class BidirectionalOneToManyLinkerSpec extends
HibernateGormDatastoreSpec {
!key.isForeignKeyEnabled()
!key.isConstrained()
}
+
+ void "the copied key is marked sorted so Hibernate does not apply the
identifier permutation to it again"() {
+ given:
+ def metadataContext =
getGrailsDomainBinder().getMetadataBuildingContext()
+ RootClass rootClass = new RootClass(metadataContext)
+ rootClass.setEntityName("TestEntity")
+ Table table = new Table("test_table")
+ rootClass.setTable(table)
+ Property otherSideProperty = new Property()
+ otherSideProperty.setName("owner")
+ BasicValue value = new BasicValue(metadataContext, table)
+ value.addColumn(new Column("owner_id"))
+ otherSideProperty.setValue(value)
+ rootClass.addProperty(otherSideProperty)
+ Collection collection = new Bag(metadataContext, rootClass)
+ DependantValue key = new DependantValue(metadataContext, new
Table("collection_table"), null)
+ HibernateToManyProperty otherSide = Mock(HibernateToManyProperty)
+ otherSide.getName() >> "owner"
+
+ when:
+ linker.link(collection, rootClass, key, otherSide)
+
+ then:
+ key.isSorted()
Review Comment:
Agreed: this feature only checks that the flag is set on the copied key, and
that was already true under `sortProperties()`. The feature below is the one
that tells the two apart now (see the reply there).
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/BidirectionalOneToManyLinkerSpec.groovy:
##########
@@ -80,4 +87,72 @@ class BidirectionalOneToManyLinkerSpec extends
HibernateGormDatastoreSpec {
!key.isForeignKeyEnabled()
!key.isConstrained()
}
+
+ void "the copied key is marked sorted so Hibernate does not apply the
identifier permutation to it again"() {
+ given:
+ def metadataContext =
getGrailsDomainBinder().getMetadataBuildingContext()
+ RootClass rootClass = new RootClass(metadataContext)
+ rootClass.setEntityName("TestEntity")
+ Table table = new Table("test_table")
+ rootClass.setTable(table)
+ Property otherSideProperty = new Property()
+ otherSideProperty.setName("owner")
+ BasicValue value = new BasicValue(metadataContext, table)
+ value.addColumn(new Column("owner_id"))
+ otherSideProperty.setValue(value)
+ rootClass.addProperty(otherSideProperty)
+ Collection collection = new Bag(metadataContext, rootClass)
+ DependantValue key = new DependantValue(metadataContext, new
Table("collection_table"), null)
+ HibernateToManyProperty otherSide = Mock(HibernateToManyProperty)
+ otherSide.getName() >> "owner"
+
+ when:
+ linker.link(collection, rootClass, key, otherSide)
+
+ then:
+ key.isSorted()
+ 0 * binder._
+ }
+
+ void "the to-one side is aligned with the identifier of the owner before
its columns are copied"() {
+ given:
+ def metadataContext =
getGrailsDomainBinder().getMetadataBuildingContext()
+ RootClass rootClass = new RootClass(metadataContext)
+ rootClass.setEntityName("TestEntity")
+ Table table = new Table("test_table")
+ rootClass.setTable(table)
+ ManyToOne toOneValue = new ManyToOne(metadataContext, table)
+ toOneValue.addColumn(new Column("owner_alpha"))
+ toOneValue.addColumn(new Column("owner_zeta"))
+ Property otherSideProperty = new Property()
+ otherSideProperty.setName("owner")
+ otherSideProperty.setValue(toOneValue)
+ rootClass.addProperty(otherSideProperty)
+ Collection collection = new Bag(metadataContext, rootClass)
+ DependantValue key = new DependantValue(metadataContext, new
Table("collection_table"), null)
+ HibernateCompositeIdentity compositeId =
Mock(HibernateCompositeIdentity)
+ GrailsHibernatePersistentEntity ownerEntity =
Mock(GrailsHibernatePersistentEntity) {
+ getHibernateCompositeIdentity() >> Optional.of(compositeId)
+ }
+ HibernateManyToOneProperty otherSide =
Mock(HibernateManyToOneProperty) {
+ getName() >> "owner"
+ getHibernateAssociatedEntity() >> ownerEntity
+ }
+ List<String> columnsWhenAligned = null
+
+ when:
+ linker.link(collection, rootClass, key, otherSide)
+
+ then: 'the to-one side is aligned once, before the key exists'
+ 1 * binder.alignWithReferencedIdentifier(otherSide, toOneValue,
compositeId, ownerEntity) >> {
+ columnsWhenAligned = key.columns*.name
+ }
+ columnsWhenAligned == []
+
+ and: 'the key copies the aligned columns and Hibernate leaves their
order alone'
+ key.columns*.name == ['owner_alpha', 'owner_zeta']
+ key.isSorted()
+ key.sortProperties() == null
Review Comment:
Done in cad1bf8a7f: the key now wraps an embedded `Component` prototype
whose properties are declared out of name order (`zeta`, `alpha`), so
`sortProperties()` returns a real permutation for a key the linker did not mark
sorted and `null` for one it did. The assertion fails without `setSorted(true)`
and passes with it, which is what the `and:` label claims.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/BidirectionalOneToManyLinker.java:
##########
@@ -67,6 +81,20 @@ public void link(
key.addColumn(mappingColumn);
key.getTable().addColumn(mappingColumn);
}
- key.sortProperties();
+ key.setSorted(true);
Review Comment:
Added to the class javadoc in cad1bf8a7f: marking the key sorted is
required, not only a way to skip the permutation, because
`MappingModelCreationHelper.getPropertyOrder` asserts it and otherwise maps the
columns through the identifier component's original property order again.
--
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]
