Skip to content

Commit 192058f

Browse files
gavinkinggavinking
committed
HHH-18840 validate collection mapping annotations
and fix tests which had ignored such annotations
1 parent ec4310a commit 192058f

File tree

4 files changed

+84
-40
lines changed

4 files changed

+84
-40
lines changed

‎hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java

Lines changed: 84 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,20 @@
66

77
import java.lang.annotation.Annotation;
88
import java.lang.invoke.MethodHandles;
9+
import java.util.Collection;
910
import java.util.EnumSet;
1011
import java.util.List;
1112
import java.util.Map;
1213

14+
import jakarta.persistence.MapKey;
15+
import jakarta.persistence.MapKeyClass;
16+
import jakarta.persistence.MapKeyColumn;
17+
import jakarta.persistence.MapKeyEnumerated;
18+
import jakarta.persistence.MapKeyJoinColumn;
19+
import jakarta.persistence.MapKeyJoinColumns;
20+
import jakarta.persistence.MapKeyTemporal;
21+
import jakarta.persistence.OrderBy;
22+
import jakarta.persistence.OrderColumn;
1323
import org.hibernate.AnnotationException;
1424
import org.hibernate.AssertionFailure;
1525
import org.hibernate.MappingException;
@@ -34,8 +44,6 @@
3444
import org.hibernate.generator.EventType;
3545
import org.hibernate.generator.EventTypeSets;
3646
import org.hibernate.internal.CoreMessageLogger;
37-
import org.hibernate.internal.util.collections.CollectionHelper;
38-
import org.hibernate.mapping.Collection;
3947
import org.hibernate.mapping.Component;
4048
import org.hibernate.mapping.Join;
4149
import org.hibernate.mapping.KeyValue;
@@ -49,6 +57,7 @@
4957
import org.hibernate.metamodel.spi.EmbeddableInstantiator;
5058
import org.hibernate.models.spi.AnnotationDescriptor;
5159
import org.hibernate.models.spi.AnnotationDescriptorRegistry;
60+
import org.hibernate.models.spi.ArrayTypeDetails;
5261
import org.hibernate.models.spi.ClassDetails;
5362
import org.hibernate.models.spi.MemberDetails;
5463
import org.hibernate.models.spi.SourceModelBuildingContext;
@@ -93,6 +102,7 @@
93102
import static org.hibernate.boot.model.internal.ToOneBinder.bindOneToOne;
94103
import static org.hibernate.id.IdentifierGeneratorHelper.getForeignId;
95104
import static org.hibernate.internal.util.StringHelper.qualify;
105+
import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty;
96106

97107
/**
98108
* A stateful binder responsible for creating {@link Property} objects.
@@ -275,29 +285,29 @@ private Property makePropertyAndValue() {
275285

276286
@SuppressWarnings({"rawtypes", "unchecked"})
277287
private void callAttributeBinders(Property property, Map<String, PersistentClass> persistentClasses) {
278-
final List<? extends Annotation> metaAnnotatedTargets = memberDetails.getMetaAnnotated(
279-
AttributeBinderType.class,
280-
getSourceModelContext()
281-
);
282-
283-
if ( CollectionHelper.isEmpty( metaAnnotatedTargets ) ) {
284-
return;
285-
}
286-
287-
final AnnotationDescriptorRegistry descriptorRegistry = getSourceModelContext().getAnnotationDescriptorRegistry();
288-
for ( int i = 0; i < metaAnnotatedTargets.size(); i++ ) {
289-
final Annotation metaAnnotatedTarget = metaAnnotatedTargets.get( i );
290-
final AnnotationDescriptor<? extends Annotation> metaAnnotatedDescriptor = descriptorRegistry.getDescriptor( metaAnnotatedTarget.annotationType() );
291-
final AttributeBinderType binderTypeAnn = metaAnnotatedDescriptor.getDirectAnnotationUsage( AttributeBinderType.class );
292-
try {
293-
final AttributeBinder binder = binderTypeAnn.binder().getConstructor().newInstance();
294-
final PersistentClass persistentClass = entityBinder != null
295-
? entityBinder.getPersistentClass()
296-
: persistentClasses.get( holder.getEntityName() );
297-
binder.bind( metaAnnotatedTarget, buildingContext, persistentClass, property );
298-
}
299-
catch ( Exception e ) {
300-
throw new AnnotationException( "error processing @AttributeBinderType annotation '" + metaAnnotatedDescriptor.getAnnotationType().getName() + "'", e );
288+
final List<? extends Annotation> metaAnnotatedTargets =
289+
memberDetails.getMetaAnnotated( AttributeBinderType.class, getSourceModelContext() );
290+
291+
if ( !isEmpty( metaAnnotatedTargets ) ) {
292+
final AnnotationDescriptorRegistry descriptorRegistry =
293+
getSourceModelContext().getAnnotationDescriptorRegistry();
294+
for ( int i = 0; i < metaAnnotatedTargets.size(); i++ ) {
295+
final Annotation metaAnnotatedTarget = metaAnnotatedTargets.get( i );
296+
final AnnotationDescriptor<? extends Annotation> metaAnnotatedDescriptor =
297+
descriptorRegistry.getDescriptor( metaAnnotatedTarget.annotationType() );
298+
final AttributeBinderType binderTypeAnn =
299+
metaAnnotatedDescriptor.getDirectAnnotationUsage( AttributeBinderType.class );
300+
try {
301+
final AttributeBinder binder = binderTypeAnn.binder().getConstructor().newInstance();
302+
final PersistentClass persistentClass = entityBinder != null
303+
? entityBinder.getPersistentClass()
304+
: persistentClasses.get( holder.getEntityName() );
305+
binder.bind( metaAnnotatedTarget, buildingContext, persistentClass, property );
306+
}
307+
catch ( Exception e ) {
308+
throw new AnnotationException( "error processing @AttributeBinderType annotation '" +
309+
metaAnnotatedDescriptor.getAnnotationType().getName() + "'", e );
310+
}
301311
}
302312
}
303313
}
@@ -398,12 +408,14 @@ private Component getOrCreateCompositeId(RootClass rootClass) {
398408
private Class<? extends EmbeddableInstantiator> resolveCustomInstantiator(
399409
MemberDetails property,
400410
ClassDetails embeddableClass) {
401-
final org.hibernate.annotations.EmbeddableInstantiator onEmbedded = property.getDirectAnnotationUsage( org.hibernate.annotations.EmbeddableInstantiator.class );
411+
final org.hibernate.annotations.EmbeddableInstantiator onEmbedded =
412+
property.getDirectAnnotationUsage( org.hibernate.annotations.EmbeddableInstantiator.class );
402413
if ( onEmbedded != null ) {
403414
return onEmbedded.value();
404415
}
405416

406-
final org.hibernate.annotations.EmbeddableInstantiator onEmbeddable = embeddableClass.getDirectAnnotationUsage( org.hibernate.annotations.EmbeddableInstantiator.class );
417+
final org.hibernate.annotations.EmbeddableInstantiator onEmbeddable =
418+
embeddableClass.getDirectAnnotationUsage( org.hibernate.annotations.EmbeddableInstantiator.class );
407419
if ( onEmbeddable != null ) {
408420
return onEmbeddable.value();
409421
}
@@ -414,6 +426,7 @@ private Class<? extends EmbeddableInstantiator> resolveCustomInstantiator(
414426
//used when the value is provided and the binding is done elsewhere
415427
public Property makeProperty() {
416428
validateMake();
429+
validateAnnotationsAgainstType();
417430
LOG.debugf( "Building property %s", name );
418431
Property property = new Property();
419432
property.setName( name );
@@ -505,7 +518,7 @@ private void handleNaturalId(Property property) {
505518

506519
private void inferOptimisticLocking(Property property) {
507520
// this is already handled for collections in CollectionBinder...
508-
if ( value instanceof Collection collection ) {
521+
if ( value instanceof org.hibernate.mapping.Collection collection ) {
509522
property.setOptimisticLocked( collection.isOptimisticLocked() );
510523
}
511524
else if ( memberDetails != null && memberDetails.hasDirectAnnotationUsage( OptimisticLock.class ) ) {
@@ -519,6 +532,37 @@ else if ( memberDetails != null && memberDetails.hasDirectAnnotationUsage( Optim
519532
}
520533
}
521534

535+
private void validateAnnotationsAgainstType() {
536+
if ( memberDetails != null ) {
537+
if ( !(memberDetails.getType() instanceof ArrayTypeDetails) ) {
538+
checkAnnotation( OrderColumn.class, List.class );
539+
if ( memberDetails.hasDirectAnnotationUsage( OrderBy.class )
540+
&& !memberDetails.getType().isImplementor( Collection.class )
541+
&& !memberDetails.getType().isImplementor( Map.class ) ) {
542+
throw new AnnotationException( "Property '" + qualify( holder.getPath(), name )
543+
+ "' is annotated '@OrderBy' but is not of type 'Collection' or 'Map'" );
544+
}
545+
}
546+
checkAnnotation( MapKey.class, Map.class );
547+
checkAnnotation( MapKeyColumn.class, Map.class );
548+
checkAnnotation( MapKeyClass.class, Map.class );
549+
checkAnnotation( MapKeyEnumerated.class, Map.class );
550+
checkAnnotation( MapKeyTemporal.class, Map.class );
551+
checkAnnotation( MapKeyColumn.class, Map.class );
552+
checkAnnotation( MapKeyJoinColumn.class, Map.class );
553+
checkAnnotation( MapKeyJoinColumns.class, Map.class );
554+
}
555+
}
556+
557+
private void checkAnnotation(Class<? extends Annotation> annotationClass, Class<?> propertyType) {
558+
if ( memberDetails.hasDirectAnnotationUsage( annotationClass )
559+
&& !memberDetails.getType().isImplementor( propertyType ) ) {
560+
throw new AnnotationException( "Property '" + qualify( holder.getPath(), name )
561+
+ "' is annotated '@" + annotationClass.getSimpleName()
562+
+ "' but is not of type '" + propertyType.getTypeName() + "'" );
563+
}
564+
}
565+
522566
private void validateOptimisticLock(boolean excluded) {
523567
if ( excluded ) {
524568
if ( memberDetails.hasDirectAnnotationUsage( Version.class ) ) {
@@ -587,29 +631,32 @@ private static int addProperty(
587631
inFlightPropertyDataList.add( 0, propertyAnnotatedElement );
588632
handleIdProperty( propertyContainer, context, declaringClass, ownerType, element );
589633
if ( hasToOneAnnotation( element ) ) {
590-
context.getMetadataCollector().addToOneAndIdProperty( ownerType.determineRawClass(), propertyAnnotatedElement );
634+
context.getMetadataCollector()
635+
.addToOneAndIdProperty( ownerType.determineRawClass(), propertyAnnotatedElement );
591636
}
592637
idPropertyCounter++;
593638
}
594639
else {
595640
inFlightPropertyDataList.add( propertyAnnotatedElement );
596641
}
597642
if ( element.hasDirectAnnotationUsage( MapsId.class ) ) {
598-
context.getMetadataCollector().addPropertyAnnotatedWithMapsId( ownerType.determineRawClass(), propertyAnnotatedElement );
643+
context.getMetadataCollector()
644+
.addPropertyAnnotatedWithMapsId( ownerType.determineRawClass(), propertyAnnotatedElement );
599645
}
600646

601647
return idPropertyCounter;
602648
}
603649

604650
private static void checkIdProperty(MemberDetails property, PropertyData propertyData) {
605651
final Id incomingIdProperty = property.getDirectAnnotationUsage( Id.class );
606-
final Id existingIdProperty = propertyData.getAttributeMember().getDirectAnnotationUsage( Id.class );
652+
final MemberDetails attributeMember = propertyData.getAttributeMember();
653+
final Id existingIdProperty = attributeMember.getDirectAnnotationUsage( Id.class );
607654
if ( incomingIdProperty != null && existingIdProperty == null ) {
608655
throw new MappingException(
609656
String.format(
610657
"You cannot override the [%s] non-identifier property from the [%s] base class or @MappedSuperclass and make it an identifier in the [%s] subclass",
611-
propertyData.getAttributeMember().getName(),
612-
propertyData.getAttributeMember().getDeclaringType().getName(),
658+
attributeMember.getName(),
659+
attributeMember.getDeclaringType().getName(),
613660
property.getDeclaringType().getName()
614661
)
615662
);
@@ -631,7 +678,8 @@ private static void handleIdProperty(
631678
if ( element.hasDirectAnnotationUsage( Id.class ) && element.hasDirectAnnotationUsage( Column.class ) ) {
632679
final String columnName = element.getDirectAnnotationUsage( Column.class ).name();
633680
declaringClass.forEachField( (index, fieldDetails) -> {
634-
if ( !element.hasDirectAnnotationUsage( MapsId.class ) && isJoinColumnPresent( columnName, element, sourceModelContext ) ) {
681+
if ( !element.hasDirectAnnotationUsage( MapsId.class )
682+
&& isJoinColumnPresent( columnName, element, sourceModelContext ) ) {
635683
//create a PropertyData for the specJ property holding the mapping
636684
context.getMetadataCollector().addPropertyAnnotatedWithMapsIdSpecj(
637685
ownerType.determineRawClass(),
@@ -1157,7 +1205,8 @@ private static boolean isComposite(
11571205
MemberDetails property,
11581206
ClassDetails returnedClass,
11591207
PropertyData overridingProperty) {
1160-
final InheritanceState state = inheritanceStatePerClass.get( overridingProperty.getClassOrElementType().determineRawClass() );
1208+
final InheritanceState state =
1209+
inheritanceStatePerClass.get( overridingProperty.getClassOrElementType().determineRawClass() );
11611210
return state != null ? state.hasIdClassOrEmbeddedId() : isEmbedded( property, returnedClass );
11621211
}
11631212

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/manytomany/ManyToManyMaxFetchDepth0Test.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
*
1313
* @author Gail Badner
1414
*/
15-
@SuppressWarnings("unchecked")
1615
public class ManyToManyMaxFetchDepth0Test extends ManyToManyTest {
1716
@Override
1817
protected void configure(Configuration cfg) {

‎hibernate-core/src/test/java/org/hibernate/orm/test/mapping/UserEntity.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import jakarta.persistence.GeneratedValue;
1717
import jakarta.persistence.Id;
1818
import jakarta.persistence.OneToMany;
19-
import jakarta.persistence.OrderColumn;
2019
import jakarta.persistence.Table;
2120

2221
@Entity
@@ -30,7 +29,6 @@ public class UserEntity implements Serializable{
3029
@Column(name = "user_id")
3130
private Long id;
3231

33-
@OrderColumn(name = "cnf_order")
3432
@OneToMany(mappedBy="user", fetch = EAGER, cascade = ALL, orphanRemoval = true)
3533
private Set<UserConfEntity> confs = new HashSet<UserConfEntity>();
3634

‎hibernate-core/src/test/java/org/hibernate/orm/test/query/hhh12076/AnnotationMappingJoinClassTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import jakarta.persistence.JoinColumn;
3636
import jakarta.persistence.ManyToOne;
3737
import jakarta.persistence.OneToMany;
38-
import jakarta.persistence.OrderColumn;
3938
import jakarta.persistence.Table;
4039
import jakarta.persistence.Temporal;
4140
import jakarta.persistence.TemporalType;
@@ -720,7 +719,6 @@ public static class Settlement {
720719
private SettlementStatus status = SettlementStatus.RESERVED;
721720

722721
@OneToMany(mappedBy = "settlement", cascade = CascadeType.ALL, orphanRemoval = true)
723-
@OrderColumn(name = "orderindex")
724722
private Set<SettlementExtension> extensions = new HashSet<>();
725723

726724
private transient Map<Class<?>, SettlementExtension> extensionMap;

0 commit comments

Comments
 (0)