jamesfredley commented on code in PR #16695:
URL: https://github.com/apache/grails-core/pull/16695#discussion_r4342063827
##########
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:
`amount unique: 'fee'` where `fee` is another composite property still goes
through a helper that resolves one column per group property. An explicit
multi-column mapping throws `Cannot treat multi-column property as a
single-column property`. A default mapping names a column that does not exist.
Expand each group property to all of its bound columns in the same unique
key. Please cover a composite property unique within another composite property.
##########
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:
`column.getName()` drops quoting. `createKeyForColumns` then builds new
unquoted `Column` objects. A unique composite property whose columns are
quoted, mixed-case, or reserved words gets a unique key on different
identifiers, or invalid SQL.
Keep the original column objects, or collect `getQuotedName()`. Please add a
regression with a quoted or reserved-word column name and assert the unique-key
DDL uses that name.
--
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]
