matrei commented on code in PR #15568:
URL: https://github.com/apache/grails-core/pull/15568#discussion_r3460654355
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/datastore/mapping/model/PersistentPropertySpec.groovy:
##########
@@ -21,9 +21,6 @@ package org.grails.datastore.mapping.model
import grails.gorm.tests.HibernateGormDatastoreSpec
import grails.persistence.Entity
-import spock.lang.Issue
-
-@Issue('https://github.com/apache/grails-data-mapping/issues/1299')
Review Comment:
Leave a blank line between imports and class declaration?
##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/SubclassMultipleListCollectionSpec.groovy:
##########
@@ -27,7 +27,7 @@ import spock.lang.*
/**
* Created by graemerocher on 01/03/2017.
*/
-@Ignore
+@Ignore // https://github.com/apache/grails-core/issues/14624 —
MappingException: Repeated column (iteration_id) for Product; not fixed in this
PR
Review Comment:
- Use the `@Issue` annotation instead of a comment.
- "not fixed in this PR" can be removed.
##########
grails-data-hibernate7/grails-plugin/src/main/groovy/grails/test/hibernate/HibernateSpec.groovy:
##########
@@ -58,14 +58,15 @@ import org.hibernate.boot.registry.BootstrapServiceRegistry
import org.hibernate.boot.registry.StandardServiceRegistryBuilder
import org.hibernate.dialect.H2Dialect
import org.grails.orm.hibernate.proxy.GrailsBytecodeProvider
-import org.hibernate.proxy.pojo.bytebuddy.ByteBuddyProxyHelper
import org.hibernate.internal.SessionFactoryImpl
import org.hibernate.service.spi.ServiceRegistryImplementor
import org.springframework.context.ApplicationContext
/**
* Specification for Hibernate tests
*
+ * TODO: rename to AHibernateSpec to follow the abstract class naming
convention
+ *
Review Comment:
I don't think it is a good idea to rename it to `AHibernateSpec`.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/RootBinder.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.util.stream.Stream;
+
+import jakarta.annotation.Nonnull;
+
+import org.hibernate.boot.spi.InFlightMetadataCollector;
+import org.hibernate.mapping.RootClass;
+import org.hibernate.mapping.Subclass;
+import org.jspecify.annotations.NonNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.grails.orm.hibernate.cfg.MappingCacheHolder;
+import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentEntity;
+import org.grails.orm.hibernate.cfg.domainbinding.util.MultiTenantFilterBinder;
+
+/** Binder for root classes. */
+@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
+public class RootBinder {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(RootBinder.class);
+
+ private final String dataSourceName;
+ private final MultiTenantFilterBinder multiTenantFilterBinder;
+ private final SubClassBinder subClassBinder;
+ private final RootPersistentClassCommonValuesBinder
rootPersistentClassCommonValuesBinder;
+ private final DiscriminatorPropertyBinder discriminatorPropertyBinder;
+ private final InFlightMetadataCollector mappings;
+ private final MappingCacheHolder mappingCacheHolder;
+
+ public RootBinder(
+ String dataSourceName,
+ MultiTenantFilterBinder multiTenantFilterBinder,
+ SubClassBinder subClassBinder,
+ RootPersistentClassCommonValuesBinder
rootPersistentClassCommonValuesBinder,
+ DiscriminatorPropertyBinder discriminatorPropertyBinder,
+ InFlightMetadataCollector mappings,
+ MappingCacheHolder mappingCacheHolder) {
+ this.dataSourceName = dataSourceName;
+ this.multiTenantFilterBinder = multiTenantFilterBinder;
+ this.subClassBinder = subClassBinder;
+ this.rootPersistentClassCommonValuesBinder =
rootPersistentClassCommonValuesBinder;
+ this.discriminatorPropertyBinder = discriminatorPropertyBinder;
+ this.mappings = mappings;
+ this.mappingCacheHolder = mappingCacheHolder;
+ }
+
+ /**
+ * Binds a root class (one with no super classes) to the runtime meta
model based on the supplied
+ * Grails domain class
+ *
+ * @param entity The Grails domain class
+ */
+ public void bindRoot(@Nonnull HibernatePersistentEntity entity) {
Review Comment:
Should align on JSpecify annotations (this one is Jakarta)?
##########
grails-data-hibernate7/core/src/main/groovy/grails/gorm/hibernate/HibernateEntity.groovy:
##########
Review Comment:
I do disagree. By deprecating the current methods (`findWithSql`,
`findAllWithSql`), and delegating to new methods (`findWithNativeSql`,
`findAllWithNativeSql`), you are effectively changing the public API of GORM
without discussion and consensus.
Also, please don't mark my review comments as resolved as it makes it very
hard to find them.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/proxy/GrailsBytecodeProvider.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.grails.orm.hibernate.proxy;
+
+import java.util.Map;
+
+import org.hibernate.bytecode.enhance.spi.EnhancementContext;
+import org.hibernate.bytecode.enhance.spi.Enhancer;
+import org.hibernate.bytecode.spi.BytecodeProvider;
+import org.hibernate.bytecode.spi.ProxyFactoryFactory;
+import org.hibernate.bytecode.spi.ReflectionOptimizer;
+import org.hibernate.property.access.spi.PropertyAccess;
+import org.hibernate.proxy.pojo.bytebuddy.ByteBuddyProxyHelper;
+
+/**
+ * A {@link BytecodeProvider} implementation for Hibernate 7 that provides
Groovy-aware proxies.
+ *
+ * @author Walter Duque de Estrada
Review Comment:
We have consensus on the Grails team not adding `@author` tags
(https://github.com/apache/grails-core/issues/15299)
--
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]