Merge branch 'release/0.3.3'

Author: johnynek

CHANGES.md

@@ -1,5 +1,9 @@
 # chill #
 
+### 0.3.3
+* Serialize synthetic fields by default: https://github.com/twitter/chill/pull/135
+* Prefer Java to Kryo, but check both in Externalizer: https://github.com/twitter/chill/pull/138
+
 ### 0.3.2
 
 * Add a LocaleSerializer to chill-java: https://github.com/twitter/chill/pull/128

chill-java/src/main/java/com/twitter/chill/config/ConfiguredInstantiator.java

@@ -131,7 +131,8 @@ public static void setSerialized(Config conf, Class<? extends KryoInstantiator>
     KryoInstantiator refki = reflect(reflector, conf);
     String kistr = serialize(refki.newKryo(), ki);
     // Verify, that deserialization works:
-    deserialize(refki.newKryo(), kistr); // ignore the result, just see if it throws
+    KryoInstantiator deser = deserialize(refki.newKryo(), kistr); // ignore the result, just see if it throws
+    deser.newKryo(); // just see if we can still create it
     conf.set(KEY, reflector.getName() + ":" + kistr);
   }
 

chill-java/src/test/scala/com/twitter/chill/config/ConfiguredInstantiatorTest.scala

@@ -19,8 +19,9 @@ package com.twitter.chill.config
 import org.specs._
 
 import com.twitter.chill._
+import com.esotericsoftware.kryo.Kryo
 
-class TestInst extends KryoInstantiator { override def newKryo = sys.error("blow up") }
+class TestInst extends KryoInstantiator { override def newKryo = new Kryo }
 
 class ReflectingInstantiatorTest extends Specification {
   "A ConfiguredInstantiator" should {

chill-scala/src/main/scala/com/twitter/chill/Externalizer.scala

@@ -15,7 +15,15 @@ limitations under the License.
 */
 package com.twitter.chill
 
-import _root_.java.io.{ByteArrayOutputStream, Externalizable, ObjectInput, ObjectOutput}
+import _root_.java.io.{
+  ByteArrayOutputStream,
+  ByteArrayInputStream,
+  Externalizable,
+  ObjectInput,
+  ObjectOutput,
+  ObjectInputStream,
+  ObjectOutputStream
+}
 
 object Externalizer {
   def apply[T](t: T): Externalizer[T] = {
@@ -64,6 +72,28 @@ class Externalizer[T] extends Externalizable {
   // 1 here is 1 thread, since we will likely only serialize once
   private val kpool = KryoPool.withByteArrayOutputStream(1, kryo)
 
+  /** Try to round-trip and see if it works without error
+   */
+  lazy val javaWorks: Boolean = {
+    try {
+      val baos = new ByteArrayOutputStream()
+      val oos = new ObjectOutputStream(baos)
+      oos.writeObject(item)
+      val bytes = baos.toByteArray
+      val testInput = new ByteArrayInputStream(bytes)
+      val ois = new ObjectInputStream(testInput)
+      ois.readObject // this may throw
+      true
+    }
+    catch {
+      case t: Throwable =>
+        Option(System.getenv.get("CHILL_EXTERNALIZER_DEBUG"))
+          .filter(_.toBoolean)
+          .foreach { _ => t.printStackTrace }
+        false
+    }
+  }
+
   private def safeToBytes: Option[Array[Byte]] = {
     try {
       val bytes = kpool.toBytesWithClass(item)
@@ -84,24 +114,36 @@ class Externalizer[T] extends Externalizable {
 
   def readExternal(in: ObjectInput) {
     in.read match {
+      case JAVA =>
+        item = in.readObject.asInstanceOf[Option[T]]
       case KRYO =>
         val sz = in.readInt
         val buf = new Array[Byte](sz)
         in.readFully(buf)
         item = fromBytes(buf)
-      case JAVA =>
-        item = in.readObject.asInstanceOf[Option[T]]
     }
   }
+
+  protected def writeJava(out: ObjectOutput): Boolean =
+    javaWorks && {
+      out.write(JAVA)
+      out.writeObject(item)
+      true
+    }
+
+  protected def writeKryo(out: ObjectOutput): Boolean =
+    safeToBytes.map { bytes =>
+      out.write(KRYO)
+      out.writeInt(bytes.size)
+      out.write(bytes)
+      true
+    }.getOrElse(false)
+
   def writeExternal(out: ObjectOutput) {
-    safeToBytes match {
-      case Some(bytes) =>
-        out.write(KRYO)
-        out.writeInt(bytes.size)
-        out.write(bytes)
-      case None =>
-        out.write(JAVA)
-        out.writeObject(item)
+    writeJava(out) || writeKryo(out) || {
+      val inner = item.get
+      sys.error("Neither Java nor Kyro works for class: %s instance: %s\nexport CHILL_EXTERNALIZER_DEBUG=true to see both stack traces"
+        .format(inner.getClass, inner))
     }
   }
 }

chill-scala/src/main/scala/com/twitter/chill/KryoBase.scala

@@ -49,9 +49,10 @@ class KryoBase extends Kryo {
       super.newDefaultSerializer(klass) match {
         case fs: FieldSerializer[_] =>
         //Scala has a lot of synthetic fields that must be serialized:
-          if(classOf[scala.Serializable].isAssignableFrom(klass)) {
-            fs.setIgnoreSyntheticFields(false)
-          }
+        //We also enable it by default in java since not wanting these fields
+        //serialized looks like the exception rather than the rule.
+          fs.setIgnoreSyntheticFields(false)
+
           /**
            * This breaks scalding, but something like this should be used when
            * working with the repl.

chill-scala/src/test/scala/com/twitter/chill/KryoSpec.scala

@@ -36,11 +36,18 @@ case class TestValMap(map: Map[String,Double])
 case class TestValHashMap(map: HashMap[String,Double])
 case class TestVarArgs(vargs: String*)
 
+class SomeRandom(val x: Int)
+
 object WeekDay extends Enumeration {
  type WeekDay = Value
  val Mon, Tue, Wed, Thu, Fri, Sat, Sun = Value
 }
 
+trait ExampleUsingSelf { self =>
+  def count = 0
+  def addOne = new ExampleUsingSelf {override def count=self.count+1}
+}
+
 class KryoSpec extends Specification with BaseProperties {
 
   noDetailedDiffs() //Fixes issue for scala 2.9
@@ -84,6 +91,11 @@ class KryoSpec extends Specification with BaseProperties {
         serdeser must be_==(orig)
       }
     }
+    "handle trait with reference of self" in {
+      var a= new ExampleUsingSelf{}
+      var b=rt(a.addOne)
+      b.count must be_==(1)
+    }
     "handle manifests" in {
       rt(manifest[Int]) must be_==(manifest[Int])
       rt(manifest[(Int,Int)]) must be_==(manifest[(Int,Int)])
@@ -142,8 +154,15 @@ class KryoSpec extends Specification with BaseProperties {
     "work with Externalizer" in {
       val l = List(1,2,3)
       val ext = Externalizer(l)
+      ext.javaWorks must be_==(true)
       jrt(ext).get must_==(l)
     }
+    "work with Externalizer with non-java-ser" in {
+      val l = new SomeRandom(3)
+      val ext = Externalizer(l)
+      ext.javaWorks must be_==(false)
+      jrt(ext).get.x must_==(l.x)
+    }
     "handle Regex" in {
       val test = """\bhilarious""".r
       val roundtripped = rt(test)

chill-scala/src/test/scala/com/twitter/chill/config/ReflectingInstantiatorTest.scala

@@ -64,5 +64,23 @@ class ReflectingInstantiatorTest extends Specification {
       ri.equals(ri2) must be_==(true)
       ri2.equals(ri) must be_==(true)
     }
+
+  "be serialized when added onto a ConfiguredInstantiator" in {
+      val conf = new JavaMapConfig
+      ConfiguredInstantiator.setReflect(conf, classOf[ScalaKryoInstantiator])
+       val instantiator = new ConfiguredInstantiator(conf).getDelegate()
+         .withRegistrar {
+           new ReflectingDefaultRegistrar(classOf[List[_]], classOf[com.esotericsoftware.kryo.serializers.JavaSerializer])
+       }
+      try {
+        ConfiguredInstantiator.setSerialized(
+         conf,
+         classOf[ScalaKryoInstantiator],
+         instantiator
+        )
+       }catch{
+         case e => fail("Got exception serializing the instantiator\n" + e.printStackTrace)
+       }
+    }
   }
 }

project/Build.scala

@@ -12,7 +12,7 @@ object ChillBuild extends Build {
 
   val sharedSettings = Project.defaultSettings ++ mimaDefaultSettings ++ Seq(
 
-    version := "0.3.2",
+    version := "0.3.3",
     organization := "com.twitter",
     scalaVersion := "2.9.3",
     crossScalaVersions := Seq("2.9.3", "2.10.0"),
@@ -103,7 +103,7 @@ object ChillBuild extends Build {
       .filterNot(unreleasedModules.contains(_))
       .map { s =>
       val suffix = if (javaOnly.contains(s)) "" else "_2.9.3"
-      "com.twitter" % ("chill-" + s + suffix) % "0.3.1"
+      "com.twitter" % ("chill-" + s + suffix) % "0.3.2"
     }
 
   def module(name: String) = {

project/plugins.sbt

@@ -1,5 +1,3 @@
 resolvers += Resolver.url("sbt-plugin-releases", new URL("http://scalasbt.artifactoryonline.com/scalasbt/sbt-plugin-releases/"))(Resolver.ivyStylePatterns)
 
-addSbtPlugin("com.typesafe.sbt" % "sbt-pgp" % "0.7")
-
 addSbtPlugin("com.typesafe" % "sbt-mima-plugin" % "0.1.5")