Skip to content

Commit ddf0a90

Browse files
gavinkinggavinking
committed
HHH-19303 validate @id fields against @IdClass in Processor
1 parent a0be804 commit ddf0a90

File tree

7 files changed

+152
-27
lines changed

7 files changed

+152
-27
lines changed

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/cid/CompositeIdTypeMismatchTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import jakarta.persistence.Id;
99
import jakarta.persistence.IdClass;
1010
import org.hibernate.AnnotationException;
11+
import org.hibernate.annotations.processing.Exclude;
1112
import org.hibernate.boot.MetadataSources;
1213
import org.hibernate.boot.registry.StandardServiceRegistry;
1314
import org.hibernate.testing.util.ServiceRegistryUtil;
@@ -19,6 +20,7 @@
1920
/**
2021
* @author Yanming Zhou
2122
*/
23+
@Exclude
2224
public class CompositeIdTypeMismatchTest {
2325

2426
@Test

‎hibernate-core/src/test/java/org/hibernate/orm/test/idclass/IdClassPropertiesTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.io.Serializable;
88

99
import org.hibernate.AnnotationException;
10+
import org.hibernate.annotations.processing.Exclude;
1011
import org.hibernate.boot.MetadataSources;
1112
import org.hibernate.boot.registry.StandardServiceRegistry;
1213

@@ -26,6 +27,7 @@
2627
* @author Marco Belladelli
2728
*/
2829
@Jira( "https://hibernate.atlassian.net/browse/HHH-16761" )
30+
@Exclude
2931
public class IdClassPropertiesTest {
3032
@Test
3133
public void testRight() {

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AnnotationMeta.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ private void handleNamedQueryAnnotation(String annotationName, boolean checkHql)
7272
private void handleNamedQueryRepeatableAnnotation(String annotationName, boolean checkHql) {
7373
final AnnotationMirror mirror = getAnnotationMirror( getElement(), annotationName );
7474
if ( mirror != null ) {
75-
final AnnotationValue value = getAnnotationValue( mirror, "value" );
75+
final AnnotationValue value = getAnnotationValue( mirror );
7676
if ( value != null ) {
7777
@SuppressWarnings("unchecked")
7878
final List<? extends AnnotationValue> annotationValues =
@@ -151,7 +151,7 @@ && isJavaIdentifierStart( name.charAt(1) )
151151
private void addAuxiliaryMembersForRepeatableAnnotation(String annotationName, String prefix) {
152152
final AnnotationMirror mirror = getAnnotationMirror( getElement(), annotationName );
153153
if ( mirror != null ) {
154-
final AnnotationValue value = getAnnotationValue( mirror, "value" );
154+
final AnnotationValue value = getAnnotationValue( mirror );
155155
if ( value != null ) {
156156
@SuppressWarnings("unchecked")
157157
final List<? extends AnnotationValue> annotationValues =

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AnnotationMetaEntity.java‎

Lines changed: 137 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import java.util.Set;
6161
import java.util.StringTokenizer;
6262
import java.util.regex.Pattern;
63+
import java.util.stream.Stream;
6364

6465
import jakarta.persistence.AccessType;
6566

@@ -463,31 +464,148 @@ && containsAnnotation( method, HQL, SQL, FIND ) ) {
463464
initialized = true;
464465
}
465466

466-
private void addIdClassIfNeeded(List<? extends Element> fields, List<? extends Element> methods) {
467+
/**
468+
* Creates a generated id class named {@code Entity_.Id} if the
469+
* entity has multiple {@code @Id} fields, but no {@code @IdClass}
470+
* annotation.
471+
*/
472+
private void addIdClassIfNeeded(List<VariableElement> fields, List<ExecutableElement> methods) {
467473
if ( hasAnnotation( element, ID_CLASS ) ) {
468-
return;
474+
checkIdMembers( fields, methods );
475+
}
476+
else {
477+
final List<MetaAttribute> components = getIdMemberNames( fields, methods );
478+
if ( components.size() >= 2 ) {
479+
putMember( ID_CLASS_MEMBER_NAME, new IdClassMetaAttribute( this, components ) );
480+
}
469481
}
482+
}
483+
484+
private List<MetaAttribute> getIdMemberNames(List<VariableElement> fields, List<ExecutableElement> methods) {
470485
final List<MetaAttribute> components = new ArrayList<>();
471-
for ( Element field : fields ) {
472-
if ( hasAnnotation( field, ID ) && isPersistent( field, AccessType.FIELD ) ) {
486+
for ( var field : fields ) {
487+
if ( isIdField( field ) ) {
473488
final String propertyName = propertyName( field );
474489
if ( members.containsKey( propertyName ) ) {
475490
components.add( members.get( propertyName ) );
476491
}
477492
}
478493
}
479-
for ( Element method : methods ) {
480-
if ( hasAnnotation( method, ID ) && isPersistent( method, AccessType.PROPERTY ) ) {
494+
for ( var method : methods ) {
495+
if ( isIdProperty( method ) ) {
481496
final String propertyName = propertyName( method );
482497
if ( members.containsKey( propertyName ) ) {
483498
components.add( members.get( propertyName ) );
484499
}
485500
}
486501
}
487-
if ( components.size() < 2 ) {
488-
return;
502+
return components;
503+
}
504+
505+
private void checkIdMembers(List<VariableElement> fields, List<ExecutableElement> methods) {
506+
final AnnotationMirror annotationMirror = getAnnotationMirror( element, ID_CLASS );
507+
if ( annotationMirror != null ) {
508+
final AnnotationValue annotationValue = getAnnotationValue( annotationMirror );
509+
if ( annotationValue != null && annotationValue.getValue() instanceof DeclaredType declaredType ) {
510+
final TypeElement idClass = (TypeElement) declaredType.asElement();
511+
if ( fields.stream().filter( this::isIdField ).count()
512+
+ methods.stream().filter( this::isIdProperty ).count()
513+
== 1 ) {
514+
for ( var field : fields ) {
515+
if ( isIdField( field ) ) {
516+
if ( hasAnnotation( field, ONE_TO_ONE ) ) {
517+
// Special case for @Id @OneToOne to an associated entity with an @IdClass
518+
//TODO: check the id type of the associated entity is the same as idClass
519+
return;
520+
}
521+
}
522+
}
523+
for ( var method : methods ) {
524+
if ( isIdProperty( method ) ) {
525+
if ( hasAnnotation( method, ONE_TO_ONE ) ) {
526+
// Special case for @Id @OneToOne to an associated entity with an @IdClass
527+
//TODO: check the id type of the associated entity is the same as idClass
528+
return;
529+
}
530+
}
531+
}
532+
}
533+
else {
534+
for ( var field : fields ) {
535+
if ( isIdField( field ) ) {
536+
memberStream( idClass )
537+
.filter( element -> element.getKind() == ElementKind.FIELD
538+
&& element.getSimpleName().contentEquals( field.getSimpleName() ) )
539+
.findAny()
540+
.ifPresentOrElse( match -> {
541+
if ( !isMatchingIdType( field, field.asType(), match.asType() ) ) {
542+
context.message( match,
543+
"id field should be of type '" + field.asType() + "'",
544+
Diagnostic.Kind.ERROR );
545+
}
546+
},
547+
() -> context.message( field,
548+
"no matching field in id class '" + idClass.getSimpleName() + "'",
549+
Diagnostic.Kind.ERROR ) );
550+
}
551+
}
552+
for ( var method : methods ) {
553+
if ( isIdProperty( method ) ) {
554+
memberStream( idClass )
555+
.filter( element -> element.getKind() == ElementKind.METHOD
556+
&& element.getSimpleName().contentEquals( method.getSimpleName() )
557+
&& isMatchingIdType( method, method.getReturnType(),
558+
((ExecutableElement) element).getReturnType() ) )
559+
.findAny()
560+
.ifPresentOrElse( match -> {
561+
},
562+
() -> context.message( method,
563+
"no matching property in id class '" + idClass.getSimpleName() + "'",
564+
Diagnostic.Kind.ERROR ) );
565+
}
566+
}
567+
}
568+
}
569+
}
570+
}
571+
572+
private boolean isIdProperty(ExecutableElement method) {
573+
return hasAnnotation( method, ID )
574+
&& isPersistent( method, AccessType.PROPERTY );
575+
}
576+
577+
private boolean isIdField(VariableElement field) {
578+
return hasAnnotation( field, ID )
579+
&& isPersistent( field, AccessType.FIELD );
580+
}
581+
582+
private static Stream<? extends Element> memberStream(TypeElement idClass) {
583+
Stream<? extends Element> result = idClass.getEnclosedElements().stream();
584+
TypeMirror superclass = idClass.getSuperclass();
585+
while ( superclass.getKind() == TypeKind.DECLARED ) {
586+
final DeclaredType declaredType = (DeclaredType) superclass;
587+
final TypeElement typeElement = (TypeElement) declaredType.asElement();
588+
result = Stream.concat( result, typeElement.getEnclosedElements().stream() );
589+
superclass = typeElement.getSuperclass();
489590
}
490-
putMember( ID_CLASS_MEMBER_NAME, new IdClassMetaAttribute( this, components ) );
591+
return result;
592+
}
593+
594+
private boolean isMatchingIdType(Element id, TypeMirror type, TypeMirror match) {
595+
return isSameType( type, match )
596+
|| isEquivalentPrimitiveType( type, match )
597+
|| isEquivalentPrimitiveType( match, type )
598+
//TODO: check the id type of the associated entity
599+
|| hasAnnotation( id, MANY_TO_ONE, ONE_TO_ONE );
600+
}
601+
602+
private boolean isSameType(TypeMirror type, TypeMirror match) {
603+
return context.getTypeUtils().isSameType( type, match );
604+
}
605+
606+
private boolean isEquivalentPrimitiveType(TypeMirror type, TypeMirror match) {
607+
return type.getKind().isPrimitive()
608+
&& isSameType( context.getTypeUtils().boxedClass( ((PrimitiveType) type) ).asType(), match );
491609
}
492610

493611
private boolean checkEntities(List<ExecutableElement> lifecycleMethods) {
@@ -1394,7 +1512,7 @@ else if ( !hasAnnotation( declaredType.asElement(), ENTITY )
13941512
Diagnostic.Kind.ERROR );
13951513
}
13961514
else if ( returnArgument
1397-
&& !context.getTypeUtils().isSameType( returnType, declaredParameterType ) ) {
1515+
&& !isSameType( returnType, declaredParameterType ) ) {
13981516
message( parameter,
13991517
"return type '" + returnType
14001518
+ "' disagrees with parameter type '" + parameterType + "'",
@@ -1767,7 +1885,7 @@ private List<OrderBy> orderByList(ExecutableElement method, TypeElement returnTy
17671885
final List<OrderBy> result = new ArrayList<>();
17681886
@SuppressWarnings("unchecked")
17691887
final List<AnnotationValue> list = (List<AnnotationValue>)
1770-
castNonNull( getAnnotationValue( orderByList, "value" ) ).getValue();
1888+
castNonNull( getAnnotationValue( orderByList ) ).getValue();
17711889
for ( AnnotationValue element : list ) {
17721890
result.add( orderByExpression( castNonNull( (AnnotationMirror) element.getValue() ), entityType, method ) );
17731891
}
@@ -1782,7 +1900,7 @@ private List<OrderBy> orderByList(ExecutableElement method, TypeElement returnTy
17821900
}
17831901

17841902
private OrderBy orderByExpression(AnnotationMirror orderBy, TypeElement entityType, ExecutableElement method) {
1785-
final String fieldName = castNonNull( getAnnotationValue(orderBy, "value") ).getValue().toString();
1903+
final String fieldName = castNonNull( getAnnotationValue(orderBy) ).getValue().toString();
17861904
if ( fieldName.equals("<error>") ) {
17871905
throw new ProcessLaterException();
17881906
}
@@ -2138,9 +2256,9 @@ else if ( containsAnnotation( member, NATURAL_ID ) ) {
21382256
else {
21392257
final AnnotationMirror idClass = getAnnotationMirror( entityType, ID_CLASS );
21402258
if ( idClass != null ) {
2141-
final AnnotationValue value = getAnnotationValue( idClass, "value" );
2259+
final AnnotationValue value = getAnnotationValue( idClass );
21422260
if ( value != null ) {
2143-
if ( context.getTypeUtils().isSameType( param.asType(), (TypeMirror) value.getValue() ) ) {
2261+
if ( isSameType( param.asType(), (TypeMirror) value.getValue() ) ) {
21442262
return FieldType.ID;
21452263
}
21462264
}
@@ -2360,7 +2478,7 @@ private void addQueryMethod(
23602478
final ArrayType arrayType = (ArrayType) returnType;
23612479
final TypeMirror componentType = arrayType.getComponentType();
23622480
final TypeElement object = context.getElementUtils().getTypeElement(JAVA_OBJECT);
2363-
if ( !context.getTypeUtils().isSameType( object.asType(), componentType ) ) {
2481+
if ( !isSameType( object.asType(), componentType ) ) {
23642482
returnType = componentType;
23652483
containerTypeName = "[]";
23662484
}
@@ -2377,7 +2495,7 @@ private void addQueryMethod(
23772495
containerTypeName = containerType.getQualifiedName().toString();
23782496
}
23792497

2380-
final AnnotationValue value = getAnnotationValue( mirror, "value" );
2498+
final AnnotationValue value = getAnnotationValue( mirror );
23812499
if ( value != null && value.getValue() instanceof String queryString ) {
23822500
addQueryMethod( method, returnType, containerTypeName, mirror, isNative, value, queryString );
23832501
}
@@ -2679,7 +2797,7 @@ else if ( selection instanceof JpaRoot<?> from ) {
26792797
else {
26802798
final TypeElement typeElement = context.getTypeElementForFullyQualifiedName( javaResultType.getName() );
26812799
final Types types = context.getTypeUtils();
2682-
returnTypeCorrect = context.getTypeUtils().isAssignable( returnType, types.erasure( typeElement.asType() ) );
2800+
returnTypeCorrect = types.isAssignable( returnType, types.erasure( typeElement.asType() ) );
26832801
}
26842802
}
26852803
catch (Exception e) {
@@ -3026,7 +3144,7 @@ private static String parameterName(VariableElement parameter) {
30263144
final AnnotationMirror param = getAnnotationMirror( parameter, "jakarta.data.repository.Param" );
30273145
if ( by != null ) {
30283146
final String name =
3029-
castNonNull(getAnnotationValue(by, "value"))
3147+
castNonNull(getAnnotationValue(by))
30303148
.getValue().toString();
30313149
if ( name.contains("<error>") ) {
30323150
throw new ProcessLaterException();
@@ -3037,7 +3155,7 @@ private static String parameterName(VariableElement parameter) {
30373155
}
30383156
else if ( param != null ) {
30393157
final String name =
3040-
castNonNull(getAnnotationValue(param, "value"))
3158+
castNonNull(getAnnotationValue(param))
30413159
.getValue().toString();
30423160
if ( name.contains("<error>") ) {
30433161
throw new ProcessLaterException();

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/DataMetaAttributeGenerationVisitor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import org.checkerframework.checker.nullness.qual.Nullable;
88
import org.hibernate.processor.Context;
99
import org.hibernate.processor.util.Constants;
10-
import org.hibernate.processor.util.NullnessUtil;
1110

1211
import javax.lang.model.element.Element;
1312
import javax.lang.model.element.TypeElement;
@@ -19,6 +18,7 @@
1918
import javax.lang.model.util.SimpleTypeVisitor8;
2019
import javax.lang.model.util.Types;
2120

21+
import static org.hibernate.processor.util.NullnessUtil.castNonNull;
2222
import static org.hibernate.processor.util.TypeUtils.getTargetEntity;
2323
import static org.hibernate.processor.util.TypeUtils.isBasicAttribute;
2424
import static org.hibernate.processor.util.TypeUtils.isPropertyGetter;
@@ -71,7 +71,7 @@ private Types typeUtils() {
7171
public @Nullable DataAnnotationMetaAttribute visitDeclared(DeclaredType declaredType, Element element) {
7272
final TypeElement returnedElement = (TypeElement) typeUtils().asElement( declaredType );
7373
// WARNING: .toString() is necessary here since Name equals does not compare to String
74-
final String returnTypeName = NullnessUtil.castNonNull( returnedElement ).getQualifiedName().toString();
74+
final String returnTypeName = castNonNull( returnedElement ).getQualifiedName().toString();
7575
final String collection = Constants.COLLECTIONS.get( returnTypeName );
7676
final String targetEntity = getTargetEntity( element.getAnnotationMirrors() );
7777
if ( collection != null ) {

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/MetaAttributeGenerationVisitor.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.hibernate.processor.Context;
1010
import org.hibernate.processor.util.AccessTypeInformation;
1111
import org.hibernate.processor.util.Constants;
12-
import org.hibernate.processor.util.NullnessUtil;
1312

1413
import javax.lang.model.element.AnnotationMirror;
1514
import javax.lang.model.element.Element;
@@ -92,7 +91,7 @@ private Types typeUtils() {
9291
final TypeElement returnedElement = (TypeElement) typeUtils().asElement( declaredType );
9392
assert returnedElement != null;
9493
// WARNING: .toString() is necessary here since Name equals does not compare to String
95-
final String returnTypeName = NullnessUtil.castNonNull( returnedElement ).getQualifiedName().toString();
94+
final String returnTypeName = castNonNull( returnedElement ).getQualifiedName().toString();
9695
final String collection = Constants.COLLECTIONS.get( returnTypeName );
9796
final String targetEntity = getTargetEntity( element.getAnnotationMirrors() );
9897
if ( collection != null ) {
@@ -116,7 +115,7 @@ private AnnotationMetaAttribute createMetaCollectionAttribute(
116115
getCollectionElementType( declaredType, returnTypeName, explicitTargetEntity, context );
117116
if ( collectionElementType.getKind() == TypeKind.DECLARED ) {
118117
final TypeElement collectionElement = (TypeElement) typeUtils().asElement( collectionElementType );
119-
setAccessType( collectionElementType, NullnessUtil.castNonNull( collectionElement ) );
118+
setAccessType( collectionElementType, castNonNull( collectionElement ) );
120119
}
121120
}
122121
return createMetaAttribute( declaredType, element, collection, targetEntity );

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/util/TypeUtils.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,10 @@ public static boolean hasAnnotation(Element element, String... qualifiedNames) {
260260
return false;
261261
}
262262

263+
public static @Nullable AnnotationValue getAnnotationValue(AnnotationMirror annotationMirror) {
264+
return getAnnotationValue( annotationMirror, DEFAULT_ANNOTATION_PARAMETER_NAME );
265+
}
266+
263267
public static @Nullable AnnotationValue getAnnotationValue(AnnotationMirror annotationMirror, String member) {
264268
assert annotationMirror != null;
265269
assert member != null;
@@ -474,7 +478,7 @@ private static boolean isIdAnnotation(AnnotationMirror annotationMirror) {
474478
public static @Nullable AccessType determineAnnotationSpecifiedAccessType(Element element) {
475479
final AnnotationMirror mirror = getAnnotationMirror( element, ACCESS );
476480
if ( mirror != null ) {
477-
final AnnotationValue accessType = getAnnotationValue( mirror, DEFAULT_ANNOTATION_PARAMETER_NAME );
481+
final AnnotationValue accessType = getAnnotationValue( mirror );
478482
if ( accessType != null ) {
479483
final VariableElement enumValue = (VariableElement) accessType.getValue();
480484
final Name enumValueName = enumValue.getSimpleName();

0 commit comments

Comments
 (0)