borinquenkid commented on code in PR #15568: URL: https://github.com/apache/grails-core/pull/15568#discussion_r3440349715
########## grails-data-hibernate5/core/src/test/groovy/org/grails/datastore/mapping/model/PersistentPropertySpec.groovy: ########## @@ -0,0 +1,92 @@ +/* + * 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.datastore.mapping.model + +import grails.gorm.tests.HibernateGormDatastoreSpec +import grails.persistence.Entity +import spock.lang.Issue + +@Issue('https://github.com/grails/grails-data-mapping/issues/1299') Review Comment: Yes — H5 has its own `grails.gorm.tests.HibernateGormDatastoreSpec` (backed by `GrailsDataHibernate5TckManager`) that lives in the H5 test sources alongside the spec. The import resolves to the H5 version when compiled within the H5 module. The `createPersistentEntity()` helper is provided by the H5 base class, not the H7 one. ########## grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/support/SoftKeySpec.groovy: ########## @@ -0,0 +1,138 @@ +/* + * 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.support + +import spock.lang.Specification + +class SoftKeySpec extends Specification { + + static class TestSoftKey<T> extends SoftKey<T> { + boolean forceNull = false + TestSoftKey(T referent) { + super(referent) + } + @Override + T get() { + return forceNull ? null : super.get() + } + } + + def "constructor stores referent and computes hashCode from it"() { + given: + def key = "hello" + + when: + def sk = new SoftKey<>(key) + + then: + sk.get() == key + sk.hashCode() == key.hashCode() + } + + def "hashCode is stable even after gc (uses stored hash)"() { + given: + def sk = new SoftKey<>("world") + + expect: + sk.hashCode() == "world".hashCode() + } + + def "equals returns true for same instance"() { + given: + def sk = new SoftKey<>("a") + + expect: + sk.equals(sk) + } + + def "equals returns false for null"() { + given: + def sk = new SoftKey<>("a") + + expect: + !sk.equals(null) + } + + def "equals returns false for different class"() { + given: + def sk = new SoftKey<>("a") + + expect: + !sk.equals("a") + } + + def "two SoftKeys with equal referents are equal"() { + given: + def sk1 = new SoftKey<>("same") + def sk2 = new SoftKey<>("same") + + expect: + sk1 == sk2 + sk1.hashCode() == sk2.hashCode() + } + + def "two SoftKeys with different referents are not equal"() { + given: + def sk1 = new SoftKey<>("foo") + def sk2 = new SoftKey<>("bar") + + expect: + sk1 != sk2 + } + + def "two SoftKeys with different hashes are not equal"() { + given: + // ensure different hash codes (different objects) + def sk1 = new SoftKey<>(new Integer(1)) + def sk2 = new SoftKey<>(new Integer(99999)) Review Comment: Yes — `new Integer(value)` is deprecated since JDK 9. The spec already uses `Integer.valueOf()` in the current revision (fixed earlier in this PR). ########## grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/IdentityEnumTypeSpec.groovy: ########## @@ -20,52 +20,209 @@ package grails.gorm.tests import grails.gorm.annotation.Entity import grails.gorm.transactions.Rollback -import org.grails.orm.hibernate.HibernateDatastore -import org.springframework.transaction.PlatformTransactionManager -import spock.lang.AutoCleanup -import spock.lang.Shared -import spock.lang.Specification +import jakarta.persistence.Enumerated +import jakarta.persistence.EnumType +import org.grails.orm.hibernate.cfg.IdentityEnumType +import org.hibernate.HibernateException +import org.hibernate.MappingException +import org.hibernate.engine.spi.SharedSessionContractImplementor import javax.sql.DataSource import java.sql.ResultSet /** - * Created by graemerocher on 16/11/16. + * Tests for IdentityEnumType in Hibernate 5. */ -class IdentityEnumTypeSpec extends Specification { +class IdentityEnumTypeSpec extends HibernateGormDatastoreSpec { - @Shared @AutoCleanup HibernateDatastore hibernateDatastore = new HibernateDatastore(EnumEntityDomain, FooWithEnum) - @Shared PlatformTransactionManager transactionManager = hibernateDatastore.getTransactionManager() + def setupSpec() { + manager.registerDomainClasses(EnumEntityDomain, FooWithEnum) + } @Rollback void "test identity enum type"() { when: - new EnumEntityDomain(status: EnumEntityDomain.Status.FOO).save(flush:true) - DataSource ds = hibernateDatastore.connectionSources.defaultConnectionSource.dataSource + new EnumEntityDomain(status: EnumEntityDomain.Status.FOO).save(flush: true) + DataSource ds = manager.hibernateDatastore.connectionSources.defaultConnectionSource.dataSource ResultSet resultSet = ds.getConnection().prepareStatement('select status from enum_entity_domain').executeQuery() then: resultSet.next() - resultSet.getString(1) == 'F' + resultSet.getString(1) == 'F' // FOO id is 'F' EnumEntityDomain.first().status == EnumEntityDomain.Status.FOO } @Rollback void "test identity enum type 2"() { when: - new FooWithEnum(name: "blah", mySuperValue: XEnum.X__TWO).save(flush:true) - DataSource ds = hibernateDatastore.connectionSources.defaultConnectionSource.dataSource + new FooWithEnum(name: "blah", mySuperValue: XEnum.X__TWO).save(flush: true) + DataSource ds = manager.hibernateDatastore.connectionSources.defaultConnectionSource.dataSource ResultSet resultSet = ds.getConnection().prepareStatement('select my_super_value from foo_with_enum').executeQuery() then: resultSet.next() - resultSet.getInt(1) == 100 + resultSet.getInt(1) == 100 // X__TWO id is 100 FooWithEnum.first().mySuperValue == XEnum.X__TWO } + + def "setParameterValues initializes enumClass"() { + given: + def type = new IdentityEnumType() + def props = new Properties() + props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, IdentityStatusEnum.name) + + when: + type.setParameterValues(props) + + then: + type.returnedClass() == IdentityStatusEnum + type.sqlTypes()[0] != 0 + } + + def "setParameterValues throws MappingException for enum without getId method"() { + given: + def type = new IdentityEnumType() + def props = new Properties() + props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, PlainEnum.name) + + when: + type.setParameterValues(props) + + then: + thrown(HibernateException) // Throw by BidiEnumMap constructor + } + + def "equals uses identity comparison"() { + given: + def type = new IdentityEnumType() + + expect: + type.equals(IdentityStatusEnum.ACTIVE, IdentityStatusEnum.ACTIVE) + !type.equals(IdentityStatusEnum.ACTIVE, IdentityStatusEnum.INACTIVE) + !type.equals(null, IdentityStatusEnum.ACTIVE) + } + + def "hashCode delegates to the object"() { + given: + def type = new IdentityEnumType() + def val = IdentityStatusEnum.ACTIVE + + expect: + type.hashCode(val) == val.hashCode() + } + + def "deepCopy returns the same object reference"() { + given: + def type = new IdentityEnumType() + def val = IdentityStatusEnum.ACTIVE + + expect: + type.deepCopy(val).is(val) + } + + def "isMutable returns false"() { + expect: + !new IdentityEnumType().isMutable() + } + + def "disassemble returns the value as Serializable"() { + given: + def type = new IdentityEnumType() + def val = IdentityStatusEnum.ACTIVE + + expect: + type.disassemble(val).is(val) + } + + def "assemble returns the cached value unchanged"() { + given: + def type = new IdentityEnumType() + def val = IdentityStatusEnum.ACTIVE + + expect: + type.assemble(val, null).is(val) + } + + def "replace returns the original value"() { + given: + def type = new IdentityEnumType() + + expect: + type.replace(IdentityStatusEnum.ACTIVE, IdentityStatusEnum.INACTIVE, null).is(IdentityStatusEnum.ACTIVE) + } + + def "nullSafeGet returns null for null value"() { + given: + def type = new IdentityEnumType() + def props = new Properties() + props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, IdentityStatusEnum.name) + type.setParameterValues(props) + def rs = Mock(java.sql.ResultSet) + def session = manager.sessionFactory.currentSession as SharedSessionContractImplementor + + when: + def res = type.nullSafeGet(rs, ['status'] as String[], session, null) + + then: + 1 * rs.getString('status') >> null + 1 * rs.wasNull() >> true + res == null + } + + def "nullSafeGet converts id to enum"() { + given: + def type = new IdentityEnumType() + def props = new Properties() + props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, IdentityStatusEnum.name) + type.setParameterValues(props) + def rs = Mock(java.sql.ResultSet) + def session = manager.sessionFactory.currentSession as SharedSessionContractImplementor + + when: + def res = type.nullSafeGet(rs, ['status'] as String[], session, null) + + then: + 1 * rs.getString('status') >> "A" + 2 * rs.wasNull() >> false + res == IdentityStatusEnum.ACTIVE + } + + def "nullSafeSet handles null value"() { + given: + def type = new IdentityEnumType() + def props = new Properties() + props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, IdentityStatusEnum.name) + type.setParameterValues(props) + def st = Mock(java.sql.PreparedStatement) + def session = manager.sessionFactory.currentSession as SharedSessionContractImplementor + + when: + type.nullSafeSet(st, null, 1, session) + + then: + 1 * st.setNull(1, _) + } + + def "nullSafeSet converts enum to id"() { + given: + def type = new IdentityEnumType() + def props = new Properties() + props.setProperty(IdentityEnumType.PARAM_ENUM_CLASS, IdentityStatusEnum.name) + type.setParameterValues(props) + def st = Mock(java.sql.PreparedStatement) + def session = manager.sessionFactory.currentSession as SharedSessionContractImplementor + + when: + type.nullSafeSet(st, IdentityStatusEnum.INACTIVE, 1, session) + + then: + 1 * st.setString(1, "I") + } } @Entity class EnumEntityDomain { + @Enumerated(EnumType.STRING) Review Comment: Hibernate 7's stricter type resolution requires an explicit `@Enumerated(EnumType.STRING)` annotation on fields that use a custom `UserType` like `IdentityEnumType`. Without it, Hibernate 7 falls back to ordinal mapping and bypasses the custom type. The annotation was added to ensure `IdentityEnumType` is correctly applied. ########## grails-data-hibernate5/core/src/test/groovy/grails/gorm/tests/IdentityEnumTypeSpec.groovy: ########## @@ -83,18 +240,19 @@ class EnumEntityDomain { class FooWithEnum { long id String name + @Enumerated(EnumType.STRING) Review Comment: Same reason as the previous comment — the explicit `@Enumerated(EnumType.STRING)` annotation is required on both fields that use `IdentityEnumType` to ensure Hibernate's stricter type resolution honours the custom `UserType` implementation. -- 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]
