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.

Sign up for GitHub

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 deserialize an empty string to null #630

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Allow to deserialize an empty string to null #630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b523547
Allow to deserialize an empty string to null
dstepanov Nov 1, 2023
bb33cf6
Use `isEmpty`
dstepanov Nov 2, 2023
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
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[versions]
micronaut = "4.1.10"
micronaut = "4.1.11"
micronaut-platform = "4.0.0"
micronaut-docs = "2.0.0"
micronaut-gradle-plugin = "4.1.2"
Expand Down
3 changes: 0 additions & 3 deletions serde-bson/src/test/groovy/io/micronaut/serde/bson/BsonBinaryBasicSerdeCompileSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@ package io.micronaut.serde.bson

import io.micronaut.json.JsonMapper
import io.micronaut.serde.AbstractBasicSerdeCompileSpec
import io.micronaut.serde.AbstractBasicSerdeSpec
import io.micronaut.test.extensions.spock.annotation.MicronautTest
import jakarta.inject.Inject
import org.bson.BsonDocument

class BsonBinaryBasicSerdeCompileSpec extends AbstractBasicSerdeCompileSpec implements BsonBinarySpec {
Expand Down
65 changes: 58 additions & 7 deletions serde-jackson/src/main/java/io/micronaut/serde/jackson/JacksonDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,17 @@ private Boolean decodeBooleanSlow() throws IOException {
return null;
}
case START_OBJECT, END_OBJECT, END_ARRAY, FIELD_NAME -> throw unexpectedToken(JsonToken.VALUE_TRUE, t);
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use?

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

Copy link
Member

Choose a reason for hiding this comment

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

why?

return null;
}
try {
return Boolean.parseBoolean(string);
} catch (NumberFormatException e) {
throw createDeserializationException("Unable to coerce string to boolean", string);
}
}
default -> {
return parser.getValueAsBoolean();
}
Expand All @@ -312,6 +323,17 @@ public byte decodeByte() throws IOException {
public Byte decodeByteNullable() throws IOException {
JsonToken t = nextToken();
switch (t) {
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
return Byte.parseByte(string);
} catch (NumberFormatException e) {
throw createDeserializationException("Unable to coerce string to byte", string);
}
}
case VALUE_TRUE -> {
return 1;
}
Expand Down Expand Up @@ -353,6 +375,17 @@ public short decodeShort() throws IOException {
public Short decodeShortNullable() throws IOException {
JsonToken t = nextToken();
switch (t) {
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
return Short.parseShort(string);
} catch (NumberFormatException e) {
throw createDeserializationException("Unable to coerce string to short", string);
}
}
case VALUE_TRUE -> {
return 1;
}
Expand Down Expand Up @@ -407,6 +440,9 @@ public Character decodeCharNullable() throws IOException {
}
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
if (string.length() != 1) {
throw createDeserializationException("When decoding char value, must give a single character", string);
}
Expand Down Expand Up @@ -460,6 +496,9 @@ private Integer decodeIntSlow() throws IOException {
}
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
return Integer.parseInt(string);
} catch (NumberFormatException e) {
Expand Down Expand Up @@ -528,13 +567,14 @@ private Long decodeLongSlow() throws IOException {
}
case VALUE_STRING -> {
String string = parser.getText();
long value;
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
value = Long.parseLong(string);
return Long.parseLong(string);
} catch (NumberFormatException e) {
throw createDeserializationException("Unable to coerce string to integer", string);
}
return value;
}
case VALUE_FALSE -> {
return 0L;
Expand Down Expand Up @@ -579,13 +619,14 @@ public Float decodeFloatNullable() throws IOException {
switch (t) {
case VALUE_STRING -> {
String string = parser.getText();
float value;
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
value = Float.parseFloat(string);
return Float.parseFloat(string);
} catch (NumberFormatException e) {
throw createDeserializationException("Unable to coerce string to float", string);
}
return value;
}
case START_ARRAY -> {
if (beginUnwrapArray(t)) {
Expand Down Expand Up @@ -641,6 +682,9 @@ private Double decodeDoubleSlow(JsonToken t) throws IOException {
}
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
return Double.parseDouble(string);
} catch (NumberFormatException e) {
Expand Down Expand Up @@ -691,6 +735,9 @@ public BigInteger decodeBigIntegerNullable() throws IOException {
switch (t) {
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
return new BigInteger(string);
} catch (NumberFormatException e) {
Expand Down Expand Up @@ -741,6 +788,9 @@ public BigDecimal decodeBigDecimalNullable() throws IOException {
switch (t) {
case VALUE_STRING -> {
String string = parser.getText();
if (string.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (string.isEmpty()) {
if (StringUtils.isEmpty(string)) {

return null;
}
try {
return new BigDecimal(string);
} catch (NumberFormatException e) {
Expand Down Expand Up @@ -793,7 +843,8 @@ private Number decodeNumber() throws IOException {

@Override
public boolean decodeNull() throws IOException {
if (peekToken() == JsonToken.VALUE_NULL) {
JsonToken jsonToken = peekToken();
if (jsonToken == JsonToken.VALUE_NULL || jsonToken == JsonToken.VALUE_STRING && parser.getText().isEmpty()) {
nextToken();
return true;
} else {
Expand Down
34 changes: 29 additions & 5 deletions serde-support/src/main/java/io/micronaut/serde/support/AbstractStreamDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
public abstract class AbstractStreamDecoder extends LimitingStream implements Decoder {

private boolean currentlyUnwrappingArray = false;
private String readString;

public AbstractStreamDecoder(@NonNull RemainingLimits remainingLimits) {
super(remainingLimits);
Expand Down Expand Up @@ -257,7 +258,13 @@ public String decodeString() throws IOException {
preDecodeValue(currentToken);
switch (currentToken) {
case STRING:
String text = getString();
String text;
if (readString == null) {
text = getString();
} else {
text = readString;
readString = null;
}
nextToken();
return text;
case NUMBER:
Expand Down Expand Up @@ -733,18 +740,35 @@ protected final <T> T decodeCustom(ValueDecoder<T> readFunction, boolean callNex
return value;
}

@Override
public String decodeStringNullable() throws IOException {
TokenType currentToken = currentToken();
preDecodeValue(currentToken);
if (currentToken == TokenType.NULL) {
nextToken();
return null;
}
return decodeString();
}

@Override
public final boolean decodeNull() throws IOException {
TokenType currentToken = currentToken();
preDecodeValue(currentToken);
if (currentToken == TokenType.NULL) {
nextToken();
return true;
} else {
// we don't support unwrapping null values from arrays, because the api user wouldn't be able to distinguish
// `[null]` and `null` anymore.
return false;
} else if (currentToken == TokenType.STRING) {
readString = getString();
if (readString.isEmpty()) {
readString = null;
nextToken();
return true;
}
}
// we don't support unwrapping null values from arrays, because the api user wouldn't be able to distinguish
// `[null]` and `null` anymore.
return false;
}

@Override
Expand Down
27 changes: 17 additions & 10 deletions serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,7 @@ public static final class DerProperty<B, P> {
public final P defaultValue;
public final boolean mustSetField;
public final boolean explicitlyRequired;
public final boolean primitive;
public final boolean nonNull;
public final boolean nullable;
public final boolean isAnySetter;
Expand Down Expand Up @@ -802,6 +803,7 @@ public DerProperty(ConversionService conversionService,
} else {
this.beanProperty = null;
}
this.primitive = argument.isPrimitive();
// compute default
AnnotationMetadata annotationMetadata = resolveArgumentMetadata(instrospection, argument, argumentMetadata);
this.views = SerdeAnnotationUtil.resolveViews(instrospection, annotationMetadata);
Expand Down Expand Up @@ -909,18 +911,23 @@ public void deserializeAndSetConstructorValue(Decoder objectDecoder, Deserialize
public void deserializeAndSetPropertyValue(Decoder objectDecoder, Deserializer.DecoderContext decoderContext, B beanInstance) throws IOException {
try {
P value = deserializer.deserializeNullable(objectDecoder, decoderContext, argument);
if (value == null && !nullable) {
if (!explicitlyRequired) {
value = defaultValue;
if (value == null) {
if (!mustSetField) {
return;
if (value == null) {
if (primitive) {
return;
}
if (!nullable) {
if (!explicitlyRequired) {
value = defaultValue;
if (value == null) {
if (!mustSetField) {
return;
}
value = deserializer.getDefaultValue(decoderContext, argument);
}
value = deserializer.getDefaultValue(decoderContext, argument);
} else {
throw new SerdeException("Unable to deserialize type [" + instrospection.getBeanType().getName() + "]. Required property [" + argument +
"] is not present in supplied data");
}
} else {
throw new SerdeException("Unable to deserialize type [" + instrospection.getBeanType().getName() + "]. Required property [" + argument +
"] is not present in supplied data");
}
}
beanProperty.setUnsafe(beanInstance, value);
Expand Down
1 change: 0 additions & 1 deletion ...e-support/src/main/java/io/micronaut/serde/support/serializers/RuntimeTypeSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ private SerdeException serializeIntoNotSupported(Argument<?> type) {
return new SerdeException("Serializer for type: " + type + " doesn't support serializing into an existing object");
}


@Override
public boolean isEmpty(EncoderContext context, Object value) {
if (value == null) {
Expand Down
58 changes: 58 additions & 0 deletions serde-tck/src/main/groovy/io/micronaut/serde/AbstractBasicSerdeCompileSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package io.micronaut.serde

import io.micronaut.core.type.Argument
import io.micronaut.http.HttpStatus
import spock.lang.Unroll

Expand Down Expand Up @@ -92,6 +93,63 @@ class Test {
Locale | [value: Locale.CANADA_FRENCH] | '{"value":"fr-CA"}'
}

@Unroll
void "test empty string to #type"() {
given:
def context = buildContext('test.Test', """
package test;

import io.micronaut.serde.annotation.Serdeable;

@Serdeable
class Test {
@io.micronaut.core.annotation.Nullable
private $type.name value;
public void setValue($type.name value) {
this.value = value;
}
public $type.name getValue() {
return value;
}
}
""", true)
when:
def jsonMapper = context.getBean(getJsonMapperClass())
typeUnderTest = argumentOf(context, 'test.Test')
byte[] bytes = jsonMapper.writeValueAsBytes(Argument.mapOf(String.class, String.class), Map.of("value", ""))
def read = jsonMapper.readValue(bytes, typeUnderTest)
typeUnderTest.type.isInstance(read)

then:
read.value == result

cleanup:
context.close()

where:
type | result
BigDecimal | null
BigInteger | null
String | ""
boolean | false
byte | 0
short | 0
int | 0
long | 0
double | 0
float | 0
char | 0
//wrappers
Boolean | null
Byte | null
Short | null
Integer | null
Long | null
Double | null
Float | null
Character | null
}

@Unroll
void "test basic type #type missing value"() {
given:
Expand Down
9 changes: 6 additions & 3 deletions ...etty/src/test/java/io/micronaut/serde/http/server/tck/netty/NettyHttpServerTestSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
import org.junit.platform.suite.api.SuiteDisplayName;

@Suite
@ExcludeClassNamePatterns("io.micronaut.http.server.tck.tests.constraintshandler.ControllerConstraintHandlerTest")
@ExcludeClassNamePatterns({
"io.micronaut.http.server.tck.tests.constraintshandler.ControllerConstraintHandlerTest",
"io.micronaut.http.server.tck.tests.forms.FormsSubmissionsWithListsTest" // Support converting a value to a collection
})
@SelectPackages({
"io.micronaut.http.server.tck.tests",
"io.micronaut.serde.http.server.tck.netty"
"io.micronaut.http.server.tck.tests",
"io.micronaut.serde.http.server.tck.netty"
})
@SuiteDisplayName("HTTP Server TCK for Netty")
public class NettyHttpServerTestSuite {
Expand Down