finatra: Use less joda-time inside of finatra

Problem

Finatra encourages users to use joda time sometimes, and twitter util
time other times.  It's time to spend less time thinking about times.

Solution

Further consolidating our times to twitter util time.  This time we've updated
`RetryPolicyUtils`, `FilteredThriftClientModule`, and `ThriftClientFilterChain`
to use twitter util time stuff instead of joda time stuff.

Result

finatra uses the util time more of time than it did in previous times.

JIRA Issues: CSL-5802

Differential Revision: https://phabricator.twitter.biz/D313153

CHANGELOG.rst

@@ -29,6 +29,11 @@ Added
 Changed
 ~~~~~~~
 
+* finatra-inject: Update `com.twitter.inject.utils.RetryPolicyUtils`,
+  `com.twitter.inject.thrift.modules.FilteredThriftClientModule`,  and
+  `com.twitter.inject.thrift.filters.ThriftClientFilterChain` methods to take
+  `com.twitter.util.Duration` instead of `org.joda.time.Duration`. ``PHAB_ID=D313153``
+
 * finatra: Fix Commons FileUpload vulnerability. Update `org.apache.commons-fileupload` from version
   1.3.1 to version 1.4. This closes #PR-497. ``PHAB_ID=D310470``
 

examples/benchmark-server/src/main/scala/BUILD

@@ -3,7 +3,6 @@ scala_library(
     compiler_option_sets = {"fatal_warnings"},
     dependencies = [
         "3rdparty/jvm/com/fasterxml/jackson/core:jackson-databind",
-        "3rdparty/jvm/com/fasterxml/jackson/datatype:jackson-datatype-joda",
         "3rdparty/jvm/com/fasterxml/jackson/module:jackson-module-scala",
         "3rdparty/jvm/com/google/inject:guice",
         "3rdparty/jvm/commons-lang",

examples/twitter-clone/src/main/scala/BUILD

@@ -4,7 +4,6 @@ scala_library(
     dependencies = [
         "3rdparty/jvm/com/fasterxml/jackson/core:jackson-core",
         "3rdparty/jvm/com/fasterxml/jackson/core:jackson-databind",
-        "3rdparty/jvm/com/fasterxml/jackson/datatype:jackson-datatype-joda",
         "3rdparty/jvm/com/fasterxml/jackson/module:jackson-module-scala",
         "3rdparty/jvm/com/github/nscala_time",
         "3rdparty/jvm/com/google/inject:guice",

examples/twitter-clone/src/main/scala/finatra/quickstart/modules/FirebaseHttpClientModule.scala

@@ -2,7 +2,7 @@ package finatra.quickstart.modules
 
 import com.twitter.finatra.httpclient.modules.HttpClientModule
 import com.twitter.finatra.http.response.ResponseUtils._
-import com.twitter.inject.conversions.time._
+import com.twitter.conversions.DurationOps._
 import com.twitter.inject.utils.RetryPolicyUtils._
 
 object FirebaseHttpClientModule extends HttpClientModule {

http/src/main/scala/BUILD

@@ -11,7 +11,6 @@ scala_library(
         "//:scalap",
         "3rdparty/jvm/com/fasterxml/jackson/core:jackson-core",
         "3rdparty/jvm/com/fasterxml/jackson/core:jackson-databind",
-        "3rdparty/jvm/com/fasterxml/jackson/datatype:jackson-datatype-joda",
         "3rdparty/jvm/com/fasterxml/jackson/module:jackson-module-scala",
         "3rdparty/jvm/com/github/spullara/mustache/java",
         "3rdparty/jvm/com/google/inject:guice",

http/src/test/scala/com/twitter/finatra/http/tests/HttpRetryUtilsTest.scala

@@ -3,7 +3,7 @@ package com.twitter.finatra.http.tests
 import com.twitter.finagle.http.{Response, Status}
 import com.twitter.finatra.http.response.ResponseUtils
 import com.twitter.inject.Test
-import com.twitter.inject.conversions.time._
+import com.twitter.conversions.DurationOps._
 import com.twitter.inject.utils.{RetryPolicyUtils, RetryUtils}
 import com.twitter.util.{Await, Future}
 

http/src/test/scala/com/twitter/finatra/http/tests/integration/doeverything/main/services/ComplexService.scala

@@ -3,7 +3,7 @@ package com.twitter.finatra.http.tests.integration.doeverything.main.services
 import com.google.inject.assistedinject.Assisted
 import com.twitter.inject.annotations.Flag
 import javax.inject.{Inject, Named}
-import org.joda.time.Duration
+import com.twitter.util.Duration
 
 class ComplexService @Inject()(
   exampleService: DoEverythingService,
@@ -16,6 +16,6 @@ class ComplexService @Inject()(
 ) {
 
   def execute = {
-    exampleService.doit + " " + name + " " + duration1.getMillis
+    exampleService.doit + " " + name + " " + duration1.inMillis
   }
 }

http/src/test/scala/com/twitter/finatra/http/tests/integration/doeverything/test/DoEverythingServerFeatureTest.scala

@@ -1848,14 +1848,6 @@ class DoEverythingServerFeatureTest extends FeatureTest with Mockito {
     )
   }
 
-  test("Bad request for deserialization of an invalid joda LocalDate") {
-    server.httpPost(
-      "/localDateRequest",
-      postBody = """{"date" : "2016-11-32"}""",
-      andExpect = BadRequest
-    )
-  }
-
   test("JsonPatch") {
     val request = RequestBuilder
       .patch("/jsonPatch")

http/src/test/scala/com/twitter/finatra/http/tests/integration/requestscope/RequestScopeFeatureTest.scala

@@ -14,7 +14,7 @@ import com.twitter.inject.requestscope.{
   RequestScopeBinding
 }
 import com.twitter.inject.server.FeatureTest
-import com.twitter.inject.conversions.time._
+import com.twitter.conversions.DurationOps._
 import com.twitter.inject.utils.RetryPolicyUtils.constantRetry
 import com.twitter.inject.utils.RetryUtils.retryFuture
 import com.twitter.util.{Await, Future, Return}
@@ -26,7 +26,7 @@ class RequestScopeFeatureTest extends FeatureTest {
 
   override val server = new EmbeddedHttpServer(new PooledServer)
 
-  def await[T](f: Future[T]): T = Await.result(f, 5.seconds.toTwitterDuration)
+  def await[T](f: Future[T]): T = Await.result(f, 5.seconds)
 
   test("request scope propagates to multiple future pools") {
     for (i <- 1 to 50) {

inject-thrift-client-http-mapper/src/test/scala/com/twitter/finatra/multiserver/Add1HttpServer/AdderThriftClientModule.scala

@@ -2,6 +2,7 @@ package com.twitter.finatra.multiserver.Add1HttpServer
 
 import com.twitter.adder.thriftscala.Adder
 import com.twitter.adder.thriftscala.Adder._
+import com.twitter.conversions.DurationOps._
 import com.twitter.finagle.Filter
 import com.twitter.inject.thrift.filters.ThriftClientFilterBuilder
 import com.twitter.inject.thrift.modules.FilteredThriftClientModule

inject/inject-thrift-client/src/main/scala/com/twitter/inject/thrift/filters/ThriftClientFilterChain.scala

@@ -12,7 +12,6 @@ import com.twitter.finagle.service.RetryPolicy._
 import com.twitter.finagle.service._
 import com.twitter.finagle.stats.StatsReceiver
 import com.twitter.finagle.util.DefaultTimer
-import com.twitter.inject.conversions.duration._
 import com.twitter.inject.thrift.AndThenService
 import com.twitter.inject.thrift.internal.filters.{
   IncrementCounterFilter,
@@ -24,9 +23,8 @@ import com.twitter.inject.utils.ExceptionUtils._
 import com.twitter.inject.{Injector, Logging}
 import com.twitter.scrooge.{ThriftMethod, ThriftStruct}
 import com.twitter.util.tunable.Tunable
-import com.twitter.util.{Try, Duration => TwitterDuration}
+import com.twitter.util.{Try, Duration}
 import java.util.concurrent.TimeUnit
-import org.joda.time.Duration
 
 /**
  * A [[com.twitter.finagle.Filter]] chain builder which provides helper functions for installing and
@@ -213,7 +211,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
 
     withRetryPolicy(
       constantRetryPolicy(
-        delay = start.multipliedBy(retryMultiplier),
+        delay = start * retryMultiplier,
         retries = retries,
         shouldRetry = chooseShouldRetryFunction(shouldRetry, shouldRetryResponse)
       )
@@ -248,7 +246,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
 
     withRetryPolicy(
       exponentialRetryPolicy(
-        start = start.multipliedBy(retryMultiplier),
+        start = start * retryMultiplier,
         multiplier = multiplier,
         numRetries = retries,
         shouldRetry = chooseShouldRetryFunction(shouldRetry, shouldRetryResponse)
@@ -267,7 +265,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
    */
   def withRetryPolicy(
     retryPolicy: RetryPolicy[(Req, Try[Rep])],
-    retryMsg: ((Req, Try[Rep]), TwitterDuration) => String = defaultRetryMsg
+    retryMsg: ((Req, Try[Rep]), Duration) => String = defaultRetryMsg
   ): ThriftClientFilterChain[Req, Rep] = {
 
     retryFilter = new IncrementCounterFilter[Req, Rep](invocationsCounter)
@@ -291,7 +289,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
    * @return [[ThriftClientFilterChain]]
    */
   def withTimeout(duration: Duration): ThriftClientFilterChain[Req, Rep] = {
-    val twitterTimeout = duration.toTwitterDuration * timeoutMultiplier
+    val twitterTimeout = duration * timeoutMultiplier
 
     timeoutFilter = new TimeoutFilter[Req, Rep](
       twitterTimeout,
@@ -310,11 +308,11 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
    * @return [[ThriftClientFilterChain]]
    */
   def withTimeout(duration: Tunable[Duration]): ThriftClientFilterChain[Req, Rep] = {
-    val exceptionFn = (duration: TwitterDuration) =>
+    val exceptionFn = (duration: Duration) =>
       new GlobalRequestTimeoutException(duration * timeoutMultiplier)
 
     timeoutFilter = new TimeoutFilter[Req, Rep](
-      duration.map(d => d.toTwitterDuration),
+      duration,
       exceptionFn,
       DefaultTimer
     )
@@ -330,7 +328,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
    * @return [[ThriftClientFilterChain]]
    */
   def withRequestTimeout(duration: Duration): ThriftClientFilterChain[Req, Rep] = {
-    val twitterTimeout = duration.toTwitterDuration * timeoutMultiplier
+    val twitterTimeout = duration * timeoutMultiplier
 
     requestTimeoutFilter = new TimeoutFilter[Req, Rep](
       twitterTimeout,
@@ -349,11 +347,11 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
    * @return [[ThriftClientFilterChain]]
    */
   def withRequestTimeout(duration: Tunable[Duration]): ThriftClientFilterChain[Req, Rep] = {
-    val exceptionFn = (duration: TwitterDuration) =>
+    val exceptionFn = (duration: Duration) =>
       new IndividualRequestTimeoutException(duration * timeoutMultiplier)
 
     requestTimeoutFilter = new TimeoutFilter[Req, Rep](
-      duration.map(d => d.toTwitterDuration),
+      duration,
       exceptionFn,
       DefaultTimer
     )
@@ -456,7 +454,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
 
   /* Protected */
 
-  protected def defaultRetryMsg(requestAndResponse: (Req, Try[Rep]), duration: TwitterDuration) = {
+  protected def defaultRetryMsg(requestAndResponse: (Req, Try[Rep]), duration: Duration) = {
     val (_, response) = requestAndResponse
     s"Retrying ${ThriftMethodUtils.prettyStr(method)} = ${toDetailedExceptionMessage(response)} in ${duration.inMillis} ms"
   }
@@ -499,7 +497,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
 
   private def addRetryLogging(
     retryPolicy: RetryPolicy[(Req, Try[Rep])],
-    retryMsg: ((Req, Try[Rep]), TwitterDuration) => String
+    retryMsg: ((Req, Try[Rep]), Duration) => String
   ): RetryPolicy[(Req, Try[Rep])] = {
 
     new RetryPolicy[(Req, Try[Rep])] {
@@ -531,7 +529,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
   ): RetryPolicy[T] = {
 
     backoff(
-      decorrelatedJittered(start.toTwitterDuration, start.toTwitterDuration * multiplier) take numRetries
+      decorrelatedJittered(start, start * multiplier) take numRetries
     )(shouldRetry)
   }
 
@@ -541,7 +539,7 @@ class ThriftClientFilterChain[Req <: ThriftStruct, Rep](
     retries: Int
   ): RetryPolicy[T] = {
 
-    backoff(constant(delay.toTwitterDuration) take retries)(shouldRetry)
+    backoff(constant(delay) take retries)(shouldRetry)
   }
 
   private[thrift] def toFilter: Filter[Req, Rep, Req, Rep] = {

inject/inject-thrift-client/src/main/scala/com/twitter/inject/thrift/modules/FilteredThriftClientModule.scala

@@ -1,28 +1,23 @@
 package com.twitter.inject.thrift.modules
 
-import com.github.nscala_time.time
-import com.github.nscala_time.time.DurationBuilder
 import com.google.inject.Provides
 import com.twitter.finagle._
 import com.twitter.finagle.service.Retries.Budget
 import com.twitter.finagle.stats.StatsReceiver
 import com.twitter.finagle.thrift.service.Filterable
 import com.twitter.finagle.thrift.{ClientId, MethodIfaceBuilder, ServiceIfaceBuilder, ThriftService}
 import com.twitter.inject.annotations.Flag
-import com.twitter.inject.conversions.duration._
 import com.twitter.inject.exceptions.PossiblyRetryable
 import com.twitter.inject.thrift.filters.ThriftClientFilterBuilder
 import com.twitter.inject.thrift.modules.FilteredThriftClientModule.MaxDuration
 import com.twitter.inject.thrift.{AndThenService, NonFiltered}
 import com.twitter.inject.{Injector, TwitterModule}
-import com.twitter.util.{Monitor, NullMonitor, Try, Duration => TwitterDuration}
+import com.twitter.util.{Monitor, NullMonitor, Try, Duration}
 import javax.inject.Singleton
-import org.joda.time.Duration
-import scala.language.implicitConversions
 import scala.reflect.ClassTag
 
 object FilteredThriftClientModule {
-  val MaxDuration = Duration.millis(Long.MaxValue)
+  val MaxDuration = Duration.Top
 }
 
 /**
@@ -65,8 +60,7 @@ abstract class FilteredThriftClientModule[
 ](
   implicit serviceBuilder: ServiceIfaceBuilder[ServiceIface],
   methodBuilder: MethodIfaceBuilder[ServiceIface, FutureIface]
-) extends TwitterModule
-    with time.Implicits {
+) extends TwitterModule {
 
   override val frameworkModules = Seq(AndThenServiceModule, FilteredThriftClientFlagsModule)
 
@@ -102,7 +96,7 @@ abstract class FilteredThriftClientModule[
    *
    * @see [[com.twitter.finagle.param.ClientSessionParams#acquisitionTimeout]]
    * @see [[https://twitter.github.io/finagle/guide/Clients.html#timeouts-expiration]]
-   * @return an [[org.joda.time.Duration]] which represents the acquisition timeout
+   * @return an [[com.twitter.util.Duration]] which represents the acquisition timeout
    */
   protected def sessionAcquisitionTimeout: Duration = MaxDuration
 
@@ -237,7 +231,7 @@ abstract class FilteredThriftClientModule[
     clientId: ClientId,
     statsReceiver: StatsReceiver
   ): ServiceIface = {
-    val acquisitionTimeout = sessionAcquisitionTimeout.toTwitterDuration * timeoutMultiplier
+    val acquisitionTimeout = sessionAcquisitionTimeout * timeoutMultiplier
     val clientStatsReceiver = statsReceiver.scope("clnt")
 
     val thriftClient =
@@ -273,10 +267,4 @@ abstract class FilteredThriftClientModule[
 
   protected val PossiblyRetryableExceptions: PartialFunction[Try[_], Boolean] =
     PossiblyRetryable.PossiblyRetryableExceptions
-
-  /* Common Implicits */
-
-  implicit def toTwitterDuration(duration: DurationBuilder): TwitterDuration = {
-    TwitterDuration.fromMilliseconds(duration.toDuration.getMillis)
-  }
 }

inject/inject-thrift-client/src/test/scala/com/twitter/inject/thrift/integration/filtered/GreeterFilteredThriftClientModule.scala

@@ -1,29 +1,29 @@
 package com.twitter.inject.thrift.integration.filtered
 
+import com.twitter.conversions.DurationOps._
 import com.twitter.greeter.thriftscala.Greeter.{Bye, Hello, Hi}
 import com.twitter.greeter.thriftscala.{Greeter, InvalidOperation}
 import com.twitter.inject.thrift.filters.ThriftClientFilterBuilder
 import com.twitter.inject.thrift.integration.filters.MethodLoggingTypeAgnosticFilter
 import com.twitter.inject.thrift.modules.FilteredThriftClientModule
 import com.twitter.util.tunable.Tunable
-import com.twitter.util.{Future, Return, Throw}
-import org.joda.time.Duration
+import com.twitter.util.{Duration, Future, Return, Throw}
 import scala.util.control.NonFatal
 
 object GreeterFilteredThriftClientModule
   extends FilteredThriftClientModule[Greeter[Future], Greeter.ServiceIface] {
 
   override val label = "greeter-thrift-client"
   override val dest = "flag!greeter-thrift-service"
-  override val sessionAcquisitionTimeout = 1.minute.toDuration
+  override val sessionAcquisitionTimeout = 1.minute
 
   override def filterServiceIface(
     serviceIface: Greeter.ServiceIface,
     filter: ThriftClientFilterBuilder
   ) = {
 
     val timeoutTunable = Tunable.emptyMutable[Duration]("id")
-    timeoutTunable.set(new Duration(60000)) // 1.minute
+    timeoutTunable.set(1.minute) // 1.minute
 
     serviceIface.copy(
       hi = filter

inject/inject-utils/src/main/scala/com/twitter/inject/utils/RetryPolicyUtils.scala

@@ -3,9 +3,7 @@ package com.twitter.inject.utils
 import com.twitter.finagle.service.Backoff._
 import com.twitter.finagle.service.RetryPolicy
 import com.twitter.finagle.service.RetryPolicy._
-import com.twitter.inject.conversions.time._
-import com.twitter.util.{Throw, Try}
-import org.joda.time.Duration
+import com.twitter.util.{Duration, Throw, Try}
 import scala.util.control.NonFatal
 
 object RetryPolicyUtils {
@@ -21,7 +19,7 @@ object RetryPolicyUtils {
     shouldRetry: PartialFunction[Try[T], Boolean]
   ): RetryPolicy[Try[T]] = {
 
-    backoff(exponential(start.toTwitterDuration, multiplier) take numRetries)(shouldRetry)
+    backoff(exponential(start, multiplier) take numRetries)(shouldRetry)
   }
 
   def constantRetry[T](
@@ -30,6 +28,6 @@ object RetryPolicyUtils {
     shouldRetry: PartialFunction[Try[T], Boolean]
   ): RetryPolicy[Try[T]] = {
 
-    backoff(constant(start.toTwitterDuration) take numRetries)(shouldRetry)
+    backoff(constant(start) take numRetries)(shouldRetry)
   }
 }

inject/inject-utils/src/test/scala/com/twitter/inject/tests/utils/RetryUtilsTest.scala

@@ -1,7 +1,7 @@
 package com.twitter.inject.tests.utils
 
+import com.twitter.conversions.DurationOps._
 import com.twitter.inject.Test
-import com.twitter.inject.conversions.time._
 import com.twitter.inject.utils.{RetryPolicyUtils, RetryUtils}
 import com.twitter.util.{Await, Future}
 

thrift/src/main/scala/BUILD

@@ -13,7 +13,6 @@ scala_library(
         "3rdparty/jvm/com/google/inject/extensions:guice-multibindings",
         "3rdparty/jvm/commons-lang",
         "3rdparty/jvm/javax/inject:javax.inject",
-        "3rdparty/jvm/joda-time",
         "3rdparty/jvm/net/codingwell:scala-guice",
         "3rdparty/jvm/org/apache/thrift:libthrift",
         "3rdparty/jvm/org/slf4j:jcl-over-slf4j",

utils/src/main/scala/BUILD

@@ -16,7 +16,6 @@ scala_library(
         "3rdparty/jvm/commons-io",
         "3rdparty/jvm/commons-lang",
         "3rdparty/jvm/javax/inject:javax.inject",
-        "3rdparty/jvm/joda-time",
         "3rdparty/jvm/net/codingwell:scala-guice",
         "3rdparty/jvm/org/slf4j:slf4j-api",
         "finagle/finagle-core/src/main/scala",