Skip to content

Commit e6b7943

Browse files
gavinkinggavinking
committed
clean up the @mapsid inference ("specj") stuff
1 parent 3f4b2ef commit e6b7943

21 files changed

+113
-109
lines changed

‎hibernate-core/src/main/java/org/hibernate/boot/MetadataBuilder.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55
package org.hibernate.boot;
66

7+
import org.hibernate.Incubating;
78
import org.hibernate.boot.archive.scan.spi.ScanEnvironment;
89
import org.hibernate.boot.archive.scan.spi.ScanOptions;
910
import org.hibernate.boot.archive.scan.spi.Scanner;
@@ -275,6 +276,19 @@ public interface MetadataBuilder {
275276
*/
276277
MetadataBuilder enableGlobalNationalizedCharacterDataSupport(boolean enabled);
277278

279+
/**
280+
* Specify whether missing {@link jakarta.persistence.MapsId} annotations
281+
* should be inferred.
282+
*
283+
* @param enabled {@code true} if missing {@code MapsId} should be inferred
284+
*
285+
* @return {@code this}, for method chaining
286+
*
287+
* @since 7.0
288+
*/
289+
@Incubating
290+
MetadataBuilder enableMapsIdInference(boolean enabled);
291+
278292
/**
279293
* Specify an additional or overridden basic type mapping.
280294
*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1373,7 +1373,7 @@ public void addPropertyAnnotatedWithMapsId(ClassDetails entityType, PropertyData
13731373
}
13741374

13751375
@Override
1376-
public void addPropertyAnnotatedWithMapsIdSpecj(ClassDetails entityType, PropertyData property, String mapsIdValue) {
1376+
public void addInferredMapsIdProperty(ClassDetails entityType, PropertyData property, String mapsIdValue) {
13771377
if ( propertiesAnnotatedWithMapsId == null ) {
13781378
propertiesAnnotatedWithMapsId = new HashMap<>();
13791379
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -343,8 +343,8 @@ public MetadataBuilder applyTempClassLoader(ClassLoader tempClassLoader) {
343343
return this;
344344
}
345345

346-
public MetadataBuilder allowSpecjSyntax() {
347-
options.specjProprietarySyntaxEnabled = true;
346+
public MetadataBuilder enableMapsIdInference(boolean enabled) {
347+
options.mapsIdInferenceEnabled = enabled;
348348
return this;
349349
}
350350

@@ -644,7 +644,7 @@ public static class MetadataBuildingOptionsImpl
644644
private boolean implicitDiscriminatorsForJoinedInheritanceSupported;
645645
private boolean implicitlyForceDiscriminatorInSelect;
646646
private boolean useNationalizedCharacterData;
647-
private boolean specjProprietarySyntaxEnabled;
647+
private boolean mapsIdInferenceEnabled;
648648
private boolean noConstraintByDefault;
649649

650650
private final String schemaCharset;
@@ -735,8 +735,8 @@ else if ( value instanceof AccessType accessType ) {
735735
regionFactory == null ? null : regionFactory.getDefaultAccessType()
736736
);
737737

738-
specjProprietarySyntaxEnabled = configService.getSetting(
739-
"hibernate.enable_specj_proprietary_syntax",
738+
mapsIdInferenceEnabled = configService.getSetting(
739+
"hibernate.enable_mapsid_inference",
740740
BOOLEAN,
741741
false
742742
);
@@ -920,8 +920,8 @@ public boolean useNationalizedCharacterData() {
920920
}
921921

922922
@Override
923-
public boolean isSpecjProprietarySyntaxEnabled() {
924-
return specjProprietarySyntaxEnabled;
923+
public boolean isMapsIdInferenceEnabled() {
924+
return mapsIdInferenceEnabled;
925925
}
926926

927927
@Override

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ private static int addProperty(
623623
final MemberDetails element = propertyAnnotatedElement.getAttributeMember();
624624
if ( hasIdAnnotation( element ) ) {
625625
inFlightPropertyDataList.add( idPropertyCounter, propertyAnnotatedElement );
626-
handleIdProperty( propertyContainer, context, declaringClass, ownerType, element );
626+
handleInferredMapsIdProperty( propertyContainer, context, declaringClass, ownerType, element );
627627
if ( hasToOneAnnotation( element ) ) {
628628
context.getMetadataCollector()
629629
.addToOneAndIdProperty( ownerType.determineRawClass(), propertyAnnotatedElement );
@@ -653,31 +653,40 @@ private static void checkIdProperty(MemberDetails property, PropertyData propert
653653
}
654654
}
655655

656-
private static void handleIdProperty(
656+
// The following code infers a "missing" @MapsId annotation when
657+
// an @Id Column matches a @JoinColumn of another field. No test
658+
// fails if I simply remove this code, and, indeed, it was broken
659+
// and doing nothing before I got here. I've now "fixed" it to do
660+
// what it was supposed to be doing, but honestly the semantics
661+
// aren't clear: why should it be linked to the existence of an
662+
// explicit @Column annotation? And there's still no test for it.
663+
//
664+
// The real work is done by ToOneBinder#handleInferredMapsId
665+
private static void handleInferredMapsIdProperty(
657666
PropertyContainer propertyContainer,
658667
MetadataBuildingContext context,
659668
ClassDetails declaringClass,
660669
TypeVariableScope ownerType,
661670
MemberDetails element) {
662-
// The property must be put in hibernate.properties as it's a system wide property. Fixable?
663-
//TODO support true/false/default on the property instead of present / not present
664-
//TODO is @Column mandatory?
665-
//TODO add method support
666-
final SourceModelBuildingContext sourceModelContext = context.getMetadataCollector().getSourceModelBuildingContext();
667-
if ( context.getBuildingOptions().isSpecjProprietarySyntaxEnabled() ) {
668-
if ( element.hasDirectAnnotationUsage( Id.class ) && element.hasDirectAnnotationUsage( Column.class ) ) {
671+
if ( context.getBuildingOptions().isMapsIdInferenceEnabled() ) {
672+
//TODO support true/false/default on the property instead of present / not present
673+
final SourceModelBuildingContext sourceModelContext =
674+
context.getMetadataCollector().getSourceModelBuildingContext();
675+
if ( element.hasDirectAnnotationUsage( Id.class )
676+
//TODO Explicit @Column should not be mandatory here
677+
&& element.hasDirectAnnotationUsage( Column.class ) ) {
669678
final String columnName = element.getDirectAnnotationUsage( Column.class ).name();
670-
declaringClass.forEachField( (index, fieldDetails) -> {
671-
if ( !element.hasDirectAnnotationUsage( MapsId.class )
672-
&& isJoinColumnPresent( columnName, element, sourceModelContext ) ) {
679+
declaringClass.forEachPersistableMember( memberDetails -> {
680+
if ( !memberDetails.hasDirectAnnotationUsage( MapsId.class )
681+
&& isJoinColumnPresent( columnName, memberDetails, sourceModelContext ) ) {
673682
//create a PropertyData for the specJ property holding the mapping
674-
context.getMetadataCollector().addPropertyAnnotatedWithMapsIdSpecj(
683+
context.getMetadataCollector().addInferredMapsIdProperty(
675684
ownerType.determineRawClass(),
676685
new PropertyInferredData(
677686
declaringClass,
678687
ownerType,
679688
//same dec
680-
element,
689+
memberDetails,
681690
// the actual @XToOne property
682691
propertyContainer.getClassLevelAccessType().getType(),
683692
//TODO we should get the right accessor but the same as id would do
@@ -1389,7 +1398,7 @@ private static PropertyData getPropertyOverriddenByMapperOrMapsId(
13891398
return data;
13901399
}
13911400
// TODO: is this branch even necessary?
1392-
else if ( buildingContext.getBuildingOptions().isSpecjProprietarySyntaxEnabled() ) {
1401+
else {
13931402
return metadataCollector.getPropertyAnnotatedWithMapsId( classDetails, propertyName );
13941403
}
13951404
}

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

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ static void bindManyToOne(
8989
);
9090
}
9191

92-
if ( joinColumns.hasMappedBy() && isIdentifier( propertyHolder, propertyBinder, isIdentifierMapper ) ) {
92+
if ( joinColumns.hasMappedBy()
93+
&& isIdentifier( propertyHolder, propertyBinder, isIdentifierMapper ) ) {
9394
throw new AnnotationException(
9495
"Property '" + getPath( propertyHolder, inferredData )
9596
+ "' is the inverse side of a '@ManyToOne' association and cannot be used as identifier"
@@ -213,7 +214,7 @@ private static void bindManyToOne(
213214
joinColumns.setMapsId( mapsId.value() );
214215
}
215216

216-
final boolean hasSpecjManyToOne = handleSpecjSyntax( joinColumns, inferredData, context, property );
217+
final boolean hasInferredMapsId = handleInferredMapsId( joinColumns, inferredData, property, context );
217218
value.setTypeName( inferredData.getClassOrElementName() );
218219
final String propertyName = inferredData.getPropertyName();
219220
value.setTypeUsingReflection( propertyHolder.getClassName(), propertyName );
@@ -243,11 +244,10 @@ private static void bindManyToOne(
243244
joinColumns,
244245
optional,
245246
inferredData,
246-
isIdentifierMapper,
247+
isIdentifierMapper || hasInferredMapsId,
247248
propertyBinder,
248249
value,
249250
property,
250-
hasSpecjManyToOne,
251251
propertyName
252252
);
253253
}
@@ -257,36 +257,46 @@ static boolean isTargetAnnotatedEntity(ClassDetails targetEntity, MemberDetails
257257
return target.hasDirectAnnotationUsage( Entity.class );
258258
}
259259

260-
private static boolean handleSpecjSyntax(
260+
// The following code infers a "missing" @MapsId annotation
261+
// when an @Id Column matches a @JoinColumn of another field.
262+
// There's also some related code for this case over in
263+
// PropertyBinder#handleInferredMapsIdProperty
264+
private static boolean handleInferredMapsId(
261265
AnnotatedJoinColumns columns,
262-
PropertyData inferredData,
263-
MetadataBuildingContext context,
264-
MemberDetails property) {
265-
//Make sure that JPA1 key-many-to-one columns are read only too
266-
boolean hasSpecjManyToOne = false;
267-
if ( context.getBuildingOptions().isSpecjProprietarySyntaxEnabled() ) {
266+
PropertyData propertyData,
267+
MemberDetails property,
268+
MetadataBuildingContext context) {
269+
if ( context.getBuildingOptions().isMapsIdInferenceEnabled() ) {
270+
//Make sure that JPA1 key-many-to-one columns are read only too
271+
boolean hasInferredMapsId = false;
268272
final JoinColumn joinColumn = property.getDirectAnnotationUsage( JoinColumn.class );
269-
String columnName = "";
270-
for ( MemberDetails prop : inferredData.getDeclaringClass().getFields() ) {
271-
if ( prop.hasDirectAnnotationUsage( Id.class ) && prop.hasDirectAnnotationUsage( Column.class ) ) {
272-
columnName = prop.getDirectAnnotationUsage( Column.class ).name();
273-
}
274-
275-
if ( property.hasDirectAnnotationUsage( ManyToOne.class ) && joinColumn != null ) {
276-
final String joinColumnName = joinColumn.name();
277-
if ( isNotBlank( joinColumnName )
278-
&& joinColumnName.equals( columnName )
279-
&& !property.hasDirectAnnotationUsage( MapsId.class ) ) {
280-
hasSpecjManyToOne = true;
281-
for ( AnnotatedJoinColumn column : columns.getJoinColumns() ) {
282-
column.setInsertable( false );
283-
column.setUpdatable( false );
273+
if ( joinColumn != null
274+
&& property.hasDirectAnnotationUsage( ManyToOne.class )
275+
&& !property.hasDirectAnnotationUsage( MapsId.class ) ) {
276+
final String joinColumnName = joinColumn.name();
277+
if ( isNotBlank( joinColumnName ) ) {
278+
for ( MemberDetails member : propertyData.getDeclaringClass().getFields() ) {
279+
if ( member.hasDirectAnnotationUsage( Id.class )
280+
//TODO Explicit @Column should not be mandatory here
281+
&& member.hasDirectAnnotationUsage( Column.class ) ) {
282+
final String columnName = member.getDirectAnnotationUsage( Column.class ).name();
283+
if ( joinColumnName.equals( columnName ) ) {
284+
hasInferredMapsId = true;
285+
for ( AnnotatedJoinColumn column : columns.getJoinColumns() ) {
286+
column.setInsertable( false );
287+
column.setUpdatable( false );
288+
}
289+
}
284290
}
285291
}
286292
}
293+
287294
}
295+
return hasInferredMapsId;
296+
}
297+
else {
298+
return false;
288299
}
289-
return hasSpecjManyToOne;
290300
}
291301

292302
private static void processManyToOneProperty(
@@ -298,7 +308,6 @@ private static void processManyToOneProperty(
298308
PropertyBinder propertyBinder,
299309
org.hibernate.mapping.ManyToOne value,
300310
MemberDetails property,
301-
boolean hasSpecjManyToOne,
302311
String propertyName) {
303312

304313
columns.checkPropertyConsistency();
@@ -311,10 +320,6 @@ private static void processManyToOneProperty(
311320
propertyBinder.setInsertable( false );
312321
propertyBinder.setUpdatable( false );
313322
}
314-
else if ( hasSpecjManyToOne ) {
315-
propertyBinder.setInsertable( false );
316-
propertyBinder.setUpdatable( false );
317-
}
318323
propertyBinder.setColumns( columns );
319324
propertyBinder.setAccessType( inferredData.getDefaultAccess() );
320325
propertyBinder.setCascade( cascadeStrategy );
@@ -323,24 +328,25 @@ else if ( hasSpecjManyToOne ) {
323328

324329
final JoinColumn joinColumn = property.getDirectAnnotationUsage( JoinColumn.class );
325330
final JoinColumns joinColumns = property.getDirectAnnotationUsage( JoinColumns.class );
326-
propertyBinder.makePropertyAndBind().setOptional( optional && isNullable( joinColumns, joinColumn ) );
331+
propertyBinder.makePropertyAndBind()
332+
.setOptional( optional && isNullable( joinColumns, joinColumn ) );
327333
}
328334

329335
private static boolean isNullable(JoinColumns joinColumns, JoinColumn joinColumn) {
330336
if ( joinColumn != null ) {
331337
return joinColumn.nullable();
332338
}
333-
334-
if ( joinColumns != null ) {
339+
else if ( joinColumns != null ) {
335340
for ( JoinColumn column : joinColumns.value() ) {
336341
if ( column.nullable() ) {
337342
return true;
338343
}
339344
}
340345
return false;
341346
}
342-
343-
return true;
347+
else {
348+
return true;
349+
}
344350
}
345351

346352
static void defineFetchingStrategy(

‎hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/IdentifierSourceAggregatedCompositeImpl.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,6 @@ public List<MapsIdSource> getMapsIdSources() {
6464
return Collections.emptyList();
6565
}
6666

67-
@Override
68-
public IdentifierGeneratorDefinition getIndividualAttributeIdGenerator(String identifierAttributeName) {
69-
// for now, return null. this is that stupid specj bs
70-
return null;
71-
}
72-
7367
@Override
7468
public IdentifierGeneratorDefinition getIdentifierGeneratorDescriptor() {
7569
return generatorDefinition;

‎hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/IdentifierSourceNonAggregatedCompositeImpl.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,6 @@ public EmbeddableSource getIdClassSource() {
122122
return idClassSource;
123123
}
124124

125-
@Override
126-
public IdentifierGeneratorDefinition getIndividualAttributeIdGenerator(String identifierAttributeName) {
127-
// for now, return null. this is that stupid specj bs
128-
return null;
129-
}
130-
131125
@Override
132126
public IdentifierGeneratorDefinition getIdentifierGeneratorDescriptor() {
133127
return generatorDefinition;

‎hibernate-core/src/main/java/org/hibernate/boot/model/source/spi/CompositeIdentifierSource.java

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
*/
55
package org.hibernate.boot.model.source.spi;
66

7-
import org.hibernate.boot.model.IdentifierGeneratorDefinition;
8-
97
/**
108
* Common contract for composite identifiers. Specific subtypes include aggregated
119
* (think {@link jakarta.persistence.EmbeddedId}) and non-aggregated (think
@@ -14,13 +12,4 @@
1412
* @author Steve Ebersole
1513
*/
1614
public interface CompositeIdentifierSource extends IdentifierSource, EmbeddableSourceContributor {
17-
/**
18-
* Handle silly SpecJ reading of the JPA spec. They believe composite identifiers should have "partial generation"
19-
* capabilities.
20-
*
21-
* @param identifierAttributeName The name of the individual attribute within the composite identifier.
22-
*
23-
* @return The generator for the named attribute (within the composite).
24-
*/
25-
IdentifierGeneratorDefinition getIndividualAttributeIdGenerator(String identifierAttributeName);
2615
}

‎hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingMetadataBuilderImplementor.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,12 @@ public MetadataBuilder enableGlobalNationalizedCharacterDataSupport(boolean enab
149149
return getThis();
150150
}
151151

152+
@Override
153+
public MetadataBuilder enableMapsIdInference(boolean enabled) {
154+
delegate.enableMapsIdInference( enabled );
155+
return getThis();
156+
}
157+
152158
@Override
153159
public MetadataBuilder applyBasicType(BasicType<?> type) {
154160
delegate.applyBasicType( type );

‎hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingMetadataBuildingOptions.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ public boolean useNationalizedCharacterData() {
131131
}
132132

133133
@Override
134-
public boolean isSpecjProprietarySyntaxEnabled() {
135-
return delegate.isSpecjProprietarySyntaxEnabled();
134+
public boolean isMapsIdInferenceEnabled() {
135+
return delegate.isMapsIdInferenceEnabled();
136136
}
137137

138138
@Override

0 commit comments

Comments
 (0)