hibernate / hibernate-reactive

A reactive API for Hibernate ORM, supporting non-blocking database drivers and a reactive style of interaction with the database.
https://hibernate.org/reactive
Apache License 2.0
432 stars 90 forks source link

Throw java.lang.IllegalArgumentException when merge null value #928

Closed semistone closed 3 years ago

semistone commented 3 years ago

When merge entity into database

public class ShippingCarrier extends BaseOwnerWithParentEntity {

    @EqualsAndHashCode.Exclude
    @Convert(converter = MultiLocaleConverter.class)
    @Column(name = "NAME")
    private MultiLocaleValue<String> name;
}

and Converter is

@Converter
public class MultiLocaleConverter implements AttributeConverter<MultiLocaleValue<String>, JsonObject> {

when insert

session.merge(entity).flatMap(ent -> session.flush().then(Mono.just(ent))));

then it will throw exception

        at java.sql/java.sql.JDBCType.valueOf(JDBCType.java:258)
        at org.hibernate.reactive.adaptor.impl.PreparedStatementAdaptor.setNull(PreparedStatementAdaptor.java:97)
        at org.hibernate.type.descriptor.sql.BasicBinder.bind(BasicBinder.java:60)
        at org.hibernate.type.descriptor.converter.AttributeConverterSqlTypeDescriptorAdapter$1.bind(AttributeConverterSqlTypeDescriptorAdapter.java:88)
        at org.hibernate.type.AbstractStandardBasicType.nullSafeSet(AbstractStandardBasicType.java:276)
        at org.hibernate.type.AbstractStandardBasicType.nullSafeSet(AbstractStandardBasicType.java:271)
        at org.hibernate.type.AbstractSingleColumnStandardBasicType.nullSafeSet(A

I just compare the CR6(OK) and CR9 (NG)

org.hibernate.reactive.adaptor.impl.PreparedStatementAdaptor

CR6

@Override
public void setNull(int parameterIndex, int sqlType)
 { put( parameterIndex, null ); }

CR9

@Override
public void setNull(int parameterIndex, int sqlType)

 { put( parameterIndex, JDBCType.valueOf(sqlType) ); }

it will cause exception, because sqltype is object id

gavinking commented 3 years ago

We're missing the full code of MultiLocaleConverter.

Anyway, it seems to me that this code is something I would not expect to work in the current release of HR, since JsonObject isn't a built-in type.

OTOH, after #869, perhaps it works against current main.

gavinking commented 3 years ago

Wait, @DavideD, did #869 go out in the latest release?

DavideD commented 3 years ago

Yes, #869 is in the latest release: 1.0.0.CR9

semistone commented 3 years ago

this is our MultiLocaleConverter


import ....
import io.vertx.core.json.JsonObject;
/**
 * The attribute converter for the MultiLocaleValue
 */
@Converter
public class MultiLocaleConverter implements AttributeConverter<MultiLocaleValue<String>, JsonObject> {

    private final TypeToken<MultiLocaleValue<String>> typeToken = new TypeToken<>(getClass()) {
    };
    private final Class<MultiLocaleValue<String>> typeClass = uncheckedCast(typeToken.getRawType());

    @Override
    public JsonObject convertToDatabaseColumn(MultiLocaleValue<String> name) {

        if (name == null || MapUtils.isEmpty(name.getValues())) {
            return null;
        }

        return JsonObject.mapFrom(name);
    }

    @Override
    public MultiLocaleValue<String> convertToEntityAttribute(JsonObject dbData) {

        if (dbData == null) {
            return null;
        }
        MultiLocaleValue<String> multiLocaleValue;
        try {
            multiLocaleValue = dbData.mapTo(typeClass);
        } catch (IllegalArgumentException ex) {
            throw ValidationException.create(
                    INPUT_STREAM_READING_ERROR,
                    new DefaultErrorStatus<>(REST, UNPROCESSABLE_ENTITY.getCode(), null),
                    null);
        }
        return multiLocaleValue;
    }
}

but it actually doesn't matter in this case, because that value is null in that testcase.

gavinking commented 3 years ago

OK, I've fixed this bug in #942.