Skip to content

Commit

Permalink
Only allow accesor methods for BigQuery fields in case classes (#3651)
Browse files Browse the repository at this point in the history
  • Loading branch information
jordiolivares authored Mar 3, 2021
1 parent 48b8428 commit 6f9cd32
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private[types] object MacroUtil {
.forall(b => t.baseClasses.contains(b.typeSymbol))

def isField(s: Symbol): Boolean =
s.isPublic && s.isMethod && !s.isSynthetic && !s.isConstructor
s.isPublic && s.isMethod && s.asMethod.isCaseAccessor && !s.isSynthetic && !s.isConstructor

// Case class helpers for macros

Expand All @@ -46,7 +46,7 @@ private[types] object MacroUtil {
}

def isField(c: blackbox.Context)(s: c.Symbol): Boolean =
s.isPublic && s.isMethod && !s.isSynthetic && !s.isConstructor
s.isPublic && s.isMethod && s.asMethod.isCaseAccessor && !s.isSynthetic && !s.isConstructor
def getFields(c: blackbox.Context)(t: c.Type): Iterable[c.Symbol] = {
import c.universe._
val fields = t.decls.filter(isField(c))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,14 @@ final class ConverterProviderSpec
EqDerivation[RepeatedNested].eqv(r1, r2) shouldBe true
}
}

property("round trip case class with method nested types") {
forAll { r1: CaseClassWithMethods =>
val r2 =
BigQueryType.fromTableRow[CaseClassWithMethods](
BigQueryType.toTableRow[CaseClassWithMethods](r1)
)
EqDerivation[CaseClassWithMethods].eqv(r1, r2) shouldBe true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ class ConverterProviderTest extends AnyFlatSpec with Matchers {
RequiredGeo.fromTableRow(TableRow("a" -> wkt)) shouldBe RequiredGeo(Geography(wkt))
BigQueryType.toTableRow[RequiredGeo](RequiredGeo(Geography(wkt))) shouldBe TableRow("a" -> wkt)
}

it should "handle case classes with methods" in {
RequiredWithMethod.fromTableRow(TableRow("a" -> "")) shouldBe RequiredWithMethod("")
BigQueryType.toTableRow[RequiredWithMethod](RequiredWithMethod("")) shouldBe TableRow("a" -> "")
}
}

object ConverterProviderTest {
Expand All @@ -60,4 +65,11 @@ object ConverterProviderTest {

@BigQueryType.toTable
case class Repeated(a: List[String])

@BigQueryType.toTable
case class RequiredWithMethod(a: String) {
val caseClassPublicValue: String = ""
def accessorMethod: String = ""
def method(x: String): String = x
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,8 @@ class SchemaProviderTest extends AnyFlatSpec with Matchers {
// The description annotation should be serializable.
SerializableUtils.ensureSerializable(new description(value = "this a field description"))
}

it should "ignore methods in case classes" in {
SchemaProvider.schemaOf[CaseClassWithMethods] shouldBe parseSchema(recordFields("REQUIRED"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,10 @@ object Schemas {
@description("account user") user: User,
@description("in USD") balance: Double
)

case class CaseClassWithMethods(required: Required, optional: Optional, repeated: Repeated) {
val innerAccessorValue: String = ""
def accessorMethod: String = ""
def nonAccessorMethod(x: String): String = x
}
}

0 comments on commit 6f9cd32

Please sign in to comment.