Json deserialisation or extraction bug

[Matt Searle]

Hi,

I am new to this community so please forgive me if I am posting in the wrong place. We use the lift json serialiser and have a problem when adding new fields to a class and then attempting to deserialise to the new class from json that was serialised without those new fields.

We would have liked to simply be able to use default values in the primary constructor to handle this but that doesn't work. Next we have tried explicitly adding a secondary constructors that doesn't require the new fields however the algorithm used for choosing the correct constructor gives both constructors the same score when finding the best constructor to use even though 1 of them is totally invalid.

package net.liftweb
package json

...

case class Constructor(targetType: TypeInfo, choices: List[DeclaredConstructor]) extends Mapping {
  def bestMatching(argNames: List[String]): Option[DeclaredConstructor] = {
    val names = Set(argNames: _*)
    def countOptionals(args: List[Arg]) =
      args.foldLeft(0)((n, x) => if (x.optional) n+1 else n)
    def score(args: List[Arg]) =
      args.foldLeft(0)((s, arg) => if (names.contains(arg.path)) s+1 else -100)

    if (choices.isEmpty) None
    else {
      val best = choices.tail.foldLeft((choices.head, score(choices.head.args))) { (best, c) =>
        val newScore = score(c.args)
        if (newScore == best._2) {
          if (countOptionals(c.args) < countOptionals(best._1.args))
            (c, newScore) else best
        } else if (newScore > best._2) (c, newScore) else best
      }
      Some(best._1)
    }
  }
}

In the example: 

class Foo(str: String, i: Int) {
   def this(str: String) = this(str, 1)
}

We want to be able to deserialise a Foo from:

{"str":"Bar"}

Following through with your algorithm both constructors get a score of 1 and both have the same number of optionals 0. Therefore both are equally valid and the constructor that happens to be closest to the tail of the list of constructors is chosen. I'd like to propose that you pre-filter the list removing the invalid constructors (constructors with non-optional parameters that aren't present in the Json) before choosing the best. I'd also be happy to implement this and raise a merge request if that's okay

Notice:  This email is confidential and may contain copyright material of members of the Ocado Group. Opinions and views expressed in this message may not necessarily reflect the opinions and views of the members of the Ocado Group. 

 

If you are not the intended recipient, please notify us immediately and delete all copies of this message. Please note that it is your responsibility to scan this message for viruses. 

 

Fetch and Sizzle are trading names of Speciality Stores Limited and Fabled is a trading name of Marie Claire Beauty Limited, both members of the Ocado Group.

 

References to the “Ocado Group” are to Ocado Group plc (registered in England and Wales with number 7098618) and its subsidiary undertakings (as that expression is defined in the Companies Act 2006) from time to time.  The registered office of Ocado Group plc is Buildings One & Two, Trident Place, Mosquito Way, Hatfield, Hertfordshire, AL10 9UL.

[Matt Farmer]

Hi there Matt, thanks for writing in.

The example you provided works fine for me. I can't reproduce the issue you're seeing. Here's what I did in the ammonite session where I tried it out... let me know if I misunderstood something:

@ class Foo(str: String, i: Int) {

     def this(str: String) = this(str, 1)

  }

defined class Foo

@ val obj: JObject = ("str" -> "Bar")

obj: JObject = JObject(List(JField("str", JString("Bar"))))

@ obj.extract[Foo]

res6: Foo = ammonite.$sess.cmd0$Foo@2d4df467

If you take a closer look at these lines in the scoring algorithm you called out, you can see that we do check the argument names and assign a score of -100 to any constructor that doesn't contain the correct argument names for the object we're attempting to run extraction on. Net-net, this should be working fine for you.

Let me know if I missed something that makes this reproducible!

Thanks,

Matt Farmer

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Matt Farmer]

Oh also, please make sure you’re on a recent version of Lift Json. I️ tested using 3.1.1.