Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to fail on primitives in constructor with failOnNullForPrimitives=true #881

Merged
merged 3 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package example

import io.micronaut.serde.annotation.Serdeable

@Serdeable
data class NonNullDto(
val longField: Long,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

package example

import io.micronaut.serde.annotation.Serdeable

@Serdeable
data class NullDto(
val longField: Long? = null
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

package example

import io.micronaut.serde.annotation.Serdeable

@Serdeable
class NullPropertyDto {
var longField: Long? = null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package example

import io.micronaut.context.annotation.Property
import io.micronaut.json.JsonMapper
import io.micronaut.serde.exceptions.SerdeException
import io.micronaut.test.extensions.junit5.annotation.MicronautTest
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test

@Property(name = "micronaut.serde.deserialization.failOnNullForPrimitives", value = "true")
@MicronautTest
class SerdeNullableFailOnMissingTest {

@Test
fun testDefaultValue(objectMapper: JsonMapper) {
val result = objectMapper.writeValueAsString(NullDto())
val bean = objectMapper.readValue(result, NullDto::class.java)
Assertions.assertEquals(null, bean.longField)
}

@Test
fun testNonNullValue(objectMapper: JsonMapper) {
val e = Assertions.assertThrows(SerdeException::class.java) {
objectMapper.readValue("{}", NonNullDto::class.java)
}
Assertions.assertEquals(
"Unable to deserialize type [example.NonNullDto]. Required constructor parameter [long longField] at index [0] is not present or is null in the supplied data",
e.message
)
}

@Test
fun testNonNullValue2(objectMapper: JsonMapper) {
val e = Assertions.assertThrows(SerdeException::class.java) {
objectMapper.readValue("{\"longField\": null}", NonNullDto::class.java)
}
e.printStackTrace();
Assertions.assertEquals(
"Unable to deserialize type [example.NonNullDto]. Required constructor parameter [long longField] at index [0] is not present or is null in the supplied data",
e.message
)
}

@Test
fun testNullPropertyValue(objectMapper: JsonMapper) {
val bean = objectMapper.readValue("{}", NullPropertyDto::class.java)
Assertions.assertEquals(null, bean.longField)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package example

import io.micronaut.json.JsonMapper
import io.micronaut.test.extensions.junit5.annotation.MicronautTest
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test

@MicronautTest
class SerdeNullableTest {

@Test
fun testDefaultValue(objectMapper: JsonMapper) {
val result = objectMapper.writeValueAsString(NullDto())
val bean = objectMapper.readValue(result, NullDto::class.java)
Assertions.assertEquals(null, bean.longField)
}

@Test
fun testNonNullValue(objectMapper: JsonMapper) {
val bean = objectMapper.readValue("{}", NonNullDto::class.java)
Assertions.assertEquals(0, bean.longField)
}

@Test
fun testNonNullValue2(objectMapper: JsonMapper) {
val bean = objectMapper.readValue("{\"longField\":null}", NonNullDto::class.java)
Assertions.assertEquals(0, bean.longField)
}

@Test
fun testNullPropertyValue(objectMapper: JsonMapper) {
val bean = objectMapper.readValue("{}", NullPropertyDto::class.java)
Assertions.assertEquals(null, bean.longField)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ final class DefaultDeserializationConfiguration implements DeserializationConfig
private final boolean ignoreUnknown;
private final int arraySizeThreshold;
private final boolean strictNullable;
private final boolean failOnNullForPrimitives;

@ConfigurationInject
DefaultDeserializationConfiguration(@Bindable(defaultValue = StringUtils.TRUE) boolean ignoreUnknown,
@Bindable(defaultValue = "100") int arraySizeThreshold,
@Bindable(defaultValue = StringUtils.FALSE) boolean strictNullable) {
@Bindable(defaultValue = StringUtils.FALSE) boolean strictNullable,
@Bindable(defaultValue = StringUtils.FALSE) boolean failOnNullForPrimitives) {
this.ignoreUnknown = ignoreUnknown;
this.arraySizeThreshold = arraySizeThreshold;
this.strictNullable = strictNullable;
this.failOnNullForPrimitives = failOnNullForPrimitives;
}

@Override
Expand All @@ -56,4 +59,9 @@ public int getArraySizeThreshold() {
public boolean isStrictNullable() {
return strictNullable;
}

@Override
public boolean isFailOnNullForPrimitives() {
return failOnNullForPrimitives;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,13 @@ public interface DeserializationConfiguration {
*/
@Bindable(defaultValue = StringUtils.FALSE)
boolean isStrictNullable();

/**
* Whether a null field or a missing value for a primitive should fail the deserialization. Defaults to {@code false}
* @return True if a null field or a missing value for a primitive should fail the deserialization
*/
@Bindable(defaultValue = StringUtils.FALSE)
default boolean isFailOnNullForPrimitives() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ final class DeserBean<T> {
public final int injectPropertiesSize;

public final boolean ignoreUnknown;
public final boolean failOnNullForPrimitives;
public final boolean delegating;
public final boolean simpleBean;
public final boolean recordLikeBean;
Expand Down Expand Up @@ -153,8 +154,11 @@ public DeserBean(DeserializationConfiguration defaultDeserializationConfiguratio
// Replicating Jackson behaviour: @JsonIncludeProperties will ignore any not-included properties
boolean hasIncludedProperties = serdeArgumentConf != null && serdeArgumentConf.getIncluded() != null
|| introspection.isAnnotationPresent(SerdeConfig.SerIncluded.class);
DeserializationConfiguration deserializationConfiguration = decoderContext.getDeserializationConfiguration().orElse(defaultDeserializationConfiguration);
this.ignoreUnknown = hasIncludedProperties || introspection.booleanValue(SerdeConfig.SerIgnored.class, SerdeConfig.SerIgnored.IGNORE_UNKNOWN)
.orElse(decoderContext.getDeserializationConfiguration().orElse(defaultDeserializationConfiguration).isIgnoreUnknown());
.orElse(deserializationConfiguration.isIgnoreUnknown());
this.failOnNullForPrimitives = deserializationConfiguration.isFailOnNullForPrimitives();

final PropertiesBag.Builder<T> creatorPropertiesBuilder = new PropertiesBag.Builder<>(introspection, constructorArguments.length);

BeanMethod<T, Object> jsonValueMethod = null;
Expand Down Expand Up @@ -220,7 +224,8 @@ public DeserBean(DeserializationConfiguration defaultDeserializationConfiguratio
null,
unwrapped,
null,
isIgnored
isIgnored,
failOnNullForPrimitives
);
if (isUnwrapped) {
if (creatorUnwrapped == null) {
Expand Down Expand Up @@ -260,7 +265,8 @@ public DeserBean(DeserializationConfiguration defaultDeserializationConfiguratio
null,
null,
null,
false
false,
failOnNullForPrimitives
);
readPropertiesBuilder.register(jsonProperty, derProperty, true);
}
Expand Down Expand Up @@ -335,7 +341,8 @@ public DeserBean(DeserializationConfiguration defaultDeserializationConfiguratio
null,
unwrapped,
null,
false
false,
failOnNullForPrimitives
);
if (isUnwrapped) {
if (unwrappedProperties == null) {
Expand Down Expand Up @@ -376,7 +383,8 @@ public AnnotationMetadata getAnnotationMetadata() {
jsonSetter,
null,
null,
false
false,
failOnNullForPrimitives
);
readPropertiesBuilder.register(property, derProperty, true);
}
Expand Down Expand Up @@ -702,7 +710,9 @@ public static final class DerProperty<B, P> {
@Nullable
public final P defaultValue;
public final boolean mustSetField;
public final boolean mustSetFieldForConstructor;
public final boolean explicitlyRequired;
public final boolean explicitlyRequiredForConstructor;
public final boolean nonNull;
public final boolean nullable;
public final boolean isAnySetter;
Expand Down Expand Up @@ -732,7 +742,8 @@ public static final class DerProperty<B, P> {
@Nullable BeanMethod<B, P> beanMethod,
@Nullable DeserBean<P> unwrapped,
@Nullable DerProperty<?, ?> unwrappedProperty,
boolean ignored) throws SerdeException {
boolean ignored,
boolean failOnNullForPrimitives) throws SerdeException {
this(conversionService,
introspection,
index,
Expand All @@ -743,7 +754,8 @@ public static final class DerProperty<B, P> {
beanMethod,
unwrapped,
unwrappedProperty,
ignored
ignored,
failOnNullForPrimitives
);
}

Expand All @@ -757,7 +769,8 @@ public static final class DerProperty<B, P> {
@Nullable BeanMethod<B, P> beanMethod,
@Nullable DeserBean<P> unwrapped,
@Nullable DerProperty<?, ?> unwrappedProperty,
boolean ignored) throws SerdeException {
boolean ignored,
boolean failOnNullForPrimitives) throws SerdeException {
this.introspection = introspection;
this.index = index;
this.argument = argument;
Expand All @@ -767,6 +780,7 @@ public static final class DerProperty<B, P> {
|| type.equals(OptionalLong.class)
|| type.equals(OptionalDouble.class)
|| type.equals(OptionalInt.class);
this.mustSetFieldForConstructor = mustSetField || argument.isPrimitive();
this.nonNull = argument.isNonNull();
this.nullable = argument.isNullable();
if (beanProperty != null) {
Expand Down Expand Up @@ -803,6 +817,7 @@ public static final class DerProperty<B, P> {
.orElse(null);
this.explicitlyRequired = annotationMetadata.booleanValue(SerdeConfig.class, SerdeConfig.REQUIRED)
.orElse(false);
this.explicitlyRequiredForConstructor = explicitlyRequired || argument.isPrimitive() && failOnNullForPrimitives;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.explicitlyRequiredForConstructor = explicitlyRequired || argument.isPrimitive() && failOnNullForPrimitives;
this.explicitlyRequiredForConstructor = explicitlyRequired || (argument.isPrimitive() && failOnNullForPrimitives);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Having trouble reading it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, no idea about the precedence rules for the logical operators

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like in math * / AND has a priority :D

}

public void setDefaultPropertyValue(Deserializer.DecoderContext decoderContext, @NonNull B bean) throws SerdeException {
Expand All @@ -817,10 +832,10 @@ public void setDefaultPropertyValue(Deserializer.DecoderContext decoderContext,
}

public void setDefaultConstructorValue(Deserializer.DecoderContext decoderContext, @NonNull Object[] params) throws SerdeException {
if (explicitlyRequired) {
if (explicitlyRequiredForConstructor) {
throw new SerdeException("Unable to deserialize type [" + introspection.getBeanType().getName() + "]. Required constructor parameter [" + argument + "] at index [" + index + "] is not present or is null in the supplied data");
}
params[index] = provideDefaultValue(decoderContext, mustSetField || argument.isPrimitive());
params[index] = provideDefaultValue(decoderContext, mustSetFieldForConstructor);
}

public void set(@NonNull Deserializer.DecoderContext decoderContext, @NonNull B obj, @Nullable P value) throws SerdeException {
Expand All @@ -841,7 +856,7 @@ public void deserializeAndSetConstructorValue(Decoder objectDecoder, Deserialize
}
}

@NextMajorVersion("Receiving a null value for a primitive or a non-null should produce an expection")
@NextMajorVersion("Receiving a null value for a primitive or a non-null should produce an exception")
public void deserializeAndSetPropertyValue(Decoder objectDecoder, Deserializer.DecoderContext decoderContext, B beanInstance) throws IOException {
try {
P value = deserializeValue(objectDecoder, decoderContext);
Expand Down
Loading