HHH-12978 - Enum value binding is not logged by BasicBinder

hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/NamedEnumValueConverter.java

@@ -13,10 +13,13 @@ import java.sql.SQLException;
 import java.sql.Types;
 import java.util.Locale;
 
+import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.metamodel.model.convert.spi.EnumValueConverter;
+import org.hibernate.type.descriptor.ValueBinder;
+import org.hibernate.type.descriptor.ValueExtractor;
 import org.hibernate.type.descriptor.java.EnumJavaTypeDescriptor;
-
+import org.hibernate.type.descriptor.java.StringTypeDescriptor;
-import org.jboss.logging.Logger;
+import org.hibernate.type.descriptor.sql.VarcharTypeDescriptor;
 
 /**
  * BasicValueConverter handling the conversion of an enum based on
@@ -25,12 +28,17 @@ import org.jboss.logging.Logger;
  * @author Steve Ebersole
  */
 public class NamedEnumValueConverter<E extends Enum> implements EnumValueConverter<E,String>, Serializable {
-	private static final Logger log = Logger.getLogger( NamedEnumValueConverter.class );
 
 	private final EnumJavaTypeDescriptor<E> enumJavaDescriptor;
 
+	private final transient ValueExtractor<E> valueExtractor;
+
+	private final transient ValueBinder<String> valueBinder;
+
 	public NamedEnumValueConverter(EnumJavaTypeDescriptor<E> enumJavaDescriptor) {
 		this.enumJavaDescriptor = enumJavaDescriptor;
+		this.valueExtractor = VarcharTypeDescriptor.INSTANCE.getExtractor( enumJavaDescriptor );
+		this.valueBinder = VarcharTypeDescriptor.INSTANCE.getBinder( StringTypeDescriptor.INSTANCE );
 	}
 
 	@Override
@@ -55,43 +63,15 @@ public class NamedEnumValueConverter<E extends Enum> implements EnumValueConvert
 	}
 
 	@Override
-	public E readValue(ResultSet resultSet, String name) throws SQLException {
+	public E readValue(ResultSet resultSet, String name, SharedSessionContractImplementor session) throws SQLException {
-		final String value = resultSet.getString( name );
+		return valueExtractor.extract( resultSet, name, session );
-
-		final boolean traceEnabled = log.isTraceEnabled();
-		if ( resultSet.wasNull() ) {
-			if ( traceEnabled ) {
-				log.trace( String.format( "Returning null as column [%s]", name ) );
-			}
-			return null;
-		}
-
-		final E enumValue = toDomainValue( value );
-		if ( traceEnabled ) {
-			log.trace( String.format( "Returning [%s] as column [%s]", enumValue, name ) );
-		}
-
-		return enumValue;
 	}
 
 	@Override
-	public void writeValue(PreparedStatement statement, E value, int position) throws SQLException {
+	public void writeValue(PreparedStatement statement, E value, int position, SharedSessionContractImplementor session) throws SQLException {
 		final String jdbcValue = value == null ? null : toRelationalValue( value );
 
-		final boolean traceEnabled = log.isTraceEnabled();
+		valueBinder.bind( statement, jdbcValue, position, session );
-		if ( jdbcValue == null ) {
-			if ( traceEnabled ) {
-				log.tracef( "Binding null to parameter: [%s]", position );
-			}
-			statement.setNull( position, getJdbcTypeCode() );
-			return;
-		}
-
-		if ( traceEnabled ) {
-			log.tracef( "Binding [%s] to parameter: [%s]", jdbcValue, position );
-		}
-
-		statement.setString( position, jdbcValue );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/OrdinalEnumValueConverter.java

@@ -12,10 +12,12 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Types;
 
+import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.metamodel.model.convert.spi.EnumValueConverter;
+import org.hibernate.type.descriptor.ValueBinder;
+import org.hibernate.type.descriptor.ValueExtractor;
 import org.hibernate.type.descriptor.java.EnumJavaTypeDescriptor;
-
+import org.hibernate.type.descriptor.sql.IntegerTypeDescriptor;
-import org.jboss.logging.Logger;
 
 /**
  * BasicValueConverter handling the conversion of an enum based on
@@ -24,12 +26,17 @@ import org.jboss.logging.Logger;
  * @author Steve Ebersole
  */
 public class OrdinalEnumValueConverter<E extends Enum> implements EnumValueConverter<E,Integer>, Serializable {
-	private static final Logger log = Logger.getLogger( OrdinalEnumValueConverter.class );
 
 	private final EnumJavaTypeDescriptor<E> enumJavaDescriptor;
 
+	private final transient ValueExtractor<E> valueExtractor;
+
+	private final transient ValueBinder<Integer> valueBinder;
+
 	public OrdinalEnumValueConverter(EnumJavaTypeDescriptor<E> enumJavaDescriptor) {
 		this.enumJavaDescriptor = enumJavaDescriptor;
+		this.valueExtractor = IntegerTypeDescriptor.INSTANCE.getExtractor( enumJavaDescriptor );
+		this.valueBinder = IntegerTypeDescriptor.INSTANCE.getBinder( org.hibernate.type.descriptor.java.IntegerTypeDescriptor.INSTANCE );
 	}
 
 	@Override
@@ -54,42 +61,15 @@ public class OrdinalEnumValueConverter<E extends Enum> implements EnumValueConve
 	}
 
 	@Override
-	public E readValue(ResultSet resultSet, String name) throws SQLException {
+	public E readValue(ResultSet resultSet, String name, SharedSessionContractImplementor session) throws SQLException {
-		final int ordinal = resultSet.getInt( name );
+		return valueExtractor.extract( resultSet, name, session );
-		final boolean traceEnabled = log.isTraceEnabled();
-		if ( resultSet.wasNull() ) {
-			if ( traceEnabled ) {
-				log.trace(String.format("Returning null as column [%s]", name));
-			}
-			return null;
-		}
-
-		final E enumValue = toDomainValue( ordinal );
-		if ( traceEnabled ) {
-			log.trace(String.format("Returning [%s] as column [%s]", enumValue, name));
-		}
-
-		return enumValue;
 	}
 
 	@Override
-	public void writeValue(PreparedStatement statement, E value, int position) throws SQLException {
+	public void writeValue(PreparedStatement statement, E value, int position, SharedSessionContractImplementor session) throws SQLException {
 		final Integer jdbcValue = value == null ? null : toRelationalValue( value );
 
-		final boolean traceEnabled = log.isTraceEnabled();
+		valueBinder.bind( statement, jdbcValue, position, session );
-		if ( jdbcValue == null ) {
-			if ( traceEnabled ) {
-				log.tracef( "Binding null to parameter: [%s]", position );
-			}
-			statement.setNull( position, getJdbcTypeCode() );
-			return;
-		}
-
-		if ( traceEnabled ) {
-			log.tracef( "Binding [%s] to parameter: [%s]", jdbcValue.intValue(), position );
-		}
-
-		statement.setInt( position, jdbcValue );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/spi/EnumValueConverter.java

@@ -10,6 +10,7 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 
+import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.type.descriptor.java.EnumJavaTypeDescriptor;
 
 /**
@@ -21,8 +22,8 @@ public interface EnumValueConverter<O extends Enum, R> extends BasicValueConvert
 	EnumJavaTypeDescriptor<O> getJavaDescriptor();
 	int getJdbcTypeCode();
 
-	O readValue(ResultSet resultSet, String name) throws SQLException;
+	O readValue(ResultSet resultSet, String name, SharedSessionContractImplementor session) throws SQLException;
-	void writeValue(PreparedStatement statement, O value, int position) throws SQLException;
+	void writeValue(PreparedStatement statement, O value, int position, SharedSessionContractImplementor session) throws SQLException;
 
 	String toSqlLiteral(Object value);
 }

hibernate-core/src/main/java/org/hibernate/type/EnumType.java

@@ -247,7 +247,7 @@ public class EnumType<T extends Enum>
 	@Override
 	public Object nullSafeGet(ResultSet rs, String[] names, SharedSessionContractImplementor session, Object owner) throws SQLException {
 		verifyConfigured();
-		return enumValueConverter.readValue( rs, names[0] );
+		return enumValueConverter.readValue( rs, names[0], session );
 	}
 
 	private void verifyConfigured() {
@@ -259,7 +259,7 @@ public class EnumType<T extends Enum>
 	@Override
 	public void nullSafeSet(PreparedStatement st, Object value, int index, SharedSessionContractImplementor session) throws HibernateException, SQLException {
 		verifyConfigured();
-		enumValueConverter.writeValue( st, (Enum) value, index );
+		enumValueConverter.writeValue( st, (Enum) value, index, session );
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/test/enums/EnumTypeTest.java

@@ -6,52 +6,120 @@
  */
 package org.hibernate.test.enums;
 
-import org.hibernate.Session;
 import org.hibernate.criterion.Restrictions;
+import org.hibernate.internal.CoreMessageLogger;
+import org.hibernate.type.descriptor.sql.BasicBinder;
+import org.hibernate.type.descriptor.sql.BasicExtractor;
 
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.logger.LoggerInspectionRule;
+import org.hibernate.testing.logger.Triggerable;
+import org.junit.Rule;
 import org.junit.Test;
 
+import org.jboss.logging.Logger;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author Brett Meyer
  */
 public class EnumTypeTest extends BaseCoreFunctionalTestCase {
 
+	@Rule
+	public LoggerInspectionRule binderLogInspection = new LoggerInspectionRule( Logger.getMessageLogger(
+			CoreMessageLogger.class,
+			BasicBinder.class.getName()
+	) );
+
+	@Rule
+	public LoggerInspectionRule extractorLogInspection = new LoggerInspectionRule( Logger.getMessageLogger(
+			CoreMessageLogger.class,
+			BasicExtractor.class.getName()
+	) );
+
+	private Person person;
+
+	private Triggerable binderTriggerable;
+
+	private Triggerable extractorTriggerable;
+
 	protected String[] getMappings() {
 		return new String[] { "enums/Person.hbm.xml" };
 	}
 
+	@Override
+	protected void prepareTest() {
+		doInHibernate( this::sessionFactory, s -> {
+			this.person = Person.person( Gender.MALE, HairColor.BROWN );
+			s.persist( person );
+			s.persist( Person.person( Gender.MALE, HairColor.BLACK ) );
+			s.persist( Person.person( Gender.FEMALE, HairColor.BROWN ) );
+			s.persist( Person.person( Gender.FEMALE, HairColor.BLACK ) );
+		} );
+
+		binderTriggerable = binderLogInspection.watchForLogMessages( "binding parameter" );
+		extractorTriggerable = extractorLogInspection.watchForLogMessages( "extracted value" );
+	}
+
+	@Override
+	protected boolean isCleanupTestDataRequired() {
+		return true;
+	}
+
 	@Test
 	@TestForIssue(jiraKey = "HHH-8153")
 	public void hbmEnumTypeTest() {
-		Session s = openSession();
+		doInHibernate( this::sessionFactory, s -> {
-		s.getTransaction().begin();
+			assertEquals( s.createCriteria( Person.class )
-		s.persist( Person.person( Gender.MALE, HairColor.BROWN ) );
+								  .add( Restrictions.eq( "gender", Gender.MALE ) )
-		s.persist( Person.person( Gender.MALE, HairColor.BLACK ) );
+								  .list().size(), 2 );
-		s.persist( Person.person( Gender.FEMALE, HairColor.BROWN ) );
+			assertEquals( s.createCriteria( Person.class )
-		s.persist( Person.person( Gender.FEMALE, HairColor.BLACK ) );
+								  .add( Restrictions.eq( "gender", Gender.MALE ) )
-		s.getTransaction().commit();
+								  .add( Restrictions.eq( "hairColor", HairColor.BROWN ) )
-		s.clear();
+								  .list().size(), 1 );
+			assertEquals( s.createCriteria( Person.class )
+								  .add( Restrictions.eq( "gender", Gender.FEMALE ) )
+								  .list().size(), 2 );
+			assertEquals( s.createCriteria( Person.class )
+								  .add( Restrictions.eq( "gender", Gender.FEMALE ) )
+								  .add( Restrictions.eq( "hairColor", HairColor.BROWN ) )
+								  .list().size(), 1 );
+		} );
+	}
 
-		s.getTransaction().begin();
+	@Test
-		assertEquals(s.createCriteria( Person.class )
+	@TestForIssue(jiraKey = "HHH-12978")
-				.add( Restrictions.eq( "gender", Gender.MALE ) )
+	public void testEnumAsBindParameterAndExtract() {
-				.list().size(), 2);
+		doInHibernate( this::sessionFactory, s -> {
-		assertEquals(s.createCriteria( Person.class )
+			binderTriggerable.reset();
-				.add( Restrictions.eq( "gender", Gender.MALE ) )
+			extractorTriggerable.reset();
-				.add( Restrictions.eq( "hairColor", HairColor.BROWN ) )
+
-				.list().size(), 1);
+			s.createQuery( "select p.id from Person p where p.id = :id", Long.class )
-		assertEquals(s.createCriteria( Person.class )
+					.setParameter( "id", person.getId() )
-				.add( Restrictions.eq( "gender", Gender.FEMALE ) )
+					.getSingleResult();
-				.list().size(), 2);
+
-		assertEquals(s.createCriteria( Person.class )
+			assertTrue( binderTriggerable.wasTriggered() );
-				.add( Restrictions.eq( "gender", Gender.FEMALE ) )
+			assertTrue( extractorTriggerable.wasTriggered() );
-				.add( Restrictions.eq( "hairColor", HairColor.BROWN ) )
+		} );
-				.list().size(), 1);
+
-		s.getTransaction().commit();
+		doInHibernate( this::sessionFactory, s -> {
-		s.close();
+			binderTriggerable.reset();
+			extractorTriggerable.reset();
+
+			s.createQuery(
+					"select p.gender from Person p where p.gender = :gender and p.hairColor = :hairColor",
+					Gender.class
+			)
+					.setParameter( "gender", Gender.MALE )
+					.setParameter( "hairColor", HairColor.BROWN )
+					.getSingleResult();
+
+			assertTrue( binderTriggerable.wasTriggered() );
+			assertTrue( extractorTriggerable.wasTriggered() );
+		} );
 	}
 }

hibernate-core/src/test/java/org/hibernate/test/enums/OrdinalEnumTypeTest.java

@@ -0,0 +1,169 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.enums;
+
+import javax.persistence.Entity;
+import javax.persistence.EnumType;
+import javax.persistence.Enumerated;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+import org.hibernate.criterion.Restrictions;
+import org.hibernate.internal.CoreMessageLogger;
+import org.hibernate.type.descriptor.sql.BasicBinder;
+import org.hibernate.type.descriptor.sql.BasicExtractor;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.logger.LoggerInspectionRule;
+import org.hibernate.testing.logger.Triggerable;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.jboss.logging.Logger;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Vlad Mihacea
+ */
+public class OrdinalEnumTypeTest extends BaseCoreFunctionalTestCase {
+
+	@Rule
+	public LoggerInspectionRule binderLogInspection = new LoggerInspectionRule( Logger.getMessageLogger(
+			CoreMessageLogger.class,
+			BasicBinder.class.getName()
+	) );
+
+	@Rule
+	public LoggerInspectionRule extractorLogInspection = new LoggerInspectionRule( Logger.getMessageLogger(
+			CoreMessageLogger.class,
+			BasicExtractor.class.getName()
+	) );
+
+	private Person person;
+
+	private Triggerable binderTriggerable;
+
+	private Triggerable extractorTriggerable;
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] {
+				Person.class
+		};
+	}
+
+	@Override
+	protected void prepareTest() {
+		doInHibernate( this::sessionFactory, s -> {
+			this.person = Person.person( Gender.MALE, HairColor.BROWN );
+			s.persist( person );
+			s.persist( Person.person( Gender.MALE, HairColor.BLACK ) );
+			s.persist( Person.person( Gender.FEMALE, HairColor.BROWN ) );
+			s.persist( Person.person( Gender.FEMALE, HairColor.BLACK ) );
+		} );
+
+		binderTriggerable = binderLogInspection.watchForLogMessages( "binding parameter" );
+		extractorTriggerable = extractorLogInspection.watchForLogMessages( "extracted value" );
+	}
+
+	@Override
+	protected boolean isCleanupTestDataRequired() {
+		return true;
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HHH-12978")
+	public void testEnumAsBindParameterAndExtract() {
+		doInHibernate( this::sessionFactory, s -> {
+			binderTriggerable.reset();
+			extractorTriggerable.reset();
+
+			s.createQuery( "select p.id from Person p where p.id = :id", Long.class )
+					.setParameter( "id", person.getId() )
+					.getSingleResult();
+
+			assertTrue( binderTriggerable.wasTriggered() );
+			assertTrue( extractorTriggerable.wasTriggered() );
+		} );
+
+		doInHibernate( this::sessionFactory, s -> {
+			binderTriggerable.reset();
+			extractorTriggerable.reset();
+
+			s.createQuery(
+					"select p.gender from Person p where p.gender = :gender and p.hairColor = :hairColor",
+					Gender.class
+			)
+					.setParameter( "gender", Gender.MALE )
+					.setParameter( "hairColor", HairColor.BROWN )
+					.getSingleResult();
+
+			assertTrue( binderTriggerable.wasTriggered() );
+			assertTrue( extractorTriggerable.wasTriggered() );
+		} );
+	}
+
+	@Entity(name = "Person")
+	public static class Person {
+
+		@Id
+		@GeneratedValue
+		private Long id;
+
+		@Enumerated(EnumType.ORDINAL)
+		private Gender gender;
+
+		@Enumerated(EnumType.ORDINAL)
+		private HairColor hairColor;
+
+		@Enumerated(EnumType.ORDINAL)
+		private HairColor originalHairColor;
+
+		public static Person person(Gender gender, HairColor hairColor) {
+			Person person = new Person();
+			person.setGender( gender );
+			person.setHairColor( hairColor );
+			return person;
+		}
+
+		public long getId() {
+			return id;
+		}
+
+		public void setId(long id) {
+			this.id = id;
+		}
+
+		public Gender getGender() {
+			return gender;
+		}
+
+		public void setGender(Gender gender) {
+			this.gender = gender;
+		}
+
+		public HairColor getHairColor() {
+			return hairColor;
+		}
+
+		public void setHairColor(HairColor hairColor) {
+			this.hairColor = hairColor;
+		}
+
+		public HairColor getOriginalHairColor() {
+			return originalHairColor;
+		}
+
+		public void setOriginalHairColor(HairColor originalHairColor) {
+			this.originalHairColor = originalHairColor;
+		}
+	}
+}