Fix bug preventing DataTemplates.readRecord to process generated temp…

…lates (#72) Also: - Enable scala-runtime test cases
coursera · Jan 16, 2018 · 9e5d82d · 9e5d82d
1 parent 4d51635
commit 9e5d82d
Show file tree

Hide file tree

Showing 5 changed files with 153 additions and 27 deletions.
diff --git a/scala/generator-test/src/test/scala/org/coursera/courier/generator/RecordGeneratorTest.scala b/scala/generator-test/src/test/scala/org/coursera/courier/generator/RecordGeneratorTest.scala
@@ -470,4 +470,15 @@ class RecordGeneratorTest extends GeneratorTest with SchemaFixtures {
     // Fortunately, nothing should depend solely on hashCode() for determining whether two
     // records match.
   }
+
+  @Test
+  def testDataTemplateHelperCompatibility(): Unit = {
+    // DataTemplates provides a bunch of convenient helper methods for
+    // serializing and deserializing records. Let's make sure the generated
+    // records are compatible with those methods.
+    val origRecord = Simple(Some("Hi"))
+    val jsString = DataTemplates.writeRecord(origRecord)
+    val roundTrippedRecord = DataTemplates.readRecord[Simple](jsString)
+    assert(origRecord === roundTrippedRecord)
+  }
 }
diff --git a/scala/generator-test/src/test/scala/org/coursera/courier/generator/UnionGeneratorTest.scala b/scala/generator-test/src/test/scala/org/coursera/courier/generator/UnionGeneratorTest.scala
@@ -155,4 +155,16 @@ class UnionGeneratorTest extends GeneratorTest with SchemaFixtures {
   def testUnionTyperefSchema(): Unit = {
     assert(TypedDefinition.TYPEREF_SCHEMA.getDereferencedDataSchema === TypedDefinition.SCHEMA)
   }
+
+  @Test
+  def testDataTemplateHelperSupport(): Unit = {
+    // DataTemplates provides a bunch of convenient helper methods for
+    // serializing and deserializing unions. Let's make sure the generated
+    // unions are compatible with those methods.
+    val origMember = WithPrimitiveCustomTypesUnion.Union.CustomIntMember(CustomInt(1))
+    val original = WithPrimitiveCustomTypesUnion(origMember)
+    val json = DataTemplates.writeUnion(origMember)
+    val roundTripped = DataTemplates.readUnion[WithPrimitiveCustomTypesUnion.Union](json)
+    assert(origMember === roundTripped)
+  }
 }
diff --git a/scala/runtime/src/main/scala/org/coursera/courier/templates/DataTemplates.scala b/scala/runtime/src/main/scala/org/coursera/courier/templates/DataTemplates.scala
@@ -16,6 +16,8 @@
 
 package org.coursera.courier.templates
 
+import java.lang.reflect.Method
+
 import com.linkedin.data.DataComplex
 import com.linkedin.data.DataList
 import com.linkedin.data.DataMap
@@ -211,32 +213,52 @@ object DataTemplates {
   private[this] def newDataMapTemplate[T <: DataTemplate[DataMap]](
       data: DataMap, clazz: Class[T]): T = {
     val companionInstance = companion(clazz)
-    val applyMethod =
-      companionInstance.getClass.getDeclaredMethod(
-        "apply",
-        classOf[DataMap],
-        classOf[DataConversion])
+    val applyMethod = getTemplateDataConstructor(companionInstance, classOf[DataMap])
+
     applyMethod.invoke(companionInstance, data, DataConversion.SetReadOnly).asInstanceOf[T]
   }
 
   private[this] def newDataListTemplate[T <: DataTemplate[DataList]](
       data: DataList, clazz: Class[T]): T = {
     val companionInstance = companion(clazz)
-    val applyMethod =
-      companionInstance.getClass.getDeclaredMethod(
-        "apply",
-        classOf[DataList],
-        classOf[DataConversion])
+    val applyMethod = getTemplateDataConstructor(companionInstance, classOf[DataList])
     applyMethod.invoke(companionInstance, data, DataConversion.SetReadOnly).asInstanceOf[T]
   }
 
   private[this] def newUnionTemplate[T <: UnionTemplate](data: DataMap, clazz: Class[T]): T = {
     val companionInstance = companion(clazz)
-    val applyMethod =
-      companionInstance.getClass.getDeclaredMethod(
-        "apply",
-        classOf[DataMap],
-        classOf[DataConversion])
+    val applyMethod = getTemplateDataConstructor(companionInstance, classOf[DataMap])
+
     applyMethod.invoke(companionInstance, data, DataConversion.SetReadOnly).asInstanceOf[T]
   }
+
+  /**
+    * Reflect for the correct method to instantiate the template from a DataMap
+    * and DataConversion. The correct method is usually "build", since
+    * pre-2.0.3. Before then it was "apply". In case this method is reading
+    * templates generated in an old version, we start with "build" and fall back
+    * on "apply".
+    *
+    * @param tmplCompanionInstance the result of calling the private method companion(clazz)
+    * @param dataClass either classOf[DataMap] or classOf[DataList], depending on whether
+    *                  operating on a record/union or an array.
+    * @return a method that can be construct a Union or Record template when
+    *         invoked a DataMap and DataConversion as args
+    */
+  private[this] def getTemplateDataConstructor(tmplCompanionInstance: AnyRef, dataClass: Class[_]): Method = {
+    val companionClass = tmplCompanionInstance.getClass
+    def getWithName(methodName: String): Method = {
+      companionClass.getDeclaredMethod(
+        methodName,
+        dataClass,
+        classOf[DataConversion]
+      )
+    }
+
+    try {
+      getWithName("build")
+    } catch {
+      case e: NoSuchMethodException => getWithName("apply")
+    }
+  }
 }
diff --git a/scala/runtime/src/test/scala/org/coursera/courier/codecs/InlineStringCodecTest.scala b/scala/runtime/src/test/scala/org/coursera/courier/codecs/InlineStringCodecTest.scala
@@ -33,9 +33,9 @@ import com.linkedin.data.template.DataTemplateUtil
 import org.coursera.courier.templates.DataTemplates
 import org.coursera.courier.templates.DataValidationException
 import org.junit.Test
-import org.scalatest.junit.AssertionsForJUnit
+import org.scalatest.junit.{AssertionsForJUnit, JUnitSuite}
 
-class InlineStringCodecTest extends AssertionsForJUnit {
+class InlineStringCodecTest extends JUnitSuite with AssertionsForJUnit {
 
   @Test
   def testPrimitives(): Unit = {

diff --git a/scala/runtime/src/test/scala/org/coursera/courier/templates/DataTemplatesTest.scala b/scala/runtime/src/test/scala/org/coursera/courier/templates/DataTemplatesTest.scala
@@ -23,40 +23,91 @@ import com.linkedin.data.schema.UnionDataSchema
 import com.linkedin.data.template.DataTemplateUtil
 import com.linkedin.data.template.RecordTemplate
 import com.linkedin.data.template.UnionTemplate
+import org.coursera.courier.data.IntArray
 import org.coursera.courier.templates.DataTemplates.DataConversion
-import org.scalatest.junit.AssertionsForJUnit
+import org.scalatest.junit.{AssertionsForJUnit, JUnitSuite}
 import org.junit.Test
 
-class DataTemplatesTest extends AssertionsForJUnit {
+class DataTemplatesTest extends JUnitSuite with AssertionsForJUnit {
   import DataTemplatesTest._
 
+  @Test
+  def readWriteLegacyRecord(): Unit = {
+    val json = """{"string":"a","int":1}"""
+    val record = DataTemplates.readRecord[MockLegacyRecord](json)
+    val roundTripped = DataTemplates.writeRecord(record)
+    assert(DataTemplates.readDataMap(roundTripped) === DataTemplates.readDataMap(json))
+  }
+
   @Test
   def readWriteRecord(): Unit = {
     val json = """{"string":"a","int":1}"""
-    val union = DataTemplates.readRecord[MockRecord](json)
-    val roundTripped = DataTemplates.writeRecord(union)
+    val record = DataTemplates.readRecord[MockRecord](json)
+    val roundTripped = DataTemplates.writeRecord(record)
     assert(DataTemplates.readDataMap(roundTripped) === DataTemplates.readDataMap(json))
   }
 
   @Test
   def readWriteUnion(): Unit = {
     val json = """{"int":1}"""
-    val union = DataTemplates.readUnion[MockTyperefUnion](json)
+    val union = DataTemplates.readUnion[MockLegacyTyperefUnion](json)
+    val roundTripped = DataTemplates.writeUnion(union)
+    assert(DataTemplates.readDataMap(roundTripped) === DataTemplates.readDataMap(json))
+  }
+
+  @Test
+  def readWriteLegacyUnion(): Unit = {
+    val json = """{"int":1}"""
+    val union = DataTemplates.readUnion[MockLegacyTyperefUnion](json)
     val roundTripped = DataTemplates.writeUnion(union)
     assert(DataTemplates.readDataMap(roundTripped) === DataTemplates.readDataMap(json))
   }
 
   @Test
   def testGetSchema(): Unit = {
-    val schemaFromClass = DataTemplates.getSchema(classOf[MockRecord])
-    assert(schemaFromClass === MockRecord.SCHEMA)
+    val schemaFromClass = DataTemplates.getSchema(classOf[MockLegacyRecord])
+    assert(schemaFromClass === MockLegacyRecord.SCHEMA)
+
+    val schemaFromClassTag = DataTemplates.getSchema[MockLegacyRecord]
+    assert(schemaFromClassTag === MockLegacyRecord.SCHEMA)
+  }
 
-    val schemaFromClassTag = DataTemplates.getSchema[MockRecord]
-    assert(schemaFromClassTag === MockRecord.SCHEMA)
+  @Test
+  def testReadWriteArray(): Unit = {
+    val array = "[1,2,3,4,5]"
+    val read = DataTemplates.readArray[IntArray](array)
+    val written = DataTemplates.writeArray(read)
+    assert(array === written)
   }
 }
 
 object DataTemplatesTest {
+  class MockLegacyRecord(private val dataMap: DataMap)
+    extends RecordTemplate(dataMap, MockLegacyRecord.SCHEMA) {
+    dataMap.makeReadOnly()
+  }
+
+  object MockLegacyRecord {
+    val SCHEMA_JSON =
+      """
+        |{
+        |  "name": "MockLegacyRecord",
+        |  "type": "record",
+        |  "fields": [
+        |    { "name": "string", "type": "string" },
+        |    { "name": "int", "type": "int" }
+        |  ]
+        |}
+        |""".stripMargin
+
+    val SCHEMA = DataTemplateUtil.parseSchema(SCHEMA_JSON).asInstanceOf[RecordDataSchema]
+
+    def apply(dataMap: DataMap, dataConversion: DataConversion) = {
+      new MockLegacyRecord(dataMap)
+    }
+  }
+
+  /** Like MockRecord, but has `build` instead of `apply`, as per */
   class MockRecord(private val dataMap: DataMap)
     extends RecordTemplate(dataMap, MockRecord.SCHEMA) {
     dataMap.makeReadOnly()
@@ -77,11 +128,41 @@ object DataTemplatesTest {
 
     val SCHEMA = DataTemplateUtil.parseSchema(SCHEMA_JSON).asInstanceOf[RecordDataSchema]
 
-    def apply(dataMap: DataMap, dataConversion: DataConversion) = {
+    def build(dataMap: DataMap, dataConversion: DataConversion) = {
       new MockRecord(dataMap)
     }
   }
 
+  class MockLegacyTyperefUnion(private val dataMap: DataMap)
+    extends UnionTemplate(dataMap, MockLegacyTyperefUnion.SCHEMA) {
+    dataMap.makeReadOnly()
+  }
+
+  object MockLegacyTyperefUnion {
+    val SCHEMA_JSON =
+      """
+        |[ "int", "string" ]
+        |""".stripMargin
+
+    val SCHEMA = DataTemplateUtil.parseSchema(SCHEMA_JSON).asInstanceOf[UnionDataSchema]
+
+    val TYPEREF_SCHEMA_JSON =
+      """
+        |{
+        |  "name": "MockTyperefUnion",
+        |  "type": "typeref",
+        |  "ref": [ "int", "string" ]
+        |}
+        |""".stripMargin
+
+    val TYPEREF_SCHEMA =
+      DataTemplateUtil.parseSchema(TYPEREF_SCHEMA_JSON).asInstanceOf[TyperefDataSchema]
+
+    def apply(dataMap: DataMap, dataConversion: DataConversion) = {
+      new MockLegacyTyperefUnion(dataMap)
+    }
+  }
+
   class MockTyperefUnion(private val dataMap: DataMap)
     extends UnionTemplate(dataMap, MockTyperefUnion.SCHEMA) {
     dataMap.makeReadOnly()