You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "borinquenkid (via GitHub)" <gi...@apache.org> on 2026/10/10 16:10:32 UTC
[PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
borinquenkid opened a new pull request, #16572:
URL: https://github.com/apache/grails-core/pull/16572
## What
First per-project pull request of the Java-to-Groovy migration tracked in #16351. It transplants the end state of the `9.0.x-java-to-groovy` branch for the Gradle projects under `grails-data-hibernate7/` onto `9.0.x`, unchanged, as four commits in dependency order (one per Gradle project, each compiling on its own):
| Commit | Gradle project | Java sources converted | Files changed | Lines |
|---|---|---|---|---|
| `b9c4e765ac` | `:grails-data-hibernate7-spring-orm` | 24 main | 26 | +1,934 / -2,105 |
| `3f3e1c50b4` | `:grails-data-hibernate7-core` | 207 main (+2 new Spock specs) | 265 | +16,534 / -15,717 |
| `33c751d633` | `:grails-data-hibernate7-dbmigration-core` (+ RAT config) | 35 main, 19 test, 29 testFixtures | 157 | +6,537 / -6,134 |
| `fcf18b26ab` | `:grails-data-hibernate7` (grails-plugin) | 5 main | 6 | +425 / -407 |
Total: 454 files, +25,430 / -24,363; 319 `.java` files removed, none left under `grails-data-hibernate7/`. The `dbmigration`, `boot-plugin` and `docs` subprojects were already pure Groovy and are byte-identical to `9.0.x`.
Every converted class is statically compiled (`@CompileStatic`; the liquibase-hibernate `HibernateDatabase` family also carries `@POJO` so its public `getProperty(String)` does not become the GroovyObject property hook). `spring-orm` gains the Groovy and code-style plugins so its new sources are compiled and Checkstyle/CodeNarc-checked; `dbmigration-core` drops Lombok (no longer used by the fixtures) and adds `jboss-logging` as `compileOnly` for Groovydoc.
## Why
Maintainers asked in #16351 for the migration to land one Gradle project at a time, stacked, instead of as one branch. The Hibernate 7 modules go first because the native domain binding work (#16568) will be rebuilt on top of Groovy sources. The original conversion commits on the umbrella branch are `2416bddc86..0c2e2c2f33` (spring-orm), `98c59ede75..25dde13cd9` (core), `44f4279fa0..e2ba1e6ffe` (dbmigration-core) and `ee295eaabb..06fb6a1c4b` (grails-plugin); their review notes (reference identity in `equals()`, `Optional.map` null-collapse, closure spread, `isX`/`getX` accessor ambiguity, `@POJO` on the liquibase database classes) are summarized in the commit bodies.
## What changed outside the modules
Only `gradle/rat-root-config.gradle`: the 47 RAT exclusions that name the liquibase-hibernate fork files (main sources, tests and test fixtures) are repointed from the old `.java` paths to the `.groovy` paths, so the forked third-party files stay excluded and `rat` stays green. No other build logic, settings, CI, docs or skills change. The modules build against the `9.0.x` Java versions of every other module; no class of another module had to come along.
## Verification
Local, on this branch (`origin/9.0.x` = `8ef440820e` plus the four commits), one Gradle process at a time, `--no-parallel`:
| Run | Result |
|---|---|
| Per-commit `:<project>:compileGroovy :<project>:compileTestGroovy` (spring-orm, core, dbmigration-core incl. `compileTestFixturesGroovy`, grails-plugin) | all green; the core test compile executed (not from cache) with the default 2 GB forked compiler heap, so no heap override was needed |
| Whole-repo `build -PskipTests --continue` | BUILD SUCCESSFUL in 34m 56s, 3446 actionable tasks (3084 executed, 185 from cache, 177 up-to-date), 0 failed tasks; every `grails-data-hibernate7*` compile, `checkstyle*` and `codenarc*` task green |
| `codeStyle` | BUILD SUCCESSFUL (1051 tasks, all up-to-date after the build above) |
| `rat` | 0 tracked unapproved files; the 2 reported ones are untracked local build artifacts under `grails-data-hibernate7`'s neighbour `grails-data-neo4j/grails-plugin/data/` (`neo4j.key`, `store_lock`), absent on `9.0.x` |
Test suites were not run locally for this pull request: the sources are byte-identical to the umbrella branch, whose suites were run on 2026-10-09 (`grails-data-hibernate7-core` 3657 tests / 21 skipped / 0 failures, `dbmigration-core` 40 / 0, `dbmigration` 193 / 0, plugin 41 / 0, whole-repo `build -PskipTests` green). CI runs them here.
A javap pass over the 271 converted main classes (reconstructed audit: `ScriptBytecodeAdapter`, `InvokerHelper`, call-site and non-cast `invokedynamic` sites, with the migration's documented exemptions) found no new dynamic dispatch beyond what the umbrella branch already carries: record `equals/hashCode/toString` bootstraps, `as HibernateCallback` coercions (`HibernateTemplate`, `InstanceApiHelper`, `ProjectionPredicate`), lambda bootstraps, the static initializers of the deprecated `HibernateQueryConstants` interface, and five slow-path property writes (`ChildHibernateDatastore.destroy`, `HibernateQuery`'s constructor, `maxResults` and `clone` closure, one `HibernateSession.updateAll` closure, one `HibernateDatastore` multi-tenant closure). They are functionally correct and are left as on the umbrella branch so the later stacked slices apply cleanly.
## Notes
- CodeNarc logs an internal `ArrayIndexOutOfBoundsException` from `SpaceAfterOpeningBraceRule` while visiting `core/.../query/SqlRestriction.groovy`; it is a CodeNarc rule bug on a closure expression, not a violation, the task succeeds, and the same message appears when building that file on the `9.0.x-h7-improvement-phase3` worktree.
- The `MetaClassRegistryCleaner`-style gotchas (`this == o` in `equals()`, verifier-time class loads from flow typing) that surfaced on the umbrella branch after the merge with `9.0.x` were in `grails-gsp`/`grails-web-url-mappings`, which stay Java here; the Hibernate 7 modules' own equivalent (`SoftKey.equals()` reference identity) was fixed in the original conversion and is included.
- After this merges, the next slice should be cut from this branch rather than from `9.0.x` so the stack stays linear.
๐ค Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4238654727
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateAlias.groovy:
##########
@@ -16,21 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.orm.hibernate.query;
+package org.grails.orm.hibernate.query
-import jakarta.persistence.criteria.JoinType;
+import groovy.transform.CompileStatic
+import jakarta.persistence.criteria.JoinType
-import org.grails.datastore.mapping.query.Query;
+import org.grails.datastore.mapping.query.Query
/**
* A internal criterion used to represent an alias for a basic collection join.
*
* @author walterduquedeestrada
*/
-public record HibernateAlias(String path, String alias, JoinType joinType)
+@CompileStatic
+@SuppressWarnings(['ClassStartsWithBlankLine', 'Indentation'])
+record HibernateAlias(String path, String alias, JoinType joinType)
implements Query.Criterion, Query.QueryElement {
- public HibernateAlias(String path, String alias) {
- this(path, alias, JoinType.INNER);
- }
}
Review Comment:
Fixed in ee3d2b0b58: the `HibernateAlias(String, String)` constructor is back and defaults to `JoinType.INNER` (the one Groovy generated for the record left `joinType` null).
##########
grails-data-hibernate7/core/src/main/groovy/grails/orm/HibernateCriteriaBuilder.groovy:
##########
@@ -125,133 +118,136 @@
* @see grails.gorm.DetachedCriteria
*/
@Slf4j
-@SuppressWarnings("PMD.AvoidDuplicateLiterals")
-public class HibernateCriteriaBuilder extends GroovyObjectSupport implements BuildableCriteria, ProjectionList {
- private final SessionFactory sessionFactory;
- private final boolean participate;
- private final org.hibernate.query.criteria.HibernateCriteriaBuilder cb;
- private final HibernateQuery hibernateQuery;
- private Class<?> targetClass;
- private CriteriaQuery<?> criteriaQuery;
- private boolean uniqueResult = false;
-
- @SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName")
- private boolean scroll;
-
- @SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName")
- private boolean count;
-
- private boolean paginationEnabledList = false;
- private int defaultFlushMode;
-
- @SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName")
- private boolean distinct = false;
- private CriteriaMethodInvoker criteriaMethodInvoker;
-
- @SuppressWarnings({"rawtypes", "PMD.CloseResource"})
- public HibernateCriteriaBuilder(Class targetClass, SessionFactory sessionFactory, HibernateDatastore datastore) {
- this.targetClass = targetClass;
- setDatastore(datastore);
- this.sessionFactory = sessionFactory;
- this.cb = sessionFactory.getCriteriaBuilder();
+@SuppressWarnings('PMD.AvoidDuplicateLiterals')
+@CompileStatic
+class HibernateCriteriaBuilder extends GroovyObjectSupport implements BuildableCriteria, ProjectionList {
+
+ private final SessionFactory sessionFactory
+ private final boolean participate
+ private final org.hibernate.query.criteria.HibernateCriteriaBuilder cb
+ private final HibernateQuery hibernateQuery
+ private Class<?> targetClass
+ private CriteriaQuery<?> criteriaQuery
+ private boolean uniqueResult = false
+
+ @SuppressWarnings('PMD.AvoidFieldNameMatchingMethodName')
+ private boolean scroll
+
+ @SuppressWarnings('PMD.AvoidFieldNameMatchingMethodName')
+ private boolean count
+
+ private boolean paginationEnabledList = false
+ private int defaultFlushMode
+
+ @SuppressWarnings('PMD.AvoidFieldNameMatchingMethodName')
+ private boolean distinct = false
+ private CriteriaMethodInvoker criteriaMethodInvoker
+
+ @SuppressWarnings(['rawtypes', 'PMD.CloseResource'])
+ HibernateCriteriaBuilder(Class targetClass, SessionFactory sessionFactory, HibernateDatastore datastore) {
+ this.targetClass = targetClass
+ setDatastore(datastore)
+ this.sessionFactory = sessionFactory
+ this.cb = sessionFactory.getCriteriaBuilder()
if (TransactionSynchronizationManager.hasResource(sessionFactory)) {
- this.participate = true;
- } else {
- this.participate = false;
- org.hibernate.Session session = sessionFactory.openSession();
- TransactionSynchronizationManager.bindResource(sessionFactory, new SessionHolder(session));
+ this.participate = true
}
- HibernateSession session = (HibernateSession) datastore.connect();
+ else {
+ this.participate = false
+ org.hibernate.Session session = sessionFactory.openSession()
+ TransactionSynchronizationManager.bindResource(sessionFactory, new SessionHolder(session))
+ }
+ HibernateSession session = (HibernateSession) datastore.connect()
hibernateQuery = new HibernateQuery(
- session, (GrailsHibernatePersistentEntity) datastore.getMappingContext().getPersistentEntity(targetClass.getName()));
- setDefaultFlushMode(GrailsHibernateTemplate.FLUSH_AUTO);
- criteriaMethodInvoker = new CriteriaMethodInvoker(this);
+ session, (GrailsHibernatePersistentEntity) datastore.getMappingContext().getPersistentEntity(targetClass.getName()))
+ setDefaultFlushMode(GrailsHibernateTemplate.FLUSH_AUTO)
+ criteriaMethodInvoker = new CriteriaMethodInvoker(this)
}
- public static final String ALIAS_SEPARATOR = ":";
+ public static final String ALIAS_SEPARATOR = ':'
// The types a sqlProjection or sqlGroupProjection takes in the criteria DSL, as on Hibernate 5
- public static final BasicTypeReference<?> BOOLEAN = StandardBasicTypes.BOOLEAN;
- public static final BasicTypeReference<?> YES_NO = StandardBasicTypes.YES_NO;
- public static final BasicTypeReference<?> BYTE = StandardBasicTypes.BYTE;
- public static final BasicTypeReference<?> CHARACTER = StandardBasicTypes.CHARACTER;
- public static final BasicTypeReference<?> SHORT = StandardBasicTypes.SHORT;
- public static final BasicTypeReference<?> INTEGER = StandardBasicTypes.INTEGER;
- public static final BasicTypeReference<?> LONG = StandardBasicTypes.LONG;
- public static final BasicTypeReference<?> FLOAT = StandardBasicTypes.FLOAT;
- public static final BasicTypeReference<?> DOUBLE = StandardBasicTypes.DOUBLE;
- public static final BasicTypeReference<?> BIG_DECIMAL = StandardBasicTypes.BIG_DECIMAL;
- public static final BasicTypeReference<?> BIG_INTEGER = StandardBasicTypes.BIG_INTEGER;
- public static final BasicTypeReference<?> STRING = StandardBasicTypes.STRING;
- public static final BasicTypeReference<?> NUMERIC_BOOLEAN = StandardBasicTypes.NUMERIC_BOOLEAN;
- public static final BasicTypeReference<?> TRUE_FALSE = StandardBasicTypes.TRUE_FALSE;
- public static final BasicTypeReference<?> URL = StandardBasicTypes.URL;
- public static final BasicTypeReference<?> TIME = StandardBasicTypes.TIME;
- public static final BasicTypeReference<?> DATE = StandardBasicTypes.DATE;
- public static final BasicTypeReference<?> TIMESTAMP = StandardBasicTypes.TIMESTAMP;
- public static final BasicTypeReference<?> CALENDAR = StandardBasicTypes.CALENDAR;
- public static final BasicTypeReference<?> CALENDAR_DATE = StandardBasicTypes.CALENDAR_DATE;
- public static final BasicTypeReference<?> CLASS = StandardBasicTypes.CLASS;
- public static final BasicTypeReference<?> LOCALE = StandardBasicTypes.LOCALE;
- public static final BasicTypeReference<?> CURRENCY = StandardBasicTypes.CURRENCY;
- public static final BasicTypeReference<?> TIMEZONE = StandardBasicTypes.TIMEZONE;
- public static final BasicTypeReference<?> UUID_BINARY = StandardBasicTypes.UUID_BINARY;
- public static final BasicTypeReference<?> UUID_CHAR = StandardBasicTypes.UUID_CHAR;
- public static final BasicTypeReference<?> BINARY = StandardBasicTypes.BINARY;
- public static final BasicTypeReference<?> WRAPPER_BINARY = StandardBasicTypes.BINARY_WRAPPER;
- public static final BasicTypeReference<?> IMAGE = StandardBasicTypes.IMAGE;
- public static final BasicTypeReference<?> BLOB = StandardBasicTypes.BLOB;
- public static final BasicTypeReference<?> MATERIALIZED_BLOB = StandardBasicTypes.MATERIALIZED_BLOB;
- public static final BasicTypeReference<?> CHAR_ARRAY = StandardBasicTypes.CHAR_ARRAY;
- public static final BasicTypeReference<?> CHARACTER_ARRAY = StandardBasicTypes.CHARACTER_ARRAY;
- public static final BasicTypeReference<?> TEXT = StandardBasicTypes.TEXT;
- public static final BasicTypeReference<?> CLOB = StandardBasicTypes.CLOB;
- public static final BasicTypeReference<?> MATERIALIZED_CLOB = StandardBasicTypes.MATERIALIZED_CLOB;
- public static final BasicTypeReference<?> SERIALIZABLE = StandardBasicTypes.SERIALIZABLE;
+ static final BasicTypeReference<?> BOOLEAN = StandardBasicTypes.BOOLEAN
+ static final BasicTypeReference<?> YES_NO = StandardBasicTypes.YES_NO
+ static final BasicTypeReference<?> BYTE = StandardBasicTypes.BYTE
+ static final BasicTypeReference<?> CHARACTER = StandardBasicTypes.CHARACTER
+ static final BasicTypeReference<?> SHORT = StandardBasicTypes.SHORT
Review Comment:
Fixed in ee3d2b0b58: the 37 SQL projection type constants are `public static final` fields again.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsHibernateUtil.groovy:
##########
@@ -0,0 +1,316 @@
+/*
+ * 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
+
+import groovy.transform.CompileStatic
+import org.hibernate.FlushMode
+import org.hibernate.Hibernate
+import org.hibernate.Session
+import org.hibernate.SessionFactory
+import org.hibernate.engine.spi.EntityEntry
+import org.hibernate.engine.spi.SessionImplementor
+import org.hibernate.engine.spi.Status
+import org.hibernate.internal.util.StringHelper
+import org.hibernate.proxy.HibernateProxy
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import org.springframework.transaction.support.TransactionSynchronizationManager
+
+import grails.gorm.annotation.Entity
+import org.grails.datastore.gorm.GormEntity
+import org.grails.datastore.mapping.model.PersistentEntity
+import org.grails.datastore.mapping.model.config.GormProperties
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity
+import org.grails.orm.hibernate.proxy.HibernateProxyHandler
+import org.grails.orm.hibernate.query.HibernateQueryArgument
+import org.grails.orm.hibernate.support.HibernateRuntimeUtils
+
+import java.lang.annotation.Annotation
+
+/**
+ * Utility methods for configuring Hibernate inside Grails.
+ *
+ * @author Graeme Rocher
+ * @since 0.4
+ */
+@CompileStatic
+class GrailsHibernateUtil extends HibernateRuntimeUtils {
+
+ /** @deprecated Use {@link org.grails.orm.hibernate.query.HibernateQueryArgument#FETCH_SIZE} */
+ @Deprecated(since = '8.0', forRemoval = true)
+ static final String ARGUMENT_FETCH_SIZE = HibernateQueryArgument.FETCH_SIZE.value()
Review Comment:
Fixed in ee3d2b0b58: the 14 deprecated constants are `public static final` fields again.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/IdentityEnumType.groovy:
##########
@@ -0,0 +1,240 @@
+/*
+ * 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
+
+import groovy.transform.CompileStatic
+import org.hibernate.HibernateException
+import org.hibernate.MappingException
+import org.hibernate.engine.spi.SharedSessionContractImplementor
+import org.hibernate.type.descriptor.WrapperOptions
+import org.hibernate.type.descriptor.java.JavaType
+import org.hibernate.type.descriptor.jdbc.BigIntJdbcType
+import org.hibernate.type.descriptor.jdbc.IntegerJdbcType
+import org.hibernate.type.descriptor.jdbc.JdbcType
+import org.hibernate.type.descriptor.jdbc.VarcharJdbcType
+import org.hibernate.type.spi.TypeConfiguration
+import org.hibernate.usertype.ParameterizedType
+import org.hibernate.usertype.UserType
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import java.lang.reflect.InvocationTargetException
+import java.lang.reflect.Method
+import java.sql.PreparedStatement
+import java.sql.ResultSet
+import java.sql.SQLException
+import java.sql.Types
+
+/**
+ * Hibernate 7 UserType to map Enums to their "id" value.
+ */
+@CompileStatic
+class IdentityEnumType implements UserType<Object>, ParameterizedType, Serializable {
+
+ private static final Logger LOG = LoggerFactory.getLogger(IdentityEnumType)
+ private static final TypeConfiguration typeConfiguration = new TypeConfiguration()
+
+ static final String ENUM_ID_ACCESSOR = 'getId'
+ static final String PARAM_ENUM_CLASS = 'enumClass'
Review Comment:
Fixed in ee3d2b0b58: `ENUM_ID_ACCESSOR` and `PARAM_ENUM_CLASS` are `public static final` fields again.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BackticksRemover.groovy:
##########
@@ -28,16 +29,21 @@
*
* @since 8.0
*/
-public class BackticksRemover implements Function<String, String> {
+@CompileStatic
+class BackticksRemover implements Function<String, String> {
- public static final String BACKTICK = "`";
+ static final String BACKTICK = '`'
Review Comment:
Fixed in ee3d2b0b58: `BACKTICK` is a `public static final` field again.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/ClosureEventTriggeringInterceptor.groovy:
##########
@@ -103,97 +99,96 @@ public class ClosureEventTriggeringInterceptor
* @deprecated Use {@link AbstractPersistenceEvent#ONLOAD_EVENT} instead
*/
@Deprecated
- public static final String ONLOAD_EVENT = AbstractPersistenceEvent.ONLOAD_EVENT;
+ static final String ONLOAD_EVENT = AbstractPersistenceEvent.ONLOAD_EVENT
Review Comment:
Fixed in ee3d2b0b58: the 10 deprecated event constants are `public static final` fields again.
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/HibernateCallback.groovy:
##########
@@ -48,7 +46,6 @@ public interface HibernateCallback<T> {
* @throws HibernateException if thrown by the Hibernate API
* @see HibernateTemplate#execute
*/
- @Nullable
- T doInHibernate(Session session) throws HibernateException;
+ T doInHibernate(Session session) throws HibernateException
Review Comment:
Fixed in ee3d2b0b58: the jspecify `@Nullable` on `doInHibernate` is back, and `JavaFacingApiSpec` (4b481384f6) pins it.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6308557040
## โ
All tests passed โ
๐ท๏ธ Commit: 4e0951067a09c2e94c5b4d6a957323ff36c94358
โถ๏ธ Tests: 95090 executed
โช๏ธ Checks: 90/90 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4363532054
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.groovy:
##########
@@ -134,111 +127,113 @@ public class GrailsHibernateTemplate implements IHibernateTemplate {
*
* @see #setFlushMode
*/
- public static final int FLUSH_ALWAYS = 4;
-
- private static final Logger LOG = LoggerFactory.getLogger(GrailsHibernateTemplate.class);
- protected boolean exposeNativeSession = true;
- protected boolean cacheQueries = false;
- protected SessionFactory sessionFactory;
- protected DataSource dataSource = null;
- protected SQLExceptionTranslator jdbcExceptionTranslator;
- protected int flushMode = FLUSH_AUTO;
- private boolean osivReadOnly;
- private boolean passReadOnlyToHibernate = false;
- private boolean applyFlushModeOnlyToNonExistingTransactions = false;
- protected TransactionResources txResources = new DefaultTransactionResources();
+ public static final int FLUSH_ALWAYS = 4
+
+ private static final Logger LOG = LoggerFactory.getLogger(GrailsHibernateTemplate)
+ protected boolean exposeNativeSession = true
+ protected boolean cacheQueries = false
+ protected SessionFactory sessionFactory
+ protected DataSource dataSource = null
+ protected SQLExceptionTranslator jdbcExceptionTranslator
+ protected int flushMode = FLUSH_AUTO
+ private boolean osivReadOnly
+ private boolean passReadOnlyToHibernate = false
+ private boolean applyFlushModeOnlyToNonExistingTransactions = false
+ protected TransactionResources txResources = new DefaultTransactionResources()
protected GrailsHibernateTemplate() {
// for testing
}
- public GrailsHibernateTemplate(SessionFactory sessionFactory) {
- Assert.notNull(sessionFactory, "Property 'sessionFactory' is required");
- this.sessionFactory = sessionFactory;
+ GrailsHibernateTemplate(SessionFactory sessionFactory) {
+ Assert.notNull(sessionFactory, "Property 'sessionFactory' is required")
+ this.sessionFactory = sessionFactory
ConnectionProvider connectionProvider = ((SessionFactoryImplementor) sessionFactory)
- .getServiceRegistry()
- .getService(ConnectionProvider.class);
- this.dataSource = connectionProvider != null ? connectionProvider.unwrap(DataSource.class) : null;
+ .serviceRegistry
+ .getService(ConnectionProvider)
+ this.dataSource = connectionProvider != null ? connectionProvider.unwrap(DataSource) : null
if (this.dataSource != null) {
if (this.dataSource instanceof TransactionAwareDataSourceProxy) {
- DataSource target = ((TransactionAwareDataSourceProxy) this.dataSource).getTargetDataSource();
+ DataSource target = ((TransactionAwareDataSourceProxy) this.dataSource).targetDataSource
if (target != null) {
- this.dataSource = target;
+ this.dataSource = target
}
}
- jdbcExceptionTranslator = new SQLErrorCodeSQLExceptionTranslator(this.dataSource);
+ jdbcExceptionTranslator = new SQLErrorCodeSQLExceptionTranslator(this.dataSource)
} else {
// must be in unit test mode, setup default translator
SQLErrorCodeSQLExceptionTranslator sqlErrorCodeSQLExceptionTranslator =
- new SQLErrorCodeSQLExceptionTranslator();
- sqlErrorCodeSQLExceptionTranslator.setDatabaseProductName("H2");
- jdbcExceptionTranslator = sqlErrorCodeSQLExceptionTranslator;
+ new SQLErrorCodeSQLExceptionTranslator()
+ sqlErrorCodeSQLExceptionTranslator.databaseProductName = 'H2'
+ jdbcExceptionTranslator = sqlErrorCodeSQLExceptionTranslator
}
}
- public GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore) {
- this(sessionFactory);
+ GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore) {
+ this(sessionFactory)
if (datastore != null) {
- cacheQueries = datastore.isCacheQueries();
- this.osivReadOnly = datastore.isOsivReadOnly();
- this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate();
- this.flushMode = hibernateFlushModeToConstant(datastore.getDefaultFlushMode());
+ cacheQueries = datastore.isCacheQueries()
+ this.osivReadOnly = datastore.isOsivReadOnly()
+ this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate()
+ this.flushMode = hibernateFlushModeToConstant(datastore.defaultFlushMode)
}
}
- public GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore, int defaultFlushMode) {
- this(sessionFactory);
+ GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore, int defaultFlushMode) {
+ this(sessionFactory)
if (datastore != null) {
- cacheQueries = datastore.isCacheQueries();
- this.osivReadOnly = datastore.isOsivReadOnly();
- this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate();
+ cacheQueries = datastore.isCacheQueries()
+ this.osivReadOnly = datastore.isOsivReadOnly()
+ this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate()
}
- this.flushMode = defaultFlushMode;
+ this.flushMode = defaultFlushMode
}
/** Maps a Hibernate {@link FlushMode} to one of the {@code FLUSH_*} constants of this class. */
- static int hibernateFlushModeToConstant(FlushMode mode) {
- return switch (mode) {
- case MANUAL -> FLUSH_NEVER;
- case COMMIT -> FLUSH_COMMIT;
- case ALWAYS -> FLUSH_ALWAYS;
- default -> FLUSH_AUTO;
- };
+ private static int hibernateFlushModeToConstant(FlushMode mode) {
Review Comment:
Done in 9f74af0796: `@PackageScope static`, the Java visibility, and the spec calls a real package-private method.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HqlQueryContext.groovy:
##########
@@ -52,83 +43,102 @@
*
* <p>Use {@link #prepare} to build an instance from raw inputs.
*/
-@SuppressWarnings({
+// TODO(groovyism-cleanup): semicolons + double-quoted strings kept to preserve rename-similarity against the original .java (see matrei's review on #16576). Removed by the stacked groovyism-cleanup PR.
+@CompileStatic
+@SuppressWarnings([
"PMD.AvoidDuplicateLiterals",
"PMD.DataflowAnomalyAnalysis",
"PMD.AvoidLiteralsInIfCondition",
- "PMD.UseLocaleWithCaseConversions"
-})
-public record HqlQueryContext(
+ 'PMD.UseLocaleWithCaseConversions',
+ 'ClassStartsWithBlankLine',
+ 'Indentation',
+ 'UnnecessarySemicolon',
+ 'UnnecessaryGString'
+])
+@RecordOptions(getAt = false, toList = false, toMap = false, size = false)
+@TupleConstructor(defaults = false)
Review Comment:
Done in 9f74af0796: `@TupleConstructor(defaults = false, namedVariant = false)` on `HqlQueryContext`, `HibernateQueryExecutor`, `HibernateAlias`, `PropertyArithmetic` and `CollectionHolder`; the `javap` sweep no longer shows `createAssertError` for them.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/SqlRestriction.groovy:
##########
@@ -34,100 +33,106 @@
* @param values the values of the {@code ?} placeholders, in order
* @since 8.0.0
*/
-public record SqlRestriction(String sql, List<?> values) implements Query.Criterion {
+@CompileStatic
+record SqlRestriction(String sql, List<?> values) implements Query.Criterion {
Review Comment:
Done in 9f74af0796, with a detail worth knowing: adding `@TupleConstructor` to `SqlRestriction` made the generated constructor skip the validation of its compact constructor (`a null value is rejected` failed with a NullPointerException), so its canonical constructor is written out and the options are on it. `GroovyProxyInterceptorLogic.InterceptorState` and the liquibase `IdentifierGeneratorSupport.SequenceKey` got the same options; CodeNarc reads the components of an annotated record as fields and reports their indentation, so the three suppress `Indentation` with a comment.
##########
grails-data-hibernate7/dbmigration-core/src/test/groovy/liquibase/ext/hibernate/database/HibernateDatabasePojoTest.groovy:
##########
@@ -0,0 +1,22 @@
+package liquibase.ext.hibernate.database
+
+import org.junit.Test
+
+import static org.junit.Assert.assertFalse
+
+/**
+ * {@link HibernateDatabase} has a public {@code getProperty(String)} that returns a connection property.
+ * A Groovy class would take it for the {@code GroovyObject} hook behind every dynamic property access,
+ * so the whole family is compiled without {@code GroovyObject}.
+ */
+class HibernateDatabasePojoTest {
Review Comment:
Done in 9f74af0796: the test has the ASF header.
##########
grails-data-hibernate7/core/src/main/groovy/grails/orm/HibernateCriteriaBuilder.groovy:
##########
@@ -1219,37 +1215,37 @@ public ProjectionList sqlGroupProjection(String sql, String groupBy, List<String
*/
private <T> T closingSessionOnError(Supplier<T> projections) {
try {
- return projections.get();
+ return projections.get()
} catch (IllegalArgumentException e) {
- closeSessionFollowingException();
- throw e;
+ closeSessionFollowingException()
+ throw e
}
}
/**
* Restricts the results with a native SQL condition. {@code {alias}} in the SQL stands for the table alias of
* the queried entity.
*
- * @param sqlRestriction the SQL condition
+ * @param sql the SQL condition
* @return this criteria
*/
- public org.grails.datastore.mapping.query.api.Criteria sqlRestriction(String sqlRestriction) {
- return sqlRestriction(sqlRestriction, Collections.emptyList());
+ org.grails.datastore.mapping.query.api.Criteria sqlRestriction(String sql) {
Review Comment:
Done in 9f74af0796: the parameter is `sqlRestriction` in `HibernateCriteriaBuilder` (both overloads) and `HibernateDetachedCriteriaExtension`, with the Javadoc `@param`. In Groovy a parameter with the name of a method of the same class shadows it, which is probably why the conversion renamed it, so the overloads call each other with an explicit receiver (`this.sqlRestriction(...)`, `HibernateDetachedCriteriaExtension.sqlRestriction(...)`).
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6299983981
## ๐จ All tests passed but jobs failed ๐จ
### Failed Jobs without Test Failures
โ [CI / Mongodb Functional Tests \(Java 21, MongoDB 8.0, indy\=false\)](https://github.com/apache/grails-core/actions/runs/39472092132/job/118799005690?pr=16572)
๐ท๏ธ Commit: d2e9cf901d0ba550a68f01152cc0dd996ccfbeb2
โถ๏ธ Tests: 41954 executed
๐ก Checks: 75/83 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
๐ฒ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub) via notifications" <no...@grails.apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6548968683
## ๐ No tests executed ๐
๐ท๏ธ Commit: 7cf2830cd51c6bc3ab4570e73b0abe7af1743fa4
โถ๏ธ Tests: 0 executed
โช๏ธ Checks: 23/23 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6100553553
Besides the ten findings, the same sweep (javap -public of the 9.0.x build against this branch, for all 319 converted files) found about 130 dropped nullness annotations, a record that had stopped being one (CollectionHolder) and the checked exceptions of configureNamingStrategy; all restored in ee3d2b0b58 and pinned by JavaFacingApiSpec. The few differences that cannot be matched are Groovy artifacts (extra generated record constructors, two bridge methods, two classes that became public); the stack on top (#16568-#16570) has the fix merged in.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6101687757
## โ
All tests passed โ
๐ท๏ธ Commit: 4b481384f6fb3e5c664b7fe65fc6d2d676729859
โถ๏ธ Tests: 102968 executed
โช๏ธ Checks: 89/89 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4365834134
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsDomainBinder.groovy:
##########
@@ -0,0 +1,311 @@
+/*
Review Comment:
Done in the rebuild: `566d61db27` only moves the Java sources to their Groovy paths (324 `R100` renames, `GrailsDomainBinder.java` โ `GrailsDomainBinder.groovy` among them), and `68ff6d8967` converts them. `git log --follow` on `GrailsDomainBinder.groovy` now reaches the Java history back to the hibernate6 folder rename, and the same holds for the files whose combined diff is below the rename threshold. The PR description asks for a merge commit so that both commits survive.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4238302016
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HibernateAlias.groovy:
##########
@@ -16,21 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.orm.hibernate.query;
+package org.grails.orm.hibernate.query
-import jakarta.persistence.criteria.JoinType;
+import groovy.transform.CompileStatic
+import jakarta.persistence.criteria.JoinType
-import org.grails.datastore.mapping.query.Query;
+import org.grails.datastore.mapping.query.Query
/**
* A internal criterion used to represent an alias for a basic collection join.
*
* @author walterduquedeestrada
*/
-public record HibernateAlias(String path, String alias, JoinType joinType)
+@CompileStatic
+@SuppressWarnings(['ClassStartsWithBlankLine', 'Indentation'])
+record HibernateAlias(String path, String alias, JoinType joinType)
implements Query.Criterion, Query.QueryElement {
- public HibernateAlias(String path, String alias) {
- this(path, alias, JoinType.INNER);
- }
}
Review Comment:
The two-argument constructor was dropped during the record conversion. Existing callers in `HibernateQuerySpec` and `PredicateGeneratorSpec` still instantiate `new HibernateAlias(path, alias)`, so those paths now fail at runtime instead of defaulting to `JoinType.INNER`. Restore the overload.
##########
grails-data-hibernate7/core/src/main/groovy/grails/orm/HibernateCriteriaBuilder.groovy:
##########
@@ -125,133 +118,136 @@
* @see grails.gorm.DetachedCriteria
*/
@Slf4j
-@SuppressWarnings("PMD.AvoidDuplicateLiterals")
-public class HibernateCriteriaBuilder extends GroovyObjectSupport implements BuildableCriteria, ProjectionList {
- private final SessionFactory sessionFactory;
- private final boolean participate;
- private final org.hibernate.query.criteria.HibernateCriteriaBuilder cb;
- private final HibernateQuery hibernateQuery;
- private Class<?> targetClass;
- private CriteriaQuery<?> criteriaQuery;
- private boolean uniqueResult = false;
-
- @SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName")
- private boolean scroll;
-
- @SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName")
- private boolean count;
-
- private boolean paginationEnabledList = false;
- private int defaultFlushMode;
-
- @SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName")
- private boolean distinct = false;
- private CriteriaMethodInvoker criteriaMethodInvoker;
-
- @SuppressWarnings({"rawtypes", "PMD.CloseResource"})
- public HibernateCriteriaBuilder(Class targetClass, SessionFactory sessionFactory, HibernateDatastore datastore) {
- this.targetClass = targetClass;
- setDatastore(datastore);
- this.sessionFactory = sessionFactory;
- this.cb = sessionFactory.getCriteriaBuilder();
+@SuppressWarnings('PMD.AvoidDuplicateLiterals')
+@CompileStatic
+class HibernateCriteriaBuilder extends GroovyObjectSupport implements BuildableCriteria, ProjectionList {
+
+ private final SessionFactory sessionFactory
+ private final boolean participate
+ private final org.hibernate.query.criteria.HibernateCriteriaBuilder cb
+ private final HibernateQuery hibernateQuery
+ private Class<?> targetClass
+ private CriteriaQuery<?> criteriaQuery
+ private boolean uniqueResult = false
+
+ @SuppressWarnings('PMD.AvoidFieldNameMatchingMethodName')
+ private boolean scroll
+
+ @SuppressWarnings('PMD.AvoidFieldNameMatchingMethodName')
+ private boolean count
+
+ private boolean paginationEnabledList = false
+ private int defaultFlushMode
+
+ @SuppressWarnings('PMD.AvoidFieldNameMatchingMethodName')
+ private boolean distinct = false
+ private CriteriaMethodInvoker criteriaMethodInvoker
+
+ @SuppressWarnings(['rawtypes', 'PMD.CloseResource'])
+ HibernateCriteriaBuilder(Class targetClass, SessionFactory sessionFactory, HibernateDatastore datastore) {
+ this.targetClass = targetClass
+ setDatastore(datastore)
+ this.sessionFactory = sessionFactory
+ this.cb = sessionFactory.getCriteriaBuilder()
if (TransactionSynchronizationManager.hasResource(sessionFactory)) {
- this.participate = true;
- } else {
- this.participate = false;
- org.hibernate.Session session = sessionFactory.openSession();
- TransactionSynchronizationManager.bindResource(sessionFactory, new SessionHolder(session));
+ this.participate = true
}
- HibernateSession session = (HibernateSession) datastore.connect();
+ else {
+ this.participate = false
+ org.hibernate.Session session = sessionFactory.openSession()
+ TransactionSynchronizationManager.bindResource(sessionFactory, new SessionHolder(session))
+ }
+ HibernateSession session = (HibernateSession) datastore.connect()
hibernateQuery = new HibernateQuery(
- session, (GrailsHibernatePersistentEntity) datastore.getMappingContext().getPersistentEntity(targetClass.getName()));
- setDefaultFlushMode(GrailsHibernateTemplate.FLUSH_AUTO);
- criteriaMethodInvoker = new CriteriaMethodInvoker(this);
+ session, (GrailsHibernatePersistentEntity) datastore.getMappingContext().getPersistentEntity(targetClass.getName()))
+ setDefaultFlushMode(GrailsHibernateTemplate.FLUSH_AUTO)
+ criteriaMethodInvoker = new CriteriaMethodInvoker(this)
}
- public static final String ALIAS_SEPARATOR = ":";
+ public static final String ALIAS_SEPARATOR = ':'
// The types a sqlProjection or sqlGroupProjection takes in the criteria DSL, as on Hibernate 5
- public static final BasicTypeReference<?> BOOLEAN = StandardBasicTypes.BOOLEAN;
- public static final BasicTypeReference<?> YES_NO = StandardBasicTypes.YES_NO;
- public static final BasicTypeReference<?> BYTE = StandardBasicTypes.BYTE;
- public static final BasicTypeReference<?> CHARACTER = StandardBasicTypes.CHARACTER;
- public static final BasicTypeReference<?> SHORT = StandardBasicTypes.SHORT;
- public static final BasicTypeReference<?> INTEGER = StandardBasicTypes.INTEGER;
- public static final BasicTypeReference<?> LONG = StandardBasicTypes.LONG;
- public static final BasicTypeReference<?> FLOAT = StandardBasicTypes.FLOAT;
- public static final BasicTypeReference<?> DOUBLE = StandardBasicTypes.DOUBLE;
- public static final BasicTypeReference<?> BIG_DECIMAL = StandardBasicTypes.BIG_DECIMAL;
- public static final BasicTypeReference<?> BIG_INTEGER = StandardBasicTypes.BIG_INTEGER;
- public static final BasicTypeReference<?> STRING = StandardBasicTypes.STRING;
- public static final BasicTypeReference<?> NUMERIC_BOOLEAN = StandardBasicTypes.NUMERIC_BOOLEAN;
- public static final BasicTypeReference<?> TRUE_FALSE = StandardBasicTypes.TRUE_FALSE;
- public static final BasicTypeReference<?> URL = StandardBasicTypes.URL;
- public static final BasicTypeReference<?> TIME = StandardBasicTypes.TIME;
- public static final BasicTypeReference<?> DATE = StandardBasicTypes.DATE;
- public static final BasicTypeReference<?> TIMESTAMP = StandardBasicTypes.TIMESTAMP;
- public static final BasicTypeReference<?> CALENDAR = StandardBasicTypes.CALENDAR;
- public static final BasicTypeReference<?> CALENDAR_DATE = StandardBasicTypes.CALENDAR_DATE;
- public static final BasicTypeReference<?> CLASS = StandardBasicTypes.CLASS;
- public static final BasicTypeReference<?> LOCALE = StandardBasicTypes.LOCALE;
- public static final BasicTypeReference<?> CURRENCY = StandardBasicTypes.CURRENCY;
- public static final BasicTypeReference<?> TIMEZONE = StandardBasicTypes.TIMEZONE;
- public static final BasicTypeReference<?> UUID_BINARY = StandardBasicTypes.UUID_BINARY;
- public static final BasicTypeReference<?> UUID_CHAR = StandardBasicTypes.UUID_CHAR;
- public static final BasicTypeReference<?> BINARY = StandardBasicTypes.BINARY;
- public static final BasicTypeReference<?> WRAPPER_BINARY = StandardBasicTypes.BINARY_WRAPPER;
- public static final BasicTypeReference<?> IMAGE = StandardBasicTypes.IMAGE;
- public static final BasicTypeReference<?> BLOB = StandardBasicTypes.BLOB;
- public static final BasicTypeReference<?> MATERIALIZED_BLOB = StandardBasicTypes.MATERIALIZED_BLOB;
- public static final BasicTypeReference<?> CHAR_ARRAY = StandardBasicTypes.CHAR_ARRAY;
- public static final BasicTypeReference<?> CHARACTER_ARRAY = StandardBasicTypes.CHARACTER_ARRAY;
- public static final BasicTypeReference<?> TEXT = StandardBasicTypes.TEXT;
- public static final BasicTypeReference<?> CLOB = StandardBasicTypes.CLOB;
- public static final BasicTypeReference<?> MATERIALIZED_CLOB = StandardBasicTypes.MATERIALIZED_CLOB;
- public static final BasicTypeReference<?> SERIALIZABLE = StandardBasicTypes.SERIALIZABLE;
+ static final BasicTypeReference<?> BOOLEAN = StandardBasicTypes.BOOLEAN
+ static final BasicTypeReference<?> YES_NO = StandardBasicTypes.YES_NO
+ static final BasicTypeReference<?> BYTE = StandardBasicTypes.BYTE
+ static final BasicTypeReference<?> CHARACTER = StandardBasicTypes.CHARACTER
+ static final BasicTypeReference<?> SHORT = StandardBasicTypes.SHORT
Review Comment:
The Java version exposed all SQL projection type constants as `public static final` fields, but the conversion removed `public`. That turns them into Groovy properties and breaks Java source/binary access such as `HibernateCriteriaBuilder.BOOLEAN`. Restore public field visibility for this entire constant block.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BackticksRemover.groovy:
##########
@@ -28,16 +29,21 @@
*
* @since 8.0
*/
-public class BackticksRemover implements Function<String, String> {
+@CompileStatic
+class BackticksRemover implements Function<String, String> {
- public static final String BACKTICK = "`";
+ static final String BACKTICK = '`'
Review Comment:
This constant was public in the Java source. The conversion now exposes only Groovy property access rather than the original Java field, breaking Java callers of `BackticksRemover.BACKTICK`; keep it explicitly public.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/IdentityEnumType.groovy:
##########
@@ -0,0 +1,240 @@
+/*
+ * 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
+
+import groovy.transform.CompileStatic
+import org.hibernate.HibernateException
+import org.hibernate.MappingException
+import org.hibernate.engine.spi.SharedSessionContractImplementor
+import org.hibernate.type.descriptor.WrapperOptions
+import org.hibernate.type.descriptor.java.JavaType
+import org.hibernate.type.descriptor.jdbc.BigIntJdbcType
+import org.hibernate.type.descriptor.jdbc.IntegerJdbcType
+import org.hibernate.type.descriptor.jdbc.JdbcType
+import org.hibernate.type.descriptor.jdbc.VarcharJdbcType
+import org.hibernate.type.spi.TypeConfiguration
+import org.hibernate.usertype.ParameterizedType
+import org.hibernate.usertype.UserType
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import java.lang.reflect.InvocationTargetException
+import java.lang.reflect.Method
+import java.sql.PreparedStatement
+import java.sql.ResultSet
+import java.sql.SQLException
+import java.sql.Types
+
+/**
+ * Hibernate 7 UserType to map Enums to their "id" value.
+ */
+@CompileStatic
+class IdentityEnumType implements UserType<Object>, ParameterizedType, Serializable {
+
+ private static final Logger LOG = LoggerFactory.getLogger(IdentityEnumType)
+ private static final TypeConfiguration typeConfiguration = new TypeConfiguration()
+
+ static final String ENUM_ID_ACCESSOR = 'getId'
+ static final String PARAM_ENUM_CLASS = 'enumClass'
Review Comment:
Both constants were public static fields in the Java class. Dropping `public` changes their Java-facing ABI to Groovy properties, so custom type integrations cannot continue referencing `IdentityEnumType.ENUM_ID_ACCESSOR` or `PARAM_ENUM_CLASS`. Preserve the field visibility.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/support/ClosureEventTriggeringInterceptor.groovy:
##########
@@ -103,97 +99,96 @@ public class ClosureEventTriggeringInterceptor
* @deprecated Use {@link AbstractPersistenceEvent#ONLOAD_EVENT} instead
*/
@Deprecated
- public static final String ONLOAD_EVENT = AbstractPersistenceEvent.ONLOAD_EVENT;
+ static final String ONLOAD_EVENT = AbstractPersistenceEvent.ONLOAD_EVENT
Review Comment:
These deprecated event constants were public static fields before the conversion. Removing `public` makes them Groovy properties and breaks Java compatibility before their declared removal; restore public visibility for all ten constants in this block.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsHibernateUtil.groovy:
##########
@@ -0,0 +1,316 @@
+/*
+ * 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
+
+import groovy.transform.CompileStatic
+import org.hibernate.FlushMode
+import org.hibernate.Hibernate
+import org.hibernate.Session
+import org.hibernate.SessionFactory
+import org.hibernate.engine.spi.EntityEntry
+import org.hibernate.engine.spi.SessionImplementor
+import org.hibernate.engine.spi.Status
+import org.hibernate.internal.util.StringHelper
+import org.hibernate.proxy.HibernateProxy
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import org.springframework.transaction.support.TransactionSynchronizationManager
+
+import grails.gorm.annotation.Entity
+import org.grails.datastore.gorm.GormEntity
+import org.grails.datastore.mapping.model.PersistentEntity
+import org.grails.datastore.mapping.model.config.GormProperties
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity
+import org.grails.orm.hibernate.proxy.HibernateProxyHandler
+import org.grails.orm.hibernate.query.HibernateQueryArgument
+import org.grails.orm.hibernate.support.HibernateRuntimeUtils
+
+import java.lang.annotation.Annotation
+
+/**
+ * Utility methods for configuring Hibernate inside Grails.
+ *
+ * @author Graeme Rocher
+ * @since 0.4
+ */
+@CompileStatic
+class GrailsHibernateUtil extends HibernateRuntimeUtils {
+
+ /** @deprecated Use {@link org.grails.orm.hibernate.query.HibernateQueryArgument#FETCH_SIZE} */
+ @Deprecated(since = '8.0', forRemoval = true)
+ static final String ARGUMENT_FETCH_SIZE = HibernateQueryArgument.FETCH_SIZE.value()
Review Comment:
These deprecated constants were public fields before the conversion. In Groovy, omitting `public` declares properties rather than preserving the public static fields, so Java consumers can no longer compile against `GrailsHibernateUtil.ARGUMENT_*`; deprecation promises they remain available until removal. Restore `public` on every converted constant in this block.
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/HibernateOperations.groovy:
##########
@@ -80,9 +75,7 @@ public interface HibernateOperations {
* @see HibernateTransactionManager
* @see org.hibernate.Session
*/
- @Nullable
- <T> T execute(HibernateCallback<T> action) throws DataAccessException;
-
+ def <T> T execute(HibernateCallback<T> action) throws DataAccessException
Review Comment:
`HibernateOperations.execute` previously declared a nullable result, matching its documentation and callback contract. Removing `@Nullable` changes the public interface consumed by Java/Kotlin callers; restore the return annotation here (and on the other nullable operations converted in this interface).
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/HibernateCallback.groovy:
##########
@@ -48,7 +46,6 @@ public interface HibernateCallback<T> {
* @throws HibernateException if thrown by the Hibernate API
* @see HibernateTemplate#execute
*/
- @Nullable
- T doInHibernate(Session session) throws HibernateException;
+ T doInHibernate(Session session) throws HibernateException
Review Comment:
This callback explicitly allowed a null result before the conversion, but the `@Nullable` return annotation was removed. Since this is a public functional interface, that changes the nullness contract seen by Java/Kotlin callers and static-analysis tools even though `HibernateTemplate` still documents and returns null results. Preserve the annotation.
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/support/OpenSessionInViewInterceptor.groovy:
##########
@@ -69,111 +67,109 @@
* @see TransactionSynchronizationManager
* @see SessionFactory#getCurrentSession()
*/
-public class OpenSessionInViewInterceptor implements AsyncWebRequestInterceptor {
+@CompileStatic
+class OpenSessionInViewInterceptor implements AsyncWebRequestInterceptor {
/**
* Suffix that gets appended to the {@code SessionFactory}
* {@code toString()} representation for the "participate in existing
* session handling" request attribute.
* @see #getParticipateAttributeName
*/
- public static final String PARTICIPATE_SUFFIX = ".PARTICIPATE";
+ static final String PARTICIPATE_SUFFIX = '.PARTICIPATE'
Review Comment:
`PARTICIPATE_SUFFIX` was public before this conversion. An unqualified Groovy property does not preserve the public static field consumed by Java extensions, so this is an avoidable API break; retain the original visibility.
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/SessionFactoryUtils.groovy:
##########
@@ -80,18 +77,20 @@
* @see HibernateExceptionTranslator
* @see HibernateTransactionManager
*/
-public abstract class SessionFactoryUtils {
+@CompileStatic
+abstract class SessionFactoryUtils {
/**
* Order value for TransactionSynchronization objects that clean up Hibernate Sessions.
* Returns {@code DataSourceUtils.CONNECTION_SYNCHRONIZATION_ORDER - 100}
* to execute Session cleanup before JDBC Connection cleanup, if any.
* @see DataSourceUtils#CONNECTION_SYNCHRONIZATION_ORDER
*/
- public static final int SESSION_SYNCHRONIZATION_ORDER =
- DataSourceUtils.CONNECTION_SYNCHRONIZATION_ORDER - 100;
+ static final int SESSION_SYNCHRONIZATION_ORDER =
+ DataSourceUtils.CONNECTION_SYNCHRONIZATION_ORDER - 100
Review Comment:
This synchronization-order constant was a public static field before conversion. Without explicit `public`, Java subclasses and integrations can no longer access `SessionFactoryUtils.SESSION_SYNCHRONIZATION_ORDER`; restore its original visibility.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6099984052
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16572?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 60.9253%. Comparing base ([`8ef4408`](https://app.codecov.io/gh/apache/grails-core/commit/8ef440820ec0d9c9343ab6c768df65e5f0c7062c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`fcf18b2`](https://app.codecov.io/gh/apache/grails-core/commit/fcf18b26abb936ae47be575cd6174d889f013ebe?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16572?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 9.0.x #16572 +/- ##
==================================================
+ Coverage 60.8615% 60.9253% +0.0638%
- Complexity 26375 26894 +519
==================================================
Files 2226 2259 +33
Lines 110970 113388 +2418
Branches 20095 20524 +429
==================================================
+ Hits 67538 69082 +1544
- Misses 34519 35168 +649
- Partials 8913 9138 +225
```
[see 41 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16572/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6197897791
## โ
All tests passed โ
๐ท๏ธ Commit: f742ce8a868126364b7f68f107bb181241b71648
โถ๏ธ Tests: 61502 executed
โช๏ธ Checks: 90/90 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub) via notifications" <no...@grails.apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6562987246
@matrei @jdaugherty the branch was rebuilt on 10.0.x as the two-commit split (1c730e1be4 move, 7cf2830cd5 convert) and no longer conflicts. All inline threads have a reply. Could you take another look when you have time?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6207204715
## ๐จ All tests passed but jobs failed ๐จ
### Failed Jobs without Test Failures
โ [CI / Validate githubBranch](https://github.com/apache/grails-core/actions/runs/38844093992/job/116790542987?pr=16572)
๐ท๏ธ Commit: 67f85fe13c57e1414559a9cfe989c72f94d75871
โถ๏ธ Tests: 378 executed
๐ก Checks: 15/83 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
๐ฒ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6160543608
## โ
All tests passed โ
๐ท๏ธ Commit: 1000a858733fef3ba05fe4b14c864837ceb1ba91
โถ๏ธ Tests: 92027 executed
โช๏ธ Checks: 89/89 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4361364345
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateSession.groovy:
##########
@@ -76,8 +67,10 @@
* @author Graeme Rocher
* @since 1.0
*/
-@SuppressWarnings({"rawtypes", "PMD.DataflowAnomalyAnalysis", "PMD.AvoidDuplicateLiterals"})
-public class HibernateSession extends AbstractAttributeStoringSession implements QueryAliasAwareSession {
+// TODO(groovyism-cleanup): semicolons + double-quoted strings kept to preserve rename-similarity against the original .java (see matrei's review on #16576). Removed by the stacked groovyism-cleanup PR.
Review Comment:
This comment, and the `@SuppressWarnings('UnnecessarySemicolon')` on the package statement above, are in 93 files, and the reason they give is gone: the move commit preserves the history now, not the similarity of this diff. What is left is two styles across the 324 converted files, chosen by a git heuristic: 136 files keep 5,431 lines ending in a semicolon (1,423 imports and 119 package statements among them), 47 class-level suppressions hide `UnnecessaryGString` over 607 double-quoted strings without interpolation, and 188 files are already clean. Please strip the semicolons and the plain double quotes in the convert commit and delete the suppressions and these comments, so CodeNarc enforces it from here. The semicolons are a `sed -E 's/;[[:space:]]*$//'` over the converted files (or the semicolon step of the `codenarcFix` task; its GString step also rewrites quotes inside comments and the license headers, so that output can't be taken as is). `codenarcTest` is skipped in this b
uild, so the 43 test and fixture files in `dbmigration-core` that have semicolons and no suppression need the same pass by hand.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/OrderByClauseBuilder.groovy:
##########
@@ -16,110 +16,120 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.orm.hibernate.cfg.domainbinding.util;
+package org.grails.orm.hibernate.cfg.domainbinding.util
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Locale;
-import java.util.stream.Collectors;
+import groovy.transform.CompileStatic
+import groovy.transform.PackageScope
+import org.hibernate.boot.model.internal.BinderHelper
+import org.hibernate.mapping.PersistentClass
+import org.hibernate.mapping.Property
+import org.hibernate.mapping.SingleTableSubclass
+import org.hibernate.mapping.Selectable
-import org.hibernate.boot.model.internal.BinderHelper;
-import org.hibernate.mapping.PersistentClass;
-import org.hibernate.mapping.Property;
-import org.hibernate.mapping.SingleTableSubclass;
-
-import org.grails.datastore.mapping.model.DatastoreConfigurationException;
+import org.grails.datastore.mapping.model.DatastoreConfigurationException
/** Utility class to build SQL order by clauses from HQL-style order by strings. */
-@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
-public class OrderByClauseBuilder {
+@CompileStatic
+@SuppressWarnings('PMD.DataflowAnomalyAnalysis')
+class OrderByClauseBuilder {
- public String buildOrderByClause(
+ String buildOrderByClause(
String hqlOrderBy, PersistentClass associatedClass, String role, String defaultOrder) {
if (hqlOrderBy == null) {
- return null;
+ return null
}
if (hqlOrderBy.isEmpty()) {
- return associatedClass.getIdentifier().getSelectables().stream()
- .map(selectable -> selectable.getText() + " asc")
- .collect(Collectors.joining(", "));
+ List<String> parts = []
+ for (Selectable selectable : associatedClass.identifier.selectables) {
+ parts.add(selectable.text + ' asc')
+ }
+ return parts.join(', ')
}
- List<SortEntry> entries = parseSortEntries(hqlOrderBy, role, defaultOrder);
+ List<SortEntry> entries = parseSortEntries(hqlOrderBy, role, defaultOrder)
- return entries.stream()
- .map(entry -> buildPropertyOrderBy(entry, associatedClass))
- .collect(Collectors.joining(", "));
+ List<String> parts = []
+ for (SortEntry entry : entries) {
+ parts.add(buildPropertyOrderBy(entry, associatedClass))
+ }
+ return parts.join(', ')
}
private List<SortEntry> parseSortEntries(String hqlOrderBy, String role, String defaultOrder) {
- String[] tokens = hqlOrderBy.split("[ ,]+");
- List<SortEntry> entries = new ArrayList<>();
- SortEntry currentEntry = null;
+ String[] tokens = hqlOrderBy.split('[ ,]+')
+ List<SortEntry> entries = new ArrayList<>()
+ SortEntry currentEntry = null
for (String token : tokens) {
- if (token.isEmpty()) continue;
+ if (token.isEmpty()) {
+ continue
+ }
if (isDirectionToken(token)) {
if (currentEntry == null || currentEntry.direction != null) {
throw new DatastoreConfigurationException(
- "Error while parsing sort clause: " + hqlOrderBy + " (" + role + ")");
+ "Error while parsing sort clause: ${hqlOrderBy} (${role})".toString())
}
- currentEntry.direction = token.toLowerCase(Locale.ROOT);
- } else {
+ currentEntry.direction = token.toLowerCase(Locale.ROOT)
Review Comment:
`javap` shows this write, and the two below it, compile to `ScriptBytecodeAdapter.setGroovyObjectProperty`: `direction` is a `@PackageScope` field of a private nested class, written from the outer class. It isn't in the description's list of remaining dynamic sites. `currentEntry.@direction = โฆ` (or a setter on `SortEntry`) makes it a field write.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/PersistentEntityNamingStrategy.groovy:
##########
@@ -16,26 +16,27 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.orm.hibernate.cfg;
+package org.grails.orm.hibernate.cfg
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity;
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty;
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty
/**
* Allows plugging into to custom naming strategies
*
* @author Graeme Rocher
* @since 5.0
*/
-public interface PersistentEntityNamingStrategy {
+interface PersistentEntityNamingStrategy {
Review Comment:
Default methods are code, so this interface needs `@CompileStatic` like every other converted type; without it their bodies compile dynamically. `grep -L '@CompileStatic'` over the moved files finds this one plus 14 converted tests and fixtures in `dbmigration-core` that have none either: `ColumnRemarksIntegrationTest`, `HibernateIndexDifferenceSuppressionTest`, `CustomIdGeneratorTest`, `IdentifierGeneratorSnapshotTest`, `NativeGeneratorAutoIncrementTest`, `NativeGeneratorSequenceTest`, `SequenceGeneratorTest`, `columnremarks/Item`, `CustomIdEntity`, `NativeGenEntity`, `SnowflakeId`, `SnowflakeIdGenerator`, `ForcedTableEntity`, `QualifiedSequenceEntity` (the batch #16497 added; the older fixtures have it). The remaining hits are pure interfaces and `ManagedEntity`, which need nothing.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HqlQueryContext.groovy:
##########
@@ -52,83 +43,102 @@
*
* <p>Use {@link #prepare} to build an instance from raw inputs.
*/
-@SuppressWarnings({
+// TODO(groovyism-cleanup): semicolons + double-quoted strings kept to preserve rename-similarity against the original .java (see matrei's review on #16576). Removed by the stacked groovyism-cleanup PR.
+@CompileStatic
+@SuppressWarnings([
"PMD.AvoidDuplicateLiterals",
"PMD.DataflowAnomalyAnalysis",
"PMD.AvoidLiteralsInIfCondition",
- "PMD.UseLocaleWithCaseConversions"
-})
-public record HqlQueryContext(
+ 'PMD.UseLocaleWithCaseConversions',
+ 'ClassStartsWithBlankLine',
+ 'Indentation',
+ 'UnnecessarySemicolon',
+ 'UnnecessaryGString'
+])
+@RecordOptions(getAt = false, toList = false, toMap = false, size = false)
+@TupleConstructor(defaults = false)
Review Comment:
The public `Map` constructor is still generated: `javap` shows `HqlQueryContext(java.util.Map)`, and those constructors are the only place in the converted classes where `InvokerHelper.createAssertError` appears (9 sites here, 10 in `HibernateQueryExecutor`, 4 each in `HibernateAlias` and `PropertyArithmetic`). `@TupleConstructor(defaults = false, namedVariant = false)` removes it.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.groovy:
##########
@@ -134,111 +127,113 @@ public class GrailsHibernateTemplate implements IHibernateTemplate {
*
* @see #setFlushMode
*/
- public static final int FLUSH_ALWAYS = 4;
-
- private static final Logger LOG = LoggerFactory.getLogger(GrailsHibernateTemplate.class);
- protected boolean exposeNativeSession = true;
- protected boolean cacheQueries = false;
- protected SessionFactory sessionFactory;
- protected DataSource dataSource = null;
- protected SQLExceptionTranslator jdbcExceptionTranslator;
- protected int flushMode = FLUSH_AUTO;
- private boolean osivReadOnly;
- private boolean passReadOnlyToHibernate = false;
- private boolean applyFlushModeOnlyToNonExistingTransactions = false;
- protected TransactionResources txResources = new DefaultTransactionResources();
+ public static final int FLUSH_ALWAYS = 4
+
+ private static final Logger LOG = LoggerFactory.getLogger(GrailsHibernateTemplate)
+ protected boolean exposeNativeSession = true
+ protected boolean cacheQueries = false
+ protected SessionFactory sessionFactory
+ protected DataSource dataSource = null
+ protected SQLExceptionTranslator jdbcExceptionTranslator
+ protected int flushMode = FLUSH_AUTO
+ private boolean osivReadOnly
+ private boolean passReadOnlyToHibernate = false
+ private boolean applyFlushModeOnlyToNonExistingTransactions = false
+ protected TransactionResources txResources = new DefaultTransactionResources()
protected GrailsHibernateTemplate() {
// for testing
}
- public GrailsHibernateTemplate(SessionFactory sessionFactory) {
- Assert.notNull(sessionFactory, "Property 'sessionFactory' is required");
- this.sessionFactory = sessionFactory;
+ GrailsHibernateTemplate(SessionFactory sessionFactory) {
+ Assert.notNull(sessionFactory, "Property 'sessionFactory' is required")
+ this.sessionFactory = sessionFactory
ConnectionProvider connectionProvider = ((SessionFactoryImplementor) sessionFactory)
- .getServiceRegistry()
- .getService(ConnectionProvider.class);
- this.dataSource = connectionProvider != null ? connectionProvider.unwrap(DataSource.class) : null;
+ .serviceRegistry
+ .getService(ConnectionProvider)
+ this.dataSource = connectionProvider != null ? connectionProvider.unwrap(DataSource) : null
if (this.dataSource != null) {
if (this.dataSource instanceof TransactionAwareDataSourceProxy) {
- DataSource target = ((TransactionAwareDataSourceProxy) this.dataSource).getTargetDataSource();
+ DataSource target = ((TransactionAwareDataSourceProxy) this.dataSource).targetDataSource
if (target != null) {
- this.dataSource = target;
+ this.dataSource = target
}
}
- jdbcExceptionTranslator = new SQLErrorCodeSQLExceptionTranslator(this.dataSource);
+ jdbcExceptionTranslator = new SQLErrorCodeSQLExceptionTranslator(this.dataSource)
} else {
// must be in unit test mode, setup default translator
SQLErrorCodeSQLExceptionTranslator sqlErrorCodeSQLExceptionTranslator =
- new SQLErrorCodeSQLExceptionTranslator();
- sqlErrorCodeSQLExceptionTranslator.setDatabaseProductName("H2");
- jdbcExceptionTranslator = sqlErrorCodeSQLExceptionTranslator;
+ new SQLErrorCodeSQLExceptionTranslator()
+ sqlErrorCodeSQLExceptionTranslator.databaseProductName = 'H2'
+ jdbcExceptionTranslator = sqlErrorCodeSQLExceptionTranslator
}
}
- public GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore) {
- this(sessionFactory);
+ GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore) {
+ this(sessionFactory)
if (datastore != null) {
- cacheQueries = datastore.isCacheQueries();
- this.osivReadOnly = datastore.isOsivReadOnly();
- this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate();
- this.flushMode = hibernateFlushModeToConstant(datastore.getDefaultFlushMode());
+ cacheQueries = datastore.isCacheQueries()
+ this.osivReadOnly = datastore.isOsivReadOnly()
+ this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate()
+ this.flushMode = hibernateFlushModeToConstant(datastore.defaultFlushMode)
}
}
- public GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore, int defaultFlushMode) {
- this(sessionFactory);
+ GrailsHibernateTemplate(SessionFactory sessionFactory, HibernateDatastore datastore, int defaultFlushMode) {
+ this(sessionFactory)
if (datastore != null) {
- cacheQueries = datastore.isCacheQueries();
- this.osivReadOnly = datastore.isOsivReadOnly();
- this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate();
+ cacheQueries = datastore.isCacheQueries()
+ this.osivReadOnly = datastore.isOsivReadOnly()
+ this.passReadOnlyToHibernate = datastore.isPassReadOnlyToHibernate()
}
- this.flushMode = defaultFlushMode;
+ this.flushMode = defaultFlushMode
}
/** Maps a Hibernate {@link FlushMode} to one of the {@code FLUSH_*} constants of this class. */
- static int hibernateFlushModeToConstant(FlushMode mode) {
- return switch (mode) {
- case MANUAL -> FLUSH_NEVER;
- case COMMIT -> FLUSH_COMMIT;
- case ALWAYS -> FLUSH_ALWAYS;
- default -> FLUSH_AUTO;
- };
+ private static int hibernateFlushModeToConstant(FlushMode mode) {
Review Comment:
`private` here, package-private in Java, and `GrailsHibernateTemplateSpec` still calls it; that only works because the spec is dynamic Groovy. `@PackageScope` keeps the Java visibility and keeps the spec on a real API.
##########
grails-data-hibernate7/core/src/main/groovy/grails/orm/HibernateCriteriaBuilder.groovy:
##########
@@ -1219,37 +1215,37 @@ public ProjectionList sqlGroupProjection(String sql, String groupBy, List<String
*/
private <T> T closingSessionOnError(Supplier<T> projections) {
try {
- return projections.get();
+ return projections.get()
} catch (IllegalArgumentException e) {
- closeSessionFollowingException();
- throw e;
+ closeSessionFollowingException()
+ throw e
}
}
/**
* Restricts the results with a native SQL condition. {@code {alias}} in the SQL stands for the table alias of
* the queried entity.
*
- * @param sqlRestriction the SQL condition
+ * @param sql the SQL condition
* @return this criteria
*/
- public org.grails.datastore.mapping.query.api.Criteria sqlRestriction(String sqlRestriction) {
- return sqlRestriction(sqlRestriction, Collections.emptyList());
+ org.grails.datastore.mapping.query.api.Criteria sqlRestriction(String sql) {
Review Comment:
The parameter was `sqlRestriction` in Java (here, in the two-argument overload and in `HibernateDetachedCriteriaExtension`), and the Javadoc `@param` moved with it. Callers don't notice, but the conversion shouldn't rename public parameters; please keep the Java names.
##########
grails-data-hibernate7/dbmigration-core/src/test/groovy/liquibase/ext/hibernate/database/HibernateDatabasePojoTest.groovy:
##########
@@ -0,0 +1,22 @@
+package liquibase.ext.hibernate.database
+
+import org.junit.Test
+
+import static org.junit.Assert.assertFalse
+
+/**
+ * {@link HibernateDatabase} has a public {@code getProperty(String)} that returns a connection property.
+ * A Groovy class would take it for the {@code GroovyObject} hook behind every dynamic property access,
+ * so the whole family is compiled without {@code GroovyObject}.
+ */
+class HibernateDatabasePojoTest {
Review Comment:
New Grails test, not fork code, so it needs the ASF header. RAT doesn't see it because `database/**` is excluded wholesale.
##########
grails-data-hibernate7/dbmigration-core/src/test/groovy/liquibase/ext/hibernate/HibernateIntegrationTest.groovy:
##########
@@ -1,81 +1,79 @@
-package liquibase.ext.hibernate;
+package liquibase.ext.hibernate
-import liquibase.Liquibase;
-import liquibase.Scope;
-import liquibase.database.Database;
-import liquibase.database.core.H2Database;
-import liquibase.database.jvm.JdbcConnection;
-import liquibase.diff.DiffResult;
-import liquibase.diff.Difference;
-import liquibase.diff.ObjectDifferences;
-import liquibase.diff.compare.CompareControl;
-import liquibase.diff.output.DiffOutputControl;
-import liquibase.diff.output.changelog.DiffToChangeLog;
-import liquibase.diff.output.report.DiffToReport;
-import liquibase.ext.hibernate.database.HibernateClassicDatabase;
-import liquibase.ext.hibernate.database.connection.HibernateConnection;
-import liquibase.resource.ClassLoaderResourceAccessor;
-import liquibase.resource.DirectoryResourceAccessor;
-import liquibase.structure.DatabaseObject;
-import liquibase.structure.core.*;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
+import java.io.*
Review Comment:
Wildcard imports, here and in `SpringPackageScanningIntegrationTest`; the repository rule is explicit imports, fork test or not.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1449,6 +1449,12 @@ class SportsCar extends Car {
}
----
+===== 26.11 Custom User Types
Review Comment:
This section is #16695's (see the comment on `CompositeUserTypeBinder`); it goes with the rest of that PR.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/SqlRestriction.groovy:
##########
@@ -34,100 +33,106 @@
* @param values the values of the {@code ?} placeholders, in order
* @since 8.0.0
*/
-public record SqlRestriction(String sql, List<?> values) implements Query.Criterion {
+@CompileStatic
+record SqlRestriction(String sql, List<?> values) implements Query.Criterion {
Review Comment:
Public API (`@since 8.0.0`) and still without the record options the other records got: `SqlRestriction()`, `SqlRestriction(String)`, `SqlRestriction(Map)`, `getAt`, `toList`, `toMap` and `size` are all generated. Same for `GroovyProxyInterceptorLogic.InterceptorState`.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4361364307
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeUserTypeBinder.groovy:
##########
@@ -0,0 +1,265 @@
+/*
+ * 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 java.beans.PropertyDescriptor
+import java.lang.reflect.Field
+import java.lang.reflect.Method
+import java.lang.reflect.Modifier
+import java.lang.reflect.RecordComponent
+import java.lang.reflect.Type
+
+import groovy.transform.CompileStatic
+import org.hibernate.MappingException
+import org.hibernate.boot.spi.MetadataBuildingContext
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment
+import org.hibernate.mapping.BasicValue
+import org.hibernate.mapping.Column
+import org.hibernate.mapping.Component
+import org.hibernate.mapping.PersistentClass
+import org.hibernate.mapping.Property
+import org.hibernate.mapping.Table
+import org.hibernate.property.access.internal.PropertyAccessStrategyCompositeUserTypeImpl
+import org.hibernate.property.access.spi.PropertyAccessStrategy
+import org.hibernate.usertype.CompositeUserType
+import org.jspecify.annotations.NonNull
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+import org.springframework.beans.BeanUtils
+import org.springframework.util.ReflectionUtils
+
+import org.grails.datastore.mapping.model.config.GormProperties
+import org.grails.datastore.mapping.reflect.ClassPropertyFetcher
+import org.grails.orm.hibernate.cfg.ColumnConfig
+import org.grails.orm.hibernate.cfg.GrailsHibernateUtil
+import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy
+import org.grails.orm.hibernate.cfg.PropertyConfig
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty
+import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover
+
+/**
+ * Binds a property mapped with a Hibernate {@link CompositeUserType} as a {@link Component} of that type, the way
+ * Hibernate's own annotation binder binds {@code @CompositeType}: one property per attribute of the user type's
+ * {@linkplain CompositeUserType#embeddable() embeddable class}, in alphabetical attribute order, each a basic value
+ * typed by reflection on the embeddable class and accessed through Hibernate's composite-user-type property access.
+ * The columns of the property mapping are assigned to the attributes in that order; an attribute without a configured
+ * column is named {@code <property>_<attribute>}.
+ *
+ * @since 8.0.1
+ */
+@CompileStatic
+class CompositeUserTypeBinder {
Review Comment:
This class, `CompositeUserTypeMappingSpec`, the `CompositeUserType` logic in `GrailsDomainBinder`, `GrailsPropertyBinder` and `HibernatePersistentProperty`, the changes to `GrailsPropertyBinderSpec`, `ListSecondPassBinderSpec`, `MapSecondPassBinderSpec` and `GrailsHibernatePersistentPropertySpec`, the `customHibernateTypes.adoc`, `type.adoc` and `upgrading80x.adoc` ยง26.11 changes and the `hibernate-developer` skill line are #16695, which is open on `8.0.x`, and this copy is behind its head (no `CompositeUserTypeColumns`). None of it is on `10.0.x`. A conversion PR can't carry it: when #16695 merges up, its `.java` files land next to these Groovy twins, and the feature is reviewed twice at two revisions. Please drop everything that comes from #16695; those classes get converted in the slice after it reaches `10.0.x`.
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/MultipleUnidirectionalHasManySpec.groovy:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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
+
+/**
+ * An owner with several unidirectional hasMany associations keeps a join table for each of them, whether they point
+ * at different classes or at the same class with join tables of their own, and an element added to one of them is
+ * written to its own join table. A sibling to-many that is not bound as a collection, such as a serializable one,
+ * neither blocks the session factory nor counts as a sharer of the join table.
+ */
+class MultipleUnidirectionalHasManySpec extends HibernateGormDatastoreSpec {
Review Comment:
Same for #16686, open on `8.0.x`: this spec, the `CollectionKeyColumnUpdaterSpec` changes and the join-table logic in `CollectionKeyColumnUpdater`. Please take them out and convert `CollectionKeyColumnUpdater` as it is on `10.0.x`.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6385192144
@jdaugherty, 10.0.x now carries #16695 (composite user types), so this branch conflicted. Following your checklist:
- `7d7773057f` merges `10.0.x` and ports #16695's Java changes by hand into the Groovy twins: `ColumnBinder`, `CreateKeyForProps`, `ColumnNameForPropertyAndPathFetcher`, `GrailsPropertyBinder`, `HibernatePersistentProperty`, `GrailsDomainBinder`, `SimpleValueBinder`, `NumericColumnConstraintsBinder`, plus the three conflicting specs. All 21 files of #16695 are accounted for.
- `e69d386b26` moves its two new classes, `CompositeUserTypeBinder` and `CompositeUserTypeColumns`: two `R100` lines, nothing else.
- `0cfca82994` converts them: `@CompileStatic`, the nested `Attribute` record with the series' record options and tuple constructor, lambdas, no PMD suppression, comments and Javadoc kept.
Checked locally: no `.java` left in the module apart from `JavaCallerPin`; `javap -c -p` over the new and changed classes shows only `$getStaticMetaClass`, and `Stream.toList()` is the JDK method; core 3935 tests, 1 failure (the Oracle container did not start on this machine), dbmigration-core 41/0, grails-plugin 39/0; `codenarcMain`, `rat` and `validateRepositoryConventions` clean.
`no ci` is back on while the 8.0.x merge build runs; a full run comes before merge, as you asked.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6282942291
This branch carries the hand port of the fix for #16665 (#16686 on 8.0.x) to the Groovy `CollectionKeyColumnUpdater`: the key of a unidirectional `hasMany` is non-updatable only when it shares a join table with another unidirectional collection of the owner, so an owner with several `hasMany` to different classes no longer loses `addTo*` on flush. `CollectionKeyColumnUpdaterSpec` and the new `MultipleUnidirectionalHasManySpec` are included. The Java file differs only in syntax, so the merge from 10.0.x may need the Groovy side kept.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6285180815
## ๐จ All tests passed but jobs failed ๐จ
### Failed Jobs without Test Failures
โ [CI / Validate githubBranch](https://github.com/apache/grails-core/actions/runs/39382151958/job/118493468634?pr=16572)
๐ท๏ธ Commit: 06d5ae44a472814f4fdc20b9ee34379c8432be05
โถ๏ธ Tests: 700 executed
๐ก Checks: 10/81 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
๐ฒ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub) via notifications" <no...@grails.apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6596590383
## ๐ No tests executed ๐
๐ท๏ธ Commit: 85ee606d6d4d2b68607bf713e089a65748d5eb42
โถ๏ธ Tests: 0 executed
โช๏ธ Checks: 27/27 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4238655280
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/HibernateOperations.groovy:
##########
@@ -80,9 +75,7 @@ public interface HibernateOperations {
* @see HibernateTransactionManager
* @see org.hibernate.Session
*/
- @Nullable
- <T> T execute(HibernateCallback<T> action) throws DataAccessException;
-
+ def <T> T execute(HibernateCallback<T> action) throws DataAccessException
Review Comment:
Fixed in ee3d2b0b58: `execute` and the `get` overloads are `@Nullable` again, along with about 130 other dropped jspecify annotations found by diffing `javap -public` of the 9.0.x build against this branch.
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/SessionFactoryUtils.groovy:
##########
@@ -80,18 +77,20 @@
* @see HibernateExceptionTranslator
* @see HibernateTransactionManager
*/
-public abstract class SessionFactoryUtils {
+@CompileStatic
+abstract class SessionFactoryUtils {
/**
* Order value for TransactionSynchronization objects that clean up Hibernate Sessions.
* Returns {@code DataSourceUtils.CONNECTION_SYNCHRONIZATION_ORDER - 100}
* to execute Session cleanup before JDBC Connection cleanup, if any.
* @see DataSourceUtils#CONNECTION_SYNCHRONIZATION_ORDER
*/
- public static final int SESSION_SYNCHRONIZATION_ORDER =
- DataSourceUtils.CONNECTION_SYNCHRONIZATION_ORDER - 100;
+ static final int SESSION_SYNCHRONIZATION_ORDER =
+ DataSourceUtils.CONNECTION_SYNCHRONIZATION_ORDER - 100
Review Comment:
Fixed in ee3d2b0b58: `SESSION_SYNCHRONIZATION_ORDER` is a `public static final` field again.
##########
grails-data-hibernate7/spring-orm/src/main/groovy/org/grails/orm/hibernate/support/hibernate7/support/OpenSessionInViewInterceptor.groovy:
##########
@@ -69,111 +67,109 @@
* @see TransactionSynchronizationManager
* @see SessionFactory#getCurrentSession()
*/
-public class OpenSessionInViewInterceptor implements AsyncWebRequestInterceptor {
+@CompileStatic
+class OpenSessionInViewInterceptor implements AsyncWebRequestInterceptor {
/**
* Suffix that gets appended to the {@code SessionFactory}
* {@code toString()} representation for the "participate in existing
* session handling" request attribute.
* @see #getParticipateAttributeName
*/
- public static final String PARTICIPATE_SUFFIX = ".PARTICIPATE";
+ static final String PARTICIPATE_SUFFIX = '.PARTICIPATE'
Review Comment:
Fixed in ee3d2b0b58: `PARTICIPATE_SUFFIX` is a `public static final` field again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6280981452
## ๐จ All tests passed but jobs failed ๐จ
### Failed Jobs without Test Failures
โ [CI / Validate githubBranch](https://github.com/apache/grails-core/actions/runs/39351083541/job/118395144382?pr=16572)
๐ท๏ธ Commit: d79bdc68b95f2c1a11018adc8e62b60e0586bd30
โถ๏ธ Tests: 6501 executed
๐ก Checks: 10/81 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
๐ฒ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6354421237
Done as the two steps, in this pull request: the branch is now the move commit and the convert commit, with the tree byte for byte what the previous head 194cd43cc2 had.
- 566d61db27 moves the 324 Java sources to their Groovy paths and changes nothing else: `git diff -M --name-status <base> 566d61db27` is 324 `R100` lines.
- 68ff6d8967 converts the moved files; it carries the content of the previous head, docs and build changes included.
`git log --follow` now reaches the Java history of every converted class, the thirteen you listed included (GrailsDomainBinder: 196 commits back to 2025-05). The combined diff of the two commits still reports those thirteen below the cutoff, as any single-commit view of a rewrite would; the history is in the move commit, as you described.
The branches built on this one (#16696 and the native-binding stack) have recorded the rebuilt root with a merge that changes no tree, so their diffs against their bases are unchanged. Everything still carries `no ci`.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6354421623
## ๐ No tests executed ๐
๐ท๏ธ Commit: 68ff6d8967eb4c99d8582b718da0557491954895
โถ๏ธ Tests: 0 executed
โช๏ธ Checks: 34/34 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6376296697
## โ
All tests passed โ
๐ท๏ธ Commit: e98d125349f2e828793b7e29ec16ffefc49a1c80
โถ๏ธ Tests: 135855 executed
โช๏ธ Checks: 141/141 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6461282589
@jdaugherty @matrei @jamesfredley
The branch history has been rebuilt directly on `10.0.x` into the strict two-commit structure:
1. **Commit 1 (`92c41267a0`)**: `git mv` only across all 326 `.java` files (including `CompositeUserTypeBinder` and `CompositeUserTypeColumns` from merged #16695). Every line is `R100` with 0 content modifications.
2. **Commit 2 (`a3b6fa56d6`)**: Pure conversion to statically-compiled Groovy addressing all review items:
- **Idiomatic Style**: Stripped trailing semicolons, plain double quotes, legacy `TODO(groovyism-cleanup)` comments, and `UnnecessarySemicolon` / PMD / SpotBugs suppressions.
- **Static Compilation**: `@CompileStatic` is present across all converted types, traits, enums, records, and `dbmigration-core` test/fixture classes.
- **Records**: Annotated with `@RecordOptions(getAt = false, toList = false, toMap = false, size = false)` and `@TupleConstructor(defaults = false, namedVariant = false)`. `SqlRestriction` defines an explicit canonical constructor and handles the CodeNarc component indentation false-positive with `@SuppressWarnings('Indentation')`.
- **Binary Compatibility & API**: Preserved Java-facing API contracts, `@PackageScope` on package-private helpers (`hibernateFlushModeToConstant`), direct field access (`currentEntry.@direction`), explicit receivers on shadowed parameter names (`this.sqlRestriction(...)` / `HibernateDetachedCriteriaExtension.sqlRestriction(...)`), and compile-time verification via `JavaCallerPin.java`.
- **Scope Discipline**: Purged unmerged logic from open PRs (#16686).
- **Agent Skill**: Added `.agents/skills/java-to-groovy-conversion/SKILL.md` (unreleased framework skill) and registered in `AGENTS.md` and `.agents/skills/README.md`.
- **Verification**: `codeStyle`, `codenarcMain`, `validateRepositoryConventions`, and `rat` pass cleanly without violations.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6249596530
## ๐จ All tests passed but jobs failed ๐จ
### Failed Jobs without Test Failures
โ [CI / Validate githubBranch](https://github.com/apache/grails-core/actions/runs/39124970656/job/117668046101?pr=16572)
๐ท๏ธ Commit: 4f2cdef51ebe1d5e060aafd8dd22722465c6f5d1
โถ๏ธ Tests: 65 executed
๐ก Checks: 6/81 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
๐ฒ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6282029040
Note for reviewers: #16576 and #16572 are the only PRs of this effort that run full CI on every push. The PRs stacked above them carry a temporary commit (`CHANGEME FOR ACCEPTANCE`) that disables their PR workflows so a push or rebase of the stack does not start dozens of matrices on shared runners. Each of those PRs states this and must have that commit reverted, with green CI, before it merges; they merge bottom-up.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6305432034
## ๐จ All tests passed but jobs failed ๐จ
### Failed Jobs without Test Failures
โ [CI / Neo4j Functional Tests \(Java 21, indy\=false\)](https://github.com/apache/grails-core/actions/runs/39520831012/job/118964454050?pr=16572)
๐ท๏ธ Commit: 4773d601ebb99e671100a8813454520d8ea4a6d4
โถ๏ธ Tests: 35467 executed
๐ก Checks: 48/82 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
๐ฒ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4363530317
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeUserTypeBinder.groovy:
##########
@@ -0,0 +1,265 @@
+/*
+ * 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 java.beans.PropertyDescriptor
+import java.lang.reflect.Field
+import java.lang.reflect.Method
+import java.lang.reflect.Modifier
+import java.lang.reflect.RecordComponent
+import java.lang.reflect.Type
+
+import groovy.transform.CompileStatic
+import org.hibernate.MappingException
+import org.hibernate.boot.spi.MetadataBuildingContext
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment
+import org.hibernate.mapping.BasicValue
+import org.hibernate.mapping.Column
+import org.hibernate.mapping.Component
+import org.hibernate.mapping.PersistentClass
+import org.hibernate.mapping.Property
+import org.hibernate.mapping.Table
+import org.hibernate.property.access.internal.PropertyAccessStrategyCompositeUserTypeImpl
+import org.hibernate.property.access.spi.PropertyAccessStrategy
+import org.hibernate.usertype.CompositeUserType
+import org.jspecify.annotations.NonNull
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+import org.springframework.beans.BeanUtils
+import org.springframework.util.ReflectionUtils
+
+import org.grails.datastore.mapping.model.config.GormProperties
+import org.grails.datastore.mapping.reflect.ClassPropertyFetcher
+import org.grails.orm.hibernate.cfg.ColumnConfig
+import org.grails.orm.hibernate.cfg.GrailsHibernateUtil
+import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy
+import org.grails.orm.hibernate.cfg.PropertyConfig
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty
+import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover
+
+/**
+ * Binds a property mapped with a Hibernate {@link CompositeUserType} as a {@link Component} of that type, the way
+ * Hibernate's own annotation binder binds {@code @CompositeType}: one property per attribute of the user type's
+ * {@linkplain CompositeUserType#embeddable() embeddable class}, in alphabetical attribute order, each a basic value
+ * typed by reflection on the embeddable class and accessed through Hibernate's composite-user-type property access.
+ * The columns of the property mapping are assigned to the attributes in that order; an attribute without a configured
+ * column is named {@code <property>_<attribute>}.
+ *
+ * @since 8.0.1
+ */
+@CompileStatic
+class CompositeUserTypeBinder {
Review Comment:
Done in 9f74af0796: `CompositeUserTypeBinder`, `CompositeUserTypeMappingSpec` and the `CompositeUserType` code in `GrailsDomainBinder`, `GrailsPropertyBinder` and `HibernatePersistentProperty` are out, with the matching changes to `GrailsPropertyBinderSpec`, `ListSecondPassBinderSpec`, `MapSecondPassBinderSpec`, `GrailsHibernatePersistentPropertySpec`, the two docs pages and the skill line. The affected files are the straightforward conversion of their `10.0.x` Java again; a grep for `CompositeUserType` finds only the sentence in `customHibernateTypes.adoc` that is on `10.0.x`. The classes get converted in the slice after #16695 reaches `10.0.x`.
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/MultipleUnidirectionalHasManySpec.groovy:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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
+
+/**
+ * An owner with several unidirectional hasMany associations keeps a join table for each of them, whether they point
+ * at different classes or at the same class with join tables of their own, and an element added to one of them is
+ * written to its own join table. A sibling to-many that is not bound as a collection, such as a serializable one,
+ * neither blocks the session factory nor counts as a sharer of the join table.
+ */
+class MultipleUnidirectionalHasManySpec extends HibernateGormDatastoreSpec {
Review Comment:
Done in 9f74af0796: `MultipleUnidirectionalHasManySpec` is deleted, `CollectionKeyColumnUpdaterSpec` is the `10.0.x` content with only the quote style changed, and `CollectionKeyColumnUpdater` is the conversion of its `10.0.x` Java again, counting every unidirectional `HibernateToManyProperty` of the owner.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/PersistentEntityNamingStrategy.groovy:
##########
@@ -16,26 +16,27 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.orm.hibernate.cfg;
+package org.grails.orm.hibernate.cfg
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity;
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty;
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty
/**
* Allows plugging into to custom naming strategies
*
* @author Graeme Rocher
* @since 5.0
*/
-public interface PersistentEntityNamingStrategy {
+interface PersistentEntityNamingStrategy {
Review Comment:
Done in 9f74af0796: `PersistentEntityNamingStrategy` and the 14 tests and fixtures you listed have `@CompileStatic`, as has `HibernateDatabasePojoTest`. The annotation type `SnowflakeId` has no code and needs none. They compile as they are, and the `dbmigration-core` suite passes (41 tests).
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateSession.groovy:
##########
@@ -76,8 +67,10 @@
* @author Graeme Rocher
* @since 1.0
*/
-@SuppressWarnings({"rawtypes", "PMD.DataflowAnomalyAnalysis", "PMD.AvoidDuplicateLiterals"})
-public class HibernateSession extends AbstractAttributeStoringSession implements QueryAliasAwareSession {
+// TODO(groovyism-cleanup): semicolons + double-quoted strings kept to preserve rename-similarity against the original .java (see matrei's review on #16576). Removed by the stacked groovyism-cleanup PR.
Review Comment:
Done in 9f74af0796, for the whole tree: the `TODO(groovyism-cleanup)` comments and the `UnnecessarySemicolon`, `UnnecessaryGString` and PMD suppressions are gone, CodeNarc found the 4,401 semicolons and 198 plain double quoted strings that the suppressions hid, and they are fixed in the 146 files that had them (a small lexer that leaves strings and comments alone, validated by CodeNarc itself: `codenarcMain` is clean on all five modules with no suppression). The 175 `public` modifiers of `HibernateConnectionMetadata` are gone too. `codenarcTest` is skipped by the build, so the tests got the same pass, checked by compiling and running the suites. The idioms beyond the first two points of the style guide stay for the groovyism pull request, which sits before the next slice.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/OrderByClauseBuilder.groovy:
##########
@@ -16,110 +16,120 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.orm.hibernate.cfg.domainbinding.util;
+package org.grails.orm.hibernate.cfg.domainbinding.util
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Locale;
-import java.util.stream.Collectors;
+import groovy.transform.CompileStatic
+import groovy.transform.PackageScope
+import org.hibernate.boot.model.internal.BinderHelper
+import org.hibernate.mapping.PersistentClass
+import org.hibernate.mapping.Property
+import org.hibernate.mapping.SingleTableSubclass
+import org.hibernate.mapping.Selectable
-import org.hibernate.boot.model.internal.BinderHelper;
-import org.hibernate.mapping.PersistentClass;
-import org.hibernate.mapping.Property;
-import org.hibernate.mapping.SingleTableSubclass;
-
-import org.grails.datastore.mapping.model.DatastoreConfigurationException;
+import org.grails.datastore.mapping.model.DatastoreConfigurationException
/** Utility class to build SQL order by clauses from HQL-style order by strings. */
-@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
-public class OrderByClauseBuilder {
+@CompileStatic
+@SuppressWarnings('PMD.DataflowAnomalyAnalysis')
+class OrderByClauseBuilder {
- public String buildOrderByClause(
+ String buildOrderByClause(
String hqlOrderBy, PersistentClass associatedClass, String role, String defaultOrder) {
if (hqlOrderBy == null) {
- return null;
+ return null
}
if (hqlOrderBy.isEmpty()) {
- return associatedClass.getIdentifier().getSelectables().stream()
- .map(selectable -> selectable.getText() + " asc")
- .collect(Collectors.joining(", "));
+ List<String> parts = []
+ for (Selectable selectable : associatedClass.identifier.selectables) {
+ parts.add(selectable.text + ' asc')
+ }
+ return parts.join(', ')
}
- List<SortEntry> entries = parseSortEntries(hqlOrderBy, role, defaultOrder);
+ List<SortEntry> entries = parseSortEntries(hqlOrderBy, role, defaultOrder)
- return entries.stream()
- .map(entry -> buildPropertyOrderBy(entry, associatedClass))
- .collect(Collectors.joining(", "));
+ List<String> parts = []
+ for (SortEntry entry : entries) {
+ parts.add(buildPropertyOrderBy(entry, associatedClass))
+ }
+ return parts.join(', ')
}
private List<SortEntry> parseSortEntries(String hqlOrderBy, String role, String defaultOrder) {
- String[] tokens = hqlOrderBy.split("[ ,]+");
- List<SortEntry> entries = new ArrayList<>();
- SortEntry currentEntry = null;
+ String[] tokens = hqlOrderBy.split('[ ,]+')
+ List<SortEntry> entries = new ArrayList<>()
+ SortEntry currentEntry = null
for (String token : tokens) {
- if (token.isEmpty()) continue;
+ if (token.isEmpty()) {
+ continue
+ }
if (isDirectionToken(token)) {
if (currentEntry == null || currentEntry.direction != null) {
throw new DatastoreConfigurationException(
- "Error while parsing sort clause: " + hqlOrderBy + " (" + role + ")");
+ "Error while parsing sort clause: ${hqlOrderBy} (${role})".toString())
}
- currentEntry.direction = token.toLowerCase(Locale.ROOT);
- } else {
+ currentEntry.direction = token.toLowerCase(Locale.ROOT)
Review Comment:
Done in 9f74af0796: `currentEntry.@direction = ...`, so the three writes are field writes. The `javap` sweep no longer lists `OrderByClauseBuilder`.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4363533745
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -1449,6 +1449,12 @@ class SportsCar extends Car {
}
----
+===== 26.11 Custom User Types
Review Comment:
Done in 9f74af0796: `upgrading80x.adoc` is back to its `10.0.x` content.
##########
grails-data-hibernate7/dbmigration-core/src/test/groovy/liquibase/ext/hibernate/HibernateIntegrationTest.groovy:
##########
@@ -1,81 +1,79 @@
-package liquibase.ext.hibernate;
+package liquibase.ext.hibernate
-import liquibase.Liquibase;
-import liquibase.Scope;
-import liquibase.database.Database;
-import liquibase.database.core.H2Database;
-import liquibase.database.jvm.JdbcConnection;
-import liquibase.diff.DiffResult;
-import liquibase.diff.Difference;
-import liquibase.diff.ObjectDifferences;
-import liquibase.diff.compare.CompareControl;
-import liquibase.diff.output.DiffOutputControl;
-import liquibase.diff.output.changelog.DiffToChangeLog;
-import liquibase.diff.output.report.DiffToReport;
-import liquibase.ext.hibernate.database.HibernateClassicDatabase;
-import liquibase.ext.hibernate.database.connection.HibernateConnection;
-import liquibase.resource.ClassLoaderResourceAccessor;
-import liquibase.resource.DirectoryResourceAccessor;
-import liquibase.structure.DatabaseObject;
-import liquibase.structure.core.*;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
+import java.io.*
Review Comment:
Done in 9f74af0796: explicit imports in both tests; the `java.io` ones are dropped, since Groovy imports them anyway.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6385194303
## ๐ No tests executed ๐
๐ท๏ธ Commit: 0cfca82994711bbde8446a2e3de5e6c4c508598e
โถ๏ธ Tests: 0 executed
โช๏ธ Checks: 37/37 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#discussion_r4340567127
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsDomainBinder.groovy:
##########
@@ -0,0 +1,311 @@
+/*
Review Comment:
This file is added, not renamed. At a 20% similarity threshold git still does not pair it with `GrailsDomainBinder.java`, and `git log --follow` on this path stops at the conversion commit.
Move the Java file to this path in its own commit, then convert it. The Java history has to be visible from this Groovy file before the rest of the series is reviewed.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: convert the grails-data-hibernate7 modules to Groovy [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16572:
URL: https://github.com/apache/grails-core/pull/16572#issuecomment-6343895615
## ๐ No tests executed ๐
๐ท๏ธ Commit: 194cd43cc276cc8df0e3cabb4bfecb949dfdb3b3
โถ๏ธ Tests: 0 executed
โช๏ธ Checks: 46/46 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org