Respond to review comments

sri-adarsh-kumar · sri-adarsh-kumar · commit 8d99b6dba96b · 2024-10-29T13:01:16.000+01:00
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
diff --git a/javax-money/pom.xml b/javax-money/pom.xml
@@ -14,13 +14,13 @@
     <name>Jackson datatype: javax-money</name>
     <packaging>jar</packaging>
     <version>2.19.0-SNAPSHOT</version>
-    <description>Support for datatypes of Javax Money library (https://javamoney.github.io/)
+    <description>Support for datatypes of Money API spec from JSR 354 (https://javamoney.github.io/api.html)
     </description>
     <url>https://github.com/FasterXML/jackson-datatypes-misc</url>
     <licenses>
         <license>
-            <name>The Apache Software License, Version 2.0</name>
-            <url>https://www.apache.org/licenses/LICENSE-2.0.txt</url>
+            <name>MIT License</name>
+            <url>https://opensource.org/licenses/MIT</url>
             <distribution>repo</distribution>
         </license>
     </licenses>
@@ -82,11 +82,6 @@
             <version>4.5.1</version>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>com.fasterxml.jackson.module</groupId>
-            <artifactId>jackson-module-jsonSchema</artifactId>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>com.kjetland</groupId>
             <artifactId>mbknor-jackson-jsonschema_2.12</artifactId>
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
@@ -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<CurrencyUnit> {
+public final class CurrencyUnitDeserializer extends StdScalarDeserializer<CurrencyUnit> {
+
+    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
@@ -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<CurrencyUnit> {
+public final class CurrencyUnitSerializer extends StdScalarSerializer<CurrencyUnit> {
 
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<M> 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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,3 @@`
`1`		`-TODO What goes here? (This original one or another?)`
`2`		`-`
`3`	`1`	`The MIT License (MIT)`
`4`	`2`
`5`	`3`	`Copyright (c) 2015-2016 Zalando SE`