Merge commit '1d6e4df1e3255d1757ee7e9dcc1e705651d9b122'

CONTRIBUTING.md

@@ -55,7 +55,7 @@ please scan this list and see which apply. It's okay if something is missed;
 the maintainers will help out during code review.
 
 1. Include [tests](CONTRIBUTING.md#testing).
-1. Update the [changelog](CHANGELOG.md) for new features, API breakages, runtime behavior changes,
+1. Update the [changelog](CHANGELOG.rst) for new features, API breakages, runtime behavior changes,
    deprecations, and bug fixes.
 1. All public APIs should have [Scaladoc][scaladoc].
 1. When adding a constructor to an existing class or arguments to an existing
@@ -91,8 +91,8 @@ compatibility can be kept by adding a forwarding method. Note that we
 avoid adding default arguments because this is not a compatible change
 for our Java users.  However, when the benefits outweigh the costs, we
 are willing to break APIs. The break should be noted in the Breaking
-API Changes section of the [changelog](CHANGELOG.md). Note that changes to
-non-public APIs will not be called out in the [changelog](CHANGELOG.md).
+API Changes section of the [changelog](CHANGELOG.rst). Note that changes to
+non-public APIs will not be called out in the [changelog](CHANGELOG.rst).
 
 ## Java
 

README.md

@@ -108,7 +108,7 @@ available on Maven Central. See the [First Steps](https://twitter.github.io/fina
 
 Releases are done on an approximately monthly schedule. While
 [semver](http://semver.org/) is not followed, the
-[changelogs](CHANGELOG.md) are detailed and include sections on public API
+[changelogs](CHANGELOG.rst) are detailed and include sections on public API
 breaks and changes in runtime behavior.
 
 ## Development version

build.sbt

@@ -4,7 +4,7 @@ import scoverage.ScoverageKeys
 concurrentRestrictions in Global += Tags.limit(Tags.Test, 1)
 
 // All Twitter library releases are date versioned as YY.MM.patch
-val releaseVersion = "18.8.0"
+val releaseVersion = "18.9.0"
 
 lazy val buildSettings = Seq(
   version := releaseVersion,
@@ -57,7 +57,7 @@ lazy val versions = new {
   val commonsLang = "2.6"
   val guava = "19.0"
   val guice = "4.0"
-  val jackson = "2.8.4"
+  val jackson = "2.9.6"
   val jodaConvert = "1.2"
   val jodaTime = "2.5"
   val junit = "4.12"

doc/src/sphinx/Changelog.rst

@@ -0,0 +1,7 @@
+Changelog
+=========
+
+Below is a list of changes for each Finatra release.
+
+
+.. include:: ../../../CHANGELOG.rst

doc/src/sphinx/index.rst

@@ -133,6 +133,7 @@ Useful Links
   Finagle Blog <https://finagle.github.io/blog/>
   presentations/index
   Forum <https://groups.google.com/forum/#!forum/finatra-users>
+  Changelog
 
 Contributing
 ------------
@@ -150,4 +151,4 @@ For support feel free to follow and/or tweet at the `@finatra <https://twitter.c
    :target: https://github.com/twitter/finatra#status
 .. |Maven Central| image:: https://maven-badges.herokuapp.com/maven-central/com.twitter/finatra-http_2.12/badge.svg
 .. |Gitter| image:: https://badges.gitter.im/Join%20Chat.svg 
-   :target: https://gitter.im/twitter/finatra
+   :target: https://gitter.im/twitter/finatra

http/src/main/scala/com/twitter/finatra/http/response/StreamingResponse.scala

@@ -175,7 +175,7 @@ class StreamingResponse[T, U] private (
     Future.value(response)
   }
 
-  private[this] def write(writer: Writer): Future[Unit] = {
+  private[this] def write(writer: Writer[Buf]): Future[Unit] = {
     streamTransformer(asyncStream).foreachF {
       case (item, buf) =>
         writer.write(buf).respond(onWrite(item, buf))

http/src/test/scala/com/twitter/finatra/http/tests/response/StreamingResponseTest.scala

@@ -28,7 +28,7 @@ class StreamingResponseTest extends Test {
     await(streamingResponse.toFutureFinagleResponse)
   }
 
-  private def burnLoop(reader: Reader): Future[Unit] = reader.read(Int.MaxValue).flatMap {
+  private def burnLoop(reader: Reader[Buf]): Future[Unit] = reader.read(Int.MaxValue).flatMap {
     case Some(_) => burnLoop(reader)
     case None => Future.Unit
   }

httpclient/src/test/scala/com/twitter/finatra/httpclient/HttpClientIntegrationTest.scala

@@ -62,7 +62,7 @@ class HttpClientIntegrationTest extends IntegrationTest {
     val e = intercept[HttpClientException] {
       Await.result(httpClient.executeJson[Int](request))
     }
-    assert(e.getMessage.contains("com.fasterxml.jackson.databind.JsonMappingException"))
+    assert(e.getMessage.contains("com.fasterxml.jackson.databind.exc.MismatchedInputException"))
   }
 
   test("get") {

inject/inject-core/src/test/scala/com/twitter/inject/TestMixin.scala

@@ -78,8 +78,7 @@ trait TestMixin
 
   protected def assertFutureValue[A](result: Future[A], expected: A): Unit = {
     val resultVal = Await.result(result, 5.seconds)
-    val expectedVal = Await.result(Future.value(expected), 5.seconds)
-    resultVal should equal(expectedVal)
+    resultVal should equal(expected)
   }
 
   protected def assertFailedFuture[T <: Throwable: Manifest](result: Future[_]): T = {

inject/inject-core/src/test/scala/com/twitter/inject/WhenReadyMixin.scala

@@ -0,0 +1,42 @@
+package com.twitter.inject
+
+import com.twitter.util.{Duration, Future, Return, Throw}
+import org.scalatest.concurrent.PatienceConfiguration.{Interval, Timeout}
+import org.scalatest.concurrent.ScalaFutures.{FutureConcept, PatienceConfig}
+import org.scalatest.time.{Millis, Seconds, Span}
+import scala.language.implicitConversions
+
+/**
+ * ScalaTest provides a whenReady function that handles async computations. whenReady takes
+ * a FutureConcept and an implicit PatienceConfig as parameters. Extend this trait in your
+ * test to get an implicit conversion from a Twitter Future to a FutureConcept. It also provides
+ * a default PatienceConfig which you can override in your test to suit your needs.
+ *
+ * @see <a href="http://doc.scalatest.org/3.0.0/index.html#org.scalatest.concurrent.ScalaFutures">ScalaFutures</a>
+ */
+trait WhenReadyMixin {
+  protected implicit val patienceConfig =
+    PatienceConfig(timeout = Span(5, Seconds), interval = Span(20, Millis))
+
+  final protected implicit def convertTwitterFuture[T](twitterFuture: Future[T]): FutureConcept[T] =
+    new FutureConcept[T] {
+      def eitherValue: Option[Either[Throwable, T]] =
+        twitterFuture.poll.map {
+          case Return(result) => Right(result)
+          case Throw(e) => Left(e)
+        }
+      // As per the ScalaTest docs if the underlying future does not support these they
+      // must return false.
+      def isExpired: Boolean = false
+      def isCanceled: Boolean = false
+    }
+
+  final implicit def twitterDurationToScalaTestTimeout(duration: Duration): Timeout = {
+    Timeout(Span(duration.inMillis, Millis))
+  }
+
+  final implicit def twitterDurationToScalaTestInterval(duration: Duration): Interval = {
+    Interval(Span(duration.inMillis, Millis))
+  }
+
+}

inject/inject-core/src/test/scala/com/twitter/inject/tests/WhenReadyMixinTest.scala

@@ -0,0 +1,30 @@
+package com.twitter.inject.tests
+
+import com.twitter.conversions.time._
+import com.twitter.inject.{Test, WhenReadyMixin}
+import com.twitter.util.Future
+import org.scalatest.concurrent.ScalaFutures._
+
+class WhenReadyMixinTest extends Test with WhenReadyMixin {
+  test("should be able to use ScalaTest's whenReady with a Twitter Future") {
+    whenReady(Future.apply("Test it")) { result =>
+      result should be("Test it")
+    }
+  }
+
+  test(
+    "should be able to use a twitter Duration for the timeout"
+  ) {
+    whenReady(Future.apply("Test it"), timeout = 5.seconds) { result =>
+      result should be("Test it")
+    }
+  }
+
+  test(
+    "should be able to use a twitter Duration for the interval"
+  ) {
+    whenReady(Future.apply("Test it"), interval = 10.milliseconds) { result =>
+      result should be("Test it")
+    }
+  }
+}

inject/inject-server/src/test/scala/com/twitter/inject/server/tests/StartupIntegrationTest.scala

@@ -2,13 +2,14 @@ package com.twitter.inject.server.tests
 
 import com.google.inject.AbstractModule
 import com.twitter.app.CloseException
+import com.twitter.conversions.time._
 import com.twitter.finagle.http.Status
 import com.twitter.inject.app.App
 import com.twitter.inject.server.{EmbeddedTwitterServer, Ports, TwitterServer}
 import com.twitter.inject.{Injector, Test, TwitterModule}
 import com.twitter.server.Lifecycle.Warmup
 import com.twitter.server.{TwitterServer => BaseTwitterServer}
-import com.twitter.util.{Await, Closable, Future}
+import com.twitter.util.{Await, Closable, Duration, Future}
 import com.twitter.util.registry.GlobalRegistry
 import scala.util.parsing.json.JSON
 
@@ -206,12 +207,16 @@ class FailFastServer extends TwitterServer {
 }
 
 class SimpleTwitterServer extends TwitterServer {
+  /* ensure enough time to close resources */
+  override val defaultCloseGracePeriod: Duration = 15.seconds
   override val modules = Seq()
 }
 
 class SimpleHttpTwitterServer extends TwitterServer {}
 
 class ServerWithTwitterModuleInstall extends TwitterServer {
+  /* ensure enough time to close resources */
+  override val defaultCloseGracePeriod: Duration = 15.seconds
   override val modules = Seq(new TwitterModule {
     override def configure(): Unit = {
       install(new TwitterModule {})
@@ -220,6 +225,8 @@ class ServerWithTwitterModuleInstall extends TwitterServer {
 }
 
 class ServerWithModuleInstall extends TwitterServer {
+  /* ensure enough time to close resources */
+  override val defaultCloseGracePeriod: Duration = 15.seconds
   override val modules = Seq(new TwitterModule {
     override def configure(): Unit = {
       install(new AbstractModule {
@@ -249,6 +256,8 @@ class ServerPremainException extends TwitterServer {
 class StartupTestException(msg: String) extends Exception(msg)
 
 class ExtendedBaseTwitterServer extends BaseTwitterServer {
+  /* ensure enough time to close resources */
+  override val defaultCloseGracePeriod: Duration = 15.seconds
   def main(): Unit = {
     Await.ready(adminHttpServer)
   }

jackson/src/main/scala/com/twitter/finatra/json/internal/caseclass/jackson/ImmutableAnnotations.scala

@@ -16,4 +16,19 @@ private[json] case class ImmutableAnnotations(annotations: Seq[Annotation]) exte
   override def size(): Int = {
     annotationsMap.size
   }
+
+  override def has(clazz: Class[_]): Boolean = {
+    annotationsMap.get(clazz.asInstanceOf[Class[_ <: Annotation]]) != null
+  }
+
+  override def hasOneOf(clazzes: Array[Class[_ <: Annotation]]): Boolean = {
+    var i = 0
+    while (i < clazzes.length) {
+      if (has(clazzes(i))) {
+        return true
+      }
+      i += 1
+    }
+    false
+  }
 }

jackson/src/main/scala/com/twitter/finatra/json/internal/caseclass/utils/FieldInjection.scala

@@ -50,10 +50,17 @@ private[json] class FieldInjection(
     new ValueInjector(
       new PropertyName(name),
       javaType,
-      ImmutableAnnotations(annotations),
       /* mutator = */ null,
       /* valueId = */ null
-    )
+    ) {
+      // ValueInjector no longer supports passing contextAnnotations as an
+      // argument as of jackson 2.9.x
+      // https://github.com/FasterXML/jackson-databind/commit/76381c528c9b75265e8b93bf6bb4532e4aa8e957#diff-dbcd29e987f27d95f964a674963a9066R24
+      private[this] val contextAnnotations = ImmutableAnnotations(annotations)
+      override def getContextAnnotation[A <: Annotation](acls: Class[A]): A = {
+        contextAnnotations.get[A](acls)
+      }
+    }
   }
 
   /* Public */

jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonStreamParser.scala

@@ -8,7 +8,7 @@ import javax.inject.{Inject, Singleton}
 @Singleton
 private[finatra] class JsonStreamParser @Inject()(mapper: FinatraObjectMapper) {
 
-  def parseArray[T: Manifest](reader: Reader): AsyncStream[T] = {
+  def parseArray[T: Manifest](reader: Reader[Buf]): AsyncStream[T] = {
     val bufs = AsyncStream.fromReader(reader)
     parseArray[T](bufs)
   }

jackson/src/main/scala/com/twitter/finatra/json/modules/FinatraJacksonModule.scala

@@ -105,7 +105,11 @@ class FinatraJacksonModule extends TwitterModule {
 
   protected def defaultMapperConfiguration(mapper: ObjectMapper): Unit = {
     /* Serialization Config */
-    mapper.setSerializationInclusion(serializationInclusion)
+    mapper.setDefaultPropertyInclusion(
+      JsonInclude.Value.construct(serializationInclusion, serializationInclusion))
+    mapper.configOverride(classOf[Option[_]])
+      .setIncludeAsProperty(
+        JsonInclude.Value.construct(serializationInclusion, Include.ALWAYS))
     for ((feature, state) <- serializationConfig) {
       mapper.configure(feature, state)
     }

jackson/src/test/scala/com/twitter/finatra/json/tests/FinatraObjectMapperTest.scala

@@ -430,13 +430,13 @@ class FinatraObjectMapperTest extends Test with Logging {
   }
 
   test("Injection when using FinatraObjectMapper.create#Inject not found field") {
-    intercept[JsonInjectionNotSupportedException] {
+    intercept[CaseClassMappingException] {
       parse[ClassWithFooClassInject]("""{}""")
     }
   }
 
   test("Injection when using FinatraObjectMapper.create#Inject request field") {
-    intercept[JsonInjectionNotSupportedException] {
+    intercept[CaseClassMappingException] {
       parse[ClassWithQueryParamDateTimeInject]("""{}""")
     }
   }
@@ -589,25 +589,16 @@ class FinatraObjectMapperTest extends Test with Logging {
     }
   }
 
-  // Using Arrays of non-wrapped types that extend AnyVal (Integer, Long, Boolean, etc) in case
-  // classes is not recommended and bypasses both Jackson's and Finatra's null checking as
-  // demonstrated here:
-  test(
-    "Arrays of types extending AnyVal (inconsistent corner case)#" +
-      "Deserialize null -> 0 when CaseClassWithArrayLong with null array element"
-  ) {
-    val obj = parse[CaseClassWithArrayLong]("""{"array": [null]}""")
-    val expected = CaseClassWithArrayLong(array = Array(0L))
-    obj.array should equal(expected.array)
+  test("fail when CaseClassWithArrayLong with null field in object") {
+    intercept[CaseClassMappingException] {
+      parse[CaseClassWithArrayLong]("""{"array": [null]}""")
+    }
   }
 
-  test(
-    "Arrays of types extending AnyVal (inconsistent corner case)#" +
-      "Deserialize null -> false when CaseClassWithArrayBoolean with null array element"
-  ) {
-    val obj = parse[CaseClassWithArrayBoolean]("""{"array": [null]}""")
-    val expected = CaseClassWithArrayBoolean(array = Array(false))
-    obj.array should equal(expected.array)
+  test("fail when CaseClassWithArrayBoolean with null field in object") {
+    intercept[CaseClassMappingException] {
+      parse[CaseClassWithArrayBoolean]("""{"array": [null]}""")
+    }
   }
 
   // ========================================================

project/plugins.sbt

@@ -3,7 +3,7 @@ resolvers ++= Seq(
   Resolver.sonatypeRepo("snapshots")
 )
 
-val releaseVersion = "18.8.0"
+val releaseVersion = "18.9.0"
 
 addSbtPlugin("com.twitter" % "scrooge-sbt-plugin" % releaseVersion)