jdaugherty commented on code in PR #16695: URL: https://github.com/apache/grails-core/pull/16695#discussion_r4340010980
########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeUserTypeBinder.java: ########## @@ -0,0 +1,295 @@ +/* + * 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.beans.PropertyDescriptor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; +import java.util.TreeMap; + +import org.hibernate.MappingException; +import org.hibernate.boot.spi.MetadataBuildingContext; +import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment; +import org.hibernate.mapping.BasicValue; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.Component; +import org.hibernate.mapping.PersistentClass; +import org.hibernate.mapping.Property; +import org.hibernate.mapping.Table; +import org.hibernate.property.access.internal.PropertyAccessStrategyCompositeUserTypeImpl; +import org.hibernate.property.access.spi.PropertyAccessStrategy; +import org.hibernate.usertype.CompositeUserType; +import org.jspecify.annotations.NonNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.springframework.beans.BeanUtils; +import org.springframework.util.ReflectionUtils; + +import org.grails.datastore.mapping.model.config.GormProperties; +import org.grails.datastore.mapping.reflect.ClassPropertyFetcher; +import org.grails.orm.hibernate.cfg.ColumnConfig; +import org.grails.orm.hibernate.cfg.GrailsHibernateUtil; +import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy; +import org.grails.orm.hibernate.cfg.PropertyConfig; +import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty; +import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover; + +/** + * Binds a property mapped with a Hibernate {@link CompositeUserType} as a {@link Component}. + * <p> + * Hibernate resolves the type of a {@link BasicValue} as a {@link org.hibernate.type.BasicType} or a + * {@link org.hibernate.usertype.UserType} only. A composite user type spans one column per attribute of its + * {@linkplain CompositeUserType#embeddable() embeddable class}, so it is a component whose type name is the + * user type, as Hibernate binds a {@link org.hibernate.annotations.CompositeType}: one property per attribute, + * in alphabetical attribute order, read through {@link CompositeUserType#getPropertyValue(Object, int)} with the + * index of the attribute in that order and assembled through + * {@link CompositeUserType#instantiate(org.hibernate.metamodel.spi.ValueAccess)}. The columns of the property + * mapping are assigned to the attributes in the same order; an attribute without a configured column is named + * after the property and the attribute. + * + * @since 8.0.1 + */ +@SuppressWarnings("PMD.DataflowAnomalyAnalysis") +public class CompositeUserTypeBinder { + + private static final Logger LOG = LoggerFactory.getLogger(CompositeUserTypeBinder.class); + private static final String UNDERSCORE = "_"; + + private final MetadataBuildingContext metadataBuildingContext; + private final PersistentEntityNamingStrategy namingStrategy; + private final ColumnConfigToColumnBinder columnConfigToColumnBinder; + private final ColumnBinder columnBinder; + private final BackticksRemover backticksRemover; + + public CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + JdbcEnvironment jdbcEnvironment) { + this( + metadataBuildingContext, + namingStrategy, + new ColumnConfigToColumnBinder(), + new ColumnBinder(namingStrategy, jdbcEnvironment.getDialect()), + new BackticksRemover()); + } + + protected CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + ColumnConfigToColumnBinder columnConfigToColumnBinder, + ColumnBinder columnBinder, + BackticksRemover backticksRemover) { + this.metadataBuildingContext = metadataBuildingContext; + this.namingStrategy = namingStrategy; + this.columnConfigToColumnBinder = columnConfigToColumnBinder; + this.columnBinder = columnBinder; + this.backticksRemover = backticksRemover; + } + + /** + * Binds the property as a component of its composite user type. + * + * @param property the property, mapped with a composite user type + * @param parentProperty the embedded property that holds the property, or {@code null} + * @param path the path of the property within its entity + * @return the component + * @throws MappingException when the property maps more columns than its embeddable class has attributes + */ + public Component bindCompositeUserType( + @NonNull HibernatePersistentProperty property, + HibernatePersistentProperty parentProperty, + String path) { + PersistentClass owner = property.getPersistentClass(); + Class<? extends CompositeUserType<?>> userTypeClass = property.getCompositeUserType(); + if (userTypeClass == null) { + throw new MappingException("Property [" + property.getName() + "] of class [" + owner.getClassName() + + "] is not mapped with a " + CompositeUserType.class.getSimpleName()); + } + CompositeUserType<?> userType = instantiate(userTypeClass); + Class<?> embeddable = userType.embeddable(); + PropertyConfig propertyConfig = property.getHibernateMappedForm(); + Table table = property.getTable(); + + Component component = new Component(metadataBuildingContext, owner); + component.setRoleName(GrailsHibernateUtil.qualify( + property.getHibernateOwner().getJavaClass().getName(), property.getNameForPropertyAndPath(path))); + component.setTypeName(userTypeClass.getName()); + component.setComponentClassName(embeddable.getName()); Review Comment: Fixed in 4fa1565e18. The component class is now `userType.returnedClass()`, and `embeddable()` only supplies the attributes, as Hibernate's own `EmbeddableBinder` does. The template tests now assert `CompositeMoney` as the component class, and the record template test also asserts it as the JPA attribute's Java type (`sessionFactory.metamodel.entity(CompositeUserTypeTemplated).getAttribute('amount').javaType`). The new feature `a composite user type whose embeddable class differs from the value class is queried by value` uses the separate template type and runs an HQL equality query, a criteria `eq('amount', value)`, `findAllByAmount` and `countByAmount`, plus an HQL attribute path (`p.amount.currency`). ########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernatePersistentProperty.java: ########## @@ -114,23 +123,43 @@ default GrailsHibernatePersistentEntity getHibernateOwner() { return (GrailsHibernatePersistentEntity) getOwner(); } - @SuppressWarnings("PMD.DataflowAnomalyAnalysis") default Class<?> getUserType() { PropertyConfig config = getMappedForm(); if (config == null) return null; Object typeObj = config.getType(); - Class<?> userType = null; - if (typeObj instanceof Class<?>) { - userType = (Class<?>) typeObj; - } else if (typeObj != null) { - String typeName = typeObj.toString(); - try { - userType = Class.forName(typeName, true, Thread.currentThread().getContextClassLoader()); - } catch (ClassNotFoundException ignored) { - // ignore - } + if (typeObj instanceof Class<?> userType) { + return userType; + } + return typeObj == null ? null : classForName(typeObj.toString()); + } + + /** + * The {@link CompositeUserType} the property is mapped with: the {@code type} of its own mapping, or + * the {@code user-type} registered for its class in the mapping of its owner, when that class + * implements the interface. A composite user type spans one column per attribute of its + * {@linkplain CompositeUserType#embeddable() embeddable class} and is bound as a component. + * + * @return The composite user type, or {@code null} when the property is not mapped with one + */ + @SuppressWarnings("unchecked") + default @Nullable Class<? extends CompositeUserType<?>> getCompositeUserType() { + Class<?> userType = getUserType(); + if (userType == null) { + userType = getRegisteredUserType(); Review Comment: Fixed in 4fa1565e18. `getCompositeUserType()` consults the registered `user-type` only when the property mapping has no `type`; when it has one, only that type counts, so `amount type: 'serializable'` stays a single serialized column. The new feature `a type mapped on the property wins over a composite user type registered for its class` registers `CompositeMoneyType` globally and maps `amount type: 'serializable'`. It asserts a `BasicValue` with the single column `amount`, no attribute columns on the table, and a save/load round-trip. `GrailsHibernatePersistentPropertySpec` covers the same override, and the guide now states the precedence in the composite user type section. ########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeUserTypeBinder.java: ########## @@ -0,0 +1,295 @@ +/* + * 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.beans.PropertyDescriptor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; +import java.util.TreeMap; + +import org.hibernate.MappingException; +import org.hibernate.boot.spi.MetadataBuildingContext; +import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment; +import org.hibernate.mapping.BasicValue; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.Component; +import org.hibernate.mapping.PersistentClass; +import org.hibernate.mapping.Property; +import org.hibernate.mapping.Table; +import org.hibernate.property.access.internal.PropertyAccessStrategyCompositeUserTypeImpl; +import org.hibernate.property.access.spi.PropertyAccessStrategy; +import org.hibernate.usertype.CompositeUserType; +import org.jspecify.annotations.NonNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.springframework.beans.BeanUtils; +import org.springframework.util.ReflectionUtils; + +import org.grails.datastore.mapping.model.config.GormProperties; +import org.grails.datastore.mapping.reflect.ClassPropertyFetcher; +import org.grails.orm.hibernate.cfg.ColumnConfig; +import org.grails.orm.hibernate.cfg.GrailsHibernateUtil; +import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy; +import org.grails.orm.hibernate.cfg.PropertyConfig; +import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty; +import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover; + +/** + * Binds a property mapped with a Hibernate {@link CompositeUserType} as a {@link Component}. + * <p> + * Hibernate resolves the type of a {@link BasicValue} as a {@link org.hibernate.type.BasicType} or a + * {@link org.hibernate.usertype.UserType} only. A composite user type spans one column per attribute of its + * {@linkplain CompositeUserType#embeddable() embeddable class}, so it is a component whose type name is the + * user type, as Hibernate binds a {@link org.hibernate.annotations.CompositeType}: one property per attribute, + * in alphabetical attribute order, read through {@link CompositeUserType#getPropertyValue(Object, int)} with the + * index of the attribute in that order and assembled through + * {@link CompositeUserType#instantiate(org.hibernate.metamodel.spi.ValueAccess)}. The columns of the property + * mapping are assigned to the attributes in the same order; an attribute without a configured column is named + * after the property and the attribute. + * + * @since 8.0.1 + */ +@SuppressWarnings("PMD.DataflowAnomalyAnalysis") +public class CompositeUserTypeBinder { + + private static final Logger LOG = LoggerFactory.getLogger(CompositeUserTypeBinder.class); + private static final String UNDERSCORE = "_"; + + private final MetadataBuildingContext metadataBuildingContext; + private final PersistentEntityNamingStrategy namingStrategy; + private final ColumnConfigToColumnBinder columnConfigToColumnBinder; + private final ColumnBinder columnBinder; + private final BackticksRemover backticksRemover; + + public CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + JdbcEnvironment jdbcEnvironment) { + this( + metadataBuildingContext, + namingStrategy, + new ColumnConfigToColumnBinder(), + new ColumnBinder(namingStrategy, jdbcEnvironment.getDialect()), + new BackticksRemover()); + } + + protected CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + ColumnConfigToColumnBinder columnConfigToColumnBinder, + ColumnBinder columnBinder, + BackticksRemover backticksRemover) { + this.metadataBuildingContext = metadataBuildingContext; + this.namingStrategy = namingStrategy; + this.columnConfigToColumnBinder = columnConfigToColumnBinder; + this.columnBinder = columnBinder; + this.backticksRemover = backticksRemover; + } + + /** + * Binds the property as a component of its composite user type. + * + * @param property the property, mapped with a composite user type + * @param parentProperty the embedded property that holds the property, or {@code null} + * @param path the path of the property within its entity + * @return the component + * @throws MappingException when the property maps more columns than its embeddable class has attributes + */ + public Component bindCompositeUserType( + @NonNull HibernatePersistentProperty property, + HibernatePersistentProperty parentProperty, + String path) { + PersistentClass owner = property.getPersistentClass(); + Class<? extends CompositeUserType<?>> userTypeClass = property.getCompositeUserType(); + if (userTypeClass == null) { + throw new MappingException("Property [" + property.getName() + "] of class [" + owner.getClassName() + + "] is not mapped with a " + CompositeUserType.class.getSimpleName()); + } + CompositeUserType<?> userType = instantiate(userTypeClass); + Class<?> embeddable = userType.embeddable(); + PropertyConfig propertyConfig = property.getHibernateMappedForm(); + Table table = property.getTable(); + + Component component = new Component(metadataBuildingContext, owner); + component.setRoleName(GrailsHibernateUtil.qualify( + property.getHibernateOwner().getJavaClass().getName(), property.getNameForPropertyAndPath(path))); + component.setTypeName(userTypeClass.getName()); + component.setComponentClassName(embeddable.getName()); + + Properties typeParams = propertyConfig.getTypeParams(); + if (typeParams != null && !typeParams.isEmpty()) { + LOG.warn("Property [{}] of class [{}] sets typeParams, which Hibernate does not pass to the composite " + + "user type [{}]", property.getName(), owner.getClassName(), userTypeClass.getName()); + } + + List<Attribute> attributes = attributesOf(embeddable); + List<ColumnConfig> columns = Optional.ofNullable(propertyConfig.getColumns()).orElseGet(List::of); + if (columns.size() > attributes.size()) { + throw new MappingException(String.format( + "Property [%s] of class [%s] maps %d columns, but the embeddable class [%s] of its composite " + + "user type [%s] has %d attributes: %s", + property.getName(), owner.getClassName(), columns.size(), embeddable.getName(), + userTypeClass.getName(), attributes.size(), attributes.stream().map(Attribute::name).toList())); + } + + List<String> attributeNames = new ArrayList<>(attributes.size()); + List<Type> attributeTypes = new ArrayList<>(attributes.size()); + PropertyAccessStrategy accessStrategy = + new PropertyAccessStrategyCompositeUserTypeImpl(userType, attributeNames, attributeTypes); + for (int i = 0; i < attributes.size(); i++) { + Attribute attribute = attributes.get(i); + ColumnConfig columnConfig = withName( + i < columns.size() ? columns.get(i) : null, defaultColumnName(property, path, attribute.name())); + + BasicValue value = new BasicValue(metadataBuildingContext, table); + value.setTypeUsingReflection(embeddable.getName(), attribute.name()); + + Column column = new Column(); + columnConfigToColumnBinder.bindColumnConfigToColumn(column, columnConfig, propertyConfig); + columnBinder.bindColumn( Review Comment: Fixed in 4fa1565e18. The attribute columns are now bound through a new `ColumnBinder.bindColumnWithoutUniqueness`, which leaves each column non-unique and creates no key for it. The binder then creates one key through a new `CreateKeyForProps.createKeyForColumns`, across all the component columns and, for a group, the columns of the group. Regressions in `CompositeUserTypeMappingSpec`: - `unique: true`: one key over `{amount_amount, amount_currency}` and no column unique on its own. `(10, EUR)`, `(20, EUR)` and `(10, USD)` all save, an equal `(10, EUR)` fails `validate()`, and saved with `validate: false` it is rejected by the database. - `unique: 'reference'`: one key over `{amount_amount, amount_currency, reference}`. A shared attribute, or the same value with another reference, is accepted; the same value and reference is rejected by the database. One related case is unchanged: a group on another property that names a composite property (`reference unique: 'amount'`) still resolves a single column name for it. I left that out of this PR. -- 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]
