Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug preventing DataTemplates.readRecord from processing generated templates #71

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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])
applyMethod.invoke(companionInstance, data, DataConversion.SetReadOnly).asInstanceOf[T]
val buildMethod = getTemplateDataConstructor(companionInstance, classOf[DataMap])

buildMethod.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])
applyMethod.invoke(companionInstance, data, DataConversion.SetReadOnly).asInstanceOf[T]
val buildMethod = getTemplateDataConstructor(companionInstance, classOf[DataList])
buildMethod.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])
applyMethod.invoke(companionInstance, data, DataConversion.SetReadOnly).asInstanceOf[T]
val buildMethod = getTemplateDataConstructor(companionInstance, classOf[DataMap])

buildMethod.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")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down