Merge pull request #52 from AVSystem/generalizations

Relaxed field order preservation requirement for ObjectInput

Author: ghik

commons-core/src/main/scala/com/avsystem/commons/serialization/GenCodec.scala

@@ -142,6 +142,12 @@ object GenCodec extends RecursiveAutoCodecs with TupleGenCodecs {
   def createNonNullList[T](readFun: ListInput => T, writeFun: (ListOutput, T) => Any) =
     createList(readFun, writeFun, allowNull = false)
 
+  /**
+    * Helper method to manually implement a `GenCodec` that writes an object. NOTE: in most cases the easiest way to
+    * have a custom object codec is to manually implement `apply` and `unapply`/`unapplySeq` methods in companion object
+    * of your type or use [[fromApplyUnapplyProvider]] if the type comes from a third party code and you can't
+    * modify its companion object.
+    */
   def createObject[T](readFun: ObjectInput => T, writeFun: (ObjectOutput, T) => Any, allowNull: Boolean) =
     new ObjectCodec[T] {
       def nullable = allowNull
@@ -214,6 +220,11 @@ object GenCodec extends RecursiveAutoCodecs with TupleGenCodecs {
     }
   }
 
+  /**
+    * Convenience base class for `GenCodec`s that serialize values as objects.
+    * NOTE: if you need to implement a custom `GenCodec` that writes an object, the best way to do it is to have
+    * manually implemented `apply` and `unapply` in companion object or by using [[GenCodec.fromApplyUnapplyProvider]].
+    */
   trait ObjectCodec[T] extends NullSafeCodec[T] {
     def readObject(input: ObjectInput): T
     def writeObject(output: ObjectOutput, value: T): Unit

commons-core/src/main/scala/com/avsystem/commons/serialization/InputOutput.scala

@@ -1,8 +1,6 @@
 package com.avsystem.commons
 package serialization
 
-import com.avsystem.commons.serialization.GenCodec.ReadFailure
-
 /**
   * Represents an abstract sink to which a value may be serialized (written).
   * An [[Output]] instance should be assumed to be stateful. After calling any of the `write` methods, it MUST NOT be
@@ -192,12 +190,35 @@ trait ObjectInput extends Any with SequentialInput { self =>
     * You MUST NOT call `nextField()` again until this [[com.avsystem.commons.serialization.FieldInput FieldInput]]
     * is fully read or skipped.
     * </p>
-    * Subsequent invocations of `nextField` MUST return fields in exactly the same order as they were written
-    * using [[com.avsystem.commons.serialization.ObjectOutput.writeField ObjectOutput.writeField]].
-    * In other words, serialization format MUST preserve order of object fields.
+    * Serialization format implemented by this `ObjectInput` must either preserve order of fields (as they are
+    * written by corresponding `ObjectOutput`) OR it must provide random field access capability.
+    * <ul>
+    * <li>If the serialization format is able to preserve object field order then [[nextField]] must return
+    * object fields in exactly the same order as they were written by `ObjectOutput.writeField`. This is
+    * natural for most serialization formats backed by strings, raw character or byte sequences, e.g.
+    * JSON implemented by [[com.avsystem.commons.serialization.json.JsonStringOutput JsonStringOutput]]/
+    * [[com.avsystem.commons.serialization.json.JsonStringInput JsonStringInput]].</li>
+    * <li>If the serialization format is unable to preserve object field order (e.g. because it uses hash maps to
+    * represent objects) then it must instead support random, by-name field access by overriding [[peekField]].
+    * </li>
+    * </ul>
     */
   def nextField(): FieldInput
 
+  /**
+    * If serialization format implemented by `ObjectInput` does NOT preserve field order, then this method MUST
+    * be overridden to support random field access. It should return non-empty [[Opt]] containing input for every field
+    * present in the object, regardless of field order assumed by [[nextField]].
+    * `Opt.Empty` is returned when field is absent or always when this `ObjectInput` does not support random field
+    * access (in which case it must preserve field order instead).
+    * NOTE: calling [[peekField]] and using [[FieldInput]] returned by it MUST NOT change state of this `ObjectInput`.
+    * Therefore, it cannot in any way influence results returned by [[nextField]] and [[hasNext]].
+    * For example, if a [[FieldInput]] for particular field has already been
+    * accessed using [[peekField]] but has not yet been returned by [[nextField]] then it MUST be returned at some
+    * point in the future by [[nextField]].
+    */
+  def peekField(name: String): Opt[FieldInput] = Opt.Empty
+
   def skipRemaining() = while (hasNext) nextField().skip()
   def iterator[A](readFun: Input => A): Iterator[(String, A)] =
     new Iterator[(String, A)] {
@@ -214,11 +235,4 @@ trait ObjectInput extends Any with SequentialInput { self =>
   */
 trait FieldInput extends Input {
   def fieldName: String
-
-  def assertField(expectedName: String): this.type = {
-    if (fieldName != expectedName) {
-      throw new ReadFailure(s"Expected $expectedName as next field, got $fieldName")
-    }
-    this
-  }
 }

commons-core/src/main/scala/com/avsystem/commons/serialization/SimpleValueInputOutput.scala

@@ -55,7 +55,7 @@ class SimpleValueOutput(consumer: Any => Unit) extends Output {
   def writeBoolean(boolean: Boolean) = consumer(boolean)
 
   def writeObject() = new ObjectOutput {
-    private val result = new mutable.LinkedHashMap[String, Any]
+    private val result = new mutable.HashMap[String, Any]
     def writeField(key: String) = new SimpleValueOutput(v => result += ((key, v)))
     def finish() = consumer(result)
   }
@@ -95,10 +95,12 @@ class SimpleValueInput(value: Any) extends Input {
   def readNull() = if (value == null) null else throw new ReadFailure("not null")
   def readObject() =
     new ObjectInput {
-      private val it = doRead[BMap[String, Any]].iterator.map {
+      private val map = doRead[BMap[String, Any]]
+      private val it = map.iterator.map {
         case (k, v) => new SimpleValueFieldInput(k, v)
       }
       def nextField() = it.next()
+      override def peekField(name: String) = map.getOpt(name).map(new SimpleValueFieldInput(name, _))
       def hasNext = it.hasNext
     }
 

commons-core/src/main/scala/com/avsystem/commons/serialization/macroCodecs.scala

@@ -143,16 +143,19 @@ abstract class FlatSealedHierarchyCodec[T](
   final def readObject(input: ObjectInput): T = {
     val oooFields = new FieldValues(oooFieldNames, oooDeps, typeRepr)
 
+    def readCase(caseNameField: FieldInput): T = {
+      val caseName = readCaseName(caseNameField)
+      caseIndexByName(caseName) match {
+        case -1 => unknownCase(caseName)
+        case idx => readFlatCase(caseName, oooFields, input, caseDeps(idx))
+      }
+    }
+
     def read(): T =
       if (input.hasNext) {
         val fi = input.nextField()
-        if (fi.fieldName == caseFieldName) {
-          val caseName = readCaseName(fi)
-          caseIndexByName(caseName) match {
-            case -1 => unknownCase(caseName)
-            case idx => readFlatCase(caseName, oooFields, input, caseDeps(idx))
-          }
-        } else if (!oooFields.tryReadField(fi)) {
+        if (fi.fieldName == caseFieldName) readCase(fi)
+        else if (!oooFields.tryReadField(fi)) {
           if (caseDependentFieldNames.contains(fi.fieldName)) {
             if (defaultCaseIdx != -1) {
               val defaultCaseName = caseNames(defaultCaseIdx)
@@ -174,7 +177,10 @@ abstract class FlatSealedHierarchyCodec[T](
         missingCase
       }
 
-    read()
+    input.peekField(caseFieldName) match {
+      case Opt(fi) => readCase(fi)
+      case Opt.Empty => read()
+    }
   }
 }
 

commons-core/src/test/scala/com/avsystem/commons/serialization/GenCodecTest.scala

@@ -70,10 +70,10 @@ class GenCodecTest extends CodecTestBase {
     testWriteReadAndAutoWriteRead[JSortedSet[Int]](jTreeSet, List(1, 2, 3))
     testWriteReadAndAutoWriteRead[JNavigableSet[Int]](jTreeSet, List(1, 2, 3))
     testWriteReadAndAutoWriteRead[JTreeSet[Int]](jTreeSet, List(1, 2, 3))
-    testWriteReadAndAutoWriteRead[JMap[String, Int]](jHashMap, ListMap("1" -> 1, "2" -> 2, "3" -> 3))
-    testWriteReadAndAutoWriteRead[JHashMap[String, Int]](jHashMap, ListMap("1" -> 1, "2" -> 2, "3" -> 3))
-    testWriteReadAndAutoWriteRead[JLinkedHashMap[String, Int]](jLinkedHashMap, ListMap("1" -> 1, "2" -> 2, "3" -> 3))
-    testWriteReadAndAutoWriteRead[JHashMap[Int, Int]](jIntHashMap, ListMap("1" -> 1, "2" -> 2, "3" -> 3))
+    testWriteReadAndAutoWriteRead[JMap[String, Int]](jHashMap, Map("1" -> 1, "2" -> 2, "3" -> 3))
+    testWriteReadAndAutoWriteRead[JHashMap[String, Int]](jHashMap, Map("1" -> 1, "2" -> 2, "3" -> 3))
+    testWriteReadAndAutoWriteRead[JLinkedHashMap[String, Int]](jLinkedHashMap, Map("1" -> 1, "2" -> 2, "3" -> 3))
+    testWriteReadAndAutoWriteRead[JHashMap[Int, Int]](jIntHashMap, Map("1" -> 1, "2" -> 2, "3" -> 3))
   }
 
   test("NoState test") {
@@ -83,12 +83,12 @@ class GenCodecTest extends CodecTestBase {
 
   test("collections and wrappers test") {
     testWriteReadAndAutoWriteRead[Option[Int]](option, List(42))
-    testWriteReadAndAutoWriteRead[Either[Int, String]](Left(42), ListMap("Left" -> 42))
-    testWriteReadAndAutoWriteRead[Either[Int, String]](Right("lol"), ListMap("Right" -> "lol"))
+    testWriteReadAndAutoWriteRead[Either[Int, String]](Left(42), Map("Left" -> 42))
+    testWriteReadAndAutoWriteRead[Either[Int, String]](Right("lol"), Map("Right" -> "lol"))
     testWriteReadAndAutoWriteRead[List[Int]](list, list)
     testWriteReadAndAutoWriteRead[Set[Int]](set, set.toList)
     testWriteReadAndAutoWriteRead[Map[String, Int]](map, map)
-    testWriteReadAndAutoWriteRead[Map[Int, Int]](intMap, ListMap("1" -> 1, "2" -> 2, "3" -> 3))
+    testWriteReadAndAutoWriteRead[Map[Int, Int]](intMap, Map("1" -> 1, "2" -> 2, "3" -> 3))
     testWriteReadAndAutoWriteRead[IHashMap[String, Int]](hashMap, hashMap)
   }
 
@@ -122,7 +122,7 @@ class GenCodecTest extends CodecTestBase {
   object SingleArgCaseClass extends HasGenCodec[SingleArgCaseClass]
 
   test("single arg case class test") {
-    testWriteReadAndAutoWriteRead(SingleArgCaseClass("something"), ListMap("str" -> "something"))
+    testWriteReadAndAutoWriteRead(SingleArgCaseClass("something"), Map("str" -> "something"))
   }
 
   @transparent
@@ -149,7 +149,7 @@ class GenCodecTest extends CodecTestBase {
 
   test("case class test") {
     testWriteReadAndAutoWriteRead(SomeCaseClass("dafuq", List(1, 2, 3)),
-      ListMap("some.str" -> "dafuq", "intList" -> List(1, 2, 3), "someStrLen" -> 5)
+      Map("some.str" -> "dafuq", "intList" -> List(1, 2, 3), "someStrLen" -> 5)
     )
   }
 
@@ -166,7 +166,7 @@ class GenCodecTest extends CodecTestBase {
   }
 
   test("case class with wildcard test") {
-    testWriteReadAndAutoWriteRead(CaseClassWithWildcard(Stuff("lol")), ListMap("stuff" -> "lol"))
+    testWriteReadAndAutoWriteRead(CaseClassWithWildcard(Stuff("lol")), Map("stuff" -> "lol"))
   }
 
   class CaseClassLike(val str: String, val intList: List[Int])
@@ -179,7 +179,7 @@ class GenCodecTest extends CodecTestBase {
 
   test("case class like test") {
     testWriteReadAndAutoWriteRead(CaseClassLike("dafuq", List(1, 2, 3)),
-      ListMap("some.str" -> "dafuq", "intList" -> List(1, 2, 3))
+      Map("some.str" -> "dafuq", "intList" -> List(1, 2, 3))
     )
   }
 
@@ -199,7 +199,7 @@ class GenCodecTest extends CodecTestBase {
 
   test("case class like with inherited apply/unapply test") {
     testWriteReadAndAutoWriteRead(HasInheritedApply("dafuq", List(1, 2, 3)),
-      ListMap("a" -> "dafuq", "lb" -> List(1, 2, 3))
+      Map("a" -> "dafuq", "lb" -> List(1, 2, 3))
     )
   }
 
@@ -212,7 +212,7 @@ class GenCodecTest extends CodecTestBase {
   test("apply/unapply provider based codec test") {
     implicit val tpCodec: GenCodec[ThirdParty] = GenCodec.fromApplyUnapplyProvider[ThirdParty](ThirdPartyFakeCompanion)
     testWriteReadAndAutoWriteRead(ThirdParty(42, "lol"),
-      ListMap("str" -> "lol", "int" -> 42)
+      Map("str" -> "lol", "int" -> 42)
     )
   }
 
@@ -223,7 +223,7 @@ class GenCodecTest extends CodecTestBase {
 
   test("varargs case class test") {
     testWriteReadAndAutoWriteRead(VarargsCaseClass(42, "foo", "bar"),
-      ListMap("int" -> 42, "strings" -> List("foo", "bar"))
+      Map("int" -> 42, "strings" -> List("foo", "bar"))
     )
   }
 
@@ -237,7 +237,7 @@ class GenCodecTest extends CodecTestBase {
 
   test("varargs case class like test") {
     testWriteReadAndAutoWriteRead(VarargsCaseClassLike("dafuq", 1, 2, 3),
-      ListMap("some.str" -> "dafuq", "ints" -> List(1, 2, 3))
+      Map("some.str" -> "dafuq", "ints" -> List(1, 2, 3))
     )
   }
 
@@ -247,9 +247,9 @@ class GenCodecTest extends CodecTestBase {
   }
 
   test("case class with default values test") {
-    testWriteReadAndAutoWriteRead(HasDefaults(str = "lol"), ListMap("str" -> "lol"))
-    testWriteReadAndAutoWriteRead(HasDefaults(43, "lol"), ListMap("int" -> 43, "str" -> "lol"))
-    testWriteReadAndAutoWriteRead(HasDefaults(str = null), ListMap("str" -> null))
+    testWriteReadAndAutoWriteRead(HasDefaults(str = "lol"), Map("str" -> "lol"))
+    testWriteReadAndAutoWriteRead(HasDefaults(43, "lol"), Map("int" -> 43, "str" -> "lol"))
+    testWriteReadAndAutoWriteRead(HasDefaults(str = null), Map("str" -> null))
   }
 
   case class Node[T](value: T, children: List[Node[T]] = Nil)
@@ -287,41 +287,47 @@ class GenCodecTest extends CodecTestBase {
   }
 
   test("recursively defined sealed hierarchy with explicit case class codec test") {
-    testWriteReadAndAutoWriteRead[CustomList](CustomTail, ListMap("CustomTail" -> Map()))
+    testWriteReadAndAutoWriteRead[CustomList](CustomTail, Map("CustomTail" -> Map()))
     testWriteReadAndAutoWriteRead[CustomList](CustomCons(CustomCons(CustomTail)),
-      ListMap("CustomCons" -> ListMap("CustomCons" -> ListMap("CustomTail" -> Map()))))
+      Map("CustomCons" -> Map("CustomCons" -> Map("CustomTail" -> Map()))))
   }
 
   test("value class test") {
-    testWriteReadAndAutoWriteRead(ValueClass("costam"), ListMap("str" -> "costam"))
+    testWriteReadAndAutoWriteRead(ValueClass("costam"), Map("str" -> "costam"))
   }
 
   test("sealed hierarchy test") {
     testWriteReadAndAutoWriteRead[SealedBase](SealedBase.CaseObject,
-      ListMap("CaseObject" -> Map()))
+      Map("CaseObject" -> Map()))
     testWriteReadAndAutoWriteRead[SealedBase](SealedBase.CaseClass("fuu"),
-      ListMap("CaseClass" -> ListMap("str" -> "fuu")))
+      Map("CaseClass" -> Map("str" -> "fuu")))
     testWriteReadAndAutoWriteRead[SealedBase](SealedBase.InnerBase.InnerCaseObject,
-      ListMap("InnerCaseObject" -> Map()))
+      Map("InnerCaseObject" -> Map()))
     testWriteReadAndAutoWriteRead[SealedBase](SealedBase.InnerBase.InnerCaseClass("fuu"),
-      ListMap("InnerCaseClass" -> ListMap("str" -> "fuu")))
+      Map("InnerCaseClass" -> Map("str" -> "fuu")))
   }
 
   test("flat sealed hierarchy test") {
     testWriteReadAndAutoWriteRead[FlatSealedBase](FlatSealedBase.FirstCase("fuu", 42),
-      ListMap("_case" -> "FirstCase", "_id" -> "fuu", "int" -> 42, "upper_id" -> "FUU"))
+      Map("_case" -> "FirstCase", "_id" -> "fuu", "int" -> 42, "upper_id" -> "FUU"))
     testWriteReadAndAutoWriteRead[FlatSealedBase](FlatSealedBase.SecondCase("bar", 3.14, 1.0, 2.0),
-      ListMap("_case" -> "SecondCase", "_id" -> "bar", "dbl" -> 3.14, "moar" -> List(1.0, 2.0), "upper_id" -> "BAR"))
+      Map("_case" -> "SecondCase", "_id" -> "bar", "dbl" -> 3.14, "moar" -> List(1.0, 2.0), "upper_id" -> "BAR"))
     testWriteReadAndAutoWriteRead[FlatSealedBase](FlatSealedBase.ThirdCase,
-      ListMap("_case" -> "ThirdCase", "_id" -> "third", "upper_id" -> "THIRD"))
+      Map("_case" -> "ThirdCase", "_id" -> "third", "upper_id" -> "THIRD"))
+  }
+
+  test("random field access dependent flat sealed hierarchy reading test") {
+    testReadAndAutoRead[FlatSealedBase](
+      ListMap("_id" -> "fuu", "int" -> 42, "upper_id" -> "FUU", "_case" -> "FirstCase"),
+      FlatSealedBase.FirstCase("fuu", 42))
   }
 
   test("out of order field in flat sealed hierarchy test") {
     testReadAndAutoRead[FlatSealedBase](
-      ListMap("_id" -> "fuu", "upper_id" -> "FUU", "random" -> 13, "_case" -> "FirstCase", "int" -> 42),
+      Map("_id" -> "fuu", "upper_id" -> "FUU", "random" -> 13, "_case" -> "FirstCase", "int" -> 42),
       FlatSealedBase.FirstCase("fuu", 42))
     testReadAndAutoRead[FlatSealedBase](
-      ListMap("_id" -> "bar", "upper_id" -> "FUU", "random" -> 13, "_case" -> "SecondCase", "dbl" -> 3.14, "moar" -> List(1.0, 2.0)),
+      Map("_id" -> "bar", "upper_id" -> "FUU", "random" -> 13, "_case" -> "SecondCase", "dbl" -> 3.14, "moar" -> List(1.0, 2.0)),
       FlatSealedBase.SecondCase("bar", 3.14, 1.0, 2.0))
   }
 
@@ -342,10 +348,10 @@ class GenCodecTest extends CodecTestBase {
   }
 
   test("GADT test") {
-    testWriteReadAndAutoWriteRead[Expr[_]](NullExpr, ListMap("NullExpr" -> Map()))
-    testWriteReadAndAutoWriteRead[Expr[_]](StringExpr("stringzor"), ListMap("StringExpr" -> ListMap("str" -> "stringzor")))
-    testWriteReadAndAutoWriteRead[Expr[String]](StringExpr("stringzor"), ListMap("StringExpr" -> ListMap("str" -> "stringzor")))
-    testWriteReadAndAutoWriteRead[BaseExpr](StringExpr("stringzor"), ListMap("StringExpr" -> ListMap("str" -> "stringzor")))
+    testWriteReadAndAutoWriteRead[Expr[_]](NullExpr, Map("NullExpr" -> Map()))
+    testWriteReadAndAutoWriteRead[Expr[_]](StringExpr("stringzor"), Map("StringExpr" -> Map("str" -> "stringzor")))
+    testWriteReadAndAutoWriteRead[Expr[String]](StringExpr("stringzor"), Map("StringExpr" -> Map("str" -> "stringzor")))
+    testWriteReadAndAutoWriteRead[BaseExpr](StringExpr("stringzor"), Map("StringExpr" -> Map("str" -> "stringzor")))
   }
 
   sealed trait Tree[T]
@@ -370,11 +376,11 @@ class GenCodecTest extends CodecTestBase {
           Leaf(3)
         )
       ),
-      ListMap("Branch" -> Map(
-        "left" -> ListMap("Leaf" -> ListMap("value" -> 1)),
-        "right" -> ListMap("Branch" -> Map(
-          "left" -> ListMap("Leaf" -> ListMap("value" -> 2)),
-          "right" -> ListMap("Leaf" -> ListMap("value" -> 3))
+      Map("Branch" -> Map(
+        "left" -> Map("Leaf" -> Map("value" -> 1)),
+        "right" -> Map("Branch" -> Map(
+          "left" -> Map("Leaf" -> Map("value" -> 2)),
+          "right" -> Map("Leaf" -> Map("value" -> 3))
         ))
       ))
     )
@@ -391,9 +397,9 @@ class GenCodecTest extends CodecTestBase {
   }
 
   test("sealed enum test") {
-    testWriteReadAndAutoWriteRead[Enumz](Enumz.First, ListMap("Primary" -> Map()))
-    testWriteReadAndAutoWriteRead[Enumz](Enumz.Second, ListMap("Second" -> Map()))
-    testWriteReadAndAutoWriteRead[Enumz](Enumz.Third, ListMap("Third" -> Map()))
+    testWriteReadAndAutoWriteRead[Enumz](Enumz.First, Map("Primary" -> Map()))
+    testWriteReadAndAutoWriteRead[Enumz](Enumz.Second, Map("Second" -> Map()))
+    testWriteReadAndAutoWriteRead[Enumz](Enumz.Third, Map("Third" -> Map()))
   }
 
   sealed trait KeyEnumz