From e55c3bbb7ea3cc1b765777f83611619f0748ca70 Mon Sep 17 00:00:00 2001 From: Vlad Mihalcea Date: Wed, 19 Sep 2018 15:04:44 +0300 Subject: [PATCH] HHH-12978 - Enum value binding is not logged by BasicBinder --- .../internal/NamedEnumValueConverter.java | 50 ++---- .../internal/OrdinalEnumValueConverter.java | 48 ++--- .../model/convert/spi/EnumValueConverter.java | 5 +- .../java/org/hibernate/type/EnumType.java | 4 +- .../hibernate/test/enums/EnumTypeTest.java | 120 ++++++++++--- .../test/enums/OrdinalEnumTypeTest.java | 169 ++++++++++++++++++ 6 files changed, 297 insertions(+), 99 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/enums/OrdinalEnumTypeTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/NamedEnumValueConverter.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/NamedEnumValueConverter.java index c077c82e54..c67aedcd02 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/NamedEnumValueConverter.java +++ b/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.jboss.logging.Logger; +import org.hibernate.type.descriptor.java.StringTypeDescriptor; +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 implements EnumValueConverter, Serializable { - private static final Logger log = Logger.getLogger( NamedEnumValueConverter.class ); private final EnumJavaTypeDescriptor enumJavaDescriptor; + private final transient ValueExtractor valueExtractor; + + private final transient ValueBinder valueBinder; + public NamedEnumValueConverter(EnumJavaTypeDescriptor 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 implements EnumValueConvert } @Override - public E readValue(ResultSet resultSet, String name) throws SQLException { - final String value = resultSet.getString( name ); - - 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; + public E readValue(ResultSet resultSet, String name, SharedSessionContractImplementor session) throws SQLException { + return valueExtractor.extract( resultSet, name, session ); } @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(); - 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 ); + valueBinder.bind( statement, jdbcValue, position, session ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/OrdinalEnumValueConverter.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/OrdinalEnumValueConverter.java index 4f3fa4d78d..6099efcfb3 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/internal/OrdinalEnumValueConverter.java +++ b/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.jboss.logging.Logger; +import org.hibernate.type.descriptor.sql.IntegerTypeDescriptor; /** * BasicValueConverter handling the conversion of an enum based on @@ -24,12 +26,17 @@ import org.jboss.logging.Logger; * @author Steve Ebersole */ public class OrdinalEnumValueConverter implements EnumValueConverter, Serializable { - private static final Logger log = Logger.getLogger( OrdinalEnumValueConverter.class ); private final EnumJavaTypeDescriptor enumJavaDescriptor; + private final transient ValueExtractor valueExtractor; + + private final transient ValueBinder valueBinder; + public OrdinalEnumValueConverter(EnumJavaTypeDescriptor 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 implements EnumValueConve } @Override - public E readValue(ResultSet resultSet, String name) throws SQLException { - final int ordinal = resultSet.getInt( name ); - 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; + public E readValue(ResultSet resultSet, String name, SharedSessionContractImplementor session) throws SQLException { + return valueExtractor.extract( resultSet, name, session ); } @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(); - 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 ); + valueBinder.bind( statement, jdbcValue, position, session ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/spi/EnumValueConverter.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/spi/EnumValueConverter.java index dc384c4540..548de76413 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/convert/spi/EnumValueConverter.java +++ b/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 extends BasicValueConvert EnumJavaTypeDescriptor getJavaDescriptor(); int getJdbcTypeCode(); - O readValue(ResultSet resultSet, String name) throws SQLException; - void writeValue(PreparedStatement statement, O value, int position) throws SQLException; + O readValue(ResultSet resultSet, String name, SharedSessionContractImplementor session) throws SQLException; + void writeValue(PreparedStatement statement, O value, int position, SharedSessionContractImplementor session) throws SQLException; String toSqlLiteral(Object value); } diff --git a/hibernate-core/src/main/java/org/hibernate/type/EnumType.java b/hibernate-core/src/main/java/org/hibernate/type/EnumType.java index 940ae2a1c0..e6263460b8 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/EnumType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/EnumType.java @@ -247,7 +247,7 @@ public class EnumType @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 @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 diff --git a/hibernate-core/src/test/java/org/hibernate/test/enums/EnumTypeTest.java b/hibernate-core/src/test/java/org/hibernate/test/enums/EnumTypeTest.java index afc58d9f34..ff0d5ca953 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/enums/EnumTypeTest.java +++ b/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(); - s.getTransaction().begin(); - s.persist( Person.person( Gender.MALE, HairColor.BROWN ) ); - s.persist( Person.person( Gender.MALE, HairColor.BLACK ) ); - s.persist( Person.person( Gender.FEMALE, HairColor.BROWN ) ); - s.persist( Person.person( Gender.FEMALE, HairColor.BLACK ) ); - s.getTransaction().commit(); - s.clear(); + doInHibernate( this::sessionFactory, s -> { + assertEquals( s.createCriteria( Person.class ) + .add( Restrictions.eq( "gender", Gender.MALE ) ) + .list().size(), 2 ); + assertEquals( s.createCriteria( Person.class ) + .add( Restrictions.eq( "gender", Gender.MALE ) ) + .add( Restrictions.eq( "hairColor", HairColor.BROWN ) ) + .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(); - assertEquals(s.createCriteria( Person.class ) - .add( Restrictions.eq( "gender", Gender.MALE ) ) - .list().size(), 2); - assertEquals(s.createCriteria( Person.class ) - .add( Restrictions.eq( "gender", Gender.MALE ) ) - .add( Restrictions.eq( "hairColor", HairColor.BROWN ) ) - .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().commit(); - s.close(); + @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() ); + } ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/enums/OrdinalEnumTypeTest.java b/hibernate-core/src/test/java/org/hibernate/test/enums/OrdinalEnumTypeTest.java new file mode 100644 index 0000000000..5dcad2d552 --- /dev/null +++ b/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 . + */ +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; + } + } +}