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 override def ignoreUnknownKeys/@ignoreUnknownKeys(b: Boolean) to allow treatment of unknown keys to be configurable #548

Merged
merged 6 commits into from
Jan 11, 2024
Merged
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
20 changes: 19 additions & 1 deletion upickle/implicits/src-2/upickle/implicits/internal/Macros.scala
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,13 @@ object Macros {
hasDefaults: Seq[Boolean],
targetType: c.Type,
varargs: Boolean) = {
val allowUnknownKeysAnnotation = targetType.typeSymbol
.annotations
.find(_.tpe == typeOf[upickle.implicits.allowUnknownKeys])
.flatMap(_.scalaArgs.headOption)
.map { case Literal(Constant(b)) => b.asInstanceOf[Boolean] }
.headOption

val defaults = deriveDefaults(companion, hasDefaults)

val localReaders = for (i <- rawArgs.indices) yield TermName("localReader" + i)
Expand Down Expand Up @@ -271,7 +278,18 @@ object Macros {
for(i <- mappedArgs.indices)
yield cq"${mappedArgs(i)} => $i"
}
case _ => -1
case _ =>
${
allowUnknownKeysAnnotation match {
case None =>
q"""
if (${c.prefix}.allowUnknownKeys) -1
else throw new _root_.upickle.core.Abort("Unknown Key: " + s.toString)
"""
case Some(false) => q"""throw new _root_.upickle.core.Abort("Unknown Key: " + s.toString)"""
case Some(true) => q"-1"
}
}
}
}

Expand Down
16 changes: 14 additions & 2 deletions upickle/implicits/src-3/upickle/implicits/Readers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ trait ReadersVersionSpecific
with Annotator
with CaseClassReadWriters:

abstract class CaseClassReadereader[T](paramCount: Int, missingKeyCount: Long) extends CaseClassReader[T] {
abstract class CaseClassReadereader[T](paramCount: Int,
missingKeyCount: Long,
allowUnknownKeys: Boolean) extends CaseClassReader[T] {
// Bincompat stub
def this(paramCount: Int, missingKeyCount: Long) = this(paramCount, missingKeyCount, true)

def visitors0: Product
lazy val visitors = visitors0
def fromProduct(p: Product): T
Expand All @@ -31,6 +36,9 @@ trait ReadersVersionSpecific
def visitKeyValue(v: Any): Unit =
val k = objectAttributeKeyReadMap(v.toString).toString
currentIndex = keyToIndex(k)
if (currentIndex == -1 && !allowUnknownKeys) {
throw new upickle.core.Abort("Unknown Key: " + k.toString)
}

def visitEnd(index: Int): T =
storeDefaults(this)
Expand All @@ -55,7 +63,11 @@ trait ReadersVersionSpecific

inline def macroR[T](using m: Mirror.Of[T]): Reader[T] = inline m match {
case m: Mirror.ProductOf[T] =>
val reader = new CaseClassReadereader[T](macros.paramsCount[T], macros.checkErrorMissingKeysCount[T]()){
val reader = new CaseClassReadereader[T](
macros.paramsCount[T],
macros.checkErrorMissingKeysCount[T](),
macros.extractIgnoreUnknownKeys[T]().headOption.getOrElse(this.allowUnknownKeys)
){
override def visitors0 = compiletime.summonAll[Tuple.Map[m.MirroredElemTypes, Reader]]
override def fromProduct(p: Product): T = m.fromProduct(p)
override def keyToIndex(x: String): Int = macros.keyToIndex[T](x)
Expand Down
12 changes: 12 additions & 0 deletions upickle/implicits/src-3/upickle/implicits/macros.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ def extractKey[A](using Quotes)(sym: quotes.reflect.Symbol): Option[String] =
.find(_.tpe =:= TypeRepr.of[upickle.implicits.key])
.map{case Apply(_, Literal(StringConstant(s)) :: Nil) => s}

inline def extractIgnoreUnknownKeys[T](): List[Boolean] = ${extractIgnoreUnknownKeysImpl[T]}
def extractIgnoreUnknownKeysImpl[T](using Quotes, Type[T]): Expr[List[Boolean]] =
import quotes.reflect._
Expr.ofList(
TypeRepr.of[T].typeSymbol
.annotations
.find(_.tpe =:= TypeRepr.of[upickle.implicits.allowUnknownKeys])
.map{case Apply(_, Literal(BooleanConstant(b)) :: Nil) => b}
.map(Expr(_))
.toList
)

inline def paramsCount[T]: Int = ${paramsCountImpl[T]}
def paramsCountImpl[T](using Quotes, Type[T]) = {
Expr(fieldLabelsImpl0[T].size)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import upickle.core.{Abort, AbortException, ArrVisitor, NoOpVisitor, ObjVisitor,
* package to form the public API1
*/
trait CaseClassReadWriters extends upickle.core.Types{

def allowUnknownKeys: Boolean = true
abstract class CaseClassReader[V] extends SimpleReader[V] {
override def expectedMsg = "expected dictionary"

Expand Down
3 changes: 2 additions & 1 deletion upickle/implicits/src/upickle/implicits/key.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ package upickle.implicits

import scala.annotation.StaticAnnotation

case class key(s: String) extends StaticAnnotation
case class key(s: String) extends StaticAnnotation
case class allowUnknownKeys(b: Boolean) extends StaticAnnotation
8 changes: 8 additions & 0 deletions upickle/src/upickle/Api.scala
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ trait Api
override def isJsonDictKey = true
def write0[R](out: Visitor[_, R], v: T): R = readwriter.write0(out, v)
}

/**
* Configure whether you want upickle to skip unknown keys during de-serialization
* of `case class`es. Can be overriden for the entire serializer via `override def`, and
* further overriden for individual `case class`es via the annotation
* `@upickle.implicits.allowUnknownKeys(b: Boolean)`
*/
override def allowUnknownKeys: Boolean = true
// End Api
}

Expand Down
54 changes: 52 additions & 2 deletions upickle/test/src/upickle/MacroTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,28 @@ object GenericIssue545{
implicit def apiResultRw[T: upickle.default.ReadWriter]: upickle.default.ReadWriter[ApiResult[T]] = upickle.default.macroRW[ApiResult[T]]
}

object UnknownKeys{
case class Default(id: Int, name: String)

implicit val defaultRw: upickle.default.ReadWriter[Default] = upickle.default.macroRW[Default]
implicit val defaultRw2: DisallowPickler.ReadWriter[Default] = DisallowPickler.macroRW[Default]

@upickle.implicits.allowUnknownKeys(false)
case class DisAllow(id: Int, name: String)

implicit val disAllowRw: upickle.default.ReadWriter[DisAllow] = upickle.default.macroRW[DisAllow]
implicit val disAllowRw2: DisallowPickler.ReadWriter[DisAllow] = DisallowPickler.macroRW[DisAllow]

@upickle.implicits.allowUnknownKeys(true)
case class Allow(id: Int, name: String)

implicit val allowRw: upickle.default.ReadWriter[Allow] = upickle.default.macroRW[Allow]
implicit val allowRw2: DisallowPickler.ReadWriter[Allow] = DisallowPickler.macroRW[Allow]

object DisallowPickler extends upickle.AttributeTagged {
override def allowUnknownKeys = false
}
}
object MacroTests extends TestSuite {

// Doesn't work :(
Expand Down Expand Up @@ -578,13 +600,41 @@ object MacroTests extends TestSuite {
test("genericIssue545"){
// Make sure case class default values are properly picked up for
// generic case classes in Scala 3
import upickle.implicits.key

upickle.default.read[GenericIssue545.Person]("{\"id\":1}") ==>
GenericIssue545.Person(1)

upickle.default.read[GenericIssue545.ApiResult[GenericIssue545.Person]]("{\"total_count\": 10}") ==>
GenericIssue545.ApiResult[GenericIssue545.Person](None, 10)
}

test("unknownKeys"){
// For upickle default, we defualt to allowing unknown keys, and explicitly annotating
// `@allowUnknownKeys(true)` does nothing, but `@allowUnknownKeys(false)` makes unknown
// keys an error (just for the annotated class)
upickle.default.read[UnknownKeys.Default]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
UnknownKeys.Default(1, "x")

upickle.default.read[UnknownKeys.Allow]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
UnknownKeys.Allow(1, "x")

intercept[upickle.core.AbortException]{
upickle.default.read[UnknownKeys.DisAllow]("""{"id":1, "name":"x", "omg": "wtf"}""")
}

// If the upickle API sets `override def allowUnknownKeys = false`, we default to treating unknown keys
// as an error, `@allowUnknownKeys(false)` does nothing, but `@allowUnknownKeys(true)` makes unknown
// keys get ignored (just for the annotated class)
intercept[upickle.core.AbortException] {
UnknownKeys.DisallowPickler.read[UnknownKeys.Default]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
UnknownKeys.Default(1, "x")
}

UnknownKeys.DisallowPickler.read[UnknownKeys.Allow]("""{"id":1, "name":"x", "omg": "wtf"}""") ==>
UnknownKeys.Allow(1, "x")

intercept[upickle.core.AbortException]{
UnknownKeys.DisallowPickler.read[UnknownKeys.DisAllow]("""{"id":1, "name":"x", "omg": "wtf"}""")
}
}
}
}