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]

Reply via email to