jdaugherty commented on code in PR #16695: URL: https://github.com/apache/grails-core/pull/16695#discussion_r4360969050
########## 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: Fixed in fd71dcabc7. `ColumnBinder.bindColumnWithoutUniqueness` now takes the type of the attribute and defines the column from its own configuration and that type, never from the constraints of the property: a numeric attribute gets the scale and the precision of its column entry, or the default precision of the dialect when it has none, and the single-column accessors of `PropertyConfig` are no longer read for it (`NumericColumnConstraintsBinder` accepts a null property config for that case). The constraints of the property describe the whole value and stay with validation. Regression `a composite user type whose value is a number defines each attribute column from its own configuration` in `CompositeUserTypeMappingSpec`: a `CompositeUserType<BigDecimal>` stored as its scale and its unscaled value, once with a two-column list where only the unscaled column sets `precision: 20` and once without a column list. It asserts the schema (`numeric(20,0)`, the default precision of the dialect for the unlisted column, `integer` for the scale) and a round trip that keeps the scale. `ColumnBinderSpec` and `NumericColumnConstraintsBinderSpec` cover the unit level, and the guide now states which constraints apply to the columns. ########## 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: Fixed in fd71dcabc7. The user type is now instantiated in one place, `CompositeUserTypeColumns.userTypeOf`, which resolves it as the binder did: as a managed bean of the bean registry when the building options allow extensions in CDI, through its default constructor otherwise. The binder calls it too, and the `MetadataBuildingContext` reaches the helper from `GrailsDomainBinder`, `SimpleValueBinder` and `CompositeUserTypeBinder` through new `ColumnNameForPropertyAndPathFetcher` and `ColumnBinder` constructors, so the group path and the property path share both the code and the registry, which caches the bean per class. Regression `a composite user type that only a bean container creates is resolved through it for its property and for a group that names it`: a user type without a default constructor, a `hibernate.resource.beans.container` that creates it with `hibernate.cdi.extensions` on, and `reference unique: 'amount'`. It asserts the component, the three-column key and that the container created exactly one instance; a sibling feature asserts the startup error without a container. `CompositeUserTypeColumnsSpec` and `ColumnNameForPropertyAndPathFetcherSpec` cover the CDI and the default-constructor paths with a stubbed building context. ########## 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: Fixed in fd71dcabc7. `GrailsPropertyBinder` checks for a composite user type before the enum branch, so an enum property whose `type` names one is bound as a component of that type rather than handed to `EnumTypeBinder`. Regression `an enum property mapped with a composite user type is bound as a component rather than as an enum column` in `CompositeUserTypeMappingSpec`: an enum persisted as its code and its rank. It asserts the component and the JPA attribute type, the stored row, the constant read back, and `findAllByTier`, `countByTier` and an HQL attribute path. `GrailsPropertyBinderSpec` covers the branch, and the guide mentions the enum case. -- 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]
