Skip to content

HHH-14519 avoid blowing up when annotation values are blank strings #9324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 25, 2024
Merged

HHH-14519 avoid blowing up when annotation values are blank strings #9324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6329fdb
HHH-14519 improve error message
gavinking Nov 24, 2024
3c6c76f
some minor cleanups in InFlightMetadataCollectorImpl
gavinking Nov 24, 2024
c5d6546
fix capitalization in a title
gavinking Nov 24, 2024
683d807
use isBlank() instead of isEmpty() to better handle blank annotation …
gavinking Nov 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion documentation/src/main/asciidoc/introduction/Interacting.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Stateful sessions certainly have their advantages, but they're more difficult to
====

[[persistence-contexts]]
=== Persistence Contexts
=== Persistence contexts

A persistence context is a sort of cache; we sometimes call it the "first-level cache", to distinguish it from the <<second-level-cache,second-level cache>>.
For every entity instance read from the database within the scope of a persistence context, and for every new entity made persistent within the scope of the persistence context, the context holds a unique mapping from the identifier of the entity instance to the instance itself.
Expand Down
150 changes: 68 additions & 82 deletions hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,13 @@ public Class<? extends UserType<?>> findRegisteredUserType(Class<?> basicType) {

@Override
public void addCollectionTypeRegistration(CollectionTypeRegistration registrationAnnotation) {
addCollectionTypeRegistration(
registrationAnnotation.classification(),
toDescriptor( registrationAnnotation )
);
addCollectionTypeRegistration( registrationAnnotation.classification(),
toDescriptor( registrationAnnotation ) );
}

@Override
public void addCollectionTypeRegistration(CollectionClassification classification, CollectionTypeRegistrationDescriptor descriptor) {
public void addCollectionTypeRegistration(
CollectionClassification classification, CollectionTypeRegistrationDescriptor descriptor) {
if ( collectionTypeRegistrations == null ) {
collectionTypeRegistrations = new HashMap<>();
}
Expand All @@ -558,18 +557,13 @@ public void addCollectionTypeRegistration(CollectionClassification classificatio

@Override
public CollectionTypeRegistrationDescriptor findCollectionTypeRegistration(CollectionClassification classification) {
if ( collectionTypeRegistrations == null ) {
return null;
}
return collectionTypeRegistrations == null ? null : collectionTypeRegistrations.get( classification );

return collectionTypeRegistrations.get( classification );
}

private CollectionTypeRegistrationDescriptor toDescriptor(CollectionTypeRegistration registrationAnnotation) {
return new CollectionTypeRegistrationDescriptor(
registrationAnnotation.type(),
extractParameters( registrationAnnotation.parameters() )
);
return new CollectionTypeRegistrationDescriptor( registrationAnnotation.type(),
extractParameters( registrationAnnotation.parameters() ) );
}

private Map<String,String> extractParameters(Parameter[] annotationUsages) {
Expand Down Expand Up @@ -675,7 +669,7 @@ public void addFetchProfile(FetchProfile profile) {
if ( profile == null || profile.getName() == null ) {
throw new IllegalArgumentException( "Fetch profile object or name is null: " + profile );
}
FetchProfile old = fetchProfileMap.put( profile.getName(), profile );
final FetchProfile old = fetchProfileMap.put( profile.getName(), profile );
if ( old != null ) {
log.warn( "Duplicated fetch profile with same name [" + profile.getName() + "] found." );
}
Expand Down Expand Up @@ -966,16 +960,11 @@ public Table addTable(

// annotation binding depends on the "table name" for @Subselect bindings
// being set into the generated table (mainly to avoid later NPE), but for now we need to keep that :(
final Identifier logicalName;
if ( name != null ) {
logicalName = getDatabase().toIdentifier( name );
}
else {
logicalName = null;
}
final Identifier logicalName = name != null ? getDatabase().toIdentifier( name ) : null;

if ( subselectFragment != null ) {
return new Table( buildingContext.getCurrentContributorName(), namespace, logicalName, subselectFragment, isAbstract );
return new Table( buildingContext.getCurrentContributorName(),
namespace, logicalName, subselectFragment, isAbstract );
}
else {
final Table existing = namespace.locateTable( logicalName );
Expand All @@ -988,7 +977,9 @@ public Table addTable(

return namespace.createTable(
logicalName,
(physicalName) -> new Table( buildingContext.getCurrentContributorName(), namespace, physicalName, isAbstract )
(physicalName) ->
new Table( buildingContext.getCurrentContributorName(),
namespace, physicalName, isAbstract )
);
}
}
Expand Down Expand Up @@ -1216,7 +1207,16 @@ public void addColumnNameBinding(Table table, Identifier logicalName, Column col

@Override
public String getPhysicalColumnName(Table table, String logicalName) throws MappingException {
return getPhysicalColumnName( table, getDatabase().toIdentifier( logicalName ) );
final Identifier identifier = getDatabase().toIdentifier( logicalName );
if ( identifier == null ) {
throw new MappingException( String.format(
Locale.ENGLISH,
"Column with logical name '%s' in table '%s' cannot be mapped to column identifier",
logicalName,
table.getName()
) );
}
return getPhysicalColumnName( table, identifier );
}

@Override
Expand Down Expand Up @@ -1370,14 +1370,8 @@ public void addPropertyAnnotatedWithMapsId(ClassDetails entityType, PropertyData
propertiesAnnotatedWithMapsId = new HashMap<>();
}

final Map<String, PropertyData> map = propertiesAnnotatedWithMapsId.computeIfAbsent(
entityType,
k -> new HashMap<>()
);
map.put(
property.getAttributeMember().getDirectAnnotationUsage( MapsId.class ).value(),
property
);
propertiesAnnotatedWithMapsId.computeIfAbsent( entityType, k -> new HashMap<>() )
.put( property.getAttributeMember().getDirectAnnotationUsage( MapsId.class ).value(), property );
}

@Override
Expand All @@ -1386,11 +1380,8 @@ public void addPropertyAnnotatedWithMapsIdSpecj(ClassDetails entityType, Propert
propertiesAnnotatedWithMapsId = new HashMap<>();
}

final Map<String, PropertyData> map = propertiesAnnotatedWithMapsId.computeIfAbsent(
entityType,
k -> new HashMap<>()
);
map.put( mapsIdValue, property );
propertiesAnnotatedWithMapsId.computeIfAbsent( entityType, k -> new HashMap<>() )
.put( mapsIdValue, property );
}

@Override
Expand All @@ -1409,9 +1400,8 @@ public void addToOneAndIdProperty(ClassDetails entityType, PropertyData property
propertiesAnnotatedWithIdAndToOne = new HashMap<>();
}

final Map<String, PropertyData> map =
propertiesAnnotatedWithIdAndToOne.computeIfAbsent( entityType, k -> new HashMap<>() );
map.put( property.getPropertyName(), property );
propertiesAnnotatedWithIdAndToOne.computeIfAbsent( entityType, k -> new HashMap<>() )
.put( property.getPropertyName(), property );
}

@Override
Expand All @@ -1424,10 +1414,7 @@ public void addMappedBy(String entityName, String propertyName, String inversePr

@Override
public String getFromMappedBy(String entityName, String propertyName) {
if ( mappedByResolver == null ) {
return null;
}
return mappedByResolver.get( entityName + "." + propertyName );
return mappedByResolver == null ? null : mappedByResolver.get( entityName + "." + propertyName );
}

@Override
Expand All @@ -1440,10 +1427,7 @@ public void addPropertyReferencedAssociation(String entityName, String propertyN

@Override
public String getPropertyReferencedAssociation(String entityName, String propertyName) {
if ( propertyRefResolver == null ) {
return null;
}
return propertyRefResolver.get( entityName + "." + propertyName );
return propertyRefResolver == null ? null : propertyRefResolver.get( entityName + "." + propertyName );
}

private static class DelayedPropertyReferenceHandlerAnnotationImpl implements DelayedPropertyReferenceHandler {
Expand Down Expand Up @@ -1519,7 +1503,7 @@ public EntityTableXref addEntityTableXref(

@Override
public Map<String, Join> getJoins(String entityName) {
EntityTableXrefImpl xrefEntry = entityTableXrefMap.get( entityName );
final EntityTableXrefImpl xrefEntry = entityTableXrefMap.get( entityName );
return xrefEntry == null ? null : xrefEntry.secondaryTableJoinMap;
}

Expand All @@ -1532,14 +1516,16 @@ private static final class EntityTableXrefImpl implements EntityTableXref {
//private Map<Identifier,Join> secondaryTableJoinMap;
private Map<String,Join> secondaryTableJoinMap;

public EntityTableXrefImpl(Identifier primaryTableLogicalName, Table primaryTable, EntityTableXrefImpl superEntityTableXref) {
public EntityTableXrefImpl(
Identifier primaryTableLogicalName, Table primaryTable, EntityTableXrefImpl superEntityTableXref) {
this.primaryTableLogicalName = primaryTableLogicalName;
this.primaryTable = primaryTable;
this.superEntityTableXref = superEntityTableXref;
}

@Override
public void addSecondaryTable(LocalMetadataBuildingContext buildingContext, Identifier logicalName, Join secondaryTableJoin) {
public void addSecondaryTable(
LocalMetadataBuildingContext buildingContext, Identifier logicalName, Join secondaryTableJoin) {
if ( Identifier.areEqual( primaryTableLogicalName, logicalName ) ) {
throw new org.hibernate.boot.MappingException(
String.format(
Expand Down Expand Up @@ -1577,6 +1563,8 @@ public void addSecondaryTable(LocalMetadataBuildingContext buildingContext, Iden

@Override
public void addSecondaryTable(QualifiedTableName logicalQualifiedTableName, Join secondaryTableJoin) {
final Identifier tableName = logicalQualifiedTableName.getTableName();

if ( Identifier.areEqual(
toIdentifier(
new QualifiedTableName(
Expand All @@ -1586,21 +1574,21 @@ public void addSecondaryTable(QualifiedTableName logicalQualifiedTableName, Join
).render()
),
toIdentifier( logicalQualifiedTableName.render() ) ) ) {
throw new DuplicateSecondaryTableException( logicalQualifiedTableName.getTableName() );
throw new DuplicateSecondaryTableException( tableName );
}

if ( secondaryTableJoinMap == null ) {
//secondaryTableJoinMap = new HashMap<Identifier,Join>();
//secondaryTableJoinMap.put( logicalName, secondaryTableJoin );
secondaryTableJoinMap = new HashMap<>();
secondaryTableJoinMap.put( logicalQualifiedTableName.getTableName().getCanonicalName(), secondaryTableJoin );
secondaryTableJoinMap.put( tableName.getCanonicalName(), secondaryTableJoin );
}
else {
//final Join existing = secondaryTableJoinMap.put( logicalName, secondaryTableJoin );
final Join existing = secondaryTableJoinMap.put( logicalQualifiedTableName.getTableName().getCanonicalName(), secondaryTableJoin );

final Join existing =
secondaryTableJoinMap.put( tableName.getCanonicalName(), secondaryTableJoin );
if ( existing != null ) {
throw new DuplicateSecondaryTableException( logicalQualifiedTableName.getTableName() );
throw new DuplicateSecondaryTableException( tableName );
}
}
}
Expand Down Expand Up @@ -1680,41 +1668,38 @@ public void addSecondPass(SecondPass secondPass) {

@Override
public void addSecondPass(SecondPass secondPass, boolean onTopOfTheQueue) {
if ( secondPass instanceof IdGeneratorResolver ) {
addIdGeneratorResolverSecondPass( (IdGeneratorResolver) secondPass, onTopOfTheQueue );
if ( secondPass instanceof IdGeneratorResolver generatorResolver ) {
addIdGeneratorResolverSecondPass( generatorResolver, onTopOfTheQueue );
}
else if ( secondPass instanceof SetBasicValueTypeSecondPass ) {
addSetBasicValueTypeSecondPass( (SetBasicValueTypeSecondPass) secondPass, onTopOfTheQueue );
else if ( secondPass instanceof SetBasicValueTypeSecondPass setBasicValueTypeSecondPass ) {
addSetBasicValueTypeSecondPass( setBasicValueTypeSecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof AggregateComponentSecondPass ) {
addAggregateComponentSecondPass( (AggregateComponentSecondPass) secondPass, onTopOfTheQueue );
else if ( secondPass instanceof AggregateComponentSecondPass aggregateComponentSecondPass ) {
addAggregateComponentSecondPass( aggregateComponentSecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof FkSecondPass ) {
addFkSecondPass( (FkSecondPass) secondPass, onTopOfTheQueue );
else if ( secondPass instanceof FkSecondPass fkSecondPass ) {
addFkSecondPass( fkSecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof CreateKeySecondPass ) {
addCreateKeySecondPass( (CreateKeySecondPass) secondPass, onTopOfTheQueue );
else if ( secondPass instanceof CreateKeySecondPass createKeySecondPass ) {
addCreateKeySecondPass( createKeySecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof ImplicitToOneJoinTableSecondPass ) {
addImplicitToOneJoinTableSecondPass( (ImplicitToOneJoinTableSecondPass) secondPass );
else if ( secondPass instanceof ImplicitToOneJoinTableSecondPass implicitToOneJoinTableSecondPass ) {
addImplicitToOneJoinTableSecondPass( implicitToOneJoinTableSecondPass );
}
else if ( secondPass instanceof SecondaryTableSecondPass ) {
addSecondaryTableSecondPass( (SecondaryTableSecondPass) secondPass, onTopOfTheQueue );
else if ( secondPass instanceof SecondaryTableSecondPass secondaryTableSecondPass ) {
addSecondaryTableSecondPass( secondaryTableSecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof SecondaryTableFromAnnotationSecondPass ) {
addSecondaryTableFromAnnotationSecondPass(
(SecondaryTableFromAnnotationSecondPass) secondPass,
onTopOfTheQueue
);
else if ( secondPass instanceof SecondaryTableFromAnnotationSecondPass secondaryTableFromAnnotationSecondPass ) {
addSecondaryTableFromAnnotationSecondPass( secondaryTableFromAnnotationSecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof QuerySecondPass ) {
addQuerySecondPass( (QuerySecondPass) secondPass, onTopOfTheQueue );
else if ( secondPass instanceof QuerySecondPass querySecondPass ) {
addQuerySecondPass( querySecondPass, onTopOfTheQueue );
}
else if ( secondPass instanceof ImplicitColumnNamingSecondPass ) {
addImplicitColumnNamingSecondPass( (ImplicitColumnNamingSecondPass) secondPass );
else if ( secondPass instanceof ImplicitColumnNamingSecondPass implicitColumnNamingSecondPass ) {
addImplicitColumnNamingSecondPass( implicitColumnNamingSecondPass );
}
else if ( secondPass instanceof OptionalDeterminationSecondPass ) {
addOptionalDeterminationSecondPass( (OptionalDeterminationSecondPass) secondPass );
else if ( secondPass instanceof OptionalDeterminationSecondPass optionalDeterminationSecondPass ) {
addOptionalDeterminationSecondPass( optionalDeterminationSecondPass );
}
else {
// add to the general SecondPass list
Expand Down Expand Up @@ -1783,7 +1768,8 @@ private void addSecondaryTableSecondPass(SecondaryTableSecondPass secondPass, bo
addSecondPass( secondPass, secondaryTableSecondPassList, onTopOfTheQueue );
}

private void addSecondaryTableFromAnnotationSecondPass(SecondaryTableFromAnnotationSecondPass secondPass, boolean onTopOfTheQueue){
private void addSecondaryTableFromAnnotationSecondPass(
SecondaryTableFromAnnotationSecondPass secondPass, boolean onTopOfTheQueue){
if ( secondaryTableFromAnnotationSecondPassesList == null ) {
secondaryTableFromAnnotationSecondPassesList = new ArrayList<>();
}
Expand Down
8 changes: 4 additions & 4 deletions ...re/src/main/java/org/hibernate/boot/model/internal/AbstractEntityIdGeneratorResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public final void doSecondPass(Map<String, PersistentClass> persistentClasses) t
}

private void handleSequenceStrategy() {
if ( generatedValue.generator().isEmpty() ) {
if ( generatedValue.generator().isBlank() ) {
handleUnnamedSequenceGenerator();
}
else {
Expand All @@ -86,7 +86,7 @@ private void handleSequenceStrategy() {
protected abstract void handleNamedSequenceGenerator();

private void handleTableStrategy() {
if ( generatedValue.generator().isEmpty() ) {
if ( generatedValue.generator().isBlank() ) {
handleUnnamedTableGenerator();
}
else {
Expand All @@ -99,7 +99,7 @@ private void handleTableStrategy() {
protected abstract void handleNamedTableGenerator();

private void handleAutoStrategy() {
if ( generatedValue.generator().isEmpty() ) {
if ( generatedValue.generator().isBlank() ) {
handleUnnamedAutoGenerator();
}
else {
Expand Down Expand Up @@ -148,7 +148,7 @@ private Annotation findGeneratorAnnotation(AnnotationTarget annotationTarget) {
protected boolean handleAsLegacyGenerator() {
// Handle a few legacy Hibernate generators...
final String nameFromGeneratedValue = generatedValue.generator();
if ( !nameFromGeneratedValue.isEmpty() ) {
if ( !nameFromGeneratedValue.isBlank() ) {
final Class<? extends Generator> legacyNamedGenerator = mapLegacyNamedGenerator( nameFromGeneratedValue, idValue );
if ( legacyNamedGenerator != null ) {
final Map<String,String> configuration = buildLegacyGeneratorConfig();
Expand Down
Loading
Loading