matrei commented on code in PR #15568:
URL: https://github.com/apache/grails-core/pull/15568#discussion_r3391972498
##########
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GrailsCodeStylePluginSpec.groovy:
##########
@@ -19,7 +19,6 @@
package org.apache.grails.buildsrc
import org.gradle.testkit.runner.GradleRunner
-import org.gradle.testkit.runner.TaskOutcome
Review Comment:
This was the wrong choice in the merge.
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java:
##########
@@ -3143,7 +3146,7 @@ protected String
getColumnNameForPropertyAndPath(PersistentProperty grailsProp,
PropertyConfig c = m.getPropertyConfig(grailsProp.getName());
if (supportsJoinColumnMapping(grailsProp) &&
hasJoinKeyMapping(c)) {
- columnName = c.getJoinTable().getKey().getName();
+ columnName = c.getJoinTable().getKeys().get(0).getName();
Review Comment:
`getKeys().getFirst()`?
##########
dependencies.gradle:
##########
@@ -84,10 +85,11 @@ ext {
'groovy.version' : '4.0.32',
'hibernate-groovy-proxy.version': '1.1',
'jakarta-servlet-api.version' : '6.1.0',
- 'jakarta-validation.version': '3.1.1',
+ 'jakarta-validation.version' : '3.1.1',
'jquery.version' : '3.7.1',
'junit.version' : '6.0.3',
'mongodb.version' : '5.8.0',
+ 'reactor.version' : '3.8.5',
Review Comment:
Is this used somewhere?
##########
grails-data-hibernate7/grails-plugin/src/test/groovy/grails/orm/bootstrap/HibernateDatastoreSpringInitializerSpec.groovy:
##########
@@ -61,33 +67,32 @@ class HibernateDatastoreSpringInitializerSpec extends
Specification{
applicationContext.getBean("sessionFactory_moreBooks",
SessionFactory).metamodel.entity(Author.name)
and:"Each domain has the correct data source(s)"
+ HibernateDatastore hibernateDatastore =
applicationContext.getBean(HibernateDatastore)
Person.withNewSession { Person.count() == 0 }
- Person.withNewSession { Session s ->
- assert s.connection().metaData.getURL() == "jdbc:h2:mem:people"
- return true
- }
- Book.withNewSession { Book.count() == 0 }
- Book.withNewSession { Session s ->
- assert s.connection().metaData.getURL() == "jdbc:h2:mem:books"
- return true
- }
- Book.moreBooks.withNewSession { Session s ->
- assert s.connection().metaData.getURL() == "jdbc:h2:mem:moreBooks"
- return true
- }
- Author.withNewSession { Author.count() == 0 }
- Author.withNewSession { Session s ->
- assert s.connection().metaData.getURL() == "jdbc:h2:mem:people"
- return true
- }
- Author.books.withNewSession { Session s ->
- assert s.connection().metaData.getURL() == "jdbc:h2:mem:books"
- return true
- }
- Author.moreBooks.withNewSession { Session s ->
- assert s.connection().metaData.getURL() == "jdbc:h2:mem:moreBooks"
- return true
- }
+ hibernateDatastore.withNewSession { Session s ->
+ assert s.doReturningWork { it.getMetaData().getURL() } ==
"jdbc:h2:mem:people"
+ return true
+ }
+ hibernateDatastore.withNewSession("books") { Session s ->
+ assert s.doReturningWork { it.getMetaData().getURL() } ==
"jdbc:h2:mem:books"
+ return true
+ }
+ hibernateDatastore.withNewSession("moreBooks") { Session s ->
+ assert s.doReturningWork { it.getMetaData().getURL() } ==
"jdbc:h2:mem:moreBooks"
+ return true
+ }
+ hibernateDatastore.withNewSession { Session s ->
+ assert s.doReturningWork { it.getMetaData().getURL() } ==
"jdbc:h2:mem:people"
+ return true
+ }
+ hibernateDatastore.withNewSession("books") { Session s ->
+ assert s.doReturningWork { it.getMetaData().getURL() } ==
"jdbc:h2:mem:books"
+ return true
+ }
+ Author.moreBooks.withNewSession { Session s ->
+ assert s.doReturningWork { it.getMetaData().getURL() } ==
"jdbc:h2:mem:moreBooks"
+ return true
+ }
Review Comment:
Indent?
##########
grails-data-hibernate7/grails-plugin/src/test/groovy/grails/orm/bootstrap/HibernateDatastoreSpringInitializerSpec.groovy:
##########
@@ -61,33 +67,32 @@ class HibernateDatastoreSpringInitializerSpec extends
Specification{
applicationContext.getBean("sessionFactory_moreBooks",
SessionFactory).metamodel.entity(Author.name)
and:"Each domain has the correct data source(s)"
+ HibernateDatastore hibernateDatastore =
applicationContext.getBean(HibernateDatastore)
Review Comment:
`def`?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java:
##########
@@ -499,7 +499,8 @@ protected void bindCollectionSecondPass(ToMany property,
InFlightMetadataCollect
}
} else {
if (hasJoinKeyMapping(propConfig)) {
- bindSimpleValue("long", key, false,
propConfig.getJoinTable().getKey().getName(), mappings);
+ java.util.List<ColumnConfig> keys =
propConfig.getJoinTable().getKeys();
+ bindSimpleValue("long", key, false, keys.get(0).getName(),
mappings);
Review Comment:
`keys.getFirst()`?
##########
grails-datamapping-core/src/main/groovy/grails/gorm/DetachedCriteria.groovy:
##########
@@ -136,14 +136,18 @@ class DetachedCriteria<T> extends
AbstractDetachedCriteria<T> implements GormOpe
* @return A list of matching instances
*/
List<T> list(Map args = Collections.emptyMap(),
@DelegatesTo(DetachedCriteria) Closure additionalCriteria = null) {
- (List) withPopulatedQuery(args, additionalCriteria) { Query query ->
+ (List)withPopulatedQuery(args, additionalCriteria) { Query query ->
if (args?.max) {
- return new PagedResultList(query)
+ return newPagedResultList(query)
}
return query.list()
}
}
+ protected PagedResultList<T> newPagedResultList(Query query) {
+ new PagedResultList<T>(query)
+ }
+
Review Comment:
Why was this extracted as a method?
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BackticksRemover.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.util;
+
+import java.util.Optional;
+import java.util.function.Function;
+
+/** The backticks remover class. */
Review Comment:
Improve javadoc comment?
##########
grails-data-hibernate7/README.md:
##########
@@ -0,0 +1,98 @@
+<!--
+ 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.
+-->
+# GORM for Hibernate 7
+This project implements [GORM](https://gorm.grails.org) for the Hibernate 7.
+
+With the removal of Criterion API in Hibernate 7, we wanted to continue to
support the DetachedCriteia in GORM as much as possible. We also wanted to
encapsulate the JPA Criteria Building in one class so the following was done:
+* DetachedCriteria holds almost all the state of the Query being built. It
hold the target class for the query. It does not hold a session.
Review Comment:
Typo: It hold the target
##########
grails-data-hibernate7/core/src/main/groovy/grails/orm/HibernateCriteriaBuilder.java:
##########
@@ -68,226 +75,1279 @@
* }
* maxResults(10)
* order("holderLastName", "desc")
+ * cache(true)
+ * readOnly(true)
* }
* </pre>
- * <p>The builder can also be instantiated standalone with a SessionFactory
and persistent Class instance:
+ *
+ * <h2>Advanced Features</h2>
+ *
+ * <p>The builder supports several advanced Hibernate features:
+ *
+ * <ul>
+ * <li><b>Pessimistic Locking:</b> Use {@code lock(true)} to obtain a
pessimistic write lock.
+ * <li><b>Query Caching:</b> Use {@code cache(true)} to enable query caching
for the results.
+ * <li><b>Read-Only Mode:</b> Use {@code readOnly(true)} to disable dirty
checking for loaded
+ * entities.
+ * <li><b>Fetch Mode:</b> Use {@code fetchMode("association",
FetchMode.JOIN)} to specify Eager/Lazy
+ * fetching strategies.
+ * </ul>
+ *
+ * <h2>Programmatic instantiation</h2>
+ *
+ * <p>The builder requires a {@link SessionFactory}, the target persistent
class, and the {@link
+ * org.grails.orm.hibernate.HibernateDatastore} that owns the session:
+ *
* <pre>
- * new HibernateCriteriaBuilder(clazz, sessionFactory).list {
+ * new HibernateCriteriaBuilder(Account, sessionFactory, datastore).list {
* eq("firstName", "Fred")
* }
* </pre>
*
+ * <h2>Architecture</h2>
+ *
+ * <p>Closure method calls in the DSL are dispatched through {@code
invokeMethod} → {@code
+ * CriteriaMethodInvoker} → {@link HibernateQuery}, which translates each GORM
constraint into the
+ * equivalent JPA Criteria predicate. {@link grails.gorm.DetachedCriteria} can
also be passed in
+ * place of a closure to support multi-tenant and reusable query fragments.
+ *
+ * To adjust the methods to be handled you have to extend this class, extend
CriteriaMethodInvoker
+ *
* @author Graeme Rocher
+ * @author walterduquedeestrada
+ * @see HibernateQuery
+ * @see grails.gorm.DetachedCriteria
*/
-public class HibernateCriteriaBuilder extends AbstractHibernateCriteriaBuilder
{
+@Slf4j
+@SuppressWarnings("PMD.AvoidDuplicateLiterals")
+public class HibernateCriteriaBuilder extends GroovyObjectSupport implements
BuildableCriteria, ProjectionList {
/*
* Define constants which may be used inside of criteria queries
* to refer to standard Hibernate Type instances.
*/
- public static final Type BOOLEAN = StandardBasicTypes.BOOLEAN;
- public static final Type YES_NO = StandardBasicTypes.YES_NO;
- public static final Type BYTE = StandardBasicTypes.BYTE;
- public static final Type CHARACTER = StandardBasicTypes.CHARACTER;
- public static final Type SHORT = StandardBasicTypes.SHORT;
- public static final Type INTEGER = StandardBasicTypes.INTEGER;
- public static final Type LONG = StandardBasicTypes.LONG;
- public static final Type FLOAT = StandardBasicTypes.FLOAT;
- public static final Type DOUBLE = StandardBasicTypes.DOUBLE;
- public static final Type BIG_DECIMAL = StandardBasicTypes.BIG_DECIMAL;
- public static final Type BIG_INTEGER = StandardBasicTypes.BIG_INTEGER;
- public static final Type STRING = StandardBasicTypes.STRING;
- public static final Type NUMERIC_BOOLEAN =
StandardBasicTypes.NUMERIC_BOOLEAN;
- public static final Type TRUE_FALSE = StandardBasicTypes.TRUE_FALSE;
- public static final Type URL = StandardBasicTypes.URL;
- public static final Type TIME = StandardBasicTypes.TIME;
- public static final Type DATE = StandardBasicTypes.DATE;
- public static final Type TIMESTAMP = StandardBasicTypes.TIMESTAMP;
- public static final Type CALENDAR = StandardBasicTypes.CALENDAR;
- public static final Type CALENDAR_DATE = StandardBasicTypes.CALENDAR_DATE;
- public static final Type CLASS = StandardBasicTypes.CLASS;
- public static final Type LOCALE = StandardBasicTypes.LOCALE;
- public static final Type CURRENCY = StandardBasicTypes.CURRENCY;
- public static final Type TIMEZONE = StandardBasicTypes.TIMEZONE;
- public static final Type UUID_BINARY = StandardBasicTypes.UUID_BINARY;
- public static final Type UUID_CHAR = StandardBasicTypes.UUID_CHAR;
- public static final Type BINARY = StandardBasicTypes.BINARY;
- public static final Type WRAPPER_BINARY =
StandardBasicTypes.WRAPPER_BINARY;
- public static final Type IMAGE = StandardBasicTypes.IMAGE;
- public static final Type BLOB = StandardBasicTypes.BLOB;
- public static final Type MATERIALIZED_BLOB =
StandardBasicTypes.MATERIALIZED_BLOB;
- public static final Type CHAR_ARRAY = StandardBasicTypes.CHAR_ARRAY;
- public static final Type CHARACTER_ARRAY =
StandardBasicTypes.CHARACTER_ARRAY;
- public static final Type TEXT = StandardBasicTypes.TEXT;
- public static final Type CLOB = StandardBasicTypes.CLOB;
- public static final Type MATERIALIZED_CLOB =
StandardBasicTypes.MATERIALIZED_CLOB;
- public static final Type SERIALIZABLE = StandardBasicTypes.SERIALIZABLE;
Review Comment:
This comment seems to be left-over from removed code?
##########
H7_GORM_BUG_REPORT.md:
##########
Review Comment:
Should this file be checked in?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/JoinTable.groovy:
##########
@@ -36,9 +36,24 @@ import groovy.transform.builder.SimpleStrategy
class JoinTable extends Table {
/**
- * The foreign key column
+ * The foreign key columns (composite key support)
*/
- ColumnConfig key
+ List<ColumnConfig> keys = []
+
+ void setKeys(List<ColumnConfig> keys) {
+ this.keys = keys
+ }
+
+ /**
+ * Configures the keys
+ * @param names The key names
+ * @return This join table config
+ */
+ JoinTable keys(List names) {
+ this.keys = (List<ColumnConfig>) names.collect { it instanceof
ColumnConfig ? it : new ColumnConfig(name: it.toString()) }
+ return this
+ }
+
Review Comment:
Move methods below field declarations?
##########
.agents/skills/hibernate-developer/SKILL.md:
##########
@@ -0,0 +1,119 @@
+---
+name: hibernate-developer
+description: Guide for working in the grails-data-hibernate7 module,
especially Hibernate 7 domain binding, mapping migration, generators, and
integration tests. Use this when changing code or tests under
grails-data-hibernate7.
+license: Apache-2.0
+---
+<!--
+SPDX-License-Identifier: Apache-2.0
+
+Licensed to the Apache Software Foundation (ASF) under one or more contributor
license agreements; and to You under the Apache License, Version 2.0.
+-->
+
+## What I Do
+
+- Provide repository-specific guidance for the `grails-data-hibernate7`
project.
+- Help with Hibernate 7 migration work in domain binding, mapping metadata,
identifiers, generators, collections, and second-pass binding.
+- Guide changes around `GrailsDomainBinder`, `GrailsPropertyBinder`,
`IdentityBinder`, `VersionBinder`, collection binders, and related utilities.
+- Keep changes aligned with the testing constraints and migration status
documented in `grails-data-hibernate7/AGENTS.md`.
+
+## When to Use Me
+
+Activate this skill when working on the Hibernate 7 module, especially for:
+
+- Changes under `grails-data-hibernate7/**`.
+- Hibernate 7 mapping and metadata binding work.
+- Identifier, version, collection, association, or generator binding changes.
+- Hibernate 7 regression fixes and migration follow-up tasks.
+- Specs that exercise Hibernate-backed mapping behavior rather than
lightweight unit behavior.
+
+## Module Context
+
+This skill is for the Grails framework's Hibernate 7 integration module, not
for a Grails application. Prefer guidance from this skill over generic Grails
app patterns when working in `grails-data-hibernate7`.
+
+`GrailsDomainBinder` is the main entry point for binding Grails domain classes
to Hibernate metadata. Changes often ripple through:
+
+- `org.grails.orm.hibernate.cfg`
+- `org.grails.orm.hibernate.cfg.domainbinding`
+- `org.grails.orm.hibernate.cfg.domainbinding.collectionType`
+- `org.grails.orm.hibernate.cfg.domainbinding.secondpass`
+- `org.grails.orm.hibernate.cfg.domainbinding.generator`
+
+## Key Classes and Responsibilities
+
+### Main Binding Flow
+
+- `GrailsDomainBinder`: central coordinator for Hibernate 7 mapping
contribution.
+- `GrailsPropertyBinder`: main coordinator for converting persistent
properties into Hibernate `Value` instances.
+- `PropertyFromValueCreator`: shared utility for creating Hibernate `Property`
instances from a bound `Value`.
+
+### Identifier and Version Binding
+
+- `IdentityBinder`: coordinates identifier binding.
+- `SimpleIdBinder`: handles simple identifiers.
+- `CompositeIdBinder`: handles composite identifiers.
+- `VersionBinder`: binds optimistic locking version properties.
+- `NaturalIdentifierBinder`: binds `naturalId` properties.
+
+### Associations and Collections
+
+- `OneToOneBinder`, `ManyToOneBinder`, `ManyToOneValuesBinder`: association
binding.
+- `CollectionBinder`: collection mapping.
+- `CollectionSecondPassBinder`, `ListSecondPassBinder`, `MapSecondPassBinder`:
second-pass association and collection binding.
+- `CollectionHolder` plus the collection type classes: carry collection
metadata through binding.
+
+### Value and Column Binding
+
+- `SimpleValueBinder`: binds simple properties.
+- `SimpleValueColumnBinder`: binds columns to simple values.
+- `ComponentBinder`, `ComponentPropertyBinder`: embedded/component binding.
+- `EnumTypeBinder`: enum mapping.
+
+### Generators
+
+- `BasicValueCreator`: creates identifier values and generators.
+- `GrailsSequenceWrapper`, `GrailsSequenceGeneratorEnum`: generator
integration helpers.
+- `GrailsIdentityGenerator`, `GrailsIncrementGenerator`,
`GrailsNativeGenerator`, `GrailsSequenceStyleGenerator`,
`GrailsTableGenerator`: Grails-specific Hibernate 7 generator implementations.
+
+## Current Module Guidance
+
+Keep these module-specific expectations in mind:
+
+- `GrailsPropertyBinder` has already been simplified to a unified
binder-dispatch structure. Preserve that consolidation instead of reintroducing
scattered property creation or ad hoc branching.
+- Property creation and addition should stay centralized through callers using
`PropertyFromValueCreator` where applicable.
+- Utility classes in `domainbinding.util` should prefer Hibernate-aware GORM
types internally, but public signatures may still need base interfaces when
Spock mocks require them.
+- `GrailsIncrementGenerator` still contains reflection-based Hibernate 7
compatibility workarounds; avoid broad refactors unless the change explicitly
addresses that area.
+
+## Testing Rules
+
+When touching `grails-data-hibernate7`, test through real Hibernate wiring
rather than assuming mocks are enough.
+
+- Use `HibernateGormDatastoreSpec` for Hibernate 7 integration and
domain-binding specifications.
+- Prefer `manager.registerDomainClasses(...)` in `setupSpec()` to register
entities for specs.
+- Define test entities as top-level classes in the same Groovy spec file.
+- Ensure test domain class names are globally unique within the package to
avoid collisions during parallel execution.
Review Comment:
I don't get the "parallel execution" problem?
##########
.agents/skills/hibernate-developer/SKILL.md:
##########
@@ -0,0 +1,119 @@
+---
+name: hibernate-developer
+description: Guide for working in the grails-data-hibernate7 module,
especially Hibernate 7 domain binding, mapping migration, generators, and
integration tests. Use this when changing code or tests under
grails-data-hibernate7.
+license: Apache-2.0
+---
+<!--
+SPDX-License-Identifier: Apache-2.0
+
+Licensed to the Apache Software Foundation (ASF) under one or more contributor
license agreements; and to You under the Apache License, Version 2.0.
+-->
+
+## What I Do
+
+- Provide repository-specific guidance for the `grails-data-hibernate7`
project.
+- Help with Hibernate 7 migration work in domain binding, mapping metadata,
identifiers, generators, collections, and second-pass binding.
+- Guide changes around `GrailsDomainBinder`, `GrailsPropertyBinder`,
`IdentityBinder`, `VersionBinder`, collection binders, and related utilities.
+- Keep changes aligned with the testing constraints and migration status
documented in `grails-data-hibernate7/AGENTS.md`.
+
+## When to Use Me
+
+Activate this skill when working on the Hibernate 7 module, especially for:
+
+- Changes under `grails-data-hibernate7/**`.
+- Hibernate 7 mapping and metadata binding work.
+- Identifier, version, collection, association, or generator binding changes.
+- Hibernate 7 regression fixes and migration follow-up tasks.
+- Specs that exercise Hibernate-backed mapping behavior rather than
lightweight unit behavior.
+
+## Module Context
+
+This skill is for the Grails framework's Hibernate 7 integration module, not
for a Grails application. Prefer guidance from this skill over generic Grails
app patterns when working in `grails-data-hibernate7`.
+
+`GrailsDomainBinder` is the main entry point for binding Grails domain classes
to Hibernate metadata. Changes often ripple through:
+
+- `org.grails.orm.hibernate.cfg`
+- `org.grails.orm.hibernate.cfg.domainbinding`
+- `org.grails.orm.hibernate.cfg.domainbinding.collectionType`
+- `org.grails.orm.hibernate.cfg.domainbinding.secondpass`
+- `org.grails.orm.hibernate.cfg.domainbinding.generator`
+
+## Key Classes and Responsibilities
+
+### Main Binding Flow
+
+- `GrailsDomainBinder`: central coordinator for Hibernate 7 mapping
contribution.
+- `GrailsPropertyBinder`: main coordinator for converting persistent
properties into Hibernate `Value` instances.
+- `PropertyFromValueCreator`: shared utility for creating Hibernate `Property`
instances from a bound `Value`.
+
+### Identifier and Version Binding
+
+- `IdentityBinder`: coordinates identifier binding.
+- `SimpleIdBinder`: handles simple identifiers.
+- `CompositeIdBinder`: handles composite identifiers.
+- `VersionBinder`: binds optimistic locking version properties.
+- `NaturalIdentifierBinder`: binds `naturalId` properties.
+
+### Associations and Collections
+
+- `OneToOneBinder`, `ManyToOneBinder`, `ManyToOneValuesBinder`: association
binding.
+- `CollectionBinder`: collection mapping.
+- `CollectionSecondPassBinder`, `ListSecondPassBinder`, `MapSecondPassBinder`:
second-pass association and collection binding.
+- `CollectionHolder` plus the collection type classes: carry collection
metadata through binding.
+
+### Value and Column Binding
+
+- `SimpleValueBinder`: binds simple properties.
+- `SimpleValueColumnBinder`: binds columns to simple values.
+- `ComponentBinder`, `ComponentPropertyBinder`: embedded/component binding.
+- `EnumTypeBinder`: enum mapping.
+
+### Generators
+
+- `BasicValueCreator`: creates identifier values and generators.
+- `GrailsSequenceWrapper`, `GrailsSequenceGeneratorEnum`: generator
integration helpers.
+- `GrailsIdentityGenerator`, `GrailsIncrementGenerator`,
`GrailsNativeGenerator`, `GrailsSequenceStyleGenerator`,
`GrailsTableGenerator`: Grails-specific Hibernate 7 generator implementations.
+
+## Current Module Guidance
+
+Keep these module-specific expectations in mind:
+
+- `GrailsPropertyBinder` has already been simplified to a unified
binder-dispatch structure. Preserve that consolidation instead of reintroducing
scattered property creation or ad hoc branching.
+- Property creation and addition should stay centralized through callers using
`PropertyFromValueCreator` where applicable.
+- Utility classes in `domainbinding.util` should prefer Hibernate-aware GORM
types internally, but public signatures may still need base interfaces when
Spock mocks require them.
+- `GrailsIncrementGenerator` still contains reflection-based Hibernate 7
compatibility workarounds; avoid broad refactors unless the change explicitly
addresses that area.
+
+## Testing Rules
+
+When touching `grails-data-hibernate7`, test through real Hibernate wiring
rather than assuming mocks are enough.
+
+- Use `HibernateGormDatastoreSpec` for Hibernate 7 integration and
domain-binding specifications.
+- Prefer `manager.registerDomainClasses(...)` in `setupSpec()` to register
entities for specs.
+- Define test entities as top-level classes in the same Groovy spec file.
+- Ensure test domain class names are globally unique within the package to
avoid collisions during parallel execution.
+- Prefer real entities over heavy mocking for binder logic.
+
+## Change Workflow
+
+1. Identify which binder, creator, generator, fetcher, or second-pass class
owns the behavior.
+2. Trace whether the change affects only `Value` creation, `Property`
creation, or both.
+3. Preserve the existing separation between logical mapping decisions and
Hibernate object construction.
+4. Update or add specs in `grails-data-hibernate7` that exercise the affected
behavior through the public Hibernate-backed path.
+5. Run the relevant Hibernate 7 module tests, and expand test coverage when
binder flow or entity registration behavior changes.
+
+## Pitfalls to Avoid
+
+- Do not treat this module like a simple Grails application layer; it is
framework and mapping infrastructure code.
+- Do not reintroduce duplicated property-creation logic if a shared binder or
creator already owns it.
+- Do not rely on unit-only mocking for Hibernate internals when the behavior
depends on real metadata binding.
+- Do not use nested or inner entity classes in Hibernate 7 specs when
top-level classes are required for AST transforms and reliable registration.
+
+## Known Status and Constraints
+
+- The Hibernate 7 binder migration is largely in migrated state across the
main binders, collection types, second-pass binders, generators, and utilities.
+- Unidirectional many-to-many support in `CollectionSecondPassBinder` is
implemented.
+- `GrailsIncrementGenerator` reflection hacks remain a known temporary
compromise until a later Hibernate upgrade removes the need.
+
+## Source of Truth
+
+This skill is derived from `grails-data-hibernate7/AGENTS.md`. When the module
guidance changes, update this skill so agents can load the same rules directly
from `.agents/skills/hibernate-developer/SKILL.md`.
Review Comment:
This does not seem up-to-date?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/query/AbstractHibernateQuery.java:
##########
@@ -565,6 +569,18 @@ public ProjectionList projections() {
return hibernateProjectionList;
}
+ @Override
+ public Number countResults() {
Review Comment:
`@since`?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java:
##########
@@ -2392,7 +2393,9 @@ protected void bindManyToOne(Association property,
ManyToOne manyToOne,
final ColumnConfig columnConfig = new ColumnConfig();
columnConfig.setName(namingStrategy.propertyToColumnName(property.getName()) +
UNDERSCORE + FOREIGN_KEY_SUFFIX);
- jt.setKey(columnConfig);
+ java.util.List<ColumnConfig> keys = new
java.util.ArrayList<>();
Review Comment:
Why fully qualified?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/CascadeValidationSpec.groovy:
##########
@@ -35,7 +35,7 @@ class CascadeValidationSpec extends Specification {
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(Business, Person, Employee)
@Rollback
- @Issue('https://github.com/apache/grails-data-mapping/issues/926')
+ @Issue('https://github.com/grails/grails-data-mapping/issues/926')
Review Comment:
Why?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java:
##########
@@ -3154,7 +3157,7 @@ else if (c != null && c.getColumn() != null) {
if (supportsJoinColumnMapping(grailsProp)) {
PropertyConfig pc = getPropertyConfig(grailsProp);
if (hasJoinKeyMapping(pc)) {
- columnName = pc.getJoinTable().getKey().getName();
+ columnName = pc.getJoinTable().getKeys().get(0).getName();
Review Comment:
`getKeys().getFirst()?`
##########
grails-data-hibernate7/grails-plugin/build.gradle:
##########
@@ -72,9 +76,9 @@ dependencies {
testRuntimeOnly 'com.h2database:h2'
testRuntimeOnly 'org.apache.tomcat:tomcat-jdbc'
- testRuntimeOnly 'org.hibernate:hibernate-ehcache', {
+ testRuntimeOnly 'org.hibernate.orm:hibernate-jcache', {
// exclude javax variant of hibernate-core 5.6
Review Comment:
This comment is now redundant as the exclusion was removed.
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java:
##########
@@ -499,7 +499,8 @@ protected void bindCollectionSecondPass(ToMany property,
InFlightMetadataCollect
}
} else {
if (hasJoinKeyMapping(propConfig)) {
- bindSimpleValue("long", key, false,
propConfig.getJoinTable().getKey().getName(), mappings);
+ java.util.List<ColumnConfig> keys =
propConfig.getJoinTable().getKeys();
Review Comment:
Why fully qualified?
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/access/TraitPropertyAccessStrategy.java:
##########
@@ -43,72 +46,97 @@
* @author Graeme Rocher
* @since 6.1.3
*/
+@SuppressWarnings({"rawtypes", "PMD.DataflowAnomalyAnalysis"})
public class TraitPropertyAccessStrategy implements PropertyAccessStrategy {
- @Override
+
public PropertyAccess buildPropertyAccess(Class containerJavaType, String
propertyName) {
+ return buildPropertyAccess(containerJavaType, propertyName, true);
+ }
+
+ protected String getTraitFieldName(Class traitClass, String fieldName) {
+ return traitClass.getName().replace('.', '_') + "__" + fieldName;
+ }
+
+ @java.lang.Override
+ public @UnknownKeyFor @NonNull @Initialized PropertyAccess
buildPropertyAccess(
Review Comment:
Why are we introducing this `checkerframework`?
From the javadoc of `@UnknownKeyFor`: "Used internally by the type system;
should never be written by a programmer."
##########
grails-data-hibernate7/grails-plugin/build.gradle:
##########
@@ -72,9 +76,9 @@ dependencies {
testRuntimeOnly 'com.h2database:h2'
testRuntimeOnly 'org.apache.tomcat:tomcat-jdbc'
- testRuntimeOnly 'org.hibernate:hibernate-ehcache', {
+ testRuntimeOnly 'org.hibernate.orm:hibernate-jcache', {
// exclude javax variant of hibernate-core 5.6
- exclude group: 'org.hibernate', module: 'hibernate-core'
+
}
testRuntimeOnly
"org.jboss.spec.javax.transaction:jboss-transaction-api_1.3_spec:$jbossTransactionApiVersion",
{
Review Comment:
Is this still needed?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/Hibernate5OptimisticLockingSpec.groovy:
##########
@@ -44,29 +69,36 @@ class Hibernate5OptimisticLockingSpec extends
GrailsDataTckSpec<GrailsDataHibern
when:
OptLockVersioned.withTransaction {
- o = OptLockVersioned.get(o.id)
+ try {
+ o = OptLockVersioned.get(o.id)
+
+ Thread.start {
+ OptLockVersioned.withTransaction { s ->
+ def reloaded = OptLockVersioned.get(o.id)
+ assert reloaded
+ assert reloaded != o
+ reloaded.name += ' in new session'
+ reloaded.save(flush: true)
+ assert reloaded.version == 1
+ assert o.version == 0
+ }
+
+ }.join()
+
+ o.name += ' in main session'
+ o.save(flush: true)
- Thread.start {
- OptLockVersioned.withTransaction { s ->
- def reloaded = OptLockVersioned.get(o.id)
- assert reloaded
- assert reloaded != o
- reloaded.name += ' in new session'
- reloaded.save(flush: true)
- assert reloaded.version == 1
- assert o.version == 0
+ manager.session.clear()
+ o = OptLockVersioned.get(o.id)
+ } catch (Throwable e) {
+ System.getProperties().each { key, value ->
+ println "${key}: ${value}"
}
-
- }.join()
-
- o.name += ' in main session'
- o.save(flush: true)
-
- manager.session.clear()
- o = OptLockVersioned.get(o.id)
+ throw e
+ }
}
then:
- thrown HibernateOptimisticLockingFailureException
+ thrown OptimisticLockingFailureException
Review Comment:
So here is actually another Exception thrown now? Previously it seems to
have been a `HibernateOptimisticLockingFailureException`.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -136,15 +136,52 @@ spring:
==== 5. Hibernate ORM Package Relocations
Spring Framework 7 removed the `org.springframework.orm.hibernate5` package
entirely.
-Grails 8 vendors these classes from Spring Framework 6.2.x into a new module
(`grails-data-hibernate5-spring-orm`) under the package
`org.grails.orm.hibernate.support.hibernate5`.
+Grails 8 vendors these classes from Spring Framework 6.2.x into two new
modules — one per supported Hibernate version — so that both Hibernate 5 and
Hibernate 7 users have a drop-in replacement.
Review Comment:
Mention which modules?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/SaveWithInvalidEntitySpec.groovy:
##########
@@ -30,24 +31,30 @@ import spock.lang.Specification
/**
* Created by graemerocher on 03/05/2017.
*/
+//TODO Should this test be rewritten?
class SaveWithInvalidEntitySpec extends Specification {
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(A, B)
/**
- * This currently fails with a NPE. See explanation
https://github.com/apache/grails-core/issues/14616#issuecomment-298943022
+ * This currently fails with a NPE. See explanation
https://github.com/grails/grails-core/issues/10604#issuecomment-298943022
Review Comment:
`grails` -> `apache`?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsHibernateUtil.java:
##########
@@ -422,30 +420,6 @@ public static Object unwrapIfProxy(Object instance) {
return proxyHandler.unwrap(instance);
}
- /**
- * @deprecated Use {@link
MultipleDataSourceSupport#getDefaultDataSource(PersistentEntity)} instead
- */
- @Deprecated
- public static String getDefaultDataSource(PersistentEntity domainClass) {
- return MultipleDataSourceSupport.getDefaultDataSource(domainClass);
- }
-
- /**
- * @deprecated Use {@link
MultipleDataSourceSupport#getDatasourceNames(PersistentEntity)} instead
- */
- @Deprecated
- public static List<String> getDatasourceNames(PersistentEntity
domainClass) {
- return MultipleDataSourceSupport.getDatasourceNames(domainClass);
- }
-
- /**
- * @deprecated Use {@link
MultipleDataSourceSupport#getDefaultDataSource(PersistentEntity)} instead
- */
- @Deprecated
- public static boolean usesDatasource(PersistentEntity domainClass, String
dataSourceName) {
- return MultipleDataSourceSupport.usesDatasource(domainClass,
dataSourceName);
- }
-
Review Comment:
Should we add `forRemoval = true` before removing these?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithJoinTableSpec.groovy:
##########
@@ -19,45 +19,40 @@
package grails.gorm.tests
-import static grails.gorm.hibernate.mapping.MappingBuilder.define
-
import grails.gorm.annotation.Entity
-import grails.gorm.transactions.Rollback
-import org.grails.orm.hibernate.HibernateDatastore
-import org.springframework.transaction.PlatformTransactionManager
-import spock.lang.AutoCleanup
-import spock.lang.Shared
-import spock.lang.Specification
+
+import static grails.gorm.hibernate.mapping.MappingBuilder.define
/**
* Created by graemerocher on 26/01/2017.
*/
-class CompositeIdWithJoinTableSpec extends Specification {
-
- @AutoCleanup @Shared HibernateDatastore datastore = new
HibernateDatastore(CompositeIdParent, CompositeIdChild)
- @Shared PlatformTransactionManager transactionManager =
datastore.transactionManager
+class CompositeIdWithJoinTableSpec extends HibernateGormDatastoreSpec {
+ def setupSpec() {
+ manager.registerDomainClasses(CompositeIdParent, CompositeIdChild)
+ }
- @Rollback
+ // @Rollback
void "test composite id with join table"() {
- when:"A parent with a composite id and a join table is saved"
- new CompositeIdParent(name: "Test" , last:"Test 2")
- .addToChildren(new CompositeIdChild())
- .save(flush:true)
+ when: "A parent with a composite id and a join table is saved"
+ new CompositeIdParent(name: "Test", last: "Test 2")
+ .addToChildren(new CompositeIdChild(foo: "bar"))
+ .save(flush: true)
- then:"The entity was saved"
+ then: "The entity was saved"
CompositeIdParent.count() == 1
CompositeIdParent.list().first().children.size() == 1
}
}
@Entity
-class CompositeIdParent implements Serializable {
+class CompositeIdParent implements Serializable, Comparable<CompositeIdParent>
{
Review Comment:
Why was `Comparable` added? I find no notes in the commit message about this
change.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/HibernateValidationSpec.groovy:
##########
@@ -22,6 +22,9 @@ import org.apache.grails.data.testing.tck.domains.ChildEntity
import
org.apache.grails.data.testing.tck.domains.ClassWithListArgBeforeValidate
import org.apache.grails.data.testing.tck.domains.ClassWithNoArgBeforeValidate
import
org.apache.grails.data.testing.tck.domains.ClassWithOverloadedBeforeValidate
+import org.apache.grails.data.testing.tck.domains.Location
+import org.apache.grails.data.testing.tck.domains.Person
+import org.apache.grails.data.testing.tck.domains.Pet
Review Comment:
These seem to be unused?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/WhereQueryOldIssueVerificationSpec.groovy:
##########
@@ -235,13 +236,11 @@ class WhereQueryOldIssueVerificationSpec extends
Specification {
@Issue('https://github.com/apache/grails-core/issues/14600')
def "findAllBy works with bidirectional hasMany relation"() {
given: "authors with books in a bidirectional hasMany"
- def author1 = new WqBiAuthor(name: "Stephen King").save(flush: true)
- def book1 = new WqBiBook(title: "IT").save(flush: true)
- def book2 = new WqBiBook(title: "The Shining").save(flush: true)
+ def author1 = new WqBiAuthor(name: "Stephen King")
+ def book1 = new WqBiBook(title: "IT")
+ def book2 = new WqBiBook(title: "The Shining")
author1.addToBooks(book1)
author1.addToBooks(book2)
- book1.addToAuthors(author1)
- book2.addToAuthors(author1)
author1.save(flush: true)
Review Comment:
Why?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/datastore/gorm/GormEnhancerCleanupSpec.groovy:
##########
@@ -0,0 +1,85 @@
+/* Copyright (C) 2026 the original author or authors.
+ *
+ * Licensed 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.datastore.gorm
+
+import grails.gorm.annotation.Entity
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.mapping.core.Datastore
+import spock.lang.Specification
+import java.util.concurrent.ConcurrentHashMap
Review Comment:
Unused imports?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/TwoBidirectionalOneToManySpec.groovy:
##########
@@ -46,12 +46,30 @@ class TwoBidirectionalOneToManySpec extends Specification {
then:"The entity was saved"
!r.errors.hasErrors()
Room.count == 1
+ PointX.count == 1
+ PointY.count == 1
+
+ }
+
+ @Rollback
+ void "test an entity with 1 one directional one-to-many mappings"() {
+ when:"A new entity is created is created"
Review Comment:
Typo in label? (same above)
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/UniqueWithinGroupSpec.groovy:
##########
@@ -19,35 +19,28 @@
package grails.gorm.tests.validation
import grails.gorm.annotation.Entity
+import grails.gorm.tests.HibernateGormDatastoreSpec
import grails.gorm.transactions.Rollback
import groovy.transform.EqualsAndHashCode
-import org.grails.orm.hibernate.HibernateDatastore
-import org.hibernate.SessionFactory
import org.springframework.dao.DuplicateKeyException
-import spock.lang.AutoCleanup
import spock.lang.Issue
-import spock.lang.Shared
-import spock.lang.Specification
/**
* Created by graemerocher on 29/05/2017.
*/
-@Issue('https://github.com/grails/grails-data-hibernate5/issues/36')
-class UniqueWithinGroupSpec extends Specification {
+@Issue('https://github.com/grails/gorm-hibernate5/issues/36')
Review Comment:
https://github.com/grails/grails-data-hibernate5/issues/36 ?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/Hibernate5OptimisticLockingSpec.groovy:
##########
@@ -44,29 +69,36 @@ class Hibernate5OptimisticLockingSpec extends
GrailsDataTckSpec<GrailsDataHibern
when:
OptLockVersioned.withTransaction {
- o = OptLockVersioned.get(o.id)
+ try {
+ o = OptLockVersioned.get(o.id)
+
+ Thread.start {
+ OptLockVersioned.withTransaction { s ->
+ def reloaded = OptLockVersioned.get(o.id)
+ assert reloaded
+ assert reloaded != o
+ reloaded.name += ' in new session'
+ reloaded.save(flush: true)
+ assert reloaded.version == 1
+ assert o.version == 0
+ }
+
+ }.join()
+
+ o.name += ' in main session'
+ o.save(flush: true)
- Thread.start {
- OptLockVersioned.withTransaction { s ->
- def reloaded = OptLockVersioned.get(o.id)
- assert reloaded
- assert reloaded != o
- reloaded.name += ' in new session'
- reloaded.save(flush: true)
- assert reloaded.version == 1
- assert o.version == 0
+ manager.session.clear()
+ o = OptLockVersioned.get(o.id)
+ } catch (Throwable e) {
+ System.getProperties().each { key, value ->
+ println "${key}: ${value}"
}
-
- }.join()
-
- o.name += ' in main session'
- o.save(flush: true)
-
- manager.session.clear()
- o = OptLockVersioned.get(o.id)
+ throw e
Review Comment:
Why are we printing the system properties here?
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -964,3 +1003,182 @@ Previously, rendering an enum value would produce a JSON
string with the type an
Rendering an enum value as JSON will now instead throw a `ConverterException`.
See https://github.com/apache/grails-core/pull/15212[PR 15212] for more
details on this change.
+
+[[_hibernate_5_to_hibernate_7_migration]]
+==== 23. Hibernate 5 to Hibernate 7 Migration
+
+Grails 8 supports both Hibernate 5 (the default) and Hibernate 7.
+If you are upgrading from Grails 7 and also want to migrate from Hibernate 5
to Hibernate 7, apply the `grails-hibernate7-bom` and the Hibernate 7 plugin in
addition to the standard Grails 8 upgrade steps.
+
+[source,groovy]
+.build.gradle — switch to Hibernate 7
+----
+dependencies {
+ implementation
enforcedPlatform("org.apache.grails:grails-hibernate7-bom:$grailsVersion")
+ implementation 'org.apache.grails:grails-hibernate7'
+}
+----
+
+The following sections cover every breaking change introduced between
Hibernate ORM 5.6.x (used by Grails 7) and Hibernate ORM 7.0.x, and what you
need to do in your Grails application.
+
+===== 23.1 Hibernate Session API Removals
+
+Hibernate 7 removed long-deprecated Hibernate-specific session methods in
favour of the standard JPA equivalents.
+GORM's dynamic methods (`save()`, `delete()`, `get()`, `load()`, `merge()`,
etc.) are **not** affected — these go through GORM's own persistence API and
have been updated internally.
+
+You are only affected if your code calls the Hibernate `Session` or
`StatelessSession` directly (e.g. inside a `withSession` or
`withStatelessSession` block).
+
+[cols="1,1", options="header"]
+|===
+| Removed (Hibernate 5/6)
+| Replacement (Hibernate 7 / JPA)
+
+| `session.save(entity)`
+| `session.persist(entity)`
+
+| `session.update(entity)`
+| `session.merge(entity)`
+
+| `session.saveOrUpdate(entity)`
+| `session.persist(entity)` (new) or `session.merge(entity)` (detached)
+
+| `session.delete(entity)`
+| `session.remove(entity)`
+
+| `session.load(Class, id)`
+| `session.getReference(Class, id)`
+
+| `session.get(Class, id)`
+| `session.find(Class, id)`
+|===
+
+===== 23.2 Removed Hibernate Annotations
+
+The following Hibernate-specific annotations were removed in Hibernate 7.
+Where a replacement exists, migrate before upgrading.
+
+[cols="1,2", options="header"]
+|===
+| Removed annotation
+| Action required
+
+| `@org.hibernate.annotations.Where`
+| Replace with `@org.hibernate.annotations.SQLRestriction`
+
+| `@org.hibernate.annotations.WhereJoinTable`
+| Replace with `@org.hibernate.annotations.SQLJoinTableRestriction`
+
+| `@org.hibernate.annotations.Proxy`
+| Remove — proxy configuration is no longer supported
+
+| `@org.hibernate.annotations.LazyCollection`
+| Remove and use `@ManyToMany(fetch = FetchType.LAZY)` or `EAGER` directly
+
+| `@org.hibernate.annotations.Persister`
+| Remove — custom persisters are no longer supported
+
+| `@org.hibernate.annotations.SelectBeforeUpdate`
+| Remove — behaviour is now configurable via `@DynamicUpdate`
+
+| `@org.hibernate.annotations.Loader`
+| Remove — custom SQL loaders are no longer supported; use `@SQLSelect` on the
entity itself
+|===
+
+NOTE: Grails domain classes that use the `mapping { }` DSL are not affected by
annotation removals.
+These annotations only apply if you are using Hibernate annotations directly
on Java or Groovy classes.
+
+===== 23.3 CascadeType.SAVE_UPDATE Removed
+
+`CascadeType.SAVE_UPDATE` (a Hibernate-specific cascade type) was removed in
Hibernate 7.
+Persisting a transient entity that has detached associations now throws
`EntityExistsException` instead of silently merging.
+
+If your domain mapping or annotated classes used `cascade =
CascadeType.SAVE_UPDATE`, replace it with `cascade = CascadeType.ALL` or
`cascade = [CascadeType.PERSIST, CascadeType.MERGE]` as appropriate.
+
+Also note that automatic `cascade=PERSIST` on `@Id` and `@MapsId` associations
was removed.
Review Comment:
Align whitespace in `cascade=PERSIST` with others.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsHibernateUtil.java:
##########
@@ -62,256 +55,80 @@
* @since 0.4
*/
public class GrailsHibernateUtil extends HibernateRuntimeUtils {
- protected static final Logger LOG =
LoggerFactory.getLogger(GrailsHibernateUtil.class);
-
- public static final String ARGUMENT_FETCH_SIZE = "fetchSize";
- public static final String ARGUMENT_TIMEOUT = "timeout";
- public static final String ARGUMENT_READ_ONLY = "readOnly";
- public static final String ARGUMENT_FLUSH_MODE = "flushMode";
- public static final String ARGUMENT_MAX = "max";
- public static final String ARGUMENT_OFFSET = "offset";
- public static final String ARGUMENT_ORDER = "order";
- public static final String ARGUMENT_SORT = "sort";
- public static final String ORDER_DESC = "desc";
- public static final String ORDER_ASC = "asc";
- public static final String ARGUMENT_FETCH = "fetch";
- public static final String ARGUMENT_IGNORE_CASE = "ignoreCase";
- public static final String ARGUMENT_CACHE = "cache";
- public static final String ARGUMENT_LOCK = "lock";
- public static final Class<?>[] EMPTY_CLASS_ARRAY = {};
-
- private static HibernateProxyHandler proxyHandler = new
HibernateProxyHandler();
-
- public static void populateArgumentsForCriteria(AbstractHibernateDatastore
datastore, Class<?> targetClass, Criteria c, Map argMap, ConversionService
conversionService) {
- populateArgumentsForCriteria(datastore, targetClass, c, argMap,
conversionService, true);
- }
-
- /**
- * Populates criteria arguments for the given target class and arguments
map
- *
- * @param datastore the GrailsApplication instance
- * @param targetClass The target class
- * @param c The criteria instance
- * @param argMap The arguments map
- */
- @SuppressWarnings("rawtypes")
- public static void populateArgumentsForCriteria(AbstractHibernateDatastore
datastore, Class<?> targetClass, Criteria c, Map argMap, ConversionService
conversionService, boolean useDefaultMapping) {
- Integer maxParam = null;
- Integer offsetParam = null;
- if (argMap.containsKey(ARGUMENT_MAX)) {
- maxParam = conversionService.convert(argMap.get(ARGUMENT_MAX),
Integer.class);
- }
- if (argMap.containsKey(ARGUMENT_OFFSET)) {
- offsetParam =
conversionService.convert(argMap.get(ARGUMENT_OFFSET), Integer.class);
- }
- if (argMap.containsKey(ARGUMENT_FETCH_SIZE)) {
-
c.setFetchSize(conversionService.convert(argMap.get(ARGUMENT_FETCH_SIZE),
Integer.class));
- }
- if (argMap.containsKey(ARGUMENT_TIMEOUT)) {
-
c.setTimeout(conversionService.convert(argMap.get(ARGUMENT_TIMEOUT),
Integer.class));
- }
- if (argMap.containsKey(ARGUMENT_FLUSH_MODE)) {
- c.setFlushMode(convertFlushMode(argMap.get(ARGUMENT_FLUSH_MODE)));
- }
- if (argMap.containsKey(ARGUMENT_READ_ONLY)) {
- c.setReadOnly(ClassUtils.getBooleanFromMap(ARGUMENT_READ_ONLY,
argMap));
- }
- String orderParam = (String) argMap.get(ARGUMENT_ORDER);
- Object fetchObj = argMap.get(ARGUMENT_FETCH);
- if (fetchObj instanceof Map) {
- Map fetch = (Map) fetchObj;
- for (Object o : fetch.keySet()) {
- String associationName = (String) o;
- c.setFetchMode(associationName,
getFetchMode(fetch.get(associationName)));
- }
- }
-
- final int max = maxParam == null ? -1 : maxParam;
- final int offset = offsetParam == null ? -1 : offsetParam;
- if (max > -1) {
- c.setMaxResults(max);
- }
- if (offset > -1) {
- c.setFirstResult(offset);
- }
- if (ClassUtils.getBooleanFromMap(ARGUMENT_LOCK, argMap)) {
- c.setLockMode(LockMode.PESSIMISTIC_WRITE);
- c.setCacheable(false);
- }
- else {
- if (argMap.containsKey(ARGUMENT_CACHE)) {
- c.setCacheable(ClassUtils.getBooleanFromMap(ARGUMENT_CACHE,
argMap));
- } else {
- cacheCriteriaByMapping(targetClass, c);
- }
- }
-
- final Object sortObj = argMap.get(ARGUMENT_SORT);
- if (sortObj != null) {
- boolean ignoreCase = true;
- Object caseArg = argMap.get(ARGUMENT_IGNORE_CASE);
- if (caseArg instanceof Boolean) {
- ignoreCase = (Boolean) caseArg;
- }
- if (sortObj instanceof Map) {
- Map sortMap = (Map) sortObj;
- for (Object sort : sortMap.keySet()) {
- final String order = ORDER_DESC.equalsIgnoreCase((String)
sortMap.get(sort)) ? ORDER_DESC : ORDER_ASC;
- addOrderPossiblyNested(datastore, c, targetClass, (String)
sort, order, ignoreCase);
- }
- } else {
- final String sort = (String) sortObj;
- final String order = ORDER_DESC.equalsIgnoreCase(orderParam) ?
ORDER_DESC : ORDER_ASC;
- addOrderPossiblyNested(datastore, c, targetClass, sort, order,
ignoreCase);
- }
- }
- else if (useDefaultMapping) {
- Mapping m = GrailsDomainBinder.getMapping(targetClass);
- if (m != null) {
- Map sortMap = m.getSort().getNamesAndDirections();
- for (Object sort : sortMap.keySet()) {
- final String order = ORDER_DESC.equalsIgnoreCase((String)
sortMap.get(sort)) ? ORDER_DESC : ORDER_ASC;
- addOrderPossiblyNested(datastore, c, targetClass, (String)
sort, order, true);
- }
- }
- }
- }
-
- /**
- * @deprecated No replacement. Do not use.
- */
- @Deprecated
- public static void setBinder(GrailsDomainBinder binder) {
- }
-
- /**
- * Populates criteria arguments for the given target class and arguments
map
- *
- * @param targetClass The target class
- * @param c The criteria instance
- * @param argMap The arguments map
- *
- */
- @Deprecated
- @SuppressWarnings("rawtypes")
- public static void populateArgumentsForCriteria(Class<?> targetClass,
Criteria c, Map argMap, ConversionService conversionService) {
- populateArgumentsForCriteria(null, targetClass, c, argMap,
conversionService);
- }
-
- @SuppressWarnings("rawtypes")
- public static void populateArgumentsForCriteria(Criteria c, Map argMap,
ConversionService conversionService) {
- populateArgumentsForCriteria(null, null, c, argMap, conversionService);
- }
-
- private static FlushMode convertFlushMode(Object object) {
- if (object == null) {
- return null;
- }
- if (object instanceof FlushMode) {
- return (FlushMode) object;
- }
- return FlushMode.valueOf(String.valueOf(object));
- }
-
- /**
- * Add order to criteria, creating necessary subCriteria if nested sort
property (ie. sort:'nested.property').
- */
- private static void addOrderPossiblyNested(AbstractHibernateDatastore
datastore, Criteria c, Class<?> targetClass, String sort, String order, boolean
ignoreCase) {
- int firstDotPos = sort.indexOf(".");
- if (firstDotPos == -1) {
- addOrder(c, sort, order, ignoreCase);
- } else { // nested property
- String sortHead = sort.substring(0, firstDotPos);
- String sortTail = sort.substring(firstDotPos + 1);
- PersistentProperty property =
getGrailsDomainClassProperty(datastore, targetClass, sortHead);
- if (property instanceof Embedded) {
- // embedded objects cannot reference entities (at time of
writing), so no more recursion needed
- addOrder(c, sort, order, ignoreCase);
- } else if (property instanceof Association) {
- Criteria subCriteria = c.createCriteria(sortHead);
- Class<?> propertyTargetClass = ((Association)
property).getAssociatedEntity().getJavaClass();
- GrailsHibernateUtil.cacheCriteriaByMapping(datastore,
propertyTargetClass, subCriteria);
- addOrderPossiblyNested(datastore, subCriteria,
propertyTargetClass, sortTail, order, ignoreCase); // Recurse on nested sort
- }
- }
- }
- /**
- * Add order directly to criteria.
- */
- private static void addOrder(Criteria c, String sort, String order,
boolean ignoreCase) {
- if (ORDER_DESC.equals(order)) {
- c.addOrder(ignoreCase ? Order.desc(sort).ignoreCase() :
Order.desc(sort));
- }
- else {
- c.addOrder(ignoreCase ? Order.asc(sort).ignoreCase() :
Order.asc(sort));
- }
- }
-
- /**
- * Get hold of the GrailsDomainClassProperty represented by the
targetClass' propertyName,
- * assuming targetClass corresponds to a GrailsDomainClass.
- */
- private static PersistentProperty
getGrailsDomainClassProperty(AbstractHibernateDatastore datastore, Class<?>
targetClass, String propertyName) {
- PersistentEntity grailsClass = datastore != null ?
datastore.getMappingContext().getPersistentEntity(targetClass.getName()) : null;
- if (grailsClass == null) {
- throw new IllegalArgumentException("Unexpected: class is not a
domain class:" + targetClass.getName());
- }
- return grailsClass.getPropertyByName(propertyName);
- }
+ private static final String VERSION_8_0 = "8.0";
Review Comment:
Do we need a constant for this?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler.java:
##########
@@ -1,35 +1,42 @@
/*
- * 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
+ * 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
+ * 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.
+ * 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.
*/
Review Comment:
We should not reformat the license header
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsDomainBinder.java:
##########
@@ -0,0 +1,293 @@
+/*
+ * 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.binder;
+
+import org.hibernate.boot.ResourceStreamLocator;
+import org.hibernate.boot.internal.MetadataBuildingContextRootImpl;
+import org.hibernate.boot.model.TypeContributions;
+import org.hibernate.boot.model.TypeContributor;
+import org.hibernate.boot.spi.AdditionalMappingContributions;
+import org.hibernate.boot.spi.AdditionalMappingContributor;
+import org.hibernate.boot.spi.InFlightMetadataCollector;
+import org.hibernate.boot.spi.MetadataBuildingContext;
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
+import org.hibernate.mapping.BasicValue;
+import org.hibernate.service.ServiceRegistry;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.grails.datastore.mapping.core.connections.ConnectionSource;
+import org.grails.orm.hibernate.cfg.HibernateMappingContext;
+import org.grails.orm.hibernate.cfg.MappingCacheHolder;
+import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy;
+import
org.grails.orm.hibernate.cfg.domainbinding.collectionType.CollectionHolder;
+import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentEntity;
+import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover;
+import org.grails.orm.hibernate.cfg.domainbinding.util.BasicValueCreator;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.ColumnNameForPropertyAndPathFetcher;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.DefaultColumnNameFetcher;
+import org.grails.orm.hibernate.cfg.domainbinding.util.GrailsPropertyResolver;
+import org.grails.orm.hibernate.cfg.domainbinding.util.MultiTenantFilterBinder;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.MultiTenantFilterDefinitionBinder;
+import org.grails.orm.hibernate.cfg.domainbinding.util.NamingStrategyProvider;
+import org.grails.orm.hibernate.cfg.domainbinding.util.NamingStrategyWrapper;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.PropertyFromValueCreator;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.SimpleValueColumnFetcher;
+import org.grails.orm.hibernate.cfg.domainbinding.util.TableForManyCalculator;
+
+/**
+ * Handles the binding Grails domain classes and properties to the Hibernate
runtime meta model.
+ * Based on the HbmBinder code in Hibernate core and influenced by
AnnotationsBinder.
+ *
+ * @author Graeme Rocher
+ * @since 0.1
Review Comment:
Since 8.0?
##########
grails-data-hibernate7/core/src/main/java/org/grails/orm/hibernate/cfg/domainbinding/util/GeneratorCreationContextWrapper.java:
##########
Review Comment:
Why was this put in the `java` source set, it is the only file there?
Could it use Groovy `@Delegate`?
```groovy
@CompileStatic
class GeneratorCreationContextWrapper implements GeneratorCreationContext {
@Delegate(excludes = ['getValue'])
private final GeneratorCreationContext delegate
private final Value value
GeneratorCreationContextWrapper(GeneratorCreationContext delegate, Value
value) {
this.delegate = delegate
this.value = value
}
@Override
Value getValue() {
value ?: delegate.value
}
}
```
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler.java:
##########
@@ -85,13 +155,17 @@ public Object unwrap(Object object) {
*/
@Override
public Serializable getIdentifier(Object o) {
+ if (o instanceof EntityProxy) {
+ return ((EntityProxy) o).getProxyKey();
+ }
+ ProxyInstanceMetaClass proxyMc = getProxyInstanceMetaClass(o);
+ if (proxyMc != null) {
+ return proxyMc.getKey();
+ }
if (o instanceof HibernateProxy) {
- return ((HibernateProxy)
o).getHibernateLazyInitializer().getIdentifier();
+ return (Serializable) ((HibernateProxy)
o).getHibernateLazyInitializer().getIdentifier();
Review Comment:
Do we need to cast to `Serializable`?
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsDomainBinder.java:
##########
@@ -0,0 +1,293 @@
+/*
+ * 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.binder;
+
+import org.hibernate.boot.ResourceStreamLocator;
+import org.hibernate.boot.internal.MetadataBuildingContextRootImpl;
+import org.hibernate.boot.model.TypeContributions;
+import org.hibernate.boot.model.TypeContributor;
+import org.hibernate.boot.spi.AdditionalMappingContributions;
+import org.hibernate.boot.spi.AdditionalMappingContributor;
+import org.hibernate.boot.spi.InFlightMetadataCollector;
+import org.hibernate.boot.spi.MetadataBuildingContext;
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
+import org.hibernate.mapping.BasicValue;
+import org.hibernate.service.ServiceRegistry;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.grails.datastore.mapping.core.connections.ConnectionSource;
+import org.grails.orm.hibernate.cfg.HibernateMappingContext;
+import org.grails.orm.hibernate.cfg.MappingCacheHolder;
+import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy;
+import
org.grails.orm.hibernate.cfg.domainbinding.collectionType.CollectionHolder;
+import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentEntity;
+import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover;
+import org.grails.orm.hibernate.cfg.domainbinding.util.BasicValueCreator;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.ColumnNameForPropertyAndPathFetcher;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.DefaultColumnNameFetcher;
+import org.grails.orm.hibernate.cfg.domainbinding.util.GrailsPropertyResolver;
+import org.grails.orm.hibernate.cfg.domainbinding.util.MultiTenantFilterBinder;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.MultiTenantFilterDefinitionBinder;
+import org.grails.orm.hibernate.cfg.domainbinding.util.NamingStrategyProvider;
+import org.grails.orm.hibernate.cfg.domainbinding.util.NamingStrategyWrapper;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.PropertyFromValueCreator;
+import
org.grails.orm.hibernate.cfg.domainbinding.util.SimpleValueColumnFetcher;
+import org.grails.orm.hibernate.cfg.domainbinding.util.TableForManyCalculator;
+
+/**
+ * Handles the binding Grails domain classes and properties to the Hibernate
runtime meta model.
+ * Based on the HbmBinder code in Hibernate core and influenced by
AnnotationsBinder.
+ *
+ * @author Graeme Rocher
Review Comment:
Remove author tag?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/dirtychecking/HibernateUpdateFromListenerSpec.groovy:
##########
@@ -85,6 +85,9 @@ class HibernateUpdateFromListenerSpec extends Specification {
if (event.entityObject instanceof Person) {
Person person = (Person) event.entityObject
person.occupation = person.occupation + " listener"
+ if (event.getEntityAccess() != null) {
+ event.getEntityAccess().setProperty("occupation",
person.occupation)
+ }
Review Comment:
Use groovy property accessor?
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsDomainBinder.java:
##########
Review Comment:
If we use `var`, these "walls of text" becomes easier to read.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsHibernateUtil.java:
##########
@@ -62,256 +55,80 @@
* @since 0.4
*/
public class GrailsHibernateUtil extends HibernateRuntimeUtils {
- protected static final Logger LOG =
LoggerFactory.getLogger(GrailsHibernateUtil.class);
-
- public static final String ARGUMENT_FETCH_SIZE = "fetchSize";
- public static final String ARGUMENT_TIMEOUT = "timeout";
- public static final String ARGUMENT_READ_ONLY = "readOnly";
- public static final String ARGUMENT_FLUSH_MODE = "flushMode";
- public static final String ARGUMENT_MAX = "max";
- public static final String ARGUMENT_OFFSET = "offset";
- public static final String ARGUMENT_ORDER = "order";
- public static final String ARGUMENT_SORT = "sort";
- public static final String ORDER_DESC = "desc";
- public static final String ORDER_ASC = "asc";
- public static final String ARGUMENT_FETCH = "fetch";
- public static final String ARGUMENT_IGNORE_CASE = "ignoreCase";
- public static final String ARGUMENT_CACHE = "cache";
- public static final String ARGUMENT_LOCK = "lock";
- public static final Class<?>[] EMPTY_CLASS_ARRAY = {};
-
- private static HibernateProxyHandler proxyHandler = new
HibernateProxyHandler();
-
- public static void populateArgumentsForCriteria(AbstractHibernateDatastore
datastore, Class<?> targetClass, Criteria c, Map argMap, ConversionService
conversionService) {
- populateArgumentsForCriteria(datastore, targetClass, c, argMap,
conversionService, true);
- }
-
- /**
- * Populates criteria arguments for the given target class and arguments
map
- *
- * @param datastore the GrailsApplication instance
- * @param targetClass The target class
- * @param c The criteria instance
- * @param argMap The arguments map
- */
- @SuppressWarnings("rawtypes")
- public static void populateArgumentsForCriteria(AbstractHibernateDatastore
datastore, Class<?> targetClass, Criteria c, Map argMap, ConversionService
conversionService, boolean useDefaultMapping) {
- Integer maxParam = null;
- Integer offsetParam = null;
- if (argMap.containsKey(ARGUMENT_MAX)) {
- maxParam = conversionService.convert(argMap.get(ARGUMENT_MAX),
Integer.class);
- }
- if (argMap.containsKey(ARGUMENT_OFFSET)) {
- offsetParam =
conversionService.convert(argMap.get(ARGUMENT_OFFSET), Integer.class);
- }
- if (argMap.containsKey(ARGUMENT_FETCH_SIZE)) {
-
c.setFetchSize(conversionService.convert(argMap.get(ARGUMENT_FETCH_SIZE),
Integer.class));
- }
- if (argMap.containsKey(ARGUMENT_TIMEOUT)) {
-
c.setTimeout(conversionService.convert(argMap.get(ARGUMENT_TIMEOUT),
Integer.class));
- }
- if (argMap.containsKey(ARGUMENT_FLUSH_MODE)) {
- c.setFlushMode(convertFlushMode(argMap.get(ARGUMENT_FLUSH_MODE)));
- }
- if (argMap.containsKey(ARGUMENT_READ_ONLY)) {
- c.setReadOnly(ClassUtils.getBooleanFromMap(ARGUMENT_READ_ONLY,
argMap));
- }
- String orderParam = (String) argMap.get(ARGUMENT_ORDER);
- Object fetchObj = argMap.get(ARGUMENT_FETCH);
- if (fetchObj instanceof Map) {
- Map fetch = (Map) fetchObj;
- for (Object o : fetch.keySet()) {
- String associationName = (String) o;
- c.setFetchMode(associationName,
getFetchMode(fetch.get(associationName)));
- }
- }
-
- final int max = maxParam == null ? -1 : maxParam;
- final int offset = offsetParam == null ? -1 : offsetParam;
- if (max > -1) {
- c.setMaxResults(max);
- }
- if (offset > -1) {
- c.setFirstResult(offset);
- }
- if (ClassUtils.getBooleanFromMap(ARGUMENT_LOCK, argMap)) {
- c.setLockMode(LockMode.PESSIMISTIC_WRITE);
- c.setCacheable(false);
- }
- else {
- if (argMap.containsKey(ARGUMENT_CACHE)) {
- c.setCacheable(ClassUtils.getBooleanFromMap(ARGUMENT_CACHE,
argMap));
- } else {
- cacheCriteriaByMapping(targetClass, c);
- }
- }
-
- final Object sortObj = argMap.get(ARGUMENT_SORT);
- if (sortObj != null) {
- boolean ignoreCase = true;
- Object caseArg = argMap.get(ARGUMENT_IGNORE_CASE);
- if (caseArg instanceof Boolean) {
- ignoreCase = (Boolean) caseArg;
- }
- if (sortObj instanceof Map) {
- Map sortMap = (Map) sortObj;
- for (Object sort : sortMap.keySet()) {
- final String order = ORDER_DESC.equalsIgnoreCase((String)
sortMap.get(sort)) ? ORDER_DESC : ORDER_ASC;
- addOrderPossiblyNested(datastore, c, targetClass, (String)
sort, order, ignoreCase);
- }
- } else {
- final String sort = (String) sortObj;
- final String order = ORDER_DESC.equalsIgnoreCase(orderParam) ?
ORDER_DESC : ORDER_ASC;
- addOrderPossiblyNested(datastore, c, targetClass, sort, order,
ignoreCase);
- }
- }
- else if (useDefaultMapping) {
- Mapping m = GrailsDomainBinder.getMapping(targetClass);
- if (m != null) {
- Map sortMap = m.getSort().getNamesAndDirections();
- for (Object sort : sortMap.keySet()) {
- final String order = ORDER_DESC.equalsIgnoreCase((String)
sortMap.get(sort)) ? ORDER_DESC : ORDER_ASC;
- addOrderPossiblyNested(datastore, c, targetClass, (String)
sort, order, true);
- }
- }
- }
- }
-
- /**
- * @deprecated No replacement. Do not use.
- */
- @Deprecated
- public static void setBinder(GrailsDomainBinder binder) {
- }
-
- /**
- * Populates criteria arguments for the given target class and arguments
map
- *
- * @param targetClass The target class
- * @param c The criteria instance
- * @param argMap The arguments map
- *
- */
- @Deprecated
- @SuppressWarnings("rawtypes")
- public static void populateArgumentsForCriteria(Class<?> targetClass,
Criteria c, Map argMap, ConversionService conversionService) {
- populateArgumentsForCriteria(null, targetClass, c, argMap,
conversionService);
- }
-
- @SuppressWarnings("rawtypes")
- public static void populateArgumentsForCriteria(Criteria c, Map argMap,
ConversionService conversionService) {
- populateArgumentsForCriteria(null, null, c, argMap, conversionService);
- }
-
- private static FlushMode convertFlushMode(Object object) {
- if (object == null) {
- return null;
- }
- if (object instanceof FlushMode) {
- return (FlushMode) object;
- }
- return FlushMode.valueOf(String.valueOf(object));
- }
-
- /**
- * Add order to criteria, creating necessary subCriteria if nested sort
property (ie. sort:'nested.property').
- */
- private static void addOrderPossiblyNested(AbstractHibernateDatastore
datastore, Criteria c, Class<?> targetClass, String sort, String order, boolean
ignoreCase) {
- int firstDotPos = sort.indexOf(".");
- if (firstDotPos == -1) {
- addOrder(c, sort, order, ignoreCase);
- } else { // nested property
- String sortHead = sort.substring(0, firstDotPos);
- String sortTail = sort.substring(firstDotPos + 1);
- PersistentProperty property =
getGrailsDomainClassProperty(datastore, targetClass, sortHead);
- if (property instanceof Embedded) {
- // embedded objects cannot reference entities (at time of
writing), so no more recursion needed
- addOrder(c, sort, order, ignoreCase);
- } else if (property instanceof Association) {
- Criteria subCriteria = c.createCriteria(sortHead);
- Class<?> propertyTargetClass = ((Association)
property).getAssociatedEntity().getJavaClass();
- GrailsHibernateUtil.cacheCriteriaByMapping(datastore,
propertyTargetClass, subCriteria);
- addOrderPossiblyNested(datastore, subCriteria,
propertyTargetClass, sortTail, order, ignoreCase); // Recurse on nested sort
- }
- }
- }
- /**
- * Add order directly to criteria.
- */
- private static void addOrder(Criteria c, String sort, String order,
boolean ignoreCase) {
- if (ORDER_DESC.equals(order)) {
- c.addOrder(ignoreCase ? Order.desc(sort).ignoreCase() :
Order.desc(sort));
- }
- else {
- c.addOrder(ignoreCase ? Order.asc(sort).ignoreCase() :
Order.asc(sort));
- }
- }
-
- /**
- * Get hold of the GrailsDomainClassProperty represented by the
targetClass' propertyName,
- * assuming targetClass corresponds to a GrailsDomainClass.
- */
- private static PersistentProperty
getGrailsDomainClassProperty(AbstractHibernateDatastore datastore, Class<?>
targetClass, String propertyName) {
- PersistentEntity grailsClass = datastore != null ?
datastore.getMappingContext().getPersistentEntity(targetClass.getName()) : null;
- if (grailsClass == null) {
- throw new IllegalArgumentException("Unexpected: class is not a
domain class:" + targetClass.getName());
- }
- return grailsClass.getPropertyByName(propertyName);
- }
+ private static final String VERSION_8_0 = "8.0";
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#FETCH_SIZE} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_FETCH_SIZE =
HibernateQueryArgument.FETCH_SIZE.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#TIMEOUT} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_TIMEOUT =
HibernateQueryArgument.TIMEOUT.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#READ_ONLY} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_READ_ONLY =
HibernateQueryArgument.READ_ONLY.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#FLUSH_MODE} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_FLUSH_MODE =
HibernateQueryArgument.FLUSH_MODE.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#MAX} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_MAX =
HibernateQueryArgument.MAX.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#OFFSET} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_OFFSET =
HibernateQueryArgument.OFFSET.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#ORDER} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_ORDER =
HibernateQueryArgument.ORDER.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#SORT} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_SORT =
HibernateQueryArgument.SORT.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#ORDER_DESC} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ORDER_DESC =
HibernateQueryArgument.ORDER_DESC.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#ORDER_ASC} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ORDER_ASC =
HibernateQueryArgument.ORDER_ASC.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#FETCH} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_FETCH =
HibernateQueryArgument.FETCH.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#IGNORE_CASE} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_IGNORE_CASE =
HibernateQueryArgument.IGNORE_CASE.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#CACHE} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_CACHE =
HibernateQueryArgument.CACHE.value();
+ /** @deprecated Use {@link
org.grails.orm.hibernate.query.HibernateQueryArgument#LOCK} */
+ @Deprecated(since = VERSION_8_0, forRemoval = true)
+ public static final String ARGUMENT_LOCK =
HibernateQueryArgument.LOCK.value();
- /**
- * Configures the criteria instance to cache based on the configured
mapping.
- *
- * @param targetClass The target class
- * @param criteria The criteria
- */
- public static void cacheCriteriaByMapping(Class<?> targetClass, Criteria
criteria) {
- Mapping m = GrailsDomainBinder.getMapping(targetClass);
- if (m != null && m.getCache() != null && m.getCache().getEnabled()) {
- criteria.setCacheable(true);
- }
- }
+ protected static final Logger LOG =
LoggerFactory.getLogger(GrailsHibernateUtil.class);
- public static void cacheCriteriaByMapping(AbstractHibernateDatastore
datastore, Class<?> targetClass, Criteria criteria) {
- cacheCriteriaByMapping(targetClass, criteria);
- }
+ private static HibernateProxyHandler proxyHandler = new
HibernateProxyHandler();
- /**
- * Retrieves the fetch mode for the specified instance; otherwise returns
the default FetchMode.
- *
- * @param object The object, converted to a string
- * @return The FetchMode
- */
- public static FetchMode getFetchMode(Object object) {
- String name = object != null ? object.toString() : "default";
- if (name.equalsIgnoreCase(FetchMode.JOIN.toString()) ||
name.equalsIgnoreCase("eager")) {
- return FetchMode.JOIN;
- }
- if (name.equalsIgnoreCase(FetchMode.SELECT.toString()) ||
name.equalsIgnoreCase("lazy")) {
- return FetchMode.SELECT;
- }
- return FetchMode.DEFAULT;
+ public static void setProxyHandler(HibernateProxyHandler handler) {
Review Comment:
This is allowing changes to global state. Can we add overloaded methods with
an additional HibernateProxyHandler parameter instead?
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/hibernate/mapping/HibernateMappingBuilderSpec.groovy:
##########
@@ -0,0 +1,892 @@
+/*
+ * 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.hibernate.mapping
+
+import jakarta.persistence.AccessType
+import org.grails.orm.hibernate.cfg.CacheConfig
+import org.grails.orm.hibernate.cfg.HibernateCompositeIdentity
+import org.grails.orm.hibernate.cfg.Mapping
+import org.grails.orm.hibernate.cfg.PropertyConfig
+import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateMappingBuilder
+import org.hibernate.FetchMode
+import spock.lang.Specification
+
+/**
+ * Covers branches of {@link HibernateMappingBuilder} not exercised by
+ */
+class HibernateMappingBuilderSpec extends Specification {
+
+ private HibernateMappingBuilder builder(String name = 'Foo') {
+ new HibernateMappingBuilder(new Mapping(), name)
+ }
+
+ private Mapping evaluate(@DelegatesTo(HibernateMappingBuilder) Closure cl)
{
+ builder().evaluate(cl)
+ }
+
+ //
-------------------------------------------------------------------------
+ // table / catalog / schema / comment
+ //
-------------------------------------------------------------------------
+
+ def "table with name only"() {
+ when:
+ Mapping m = evaluate { table 'myTable' }
+
+ then:
+ m.tableName == 'myTable'
+ }
+
+ def "table with catalog and schema"() {
+ when:
+ Mapping m = evaluate { table name: 'table', catalog: 'CRM', schema:
'dbo' }
+
+ then:
+ m.table.name == 'table'
+ m.table.schema == 'dbo'
+ m.table.catalog == 'CRM'
+ }
+
+ def "table comment is stored"() {
+ when:
+ Mapping m = evaluate { comment 'wahoo' }
+
+ then:
+ m.comment == 'wahoo'
+ }
+
+ //
-------------------------------------------------------------------------
+ // version / autoTimestamp
+ //
-------------------------------------------------------------------------
+
+ def "version column can be changed"() {
+ when:
+ Mapping m = evaluate { version 'v_number' }
+
+ then:
+ m.getPropertyConfig("version").column == 'v_number'
+ }
+
+ def "versioning can be disabled"() {
+ when:
+ Mapping m = evaluate { version false }
+
+ then:
+ !m.versioned
+ }
+
+ def "autoTimestamp can be disabled"() {
+ when:
+ Mapping m = evaluate { autoTimestamp false }
+
+ then:
+ !m.autoTimestamp
+ }
+
+ //
-------------------------------------------------------------------------
+ // discriminator
+ //
-------------------------------------------------------------------------
+
+ def "discriminator value only"() {
+ when:
+ Mapping m = evaluate { discriminator 'one' }
+
+ then:
+ m.discriminator.value == 'one'
+ m.discriminator.column == null
+ }
+
+ def "discriminator with column name"() {
+ when:
+ Mapping m = evaluate { discriminator value: 'one', column: 'type' }
+
+ then:
+ m.discriminator.value == 'one'
+ m.discriminator.column.name == 'type'
+ }
+
+ def "discriminator with column map"() {
+ when:
+ Mapping m = evaluate { discriminator value: 'one', column: [name:
'type', sqlType: 'integer'] }
+
+ then:
+ m.discriminator.value == 'one'
+ m.discriminator.column.name == 'type'
+ m.discriminator.column.sqlType == 'integer'
+ }
+
+ def "discriminator with formula and other settings"() {
+ when:
+ Mapping m = evaluate {
+ discriminator value: '1', formula: "case when CLASS_TYPE in ('a',
'b', 'c') then 0 else 1 end", type: 'integer', insert: false
+ }
+
+ then:
+ m.discriminator.value == '1'
+ m.discriminator.formula == "case when CLASS_TYPE in ('a', 'b', 'c')
then 0 else 1 end"
+ m.discriminator.type == 'integer'
+ !m.discriminator.insertable
+ }
+
+ //
-------------------------------------------------------------------------
+ // inheritance
+ //
-------------------------------------------------------------------------
+
+ def "tablePerHierarchy false disables it"() {
+ when:
+ Mapping m = evaluate { tablePerHierarchy false }
+
+ then:
+ !m.tablePerHierarchy
+ }
+
+ def "tablePerSubclass true disables tablePerHierarchy"() {
+ when:
+ Mapping m = evaluate { tablePerSubclass true }
+
+ then:
+ !m.tablePerHierarchy
+ }
+
+ def "tablePerConcreteClass true enables it and disables
tablePerHierarchy"() {
+ when:
+ Mapping m = evaluate { tablePerConcreteClass true }
+
+ then:
+ m.tablePerConcreteClass
+ !m.tablePerHierarchy
+ }
+
+ //
-------------------------------------------------------------------------
+ // cache settings
+ //
-------------------------------------------------------------------------
+
+ def "default cache strategy"() {
+ when:
+ Mapping m = evaluate { cache true }
+
+ then:
+ m.cache.usage.toString() == 'read-write'
+ m.cache.include.toString() == 'all'
+ }
+
+ def "custom cache strategy"() {
+ when:
+ Mapping m = evaluate { cache usage: 'read-only', include: 'non-lazy' }
+
+ then:
+ m.cache.usage.toString() == 'read-only'
+ m.cache.include.toString() == 'non-lazy'
+ }
+
+ def "custom cache strategy with usage string only"() {
+ when:
+ Mapping m = evaluate { cache 'read-only' }
+
+ then:
+ m.cache.usage.toString() == 'read-only'
+ m.cache.include.toString() == 'all'
+ }
+
+ def "invalid cache values are ignored and defaults used"() {
+ when:
+ Mapping m = evaluate { cache usage: 'rubbish', include: 'more-rubbish'
}
+
+ then:
+ m.cache.usage.toString() == 'read-write'
+ m.cache.include.toString() == 'all'
+ }
+
+ //
-------------------------------------------------------------------------
+ // identity / id
+ //
-------------------------------------------------------------------------
+
+ def "identity column mapping"() {
+ when:
+ Mapping m = evaluate { id column: 'foo_id', type: Integer }
+
+ then:
+ m.identity.type == Long // Default remains Long? No, wait.
+ // In HibernateMappingBuilderTests:
+ // assertEquals Long, mapping.identity.type
+ // assertEquals 'foo_id', mapping.getPropertyConfig("id").column
+ // assertEquals Integer, mapping.getPropertyConfig("id").type
Review Comment:
Remove redundant comment?
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/hibernate/mapping/HibernateMappingBuilderSpec.groovy:
##########
@@ -0,0 +1,892 @@
+/*
+ * 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.hibernate.mapping
+
+import jakarta.persistence.AccessType
+import org.grails.orm.hibernate.cfg.CacheConfig
+import org.grails.orm.hibernate.cfg.HibernateCompositeIdentity
+import org.grails.orm.hibernate.cfg.Mapping
+import org.grails.orm.hibernate.cfg.PropertyConfig
+import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateMappingBuilder
+import org.hibernate.FetchMode
+import spock.lang.Specification
+
+/**
+ * Covers branches of {@link HibernateMappingBuilder} not exercised by
Review Comment:
Javadoc seems to be cut off mid-sentence?
##########
.agents/skills/hibernate-developer/SKILL.md:
##########
@@ -0,0 +1,119 @@
+---
+name: hibernate-developer
+description: Guide for working in the grails-data-hibernate7 module,
especially Hibernate 7 domain binding, mapping migration, generators, and
integration tests. Use this when changing code or tests under
grails-data-hibernate7.
+license: Apache-2.0
+---
+<!--
+SPDX-License-Identifier: Apache-2.0
+
+Licensed to the Apache Software Foundation (ASF) under one or more contributor
license agreements; and to You under the Apache License, Version 2.0.
+-->
+
+## What I Do
+
+- Provide repository-specific guidance for the `grails-data-hibernate7`
project.
+- Help with Hibernate 7 migration work in domain binding, mapping metadata,
identifiers, generators, collections, and second-pass binding.
Review Comment:
Should this skill help with "migration work"?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler.java:
##########
@@ -129,12 +206,58 @@ public boolean isProxy(Object o) {
*/
@Override
public void initialize(Object o) {
- Hibernate.initialize(o);
+ if (o instanceof EntityProxy) {
+ ((EntityProxy) o).initialize();
+ }
+ else {
+ ProxyInstanceMetaClass proxyMc = getProxyInstanceMetaClass(o);
+ if (proxyMc != null) {
+ proxyMc.getProxyTarget();
+ }
+ else {
+ Hibernate.initialize(o);
+ }
+ }
+ }
+
+ private ProxyInstanceMetaClass getProxyInstanceMetaClass(Object o) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("getProxyInstanceMetaClass() - checking if object is
GroovyObject: {}", o != null ? o.getClass().getName() : "null");
+ }
+ if (o instanceof GroovyObject) {
+ MetaClass mc = ((GroovyObject) o).getMetaClass();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("getProxyInstanceMetaClass() - metaClass type: {}",
mc.getClass().getName());
+ }
+ if (mc instanceof HandleMetaClass) {
+ mc = ((HandleMetaClass) mc).getAdaptee();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("getProxyInstanceMetaClass() - handleMetaClass
adaptee type: {}", mc.getClass().getName());
+ }
+ }
+ if (mc instanceof ProxyInstanceMetaClass) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("getProxyInstanceMetaClass() - found
ProxyInstanceMetaClass");
+ }
+ return (ProxyInstanceMetaClass) mc;
+ }
+ }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("getProxyInstanceMetaClass() - no ProxyInstanceMetaClass
found");
+ }
+ return null;
}
@Override
public <T> T createProxy(Session session, Class<T> type, Serializable key)
{
- throw new UnsupportedOperationException("createProxy not supported in
HibernateProxyHandler");
+ org.hibernate.Session hibSession = null;
+ if (session.getNativeInterface() instanceof GrailsHibernateTemplate
grailsHibernateTemplate) {
+ hibSession = grailsHibernateTemplate.getSession();
+ }
+ if (hibSession == null) {
+ throw new IllegalStateException("Could not obtain native Hibernate
Session from Session#getNativeInterface()");
+ }
+ return (T) hibSession.getReference(type, key);
Review Comment:
Do we need to cast?
##########
.agents/skills/hibernate-developer/SKILL.md:
##########
@@ -0,0 +1,119 @@
+---
+name: hibernate-developer
+description: Guide for working in the grails-data-hibernate7 module,
especially Hibernate 7 domain binding, mapping migration, generators, and
integration tests. Use this when changing code or tests under
grails-data-hibernate7.
+license: Apache-2.0
+---
+<!--
+SPDX-License-Identifier: Apache-2.0
+
+Licensed to the Apache Software Foundation (ASF) under one or more contributor
license agreements; and to You under the Apache License, Version 2.0.
+-->
+
+## What I Do
+
+- Provide repository-specific guidance for the `grails-data-hibernate7`
project.
+- Help with Hibernate 7 migration work in domain binding, mapping metadata,
identifiers, generators, collections, and second-pass binding.
+- Guide changes around `GrailsDomainBinder`, `GrailsPropertyBinder`,
`IdentityBinder`, `VersionBinder`, collection binders, and related utilities.
+- Keep changes aligned with the testing constraints and migration status
documented in `grails-data-hibernate7/AGENTS.md`.
Review Comment:
I cannot find any `grails-data-hibernate7/AGENTS.md` file?
##########
grails-data-hibernate7/grails-plugin/build.gradle:
##########
@@ -56,9 +59,10 @@ dependencies {
exclude group:'org.apache.grails', module:'grails-core'
exclude group:'javax.transaction', module:'jta'
}
+ api project(':grails-spring')
+ api project(':grails-core')
Review Comment:
Why was this changed from `compileOnly`?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/PropertyConfig.groovy:
##########
@@ -473,4 +476,8 @@ class PropertyConfig extends Property {
}
return pc
}
+
+ boolean hasJoinKeyMapping() {
Review Comment:
`@since`?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/txs/CustomIsolationLevelSpec.groovy:
##########
@@ -36,7 +36,7 @@ class CustomIsolationLevelSpec extends Specification {
@AutoCleanup @Shared HibernateDatastore hibernateDatastore = new
HibernateDatastore(Product, Attribute)
- @Issue('https://github.com/apache/grails-data-mapping/issues/952')
+ @Issue('https://github.com/grails/grails-data-mapping/issues/952')
Review Comment:
Why change from `apache` -> `grails`?
##########
.agents/skills/hibernate-developer/SKILL.md:
##########
@@ -0,0 +1,119 @@
+---
+name: hibernate-developer
+description: Guide for working in the grails-data-hibernate7 module,
especially Hibernate 7 domain binding, mapping migration, generators, and
integration tests. Use this when changing code or tests under
grails-data-hibernate7.
+license: Apache-2.0
+---
+<!--
+SPDX-License-Identifier: Apache-2.0
+
+Licensed to the Apache Software Foundation (ASF) under one or more contributor
license agreements; and to You under the Apache License, Version 2.0.
+-->
+
+## What I Do
+
+- Provide repository-specific guidance for the `grails-data-hibernate7`
project.
+- Help with Hibernate 7 migration work in domain binding, mapping metadata,
identifiers, generators, collections, and second-pass binding.
+- Guide changes around `GrailsDomainBinder`, `GrailsPropertyBinder`,
`IdentityBinder`, `VersionBinder`, collection binders, and related utilities.
+- Keep changes aligned with the testing constraints and migration status
documented in `grails-data-hibernate7/AGENTS.md`.
+
+## When to Use Me
+
+Activate this skill when working on the Hibernate 7 module, especially for:
+
+- Changes under `grails-data-hibernate7/**`.
+- Hibernate 7 mapping and metadata binding work.
+- Identifier, version, collection, association, or generator binding changes.
+- Hibernate 7 regression fixes and migration follow-up tasks.
+- Specs that exercise Hibernate-backed mapping behavior rather than
lightweight unit behavior.
+
+## Module Context
+
+This skill is for the Grails framework's Hibernate 7 integration module, not
for a Grails application. Prefer guidance from this skill over generic Grails
app patterns when working in `grails-data-hibernate7`.
+
+`GrailsDomainBinder` is the main entry point for binding Grails domain classes
to Hibernate metadata. Changes often ripple through:
+
+- `org.grails.orm.hibernate.cfg`
+- `org.grails.orm.hibernate.cfg.domainbinding`
+- `org.grails.orm.hibernate.cfg.domainbinding.collectionType`
+- `org.grails.orm.hibernate.cfg.domainbinding.secondpass`
+- `org.grails.orm.hibernate.cfg.domainbinding.generator`
+
+## Key Classes and Responsibilities
+
+### Main Binding Flow
+
+- `GrailsDomainBinder`: central coordinator for Hibernate 7 mapping
contribution.
+- `GrailsPropertyBinder`: main coordinator for converting persistent
properties into Hibernate `Value` instances.
+- `PropertyFromValueCreator`: shared utility for creating Hibernate `Property`
instances from a bound `Value`.
+
+### Identifier and Version Binding
+
+- `IdentityBinder`: coordinates identifier binding.
+- `SimpleIdBinder`: handles simple identifiers.
+- `CompositeIdBinder`: handles composite identifiers.
+- `VersionBinder`: binds optimistic locking version properties.
+- `NaturalIdentifierBinder`: binds `naturalId` properties.
+
+### Associations and Collections
+
+- `OneToOneBinder`, `ManyToOneBinder`, `ManyToOneValuesBinder`: association
binding.
+- `CollectionBinder`: collection mapping.
+- `CollectionSecondPassBinder`, `ListSecondPassBinder`, `MapSecondPassBinder`:
second-pass association and collection binding.
+- `CollectionHolder` plus the collection type classes: carry collection
metadata through binding.
+
+### Value and Column Binding
+
+- `SimpleValueBinder`: binds simple properties.
+- `SimpleValueColumnBinder`: binds columns to simple values.
+- `ComponentBinder`, `ComponentPropertyBinder`: embedded/component binding.
+- `EnumTypeBinder`: enum mapping.
+
+### Generators
+
+- `BasicValueCreator`: creates identifier values and generators.
+- `GrailsSequenceWrapper`, `GrailsSequenceGeneratorEnum`: generator
integration helpers.
+- `GrailsIdentityGenerator`, `GrailsIncrementGenerator`,
`GrailsNativeGenerator`, `GrailsSequenceStyleGenerator`,
`GrailsTableGenerator`: Grails-specific Hibernate 7 generator implementations.
+
+## Current Module Guidance
+
+Keep these module-specific expectations in mind:
+
+- `GrailsPropertyBinder` has already been simplified to a unified
binder-dispatch structure. Preserve that consolidation instead of reintroducing
scattered property creation or ad hoc branching.
+- Property creation and addition should stay centralized through callers using
`PropertyFromValueCreator` where applicable.
+- Utility classes in `domainbinding.util` should prefer Hibernate-aware GORM
types internally, but public signatures may still need base interfaces when
Spock mocks require them.
+- `GrailsIncrementGenerator` still contains reflection-based Hibernate 7
compatibility workarounds; avoid broad refactors unless the change explicitly
addresses that area.
+
+## Testing Rules
+
+When touching `grails-data-hibernate7`, test through real Hibernate wiring
rather than assuming mocks are enough.
+
+- Use `HibernateGormDatastoreSpec` for Hibernate 7 integration and
domain-binding specifications.
+- Prefer `manager.registerDomainClasses(...)` in `setupSpec()` to register
entities for specs.
+- Define test entities as top-level classes in the same Groovy spec file.
+- Ensure test domain class names are globally unique within the package to
avoid collisions during parallel execution.
+- Prefer real entities over heavy mocking for binder logic.
+
+## Change Workflow
+
+1. Identify which binder, creator, generator, fetcher, or second-pass class
owns the behavior.
+2. Trace whether the change affects only `Value` creation, `Property`
creation, or both.
+3. Preserve the existing separation between logical mapping decisions and
Hibernate object construction.
+4. Update or add specs in `grails-data-hibernate7` that exercise the affected
behavior through the public Hibernate-backed path.
+5. Run the relevant Hibernate 7 module tests, and expand test coverage when
binder flow or entity registration behavior changes.
+
+## Pitfalls to Avoid
+
+- Do not treat this module like a simple Grails application layer; it is
framework and mapping infrastructure code.
+- Do not reintroduce duplicated property-creation logic if a shared binder or
creator already owns it.
+- Do not rely on unit-only mocking for Hibernate internals when the behavior
depends on real metadata binding.
+- Do not use nested or inner entity classes in Hibernate 7 specs when
top-level classes are required for AST transforms and reliable registration.
+
+## Known Status and Constraints
+
+- The Hibernate 7 binder migration is largely in migrated state across the
main binders, collection types, second-pass binders, generators, and utilities.
+- Unidirectional many-to-many support in `CollectionSecondPassBinder` is
implemented.
+- `GrailsIncrementGenerator` reflection hacks remain a known temporary
compromise until a later Hibernate upgrade removes the need.
+
Review Comment:
Are these comments necessary/up-to-date?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler.java:
##########
@@ -40,13 +47,59 @@
*/
public class HibernateProxyHandler implements ProxyHandler, ProxyFactory {
+ private static final Logger LOG =
LoggerFactory.getLogger(HibernateProxyHandler.class);
Review Comment:
Do we need all the debug logging in this class. Its like 60% of the code.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/dirtychecking/PropertyFieldSpec.groovy:
##########
@@ -33,7 +33,7 @@ class PropertyFieldSpec extends Specification {
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(getClass().getPackage())
@Rollback
- @Issue('https://github.com/apache/grails-data-mapping/issues/934')
+ @Issue('https://github.com/grails/grails-data-mapping/issues/934')
Review Comment:
This should probably not change from `apache` -> `grails`.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/SaveWithInvalidEntitySpec.groovy:
##########
@@ -30,24 +31,30 @@ import spock.lang.Specification
/**
* Created by graemerocher on 03/05/2017.
*/
+//TODO Should this test be rewritten?
Review Comment:
Is this comment still warranted?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/SaveWithInvalidEntitySpec.groovy:
##########
@@ -30,24 +31,30 @@ import spock.lang.Specification
/**
* Created by graemerocher on 03/05/2017.
*/
+//TODO Should this test be rewritten?
class SaveWithInvalidEntitySpec extends Specification {
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(A, B)
/**
- * This currently fails with a NPE. See explanation
https://github.com/apache/grails-core/issues/14616#issuecomment-298943022
+ * This currently fails with a NPE. See explanation
https://github.com/grails/grails-core/issues/10604#issuecomment-298943022
*/
@Rollback
- @Ignore
- @Issue('https://github.com/apache/grails-core/issues/10604')
+ @Issue('https://github.com/grails/grails-core/issues/10604')
void "test save with an invalid entity"() {
+ given:
+ def b = new B(field2: "test")
+ def a = new A(b: b)
+
when:
- hibernateDatastore.currentSession.persist(new A(b:new B(field2:
"test")))
+ hibernateDatastore.currentSession.persist(a)
hibernateDatastore.currentSession.flush()
then:
- A.count() == 1
-
+ Exception e = thrown()
+ e.getClass().simpleName in ['EntityActionVetoException',
'HibernateSystemException', 'IllegalStateException']
Review Comment:
This seems to be failing at the moment. A `ConstraintViolationException` is
thrown.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/txs/TransactionalWithinReadOnlySpec.groovy:
##########
@@ -33,9 +33,8 @@ import spock.lang.Specification
*/
class TransactionalWithinReadOnlySpec extends Specification {
- @Shared
- @AutoCleanup
- HibernateDatastore datastore = new HibernateDatastore(Product, Attribute)
+ @Shared @AutoCleanup HibernateDatastore datastore = new
HibernateDatastore(Product, Attribute)
Review Comment:
I think it's better to stack multiple annotations on top of each other
instead of on the same line.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/WhereQueryOldIssueVerificationSpec.groovy:
##########
@@ -360,7 +359,7 @@ class WqBiBook implements HibernateEntity<WqBiBook> {
String title
static hasMany = [authors: WqBiAuthor]
- static belongsTo = WqBiAuthor
+ static belongsTo = [WqBiAuthor]
Review Comment:
Why?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithManyToOneAndSequenceSpec.groovy:
##########
Review Comment:
This test seems to have changed substantially, but I find now description of
why?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/HibernateGormDatastoreSpec.groovy:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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 org.apache.grails.data.hibernate5.core.GrailsDataHibernate5TckManager
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.grails.datastore.mapping.model.PersistentEntity
+import org.grails.orm.hibernate.AbstractHibernateSession
+import org.grails.orm.hibernate.HibernateDatastore
+import org.grails.orm.hibernate.cfg.GrailsDomainBinder
+import org.grails.orm.hibernate.cfg.HibernateMappingContext
+import org.grails.orm.hibernate.cfg.HibernatePersistentEntity
+import org.grails.orm.hibernate.query.HibernateQuery
+
+import org.hibernate.boot.MetadataSources
+import org.hibernate.boot.internal.BootstrapContextImpl
+import org.hibernate.boot.internal.InFlightMetadataCollectorImpl
+import org.hibernate.boot.internal.MetadataBuilderImpl
+import org.hibernate.boot.registry.BootstrapServiceRegistry
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder
+import org.hibernate.boot.registry.classloading.spi.ClassLoaderService
+import org.hibernate.dialect.H2Dialect
+import org.hibernate.internal.SessionFactoryImpl
+import org.hibernate.service.spi.ServiceRegistryImplementor
+import org.hibernate.boot.spi.MetadataContributor
+
+/**
+ * The original GormDataStoreSpec destroyed the setup
+ * between tests instead of at the end of all tests
+ * It also was default configured for H2 which
+ * made it break with some Java types.
+ * Finally, it loaded all the test Entities,
+ * now it can be setup individually.
+ */
+class HibernateGormDatastoreSpec extends
GrailsDataTckSpec<GrailsDataHibernate5TckManager> {
Review Comment:
I think its better to describe this class instead of some obsolete class.
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/UniqueFalseConstraintSpec.groovy:
##########
@@ -31,7 +31,7 @@ class UniqueFalseConstraintSpec extends Specification {
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(User)
- @Issue('https://github.com/apache/grails-data-mapping/issues/1059')
+ @Issue('https://github.com/grails/grails-data-mapping/issues/1059')
Review Comment:
?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/CompositeIdWithJoinTableSpec.groovy:
##########
@@ -19,45 +19,40 @@
package grails.gorm.tests
-import static grails.gorm.hibernate.mapping.MappingBuilder.define
-
import grails.gorm.annotation.Entity
-import grails.gorm.transactions.Rollback
-import org.grails.orm.hibernate.HibernateDatastore
-import org.springframework.transaction.PlatformTransactionManager
-import spock.lang.AutoCleanup
-import spock.lang.Shared
-import spock.lang.Specification
+
+import static grails.gorm.hibernate.mapping.MappingBuilder.define
/**
* Created by graemerocher on 26/01/2017.
*/
-class CompositeIdWithJoinTableSpec extends Specification {
-
- @AutoCleanup @Shared HibernateDatastore datastore = new
HibernateDatastore(CompositeIdParent, CompositeIdChild)
- @Shared PlatformTransactionManager transactionManager =
datastore.transactionManager
+class CompositeIdWithJoinTableSpec extends HibernateGormDatastoreSpec {
+ def setupSpec() {
+ manager.registerDomainClasses(CompositeIdParent, CompositeIdChild)
+ }
- @Rollback
+ // @Rollback
Review Comment:
Why comment out `@Rollback`?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/IdentityEnumTypeSpec.groovy:
##########
@@ -83,18 +240,19 @@ class EnumEntityDomain {
class FooWithEnum {
long id
String name
+ @Enumerated(EnumType.STRING)
Review Comment:
Why add this?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/AddToManagedEntitySpec.groovy:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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 grails.gorm.hibernate.HibernateEntity
+import org.grails.datastore.gorm.GormEntity
+
+/**
+ * Regression tests for H7 "Found two representations of same collection"
error.
+ *
+ * H7 enforces strict collection identity — after an entity is persisted and
+ * managed by the session, calling addTo* and then save(flush:true) must not
+ * replace the Hibernate-tracked PersistentCollection with a plain collection.
+ */
+class AddToManagedEntitySpec extends HibernateGormDatastoreSpec {
+
+ void setupSpec() {
+ manager.registerDomainClasses(CascadeAuthor, CascadeBook)
+ }
+
+ void cleanup() {
+ CascadeBook.withNewTransaction {
+ CascadeBook.executeUpdate('delete from CascadeBook', [:])
+ CascadeAuthor.executeUpdate('delete from CascadeAuthor', [:])
Review Comment:
Do we still need the empty map?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/SaveWithInvalidEntitySpec.groovy:
##########
@@ -30,24 +31,30 @@ import spock.lang.Specification
/**
* Created by graemerocher on 03/05/2017.
*/
+//TODO Should this test be rewritten?
class SaveWithInvalidEntitySpec extends Specification {
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(A, B)
/**
- * This currently fails with a NPE. See explanation
https://github.com/apache/grails-core/issues/14616#issuecomment-298943022
+ * This currently fails with a NPE. See explanation
https://github.com/grails/grails-core/issues/10604#issuecomment-298943022
*/
@Rollback
- @Ignore
- @Issue('https://github.com/apache/grails-core/issues/10604')
+ @Issue('https://github.com/grails/grails-core/issues/10604')
Review Comment:
https://github.com/apache/grails-core/issues/14616?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/UniqueWithinGroupSpec.groovy:
##########
@@ -19,35 +19,28 @@
package grails.gorm.tests.validation
import grails.gorm.annotation.Entity
+import grails.gorm.tests.HibernateGormDatastoreSpec
import grails.gorm.transactions.Rollback
import groovy.transform.EqualsAndHashCode
-import org.grails.orm.hibernate.HibernateDatastore
-import org.hibernate.SessionFactory
import org.springframework.dao.DuplicateKeyException
-import spock.lang.AutoCleanup
import spock.lang.Issue
-import spock.lang.Shared
-import spock.lang.Specification
/**
* Created by graemerocher on 29/05/2017.
*/
-@Issue('https://github.com/grails/grails-data-hibernate5/issues/36')
-class UniqueWithinGroupSpec extends Specification {
+@Issue('https://github.com/grails/gorm-hibernate5/issues/36')
Review Comment:
https://github.com/grails/grails-data-hibernate5/issues/36?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/SubclassMultipleListCollectionSpec.groovy:
##########
@@ -41,9 +41,8 @@ class SubclassMultipleListCollectionSpec extends
Specification {
transactionManager = hibernateDatastore.getTransactionManager()
}
- @Ignore // not yet implemented
@Rollback
- @Issue('https://github.com/apache/grails-data-mapping/issues/882')
+ @Issue('https://github.com/grails/grails-data-mapping/issues/882')
Review Comment:
https://github.com/apache/grails-core/issues/14624
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/IdentityEnumTypeSpec.groovy:
##########
@@ -20,52 +20,209 @@ package grails.gorm.tests
import grails.gorm.annotation.Entity
import grails.gorm.transactions.Rollback
-import org.grails.orm.hibernate.HibernateDatastore
-import org.springframework.transaction.PlatformTransactionManager
-import spock.lang.AutoCleanup
-import spock.lang.Shared
-import spock.lang.Specification
+import jakarta.persistence.Enumerated
+import jakarta.persistence.EnumType
+import org.grails.orm.hibernate.cfg.IdentityEnumType
+import org.hibernate.HibernateException
+import org.hibernate.MappingException
+import org.hibernate.engine.spi.SharedSessionContractImplementor
import javax.sql.DataSource
import java.sql.ResultSet
/**
- * Created by graemerocher on 16/11/16.
+ * Tests for IdentityEnumType in Hibernate 5.
*/
-class IdentityEnumTypeSpec extends Specification {
+class IdentityEnumTypeSpec extends HibernateGormDatastoreSpec {
- @Shared @AutoCleanup HibernateDatastore hibernateDatastore = new
HibernateDatastore(EnumEntityDomain, FooWithEnum)
- @Shared PlatformTransactionManager transactionManager =
hibernateDatastore.getTransactionManager()
+ def setupSpec() {
+ manager.registerDomainClasses(EnumEntityDomain, FooWithEnum)
+ }
@Rollback
void "test identity enum type"() {
when:
- new EnumEntityDomain(status:
EnumEntityDomain.Status.FOO).save(flush:true)
- DataSource ds =
hibernateDatastore.connectionSources.defaultConnectionSource.dataSource
+ new EnumEntityDomain(status: EnumEntityDomain.Status.FOO).save(flush:
true)
+ DataSource ds =
manager.hibernateDatastore.connectionSources.defaultConnectionSource.dataSource
ResultSet resultSet = ds.getConnection().prepareStatement('select
status from enum_entity_domain').executeQuery()
then:
resultSet.next()
- resultSet.getString(1) == 'F'
+ resultSet.getString(1) == 'F' // FOO id is 'F'
EnumEntityDomain.first().status == EnumEntityDomain.Status.FOO
}
@Rollback
void "test identity enum type 2"() {
when:
- new FooWithEnum(name: "blah", mySuperValue:
XEnum.X__TWO).save(flush:true)
- DataSource ds =
hibernateDatastore.connectionSources.defaultConnectionSource.dataSource
+ new FooWithEnum(name: "blah", mySuperValue: XEnum.X__TWO).save(flush:
true)
+ DataSource ds =
manager.hibernateDatastore.connectionSources.defaultConnectionSource.dataSource
ResultSet resultSet = ds.getConnection().prepareStatement('select
my_super_value from foo_with_enum').executeQuery()
then:
resultSet.next()
- resultSet.getInt(1) == 100
+ resultSet.getInt(1) == 100 // X__TWO id is 100
FooWithEnum.first().mySuperValue == XEnum.X__TWO
}
+
+ def "setParameterValues initializes enumClass"() {
+ given:
+ def type = new IdentityEnumType()
+ def props = new Properties()
+ props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS,
IdentityStatusEnum.name)
+
+ when:
+ type.setParameterValues(props)
+
+ then:
+ type.returnedClass() == IdentityStatusEnum
+ type.sqlTypes()[0] != 0
+ }
+
+ def "setParameterValues throws MappingException for enum without getId
method"() {
+ given:
+ def type = new IdentityEnumType()
+ def props = new Properties()
+ props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, PlainEnum.name)
+
+ when:
+ type.setParameterValues(props)
+
+ then:
+ thrown(HibernateException) // Throw by BidiEnumMap constructor
+ }
+
+ def "equals uses identity comparison"() {
+ given:
+ def type = new IdentityEnumType()
+
+ expect:
+ type.equals(IdentityStatusEnum.ACTIVE, IdentityStatusEnum.ACTIVE)
+ !type.equals(IdentityStatusEnum.ACTIVE, IdentityStatusEnum.INACTIVE)
+ !type.equals(null, IdentityStatusEnum.ACTIVE)
+ }
+
+ def "hashCode delegates to the object"() {
+ given:
+ def type = new IdentityEnumType()
+ def val = IdentityStatusEnum.ACTIVE
+
+ expect:
+ type.hashCode(val) == val.hashCode()
+ }
+
+ def "deepCopy returns the same object reference"() {
+ given:
+ def type = new IdentityEnumType()
+ def val = IdentityStatusEnum.ACTIVE
+
+ expect:
+ type.deepCopy(val).is(val)
+ }
+
+ def "isMutable returns false"() {
+ expect:
+ !new IdentityEnumType().isMutable()
+ }
+
+ def "disassemble returns the value as Serializable"() {
+ given:
+ def type = new IdentityEnumType()
+ def val = IdentityStatusEnum.ACTIVE
+
+ expect:
+ type.disassemble(val).is(val)
+ }
+
+ def "assemble returns the cached value unchanged"() {
+ given:
+ def type = new IdentityEnumType()
+ def val = IdentityStatusEnum.ACTIVE
+
+ expect:
+ type.assemble(val, null).is(val)
+ }
+
+ def "replace returns the original value"() {
+ given:
+ def type = new IdentityEnumType()
+
+ expect:
+ type.replace(IdentityStatusEnum.ACTIVE, IdentityStatusEnum.INACTIVE,
null).is(IdentityStatusEnum.ACTIVE)
+ }
+
+ def "nullSafeGet returns null for null value"() {
+ given:
+ def type = new IdentityEnumType()
+ def props = new Properties()
+ props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS,
IdentityStatusEnum.name)
+ type.setParameterValues(props)
+ def rs = Mock(java.sql.ResultSet)
+ def session = manager.sessionFactory.currentSession as
SharedSessionContractImplementor
+
+ when:
+ def res = type.nullSafeGet(rs, ['status'] as String[], session, null)
+
+ then:
+ 1 * rs.getString('status') >> null
+ 1 * rs.wasNull() >> true
+ res == null
+ }
+
+ def "nullSafeGet converts id to enum"() {
+ given:
+ def type = new IdentityEnumType()
+ def props = new Properties()
+ props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS,
IdentityStatusEnum.name)
+ type.setParameterValues(props)
+ def rs = Mock(java.sql.ResultSet)
+ def session = manager.sessionFactory.currentSession as
SharedSessionContractImplementor
+
+ when:
+ def res = type.nullSafeGet(rs, ['status'] as String[], session, null)
+
+ then:
+ 1 * rs.getString('status') >> "A"
+ 2 * rs.wasNull() >> false
+ res == IdentityStatusEnum.ACTIVE
+ }
+
+ def "nullSafeSet handles null value"() {
+ given:
+ def type = new IdentityEnumType()
+ def props = new Properties()
+ props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS,
IdentityStatusEnum.name)
+ type.setParameterValues(props)
+ def st = Mock(java.sql.PreparedStatement)
+ def session = manager.sessionFactory.currentSession as
SharedSessionContractImplementor
+
+ when:
+ type.nullSafeSet(st, null, 1, session)
+
+ then:
+ 1 * st.setNull(1, _)
+ }
+
+ def "nullSafeSet converts enum to id"() {
+ given:
+ def type = new IdentityEnumType()
+ def props = new Properties()
+ props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS,
IdentityStatusEnum.name)
+ type.setParameterValues(props)
+ def st = Mock(java.sql.PreparedStatement)
+ def session = manager.sessionFactory.currentSession as
SharedSessionContractImplementor
+
+ when:
+ type.nullSafeSet(st, IdentityStatusEnum.INACTIVE, 1, session)
+
+ then:
+ 1 * st.setString(1, "I")
+ }
}
@Entity
class EnumEntityDomain {
+ @Enumerated(EnumType.STRING)
Review Comment:
Why add this?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/validation/UniqueWithinGroupSpec.groovy:
##########
@@ -19,35 +19,28 @@
package grails.gorm.tests.validation
import grails.gorm.annotation.Entity
+import grails.gorm.tests.HibernateGormDatastoreSpec
import grails.gorm.transactions.Rollback
import groovy.transform.EqualsAndHashCode
-import org.grails.orm.hibernate.HibernateDatastore
-import org.hibernate.SessionFactory
import org.springframework.dao.DuplicateKeyException
-import spock.lang.AutoCleanup
import spock.lang.Issue
-import spock.lang.Shared
-import spock.lang.Specification
/**
* Created by graemerocher on 29/05/2017.
*/
-@Issue('https://github.com/grails/grails-data-hibernate5/issues/36')
-class UniqueWithinGroupSpec extends Specification {
+@Issue('https://github.com/grails/gorm-hibernate5/issues/36')
+class UniqueWithinGroupSpec extends HibernateGormDatastoreSpec {
- @AutoCleanup
- @Shared
- HibernateDatastore hibernateDatastore = new
HibernateDatastore(getClass().getPackage())
-
- @Shared
- SessionFactory sessionFactory = hibernateDatastore.sessionFactory
+ def setupSpec() {
+ manager.registerDomainClasses(Thing)
+ }
@Rollback
void "test insert"() {
when:
Thing thing1 = new Thing(hello: 1, world: 2)
thing1.insert(flush: true)
- sessionFactory.currentSession.flush()
+
Review Comment:
Why was the flush removed here but not in the other specs?
##########
grails-data-hibernate5/core/src/test/groovy/org/apache/grails/data/hibernate5/core/GrailsDataHibernate5TckManager.groovy:
##########
@@ -62,17 +63,20 @@ class GrailsDataHibernate5TckManager extends
GrailsDataTckManager {
@Override
Session createSession() {
- ConfigObject grailsConfig = new ConfigObject()
+ ConfigObject config = new ConfigObject()
+ if (grailsConfig) {
+ config.putAll(grailsConfig)
+ }
+ if (!config.containsKey('dataSource.dbCreate') &&
!config.dataSource.containsKey('dbCreate')) {
+ config.dataSource.dbCreate = "create-drop"
+ }
boolean isTransactional = true
System.setProperty('hibernate5.gorm.suite', "true")
- grailsApplication = new DefaultGrailsApplication(domainClasses, new
GroovyClassLoader(GrailsDataHibernate5TckManager.getClassLoader()))
- if (grailsConfig) {
- grailsApplication.config.putAll(grailsConfig)
- }
+ grailsApplication = new DefaultGrailsApplication(domainClasses as
Class[], new GroovyClassLoader(GrailsDataHibernate5TckManager.getClassLoader()))
Review Comment:
No need to cast `domainClasses`?
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/query/AbstractHibernateCriterionAdapter.java:
##########
@@ -385,6 +385,16 @@ public Criterion
toHibernateCriterion(AbstractHibernateQuery hibernateQuery, Que
}
});
+ criterionAdaptors.put(Query.SizeNotEquals.class, new
CriterionAdaptor<Query.SizeNotEquals>() {
Review Comment:
Ok, we can backport it.
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/HibernateDatastoreMultiTenancySpec.groovy:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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
+
+import grails.gorm.MultiTenant
+import grails.gorm.annotation.Entity
+import grails.gorm.multitenancy.Tenants
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.mapping.core.connections.ConnectionSource
+import org.grails.datastore.mapping.multitenancy.MultiTenancySettings
+import
org.grails.datastore.mapping.multitenancy.resolvers.SystemPropertyTenantResolver
+import org.grails.orm.hibernate.cfg.Settings
+import org.hibernate.FlushMode
+import spock.lang.Issue
+
+import javax.sql.DataSource
+
Review Comment:
Many unsed imports?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/HibernateGormEnhancerSpec.groovy:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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
+
+import grails.gorm.annotation.Entity
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.gorm.GormEnhancer
Review Comment:
Unused import?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/datastore/mapping/model/PersistentPropertySpec.groovy:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.datastore.mapping.model
+
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import grails.persistence.Entity
+import spock.lang.Issue
+
+@Issue('https://github.com/grails/grails-data-mapping/issues/1299')
Review Comment:
Is this correct?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/support/SoftKeySpec.groovy:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.support
+
+import spock.lang.Specification
+
+class SoftKeySpec extends Specification {
+
+ static class TestSoftKey<T> extends SoftKey<T> {
+ boolean forceNull = false
+ TestSoftKey(T referent) {
+ super(referent)
+ }
+ @Override
+ T get() {
+ return forceNull ? null : super.get()
+ }
+ }
+
+ def "constructor stores referent and computes hashCode from it"() {
+ given:
+ def key = "hello"
+
+ when:
+ def sk = new SoftKey<>(key)
+
+ then:
+ sk.get() == key
+ sk.hashCode() == key.hashCode()
+ }
+
+ def "hashCode is stable even after gc (uses stored hash)"() {
+ given:
+ def sk = new SoftKey<>("world")
+
+ expect:
+ sk.hashCode() == "world".hashCode()
+ }
+
+ def "equals returns true for same instance"() {
+ given:
+ def sk = new SoftKey<>("a")
+
+ expect:
+ sk.equals(sk)
+ }
+
+ def "equals returns false for null"() {
+ given:
+ def sk = new SoftKey<>("a")
+
+ expect:
+ !sk.equals(null)
+ }
+
+ def "equals returns false for different class"() {
+ given:
+ def sk = new SoftKey<>("a")
+
+ expect:
+ !sk.equals("a")
+ }
+
+ def "two SoftKeys with equal referents are equal"() {
+ given:
+ def sk1 = new SoftKey<>("same")
+ def sk2 = new SoftKey<>("same")
+
+ expect:
+ sk1 == sk2
+ sk1.hashCode() == sk2.hashCode()
+ }
+
+ def "two SoftKeys with different referents are not equal"() {
+ given:
+ def sk1 = new SoftKey<>("foo")
+ def sk2 = new SoftKey<>("bar")
+
+ expect:
+ sk1 != sk2
+ }
+
+ def "two SoftKeys with different hashes are not equal"() {
+ given:
+ // ensure different hash codes (different objects)
+ def sk1 = new SoftKey<>(new Integer(1))
+ def sk2 = new SoftKey<>(new Integer(99999))
Review Comment:
Uses deprecated API?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/support/HibernateDatastoreConnectionSourcesRegistrarSpec.groovy:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.support
+
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.gorm.bootstrap.support.InstanceFactoryBean
+import org.grails.datastore.mapping.config.Settings
+import org.grails.datastore.mapping.core.connections.ConnectionSource
Review Comment:
Unused?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/proxy/HibernateProxyHandler5Spec.groovy:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.proxy
+
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.apache.grails.data.hibernate5.core.GrailsDataHibernate5TckManager
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.apache.grails.data.testing.tck.domains.Location
+import org.apache.grails.data.testing.tck.domains.Person
+import org.apache.grails.data.testing.tck.domains.Pet
+import org.hibernate.Hibernate
+import spock.lang.Shared
+import org.grails.datastore.gorm.proxy.GroovyProxyFactory
+
+class HibernateProxyHandler5Spec extends
GrailsDataTckSpec<GrailsDataHibernate5TckManager> {
Review Comment:
Should this class extend HibernateGormDatastoreSpec?
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/HibernateDatastoreMultiTenancySpec.groovy:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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
+
+import grails.gorm.MultiTenant
+import grails.gorm.annotation.Entity
+import grails.gorm.multitenancy.Tenants
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.mapping.core.connections.ConnectionSource
+import org.grails.datastore.mapping.multitenancy.MultiTenancySettings
+import
org.grails.datastore.mapping.multitenancy.resolvers.SystemPropertyTenantResolver
+import org.grails.orm.hibernate.cfg.Settings
+import org.hibernate.FlushMode
+import spock.lang.Issue
+
+import javax.sql.DataSource
+
+class HibernateDatastoreMultiTenancySpec extends HibernateGormDatastoreSpec {
+
+ def setupSpec() {
+ manager.grailsConfig = [
+ 'dataSource.url' :
"jdbc:h2:mem:grailsDB-multi;LOCK_TIMEOUT=10000",
+ 'dataSource.dbCreate' : 'create-drop',
+ 'hibernate.flush.mode' : 'COMMIT',
+ 'grails.gorm.multiTenancy.mode':
MultiTenancySettings.MultiTenancyMode.DISCRIMINATOR,
+ 'grails.gorm.multiTenancy.tenantResolver': new
SystemPropertyTenantResolver()
+ ]
+ manager.registerDomainClasses(MultiTenantBook)
+ }
+
+ void "test discriminator multi-tenancy filter"() {
+ given:
+ System.setProperty(SystemPropertyTenantResolver.PROPERTY_NAME,
"tenant1")
+
+ when:
+ def result = datastore.withSession {
+ new MultiTenantBook(title: "Book 1").save()
+ MultiTenantBook.list()
+ }
+
+ then:
+ result.size() == 1
+ result[0].tenantId == "tenant1"
+
+ when:
+ System.setProperty(SystemPropertyTenantResolver.PROPERTY_NAME,
"tenant2")
+ result = datastore.withSession {
+ new MultiTenantBook(title: "Book 2").save()
+ MultiTenantBook.list()
+ }
+
+ then:
+ result.size() == 1
+ result[0].tenantId == "tenant2"
+
+ cleanup:
+ System.clearProperty(SystemPropertyTenantResolver.PROPERTY_NAME)
Review Comment:
Use `@RestoreSystemProperties`?
--
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]
