From 32020acbb7f45dac5f9fb66ddafa2dd7ecaa2247 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Mon, 6 Nov 2023 15:43:08 +0100 Subject: [PATCH] Revert "Allow to deserialize an empty string to null (#630)" This reverts commit 017c03e66e05dda237e3346013ab5c03ff166b41. --- gradle/libs.versions.toml | 2 +- .../BsonBinaryBasicSerdeCompileSpec.groovy | 3 + .../serde/jackson/JacksonDecoder.java | 65 ++----------------- .../serde/support/AbstractStreamDecoder.java | 34 ++-------- .../support/deserializers/DeserBean.java | 27 +++----- .../serializers/RuntimeTypeSerializer.java | 1 + .../AbstractBasicSerdeCompileSpec.groovy | 58 ----------------- .../tck/netty/NettyHttpServerTestSuite.java | 9 +-- 8 files changed, 30 insertions(+), 169 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ef93a312f..cbc9549a1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,5 +1,5 @@ [versions] -micronaut = "4.1.11" +micronaut = "4.1.10" micronaut-platform = "4.0.0" micronaut-docs = "2.0.0" micronaut-gradle-plugin = "4.1.2" diff --git a/serde-bson/src/test/groovy/io/micronaut/serde/bson/BsonBinaryBasicSerdeCompileSpec.groovy b/serde-bson/src/test/groovy/io/micronaut/serde/bson/BsonBinaryBasicSerdeCompileSpec.groovy index 6a422833a..6171e3181 100644 --- a/serde-bson/src/test/groovy/io/micronaut/serde/bson/BsonBinaryBasicSerdeCompileSpec.groovy +++ b/serde-bson/src/test/groovy/io/micronaut/serde/bson/BsonBinaryBasicSerdeCompileSpec.groovy @@ -2,6 +2,9 @@ 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 { diff --git a/serde-jackson/src/main/java/io/micronaut/serde/jackson/JacksonDecoder.java b/serde-jackson/src/main/java/io/micronaut/serde/jackson/JacksonDecoder.java index 4d9a8e6f2..14b7becc5 100644 --- a/serde-jackson/src/main/java/io/micronaut/serde/jackson/JacksonDecoder.java +++ b/serde-jackson/src/main/java/io/micronaut/serde/jackson/JacksonDecoder.java @@ -292,17 +292,6 @@ 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()) { - return null; - } - try { - return Boolean.parseBoolean(string); - } catch (NumberFormatException e) { - throw createDeserializationException("Unable to coerce string to boolean", string); - } - } default -> { return parser.getValueAsBoolean(); } @@ -323,17 +312,6 @@ 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()) { - return null; - } - try { - return Byte.parseByte(string); - } catch (NumberFormatException e) { - throw createDeserializationException("Unable to coerce string to byte", string); - } - } case VALUE_TRUE -> { return 1; } @@ -375,17 +353,6 @@ 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()) { - return null; - } - try { - return Short.parseShort(string); - } catch (NumberFormatException e) { - throw createDeserializationException("Unable to coerce string to short", string); - } - } case VALUE_TRUE -> { return 1; } @@ -440,9 +407,6 @@ public Character decodeCharNullable() throws IOException { } case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } if (string.length() != 1) { throw createDeserializationException("When decoding char value, must give a single character", string); } @@ -496,9 +460,6 @@ private Integer decodeIntSlow() throws IOException { } case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } try { return Integer.parseInt(string); } catch (NumberFormatException e) { @@ -567,14 +528,13 @@ private Long decodeLongSlow() throws IOException { } case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } + long value; try { - return Long.parseLong(string); + value = Long.parseLong(string); } catch (NumberFormatException e) { throw createDeserializationException("Unable to coerce string to integer", string); } + return value; } case VALUE_FALSE -> { return 0L; @@ -619,14 +579,13 @@ public Float decodeFloatNullable() throws IOException { switch (t) { case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } + float value; try { - return Float.parseFloat(string); + value = Float.parseFloat(string); } catch (NumberFormatException e) { throw createDeserializationException("Unable to coerce string to float", string); } + return value; } case START_ARRAY -> { if (beginUnwrapArray(t)) { @@ -682,9 +641,6 @@ private Double decodeDoubleSlow(JsonToken t) throws IOException { } case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } try { return Double.parseDouble(string); } catch (NumberFormatException e) { @@ -735,9 +691,6 @@ public BigInteger decodeBigIntegerNullable() throws IOException { switch (t) { case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } try { return new BigInteger(string); } catch (NumberFormatException e) { @@ -788,9 +741,6 @@ public BigDecimal decodeBigDecimalNullable() throws IOException { switch (t) { case VALUE_STRING -> { String string = parser.getText(); - if (string.isEmpty()) { - return null; - } try { return new BigDecimal(string); } catch (NumberFormatException e) { @@ -843,8 +793,7 @@ private Number decodeNumber() throws IOException { @Override public boolean decodeNull() throws IOException { - JsonToken jsonToken = peekToken(); - if (jsonToken == JsonToken.VALUE_NULL || jsonToken == JsonToken.VALUE_STRING && parser.getText().isEmpty()) { + if (peekToken() == JsonToken.VALUE_NULL) { nextToken(); return true; } else { diff --git a/serde-support/src/main/java/io/micronaut/serde/support/AbstractStreamDecoder.java b/serde-support/src/main/java/io/micronaut/serde/support/AbstractStreamDecoder.java index 6239036f7..31c14ee36 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/AbstractStreamDecoder.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/AbstractStreamDecoder.java @@ -41,7 +41,6 @@ public abstract class AbstractStreamDecoder extends LimitingStream implements Decoder { private boolean currentlyUnwrappingArray = false; - private String readString; public AbstractStreamDecoder(@NonNull RemainingLimits remainingLimits) { super(remainingLimits); @@ -258,13 +257,7 @@ public String decodeString() throws IOException { preDecodeValue(currentToken); switch (currentToken) { case STRING: - String text; - if (readString == null) { - text = getString(); - } else { - text = readString; - readString = null; - } + String text = getString(); nextToken(); return text; case NUMBER: @@ -740,17 +733,6 @@ protected final T decodeCustom(ValueDecoder 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(); @@ -758,17 +740,11 @@ public final boolean decodeNull() throws IOException { if (currentToken == TokenType.NULL) { nextToken(); return true; - } else if (currentToken == TokenType.STRING) { - readString = getString(); - if (readString.isEmpty()) { - readString = 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; } - // 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 diff --git a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java index d76e70237..6c0d20161 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java @@ -792,7 +792,6 @@ public static final class DerProperty { 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; @@ -861,7 +860,6 @@ 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); @@ -969,23 +967,18 @@ 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) { - if (primitive) { - return; - } - if (!nullable) { - if (!explicitlyRequired) { - value = defaultValue; - if (value == null) { - if (!mustSetField) { - return; - } - value = deserializer.getDefaultValue(decoderContext, argument); + if (value == null && !nullable) { + if (!explicitlyRequired) { + value = defaultValue; + if (value == null) { + if (!mustSetField) { + return; } - } else { - throw new SerdeException("Unable to deserialize type [" + instrospection.getBeanType().getName() + "]. Required property [" + argument + - "] is not present in supplied data"); + 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"); } } beanProperty.setUnsafe(beanInstance, value); diff --git a/serde-support/src/main/java/io/micronaut/serde/support/serializers/RuntimeTypeSerializer.java b/serde-support/src/main/java/io/micronaut/serde/support/serializers/RuntimeTypeSerializer.java index be0104373..5695031bc 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/serializers/RuntimeTypeSerializer.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/serializers/RuntimeTypeSerializer.java @@ -100,6 +100,7 @@ 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) { diff --git a/serde-tck/src/main/groovy/io/micronaut/serde/AbstractBasicSerdeCompileSpec.groovy b/serde-tck/src/main/groovy/io/micronaut/serde/AbstractBasicSerdeCompileSpec.groovy index d3b092806..5748e1ff6 100644 --- a/serde-tck/src/main/groovy/io/micronaut/serde/AbstractBasicSerdeCompileSpec.groovy +++ b/serde-tck/src/main/groovy/io/micronaut/serde/AbstractBasicSerdeCompileSpec.groovy @@ -15,7 +15,6 @@ */ package io.micronaut.serde -import io.micronaut.core.type.Argument import io.micronaut.http.HttpStatus import spock.lang.Unroll @@ -93,63 +92,6 @@ 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: diff --git a/test-suite-http-server-tck-netty/src/test/java/io/micronaut/serde/http/server/tck/netty/NettyHttpServerTestSuite.java b/test-suite-http-server-tck-netty/src/test/java/io/micronaut/serde/http/server/tck/netty/NettyHttpServerTestSuite.java index 14fdff312..98e4e40b5 100644 --- a/test-suite-http-server-tck-netty/src/test/java/io/micronaut/serde/http/server/tck/netty/NettyHttpServerTestSuite.java +++ b/test-suite-http-server-tck-netty/src/test/java/io/micronaut/serde/http/server/tck/netty/NettyHttpServerTestSuite.java @@ -21,13 +21,10 @@ import org.junit.platform.suite.api.SuiteDisplayName; @Suite -@ExcludeClassNamePatterns({ - "io.micronaut.http.server.tck.tests.constraintshandler.ControllerConstraintHandlerTest", - "io.micronaut.http.server.tck.tests.forms.FormsSubmissionsWithListsTest" // Support converting a value to a collection -}) +@ExcludeClassNamePatterns("io.micronaut.http.server.tck.tests.constraintshandler.ControllerConstraintHandlerTest") @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 {