Skip to content

Commit

Permalink
finagle-mysql: Correctly convert java.sql.{Date,Timestamp} to Parameter
Browse files Browse the repository at this point in the history
Problem

The java.sql.Date and java.sql.Timestamp implicits are ignored and the
java.util.Date conversion is used instead, because of the contravariance of the
CanBeParameter type parameter and the fact that java.sql.Date and
java.sql.Timestamp extend java.util.Date.

Solution

Use only one implicit (for the supertype) and pattern match within it to create
the correct Parameter.

JIRA Issues: CSL-6021

TBR=true

Differential Revision: https://phabricator.twitter.biz/D195351
  • Loading branch information
Stefan Lance authored and jenkins committed Jul 24, 2018
1 parent 77a6a31 commit b486772
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 29 deletions.
4 changes: 4 additions & 0 deletions CHANGES
Expand Up @@ -29,6 +29,10 @@ Breaking API Changes
* finagle-core: `c.t.f.tracing.Trace.record(Record)` now accepts the record argument by
value (previously by name). ``PHAB_ID=D193300``

* finagle-mysql: `c.t.f.mysql.CanBeParameter`'s implicit conversions `timestampCanBeParameter`,
`sqlDateCanBeParameter`, and `javaDateCanBeParameter` have been consolidated into a single
implicit, `dateCanBeParameter`. ``PHAB_ID=D195351``

Runtime Behavior Changes
~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
@@ -1,7 +1,6 @@
package com.twitter.finagle.mysql

import com.twitter.finagle.mysql.transport.{MysqlBuf, MysqlBufWriter}
import java.util.TimeZone
import java.{lang => jl}

trait CanBeParameter[-A] {
Expand Down Expand Up @@ -271,39 +270,36 @@ object CanBeParameter {
}
}

implicit val timestampCanBeParameter: CanBeParameter[java.sql.Timestamp] = {
new CanBeParameter[java.sql.Timestamp] {
def sizeOf(param: java.sql.Timestamp): Int = 12
def typeCode(param: java.sql.Timestamp): Short = Type.Timestamp

def write(writer: MysqlBufWriter, param: java.sql.Timestamp): Unit = {
valueCanBeParameter.write(
writer,
TimestampValue(param)
)
/**
* Because java.sql.Date and java.sql.Timestamp extend java.util.Date and
* because CanBeParameter's type parameter is contravariant, having separate
* implicits for these types results in the one for the supertype being used
* when the one for the subtype should be used. To work around this we use
* just one implicit and pattern match within it.
*/
implicit val dateCanBeParameter: CanBeParameter[java.util.Date] = {
new CanBeParameter[java.util.Date] {
def sizeOf(param: java.util.Date): Int = param match {
case _: java.sql.Date => 5
case _: java.sql.Timestamp => 12
case _ => 12
}
}
}

implicit val sqlDateCanBeParameter: CanBeParameter[java.sql.Date] = {
new CanBeParameter[java.sql.Date] {
def sizeOf(param: java.sql.Date): Int = 5
def typeCode(param: java.sql.Date): Short = Type.Date
def write(writer: MysqlBufWriter, param: java.sql.Date): Unit = {
valueCanBeParameter.write(writer, DateValue(param))
def typeCode(param: java.util.Date): Short = param match {
case _: java.sql.Date => Type.Date
case _: java.sql.Timestamp => Type.Timestamp
case _ => Type.DateTime
}
}
}

implicit val javaDateCanBeParameter: CanBeParameter[java.util.Date] = {
new CanBeParameter[java.util.Date] {
def sizeOf(param: java.util.Date): Int = 12
def typeCode(param: java.util.Date): Short = Type.DateTime

def write(writer: MysqlBufWriter, param: java.util.Date): Unit = {
valueCanBeParameter.write(
def write(writer: MysqlBufWriter, param: java.util.Date): Unit = param match {
case sqlDate: java.sql.Date => valueCanBeParameter.write(writer, DateValue(sqlDate))
case sqlTimestamp: java.sql.Timestamp => valueCanBeParameter.write(
writer,
TimestampValue(sqlTimestamp)
)
case javaDate => valueCanBeParameter.write(
writer,
TimestampValue(new java.sql.Timestamp(param.getTime))
TimestampValue(new java.sql.Timestamp(javaDate.getTime))
)
}
}
Expand Down
Expand Up @@ -11,139 +11,165 @@ class ParameterTest extends FunSuite {
test("String") {
val value = "yep"
val param: Parameter = value
assert(param.evidence == CanBeParameter.stringCanBeParameter)
assert(param.value == value)
}

test("Boolean") {
val value = true
val param: Parameter = value
assert(param.evidence == CanBeParameter.booleanCanBeParameter)
assert(param.value == value)
}

test("java.lang.Boolean") {
val value = Boolean.box(true)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangBooleanCanBeParameter)
assert(param.value == value)
}

test("Byte") {
val value = 3.toByte
val param: Parameter = value
assert(param.evidence == CanBeParameter.byteCanBeParameter)
assert(param.value == value)
}

test("java.lang.Byte") {
val value = Byte.box(3)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangByteCanBeParameter)
assert(param.value == value)
}

test("Short") {
val value = 3.toShort
val param: Parameter = value
assert(param.evidence == CanBeParameter.shortCanBeParameter)
assert(param.value == value)
}

test("java.lang.Short") {
val value = Short.box(3)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangShortCanBeParameter)
assert(param.value == value)
}

test("Int") {
val value = 3
val param: Parameter = value
assert(param.evidence == CanBeParameter.intCanBeParameter)
assert(param.value == value)
}

test("java.lang.Int") {
val value = Int.box(3)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangIntCanBeParameter)
assert(param.value == value)
}

test("Long") {
val value = 3L
val param: Parameter = value
assert(param.evidence == CanBeParameter.longCanBeParameter)
assert(param.value == value)
}

test("java.lang.Long") {
val value = Long.box(3L)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangLongCanBeParameter)
assert(param.value == value)
}

test("BigInt") {
val value = BigInt(3)
val param: Parameter = value
assert(param.evidence == CanBeParameter.bigIntCanBeParameter)
assert(param.value == value)
}

test("Float") {
val value = 3.0F
val param: Parameter = value
assert(param.evidence == CanBeParameter.floatCanBeParameter)
assert(param.value == value)
}

test("java.lang.Float") {
val value = Float.box(3.0F)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangFloatCanBeParameter)
assert(param.value == value)
}


test("Double") {
val value = 3.0
val param: Parameter = value
assert(param.evidence == CanBeParameter.doubleCanBeParameter)
assert(param.value == value)
}

test("java.lang.Double") {
val value = Double.box(3.0)
val param: Parameter = value
assert(param.evidence == CanBeParameter.javaLangDoubleCanBeParameter)
assert(param.value == value)
}

test("BigDecimal") {
val value = BigDecimal(3.0)
val param: Parameter = value
assert(param.evidence == CanBeParameter.bigDecimalCanBeParameter)
assert(param.value == value)
}

test("Array[Byte]") {
val value = Array(3.toByte)
val param: Parameter = value
assert(param.evidence == CanBeParameter.byteArrayCanBeParameter)
assert(param.value == value)
}

test("java.sql.Timestamp") {
val value = new java.sql.Timestamp(3L)
val param: Parameter = value
assert(param.evidence == CanBeParameter.dateCanBeParameter)
assert(param.typeCode == Type.Timestamp)
assert(param.value == value)
}

test("java.sql.Date") {
val value = new java.sql.Date(3L)
val param: Parameter = value
assert(param.evidence == CanBeParameter.dateCanBeParameter)
assert(param.typeCode == Type.Date)
assert(param.value == value)
}

test("java.util.Date") {
val value = new java.util.Date(3L)
val param: Parameter = value
assert(param.evidence == CanBeParameter.dateCanBeParameter)
assert(param.typeCode == Type.DateTime)
assert(param.value == value)
}

test("com.twitter.util.Time") {
val value = com.twitter.util.Time.fromMilliseconds(3L)
val param: Parameter = value
assert(param.evidence == CanBeParameter.ctuTimeCanBeParameter)
assert(param.value == value)
}

test("null") {
val value: String = null
val param: Parameter = value
assert(param.evidence == CanBeParameter.nullCanBeParameter)
assert(param.value == null)
}

Expand Down

0 comments on commit b486772

Please sign in to comment.