From 8d99b6dba96b12024760ac181c0905d581b2c831 Mon Sep 17 00:00:00 2001 From: Sri Adarsh Kumar Date: Mon, 28 Oct 2024 13:20:20 +0100 Subject: [PATCH] Respond to review comments Changed License reference (WIP) Removed cross-module dependency to jsonSchema Testing specific Modules instead of findAndRegisterModules Assert subtypes of JSONProcessingException CurrencyUnitDeserializer extends StdScalarDeserializer CurrencyUnitSerializer extends StdScalarSerializer MonetaryAmountDeserializer throws semantic exceptions using DeserializationContext MoneyModule version uses PackageVersion --- javax-money/pom.xml | 11 ++----- .../money/CurrencyUnitDeserializer.java | 7 +++- .../money/CurrencyUnitSerializer.java | 3 +- .../money/MonetaryAmountDeserializer.java | 21 +++++++----- .../jackson/datatype/money/MoneyModule.java | 5 +-- .../src/main/resources/META-INF/LICENSE | 2 -- .../money/CurrencyUnitDeserializerTest.java | 2 +- .../CurrencyUnitSchemaSerializerTest.java | 17 ++++------ .../money/MonetaryAmountDeserializerTest.java | 5 +-- .../MonetaryAmountSchemaSerializerTest.java | 33 +++++++++---------- 10 files changed, 50 insertions(+), 56 deletions(-) diff --git a/javax-money/pom.xml b/javax-money/pom.xml index 575601d..f3dd5a8 100644 --- a/javax-money/pom.xml +++ b/javax-money/pom.xml @@ -14,13 +14,13 @@ Jackson datatype: javax-money jar 2.19.0-SNAPSHOT - Support for datatypes of Javax Money library (https://javamoney.github.io/) + Support for datatypes of Money API spec from JSR 354 (https://javamoney.github.io/api.html) https://github.com/FasterXML/jackson-datatypes-misc - The Apache Software License, Version 2.0 - https://www.apache.org/licenses/LICENSE-2.0.txt + MIT License + https://opensource.org/licenses/MIT repo @@ -82,11 +82,6 @@ 4.5.1 test - - com.fasterxml.jackson.module - jackson-module-jsonSchema - test - com.kjetland mbknor-jackson-jsonschema_2.12 diff --git a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializer.java b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializer.java index 7ed5c34..917beca 100644 --- a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializer.java +++ b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializer.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer; import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; import org.apiguardian.api.API; @@ -13,7 +14,11 @@ import static org.apiguardian.api.API.Status.MAINTAINED; @API(status = MAINTAINED) -public final class CurrencyUnitDeserializer extends JsonDeserializer { +public final class CurrencyUnitDeserializer extends StdScalarDeserializer { + + public CurrencyUnitDeserializer() { + super(CurrencyUnit.class); + } @Override public Object deserializeWithType(final JsonParser parser, final DeserializationContext context, diff --git a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSerializer.java b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSerializer.java index fb511c0..745dbc0 100644 --- a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSerializer.java +++ b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSerializer.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.jsonFormatVisitors.JsonFormatVisitorWrapper; +import com.fasterxml.jackson.databind.ser.std.StdScalarSerializer; import com.fasterxml.jackson.databind.ser.std.StdSerializer; import org.apiguardian.api.API; @@ -14,7 +15,7 @@ import static org.apiguardian.api.API.Status.MAINTAINED; @API(status = MAINTAINED) -public final class CurrencyUnitSerializer extends StdSerializer { +public final class CurrencyUnitSerializer extends StdScalarSerializer { CurrencyUnitSerializer() { super(CurrencyUnit.class); diff --git a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializer.java b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializer.java index e053b50..d4a869e 100644 --- a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializer.java +++ b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializer.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; @@ -14,6 +15,7 @@ import java.io.IOException; import java.math.BigDecimal; import java.util.Arrays; +import java.util.Objects; import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES; import static java.lang.String.format; @@ -61,17 +63,18 @@ public M deserialize(final JsonParser parser, final DeserializationContext conte } } - checkPresent(parser, amount, names.getAmount()); - checkPresent(parser, currency, names.getCurrency()); + String missingName; - return factory.create(amount, currency); - } - - private void checkPresent(final JsonParser parser, @Nullable final Object value, final String name) - throws JsonParseException { - if (value == null) { - throw new JsonParseException(parser, format("Missing property: '%s'", name)); + if (Objects.isNull(currency)) { + missingName = names.getCurrency(); + } else if (Objects.isNull(amount)) { + missingName = names.getAmount(); + } else { + return factory.create(amount, currency); } + + return context.reportPropertyInputMismatch(MonetaryAmount.class, missingName, format("Missing property: '%s'", missingName)); + } } diff --git a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MoneyModule.java b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MoneyModule.java index f350ded..75157f6 100644 --- a/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MoneyModule.java +++ b/javax-money/src/main/java/com/fasterxml/jackson/datatype/money/MoneyModule.java @@ -1,7 +1,6 @@ package com.fasterxml.jackson.datatype.money; import com.fasterxml.jackson.core.Version; -import com.fasterxml.jackson.core.util.VersionUtil; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.module.SimpleDeserializers; import com.fasterxml.jackson.databind.module.SimpleSerializers; @@ -59,10 +58,8 @@ public String getModuleName() { } @Override - @SuppressWarnings("deprecation") public Version version() { - final ClassLoader loader = MoneyModule.class.getClassLoader(); - return VersionUtil.mavenVersionFor(loader, "com.fasterxml.jackson.datatype.money", "jackson-datatype-money"); + return PackageVersion.VERSION; } @Override diff --git a/javax-money/src/main/resources/META-INF/LICENSE b/javax-money/src/main/resources/META-INF/LICENSE index 003d9c7..6448684 100644 --- a/javax-money/src/main/resources/META-INF/LICENSE +++ b/javax-money/src/main/resources/META-INF/LICENSE @@ -1,5 +1,3 @@ -TODO What goes here? (This original one or another?) - The MIT License (MIT) Copyright (c) 2015-2016 Zalando SE diff --git a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializerTest.java b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializerTest.java index 670848c..3e9e339 100644 --- a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializerTest.java +++ b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitDeserializerTest.java @@ -14,7 +14,7 @@ public final class CurrencyUnitDeserializerTest { - private final ObjectMapper unit = new ObjectMapper().findAndRegisterModules(); + private final ObjectMapper unit = new ObjectMapper().registerModule(new MoneyModule()); @Test public void shouldDeserialize() throws IOException { diff --git a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSchemaSerializerTest.java b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSchemaSerializerTest.java index ceaa6f6..54e3352 100644 --- a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSchemaSerializerTest.java +++ b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/CurrencyUnitSchemaSerializerTest.java @@ -1,25 +1,22 @@ package com.fasterxml.jackson.datatype.money; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.module.jsonSchema.JsonSchema; -import com.fasterxml.jackson.module.jsonSchema.JsonSchemaGenerator; +import com.kjetland.jackson.jsonSchema.JsonSchemaGenerator; import org.junit.Test; import javax.money.CurrencyUnit; - import static org.assertj.core.api.Assertions.assertThat; public final class CurrencyUnitSchemaSerializerTest { - private final ObjectMapper unit = new ObjectMapper().findAndRegisterModules(); + private final ObjectMapper unit = new ObjectMapper().registerModule(new MoneyModule()); @Test - public void shouldSerializeJsonSchema() throws Exception { + public void shouldSerializeJsonSchema() { JsonSchemaGenerator generator = new JsonSchemaGenerator(unit); - JsonSchema jsonSchema = generator.generateSchema(CurrencyUnit.class); - final String actual = unit.writeValueAsString(jsonSchema); - final String expected = "{\"type\":\"string\"}"; - - assertThat(actual).isEqualTo(expected); + JsonNode schemaNode = generator.generateJsonSchema(CurrencyUnit.class); + assertThat(schemaNode.get("type")).isNotNull(); + assertThat(schemaNode.get("type").asText()).isEqualTo("string"); } } diff --git a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializerTest.java b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializerTest.java index 55551b1..3e8e4f6 100644 --- a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializerTest.java +++ b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountDeserializerTest.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; import junitparams.JUnitParamsRunner; @@ -209,8 +210,8 @@ public void shouldFailToDeserializeWithoutCurrency(final Class type, final Co final String content = "{\"amount\":29.95}"; - final JsonProcessingException exception = assertThrows( - JsonProcessingException.class, () -> unit.readValue(content, type)); + final MismatchedInputException exception = assertThrows( + MismatchedInputException.class, () -> unit.readValue(content, type)); assertThat(exception.getMessage()).contains("Missing property: 'currency'"); } diff --git a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountSchemaSerializerTest.java b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountSchemaSerializerTest.java index c827bd6..db7a965 100644 --- a/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountSchemaSerializerTest.java +++ b/javax-money/src/test/java/com/fasterxml/jackson/datatype/money/MonetaryAmountSchemaSerializerTest.java @@ -3,8 +3,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.module.jsonSchema.JsonSchema; -import com.fasterxml.jackson.module.jsonSchema.JsonSchemaGenerator; +import com.kjetland.jackson.jsonSchema.JsonSchemaGenerator; import org.junit.Test; import javax.money.MonetaryAmount; @@ -17,12 +16,12 @@ public final class MonetaryAmountSchemaSerializerTest { public void shouldSerializeJsonSchema() throws Exception { final ObjectMapper unit = unit(module()); final JsonSchemaGenerator generator = new JsonSchemaGenerator(unit); - final JsonSchema jsonSchema = generator.generateSchema(MonetaryAmount.class); + final JsonNode jsonSchema = generator.generateJsonSchema(MonetaryAmount.class); final String actual = unit.writeValueAsString(jsonSchema); - final String expected = "{\"type\":\"object\",\"id\":\"urn:jsonschema:javax:money:MonetaryAmount\",\"properties\":" + - "{\"amount\":{\"type\":\"number\",\"required\":true}," + - "\"currency\":{\"type\":\"string\",\"required\":true}," + - "\"formatted\":{\"type\":\"string\"}}}"; + final String expected = "{\"$schema\":\"http://json-schema.org/draft-04/schema#\",\"title\":\"Monetary Amount\"" + + ",\"type\":\"object\",\"additionalProperties\":false,\"properties\":{\"amount\":{\"type\":\"number\"}" + + ",\"currency\":{\"type\":\"string\"},\"formatted\":{\"type\":\"string\"}}" + + ",\"required\":[\"amount\",\"currency\"]}"; assertThat(actual).isEqualTo(expected); } @@ -33,12 +32,11 @@ public void shouldSerializeJsonSchemaWithCustomFieldNames() throws Exception { .withCurrencyFieldName("unit") .withFormattedFieldName("pretty")); final JsonSchemaGenerator generator = new JsonSchemaGenerator(unit); - final JsonSchema jsonSchema = generator.generateSchema(MonetaryAmount.class); + final JsonNode jsonSchema = generator.generateJsonSchema(MonetaryAmount.class); final String actual = unit.writeValueAsString(jsonSchema); - final String expected = "{\"type\":\"object\",\"id\":\"urn:jsonschema:javax:money:MonetaryAmount\",\"properties\":" + - "{\"value\":{\"type\":\"number\",\"required\":true}," + - "\"unit\":{\"type\":\"string\",\"required\":true}," + - "\"pretty\":{\"type\":\"string\"}}}"; + final String expected = "{\"$schema\":\"http://json-schema.org/draft-04/schema#\",\"title\":\"Monetary Amount\"" + + ",\"type\":\"object\",\"additionalProperties\":false,\"properties\":{\"value\":{\"type\":\"number\"}" + + ",\"unit\":{\"type\":\"string\"},\"pretty\":{\"type\":\"string\"}},\"required\":[\"value\",\"unit\"]}"; assertThat(actual).isEqualTo(expected); } @@ -47,18 +45,17 @@ public void shouldSerializeJsonSchemaWithCustomFieldNames() throws Exception { public void shouldSerializeJsonSchemaWithQuotedDecimalNumbers() throws Exception { final ObjectMapper unit = unit(module().withQuotedDecimalNumbers()); final JsonSchemaGenerator generator = new JsonSchemaGenerator(unit); - final JsonSchema jsonSchema = generator.generateSchema(MonetaryAmount.class); + final JsonNode jsonSchema = generator.generateJsonSchema(MonetaryAmount.class); final String actual = unit.writeValueAsString(jsonSchema); - final String expected = "{\"type\":\"object\",\"id\":\"urn:jsonschema:javax:money:MonetaryAmount\",\"properties\":" + - "{\"amount\":{\"type\":\"string\",\"required\":true}," + - "\"currency\":{\"type\":\"string\",\"required\":true}," + - "\"formatted\":{\"type\":\"string\"}}}"; + final String expected = "{\"$schema\":\"http://json-schema.org/draft-04/schema#\",\"title\":\"Monetary Amount\"" + + ",\"type\":\"object\",\"additionalProperties\":false,\"properties\":{\"amount\":{\"type\":\"string\"}" + + ",\"currency\":{\"type\":\"string\"},\"formatted\":{\"type\":\"string\"}},\"required\":[\"amount\",\"currency\"]}"; assertThat(actual).isEqualTo(expected); } @Test - public void shouldSerializeJsonSchemaWithMultipleMonetayAmountsAndAlternativeGenerator() throws Exception { + public void shouldSerializeJsonSchemaWithMultipleMonetayAmounts() throws Exception { final ObjectMapper unit = unit(module()); final com.kjetland.jackson.jsonSchema.JsonSchemaGenerator generator = new com.kjetland.jackson.jsonSchema.JsonSchemaGenerator(unit);