Merge commit '5bf268d03d1bc04134a44400ee419a502a14dfc6'

CHANGELOG.rst

@@ -7,14 +7,134 @@ Note that ``RB_ID=#`` and ``PHAB_ID=#`` correspond to associated message in comm
 Unreleased
 ----------
 
-19.1.0
+19.2.0
 -------
 
+Added
+~~~~~
+* finatra-kafka: Expose timeout duration in FinagleKafkaConsumerBuilder dest(). ``PHAB_ID=D269701``
+
+* finatra-kafka-streams: Expose all existing RocksDb configurations. See
+  `c.t.f.k.config.FinatraRocksDBConfig` for details on flag names,
+  descriptions and default values. ``PHAB_ID=D272068``
+
+* finatra-kafka-streams: Added two RocksDB flags related to block cache tuning,
+  `cache_index_and_filter_blocks` and `pin_l0_filter_and_index_blocks_in_cache`.
+  ``PHAB_ID=D269516``
+
+* finatra-kafka: Adding an implicit implementation of
+  `c.t.app.Flaggable[c.t.finatra.kafka.domain.SeekStrategy]`
+  and `c.t.app.Flaggable[org.apache.kafka.clients.consumer.OffsetResetStrategy]`.
+  ``PHAB_ID=D271098``
+
+* finatra-http: Added support to serve `c.t.io.Reader` as a streaming response in
+  `c.t.finatra.http.internal.marshalling.CallbackConverter`. ``PHAB_ID=D266863``
+
+* finatra-kafka: Expose endOffsets() in FinagleKafkaConsumer. ``PHAB_ID=D263573``
+
+* finatra-kafka-streams: Adding missing ScalaDocs. Adding metric for elapsed state
+  restore time. RocksDB configuration now contains a flag for adjusting the number
+  of cache shard bits, `rocksdb.block.cache.shard.bits`. ``PHAB_ID=D255771``
+
+* finatra-jackson: Added @Pattern annotation to support finatra/jackson for regex pattern
+  validation on string values. ``PHAB_ID=D259719``
+
+Changed
+~~~~~~~
+
+* finatra-kafka-streams: Refactor package names. All classes moved from
+  com.twitter.finatra.streams to com.twitter.finatra.kafkastreams. ``PHAB_ID=D268027``
+
+* finatra-kafka-streams: Delete deprecated and unused classes. ``PHAB_ID=D267921``
+
+* finatra-kafka-streams: `c.t.finatra.streams.transformer.domain.Time` is now the canonical
+   representation of time for watermarks and timers. `RichLong` implicit from
+   `com.twitter.finatra.streams.converters.time` has been renamed to `RichFinatraKafkaStreamsLong`.
+   ``PHAB_ID=D255736``
+
+* finatra-jackson: Fix `CaseClassField` annotation reflection for Scala 2.12. ``PHAB_ID=D264423``
+
+* finatra-kafka-streams: Combine FinatraTransformer with FinatraTransformerV2. ``PHAB_ID=D254411``
+
+* finatra-thrift: The return type of `ReqRepDarkTrafficFilterModule#newFilter` has been changed from
+  `DarkTrafficFilter[MethodIface]` to `Filter.TypeAgnostic`. ``PHAB_ID=D261868``
+
+* finatra-kafka: Add lookupBootstrapServers function that takes timeout as a parameter.
+  ``PHAB_ID=D256997``
+
+* finatra-thrift: If a Controller is not configured with exactly one endpoint
+  per method, it will throw an AssertionError instead of logging an error message.
+  An attempt to use non-legacy functionality with a legacy Controller will throw
+  an AssertionError. ``PHAB_ID=D260230``
+
+* finatra-kafka: Add flags for controlling rocksdb internal LOG file growth.
+  - `rocksdb.log.info.level` Allows the setting of rocks log levels
+    `DEBUG_LEVEL`, `INFO_LEVEL`, `WARN_LEVEL`, `ERROR_LEVEL`, `FATAL_LEVEL`,
+    `HEADER_LEVEL`.
+  - `rocksdb.log.max.file.size` The maximal size of the info log file.
+  - `rocksdb.log.keep.file.num` Maximal info log files to be kept.
+  ``PHAB_ID=D259579``
+
+* finatra-kafka: Add admin routes for properties and topology information
+  - `/admin/kafka/streams/properties` Dumps the
+    `KafkaStreamsTwitterServer#properties` as plain text in the TwitterServer
+    admin page.
+  - `/admin/kafka/streams/topology` Dumps the
+    `KafkaStreamsTwitterServer#topology` as plain text in the TwitterServer
+    admin page.
+  ``PHAB_ID=D259597``
+
+* inject-server: EmbeddedTwitterServer that fails to start will now continue to
+  throw the startup failure on calls to methods that require a successfully started server.
+  ``PHAB_ID=D265543``
+
+Fixed
+~~~~~
+
+* finatra-kafka-streams: `FinatraTopologyTester` did not set
+  `TopologyTestDriver#initialWallClockTimeMs` on initialization causing diverging wall clock time
+  when `TopologyTestDriver#advanceWallClockTime` advanced time. The divergence was between
+  system time set by `org.joda.time.DateTimeUtils.setCurrentMillisFixed` and internal mock timer
+  `TopologyTestDriver#mockWallClockTime`. `FinatraTopologyTester.inMemoryStatsReceiver` is reset on
+  `TopologyFeatureTest#beforeEach` for all test that extend `TopologyFeatureTest`.
+  ``PHAB_ID=D269013``
+
+* finatra-kafka-streams: Improve watermark assignment/propagation upon reading the first
+  message and when caching key value stores are used. ``PHAB_ID=D262054``
+
+* finatra-jackson: Support inherited annotations in case class deserialization. Case class
+  deserialization support does not properly find inherited Jackson annotations. This means
+  that code like this:
+
+  ```
+  trait MyTrait {
+    @JsonProperty("differentName")
+    def name: String
+  }
+  case class MyCaseClass(name: String) extends MyTrait
+  ```
+
+  would not properly expect an incoming field with name `differentName` to parse into the
+  case class `name` field. This commit provides support for capturing inherited annotations
+  on case class fields. Annotations processed in order, thus if the same annotation appears
+  in the class hierarchy multiple times, the value configured on the class will win otherwise
+  will be in the order of trait linearization with the "last" declaration prevailing.
+  ``PHAB_ID=D260376``
+
+* finatra: Remove extraneous dependency on old `javax.servlet` ServletAPI dependency.
+  The fixes #478. ``PHAB_ID=D259671``
+
+Closed
+~~~~~~
+
+19.1.0
+------
+
 Added
 ~~~~~
 
 * finatra-kafka-streams: SumAggregator and CompositeSumAggregator only support enhanced window
-  aggregations for the sum operation. Deprecate SumAggregator and CompositeSumAggregator and create 
+  aggregations for the sum operation. Deprecate SumAggregator and CompositeSumAggregator and create
   an AggregatorTransformer class that can perform arbitrary aggregations. ``PHAB_ID=D257138``
 
 * finatra-streams: Open-source Finatra Streams. Finatra Streams is an integration
@@ -100,11 +220,11 @@ Changed
   now-removed `c.t.f.b.Server` have been modified or removed.
   ``PHAB_ID=D254339``
 
-* finatra-kafka-streams: Finatra Queryable State methods currently require the window size 
-  to be passed into query methods for windowed key value stores. This is unnecessary, as 
-  the queryable state class can be passed the window size at construction time. We also now 
-  save off all FinatraKeyValueStores in a global manager class to allow query services 
-  (e.g. thrift) to access the same KeyValueStore implementation that the FinatraTransformer 
+* finatra-kafka-streams: Finatra Queryable State methods currently require the window size
+  to be passed into query methods for windowed key value stores. This is unnecessary, as
+  the queryable state class can be passed the window size at construction time. We also now
+  save off all FinatraKeyValueStores in a global manager class to allow query services
+  (e.g. thrift) to access the same KeyValueStore implementation that the FinatraTransformer
   is using. ``PHAB_ID=D256920``
 
 Fixed

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 = "19.1.0"
+val releaseVersion = "19.2.0"
 
 lazy val buildSettings = Seq(
   version := releaseVersion,
@@ -74,7 +74,6 @@ lazy val versions = new {
   val scalaCheck = "1.13.4"
   val scalaGuice = "4.1.0"
   val scalaTest = "3.0.0"
-  val servletApi = "2.5"
   val slf4j = "1.7.21"
   val snakeyaml = "1.12"
   val specs2 = "2.4.17"
@@ -645,7 +644,6 @@ lazy val http = project
       "com.twitter" %% "finagle-exp" % versions.twLibVersion,
       "com.twitter" %% "finagle-http" % versions.twLibVersion,
       "commons-fileupload" % "commons-fileupload" % versions.commonsFileupload,
-      "javax.servlet" % "servlet-api" % versions.servletApi,
       "com.novocode" % "junit-interface" % "0.11" % Test
     ),
     unmanagedResourceDirectories in Test += baseDirectory(

doc/src/sphinx/user-guide/getting-started/flags.rst

@@ -17,12 +17,23 @@ of "*environment*\ " string within code, e.g.
 
 .. code:: bash
 
-	if (env == "production") { ... }
+  if (env == "production") { ... }
 
 It is generally good practice to make Flags *granular* controls that are  fully orthogonal to one
 another. They can then be independently managed for each deploy and this scales consistently as the
 number of supported  "environments" scales.
 
+Global Flags
+------------
+
+`TwitterUtil <https://github.com/twitter/util>`__ `Flags <https://github.com/twitter/util/blob/1dd3e6228162c78498338b1c3aa11afe2f8cee22/util-app/src/main/scala/com/twitter/app/Flag.scala>`__
+also has the concept of a "global" flag. That is, a flag that is "global" to the JVM process (as it is
+generally defined as a Scala object). In the discussion of Flags with Finatra we **do not** mean
+"global" flags unless it is explicitly stated.
+
+See the `scaladoc <http://twitter.github.io/util/docs/com/twitter/app/GlobalFlag.html>`__ for
+`c.t.app.GlobalFlag` for more information.
+
 But I have a lot of Flags
 -------------------------
 

doc/src/sphinx/user-guide/http/server.rst

@@ -264,7 +264,8 @@ specifically the server documentation `here <https://twitter.github.io/finagle/g
 Server-side Response Classification
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-To configure server-side `Response Classification <https://twitter.github.io/finagle/guide/Servers.html#response-classification>`__ you could choose to
+The default Response Classifier for HTTP servers is `HttpResponseClassifier.ServerErrorsAsFailures <https://github.com/twitter/finatra/blob/8b448065f5f74c1eedd744bd15618cbf932ea1bc/http/src/main/scala/com/twitter/finatra/http/response/HttpResponseClassifier.scala#L15>`__,
+which classifies any HTTP 5xx response code as a failure. To configure server-side `Response Classification <https://twitter.github.io/finagle/guide/Servers.html#response-classification>`__ you could choose to
 set the classifier directly on the underlying Finagle server by overriding the `configureHttpServer` (or `configureHttpsServer`) in your server, e.g.,
 
 .. code:: scala

doc/src/sphinx/user-guide/json/validations.rst

@@ -10,6 +10,7 @@ The validations framework integrates Finatra's custom `case class` deserializer
 -  ``@CountryCode``
 -  ``@FutureTime``
 -  ``@PastTime``
+-  ``@Pattern``
 -  ``@Max``
 -  ``@Min``
 -  ``@NotEmpty``

doc/src/sphinx/user-guide/kafka-streams/index.rst

@@ -23,7 +23,7 @@ a fully functional service can be written by simply configuring the Kafka Stream
 Transformers
 ~~~~~~~~~~~~
 
-Implement custom `transformers <https://kafka.apache.org/21/javadoc/org/apache/kafka/streams/kstream/Transformer.html>`__ using `FinatraTransformerV2 <https://github.com/twitter/finatra/blob/develop/kafka-streams/kafka-streams/src/main/scala/com/twitter/finatra/streams/transformer/FinatraTransformerV2.scala>`__.
+Implement custom `transformers <https://kafka.apache.org/21/javadoc/org/apache/kafka/streams/kstream/Transformer.html>`__ using `FinatraTransformer <https://github.com/twitter/finatra/blob/develop/kafka-streams/kafka-streams/src/main/scala/com/twitter/finatra/streams/transformer/FinatraTransformer.scala>`__.
 
 Aggregations
 ^^^^^^^^^^^^

doc/src/sphinx/user-guide/logging/asyncappender.rst

@@ -55,8 +55,8 @@ basics.html#verbosity-levels>`__ of the metrics via a Finagle `Tunable <https://
 finagle/guide/Configuration.html>`__.
 
 Users need to create a JSON file and place it in the `src/main/resources` folder in
-`com/twitter/tunables/finagle/instances.json` to whitelist the Logback metrics.
-To whitelist all Logback metrics the JSON file could contain the following:
+`com/twitter/tunables/finagle/instances.json` to acceptlist the Logback metrics.
+To acceptlist all Logback metrics the JSON file could contain the following:
 
 .. code-block:: json
 

doc/src/sphinx/user-guide/testing/embedded.rst

@@ -29,6 +29,46 @@ You'll notice that this hierarchy generally follows the server trait hierarchy a
 and |c.t.finatra.thrift.ThriftServer|_ extend from |c.t.server.TwitterServer|_ which extends from
 |c.t.app.App|_.
 
+Testing With `Global Flags`
+---------------------------
+
+The embedded servers and the embedded app allow for passing `TwitterUtil <https://github.com/twitter/util>`__ `Flags <https://github.com/twitter/util/blob/1dd3e6228162c78498338b1c3aa11afe2f8cee22/util-app/src/main/scala/com/twitter/app/Flag.scala>`__
+to the server under test via the `flags <https://github.com/twitter/finatra/blob/develop/inject/inject-server/src/test/scala/com/twitter/inject/server/EmbeddedTwitterServer.scala#L90>`__
+constructor argument (a map of flag name to flag value) which is meant to mimic setting flag values
+via the command line.
+
+However it is **not recommended** that users set any |GlobalFlag|_ value in this manner. In normal
+usage, the value of a |GlobalFlag|_ is **only read once during the initialization of the JVM process**.
+
+If you wish to test with toggled values of a |GlobalFlag|_ you should prefer using
+|FlagLet|_ or |FlagLetClear|_ in tests instead of passing the |GlobalFlag|_ value via the `flags <https://github.com/twitter/finatra/blob/develop/inject/inject-server/src/test/scala/com/twitter/inject/server/EmbeddedTwitterServer.scala#L90>`__
+arg of an embedded server or embedded app. For example,
+
+.. code:: scala
+
+    import com.twitter.finatra.http.EmbeddedHttpServer
+    import com.twitter.finagle.http.Status
+    import com.twitter.inject.server.FeatureTest
+
+    class ExampleServerFeatureTest extends FeatureTest {
+      override val server = new EmbeddedHttpServer(new ExampleServer)
+
+      test("ExampleServer#perform feature") {
+
+        someGlobalFlag.let("a value") {
+          // any read of the `someGlobalFlag` value in this closure will be "a value"
+          server.httpGet(
+            path = "/",
+            andExpect = Status.Ok)
+
+          ???
+        }
+      }
+    }
+
+See the `scaladoc <http://twitter.github.io/util/docs/com/twitter/app/Flag.html>`_ for `c.t.app.Flag`
+for more information on using |FlagLet|_ or |FlagLetClear|_.
+
 InMemoryStatsReceiver
 ---------------------
 
@@ -82,3 +122,12 @@ More Information
 
 .. |EmbeddedThriftServer| replace:: `EmbeddedThriftServer`
 .. _EmbeddedThriftServer: https://github.com/twitter/finatra/blob/develop/thrift/src/test/scala/com/twitter/finatra/thrift/EmbeddedThriftServer.scala
+
+.. |GlobalFlag| replace:: `GlobalFlag`
+.. _GlobalFlag: https://github.com/twitter/util/blob/f2a05474ec41f34146d710bdc2a789efd6da9d21/util-app/src/main/scala/com/twitter/app/GlobalFlag.scala
+
+.. |FlagLet| replace:: `Flag.let`
+.. _FlagLet: http://twitter.github.io/util/docs/com/twitter/app/Flag.html#let[R](t:T)(f:=%3ER):R
+
+.. |FlagLetClear| replace:: `Flag.letClear`
+.. _FlagLetClear: http://twitter.github.io/util/docs/com/twitter/app/Flag.html#letClear[R](f:=%3ER):R

doc/src/sphinx/user-guide/thrift/filters.rst

@@ -142,7 +142,7 @@ E.g., for the `UserFilter` defined above (shown with common filters in a recomme
           .filter[AccessLoggingFilter]
           .filter[StatsFilter]
           .filter[ExceptionMappingFilter]
-          .filter[ClientIdWhitelistFilter]
+          .filter[ClientIdAcceptlistFilter]
           .filter[FinagleRequestScopeFilter]
           .filter[UserFilter]
           .exceptionMapper[FinatraThriftExceptionMapper]

doc/src/sphinx/user-guide/thrift/server.rst

@@ -237,7 +237,8 @@ the server documentation `here <https://twitter.github.io/finagle/guide/Servers.
 Server-side Response Classification
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-To configure server-side `Response Classification <https://twitter.github.io/finagle/guide/Servers.html#response-classification>`__
+The default Response Classifier for Thrift servers is `ThriftResponseClassifier.ThriftExceptionsAsFailures <https://github.com/twitter/finatra/blob/8b448065f5f74c1eedd744bd15618cbf932ea1bc/thrift/src/main/scala/com/twitter/finatra/thrift/response/ThriftResponseClassifier.scala#L14>`__,
+which classifies any deserialized Thrift Exception as a failure. To configure server-side `Response Classification <https://twitter.github.io/finagle/guide/Servers.html#response-classification>`__
 you could choose to set the classifier directly on the underlying Finagle server by overriding the
 `configureThriftServer` in your server, e.g.,
 

examples/thrift-server/thrift-example-server/src/main/scala/com/twitter/calculator/CalculatorController.scala

@@ -7,18 +7,18 @@ import com.twitter.util.Future
 import javax.inject.Singleton
 
 @Singleton
-class CalculatorController extends Controller with Calculator.BaseServiceIface {
+class CalculatorController extends Controller(Calculator) {
 
-  override val addNumbers = handle(AddNumbers) { args: AddNumbers.Args =>
+  handle(AddNumbers) { args: AddNumbers.Args =>
     info(s"Adding numbers $args.a + $args.b")
     Future.value(args.a + args.b)
   }
 
-  override val addStrings = handle(AddStrings) { args: AddStrings.Args =>
+  handle(AddStrings) { args: AddStrings.Args =>
     Future.value((args.a.toInt + args.b.toInt).toString)
   }
 
-  override val increment = handle(Increment) { args: Increment.Args =>
+  handle(Increment) { args: Increment.Args =>
     Future.value(args.a + 1)
   }
 }

examples/thrift-server/thrift-example-server/src/test/scala/com/twitter/calculator/CalculatorServerFeatureTest.scala

@@ -12,13 +12,13 @@ class CalculatorServerFeatureTest extends FeatureTest {
 
   val client = server.thriftClient[Calculator[Future]](clientId = "client123")
 
-  test("whitelist#clients allowed") {
+  test("acceptlist#clients allowed") {
     await(client.increment(1)) should equal(2)
     await(client.addNumbers(1, 2)) should equal(3)
     await(client.addStrings("1", "2")) should equal("3")
   }
 
-  test("blacklist#clients blocked with UnknownClientIdException") {
+  test("denylist#clients blocked with UnknownClientIdException") {
     val clientWithUnknownId = server.thriftClient[Calculator[Future]](clientId = "unlisted-client")
     intercept[UnknownClientIdError] { 
       await(clientWithUnknownId.increment(2)) 

http/src/main/scala/BUILD

@@ -47,7 +47,6 @@ scala_library(
         "finatra/utils/src/main/scala",
         "twitter-server/server/src/main/scala",
         "util/util-app/src/main/scala",
-        "util/util-collection/src/main/scala",
         "util/util-core/src/main/scala",
         "util/util-lint/src/main/scala",
         "util/util-logging/src/main/scala",