Repository navigation
Bind a CompositeUserType property as a component under Hibernate 7 - #16695
Conversation
Hibernate 7 resolves the type name of a BasicValue through TypeDefinition, which accepts a BasicType or a UserType only, so a property mapped with `type: MonetaryAmountType` where that class implements CompositeUserType failed the SessionFactory build with "Named type [...] did not implement BasicType nor UserType". Hibernate binds a @CompositeType as a Component whose type name is the user type, with one property per attribute of the embeddable() class in alphabetical order, read through getPropertyValue(value, index) and rebuilt through instantiate(ValueAccess). GrailsPropertyBinder now routes a property whose type, or the user-type registered for its class, implements CompositeUserType to the new CompositeUserTypeBinder, which builds that Component: the columns of the property mapping are assigned to the attributes in alphabetical order and bound through ColumnBinder, an attribute without a column is named after the property and the attribute, and more columns than attributes is a MappingException. typeParams are logged as ignored, since Hibernate instantiates a composite user type itself. Documents composite user types in the GORM guide, the `type` mapping reference and the Grails 8 upgrade guide. Fixes apache#16688
borinquenkid
left a comment
There was a problem hiding this comment.
Reviewed at 68a5522 (base 8.0.x). I read the binder against Hibernate 7.4.10's EmbeddableBinder (fillEmbeddable, processCompositeUserType, compositeUserType()), PropertyAccessStrategyCompositeUserTypeImpl and PropertyAccessGetterImpl, and ran one throwaway probe spec in grails-data-hibernate7-core on H2 at this head, which I then deleted. CI is still queued.
The approach is the right one and matches what Hibernate does for @CompositeType: the component's type name is the user type, its class is embeddable(), the properties are sorted, and PropertyAccessStrategyCompositeUserTypeImpl gets the sorted names and types. One thing to fix before merge.
1. A value class with getters but no setters binds no attributes. attributesOf keeps a bean property only when it has a read and a write method, so an immutable value class, which is the natural shape for a monetary amount, yields no attributes at all. Probe at this head, a Groovy @Immutable class ProbeImmutableMoney implements Serializable { long amount; String currency } behind a type that is otherwise the spec's CompositeMoneyType:
org.hibernate.MappingException: Property [amount] of class [ProbeImmutablePayment] maps 2 columns, but the
embeddable class [ProbeImmutableMoney] of its composite user type [ProbeImmutableMoneyType] has 0 attributes: []
Hibernate never writes the embeddable: it is "never instantiated at runtime" (CompositeUserType javadoc), values come from getPropertyValue(value, index) and go back through instantiate(ValueAccess). Its own metadata read, processCompositeUserType, resolves each attribute's type through PropertyAccessStrategyGetterImpl, which takes a getter method or, failing that, the field (PropertyAccessGetterImpl.fieldGetter), and the javadoc's reference embeddable is a class with two private fields and no accessors at all. So the write method should not be required, and a field without an accessor should count too: an attribute is a non-static, non-transient field, or a readable bean property, less transients and the GORM-excluded names, in alphabetical order. A case in CompositeUserTypeMappingSpec with a getter-only value class would pin it (a Groovy @Immutable class, or a Java class with final fields and getters).
Verified as correct
getCompositeUserType()resolves the per-propertytypefirst and the owner'suser-typeregistration for the value class second, and only when the class implementsCompositeUserType;getUserType()keeps its behaviour through the sharedclassForName.- The dispatch in
GrailsPropertyBinder.bindPropertyruns the composite check beforeisUserButNotCollectionType(), so a composite type no longer reachesBasicValueand thedid not implement BasicType nor UserTypefailure in #16688; every other branch is untouched. - Column handling: the configured columns are assigned in alphabetical attribute order, which is the index order of
getPropertyValue; each goes throughColumnConfigToColumnBinderandColumnBinder, sosqlType,length,index,uniqueand the property's nullability apply; a missing column is named<path>_<property>_<attribute>through the naming strategy; more columns than attributes fails with the property, both counts and the attribute names. - Instantiation mirrors Hibernate's
compositeUserType(): the managed-bean registry when CDI extensions are allowed, a plain instance otherwise.typeParamsare logged as ignored, which is right, since@CompositeTypetakes none. - The spec covers the reporter's two-property mapping,
char(3)andNOT NULL, insert, read-back, update, clearing to null, HQL on the value and onamount.currency, criteria, dynamic finders, default column names,user-typeregistration, a recordembeddable()distinct from the value class, and the column-count error;GrailsPropertyBinderSpecandGrailsHibernatePersistentPropertySpeccover the binder and the resolution. - Docs: the guide's new section states the alphabetical index contract and the column order, the
typereference example follows it, and the upgrade guide names both theUserTypeAPI change and the portedCompositeUserTypecolumn order. The skill file lists the new binder.
Two follow-ups outside this PR, which I'll take. The 9.0.x native binding has no equivalent yet: GrailsDomainGenerator.decideType rejects a class that is neither a UserType nor a registered type, so this mapping fails there too; the counterpart is an @Embedded field with @CompositeType and @AttributeOverrides for the columns. And the Java-to-Groovy branch of the module (#16572) will need this binder ported by hand when it merges up, as the files it touches are renamed there.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16695 +/- ##
==================================================
+ Coverage 62.1164% 62.2352% +0.1188%
- Complexity 26776 26947 +171
==================================================
Files 2237 2242 +5
Lines 110433 110762 +329
Branches 20152 20222 +70
==================================================
+ Hits 68597 68933 +336
+ Misses 32617 32567 -50
- Partials 9219 9262 +43 🚀 New features to boost your workflow:
|
A value class with getters but no setters, such as a Groovy @immutable class, yielded no attributes, because attributesOf kept a bean property only when it had both a read and a write method. Hibernate never writes the embeddable class of a CompositeUserType: it reads a value through getPropertyValue(value, index) and rebuilds one through instantiate(ValueAccess), and its reference embeddable is a class with private fields and no accessors. An attribute is now a non-static, non-transient field or a readable bean property of the embeddable class, less the names in transients and the ones backed by a transient field, typed by the field when there is one, as Hibernate resolves it. ListSecondPassBinderSpec and MapSecondPassBinderSpec wire GrailsPropertyBinder by hand and now pass the CompositeUserTypeBinder, which failed 13 of their tests in CI.
…the Groovy conversion The three binder files the fix touches are Groovy here, so the changes are ported by hand into GrailsDomainBinder.groovy, GrailsPropertyBinder.groovy and HibernatePersistentProperty.groovy, and the new CompositeUserTypeBinder is added as Groovy. The specs and the docs merge as they are.
… since the last freeze The composite user type fixtures of #16695, the MultipleUnidirectionalHasManySpec entities of #16665, and the proxy, trait-initializer, CKCU and property-binder fixtures that arrived with earlier merges were not recorded, so ClassicOracleSpec failed on every native branch. Recorded with -Pgrails.test.refreezeClassicOracle=true; the two composite groups the classic binder cannot boot without a default mapping are recorded as unbootable.
The native binding now states what CompositeUserTypeBinder binds: a property mapped with a Hibernate CompositeUserType becomes an @Embedded field of the value class with @CompositeType and one @AttributeOverride per attribute of the user type's embeddable class, the attributes taken from its record components or its fields and readable properties in alphabetical order, the configured columns assigned by position and an attribute without one named <property>_<attribute>. Each column carries the property's nullability, uniqueness and constraints and its own column settings, so both bindings create the same schema; more columns than attributes is refused as the binder refuses it. A composite user type inside an embedded type is reported as unsupported. The differential harness compares the attribute columns of such a property with the recorded component, and the oracle records the two fixtures #16695 added. GeneratedDomainClassesCompositeUserTypeSpec boots the fixtures of CompositeUserTypeMappingSpec in both modes and compares the schemas, round-trips a value and queries by attribute.
jamesfredley
left a comment
There was a problem hiding this comment.
Not approving. Binding a CompositeUserType as a component is the right shape, and the basic multi-column path is sound. Three production problems remain.
The component is registered as the embeddable template class rather than the value class, so whole-value queries describe the template. An explicit mapping type such as serializable loses to a globally registered composite type, because the alias does not resolve as a class and the fallback then picks the registration. And unique: true is applied once per attribute column, so a money value rejects (20, EUR) after (10, EUR).
The integration spec does exercise public GORM save/load. It does not catch these three. A smaller compatibility note: GrailsPropertyBinder's previous constructor is removed rather than overloaded, which is source-incompatible for any external caller. That one is not the reason for holding the approval.
…site user types once A composite user type property was bound with its embeddable class as the component class, so ComponentType.getReturnedClass() and the JPA attribute described a template such as a record instead of the value class, and whole-value HQL, criteria and finders saw the template. The component class is now returnedClass(), as Hibernate's own binding of a composite type keeps the attribute's value class and uses embeddable() only for the attributes. A user-type registered for the value class applied whenever getUserType() returned null, which includes a type alias such as 'serializable', so an explicit per-property type lost to the registration and the column became a component. The registration now applies only when the property mapping has no type. A unique property marked every attribute column unique, and a unique group created one key per attribute column, so (20, EUR) was rejected after (10, EUR). ColumnBinder.bindColumnWithoutUniqueness binds the attribute columns without uniqueness, and CreateKeyForProps.createKeyForColumns creates one key across all of them and the columns of the group. The previous GrailsPropertyBinder constructor is restored, deprecated: it binds everything but a composite user type, which it rejects with a MappingException naming the missing CompositeUserTypeBinder.
|
The previous |
jamesfredley
left a comment
There was a problem hiding this comment.
The three earlier comments are fixed. The component class is the value class, an explicit type wins over a global composite registration, and unique: true is one key across the attribute columns rather than one unique flag per column. Not approving yet because the new unique-key path drops quoting and cannot group another composite property.
…perty to all of its columns `CreateKeyForProps.createKeyForColumns` now takes the bound `Column` objects, so the unique key of a composite user type property keeps the quoting and the case of its columns instead of holding copies named after `getName()`, which dropped the quoting of a mixed-case or reserved-word column. `addGroupColumns` resolves every column of a group property through the new `ColumnNameForPropertyAndPathFetcher.getColumnNamesForPropertyAndPath`, so a uniqueness group that names a composite user type property, from another composite property or from a plain one, spans all of its columns rather than one nonexistent default name or a multi-column mapping error. The attribute discovery and the column naming of `CompositeUserTypeBinder` move to `CompositeUserTypeColumns`, which the binder and the fetcher share.
jamesfredley
left a comment
There was a problem hiding this comment.
The two earlier comments are fixed. Quoted column names are kept on the unique key, and a composite property in a uniqueness group contributes every column. Those threads were already resolved. Not approving yet because three other composite mappings still fail at startup.
…e user type once and bind an enum mapped with one as a component A column of a composite user type property is one of several, so ColumnBinder.bindColumnWithoutUniqueness now takes the type of its attribute and defines the column from its own configuration: a numeric attribute gets the scale and precision of its column entry, or the default precision of the dialect, and the constraints of the property, which describe the whole value, are no longer read through PropertyConfig's single-column accessors (a CompositeUserType<BigDecimal> with two columns threw "Cannot treat multi-column property as a single-column property" unless every column set a scale). NumericColumnConstraintsBinder accepts a null property config for that case. The user type is now instantiated in one place, CompositeUserTypeColumns.userTypeOf, which is CDI-aware when it has the MetadataBuildingContext, and the context is passed to it from GrailsDomainBinder, SimpleValueBinder and CompositeUserTypeBinder through new ColumnNameForPropertyAndPathFetcher and ColumnBinder constructors. A uniqueness group that names a composite property built by a bean container no longer requires a default constructor. GrailsPropertyBinder checks for a composite user type before the enum branch, so an enum property whose type names one is bound as a component rather than handed to EnumTypeBinder, which built a BasicValue with the composite type name. Regressions in CompositeUserTypeMappingSpec: a BigDecimal split into scale and unscaled value with configured and default columns, an enum persisted as its code and rank, a user type without a default constructor resolved through a hibernate.resource.beans.container for its property and for the group that names it, and the startup error without a container. Unit coverage in ColumnBinderSpec, NumericColumnConstraintsBinderSpec, CompositeUserTypeColumnsSpec, ColumnNameForPropertyAndPathFetcherSpec and GrailsPropertyBinderSpec. The guide states the per-column constraints, the enum case and the bean container.
…ta-hibernate7 conversion
✅ All tests passed ✅🏷️ Commit: fd71dca Learn more about TestLens at testlens.app/docs. |
jamesfredley
left a comment
There was a problem hiding this comment.
Approving. Each composite user type column is bound from its attribute, so a two-column decimal mapping no longer calls getScale() on the composite. A CDI user type is resolved once, including in a uniqueness group. An enum mapped with a composite user type is bound as a component. The three earlier comments are resolved.
10.0.x now carries #16695 (Hibernate 7 composite user types). Its changes to the converted classes are ported by hand into their Groovy twins: ColumnBinder (the column definition split from its uniqueness, and bindColumnWithoutUniqueness), CreateKeyForProps (createKeyForColumns and group columns of composite types), ColumnNameForPropertyAndPathFetcher (the MetadataBuildingContext constructors and getColumnNamesForPropertyAndPath), GrailsPropertyBinder (the composite user type branch and the deprecated 7-argument constructor), HibernatePersistentProperty (getCompositeUserType), GrailsDomainBinder, SimpleValueBinder and NumericColumnConstraintsBinder. Its two new classes, CompositeUserTypeBinder and CompositeUserTypeColumns, arrive as Java here and are moved and converted in the next two commits. Claude-Session: https://claude.ai/code/session_01MDHn7fC3hsGJ27vFHD2o1p
The two classes #16695 added, now statically compiled Groovy like the rest of the module: @CompileStatic, the nested Attribute record with the record options and tuple constructor of the series, lambdas rather than coerced closures, Stream.toList() kept (javap shows the JDK method), the PMD suppression dropped and every comment and Javadoc tag kept. Also removes a doubled blank line from the ported fetcher. Claude-Session: https://claude.ai/code/session_01MDHn7fC3hsGJ27vFHD2o1p
Fixes #16688
Problem
A domain property mapped with a Hibernate
CompositeUserType, as the GORM guide documents for multi-column types,fails the
HibernateDatastorebootstrap undergrails-data-hibernate7withThe binder puts every
type:on aBasicValue, and Hibernate 7 resolves the type name of aBasicValuethroughTypeDefinition, which accepts aBasicTypeor aUserTypeonly. A composite user type is not a basic value in Hibernate 6 and 7:@CompositeTypeis bound as aComponentwhose type name is the user type, with one property per attribute of theembeddable()class, in alphabetical attribute order, read throughgetPropertyValue(value, index)and rebuilt throughinstantiate(ValueAccess).Fix
HibernatePersistentProperty.getCompositeUserType()resolves the composite user type of a property from its owntype:or from theuser-typeregistered for its class.GrailsPropertyBinderroutes such a property to the newCompositeUserTypeBinder, which builds the component the way Hibernate's ownEmbeddableBinderdoes: a property per attribute of the embeddable class in alphabetical order, each aBasicValuetyped by reflection on the embeddable class and accessed through Hibernate's composite-user-type property access strategy. The attributes are the components of a record, or otherwise the fields and the readable bean properties of the class, less the static, transient and synthetic ones, the names intransientsand the properties backed by a transient field. No setter, and no accessor at all, is required: Hibernate never writes the embeddable class, so an immutable value class or a class with private fields only (the shape in theCompositeUserTypejavadoc) works.ColumnBinder, sosqlType,length,index,uniqueand the property'snullableconstraint apply. An attribute without a configured column is named<property>_<attribute>. More columns than attributes is aMappingExceptionnaming the property, the counts and the attributes.typeParamsare logged as ignored, because Hibernate instantiates a composite user type itself.Tests
CompositeUserTypeMappingSpec(new) covers the reporter's mapping with two properties of the same type, the metadata and schema (char(3),NOT NULLon the required property, nullable columns on the optional one), insert, read-back, update and clearing to null, HQL on the whole value and on an attribute path, criteriaeq,findAllByandcountBy, default column names, a type registered withuser-type, anembeddable()record distinct from the value class, a Groovy@Immutablevalue class with getters only whose computed getter is listed intransients, anembeddable()class with private fields and no accessors, and the column-count error.GrailsPropertyBinderSpecgets a binder-level case andGrailsHibernatePersistentPropertySpeccoversgetCompositeUserType().ListSecondPassBinderSpecandMapSecondPassBinderSpec, which wireGrailsPropertyBinderby hand, pass the new binder. All eight features of the new spec fail on8.0.xwith the reported exception before the fix.Run locally on JDK 21: the full
:grails-data-hibernate7-core:testsuite,:grails-data-hibernate7-core:codeStyleandvalidateRepositoryConventions.Docs
user-typeregistration,typeParams).typemapping reference: the multi-column example follows the attribute order and usessqlType: "char(3)"(an explicitsqlTypeis emitted verbatim, so a separatelengthhas no effect).UserTypeAPI change and on the column order of a portedCompositeUserType.Not verified against the reporter's example application end to end (it would need a local snapshot publish); its
grails-data-hibernate7branch also calls a non-existentCurrency.of(String)ininstantiate, which fails independently of this bug.