From b139d449c86538ab86b4113fdaaf0fa5435cf215 Mon Sep 17 00:00:00 2001 From: datazuul Date: Fri, 23 Feb 2024 13:43:38 +0100 Subject: [PATCH] HHH-17275: Fix NPE in BooleanJavaType for converter returning NULL for relational value --- .../java/org/hibernate/dialect/Dialect.java | 45 ++++++- .../type/descriptor/java/BooleanJavaType.java | 14 ++- .../java/BooleanJavaTypeDescriptorTest.java | 119 +++++++++++++++++- 3 files changed, 168 insertions(+), 10 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index 4d041cd6ba..e89d6e9ab9 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -808,7 +808,8 @@ public abstract class Dialect implements ConversionContext, TypeContributor, Fun /** * Render a SQL check condition for a column that represents an enumerated value - * by its {@linkplain jakarta.persistence.EnumType#STRING string representation}. + * by its {@linkplain jakarta.persistence.EnumType#STRING string representation} + * or a given list of values (with NULL value allowed). * * @return a SQL expression that will occur in a {@code check} constraint */ @@ -816,11 +817,20 @@ public abstract class Dialect implements ConversionContext, TypeContributor, Fun StringBuilder check = new StringBuilder(); check.append( columnName ).append( " in (" ); String separator = ""; + boolean nullIsValid = false; for ( String value : values ) { + if ( value == null ) { + nullIsValid = true; + continue; + } check.append( separator ).append('\'').append( value ).append('\''); separator = ","; } - return check.append( ')' ).toString(); + check.append( ')' ); + if ( nullIsValid ) { + check.append( " or " ).append( columnName ).append( " is null" ); + } + return check.toString(); } public String getCheckCondition(String columnName, Class> enumType) { @@ -842,16 +852,43 @@ public abstract class Dialect implements ConversionContext, TypeContributor, Fun * by its {@linkplain jakarta.persistence.EnumType#ORDINAL ordinal representation}. * * @return a SQL expression that will occur in a {@code check} constraint + * @deprecated use {@link #getCheckCondition(String, Long[])} instead */ + @Deprecated(forRemoval = true) public String getCheckCondition(String columnName, long[] values) { + Long objValues [] = new Long[ values.length ]; + int i = 0; + for( long temp : values){ + objValues[ i++ ] = temp; + } + return getCheckCondition(columnName, objValues); + } + + /** + * Render a SQL check condition for a column that represents an enumerated value + * by its {@linkplain jakarta.persistence.EnumType#ORDINAL ordinal representation} + * or a given list of values. + * + * @return a SQL expression that will occur in a {@code check} constraint + */ + public String getCheckCondition(String columnName, Long[] values) { StringBuilder check = new StringBuilder(); check.append( columnName ).append( " in (" ); String separator = ""; - for ( long value : values ) { + boolean nullIsValid = false; + for ( Long value : values ) { + if ( value == null ) { + nullIsValid = true; + continue; + } check.append( separator ).append( value ); separator = ","; } - return check.append( ')' ).toString(); + check.append( ')' ); + if ( nullIsValid ) { + check.append( " or " ).append( columnName ).append( " is null" ); + } + return check.toString(); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java index b83a44643b..e8487b35e7 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java @@ -191,9 +191,11 @@ public class BooleanJavaType extends AbstractClassJavaType implements @SuppressWarnings("unchecked") BasicValueConverter stringConverter = (BasicValueConverter) converter; + final Object falseValue = stringConverter.toRelationalValue( false ); + final Object trueValue = stringConverter.toRelationalValue( true ); String[] values = new String[] { - stringConverter.toRelationalValue(false).toString(), - stringConverter.toRelationalValue(true).toString() + falseValue != null ? falseValue.toString() : null, + trueValue != null ? trueValue.toString() : null }; return dialect.getCheckCondition( columnName, values ); } @@ -201,9 +203,11 @@ public class BooleanJavaType extends AbstractClassJavaType implements @SuppressWarnings("unchecked") BasicValueConverter numericConverter = (BasicValueConverter) converter; - long[] values = new long[] { - numericConverter.toRelationalValue(false).longValue(), - numericConverter.toRelationalValue(true).longValue() + final Number falseValue = numericConverter.toRelationalValue( false ); + final Number trueValue = numericConverter.toRelationalValue( true ); + Long[] values = new Long[] { + falseValue != null ? Long.valueOf(falseValue.longValue()) : null, + trueValue != null ? Long.valueOf(trueValue.longValue()) : null }; return dialect.getCheckCondition( columnName, values ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java index 967f8041e3..df7831d4f5 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java @@ -6,8 +6,19 @@ */ package org.hibernate.orm.test.mapping.type.java; +import jakarta.persistence.AttributeConverter; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.hibernate.dialect.Dialect; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.type.NumericBooleanConverter; +import org.hibernate.type.TrueFalseConverter; +import org.hibernate.type.descriptor.converter.spi.BasicValueConverter; import org.hibernate.type.descriptor.java.BooleanJavaType; - +import org.hibernate.type.descriptor.java.IntegerJavaType; +import org.hibernate.type.descriptor.java.JavaType; +import org.hibernate.type.descriptor.java.StringJavaType; +import org.hibernate.type.descriptor.jdbc.IntegerJdbcType; +import org.hibernate.type.descriptor.jdbc.VarcharJdbcType; import org.junit.Test; import static org.junit.Assert.*; @@ -15,6 +26,43 @@ import static org.junit.Assert.*; public class BooleanJavaTypeDescriptorTest { private BooleanJavaType underTest = new BooleanJavaType(); + @Test + @JiraKey( "HHH-17275" ) + public void testCheckConditionShouldReturnCorrectStatementWhenNullXStringGiven() { + // given + // when + String checkCondition = underTest.getCheckCondition("is_active", VarcharJdbcType.INSTANCE, new BooleanXConverter(), new AnyDialect()); + // then + assertEquals("is_active in ('X') or is_active is null", checkCondition); + } + + @Test + public void testCheckConditionShouldReturnCorrectStatementWhenTFStringGiven() { + // given + // when + String checkCondition = underTest.getCheckCondition("is_active", VarcharJdbcType.INSTANCE, new TrueFalseConverter(), new AnyDialect()); + // then + assertEquals("is_active in ('F','T')", checkCondition); + } + + @Test + public void testCheckConditionShouldReturnCorrectStatementWhen1AndNullIntegerGiven() { + // given + // when + String checkCondition = underTest.getCheckCondition("is_active", IntegerJdbcType.INSTANCE, new OneNullBooleanConverter(), new AnyDialect()); + // then + assertEquals("is_active in (1) or is_active is null", checkCondition); + } + + @Test + public void testCheckConditionShouldReturnCorrectStatementWhen1And0IntegerGiven() { + // given + // when + String checkCondition = underTest.getCheckCondition("is_active", IntegerJdbcType.INSTANCE, new NumericBooleanConverter(), new AnyDialect()); + // then + assertEquals("is_active in (0,1)", checkCondition); + } + @Test public void testWrapShouldReturnTrueWhenYStringGiven() { // given @@ -59,4 +107,73 @@ public class BooleanJavaTypeDescriptorTest { // then assertFalse(result); } + + private static class AnyDialect extends Dialect { + + } + + private static class BooleanXConverter implements AttributeConverter, BasicValueConverter { + + @Override + public String convertToDatabaseColumn(Boolean value) { + return value != null && value ? "X" : null; + } + + @Override + public Boolean convertToEntityAttribute(String value) { + return value != null; + } + + @Override + public @Nullable Boolean toDomainValue(@Nullable String relationalForm) { + return convertToEntityAttribute(relationalForm); + } + + @Override + public @Nullable String toRelationalValue(@Nullable Boolean domainForm) { + return convertToDatabaseColumn(domainForm); + } + + @Override + public JavaType getDomainJavaType() { + return BooleanJavaType.INSTANCE; + } + + @Override + public JavaType getRelationalJavaType() { + return StringJavaType.INSTANCE; + } + } + + private static class OneNullBooleanConverter implements AttributeConverter, BasicValueConverter { + @Override + public Integer convertToDatabaseColumn(Boolean attribute) { + return attribute != null && attribute ? 1 : null; + } + + @Override + public Boolean convertToEntityAttribute(Integer dbData) { + return dbData != null && dbData == 1; + } + + @Override + public @Nullable Boolean toDomainValue(@Nullable Integer relationalForm) { + return convertToEntityAttribute(relationalForm); + } + + @Override + public @Nullable Integer toRelationalValue(@Nullable Boolean domainForm) { + return convertToDatabaseColumn(domainForm); + } + + @Override + public JavaType getDomainJavaType() { + return BooleanJavaType.INSTANCE; + } + + @Override + public JavaType getRelationalJavaType() { + return IntegerJavaType.INSTANCE; + } + } } \ No newline at end of file