jdaugherty commented on code in PR #16695: URL: https://github.com/apache/grails-core/pull/16695#discussion_r4348615224
########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeUserTypeBinder.java: ########## @@ -0,0 +1,312 @@ +/* + * 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; +import org.grails.orm.hibernate.cfg.domainbinding.util.ColumnNameForPropertyAndPathFetcher; +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 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. 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 static final String UNDERSCORE = "_"; + + private final MetadataBuildingContext metadataBuildingContext; + private final PersistentEntityNamingStrategy namingStrategy; + private final ColumnConfigToColumnBinder columnConfigToColumnBinder; + private final ColumnBinder columnBinder; + private final CreateKeyForProps createKeyForProps; + private final BackticksRemover backticksRemover; + + public CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + JdbcEnvironment jdbcEnvironment) { + this( + metadataBuildingContext, + namingStrategy, + new ColumnConfigToColumnBinder(), + new ColumnBinder(namingStrategy, jdbcEnvironment.getDialect()), + new CreateKeyForProps(new ColumnNameForPropertyAndPathFetcher( + namingStrategy, new DefaultColumnNameFetcher(namingStrategy), new BackticksRemover())), + new BackticksRemover()); + } + + protected CompositeUserTypeBinder( + MetadataBuildingContext metadataBuildingContext, + PersistentEntityNamingStrategy namingStrategy, + ColumnConfigToColumnBinder columnConfigToColumnBinder, + ColumnBinder columnBinder, + CreateKeyForProps createKeyForProps, + BackticksRemover backticksRemover) { + this.metadataBuildingContext = metadataBuildingContext; + this.namingStrategy = namingStrategy; + this.columnConfigToColumnBinder = columnConfigToColumnBinder; + this.columnBinder = columnBinder; + this.createKeyForProps = createKeyForProps; + 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()); + // 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 = 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()); + List<String> columnNames = 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.bindColumnWithoutUniqueness( + property, parentProperty, column, columnConfig, path, table, userTypeClass.getName()); + if (table != null) { + table.addColumn(column); + } + value.addColumn(column); + columnNames.add(column.getName()); Review Comment: Fixed in 242a2eaba0. `createKeyForColumns` now takes the bound `Column` objects and puts them in the key as they are, so a quoted name keeps its quoting and its case. The group columns are still created by name, which keeps a backticked name quoted through `Column.setName`. Regression `the unique key of a composite user type property keeps the quoting and the case of its columns` in `CompositeUserTypeMappingSpec`: columns `` `Amount` `` and `` `order` `` (mixed case, and an H2 reserved word) on a `unique: true` property. It asserts that the key holds the table's own column objects, quoted; that the schema (`information_schema.table_constraints` joined to `constraint_column_usage`) has one unique constraint on exactly `Amount` and `order`; and that the database rejects an equal value. `CreateKeyForPropsSpec` asserts the identity and the quoting of the key columns at the unit level. ########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/CreateKeyForProps.java: ########## @@ -62,20 +62,49 @@ public void createKeyForProps(HibernatePersistentProperty grailsProp, String pat List<Column> keyList = new ArrayList<>(); keyList.add(new Column(columnName)); - List<String> propertyNames = mappedForm.getUniquenessGroup(); - GrailsHibernatePersistentEntity owner = grailsProp.getHibernateOwner(); - for (String propertyName : propertyNames) { - HibernatePersistentProperty otherProp = owner.getHibernatePropertyByName(propertyName); - if (otherProp == null) { - throw new MappingException( - owner.getJavaClass().getName() + " references an unknown property " + propertyName); - } - String otherColumnName = - columnNameForPropertyAndPathFetcher.getColumnNameForPropertyAndPath(otherProp, path, null); - keyList.add(new Column(otherColumnName)); - } - + addGroupColumns(grailsProp, mappedForm, path, keyList); uniqueKeyForColumnsCreator.createUniqueKeyForColumns(table, keyList); } } + + /** + * Creates the unique key of a property that spans several columns, such as one mapped with a composite user + * type, when the property is unique: one key across all of its columns and, when it is unique within a group, + * the columns of the other properties of the group. None of the columns is unique on its own, since two + * different values of the property can share the value of one of them, so a property that is unique on its + * own needs the key as well. + * + * @param grailsProp the property + * @param path the path of the property within its entity + * @param table the table that holds the columns + * @param columnNames the names of the columns of the property + */ + public void createKeyForColumns( + HibernatePersistentProperty grailsProp, String path, Table table, List<String> columnNames) { + PropertyConfig mappedForm = grailsProp.getMappedForm(); + if (!mappedForm.isUnique() || columnNames.isEmpty()) { + return; + } + List<Column> keyList = new ArrayList<>(columnNames.size()); + columnNames.forEach(name -> keyList.add(new Column(name))); + if (mappedForm.isUniqueWithinGroup()) { Review Comment: Fixed in 242a2eaba0. `addGroupColumns` now expands each group property through a new `ColumnNameForPropertyAndPathFetcher.getColumnNamesForPropertyAndPath`: one name for a single-column property, and for a composite user type property the name of each attribute column, configured or default, from the new `CompositeUserTypeColumns` helper that the binder uses for the same assignment (the attribute discovery and the column naming moved there from the binder). To name the columns of a group property, the helper instantiates its user type through the default constructor, as Hibernate's `FallbackBeanInstanceProducer` does outside CDI; the binder keeps the CDI-aware instantiation for the instance Hibernate uses at runtime. Since `createKeyForProps` and `createKeyForColumns` share the expansion, it also covers the case I had left out, a plain property grouped with a composite one (`reference unique: 'amount'`). Regression `a uniqueness group that names a composite user type property spans all of its columns`: `amount unique: 'fee'` where `fee` has an explicit two-column mapping (the multi-column error) and `reference unique: 'amount'` where `amount` has the default mapping (the nonexistent column). It asserts both keys in the metadata and in the schema, that rows differing in the fee or in the reference are accepted, and that the database rejects the same amount and fee under another reference, and the same reference and amount with another fee. `CreateKeyForPropsSpec` covers the expansion on both paths, and the guide paragraph on `unique` now states it. -- 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]
