From 6739ce4f2072b80052193d6133d18922f643758b Mon Sep 17 00:00:00 2001 From: Aleksi Huotala <7612995+alehuo@users.noreply.github.com> Date: Tue, 25 Apr 2023 12:52:53 +0300 Subject: [PATCH] Add omitNullValues -parameter to context; With omitNullValues you can omit key-value pairs of the JSON input that have a value of 'null'. This fixes a bug, where Scala traits could not be deserialized properly, if some of the case classes of the trait used an Option[T] -type and the input included key-value pairs, that had the value of 'null'. --- README.md | 30 ++++++++++++ .../oph/scalaschema/ExtractionContext.scala | 3 +- .../SchemaValidatingExtractor.scala | 17 +++++-- .../scalaschema/OmitNullFromInputTest.scala | 47 +++++++++++++++++++ .../oph/scalaschema/SerializationTest.scala | 11 ++++- .../scala/fi/oph/scalaschema/TestData.scala | 1 + 6 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 src/test/scala/fi/oph/scalaschema/OmitNullFromInputTest.scala diff --git a/README.md b/README.md index eb752d3..a5c19ff 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,36 @@ case class ValidationTestClass(name: String, stuff: List[Int]) The `ExtractionContext` object created in the example above is used by the `scala-schema` extraction mechanism to cache some information to make subsequent extractions faster. Hence it makes sense to store this object in a variable. +#### Treat null values as missing + +**In scala-schema versions prior to `2.33.0`, when trying to extract a Scala schema, which included the type `Option[T]` with JSON data that included the type `null`, the schema fails to narrow down.** + +**Starting from `scala-schema` version `2.33.0`, you can omit null values from the input JSON by setting `omitNullValues = true` in the `SerializationContext`:** + +```scala +trait SomeTrait { +} + +case class SomeTraitBranch1(first: String, second: Option[String] = None, third: Option[String] = None) extends SomeTrait { +} + +case class SomeTraitBranch2(second: String, third: Option[String] = None) extends SomeTrait { +} + +case class SomeTraitBranch3(third: String) extends SomeTrait { +} + + +object SerializationExample extends App { + implicit val context = SerializationContext(SchemaFactory.default, omitNullValues = true) + val branch2 = SchemaValidatingExtractor.extract[SomeTrait]("""{"first": null, "second": "Example second", "third": null}""") + /* + Output type is SomeTraitBranch2. This threw a ValidationError in scala-schema versions prior to 2.33.0 + */ +} +``` + + More examples in this [test](https://github.com/Opetushallitus/scala-schema/blob/scala-2.12/src/test/scala/fi/oph/scalaschema/ValidationAndExtractionTest.scala) ### Serialization diff --git a/src/main/scala/fi/oph/scalaschema/ExtractionContext.scala b/src/main/scala/fi/oph/scalaschema/ExtractionContext.scala index 49af193..f688815 100644 --- a/src/main/scala/fi/oph/scalaschema/ExtractionContext.scala +++ b/src/main/scala/fi/oph/scalaschema/ExtractionContext.scala @@ -17,7 +17,8 @@ case class ExtractionContext(schemaFactory: SchemaFactory, allowEmptyStrings: Boolean = true, criteriaCache: collection.mutable.Map[String, DiscriminatorCriterion] = collection.mutable.Map.empty, ignoreNonValidatingListItems: Boolean = false, - stripClassReferences: Boolean = true) { + stripClassReferences: Boolean = true, + omitNullFromInput: Boolean = false) { def hasSerializerFor(schema: SchemaWithClassName) = customSerializerFor(schema).isDefined def customSerializerFor(schema: SchemaWithClassName) = customDeserializers.find(_.isApplicable(schema)) def ifValidating(errors: => List[ValidationError]) = if (validate) { errors } else { Nil } diff --git a/src/main/scala/fi/oph/scalaschema/SchemaValidatingExtractor.scala b/src/main/scala/fi/oph/scalaschema/SchemaValidatingExtractor.scala index 9e7e097..57045f2 100644 --- a/src/main/scala/fi/oph/scalaschema/SchemaValidatingExtractor.scala +++ b/src/main/scala/fi/oph/scalaschema/SchemaValidatingExtractor.scala @@ -10,11 +10,15 @@ object SchemaValidatingExtractor { def extract[T](json: JValue)(implicit context: ExtractionContext, tag: ru.TypeTag[T]): Either[List[ValidationError], T] = { val rootSchema = context.schemaFactory.createSchema[T] - if(context.stripClassReferences) { - extract(removeJsonField(json, "$class"), rootSchema, Nil)(context, rootSchema).right.map(_.asInstanceOf[T]) - } else { - extract(json, rootSchema, Nil)(context, rootSchema).right.map(_.asInstanceOf[T]) + val dta = { + (context.stripClassReferences, context.omitNullFromInput) match { + case (true, true) => removeJsonField(removeNullFields(json) , "$class") + case (true, false) => removeJsonField(json , "$class") + case (false, true) => removeNullFields(json) + case _ => json + } } + extract(dta, rootSchema, Nil)(context, rootSchema).right.map(_.asInstanceOf[T]) } def extract[T](json: String)(implicit context: ExtractionContext, tag: ru.TypeTag[T]): Either[List[ValidationError], T] = { @@ -26,6 +30,11 @@ object SchemaValidatingExtractor { case _ => false } + def removeNullFields(json: JValue) = json removeField { + case (_, JNull) => true + case _ => false + } + def extract(json: JValue, klass: Class[_])(implicit context: ExtractionContext): Either[List[ValidationError], AnyRef] = { val rootSchema = context.schemaFactory.createSchema(klass.getName) if (context.stripClassReferences) { diff --git a/src/test/scala/fi/oph/scalaschema/OmitNullFromInputTest.scala b/src/test/scala/fi/oph/scalaschema/OmitNullFromInputTest.scala new file mode 100644 index 0000000..72e37ff --- /dev/null +++ b/src/test/scala/fi/oph/scalaschema/OmitNullFromInputTest.scala @@ -0,0 +1,47 @@ +package fi.oph.scalaschema + +import org.scalatest.freespec.AnyFreeSpec +import org.scalatest.matchers.should.Matchers + +trait SomeTrait { +} + +case class SomeTraitBranch1(first: String, second: Option[String] = None, third: Option[String] = None) extends SomeTrait { +} + +case class SomeTraitBranch2(second: String, third: Option[String] = None) extends SomeTrait { +} + +case class SomeTraitBranch3(third: String) extends SomeTrait { +} + + + + +class OmitNullFromInputTest extends AnyFreeSpec with Matchers { + "OmitNullFromInput" - { + "When JSON contains a null value" - { + implicit val context = ExtractionContext(SchemaFactory.default, omitNullFromInput = true) + "Should extract case class SomeTraitBranch1 correctly" in { + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": "Example first"}""") should equal(Right(SomeTraitBranch1("Example first"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": "Example first", "second": null}""") should equal(Right(SomeTraitBranch1("Example first"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": "Example first", "second": null, "third": null}""") should equal(Right(SomeTraitBranch1("Example first"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": "Example first", "second": "Example second", "third": "Example third"}""") should equal(Right(SomeTraitBranch1("Example first",Some("Example second"),Some("Example third")))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": "Example first", "third": null}""") should equal(Right(SomeTraitBranch1("Example first"))) + } + "Should extract case class SomeTraitBranch2 correctly" in { + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": null, "second": "Example second", "third": null}""") should equal(Right(SomeTraitBranch2("Example second"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"second": "Example second", "third": null}""") should equal(Right(SomeTraitBranch2("Example second"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": null, "second": "Example second"}""") should equal(Right(SomeTraitBranch2("Example second"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"second": "Example second"}""") should equal(Right(SomeTraitBranch2("Example second"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"second": "Example second", "third": "Example third"}""") should equal(Right(SomeTraitBranch2("Example second", Some("Example third")))) + } + "Should extract case class SomeTraitBranch3 correctly" in { + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": null, "second": null, "third": "Example third"}""") should equal(Right(SomeTraitBranch3("Example third"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"first": null, "third": "Example third"}""") should equal(Right(SomeTraitBranch3("Example third"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"second": null, "third": "Example third"}""") should equal(Right(SomeTraitBranch3("Example third"))) + SchemaValidatingExtractor.extract[SomeTrait]("""{"third": "Example third"}""") should equal(Right(SomeTraitBranch3("Example third"))) + } + } + } +} \ No newline at end of file diff --git a/src/test/scala/fi/oph/scalaschema/SerializationTest.scala b/src/test/scala/fi/oph/scalaschema/SerializationTest.scala index 2ec5f1b..535b30a 100644 --- a/src/test/scala/fi/oph/scalaschema/SerializationTest.scala +++ b/src/test/scala/fi/oph/scalaschema/SerializationTest.scala @@ -5,7 +5,7 @@ import java.time.{LocalDate, LocalDateTime, ZonedDateTime} import java.util.Date import org.joda.time.format.ISODateTimeFormat import org.json4s.JValue -import org.json4s.JsonAST.{JArray, JObject, JString} +import org.json4s.JsonAST.{JArray, JNull, JObject, JString} import org.scalatest.freespec.AnyFreeSpec import org.scalatest.matchers.should.Matchers @@ -19,6 +19,11 @@ class SerializationTest extends AnyFreeSpec with Matchers { testSerialization(Numbers(1, 1l, 0.4f, 1.1), """{"a":1,"b":1,"c":0.4000000059604645,"d":1.1}""") } + "StringOptions" in { + testSerialization(StringOptions(None), """{}""") + testSerialization(StringOptions(Some("Hello")), """{"value":"Hello"}""") + } + "traits" in { testSerialization(ThingContainingTrait(Impl1("hello")), """{"x":{"x":"hello"}}""") } @@ -85,6 +90,10 @@ class SerializationTest extends AnyFreeSpec with Matchers { testSerialization(WithJValue(JString("hello")), """{"x":"hello"}""") } + "JValue null field" in { + testSerialization(WithJValue(JNull), """{"x":null}""") + } + "JValue" in { testSerialization(JString("hello").asInstanceOf[JValue], """"hello"""") } diff --git a/src/test/scala/fi/oph/scalaschema/TestData.scala b/src/test/scala/fi/oph/scalaschema/TestData.scala index 95439ee..22ab8cf 100644 --- a/src/test/scala/fi/oph/scalaschema/TestData.scala +++ b/src/test/scala/fi/oph/scalaschema/TestData.scala @@ -17,6 +17,7 @@ case class BooleansWithDefault(@DefaultValue(true) field: Boolean) case class StringsWithDefault(@DefaultValue("hello") field: String) case class NumbersWithDefault(@DefaultValue(1) field: Int) case class Numbers(a: Int, b: Long, c: Float, d: Double) +case class StringOptions(value: Option[String]) case class Strings(s: String) case class Dates(a: LocalDate, b: ZonedDateTime, c: Date, d: Timestamp, e: DateTime, f: LocalDateTime) case class Lists(things: List[Int])