jamesfredley commented on code in PR #16695: URL: https://github.com/apache/grails-core/pull/16695#discussion_r4350318303
########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeUserTypeBinder.java: ########## @@ -0,0 +1,205 @@ +/* + * 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.lang.reflect.Type; +import java.util.ArrayList; +import java.util.List; +import java.util.Properties; + +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.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; +import org.grails.orm.hibernate.cfg.domainbinding.util.ColumnNameForPropertyAndPathFetcher; +import org.grails.orm.hibernate.cfg.domainbinding.util.CompositeUserTypeColumns; +import org.grails.orm.hibernate.cfg.domainbinding.util.CompositeUserTypeColumns.Attribute; +import org.grails.orm.hibernate.cfg.domainbinding.util.CreateKeyForProps; +import org.grails.orm.hibernate.cfg.domainbinding.util.DefaultColumnNameFetcher; + +/** + * 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 are the ones + * {@link CompositeUserTypeColumns} assigns to the attributes: the columns of the property mapping in the same + * order, and a name made of the property and the attribute for an attribute without a configured column. The + * component class is the {@linkplain CompositeUserType#returnedClass() value class} of the user type, not its + * embeddable class. A unique property has one unique key across all of its columns, together with the columns + * of its uniqueness group, rather than a unique key per column. + * + * @since 8.0.1 + */ +@SuppressWarnings("PMD.DataflowAnomalyAnalysis") +public class CompositeUserTypeBinder { + + private static final Logger LOG = LoggerFactory.getLogger(CompositeUserTypeBinder.class); + + private final MetadataBuildingContext metadataBuildingContext; + private final ColumnConfigToColumnBinder columnConfigToColumnBinder; + private final ColumnBinder columnBinder; + private final CreateKeyForProps createKeyForProps; + private final CompositeUserTypeColumns compositeUserTypeColumns; + + public CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + JdbcEnvironment jdbcEnvironment) { + this( + metadataBuildingContext, + new ColumnConfigToColumnBinder(), + new ColumnBinder(namingStrategy, jdbcEnvironment.getDialect()), + new CreateKeyForProps(new ColumnNameForPropertyAndPathFetcher( + namingStrategy, new DefaultColumnNameFetcher(namingStrategy), new BackticksRemover())), + new CompositeUserTypeColumns(namingStrategy, new BackticksRemover())); + } + + protected CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + ColumnConfigToColumnBinder columnConfigToColumnBinder, + ColumnBinder columnBinder, + CreateKeyForProps createKeyForProps, + CompositeUserTypeColumns compositeUserTypeColumns) { + this.metadataBuildingContext = metadataBuildingContext; + this.columnConfigToColumnBinder = columnConfigToColumnBinder; + this.columnBinder = columnBinder; + this.createKeyForProps = createKeyForProps; + this.compositeUserTypeColumns = compositeUserTypeColumns; + } + + /** + * 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()); + // The component describes the value class, as Hibernate's own binding of a composite type does: the + // embeddable class only supplies the attributes, so whole-value queries and parameters see the value class. + component.setComponentClassName(userType.returnedClass().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 = CompositeUserTypeColumns.attributesOf(embeddable); + List<ColumnConfig> columnConfigs = compositeUserTypeColumns.columnConfigsOf(property, path, embeddable, attributes); + + List<String> attributeNames = new ArrayList<>(attributes.size()); + List<Type> attributeTypes = new ArrayList<>(attributes.size()); + List<Column> columns = 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 = columnConfigs.get(i); + + BasicValue value = new BasicValue(metadataBuildingContext, table); + value.setTypeUsingReflection(embeddable.getName(), attribute.name()); + + Column column = new Column(); + columnConfigToColumnBinder.bindColumnConfigToColumn(column, columnConfig, propertyConfig); + columnBinder.bindColumnWithoutUniqueness( Review Comment: A `CompositeUserType<BigDecimal>` with two configured columns still goes through `NumericColumnConstraintsBinder`. Unless every column sets a scale, that binder calls `PropertyConfig.getScale()` and throws `Cannot treat multi-column property as a single-column property`. Bind numeric constraints from each attribute column, not from the composite value as if it were one number. ########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/CompositeUserTypeColumns.java: ########## @@ -0,0 +1,212 @@ +/* + * 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.util; + +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.TreeMap; + +import org.hibernate.MappingException; +import org.hibernate.usertype.CompositeUserType; + +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; + +/** + * The columns of a property mapped with a Hibernate {@link CompositeUserType}: one per persistent attribute of + * the {@linkplain CompositeUserType#embeddable() embeddable class} of the user type, in the alphabetical order of + * the attribute names, which is the index that {@link CompositeUserType#getPropertyValue(Object, int)} and + * {@link CompositeUserType#instantiate(org.hibernate.metamodel.spi.ValueAccess)} use. The columns configured on + * the property are assigned to the attributes in that order, and an attribute without a configured column, or + * whose column has no name, is named after the property and the attribute. The binder of such a property binds + * these columns, and a uniqueness group that names the property spans all of them. + * + * @since 8.0.1 + */ +public class CompositeUserTypeColumns { + + private static final String UNDERSCORE = "_"; + + private final PersistentEntityNamingStrategy namingStrategy; + private final BackticksRemover backticksRemover; + + public CompositeUserTypeColumns(PersistentEntityNamingStrategy namingStrategy, BackticksRemover backticksRemover) { + this.namingStrategy = namingStrategy; + this.backticksRemover = backticksRemover; + } + + /** + * The persistent attributes of the embeddable class, in alphabetical order: the components of a record, or + * the fields and the readable bean properties of any other class, less the static, transient and synthetic + * ones, the ones every Groovy object has, the ones the class lists in {@code transients} and the ones backed + * by a transient field. A setter is not required, nor is an accessor at all: Hibernate never writes the + * embeddable class, it reads the attributes through {@link CompositeUserType#getPropertyValue(Object, int)}. + * The type of an attribute is the type of its field, or of its getter when it has no field, which is how + * Hibernate resolves it. + * + * @param embeddable the embeddable class of the user type + * @return the attributes, in alphabetical order of their names + */ + public static List<Attribute> attributesOf(Class<?> embeddable) { + if (embeddable.isRecord()) { + return Arrays.stream(embeddable.getRecordComponents()) + .map(component -> new Attribute(component.getName(), component.getGenericType())) + .sorted(Comparator.comparing(Attribute::name)) + .toList(); + } + Map<String, Type> attributes = new TreeMap<>(); + ReflectionUtils.doWithFields(embeddable, + field -> attributes.putIfAbsent(field.getName(), field.getGenericType()), + field -> !field.isSynthetic() && !Modifier.isStatic(field.getModifiers()) && + !Modifier.isTransient(field.getModifiers())); + for (PropertyDescriptor descriptor : BeanUtils.getPropertyDescriptors(embeddable)) { + Method getter = descriptor.getReadMethod(); + if (getter != null && !Modifier.isStatic(getter.getModifiers())) { + attributes.putIfAbsent(descriptor.getName(), getter.getGenericReturnType()); + } + } + Collection<?> transients = Optional.ofNullable( + ClassPropertyFetcher.getStaticPropertyValue(embeddable, GormProperties.TRANSIENT, Collection.class)) + .orElseGet(List::of); + attributes.keySet().removeIf(name -> ClassPropertyFetcher.EXCLUDED_PROPERTIES.contains(name) || + transients.contains(name) || hasTransientField(embeddable, name)); + return attributes.entrySet().stream() + .map(entry -> new Attribute(entry.getKey(), entry.getValue())) + .toList(); + } + + /** + * The column configuration of each attribute of the property, in attribute order: the configured column when + * it names its column, otherwise a copy of it, or a new configuration, named after the property and the + * attribute. + * + * @param property the property, mapped with a composite user type + * @param path the path of the property within its entity + * @param embeddable the embeddable class of the user type + * @param attributes the attributes of the embeddable class, as {@link #attributesOf} lists them + * @return one named column configuration per attribute + * @throws MappingException when the property maps more columns than the embeddable class has attributes + */ + public List<ColumnConfig> columnConfigsOf( + HibernatePersistentProperty property, String path, Class<?> embeddable, List<Attribute> attributes) { + List<ColumnConfig> columns = Optional.ofNullable(property.getHibernateMappedForm()) + .map(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(), property.getOwner().getName(), columns.size(), embeddable.getName(), + property.getCompositeUserType().getName(), attributes.size(), + attributes.stream().map(Attribute::name).toList())); + } + List<ColumnConfig> named = new ArrayList<>(attributes.size()); + for (int i = 0; i < attributes.size(); i++) { + named.add(withName(i < columns.size() ? columns.get(i) : null, + defaultColumnName(property, path, attributes.get(i).name()))); + } + return named; + } + + /** + * The names of the columns of the property, in attribute order, as {@link #columnConfigsOf} names them. The + * user type is instantiated through its default constructor to read its embeddable class, as Hibernate does + * when it resolves the attributes of a composite type outside a CDI container. + * + * @param property the property, mapped with a composite user type + * @param path the path of the property within its entity + * @return the column names + * @throws MappingException when the property is not mapped with a composite user type, or maps more columns + * than its embeddable class has attributes + */ + public List<String> columnNamesOf(HibernatePersistentProperty property, String path) { + Class<? extends CompositeUserType<?>> userTypeClass = property.getCompositeUserType(); + if (userTypeClass == null) { + throw new MappingException("Property [" + property.getName() + "] of class [" + + property.getOwner().getName() + "] is not mapped with a " + + CompositeUserType.class.getSimpleName()); + } + Class<?> embeddable = BeanUtils.instantiateClass(userTypeClass).embeddable(); Review Comment: A uniqueness group that names a composite user type built by CDI still requires a default constructor here. Binding that same property uses `CompositeUserTypeBinder.instantiate()`. Startup then fails only when another property is unique within this one. Resolve the type the same way the binder does, or reuse the attribute metadata already resolved for the property. ########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsPropertyBinder.java: ########## @@ -81,6 +112,11 @@ public Value bindProperty( // through collectionBinder.bindCollection() below so its join table gets created; // EnumTypeBinder only binds its element column, from BasicCollectionElementBinder. value = enumTypeBinder.bindEnumType(hibernateEnumProperty, path); + } else if (currentGrailsProp.getCompositeUserType() != null) { Review Comment: An enum property that maps a `CompositeUserType` still takes the `HibernateEnumProperty` branch and is bound by `EnumTypeBinder`. That binder builds a `BasicValue` with the composite user type name and hits the type-resolution failure this change was meant to avoid. Check for a composite user type before the enum branch. -- 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]
