Skip to content

Commit

Permalink
HHH-17999 use Supplier instead of Constructor
Browse files Browse the repository at this point in the history 
this is quite a lot cleaner

Signed-off-by: Gavin King <gavin@hibernate.org>
  • Loading branch information
@gavinking
gavinking committed Apr 23, 2024
1 parent abbfa53 commit 06139cf
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 74 deletions.
10 changes: 5 additions & 5 deletions hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@
import static org.hibernate.boot.model.internal.BinderHelper.extractFromPackage;
import static org.hibernate.boot.model.source.internal.hbm.ModelBinder.useEntityWhereClauseForCollections;
import static org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle.fromResultCheckStyle;
import static org.hibernate.internal.util.ReflectHelper.getDefaultConstructor;
import static org.hibernate.internal.util.ReflectHelper.getDefaultSupplier;
import static org.hibernate.internal.util.StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty;
import static org.hibernate.internal.util.StringHelper.isEmpty;
import static org.hibernate.internal.util.StringHelper.isNotEmpty;
Expand Down Expand Up @@ -1337,7 +1337,7 @@ private void bindLoader() {
fromResultCheckStyle( sqlInsert.check() )
);
if ( sqlInsert.verify() != Expectation.class ) {
collection.setInsertExpectation( getDefaultConstructor( sqlInsert.verify() ) );
collection.setInsertExpectation( getDefaultSupplier( sqlInsert.verify() ) );
}
}

Expand All @@ -1349,7 +1349,7 @@ private void bindLoader() {
fromResultCheckStyle( sqlUpdate.check() )
);
if ( sqlUpdate.verify() != Expectation.class ) {
collection.setUpdateExpectation( getDefaultConstructor( sqlUpdate.verify() ) );
collection.setUpdateExpectation( getDefaultSupplier( sqlUpdate.verify() ) );
}
}

Expand All @@ -1361,7 +1361,7 @@ private void bindLoader() {
fromResultCheckStyle( sqlDelete.check() )
);
if ( sqlDelete.verify() != Expectation.class ) {
collection.setDeleteExpectation( getDefaultConstructor( sqlDelete.verify() ) );
collection.setDeleteExpectation( getDefaultSupplier( sqlDelete.verify() ) );
}
}

Expand All @@ -1373,7 +1373,7 @@ private void bindLoader() {
fromResultCheckStyle( sqlDeleteAll.check() )
);
if ( sqlDeleteAll.verify() != Expectation.class ) {
collection.setDeleteAllExpectation( getDefaultConstructor( sqlDeleteAll.verify() ) );
collection.setDeleteAllExpectation( getDefaultSupplier( sqlDeleteAll.verify() ) );
}
}

Expand Down
14 changes: 7 additions & 7 deletions hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
import static org.hibernate.boot.model.naming.Identifier.toIdentifier;
import static org.hibernate.engine.OptimisticLockStyle.fromLockType;
import static org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle.fromResultCheckStyle;
import static org.hibernate.internal.util.ReflectHelper.getDefaultConstructor;
import static org.hibernate.internal.util.ReflectHelper.getDefaultSupplier;
import static org.hibernate.internal.util.StringHelper.isEmpty;
import static org.hibernate.internal.util.StringHelper.isNotEmpty;
import static org.hibernate.internal.util.StringHelper.nullIfEmpty;
Expand Down Expand Up @@ -1397,7 +1397,7 @@ private void bindCustomSql() {
fromResultCheckStyle( sqlInsert.check() )
);
if ( sqlInsert.verify() != Expectation.class ) {
persistentClass.setInsertExpectation( getDefaultConstructor( sqlInsert.verify() ) );
persistentClass.setInsertExpectation( getDefaultSupplier( sqlInsert.verify() ) );
}
}

Expand All @@ -1412,7 +1412,7 @@ private void bindCustomSql() {
fromResultCheckStyle( sqlUpdate.check() )
);
if ( sqlUpdate.verify() != Expectation.class ) {
persistentClass.setUpdateExpectation( getDefaultConstructor( sqlUpdate.verify() ) );
persistentClass.setUpdateExpectation( getDefaultSupplier( sqlUpdate.verify() ) );
}
}

Expand All @@ -1427,7 +1427,7 @@ private void bindCustomSql() {
fromResultCheckStyle( sqlDelete.check() )
);
if ( sqlDelete.verify() != Expectation.class ) {
persistentClass.setDeleteExpectation( getDefaultConstructor( sqlDelete.verify() ) );
persistentClass.setDeleteExpectation( getDefaultSupplier( sqlDelete.verify() ) );
}
}

Expand Down Expand Up @@ -2274,7 +2274,7 @@ private void processSecondaryTableCustomSql(Join join) {
fromResultCheckStyle( sqlInsert.check() )
);
if ( sqlInsert.verify() != Expectation.class ) {
join.setInsertExpectation( getDefaultConstructor( sqlInsert.verify() ) );
join.setInsertExpectation( getDefaultSupplier( sqlInsert.verify() ) );
}
}
else if ( matchingTable != null ) {
Expand All @@ -2297,7 +2297,7 @@ else if ( matchingTable != null ) {
fromResultCheckStyle( sqlUpdate.check() )
);
if ( sqlUpdate.verify() != Expectation.class ) {
join.setUpdateExpectation( getDefaultConstructor( sqlUpdate.verify() ) );
join.setUpdateExpectation( getDefaultSupplier( sqlUpdate.verify() ) );
}
}
else if ( matchingTable != null ) {
Expand All @@ -2320,7 +2320,7 @@ else if ( matchingTable != null ) {
fromResultCheckStyle( sqlDelete.check() )
);
if ( sqlDelete.verify() != Expectation.class ) {
join.setDeleteExpectation( getDefaultConstructor( sqlDelete.verify() ) );
join.setDeleteExpectation( getDefaultSupplier( sqlDelete.verify() ) );
}
}
else if ( matchingTable != null ) {
Expand Down
20 changes: 12 additions & 8 deletions hibernate-core/src/main/java/org/hibernate/engine/spi/ExecuteUpdateResultCheckStyle.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.checkerframework.checker.nullness.qual.Nullable;
import org.hibernate.jdbc.Expectation;

import java.lang.reflect.Constructor;
import java.util.function.Supplier;

/**
* For persistence operations (INSERT, UPDATE, DELETE) what style of
Expand Down Expand Up @@ -92,17 +92,21 @@ public static ExecuteUpdateResultCheckStyle determineDefault(@Nullable String cu
return customSql != null && callable ? PARAM : COUNT;
}

public static @Nullable Constructor<? extends Expectation> expectationConstructor(
public static @Nullable Supplier<? extends Expectation> expectationConstructor(
@Nullable ExecuteUpdateResultCheckStyle style) {
return style == null ? null : style.expectationConstructor();
}

public Constructor<? extends Expectation> expectationConstructor() {
try {
return expectationClass().getDeclaredConstructor();
}
catch ( NoSuchMethodException e ) {
throw new AssertionFailure("Could not instantiate Expectation", e);
public Supplier<? extends Expectation> expectationConstructor() {
switch (this) {
case NONE:
return Expectation.None::new;
case COUNT:
return Expectation.RowCount::new;
case PARAM:
return Expectation.OutParameter::new;
default:
throw new AssertionFailure( "Unrecognized ExecuteUpdateResultCheckStyle");
}
}

Expand Down
32 changes: 31 additions & 1 deletion hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType;
import java.util.Locale;
import java.util.function.Supplier;

import org.hibernate.AssertionFailure;
import org.hibernate.MappingException;
Expand All @@ -39,7 +41,6 @@
* @author Steve Ebersole
* @author Chris Cranford
*/
@SuppressWarnings("unchecked")
public final class ReflectHelper {

public static final Class<?>[] NO_PARAM_SIGNATURE = ArrayHelper.EMPTY_CLASS_ARRAY;
Expand Down Expand Up @@ -298,6 +299,35 @@ public static <T> Constructor<T> getDefaultConstructor(Class<T> clazz) throws Pr
}
}

public static <T> Supplier<T> getDefaultSupplier(Class<T> clazz) {
if ( isAbstractClass( clazz ) ) {
throw new IllegalArgumentException( "Abstract class cannot be instantiated: " + clazz.getName() );
}

try {
final Constructor<T> constructor = clazz.getDeclaredConstructor( NO_PARAM_SIGNATURE );
ensureAccessibility( constructor );
return () -> {
try {
return constructor.newInstance();
}
catch ( InstantiationException|IllegalAccessException|InvocationTargetException e ) {
throw new org.hibernate.InstantiationException( "Constructor threw exception", clazz, e );
}
};
}
catch ( NoSuchMethodException nme ) {
return () -> {
try {
return clazz.newInstance();
}
catch ( InstantiationException|IllegalAccessException e ) {
throw new org.hibernate.InstantiationException( "Default constructor threw exception", clazz, e );
}
};
}
}

/**
* Determine if the given class is declared abstract.
*
Expand Down
17 changes: 5 additions & 12 deletions hibernate-core/src/main/java/org/hibernate/jdbc/Expectations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
*/
package org.hibernate.jdbc;

import java.lang.reflect.Constructor;
import java.sql.CallableStatement;
import java.sql.PreparedStatement;
import java.util.function.Supplier;

import org.hibernate.AssertionFailure;
import org.hibernate.HibernateException;
import org.hibernate.InstantiationException;
import org.hibernate.Internal;
import org.hibernate.StaleStateException;
import org.hibernate.engine.jdbc.spi.SqlExceptionHelper;
Expand Down Expand Up @@ -42,26 +41,20 @@ public class Expectations {
* @since 6.5
*/
@Internal
public static Expectation createExpectation(Constructor<? extends Expectation> expectation, boolean callable) {
public static Expectation createExpectation(Supplier<? extends Expectation> expectation, boolean callable) {
final Expectation instance = instantiate( expectation, callable );
instance.validate( callable );
return instance;
}

private static Expectation instantiate(Constructor<? extends Expectation> constructor, boolean callable) {
if ( constructor == null ) {
private static Expectation instantiate(Supplier<? extends Expectation> supplier, boolean callable) {
if ( supplier == null ) {
return callable
? new Expectation.OutParameter()
: new Expectation.RowCount();
}
else {
try {
return constructor.newInstance();
}
catch ( Exception e ) {
throw new InstantiationException( "Could not instantiate Expectation",
constructor.getDeclaringClass(), e );
}
return supplier.get();
}
}

Expand Down
25 changes: 12 additions & 13 deletions hibernate-core/src/main/java/org/hibernate/mapping/Collection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/
package org.hibernate.mapping;

import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -108,10 +107,10 @@ public abstract class Collection implements Fetchable, Value, Filterable, SoftDe

private String loaderName;

private Constructor<? extends Expectation> insertExpectation;
private Constructor<? extends Expectation> updateExpectation;
private Constructor<? extends Expectation> deleteExpectation;
private Constructor<? extends Expectation> deleteAllExpectation;
private Supplier<? extends Expectation> insertExpectation;
private Supplier<? extends Expectation> updateExpectation;
private Supplier<? extends Expectation> deleteExpectation;
private Supplier<? extends Expectation> deleteAllExpectation;

/**
* hbm.xml binding
Expand Down Expand Up @@ -877,35 +876,35 @@ public Column getSoftDeleteColumn() {
return softDeleteColumn;
}

public Constructor<? extends Expectation> getInsertExpectation() {
public Supplier<? extends Expectation> getInsertExpectation() {
return insertExpectation;
}

public void setInsertExpectation(Constructor<? extends Expectation> insertExpectation) {
public void setInsertExpectation(Supplier<? extends Expectation> insertExpectation) {
this.insertExpectation = insertExpectation;
}

public Constructor<? extends Expectation> getUpdateExpectation() {
public Supplier<? extends Expectation> getUpdateExpectation() {
return updateExpectation;
}

public void setUpdateExpectation(Constructor<? extends Expectation> updateExpectation) {
public void setUpdateExpectation(Supplier<? extends Expectation> updateExpectation) {
this.updateExpectation = updateExpectation;
}

public Constructor<? extends Expectation> getDeleteExpectation() {
public Supplier<? extends Expectation> getDeleteExpectation() {
return deleteExpectation;
}

public void setDeleteExpectation(Constructor<? extends Expectation> deleteExpectation) {
public void setDeleteExpectation(Supplier<? extends Expectation> deleteExpectation) {
this.deleteExpectation = deleteExpectation;
}

public Constructor<? extends Expectation> getDeleteAllExpectation() {
public Supplier<? extends Expectation> getDeleteAllExpectation() {
return deleteAllExpectation;
}

public void setDeleteAllExpectation(Constructor<? extends Expectation> deleteAllExpectation) {
public void setDeleteAllExpectation(Supplier<? extends Expectation> deleteAllExpectation) {
this.deleteAllExpectation = deleteAllExpectation;
}
}
20 changes: 10 additions & 10 deletions hibernate-core/src/main/java/org/hibernate/mapping/Join.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
package org.hibernate.mapping;

import java.io.Serializable;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.function.Supplier;

import org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle;
import org.hibernate.jdbc.Expectation;
Expand Down Expand Up @@ -51,9 +51,9 @@ public class Join implements AttributeContainer, Serializable {
private boolean customDeleteCallable;
private ExecuteUpdateResultCheckStyle deleteCheckStyle;

private Constructor<? extends Expectation> insertExpectation;
private Constructor<? extends Expectation> updateExpectation;
private Constructor<? extends Expectation> deleteExpectation;
private Supplier<? extends Expectation> insertExpectation;
private Supplier<? extends Expectation> updateExpectation;
private Supplier<? extends Expectation> deleteExpectation;

@Override
public void addProperty(Property property) {
Expand Down Expand Up @@ -235,27 +235,27 @@ public void setOptional(boolean nullable) {
this.optional = nullable;
}

public Constructor<? extends Expectation> getInsertExpectation() {
public Supplier<? extends Expectation> getInsertExpectation() {
return insertExpectation;
}

public void setInsertExpectation(Constructor<? extends Expectation> insertExpectation) {
public void setInsertExpectation(Supplier<? extends Expectation> insertExpectation) {
this.insertExpectation = insertExpectation;
}

public Constructor<? extends Expectation> getUpdateExpectation() {
public Supplier<? extends Expectation> getUpdateExpectation() {
return updateExpectation;
}

public void setUpdateExpectation(Constructor<? extends Expectation> updateExpectation) {
public void setUpdateExpectation(Supplier<? extends Expectation> updateExpectation) {
this.updateExpectation = updateExpectation;
}

public Constructor<? extends Expectation> getDeleteExpectation() {
public Supplier<? extends Expectation> getDeleteExpectation() {
return deleteExpectation;
}

public void setDeleteExpectation(Constructor<? extends Expectation> deleteExpectation) {
public void setDeleteExpectation(Supplier<? extends Expectation> deleteExpectation) {
this.deleteExpectation = deleteExpectation;
}
}

0 comments on commit 06139cf

Please sign in to comment.