search

scala / pickling

Fast, customizable, boilerplate-free pickling support for Scala
lampwww.epfl.ch/~hmiller/pickling
BSD 3-Clause "New" or "Revised" License
831 stars 79 forks source link

Constant val is omitted from pickle causing NPE on unpickle #180

Open dallmeier opened 10 years ago

dallmeier commented 10 years ago

I have an abstract class that extends a trait with an abstract val, and a case class that extends the abstract class:

package pickling.run

import org.scalatest.FunSuite
import scala.pickling._
import json._

trait TheTrait {
  val name : String
}

sealed abstract class BaseClass extends TheTrait {
  val name = "TestName"
}

case class TheClass(value : Int) extends BaseClass

/**
 */
class TraitWithAbstractValueTest extends FunSuite {

  test("main") {
    val instance = TheClass(1)
    val pickle = instance.pickle
    println(pickle.value)
  }

}

I would expect the generated pickle to contain val name. However, the output I get is:

{
  "tpe": "pickling.run.TheClass",
  "value": 1
}
dallmeier commented 10 years ago

PS: Tested against cde968a6c1c1b020929d34e5449cf28263372a67 on branch 0.9.x.

heathermiller commented 10 years ago

Hi there, thanks for the report!

This is actually intended behavior – though I agree that it can be confusing, and that maybe we can/should do something about it.

First, background: this happens due to Scala/pickling's role as a serialization framework first – which means that its first goal is to encode (in JSON/binary/etc) the minimum required to serialize/deserialize instances of Scala classes.

In your example, name is effectively static. That is, for every new instance of TheClass, name will always be "TestName" – we know this at compile-time, so it's unnecessary (from the point of view of efficient serialization/deserialization) to include that field in the pickle since we can just call TheClass's constructor to fill name with the value "TestName".

However, what you point out is a very interesting problem that arises from the fact that Scala/pickling is more than a simple serialization framework – aside from basic serialization, people often would also like to use it to publish information for running services via formats like JSON, and in these cases, fields of Scala objects might conceptually be mapped directly to JSON fields (and, like in your case, could be elided!). In such a situation, unpickling is likely unnecessary.

So, the question is – knowing this, as a user, what would you prefer? We could add an annotation like @pickleMe (name TBD) to ensure that a field gets stored in a pickle.

dallmeier commented 10 years ago

Thanks for getting back so quickly. While reading your answer I realized that my bug report was incomplete. It's actually the unpickling that fails. I have minimized the test case too much, and so far I have failed to reproduce the unpickling error that I see in production. I'll continue working on that and let you know as soon as I have a good test case that describes my problem.

Regarding your question I agree that when using json serialized objects outside scala, being able to annotate fields would definetly be the way to go.

heathermiller commented 10 years ago

Ah, gotcha.

I tried your above example, and unpickling works fine for me on 0.9.x: 

scala> trait TheTrait {
     |   val name : String
     | }
defined trait TheTrait

scala> 

scala> sealed abstract class BaseClass extends TheTrait {
     |   val name = "TestName"
     | }
defined class BaseClass

scala> 

scala> case class TheClass(value : Int) extends BaseClass
defined class TheClass

scala> import scala.pickling._
import scala.pickling._

scala> import json._
import json._

scala>     val instance = TheClass(1)
instance: TheClass = TheClass(1)

scala>     val pickle = instance.pickle
pickle: scala.pickling.json.JSONPickle =
JSONPickle({
  "tpe": "TheClass",
  "value": 1
})

scala> pickle.unpickle[TheClass]
res0: TheClass = TheClass(1)

Added a test case for your example here: https://github.com/scala/pickling/pull/186

dallmeier commented 10 years ago

You're right. Took me a while to reproduce my actual problem. Here is my test case that fails on HEAD in develop:

package pickling.run

import org.scalatest.FunSuite
import scala.pickling._
import json._

trait Message {

  val id: String

  val recipient: String

}

case class EventMessage(id: String, ev: Event) extends Message {
  override val recipient = ev.eventType.toString
}

trait Event {
  val name : String
  val eventType: EventType
}

class EventType(private val path: String)

sealed abstract class StartTaskEvent extends Event {
  val name = "starttask"
  override val eventType = new EventType("StartTask")
}

case class StartSpecializedTaskEvent(value : Int) extends StartTaskEvent {
  override val eventType = new EventType("StartTask.Specialized")
}

/**
 */
class TraitWithAbstractValueTest extends FunSuite {

  def toPickle(message: Message) : String = {
    message.pickle.value
  }

  def fromPickle(pickleString: String) : Message = {
    val pickle = JSONPickle(pickleString)
    pickle.unpickle[Message]
  }

  test("main") {
    val event = EventMessage("theid", new StartSpecializedTaskEvent(1))
    val pickledEvent = fromPickle(toPickle(event))
    println(pickledEvent)
  }

}

Running this yields a NullPointerException:

java.lang.NullPointerException was thrown.
java.lang.NullPointerException
    at pickling.run.EventMessage.<init>(TraitWithAbstractValueTest.scala:16)
    at pickling.run.TraitWithAbstractValueTest$PicklingRunEventMessageUnpickler$macro$3$2$.unpickle(TraitWithAbstractValueTest.scala:45)
    at pickling.run.TraitWithAbstractValueTest.fromPickle(TraitWithAbstractValueTest.scala:45)
    at pickling.run.TraitWithAbstractValueTest$$anonfun$1.apply$mcV$sp(TraitWithAbstractValueTest.scala:50)
    at pickling.run.TraitWithAbstractValueTest$$anonfun$1.apply(TraitWithAbstractValueTest.scala:48)
    at pickling.run.TraitWithAbstractValueTest$$anonfun$1.apply(TraitWithAbstractValueTest.scala:48)
    at org.scalatest.Transformer$$anonfun$apply$1.apply(Transformer.scala:22)
    at org.scalatest.Transformer$$anonfun$apply$1.apply(Transformer.scala:22)
    at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85)
    at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
    at org.scalatest.Transformer.apply(Transformer.scala:22)
    at org.scalatest.Transformer.apply(Transformer.scala:20)
    at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:158)
    at org.scalatest.Suite$class.withFixture(Suite.scala:1121)
    at org.scalatest.FunSuite.withFixture(FunSuite.scala:1559)
    at org.scalatest.FunSuiteLike$class.invokeWithFixture$1(FunSuiteLike.scala:155)
    at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:167)
    at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:167)
    at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
    at org.scalatest.FunSuiteLike$class.runTest(FunSuiteLike.scala:167)
    at org.scalatest.FunSuite.runTest(FunSuite.scala:1559)
    at org.scalatest.FunSuiteLike$$anonfun$runTests$1.apply(FunSuiteLike.scala:200)
    at org.scalatest.FunSuiteLike$$anonfun$runTests$1.apply(FunSuiteLike.scala:200)
    at org.scalatest.SuperEngine$$anonfun$traverseSubNodes$1$1.apply(Engine.scala:413)
    at org.scalatest.SuperEngine$$anonfun$traverseSubNodes$1$1.apply(Engine.scala:401)
    at scala.collection.immutable.List.foreach(List.scala:381)
    at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
    at org.scalatest.SuperEngine.org$scalatest$SuperEngine$$runTestsInBranch(Engine.scala:396)
    at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:483)
    at org.scalatest.FunSuiteLike$class.runTests(FunSuiteLike.scala:200)
    at org.scalatest.FunSuite.runTests(FunSuite.scala:1559)
    at org.scalatest.Suite$class.run(Suite.scala:1423)
    at org.scalatest.FunSuite.org$scalatest$FunSuiteLike$$super$run(FunSuite.scala:1559)
    at org.scalatest.FunSuiteLike$$anonfun$run$1.apply(FunSuiteLike.scala:204)
    at org.scalatest.FunSuiteLike$$anonfun$run$1.apply(FunSuiteLike.scala:204)
    at org.scalatest.SuperEngine.runImpl(Engine.scala:545)
    at org.scalatest.FunSuiteLike$class.run(FunSuiteLike.scala:204)
    at org.scalatest.FunSuite.run(FunSuite.scala:1559)
    at org.scalatest.tools.SuiteRunner.run(SuiteRunner.scala:55)
    at org.scalatest.tools.Runner$$anonfun$doRunRunRunDaDoRunRun$3.apply(Runner.scala:2563)
    at org.scalatest.tools.Runner$$anonfun$doRunRunRunDaDoRunRun$3.apply(Runner.scala:2557)
    at scala.collection.immutable.List.foreach(List.scala:381)
    at org.scalatest.tools.Runner$.doRunRunRunDaDoRunRun(Runner.scala:2557)
    at org.scalatest.tools.Runner$$anonfun$runOptionallyWithPassFailReporter$2.apply(Runner.scala:1044)
    at org.scalatest.tools.Runner$$anonfun$runOptionallyWithPassFailReporter$2.apply(Runner.scala:1043)
    at org.scalatest.tools.Runner$.withClassLoaderAndDispatchReporter(Runner.scala:2722)
    at org.scalatest.tools.Runner$.runOptionallyWithPassFailReporter(Runner.scala:1043)
    at org.scalatest.tools.Runner$.run(Runner.scala:883)
    at org.scalatest.tools.Runner.run(Runner.scala)
    at org.jetbrains.plugins.scala.testingSupport.scalaTest.ScalaTestRunner.runScalaTest2(ScalaTestRunner.java:141)
    at org.jetbrains.plugins.scala.testingSupport.scalaTest.ScalaTestRunner.main(ScalaTestRunner.java:32)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:134)

I'm not sure of the original title of the bug report still applies. Could you run the test and let me know if I'm trying something stupid?

dallmeier commented 10 years ago

I accidentally closed the bug report. Sorry.

dallmeier commented 10 years ago

Any updates or hints on this? I reran the above test against HEAD of 0.9.x.

eed3si9n commented 9 years ago

I am still seeing NPE for https://github.com/scala/pickling/issues/180#issuecomment-54806095 with Pickling 0.10.0.

{
  "$type": "pickling.run.EventMessage",
  "id": "theid",
  "ev": {
    "$type": "pickling.run.StartSpecializedTaskEvent",
    "value": 1
  }
}

[info] TraitWithAbstractValueTest:
[info] - main *** FAILED ***
[info]   java.lang.NullPointerException:
[info]   at pickling.run.EventMessage.<init>(test.scala:16)
[info]   at pickling.run.TraitWithAbstractValueTest$PicklingRunMessageUnpickler$macro$6$2$PicklingRunEventMessageUnpickler$macro$7$2$.unpickle(test.scala:45)
[info]   at pickling.run.TraitWithAbstractValueTest$PicklingRunMessageUnpickler$macro$6$2$.unpickle(test.scala:45)
[info]   at scala.pickling.Unpickler$class.unpickleEntry(Pickler.scala:79)
[info]   at pickling.run.TraitWithAbstractValueTest$PicklingRunMessageUnpickler$macro$6$2$.unpickleEntry(test.scala:45)
[info]   at scala.pickling.functions$.unpickle(functions.scala:11)
[info]   at scala.pickling.UnpickleOps.unpickle(Ops.scala:23)
[info]   at pickling.run.TraitWithAbstractValueTest.fromPickle(test.scala:45)
[info]   at pickling.run.TraitWithAbstractValueTest$$anonfun$1.apply$mcV$sp(test.scala:52)
[info]   at pickling.run.TraitWithAbstractValueTest$$anonfun$1.apply(test.scala:48)
[info]   ...