From 5110fd465323e9f812dec8cf3d664b6524b02a30 Mon Sep 17 00:00:00 2001 From: Gavin Date: Fri, 9 Jun 2023 17:46:39 +0200 Subject: [PATCH] HHH-16776 more consistent reporting of errors in attribute paths and squash some warnings --- .../model/domain/AbstractManagedType.java | 62 ++++++++----------- .../domain/internal/BasicSqmPathSource.java | 4 +- .../internal/SingularAttributeImpl.java | 8 +-- .../entity/AbstractEntityPersister.java | 33 +++++----- .../hibernate/query/PathElementException.java | 25 ++++++++ .../org/hibernate/query/PathException.java | 3 + .../query/TerminalPathException.java | 25 ++++++++ .../hibernate/query/sqm/SqmPathSource.java | 18 +++--- .../NonAggregatedCompositeSimplePath.java | 13 +--- 9 files changed, 109 insertions(+), 82 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/query/PathElementException.java create mode 100644 hibernate-core/src/main/java/org/hibernate/query/TerminalPathException.java diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractManagedType.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractManagedType.java index f046a90ed5..333cc7bb38 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractManagedType.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractManagedType.java @@ -39,7 +39,7 @@ import org.hibernate.metamodel.mapping.MappingModelHelper; import org.hibernate.metamodel.model.domain.internal.AttributeContainer; import org.hibernate.metamodel.model.domain.internal.DomainModelHelper; import org.hibernate.metamodel.model.domain.spi.JpaMetamodelImplementor; -import org.hibernate.query.SemanticException; +import org.hibernate.query.PathException; import org.hibernate.type.descriptor.java.JavaType; /** @@ -121,9 +121,8 @@ public abstract class AbstractManagedType } @Override - @SuppressWarnings("unchecked") public Set> getAttributes() { - final HashSet attributes = new LinkedHashSet( getDeclaredAttributes() ); + final HashSet> attributes = new LinkedHashSet<>( getDeclaredAttributes() ); if ( getSuperType() != null ) { attributes.addAll( getSuperType().getAttributes() ); @@ -133,39 +132,36 @@ public abstract class AbstractManagedType } @Override - @SuppressWarnings("unchecked") public Set> getDeclaredAttributes() { final boolean isDeclaredSingularAttributesEmpty = CollectionHelper.isEmpty( declaredSingularAttributes ); final boolean isDeclaredPluralAttributes = CollectionHelper.isEmpty( declaredPluralAttributes ); if ( isDeclaredSingularAttributesEmpty && isDeclaredPluralAttributes ) { return Collections.emptySet(); } - final HashSet attributes; + final HashSet> attributes; if ( !isDeclaredSingularAttributesEmpty ) { - attributes = new LinkedHashSet( declaredSingularAttributes.values() ); + attributes = new LinkedHashSet<>( declaredSingularAttributes.values() ); if ( !isDeclaredPluralAttributes ) { attributes.addAll( declaredPluralAttributes.values() ); } } else { - attributes = new LinkedHashSet( declaredPluralAttributes.values() ); + attributes = new LinkedHashSet<>( declaredPluralAttributes.values() ); } return attributes; } @Override - @SuppressWarnings("unchecked") public PersistentAttribute getAttribute(String name) { - final PersistentAttribute attribute = findAttribute( name ); + final PersistentAttribute attribute = findAttribute( name ); checkNotNull( "Attribute", attribute, name ); return attribute; } @Override - @SuppressWarnings("unchecked") public PersistentAttribute findAttribute(String name) { // first look at declared attributes - PersistentAttribute attribute = findDeclaredAttribute( name ); + PersistentAttribute attribute = findDeclaredAttribute( name ); if ( attribute != null ) { return attribute; } @@ -177,20 +173,18 @@ public abstract class AbstractManagedType } } - for ( ManagedDomainType subType : subTypes ) { + for ( ManagedDomainType subType : subTypes ) { PersistentAttribute subTypeAttribute = subType.findSubTypesAttribute( name ); if ( subTypeAttribute != null ) { if ( attribute != null && !isCompatible( attribute, subTypeAttribute ) ) { - throw new IllegalArgumentException( - new SemanticException( - String.format( - Locale.ROOT, - "Could not resolve attribute '%s' of '%s' due to the attribute being declared in multiple subtypes: ['%s', '%s']", - name, - getTypeName(), - attribute.getDeclaringType().getTypeName(), - subTypeAttribute.getDeclaringType().getTypeName() - ) + throw new PathException( + String.format( + Locale.ROOT, + "Could not resolve attribute '%s' of '%s' due to the attribute being declared in multiple subtypes: '%s', '%s'", + name, + getTypeName(), + attribute.getDeclaringType().getTypeName(), + subTypeAttribute.getDeclaringType().getTypeName() ) ); } @@ -236,12 +230,12 @@ public abstract class AbstractManagedType @Override public PersistentAttribute findSubTypesAttribute(String name) { // first look at declared attributes - PersistentAttribute attribute = findDeclaredAttribute( name ); + PersistentAttribute attribute = findDeclaredAttribute( name ); if ( attribute != null ) { return attribute; } - for ( ManagedDomainType subType : subTypes ) { + for ( ManagedDomainType subType : subTypes ) { PersistentAttribute subTypeAttribute = subType.findAttribute( name ); if ( subTypeAttribute != null ) { return subTypeAttribute; @@ -252,10 +246,9 @@ public abstract class AbstractManagedType } @Override - @SuppressWarnings("unchecked") public PersistentAttribute findDeclaredAttribute(String name) { // try singular attribute - PersistentAttribute attribute = declaredSingularAttributes.get( name ); + PersistentAttribute attribute = declaredSingularAttributes.get( name ); if ( attribute != null ) { return attribute; } @@ -274,9 +267,8 @@ public abstract class AbstractManagedType } @Override - @SuppressWarnings("unchecked") public PersistentAttribute getDeclaredAttribute(String name) { - PersistentAttribute attr = findDeclaredAttribute( name ); + PersistentAttribute attr = findDeclaredAttribute( name ); checkNotNull( "Attribute", attr, name ); return attr; } @@ -304,9 +296,8 @@ public abstract class AbstractManagedType // Singular attributes @Override - @SuppressWarnings("unchecked") public Set> getSingularAttributes() { - HashSet attributes = new HashSet<>( declaredSingularAttributes.values() ); + HashSet> attributes = new HashSet<>( declaredSingularAttributes.values() ); if ( getSuperType() != null ) { attributes.addAll( getSuperType().getSingularAttributes() ); } @@ -319,17 +310,15 @@ public abstract class AbstractManagedType } @Override - @SuppressWarnings("unchecked") public SingularPersistentAttribute getSingularAttribute(String name) { - SingularPersistentAttribute attribute = findSingularAttribute( name ); + SingularPersistentAttribute attribute = findSingularAttribute( name ); checkNotNull( "SingularAttribute", attribute, name ); return attribute; } @Override - @SuppressWarnings("unchecked") public SingularPersistentAttribute findSingularAttribute(String name) { - SingularPersistentAttribute attribute = findDeclaredSingularAttribute( name ); + SingularPersistentAttribute attribute = findDeclaredSingularAttribute( name ); if ( attribute == null && getSuperType() != null ) { attribute = getSuperType().findSingularAttribute( name ); } @@ -339,7 +328,7 @@ public abstract class AbstractManagedType @Override @SuppressWarnings("unchecked") public SingularPersistentAttribute getSingularAttribute(String name, Class type) { - SingularAttribute attribute = findSingularAttribute( name ); + SingularAttribute attribute = findSingularAttribute( name ); checkTypeForSingleAttribute( attribute, name, type ); return (SingularPersistentAttribute) attribute; } @@ -362,7 +351,7 @@ public abstract class AbstractManagedType public SingularPersistentAttribute getDeclaredSingularAttribute(String name, Class javaType) { final SingularAttribute attr = findDeclaredSingularAttribute( name ); checkTypeForSingleAttribute( attr, name, javaType ); - return (SingularPersistentAttribute) attr; + return (SingularPersistentAttribute) attr; } private void checkTypeForSingleAttribute( @@ -747,7 +736,6 @@ public abstract class AbstractManagedType protected class InFlightAccessImpl implements InFlightAccess { @Override - @SuppressWarnings("unchecked") public void addAttribute(PersistentAttribute attribute) { if ( attribute instanceof SingularPersistentAttribute ) { declaredSingularAttributes.put( attribute.getName(), (SingularPersistentAttribute) attribute ); diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicSqmPathSource.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicSqmPathSource.java index b920b2cf2d..7a5c2de4cf 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicSqmPathSource.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicSqmPathSource.java @@ -8,6 +8,7 @@ package org.hibernate.metamodel.model.domain.internal; import org.hibernate.metamodel.model.domain.BasicDomainType; import org.hibernate.query.ReturnableType; +import org.hibernate.query.TerminalPathException; import org.hibernate.query.sqm.SqmPathSource; import org.hibernate.query.sqm.tree.domain.SqmBasicValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmPath; @@ -43,7 +44,8 @@ public class BasicSqmPathSource @Override public SqmPathSource findSubPathSource(String name) { - throw new IllegalStateException( "Basic paths cannot be dereferenced" ); + throw new TerminalPathException( "Path '" + pathModel.getPathName() + + "' has no attribute '" + name + "'" ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/SingularAttributeImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/SingularAttributeImpl.java index 6574e75b68..cfc7333d0a 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/SingularAttributeImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/SingularAttributeImpl.java @@ -152,7 +152,7 @@ public class SingularAttributeImpl } //noinspection unchecked - return new SqmSingularJoin( + return new SqmSingularJoin( lhs, this, alias, @@ -166,7 +166,7 @@ public class SingularAttributeImpl public NavigablePath createNavigablePath(SqmPath parent, String alias) { if ( parent == null ) { throw new IllegalArgumentException( - "`lhs` cannot be null for a sub-navigable reference - " + getName() + "LHS cannot be null for a sub-navigable reference - " + getName() ); } final SqmPathSource parentPathSource = parent.getReferencedPathSource(); @@ -216,7 +216,7 @@ public class SingularAttributeImpl public NavigablePath createNavigablePath(SqmPath parent, String alias) { if ( parent == null ) { throw new IllegalArgumentException( - "`lhs` cannot be null for a sub-navigable reference - " + getName() + "LHS cannot be null for a sub-navigable reference - " + getName() ); } NavigablePath navigablePath = parent.getNavigablePath(); @@ -280,7 +280,7 @@ public class SingularAttributeImpl @Override public boolean isAssociation() { return getPersistentAttributeType() == PersistentAttributeType.MANY_TO_ONE - || getPersistentAttributeType() == PersistentAttributeType.ONE_TO_ONE; + || getPersistentAttributeType() == PersistentAttributeType.ONE_TO_ONE; } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index fe504874ab..2ca8938ed0 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -209,7 +209,7 @@ import org.hibernate.persister.internal.SqlFragmentPredicate; import org.hibernate.persister.spi.PersisterCreationContext; import org.hibernate.property.access.spi.PropertyAccess; import org.hibernate.property.access.spi.Setter; -import org.hibernate.query.SemanticException; +import org.hibernate.query.PathException; import org.hibernate.query.named.NamedQueryMemento; import org.hibernate.query.spi.QueryOptions; import org.hibernate.query.sql.internal.SQLQueryParser; @@ -235,7 +235,6 @@ import org.hibernate.sql.ast.spi.SqlSelection; import org.hibernate.sql.ast.tree.expression.AliasedExpression; import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.ast.tree.expression.Expression; -import org.hibernate.sql.ast.tree.expression.JdbcParameter; import org.hibernate.sql.ast.tree.expression.QueryLiteral; import org.hibernate.sql.ast.tree.from.NamedTableReference; import org.hibernate.sql.ast.tree.from.StandardTableGroup; @@ -5718,22 +5717,20 @@ public abstract class AbstractEntityPersister final ModelPart subDefinedAttribute = subMappingType.findSubTypesSubPart( name, treatTargetType ); if ( subDefinedAttribute != null ) { if ( attribute != null && !MappingModelHelper.isCompatibleModelPart( attribute, subDefinedAttribute ) ) { - throw new IllegalArgumentException( - new SemanticException( - String.format( - Locale.ROOT, - "Could not resolve attribute '%s' of '%s' due to the attribute being declared in multiple subtypes: ['%s', '%s']", - name, - getJavaType().getJavaType().getTypeName(), - attribute.asAttributeMapping().getDeclaringType() - .getJavaType() - .getJavaType() - .getTypeName(), - subDefinedAttribute.asAttributeMapping().getDeclaringType() - .getJavaType() - .getJavaType() - .getTypeName() - ) + throw new PathException( + String.format( + Locale.ROOT, + "Could not resolve attribute '%s' of '%s' due to the attribute being declared in multiple subtypes: '%s', '%s'", + name, + getJavaType().getJavaType().getTypeName(), + attribute.asAttributeMapping().getDeclaringType() + .getJavaType() + .getJavaType() + .getTypeName(), + subDefinedAttribute.asAttributeMapping().getDeclaringType() + .getJavaType() + .getJavaType() + .getTypeName() ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/PathElementException.java b/hibernate-core/src/main/java/org/hibernate/query/PathElementException.java new file mode 100644 index 0000000000..77d75581f6 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/query/PathElementException.java @@ -0,0 +1,25 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.query; + +/** + * Indicates that an element of a path did not resolve to + * a mapped program element. + * + * @apiNote extends {@link IllegalArgumentException} to + * satisfy a questionable requirement of the JPA criteria + * query API + * + * @since 6.3 + * + * @author Gavin King + */ +public class PathElementException extends IllegalArgumentException { + public PathElementException(String message) { + super(message); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/query/PathException.java b/hibernate-core/src/main/java/org/hibernate/query/PathException.java index 3952900962..a9b8e156f1 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/PathException.java +++ b/hibernate-core/src/main/java/org/hibernate/query/PathException.java @@ -12,6 +12,9 @@ import org.hibernate.HibernateException; * Indicates an attempt to use a path in an unsupported way * * @author Steve Ebersole + * + * @see PathElementException + * @see TerminalPathException */ public class PathException extends HibernateException { public PathException(String message) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/TerminalPathException.java b/hibernate-core/src/main/java/org/hibernate/query/TerminalPathException.java new file mode 100644 index 0000000000..398beaca77 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/query/TerminalPathException.java @@ -0,0 +1,25 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.query; + +/** + * Indicates an attempt to dereference a terminal path + * (usually a path referring to something of basic type) + * + * @apiNote extends {@link IllegalStateException} to + * satisfy a questionable requirement of the JPA criteria + * query API + * + * @since 6.3 + * + * @author Gavin King + */ +public class TerminalPathException extends IllegalStateException { + public TerminalPathException(String message) { + super(message); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmPathSource.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmPathSource.java index 8c6d98be85..9de2997524 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmPathSource.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/SqmPathSource.java @@ -11,7 +11,7 @@ import java.util.Locale; import jakarta.persistence.metamodel.Bindable; import org.hibernate.metamodel.model.domain.DomainType; -import org.hibernate.query.SemanticException; +import org.hibernate.query.PathElementException; import org.hibernate.spi.NavigablePath; import org.hibernate.query.sqm.tree.SqmExpressibleAccessor; import org.hibernate.query.sqm.tree.domain.SqmPath; @@ -41,7 +41,7 @@ public interface SqmPathSource extends SqmExpressible, Bindable, SqmExp /** * Find a SqmPathSource by name relative to this source. * - * returns null if the subPathSource is not found + * @return null if the subPathSource is not found * * @throws IllegalStateException to indicate that this source cannot be de-referenced */ @@ -56,14 +56,12 @@ public interface SqmPathSource extends SqmExpressible, Bindable, SqmExp default SqmPathSource getSubPathSource(String name) { final SqmPathSource subPathSource = findSubPathSource( name ); if ( subPathSource == null ) { - throw new IllegalArgumentException( - new SemanticException( - String.format( - Locale.ROOT, - "Could not resolve attribute '%s' of '%s'", - name, - getExpressible().getTypeName() - ) + throw new PathElementException( + String.format( + Locale.ROOT, + "Could not resolve attribute '%s' of '%s'", + name, + getExpressible().getTypeName() ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/NonAggregatedCompositeSimplePath.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/NonAggregatedCompositeSimplePath.java index 2c032840a2..38b00e8237 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/NonAggregatedCompositeSimplePath.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/NonAggregatedCompositeSimplePath.java @@ -9,7 +9,6 @@ package org.hibernate.query.sqm.tree.domain; import org.hibernate.metamodel.model.domain.EntityDomainType; import org.hibernate.spi.NavigablePath; import org.hibernate.query.PathException; -import org.hibernate.query.hql.spi.SqmCreationState; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SemanticQueryWalker; import org.hibernate.query.sqm.SqmPathSource; @@ -52,16 +51,6 @@ public class NonAggregatedCompositeSimplePath extends SqmEntityValuedSimplePa return path; } - @Override - public SqmPath resolvePathPart( - String name, - boolean isTerminal, - SqmCreationState creationState) { - final SqmPath sqmPath = get( name ); - creationState.getProcessingStateStack().getCurrent().getPathRegistry().register( sqmPath ); - return sqmPath; - } - @Override public X accept(SemanticQueryWalker walker) { return walker.visitNonAggregatedCompositeValuedPath( this ); @@ -70,7 +59,7 @@ public class NonAggregatedCompositeSimplePath extends SqmEntityValuedSimplePa @Override public SqmTreatedPath treatAs(EntityDomainType treatTarget) throws PathException { - throw new PathException( "Non Aggregate composite paths cannot be TREAT-ed" ); + throw new PathException( "Non-aggregate composite paths cannot be TREAT-ed" ); } }