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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
