borinquenkid commented on code in PR #15568:
URL: https://github.com/apache/grails-core/pull/15568#discussion_r3440098658


##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/HibernateGormEnhancerSpec.groovy:
##########
@@ -0,0 +1,60 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.grails.orm.hibernate
+
+import grails.gorm.annotation.Entity
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.gorm.GormEnhancer

Review Comment:
   Fixed — the unused `import org.grails.datastore.gorm.GormEnhancer` was 
removed. The spec references `HibernateGormEnhancer` directly throughout.



##########
grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/HibernateGormDatastoreSpec.groovy:
##########
@@ -0,0 +1,158 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package grails.gorm.tests
+
+import org.apache.grails.data.hibernate5.core.GrailsDataHibernate5TckManager
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.grails.datastore.mapping.model.PersistentEntity
+import org.grails.orm.hibernate.AbstractHibernateSession
+import org.grails.orm.hibernate.HibernateDatastore
+import org.grails.orm.hibernate.cfg.GrailsDomainBinder
+import org.grails.orm.hibernate.cfg.HibernateMappingContext
+import org.grails.orm.hibernate.cfg.HibernatePersistentEntity
+import org.grails.orm.hibernate.query.HibernateQuery
+
+import org.hibernate.boot.MetadataSources
+import org.hibernate.boot.internal.BootstrapContextImpl
+import org.hibernate.boot.internal.InFlightMetadataCollectorImpl
+import org.hibernate.boot.internal.MetadataBuilderImpl
+import org.hibernate.boot.registry.BootstrapServiceRegistry
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder
+import org.hibernate.boot.registry.classloading.spi.ClassLoaderService
+import org.hibernate.dialect.H2Dialect
+import org.hibernate.internal.SessionFactoryImpl
+import org.hibernate.service.spi.ServiceRegistryImplementor
+import org.hibernate.boot.spi.MetadataContributor
+
+/**
+ * The original GormDataStoreSpec destroyed the setup
+ * between tests instead of at the end of all tests
+ * It also was default configured for H2 which
+ * made it break with some Java types.
+ * Finally, it loaded all the test Entities,
+ * now it can be setup individually.
+ */
+class HibernateGormDatastoreSpec extends 
GrailsDataTckSpec<GrailsDataHibernate5TckManager> {

Review Comment:
   Updated — the javadoc now describes the class purpose (base H7 Spock spec 
over a shared in-memory H2 datastore) rather than comparing with the obsolete 
GormDataStoreSpec.



##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/query/Query.java:
##########
@@ -589,6 +616,32 @@ public Object singleResult() {
         return results.isEmpty() ? null : results.get(0);
     }
 
+    /**
+     * Counts the rows this query would return, respecting any existing 
projections or grouping.
+     * Subclasses may override to provide an optimized implementation (e.g., 
derived-table count).
+     * The default implementation falls back to loading all rows when 
user-defined projections
+     * exist, since appending a count projection would produce incorrect 
results.
+     *
+     * @return The row count
+     */
+    public Number countResults() {
+        if (!projections.getProjectionList().isEmpty()) {
+            // When user-defined projections exist (e.g. groupProperty + 
count),
+            // a simple count() projection returns incorrect results because it
+            // appends to the existing projections rather than replacing them.
+            // Fall back to counting the grouped result rows.
+            // TODO: This needs resolved properly in Grails 8 with Hibernate 
7's

Review Comment:
   Not a bad merge — these were intentional shared-core changes for H7 
compatibility. `Integer max/offset` (replacing `int -1/0`) enables proper 
null-checking before applying limits (H5 and H7 both check `max != null`). The 
`implements Serializable` addition supports `DetachedCriteria` serialization 
needed by H7's session-scoped query caching. Both changes are safe for H5.



##########
grails-datamapping-core-test/src/test/groovy/grails/gorm/specs/DeepValidateWithSaveSpec.groovy:
##########
@@ -26,6 +26,7 @@ import org.grails.datastore.gorm.validation.CascadingValidator
 
 class DeepValidateWithSaveSpec extends 
GrailsDataTckSpec<GrailsDataCoreTckManager> {
 
+    @spock.lang.Requires({ System.getProperty('hibernate5.gorm.suite') == 
'true' || System.getProperty('hibernate7.gorm.suite') == 'true' || 
System.getProperty('mongodb.gorm.suite') == 'true' })

Review Comment:
   Agreed — TCK tests without annotations are expected to pass across all GORM 
implementations. Module-specific behaviour is annotated with 
`@PendingFeatureIf` (Hibernate-specific) or `@Requires` (datastore capability 
check).



-- 
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