Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an allowExtraKeys setting in ProductFormats #166

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,21 @@

package spray.json

import scala.collection.mutable.HashSet

trait ProductFormatsInstances { self: ProductFormats with StandardFormats =>
def allowExtraKeys: Boolean = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder (but I'm not sure) if reversing the naming could be more clear? failOnExtraKeys or something like this?

[# // Case classes with 1 parameters

def jsonFormat1[[#P1 :JF#], T <: Product :ClassManifest](construct: ([#P1#]) => T): RootJsonFormat[T] = {
val Array([#p1#]) = extractFieldNames(classManifest[T])
jsonFormat(construct, [#p1#])
}
def jsonFormat[[#P1 :JF#], T <: Product](construct: ([#P1#]) => T, [#fieldName1: String#]): RootJsonFormat[T] = new RootJsonFormat[T]{
val knownFields = HashSet[String]()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we try to make the feature free when allowExtraKeys is false (default behavior). Maybe just make this a lazy val to make sure it is not initialized when not needed.

[# knownFields.add(fieldName1)#
]

def write(p: T) = {
val fields = new collection.mutable.ListBuffer[(String, JsValue)]
fields.sizeHint(1 * 2)
Expand All @@ -34,6 +41,14 @@ trait ProductFormatsInstances { self: ProductFormats with StandardFormats =>
def read(value: JsValue) = {
[#val p1V = fromField[P1](value, fieldName1)#
]
if (!allowExtraKeys) {
val jsObject = value.asJsObject()
val keySet = jsObject.fields.keys.toSet
val keySetDiff = keySet.diff(knownFields)
if (!keySetDiff.isEmpty) {
throw new DeserializationException(s"${keySetDiff.head} is not a known key", null, keySetDiff.toList)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we give more complete information when it fails? Missing set, given set and expected set of keys.

}
}
construct([#p1V#])
}
}#
Expand Down
13 changes: 12 additions & 1 deletion src/main/scala/spray/json/ProductFormats.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ trait ProductFormats extends ProductFormatsInstances {
}

// helpers

protected def productElement2Field[T](fieldName: String, p: Product, ix: Int, rest: List[JsField] = Nil)
(implicit writer: JsonWriter[T]): List[JsField] = {
val value = p.productElement(ix).asInstanceOf[T]
Expand Down Expand Up @@ -152,3 +152,14 @@ trait NullOptions extends ProductFormats {
(fieldName, writer.write(value)) :: rest
}
}

/**
* This trait changes the behavior for reading JSON values.
* If you mix in this trait into your custom JsonProtocol, JSON deserialization
* will throw an error if the input contains keys that are not a field of the
* target case class.
*/
trait ExtraKeysOptions extends ProductFormats {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need that trait? Seems a bit weird API to just change the flag from true to false. Maybe just document allowExtraKeys directly.

this: StandardFormats =>
override def allowExtraKeys = false
}
14 changes: 14 additions & 0 deletions src/test/scala/spray/json/ProductFormatsSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class ProductFormatsSpec extends Specification {
object TestProtocol1 extends DefaultJsonProtocol with TestProtocol
object TestProtocol2 extends DefaultJsonProtocol with TestProtocol with NullOptions

object TestProtocol3 extends DefaultJsonProtocol with TestProtocol with ExtraKeysOptions

"A JsonFormat created with `jsonFormat`, for a case class with 2 elements," should {
import TestProtocol1._
val obj = Test2(42, Some(4.2))
Expand All @@ -61,6 +63,9 @@ class ProductFormatsSpec extends Specification {
"not require the presence of optional fields for deserialization" in {
JsObject("a" -> JsNumber(42)).convertTo[Test2] mustEqual Test2(42, None)
}
"allow extra fields during deserialization" in {
JsObject("a" -> JsNumber(42), "extra_key" -> JsNumber(43)).convertTo[Test2] mustEqual Test2(42, None)
}
"not render `None` members during serialization" in {
Test2(42, None).toJson mustEqual JsObject("a" -> JsNumber(42))
}
Expand Down Expand Up @@ -92,6 +97,15 @@ class ProductFormatsSpec extends Specification {
}
}

"A JsonProtocol mixing in ExtraKeysOptions" should {
"not allow extra keys to be read" in {
import TestProtocol3._
JsObject("a" -> JsNumber(42), "extra_key" -> JsNumber(43)).convertTo[Test2] must throwA[DeserializationException].like {
case DeserializationException(_, _, fieldNames) => fieldNames mustEqual "extra_key" :: Nil
}
}
}

"A JsonFormat for a generic case class and created with `jsonFormat`" should {
import TestProtocol1._
val obj = Test3(42 :: 43 :: Nil, "x" :: "y" :: "z" :: Nil)
Expand Down