Skip to content

Commit 007be83

Browse files
sebersolesebersole
committed
Full explicit AccessType tests
1 parent 4d88878 commit 007be83

5 files changed

Lines changed: 571 additions & 48 deletions

File tree

‎src/main/java/org/hibernate/boot/models/bind/internal/binders/ComponentBinder.java‎

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
package org.hibernate.boot.models.bind.internal.binders;
66

77
import java.util.ArrayList;
8+
import java.util.LinkedHashMap;
89
import java.util.List;
910
import java.util.function.BiConsumer;
1011
import java.util.function.BiFunction;
1112

13+
import org.hibernate.boot.models.AccessTypePlacementException;
1214
import org.hibernate.boot.models.bind.internal.sources.BasicValueSource;
1315
import org.hibernate.boot.models.bind.internal.sources.ColumnSource;
1416
import org.hibernate.boot.models.bind.internal.sources.ComponentSource;
@@ -23,10 +25,15 @@
2325
import org.hibernate.mapping.Property;
2426
import org.hibernate.mapping.Table;
2527
import org.hibernate.models.spi.ClassDetails;
28+
import org.hibernate.models.spi.FieldDetails;
2629
import org.hibernate.models.spi.MemberDetails;
30+
import org.hibernate.models.spi.MethodDetails;
2731

32+
import jakarta.persistence.Access;
33+
import jakarta.persistence.AccessType;
2834
import jakarta.persistence.AssociationOverride;
2935
import jakarta.persistence.Convert;
36+
import jakarta.persistence.Transient;
3037

3138
/// Shared support for binding component-valued mappings.
3239
///
@@ -74,6 +81,7 @@ List<Column> bindBasicProperties(
7481
component,
7582
table,
7683
"",
84+
determineComponentAccessType( source.componentType(), ownerType.getAccessType() ),
7785
source::columnSource,
7886
source::conversion,
7987
(path, member) -> source.associationOverride( path ),
@@ -104,6 +112,7 @@ List<Column> bindBasicProperties(
104112
component,
105113
table,
106114
"",
115+
determineComponentAccessType( componentType, ownerType.getAccessType() ),
107116
columnSourceResolver,
108117
conversionResolver,
109118
associationOverrideResolver,
@@ -121,6 +130,7 @@ private List<Column> bindProperties(
121130
Component component,
122131
Table table,
123132
String pathPrefix,
133+
AccessType accessType,
124134
BiFunction<String, MemberDetails, ColumnSource> columnSourceResolver,
125135
BiFunction<String, MemberDetails, Convert> conversionResolver,
126136
BiFunction<String, MemberDetails, AssociationOverride> associationOverrideResolver,
@@ -129,7 +139,7 @@ private List<Column> bindProperties(
129139
boolean nullableByDefault,
130140
boolean updatable) {
131141
final List<Column> columns = new ArrayList<>();
132-
componentType.forEachPersistableMember( (member) -> {
142+
for ( MemberDetails member : resolveComponentMembers( componentType, accessType ) ) {
133143
validateMember( member );
134144
final String attributeName = member.resolveAttributeName();
135145
final String memberPath = pathPrefix + attributeName;
@@ -154,7 +164,7 @@ private List<Column> bindProperties(
154164
property.setValue( manyToOne );
155165
component.addProperty( property );
156166
columns.addAll( manyToOne.getColumns() );
157-
return;
167+
continue;
158168
}
159169

160170
if ( isEmbeddedMember( member ) ) {
@@ -173,6 +183,7 @@ private List<Column> bindProperties(
173183
nestedComponent,
174184
table,
175185
memberPath + ".",
186+
determineComponentAccessType( member.getType().determineRawClass(), accessType ),
176187
columnSourceResolver,
177188
conversionResolver,
178189
associationOverrideResolver,
@@ -181,7 +192,7 @@ private List<Column> bindProperties(
181192
nullableByDefault,
182193
updatable
183194
) );
184-
return;
195+
continue;
185196
}
186197

187198
final BasicValue basicValue = createBasicValue(
@@ -202,10 +213,78 @@ private List<Column> bindProperties(
202213
);
203214
columnConsumer.accept( member, column );
204215
columns.add( column );
205-
} );
216+
}
206217
return columns;
207218
}
208219

220+
private AccessType determineComponentAccessType(ClassDetails componentType, AccessType containingAccessType) {
221+
final Access access = componentType.getDirectAnnotationUsage( Access.class );
222+
return access == null ? containingAccessType : access.value();
223+
}
224+
225+
private List<MemberDetails> resolveComponentMembers(ClassDetails componentType, AccessType accessType) {
226+
final LinkedHashMap<String, MemberDetails> results = new LinkedHashMap<>();
227+
228+
for ( FieldDetails field : componentType.getFields() ) {
229+
final Access access = field.getDirectAnnotationUsage( Access.class );
230+
if ( access == null ) {
231+
continue;
232+
}
233+
validateAttributeLevelAccess( componentType, field, access.value() );
234+
if ( !isTransient( field ) ) {
235+
results.put( field.resolveAttributeName(), field );
236+
}
237+
}
238+
239+
for ( MethodDetails method : componentType.getMethods() ) {
240+
final Access access = method.getDirectAnnotationUsage( Access.class );
241+
if ( access == null ) {
242+
continue;
243+
}
244+
validateAttributeLevelAccess( componentType, method, access.value() );
245+
if ( !isTransient( method ) ) {
246+
results.put( method.resolveAttributeName(), method );
247+
}
248+
}
249+
250+
if ( accessType == AccessType.FIELD ) {
251+
for ( FieldDetails field : componentType.getFields() ) {
252+
if ( field.isPersistable()
253+
&& !isTransient( field )
254+
&& !results.containsKey( field.resolveAttributeName() ) ) {
255+
results.put( field.resolveAttributeName(), field );
256+
}
257+
}
258+
}
259+
else {
260+
for ( MethodDetails method : componentType.getMethods() ) {
261+
if ( method.isPersistable()
262+
&& !isTransient( method )
263+
&& !results.containsKey( method.resolveAttributeName() ) ) {
264+
results.put( method.resolveAttributeName(), method );
265+
}
266+
}
267+
}
268+
269+
return new ArrayList<>( results.values() );
270+
}
271+
272+
private boolean isTransient(MemberDetails member) {
273+
return member.hasDirectAnnotationUsage( Transient.class );
274+
}
275+
276+
private void validateAttributeLevelAccess(
277+
ClassDetails componentType,
278+
MemberDetails member,
279+
AccessType attributeAccessType) {
280+
if ( ( attributeAccessType == AccessType.FIELD && !member.isField() )
281+
|| ( attributeAccessType == AccessType.PROPERTY && member.isField() )
282+
|| ( attributeAccessType == AccessType.PROPERTY
283+
&& member.asMethodDetails().getMethodKind() != MethodDetails.MethodKind.GETTER ) ) {
284+
throw new AccessTypePlacementException( componentType, member );
285+
}
286+
}
287+
209288
private void validateMember(MemberDetails member) {
210289
if ( member.isPlural()
211290
|| member.hasDirectAnnotationUsage( jakarta.persistence.OneToMany.class )

‎src/main/java/org/hibernate/boot/models/categorize/internal/CategorizationHelper.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import jakarta.persistence.Access;
88
import jakarta.persistence.Basic;
9+
import jakarta.persistence.Column;
910
import jakarta.persistence.Convert;
1011
import jakarta.persistence.ElementCollection;
1112
import jakarta.persistence.Embeddable;
@@ -83,13 +84,17 @@ public static boolean isDefaultAccessTypeIndicator(MemberDetails memberDetails)
8384
return false;
8485
}
8586

86-
// todo : add a method to cleanly iterate (not Consumer-based) annotations to OrmAnnotationHelper
87-
// to implement the fully correct approach to look for any mapping annotation.
88-
// NOTE: when we do this, be sure to distinguish actual mapping annotations; e.g. @Basic, but not @PostPersist
89-
90-
// for now, do the legacy bit and just look for @Id and @EmbeddedId
9187
return memberDetails.hasDirectAnnotationUsage( Id.class )
92-
|| memberDetails.hasDirectAnnotationUsage( EmbeddedId.class );
88+
|| memberDetails.hasDirectAnnotationUsage( EmbeddedId.class )
89+
|| memberDetails.hasDirectAnnotationUsage( Basic.class )
90+
|| memberDetails.hasDirectAnnotationUsage( Version.class )
91+
|| memberDetails.hasDirectAnnotationUsage( Embedded.class )
92+
|| memberDetails.hasDirectAnnotationUsage( ElementCollection.class )
93+
|| memberDetails.hasDirectAnnotationUsage( ManyToOne.class )
94+
|| memberDetails.hasDirectAnnotationUsage( OneToOne.class )
95+
|| memberDetails.hasDirectAnnotationUsage( OneToMany.class )
96+
|| memberDetails.hasDirectAnnotationUsage( ManyToMany.class )
97+
|| memberDetails.hasDirectAnnotationUsage( Column.class );
9398
}
9499

95100
/// Determine the attribute's nature - is it a basic mapping, an embeddable, ...?

‎src/main/java/org/hibernate/boot/models/categorize/internal/EntityHierarchyBuilder.java‎

Lines changed: 67 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@
66

77
import jakarta.persistence.Access;
88
import jakarta.persistence.AccessType;
9-
import jakarta.persistence.EmbeddedId;
109
import jakarta.persistence.Entity;
11-
import jakarta.persistence.Id;
1210
import org.checkerframework.checker.nullness.qual.NonNull;
1311
import org.hibernate.boot.models.AccessTypeDeterminationException;
1412
import org.hibernate.boot.models.JpaAnnotations;
@@ -22,6 +20,7 @@
2220
import org.hibernate.models.spi.MemberDetails;
2321
import org.hibernate.models.spi.MethodDetails;
2422

23+
import java.util.HashSet;
2524
import java.util.List;
2625
import java.util.Set;
2726

@@ -52,7 +51,7 @@ private Set<EntityHierarchy> process(
5251
final Set<EntityHierarchy> hierarchies = CollectionHelper.setOfSize( rootEntities.size() );
5352

5453
rootEntities.forEach( (rootEntity) -> {
55-
final AccessType defaultAccessType = determineDefaultAccessTypeForHierarchy( rootEntity );
54+
final AccessType defaultAccessType = determineDefaultAccessTypeForHierarchy( rootEntity, inheritanceState );
5655
hierarchies.add( new EntityHierarchyImpl(
5756
rootEntity,
5857
defaultAccessType,
@@ -77,62 +76,92 @@ private Set<EntityHierarchy> process(ManagedTypeInheritanceState inheritanceStat
7776
}
7877

7978
@NonNull
80-
private AccessType determineDefaultAccessTypeForHierarchy(ClassDetails rootEntityType) {
79+
private AccessType determineDefaultAccessTypeForHierarchy(
80+
ClassDetails rootEntityType,
81+
ManagedTypeInheritanceState inheritanceState) {
8182
assert rootEntityType != null;
8283

84+
final AccessType[] result = new AccessType[1];
85+
final Set<ClassDetails> visited = new HashSet<>();
86+
8387
ClassDetails current = rootEntityType;
8488
while ( current != null ) {
85-
// look for `@Access` on the class
86-
final Access accessAnnotation = current.getDirectAnnotationUsage( JpaAnnotations.ACCESS );
87-
if ( accessAnnotation == null ) {
88-
var inclusiveMember = findDefaultedMember( current );
89-
if ( inclusiveMember == null ) {
90-
current = current.getSuperClass();
91-
continue;
92-
}
93-
94-
if ( inclusiveMember.getKind() == AnnotationTarget.Kind.FIELD ) {
95-
return AccessType.FIELD;
96-
}
97-
else if ( inclusiveMember.getKind() == AnnotationTarget.Kind.METHOD
98-
&& inclusiveMember.asMethodDetails().getMethodKind() == MethodDetails.MethodKind.GETTER ) {
99-
return AccessType.PROPERTY;
100-
}
101-
else {
102-
// this should never happen because of the nature of the checks in findDefaultedMember()...
103-
throw new AccessTypeDeterminationException( rootEntityType );
104-
}
105-
}
106-
89+
applyDefaultedAccessType( rootEntityType, current, result, visited );
10790
current = current.getSuperClass();
10891
}
10992

110-
return modelContext.getEffectiveMappingDefaults().getDefaultPropertyAccessType();
93+
applyDefaultedAccessTypesFromSubTypes( rootEntityType, rootEntityType, inheritanceState, result, visited );
94+
95+
return result[0] == null
96+
? modelContext.getEffectiveMappingDefaults().getDefaultPropertyAccessType()
97+
: result[0];
11198
}
11299

113-
protected MemberDetails findDefaultedMember(ClassDetails current) {
114-
// For now, keep using the old approach of looking for id.
115-
// But ultimately we may want to pivot away to a more JPA way
116-
// looking for any attribute without `@Access` (identifiers could
117-
// have `@Access` which should in theory exclude them from consideration).
118-
return determineIdMember( current );
100+
private void applyDefaultedAccessTypesFromSubTypes(
101+
ClassDetails rootEntityType,
102+
ClassDetails current,
103+
ManagedTypeInheritanceState inheritanceState,
104+
AccessType[] result,
105+
Set<ClassDetails> visited) {
106+
inheritanceState.forEachSubType( current, (subType) -> {
107+
applyDefaultedAccessType( rootEntityType, subType, result, visited );
108+
applyDefaultedAccessTypesFromSubTypes( rootEntityType, subType, inheritanceState, result, visited );
109+
} );
119110
}
120111

121-
private MemberDetails determineIdMember(ClassDetails current) {
112+
private void applyDefaultedAccessType(
113+
ClassDetails rootEntityType,
114+
ClassDetails current,
115+
AccessType[] result,
116+
Set<ClassDetails> visited) {
117+
if ( !visited.add( current ) ) {
118+
return;
119+
}
120+
121+
final Access accessAnnotation = current.getDirectAnnotationUsage( JpaAnnotations.ACCESS );
122+
if ( accessAnnotation != null ) {
123+
return;
124+
}
125+
126+
final MemberDetails defaultedMember = findDefaultedMember( current );
127+
if ( defaultedMember == null ) {
128+
return;
129+
}
130+
131+
final AccessType memberAccessType = determineAccessType( rootEntityType, defaultedMember );
132+
if ( result[0] == null ) {
133+
result[0] = memberAccessType;
134+
}
135+
else if ( result[0] != memberAccessType ) {
136+
throw new AccessTypeDeterminationException( rootEntityType );
137+
}
138+
}
139+
140+
private AccessType determineAccessType(ClassDetails rootEntityType, MemberDetails memberDetails) {
141+
if ( memberDetails.getKind() == AnnotationTarget.Kind.FIELD ) {
142+
return AccessType.FIELD;
143+
}
144+
else if ( memberDetails.getKind() == AnnotationTarget.Kind.METHOD
145+
&& memberDetails.asMethodDetails().getMethodKind() == MethodDetails.MethodKind.GETTER ) {
146+
return AccessType.PROPERTY;
147+
}
148+
149+
throw new AccessTypeDeterminationException( rootEntityType );
150+
}
151+
152+
protected MemberDetails findDefaultedMember(ClassDetails current) {
122153
final List<MethodDetails> methods = current.getMethods();
123154
for ( int i = 0; i < methods.size(); i++ ) {
124155
final MethodDetails methodDetails = methods.get( i );
125-
if ( methodDetails.hasDirectAnnotationUsage( Id.class )
126-
|| methodDetails.hasDirectAnnotationUsage( EmbeddedId.class ) ) {
156+
if ( CategorizationHelper.isDefaultAccessTypeIndicator( methodDetails ) ) {
127157
return methodDetails;
128158
}
129159
}
130160

131161
final List<FieldDetails> fields = current.getFields();
132162
for ( int i = 0; i < fields.size(); i++ ) {
133163
final FieldDetails fieldDetails = fields.get( i );
134-
if ( fieldDetails.hasDirectAnnotationUsage( Id.class )
135-
|| fieldDetails.hasDirectAnnotationUsage( EmbeddedId.class ) ) {
164+
if ( CategorizationHelper.isDefaultAccessTypeIndicator( fieldDetails ) ) {
136165
return fieldDetails;
137166
}
138167
}

‎src/main/java/org/hibernate/boot/models/categorize/internal/StandardPersistentAttributeMemberResolver.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,18 @@ private void validateAttributeLevelAccess(
121121
// Mainly, it is never legal to:
122122
// 1. specify @Access(FIELD) on a getter
123123
// 2. specify @Access(PROPERTY) on a field
124+
//
125+
// Well, technically the spec says the behavior is "undefined"; but Hibernate does not support it, so make that obvious
124126

125127
if ( ( attributeAccessType == AccessType.FIELD && !annotationTarget.isField() )
126128
|| ( attributeAccessType == AccessType.PROPERTY && annotationTarget.isField() ) ) {
127129
throw new AccessTypePlacementException( classDetails, annotationTarget );
128130
}
131+
132+
if ( attributeAccessType == AccessType.PROPERTY
133+
&& annotationTarget.asMethodDetails().getMethodKind() != MethodDetails.MethodKind.GETTER ) {
134+
throw new AccessTypePlacementException( classDetails, annotationTarget );
135+
}
129136
}
130137

131138
private void processClassLevelAccess(

0 commit comments

Comments
 (0)