This is an automated email from the ASF dual-hosted git repository.

borinquenkid pushed a commit to branch 8.0.x-hibernate7-dev
in repository https://gitbox.apache.org/repos/asf/grails-core.git

commit 71504251357966821cf653a66d3f8952fe24fc6d
Author: Walter Duque de Estrada <[email protected]>
AuthorDate: Tue Mar 3 10:55:17 2026 -0600

    refactor: extract bindCollectionKey from CollectionSecondPassBinder into 
CollectionKeyBinder
---
 .../cfg/domainbinding/binder/CollectionBinder.java |   8 +-
 .../secondpass/CollectionKeyBinder.java            |  70 ++++++++
 .../secondpass/CollectionSecondPassBinder.java     |  44 +----
 .../secondpass/CollectionKeyBinderSpec.groovy      | 190 +++++++++++++++++++++
 .../CollectionSecondPassBinderSpec.groovy          |   2 +-
 5 files changed, 270 insertions(+), 44 deletions(-)

diff --git 
a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CollectionBinder.java
 
b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CollectionBinder.java
index 174e31d7da..f8ec48d3fd 100644
--- 
a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CollectionBinder.java
+++ 
b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CollectionBinder.java
@@ -30,6 +30,7 @@ import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentP
 import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateToManyProperty;
 import 
org.grails.orm.hibernate.cfg.domainbinding.secondpass.BidirectionalOneToManyLinker;
 import 
org.grails.orm.hibernate.cfg.domainbinding.secondpass.CollectionKeyColumnUpdater;
+import 
org.grails.orm.hibernate.cfg.domainbinding.secondpass.CollectionKeyBinder;
 import 
org.grails.orm.hibernate.cfg.domainbinding.secondpass.CollectionMultiTenantFilterBinder;
 import 
org.grails.orm.hibernate.cfg.domainbinding.secondpass.CollectionOrderByBinder;
 import 
org.grails.orm.hibernate.cfg.domainbinding.secondpass.CollectionSecondPassBinder;
@@ -110,12 +111,13 @@ public class CollectionBinder {
             manyToOneBinder,
             new PrimaryKeyValueCreator(metadataBuildingContext),
             new CollectionKeyColumnUpdater(),
-            new BidirectionalOneToManyLinker(grailsPropertyResolver),
-            new DependentKeyValueBinder(simpleValueBinder, 
compositeIdentifierToManyToOneBinder),
             new UnidirectionalOneToManyBinder(collectionWithJoinTableBinder, 
mappings),
             collectionWithJoinTableBinder,
             collectionForPropertyConfigBinder,
-            simpleValueColumnBinder,
+            new CollectionKeyBinder(
+                new BidirectionalOneToManyLinker(grailsPropertyResolver),
+                new DependentKeyValueBinder(simpleValueBinder, 
compositeIdentifierToManyToOneBinder),
+                simpleValueColumnBinder),
             new CollectionOrderByBinder(),
             new CollectionMultiTenantFilterBinder(new 
DefaultColumnNameFetcher(namingStrategy)));
     this.listSecondPassBinder =
diff --git 
a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinder.java
 
b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinder.java
new file mode 100644
index 0000000000..49d74fa661
--- /dev/null
+++ 
b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinder.java
@@ -0,0 +1,70 @@
+/*
+ *  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 org.grails.orm.hibernate.cfg.domainbinding.secondpass;
+
+import java.util.Map;
+import org.grails.datastore.mapping.model.types.ToOne;
+import 
org.grails.orm.hibernate.cfg.domainbinding.binder.SimpleValueColumnBinder;
+import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateManyToManyProperty;
+import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateToManyProperty;
+import org.hibernate.mapping.Collection;
+import org.hibernate.mapping.DependantValue;
+import org.hibernate.mapping.PersistentClass;
+
+/** Binds the collection key value for a to-many association. */
+public class CollectionKeyBinder {
+
+  private final BidirectionalOneToManyLinker bidirectionalOneToManyLinker;
+  private final DependentKeyValueBinder dependentKeyValueBinder;
+  private final SimpleValueColumnBinder simpleValueColumnBinder;
+
+  /** Creates a new {@link CollectionKeyBinder} instance. */
+  public CollectionKeyBinder(
+      BidirectionalOneToManyLinker bidirectionalOneToManyLinker,
+      DependentKeyValueBinder dependentKeyValueBinder,
+      SimpleValueColumnBinder simpleValueColumnBinder) {
+    this.bidirectionalOneToManyLinker = bidirectionalOneToManyLinker;
+    this.dependentKeyValueBinder = dependentKeyValueBinder;
+    this.simpleValueColumnBinder = simpleValueColumnBinder;
+  }
+
+  /** Binds the collection key for the given property and collection. */
+  public void bind(
+      HibernateToManyProperty property,
+      DependantValue key,
+      PersistentClass associatedClass,
+      Collection collection) {
+    if (property.isBidirectional()) {
+      var inverseSide = property.getHibernateInverseSide();
+      if (inverseSide instanceof ToOne && property.shouldBindWithForeignKey()) 
{
+        bidirectionalOneToManyLinker.link(collection, associatedClass, key, 
inverseSide);
+      } else if (inverseSide instanceof HibernateManyToManyProperty
+          || Map.class.isAssignableFrom(property.getType())) {
+        dependentKeyValueBinder.bind(property, key);
+      }
+    } else {
+      if (property.getMappedForm().hasJoinKeyMapping()) {
+        simpleValueColumnBinder.bindSimpleValue(
+            key, "long", 
property.getMappedForm().getJoinTable().getKey().getName(), true);
+      } else {
+        dependentKeyValueBinder.bind(property, key);
+      }
+    }
+  }
+}
diff --git 
a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java
 
b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java
index 69356c87f2..c413e9eeb2 100644
--- 
a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java
+++ 
b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java
@@ -25,7 +25,6 @@ import java.util.*;
 import java.util.Map;
 import 
org.grails.orm.hibernate.cfg.domainbinding.binder.CollectionForPropertyConfigBinder;
 import org.grails.orm.hibernate.cfg.domainbinding.binder.ManyToOneBinder;
-import 
org.grails.orm.hibernate.cfg.domainbinding.binder.SimpleValueColumnBinder;
 import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateManyToManyProperty;
 import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateOneToManyProperty;
 import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateToManyProperty;
@@ -34,49 +33,38 @@ import org.hibernate.MappingException;
 import org.hibernate.boot.spi.InFlightMetadataCollector;
 import org.hibernate.mapping.*;
 import org.hibernate.mapping.Collection;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /** Refactored from CollectionBinder to handle collection second pass binding. 
*/
 @SuppressWarnings("PMD.DataflowAnomalyAnalysis")
 public class CollectionSecondPassBinder {
-
-  private static final Logger LOG = 
LoggerFactory.getLogger(CollectionSecondPassBinder.class);
-
   private final CollectionOrderByBinder collectionOrderByBinder;
   private final CollectionMultiTenantFilterBinder 
collectionMultiTenantFilterBinder;
+  private final CollectionKeyBinder collectionKeyBinder;
   private final ManyToOneBinder manyToOneBinder;
   private final PrimaryKeyValueCreator primaryKeyValueCreator;
   private final CollectionKeyColumnUpdater collectionKeyColumnUpdater;
-  private final BidirectionalOneToManyLinker bidirectionalOneToManyLinker;
-  private final DependentKeyValueBinder dependentKeyValueBinder;
   private final UnidirectionalOneToManyBinder unidirectionalOneToManyBinder;
   private final CollectionWithJoinTableBinder collectionWithJoinTableBinder;
   private final CollectionForPropertyConfigBinder 
collectionForPropertyConfigBinder;
-  private final SimpleValueColumnBinder simpleValueColumnBinder;
 
   /** Creates a new {@link CollectionSecondPassBinder} instance. */
   public CollectionSecondPassBinder(
       ManyToOneBinder manyToOneBinder,
       PrimaryKeyValueCreator primaryKeyValueCreator,
       CollectionKeyColumnUpdater collectionKeyColumnUpdater,
-      BidirectionalOneToManyLinker bidirectionalOneToManyLinker,
-      DependentKeyValueBinder dependentKeyValueBinder,
       UnidirectionalOneToManyBinder unidirectionalOneToManyBinder,
       CollectionWithJoinTableBinder collectionWithJoinTableBinder,
       CollectionForPropertyConfigBinder collectionForPropertyConfigBinder,
-      SimpleValueColumnBinder simpleValueColumnBinder,
+      CollectionKeyBinder collectionKeyBinder,
       CollectionOrderByBinder collectionOrderByBinder,
       CollectionMultiTenantFilterBinder collectionMultiTenantFilterBinder) {
     this.manyToOneBinder = manyToOneBinder;
     this.primaryKeyValueCreator = primaryKeyValueCreator;
     this.collectionKeyColumnUpdater = collectionKeyColumnUpdater;
-    this.bidirectionalOneToManyLinker = bidirectionalOneToManyLinker;
-    this.dependentKeyValueBinder = dependentKeyValueBinder;
     this.unidirectionalOneToManyBinder = unidirectionalOneToManyBinder;
     this.collectionWithJoinTableBinder = collectionWithJoinTableBinder;
     this.collectionForPropertyConfigBinder = collectionForPropertyConfigBinder;
-    this.simpleValueColumnBinder = simpleValueColumnBinder;
+    this.collectionKeyBinder = collectionKeyBinder;
     this.collectionOrderByBinder = collectionOrderByBinder;
     this.collectionMultiTenantFilterBinder = collectionMultiTenantFilterBinder;
   }
@@ -99,7 +87,7 @@ public class CollectionSecondPassBinder {
 
     DependantValue key = 
primaryKeyValueCreator.createPrimaryKeyValue(collection);
     collection.setKey(key);
-    bindCollectionKey(property, key, associatedClass, collection);
+    collectionKeyBinder.bind(property, key, associatedClass, collection);
 
     Optional.ofNullable(property.getMappedForm().getCache())
         .ifPresent(cacheConfig -> 
collection.setCacheConcurrencyStrategy(cacheConfig.getUsage()));
@@ -124,30 +112,6 @@ public class CollectionSecondPassBinder {
   }
 
 
-  private void bindCollectionKey(
-      HibernateToManyProperty property,
-      DependantValue key,
-      PersistentClass associatedClass,
-      Collection collection) {
-    if (property.isBidirectional()) {
-      var inverseSide = property.getHibernateInverseSide();
-      if (inverseSide instanceof org.grails.datastore.mapping.model.types.ToOne
-          && property.shouldBindWithForeignKey()) {
-        bidirectionalOneToManyLinker.link(collection, associatedClass, key, 
inverseSide);
-      } else if (inverseSide instanceof HibernateManyToManyProperty
-          || java.util.Map.class.isAssignableFrom(property.getType())) {
-        dependentKeyValueBinder.bind(property, key);
-      }
-    } else {
-      if (property.getMappedForm().hasJoinKeyMapping()) {
-        simpleValueColumnBinder.bindSimpleValue(
-            key, "long", 
property.getMappedForm().getJoinTable().getKey().getName(), true);
-      } else {
-        dependentKeyValueBinder.bind(property, key);
-      }
-    }
-  }
-
   private void bindCollectionElement(
       HibernateToManyProperty property,
       InFlightMetadataCollector mappings,
diff --git 
a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinderSpec.groovy
 
b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinderSpec.groovy
new file mode 100644
index 0000000000..12d2881e03
--- /dev/null
+++ 
b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionKeyBinderSpec.groovy
@@ -0,0 +1,190 @@
+package org.grails.orm.hibernate.cfg.domainbinding.secondpass
+
+import grails.gorm.annotation.Entity
+import grails.gorm.specs.HibernateGormDatastoreSpec
+import 
org.grails.orm.hibernate.cfg.domainbinding.binder.CompositeIdentifierToManyToOneBinder
+import org.grails.orm.hibernate.cfg.domainbinding.binder.ManyToOneValuesBinder
+import org.grails.orm.hibernate.cfg.domainbinding.binder.SimpleValueBinder
+import 
org.grails.orm.hibernate.cfg.domainbinding.binder.SimpleValueColumnBinder
+import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity
+import 
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateToManyProperty
+import org.grails.orm.hibernate.cfg.domainbinding.util.GrailsPropertyResolver
+import org.hibernate.mapping.Bag
+import org.hibernate.mapping.BasicValue
+import org.hibernate.mapping.Column
+import org.hibernate.mapping.DependantValue
+import org.hibernate.mapping.Property
+import org.hibernate.mapping.RootClass
+import org.hibernate.mapping.Table
+import spock.lang.Subject
+
+class CollectionKeyBinderSpec extends HibernateGormDatastoreSpec {
+
+    @Subject
+    CollectionKeyBinder binder
+
+    void setupSpec() {
+        manager.addAllDomainClasses([
+            CKBBidOwner,
+            CKBBidItem,
+            CKBManyToManyOwner,
+            CKBManyToManyItem,
+            CKBUniOwner,
+            CKBUniItem,
+            CKBJoinKeyOwner,
+            CKBJoinKeyItem,
+        ])
+    }
+
+    void setup() {
+        def gdb = getGrailsDomainBinder()
+        def mbc = gdb.getMetadataBuildingContext()
+        def ns = gdb.getNamingStrategy()
+        def je = gdb.getJdbcEnvironment()
+        def svb = new SimpleValueBinder(mbc, ns, je)
+        def citmto = new CompositeIdentifierToManyToOneBinder(mbc, ns, je)
+        def botml = new BidirectionalOneToManyLinker(new 
GrailsPropertyResolver())
+        def dkvb = new DependentKeyValueBinder(svb, citmto)
+        def svcb = new SimpleValueColumnBinder()
+        binder = new CollectionKeyBinder(botml, dkvb, svcb)
+    }
+
+    private HibernateToManyProperty propertyFor(Class ownerClass, String name 
= "items") {
+        (getPersistentEntity(ownerClass) as 
GrailsHibernatePersistentEntity).getPropertyByName(name) as 
HibernateToManyProperty
+    }
+
+    private RootClass rootClassWith(String entityName, String propName, String 
columnName) {
+        def mbc = getGrailsDomainBinder().getMetadataBuildingContext()
+        def rootClass = new RootClass(mbc)
+        rootClass.setEntityName(entityName)
+        def table = new Table("test", entityName.toLowerCase())
+        def simpleValue = new BasicValue(mbc, table)
+        simpleValue.setTypeName("long")
+        simpleValue.addColumn(new Column(columnName))
+        def prop = new Property()
+        prop.setName(propName)
+        prop.setValue(simpleValue)
+        rootClass.addProperty(prop)
+        return rootClass
+    }
+
+    private DependantValue keyWithTable(String tableName = "test_table") {
+        def mbc = getGrailsDomainBinder().getMetadataBuildingContext()
+        def table = new Table("test", tableName)
+        def wrapped = new BasicValue(mbc, table)
+        return new DependantValue(mbc, table, wrapped)
+    }
+
+    def "bind sets collection inverse for bidirectional one-to-many with 
foreign key"() {
+        given:
+        def property = propertyFor(CKBBidOwner)
+        def associatedClass = rootClassWith(CKBBidItem.name, "owner", 
"OWNER_ID")
+        def key = keyWithTable("ckb_bid_item")
+        def collection = new 
Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null)
+
+        when:
+        binder.bind(property, key, associatedClass, collection)
+
+        then:
+        collection.isInverse()
+        key.getColumnSpan() > 0
+    }
+
+    def "bind delegates to dependentKeyValueBinder for bidirectional 
many-to-many"() {
+        given:
+        def property = propertyFor(CKBManyToManyOwner)
+        def key = new 
DependantValue(getGrailsDomainBinder().getMetadataBuildingContext(), null, null)
+        def associatedClass = new 
RootClass(getGrailsDomainBinder().getMetadataBuildingContext())
+        def collection = new 
Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null)
+
+        when:
+        binder.bind(property, key, associatedClass, collection)
+
+        then:
+        key.getColumnSpan() > 0
+        !collection.isInverse()
+    }
+
+    def "bind uses simpleValueColumnBinder for unidirectional with join key 
mapping"() {
+        given:
+        def property = propertyFor(CKBJoinKeyOwner)
+        def key = keyWithTable("ckb_join_key_owner_ckb_join_key_item")
+        def associatedClass = new 
RootClass(getGrailsDomainBinder().getMetadataBuildingContext())
+        def collection = new 
Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null)
+
+        when:
+        binder.bind(property, key, associatedClass, collection)
+
+        then:
+        key.getTypeName() == "long"
+        key.getColumnSpan() > 0
+        !collection.isInverse()
+    }
+
+    def "bind delegates to dependentKeyValueBinder for unidirectional without 
join key mapping"() {
+        given:
+        def property = propertyFor(CKBUniOwner)
+        def key = new 
DependantValue(getGrailsDomainBinder().getMetadataBuildingContext(), null, null)
+        def associatedClass = new 
RootClass(getGrailsDomainBinder().getMetadataBuildingContext())
+        def collection = new 
Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null)
+
+        when:
+        binder.bind(property, key, associatedClass, collection)
+
+        then:
+        key.getColumnSpan() > 0
+        !collection.isInverse()
+    }
+}
+
+@Entity
+class CKBBidOwner {
+    Long id
+    static hasMany = [items: CKBBidItem]
+}
+
+@Entity
+class CKBBidItem {
+    Long id
+    CKBBidOwner owner
+    static belongsTo = [owner: CKBBidOwner]
+}
+
+@Entity
+class CKBManyToManyOwner {
+    Long id
+    static hasMany = [items: CKBManyToManyItem]
+}
+
+@Entity
+class CKBManyToManyItem {
+    Long id
+    static hasMany = [owners: CKBManyToManyOwner]
+}
+
+@Entity
+class CKBUniOwner {
+    Long id
+    static hasMany = [items: CKBUniItem]
+}
+
+@Entity
+class CKBUniItem {
+    Long id
+    String description
+}
+
+@Entity
+class CKBJoinKeyOwner {
+    Long id
+    static hasMany = [items: CKBJoinKeyItem]
+    static mapping = {
+        items joinTable: [key: 'owner_fk']
+    }
+}
+
+@Entity
+class CKBJoinKeyItem {
+    Long id
+    String description
+}
diff --git 
a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinderSpec.groovy
 
b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinderSpec.groovy
index bce105641c..2f01558daf 100644
--- 
a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinderSpec.groovy
+++ 
b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinderSpec.groovy
@@ -41,7 +41,7 @@ class CollectionSecondPassBinderSpec extends 
HibernateGormDatastoreSpec {
         def dcnf = new DefaultColumnNameFetcher(ns, new BackticksRemover())
         def svcb = new SimpleValueColumnBinder()
 
-        binder = new CollectionSecondPassBinder(mtob, pkvc, cku, botml, dkvb, 
uotmb, cwjtb, cfpcb, svcb, new CollectionOrderByBinder(), new 
CollectionMultiTenantFilterBinder(dcnf))
+        binder = new CollectionSecondPassBinder(mtob, pkvc, cku, uotmb, cwjtb, 
cfpcb, new CollectionKeyBinder(botml, dkvb, svcb), new 
CollectionOrderByBinder(), new CollectionMultiTenantFilterBinder(dcnf))
     }
 
     protected HibernatePersistentProperty 
createTestHibernateToManyProperty(Class<?> domainClass = 
CSPBTestEntityWithMany, String propertyName = "items") {

Reply via email to