diff --git a/pom.xml b/pom.xml index 1377df2f..eeebf271 100644 --- a/pom.xml +++ b/pom.xml @@ -89,13 +89,9 @@ - com.fasterxml.jackson.core + tools.jackson.core jackson-databind - - com.fasterxml.jackson.datatype - jackson-datatype-jsr310 - org.apache.commons diff --git a/src/main/java/sirius/kernel/commons/Amount.java b/src/main/java/sirius/kernel/commons/Amount.java index 98a3f9ee..01cd2806 100644 --- a/src/main/java/sirius/kernel/commons/Amount.java +++ b/src/main/java/sirius/kernel/commons/Amount.java @@ -11,6 +11,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; import sirius.kernel.nls.NLS; +import tools.jackson.core.JacksonException; +import tools.jackson.core.JsonGenerator; +import tools.jackson.databind.SerializationContext; +import tools.jackson.databind.ValueSerializer; +import tools.jackson.databind.annotation.JsonSerialize; import javax.annotation.CheckReturnValue; import javax.annotation.Nonnull; @@ -46,6 +51,7 @@ /// @see NumberFormat /// @see BigDecimal @Immutable +@JsonSerialize(using = Amount.JacksonSerializer.class) public class Amount extends Number implements Comparable { @Serial @@ -262,6 +268,11 @@ public BigDecimal fetchAmountWithoutTrailingZeros() { /// Unwraps the internally used BigDecimal with rounding like in [#toMachineString()] applied. /// This is used for Jackson Object Mapping. /// + /// Note that Jackson 3 ignores `@JsonValue` on subclasses of [Number] (the standard number serializer takes + /// precedence), therefore [JacksonSerializer] is registered via `@JsonSerialize` on the class, which is consulted + /// before the standard handling. The annotation is kept for documentation purposes and in case Jackson restores its + /// precedence. + /// /// @return the internally used BigDecimal with rounding applied @Nullable @JsonValue @@ -273,6 +284,34 @@ private BigDecimal getRoundedAmount() { } } + /// Serializes an [Amount] as its rounded BigDecimal value (or null for [#NOTHING]), replicating + /// the `@JsonValue` behavior of Jackson 2 which no longer applies to subclasses of [Number] in Jackson 3. + public static class JacksonSerializer extends ValueSerializer { + /** + * @param value Value to serialize; can not be null. + * @param generator Generator used to output resulting JSON content + * @param context Context that can be used to get serializers for + * serializing Objects value contains, if any. + * @throws JacksonException when the serialization fails + */ + @Override + public void serialize(Amount value, JsonGenerator generator, SerializationContext context) { + BigDecimal roundedAmount = value.getRoundedAmount(); + if (roundedAmount == null) { + generator.writeNull(); + } else { + generator.writeNumber(roundedAmount); + } + } + + /// Considers [#NOTHING] as empty, so that `@JsonInclude(NON_EMPTY)` suppresses empty amounts + /// like the `@JsonValue` based serialization of Jackson 2 did. + @Override + public boolean isEmpty(SerializationContext context, Amount value) { + return value == null || value.isEmpty(); + } + } + @Override public int intValue() { throwExceptionIfEmpty(); diff --git a/src/main/java/sirius/kernel/commons/Json.java b/src/main/java/sirius/kernel/commons/Json.java index df7d4554..69508262 100644 --- a/src/main/java/sirius/kernel/commons/Json.java +++ b/src/main/java/sirius/kernel/commons/Json.java @@ -8,20 +8,21 @@ package sirius.kernel.commons; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonPointer; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadConstraints; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.SerializationFeature; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.fasterxml.jackson.databind.node.POJONode; -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import sirius.kernel.health.Exceptions; import sirius.kernel.health.Log; +import tools.jackson.core.JacksonException; +import tools.jackson.core.JsonPointer; +import tools.jackson.core.StreamReadConstraints; +import tools.jackson.core.json.JsonFactory; +import tools.jackson.core.json.JsonReadFeature; +import tools.jackson.core.type.TypeReference; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectMapper; +import tools.jackson.databind.cfg.DateTimeFeature; +import tools.jackson.databind.json.JsonMapper; +import tools.jackson.databind.node.ArrayNode; +import tools.jackson.databind.node.ObjectNode; +import tools.jackson.databind.node.POJONode; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -57,16 +58,17 @@ public class Json { *

* Note that this mapper is configured to allow single quotes and unquoted field names when parsing JSON strings. */ - public static final ObjectMapper MAPPER = - new ObjectMapper().configure(JsonParser.Feature.ALLOW_UNQUOTED_FIELD_NAMES, true) - .configure(JsonParser.Feature.ALLOW_SINGLE_QUOTES, true) - .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) - .registerModule(new JavaTimeModule()); - - static { - MAPPER.getFactory() - .setStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(25_000_000).build()); - } + public static final ObjectMapper MAPPER = JsonMapper.builder(JsonFactory.builderWithJackson2Defaults() + .enable(JsonReadFeature.ALLOW_UNQUOTED_PROPERTY_NAMES, + JsonReadFeature.ALLOW_SINGLE_QUOTES) + .streamReadConstraints(StreamReadConstraints.builder() + .maxStringLength( + 25_000_000) + .build()) + .build()) + .configureForJackson2() + .disable(DateTimeFeature.WRITE_DATES_AS_TIMESTAMPS) + .build(); private Json() { } @@ -126,7 +128,7 @@ public static ArrayNode createArray(@Nonnull Collection elements) { public static ObjectNode parseObject(String json) { try { return tryParseObject(json); - } catch (JsonProcessingException exception) { + } catch (JacksonException exception) { throw Exceptions.handle(LOG, exception); } } @@ -136,9 +138,9 @@ public static ObjectNode parseObject(String json) { * * @param json the JSON string to parse * @return the parsed JSON string as object node - * @throws JsonProcessingException in case of a malformed JSON string + * @throws JacksonException in case of a malformed JSON string */ - public static ObjectNode tryParseObject(String json) throws JsonProcessingException { + public static ObjectNode tryParseObject(String json) throws JacksonException { return MAPPER.readValue(json, ObjectNode.class); } @@ -151,7 +153,7 @@ public static ObjectNode tryParseObject(String json) throws JsonProcessingExcept public static ArrayNode parseArray(String json) { try { return tryParseArray(json); - } catch (JsonProcessingException exception) { + } catch (JacksonException exception) { throw Exceptions.handle(LOG, exception); } } @@ -161,9 +163,9 @@ public static ArrayNode parseArray(String json) { * * @param json the JSON string to parse * @return the parsed JSON string as array node - * @throws JsonProcessingException in case of a malformed JSON string + * @throws JacksonException in case of a malformed JSON string */ - public static ArrayNode tryParseArray(String json) throws JsonProcessingException { + public static ArrayNode tryParseArray(String json) throws JacksonException { return MAPPER.readValue(json, ArrayNode.class); } @@ -176,7 +178,7 @@ public static ArrayNode tryParseArray(String json) throws JsonProcessingExceptio public static String write(JsonNode objectNode) { try { return tryWrite(objectNode); - } catch (JsonProcessingException exception) { + } catch (JacksonException exception) { throw Exceptions.handle(LOG, exception); } } @@ -186,9 +188,9 @@ public static String write(JsonNode objectNode) { * * @param objectNode the node to convert * @return the JSON string representing the given node - * @throws JsonProcessingException in case the given node cannot be converted + * @throws JacksonException in case the given node cannot be converted */ - public static String tryWrite(JsonNode objectNode) throws JsonProcessingException { + public static String tryWrite(JsonNode objectNode) throws JacksonException { return MAPPER.writeValueAsString(objectNode); } @@ -201,7 +203,7 @@ public static String tryWrite(JsonNode objectNode) throws JsonProcessingExceptio public static String writePretty(JsonNode objectNode) { try { return tryWritePretty(objectNode); - } catch (JsonProcessingException exception) { + } catch (JacksonException exception) { throw Exceptions.handle(LOG, exception); } } @@ -211,9 +213,9 @@ public static String writePretty(JsonNode objectNode) { * * @param objectNode the node to convert * @return the pretty printed JSON string representing the given node - * @throws JsonProcessingException in case the given node cannot be converted + * @throws JacksonException in case the given node cannot be converted */ - public static String tryWritePretty(JsonNode objectNode) throws JsonProcessingException { + public static String tryWritePretty(JsonNode objectNode) throws JacksonException { return MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(objectNode); } @@ -532,10 +534,10 @@ public static Optional tryValueString(@Nonnull JsonNode jsonNode, String return Optional.empty(); } if (node.isNumber() || node.isBoolean()) { - return Optional.of(node.asText()); + return Optional.of(node.asString()); } - if (node.isTextual()) { - return Optional.of(node.textValue()); + if (node.isString()) { + return Optional.of(node.stringValue()); } return Optional.empty(); } @@ -557,8 +559,8 @@ public static Amount getValueAmount(@Nonnull JsonNode jsonNode, String fieldName if (node.isNumber()) { return Amount.ofRounded(node.decimalValue()); } - if (node.isTextual()) { - return Amount.ofMachineString(node.textValue()); + if (node.isString()) { + return Amount.ofMachineString(node.stringValue()); } if (node.isPojo()) { return tryGetFromPojoNode(node, Amount.class).orElse(Amount.NOTHING); @@ -580,11 +582,11 @@ public static Optional tryValueDate(@Nonnull JsonNode jsonNode, Strin if (node != null && node.isPojo()) { return tryGetFromPojoNode(node, LocalDate.class); } - if (node == null || node.isNull() || !node.isTextual()) { + if (node == null || node.isNull() || !node.isString()) { return Optional.empty(); } try { - return Optional.of(LocalDate.parse(node.textValue(), DateTimeFormatter.ISO_DATE)); + return Optional.of(LocalDate.parse(node.stringValue(), DateTimeFormatter.ISO_DATE)); } catch (DateTimeParseException exception) { return Optional.empty(); } @@ -605,11 +607,11 @@ public static Optional tryValueDateTime(@Nonnull JsonNode jsonNod if (node != null && node.isPojo()) { return tryGetFromPojoNode(node, LocalDateTime.class); } - if (node == null || node.isNull() || !node.isTextual()) { + if (node == null || node.isNull() || !node.isString()) { return Optional.empty(); } try { - return Optional.of(LocalDateTime.parse(node.textValue(), DateTimeFormatter.ISO_DATE_TIME)); + return Optional.of(LocalDateTime.parse(node.stringValue(), DateTimeFormatter.ISO_DATE_TIME)); } catch (DateTimeParseException exception) { return Optional.empty(); } diff --git a/src/test/kotlin/sirius/kernel/commons/JsonTest.kt b/src/test/kotlin/sirius/kernel/commons/JsonTest.kt index f3af34ca..3891a83d 100644 --- a/src/test/kotlin/sirius/kernel/commons/JsonTest.kt +++ b/src/test/kotlin/sirius/kernel/commons/JsonTest.kt @@ -8,13 +8,15 @@ package sirius.kernel.commons -import com.fasterxml.jackson.core.JsonPointer -import com.fasterxml.jackson.core.JsonProcessingException -import com.fasterxml.jackson.databind.JsonNode -import com.fasterxml.jackson.databind.node.ArrayNode -import com.fasterxml.jackson.databind.node.ObjectNode +import com.fasterxml.jackson.annotation.JsonInclude import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.Test +import tools.jackson.core.JacksonException +import tools.jackson.core.JsonPointer +import tools.jackson.databind.JsonNode +import tools.jackson.databind.exc.JsonNodeException +import tools.jackson.databind.node.ArrayNode +import tools.jackson.databind.node.ObjectNode import java.math.BigDecimal import java.time.LocalDate import java.time.LocalDateTime @@ -50,7 +52,7 @@ class JsonTest { val node = Json.parseObject(json) assertEquals(2, node.size()) assertEquals(123, node["foo"].asInt()) - assertEquals("baz", node["bar"].asText()) + assertEquals("baz", node["bar"].asString()) } @Test @@ -59,13 +61,13 @@ class JsonTest { val node = Json.parseObject(json) assertEquals(2, node.size()) assertEquals(123, node["foo"].asInt()) - assertEquals("baz", node["bar"].asText()) + assertEquals("baz", node["bar"].asString()) } @Test fun `exception is thrown when parsing invalid object`() { val invalidJson = "[1, 2, 3]" - assertThrows(JsonProcessingException::class.java) { + assertThrows(JacksonException::class.java) { Json.tryParseObject(invalidJson) } } @@ -83,7 +85,7 @@ class JsonTest { @Test fun `exception is thrown when parsing invalid array`() { val invalidJson = """{ "foo": 123, "bar": "baz" }""" - assertThrows(JsonProcessingException::class.java) { + assertThrows(JacksonException::class.java) { Json.tryParseArray(invalidJson) } } @@ -160,7 +162,7 @@ class JsonTest { val presentObject: Optional = Json.tryGetObjectAtIndex(node, 0) assertTrue(presentObject.isPresent) - assertEquals("Alice", presentObject.get().get("name").asText()) + assertEquals("Alice", presentObject.get().get("name").asString()) val notAnObject: Optional = Json.tryGetObjectAtIndex(node, 1) assertTrue(!notAnObject.isPresent) @@ -176,7 +178,7 @@ class JsonTest { val presentObject: Optional = Json.tryGetObject(node, "foo") assertTrue(presentObject.isPresent) - assertEquals("Alice", presentObject.get().get("name").asText()) + assertEquals("Alice", presentObject.get().get("name").asString()) val notAnObject: Optional = Json.tryGetObject(node, "bar") assertTrue(!notAnObject.isPresent) @@ -192,7 +194,7 @@ class JsonTest { val presentObject: Optional = Json.tryGetObjectAt(node, Json.createPointer("foo")) assertTrue(presentObject.isPresent) - assertEquals("Alice", presentObject.get().get("name").asText()) + assertEquals("Alice", presentObject.get().get("name").asString()) val notAnObject: Optional = Json.tryGetObjectAt(node, Json.createPointer("bar")) assertTrue(!notAnObject.isPresent) @@ -282,7 +284,7 @@ class JsonTest { @Test fun `tryGetArrayAt works with arrays as POJO Nodes`() { val node = Json.createObject() - .set("nested", Json.createObject().putPOJO("foo", listOf(1, 2, 3)).put("bar", 123)) + .set("nested", Json.createObject().putPOJO("foo", listOf(1, 2, 3)).put("bar", 123)) val presentArray: Optional = Json.tryGetArrayAt(node, Json.createPointer("nested/foo")) assertTrue(presentArray.isPresent) @@ -334,7 +336,7 @@ class JsonTest { val presentValue: Optional = Json.tryGetAt(node, JsonPointer.valueOf("/foo/0/name")) assertTrue(presentValue.isPresent) - assertEquals("Alice", presentValue.get().asText()) + assertEquals("Alice", presentValue.get().asString()) val nullValue: Optional = Json.tryGetAt(node, JsonPointer.valueOf("/bar")) assertTrue(!nullValue.isPresent) @@ -422,6 +424,122 @@ class JsonTest { assertEquals(inputAmount, parsedAmount) } + @Test + fun `Amount serialization handles NOTHING and unrounded values`() { + // pins the behavior of the explicit Amount serializer in Json.MAPPER which replaces the + // @JsonValue handling that Jackson 3 no longer applies to subclasses of Number + assertEquals("null", Json.MAPPER.writeValueAsString(Amount.NOTHING)) + assertEquals("1.23", Json.MAPPER.writeValueAsString(Amount.of(BigDecimal("1.23456789")))) + assertEquals("1.23456789", Json.MAPPER.writeValueAsString(Amount.ofRounded(BigDecimal("1.23456789")))) + } + + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private class NonEmptyAmountHolder { + @Suppress("unused") + val amount: Amount = Amount.NOTHING + } + + @Test + fun `empty Amount is suppressed by JsonInclude NON_EMPTY like in Jackson 2`() { + assertEquals("{}", Json.MAPPER.writeValueAsString(NonEmptyAmountHolder())) + } + + /** + * The following tests document how the Jackson 3 scalar accessors behave for filled, non-matching, + * null and missing properties, as this differs notably from Jackson 2 whose accessors (asText(), + * textValue(), longValue(), ...) leniently returned "", null, 0 or false for all null/missing/mismatch + * cases: + *

    + *
  • xxxValue() is strict: any missing property or type mismatch throws; only stringValue() + * treats an explicit JSON null as a legitimate value and returns null.
    • + *
  • asXxx() coerces: missing properties and JSON null leniently yield "", 0 or false, and + * convertible values (numeric strings, number-to-boolean, ...) are converted - but a failed + * coercion of an actual value throws.
    • + *
  • the default-taking variants (xxxValue(default), asXxx(default)) and the Optional variants + * (xxxValueOpt()) never throw and correspond to the lenient Jackson 2 behavior.
    • + *
+ */ + private val accessorTestNode = + Json.parseObject("""{ "string": "foo", "number": 42, "numberString": "42", "bool": true, "null": null }""") + + @Test + fun `documents string accessor behavior for filled, mismatching, null and missing properties`() { + // filled, matching: value is returned + assertEquals("foo", accessorTestNode.path("string").stringValue()) + assertEquals("foo", accessorTestNode.path("string").asString()) + + // filled, non-matching: stringValue() is strict, asString() coerces scalars but not containers + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("number").stringValue() } + assertEquals("42", accessorTestNode.path("number").asString()) + assertThrows(JsonNodeException::class.java) { accessorTestNode.asString() } + + // JSON null: a legitimate value for stringValue(), an empty string for asString() + assertEquals(null, accessorTestNode.path("null").stringValue()) + assertEquals("", accessorTestNode.path("null").asString()) + + // missing: stringValue() throws, asString() coerces leniently + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("missing").stringValue() } + assertEquals("", accessorTestNode.path("missing").asString()) + + // default-taking and Optional variants never throw + assertEquals(null, accessorTestNode.path("missing").stringValue(null)) + assertEquals("fallback", accessorTestNode.path("missing").stringValue("fallback")) + assertEquals("foo", accessorTestNode.path("string").stringValueOpt().get()) + assertTrue(accessorTestNode.path("missing").stringValueOpt().isEmpty) + } + + @Test + fun `documents numeric accessor behavior for filled, mismatching, null and missing properties`() { + // filled, matching: value is returned + assertEquals(42L, accessorTestNode.path("number").longValue()) + assertEquals(42L, accessorTestNode.path("number").asLong()) + + // filled, non-matching: longValue() is strict even for numeric strings, asLong() converts + // them but throws for non-numeric values + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("numberString").longValue() } + assertEquals(42L, accessorTestNode.path("numberString").asLong()) + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("string").asLong() } + + // JSON null and missing: longValue() cannot represent them and throws, asLong() yields 0 + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("null").longValue() } + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("missing").longValue() } + assertEquals(0L, accessorTestNode.path("null").asLong()) + assertEquals(0L, accessorTestNode.path("missing").asLong()) + + // default-taking and Optional variants never throw + assertEquals(0L, accessorTestNode.path("missing").longValue(0L)) + assertEquals(-1L, accessorTestNode.path("null").longValue(-1L)) + assertEquals(42L, accessorTestNode.path("number").longValueOpt().asLong) + assertTrue(accessorTestNode.path("null").longValueOpt().isEmpty) + assertTrue(accessorTestNode.path("missing").longValueOpt().isEmpty) + } + + @Test + fun `documents boolean accessor behavior for filled, mismatching, null and missing properties`() { + // filled, matching: value is returned + assertEquals(true, accessorTestNode.path("bool").booleanValue()) + assertEquals(true, accessorTestNode.path("bool").asBoolean()) + + // filled, non-matching: booleanValue() only accepts actual booleans; asBoolean() converts + // "true"/"false" strings and numbers (0 = false, everything else = true) but throws for + // other strings + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("number").booleanValue() } + assertEquals(true, accessorTestNode.path("number").asBoolean()) + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("string").asBoolean() } + + // JSON null and missing: booleanValue() throws, asBoolean() yields false + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("null").booleanValue() } + assertThrows(JsonNodeException::class.java) { accessorTestNode.path("missing").booleanValue() } + assertEquals(false, accessorTestNode.path("null").asBoolean()) + assertEquals(false, accessorTestNode.path("missing").asBoolean()) + + // default-taking and Optional variants never throw + assertEquals(true, accessorTestNode.path("missing").booleanValue(true)) + assertEquals(false, accessorTestNode.path("null").booleanValue(false)) + assertEquals(true, accessorTestNode.path("bool").booleanValueOpt().get()) + assertTrue(accessorTestNode.path("missing").booleanValueOpt().isEmpty) + } + @Test fun `Read Amount from POJONode`() { val inputAmount = Amount.ofRounded(BigDecimal("1.23456789"))