Skip to content

Commit

Permalink
Merge commit '0ff52ab60dd8bdfea907db6a7ebf7b3ae5dfa98e'
Browse files Browse the repository at this point in the history
  • Loading branch information
CSL committed Dec 9, 2017
2 parents cd3d2e7 + 0ff52ab commit 0ceb0c2
Show file tree
Hide file tree
Showing 13 changed files with 643 additions and 10 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,28 @@ All notable changes to this project will be documented in this file. Note that `

### Closed

## [finatra-17.12.0](https://github.com/twitter/finatra/tree/finatra-17.12.0) (2017-12-08)

### Added

* finatra-thrift: Add tests for new Scrooge `ReqRepServicePerEndpoint`
functionalty. ``PHAB_ID=D107397``

### Changed

* finatra-http: add a `multipart = true` arg to
`EmbeddedHttpServer.httpMultipartFormPost`
``PHAB_ID=D113151`
* inject-sever: Do not use the `c.t.inject.server.EmbeddedTwitterServer`
`InMemoryStatsReceiver` for embedded http clients. The http client stats are
emitted with the server under test stats which can be confusing, thus we now
create a new `InMemoryStatsReceiver` when creating an embedded http client.
``PHAB_ID=D112024``

### Fixed

### Closed

## [finatra-17.11.0](https://github.com/twitter/finatra/tree/finatra-17.11.0) (2017-11-15)

### Added
Expand Down
20 changes: 16 additions & 4 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "17.11.0"
val releaseVersion = "17.12.0"

lazy val buildSettings = Seq(
version := releaseVersion,
Expand Down Expand Up @@ -493,6 +493,7 @@ lazy val benchmarks = project
.settings(
libraryDependencies ++= Seq(
"org.slf4j" % "slf4j-simple" % versions.slf4j % "test"))
.settings(noPublishSettings)
.dependsOn(
http,
injectRequestScope,
Expand Down Expand Up @@ -722,6 +723,7 @@ lazy val site = (project in file("doc"))

lazy val helloWorld = (project in file("examples/hello-world"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "hello-world",
moduleName := "hello-world"
Expand All @@ -732,6 +734,7 @@ lazy val helloWorld = (project in file("examples/hello-world"))

lazy val streamingExample = (project in file("examples/streaming-example"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "streaming-example",
moduleName := "streaming-example",
Expand All @@ -745,6 +748,7 @@ lazy val streamingExample = (project in file("examples/streaming-example"))

lazy val twitterClone = (project in file("examples/twitter-clone"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "twitter-clone",
moduleName := "twitter-clone",
Expand All @@ -757,6 +761,7 @@ lazy val twitterClone = (project in file("examples/twitter-clone"))

lazy val benchmarkServer = (project in file("examples/benchmark-server"))
.settings(baseServerSettings)
.settings(noPublishSettings)
.settings(
name := "benchmark-server",
moduleName := "benchmark-server",
Expand All @@ -771,6 +776,7 @@ lazy val benchmarkServer = (project in file("examples/benchmark-server"))

lazy val exampleHttpJavaServer = (project in file("examples/java-http-server"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "java-http-server",
moduleName := "java-http-server",
Expand All @@ -785,6 +791,7 @@ lazy val exampleHttpJavaServer = (project in file("examples/java-http-server"))

lazy val exampleInjectJavaServer = (project in file("examples/java-server"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "java-server",
moduleName := "java-server",
Expand All @@ -799,6 +806,7 @@ lazy val exampleInjectJavaServer = (project in file("examples/java-server"))

lazy val thriftExampleIdl = (project in file("examples/thrift-server/thrift-example-idl"))
.settings(baseServerSettings)
.settings(noPublishSettings)
.settings(
name := "thrift-example-idl",
moduleName := "thrift-example-idl",
Expand All @@ -810,6 +818,7 @@ lazy val thriftExampleIdl = (project in file("examples/thrift-server/thrift-exam

lazy val thriftExampleServer = (project in file("examples/thrift-server/thrift-example-server"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "thrift-example-server",
moduleName := "thrift-example-server",
Expand All @@ -824,6 +833,7 @@ lazy val thriftExampleServer = (project in file("examples/thrift-server/thrift-e

lazy val thriftJavaExampleIdl = (project in file("examples/java-thrift-server/thrift-example-idl"))
.settings(baseServerSettings)
.settings(noPublishSettings)
.settings(
name := "java-thrift-example-idl",
moduleName := "java-thrift-example-idl",
Expand All @@ -836,6 +846,7 @@ lazy val thriftJavaExampleIdl = (project in file("examples/java-thrift-server/th

lazy val thriftJavaExampleServer = (project in file("examples/java-thrift-server/thrift-example-server"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "java-thrift-example-server",
moduleName := "java-thrift-example-server"
Expand All @@ -847,9 +858,10 @@ lazy val thriftJavaExampleServer = (project in file("examples/java-thrift-server
injectServer % "test->test",
injectSlf4j)

lazy val exampleWebDashboard = (project in file("examples/web-dashboard")).
settings(exampleServerSettings).
settings(
lazy val exampleWebDashboard = (project in file("examples/web-dashboard"))
.settings(exampleServerSettings)
.settings(noPublishSettings)
.settings(
name := "web-dashboard",
moduleName := "web-dashboard",
unmanagedResourceDirectories in Compile += baseDirectory.value / "src" / "main" / "webapp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,27 @@ import com.twitter.inject.Logging
import com.twitter.util.{Future, Stopwatch}
import javax.inject.{Inject, Singleton}

/**
* Provides a standard "Access Log" -- a list of all requests through this Filter.
*
* NOTE: This Filter should occur as early in the Filter chain as possible.
* Typically, this Filter is provided by the [[com.twitter.finatra.http.modules.AccessLogModule]]
* which provides an implementation for the [[LogFormatter]] param as an instance of
* [[com.twitter.finagle.http.filter.CommonLogFormatter]].
*
* This Filter is included in the Finatra [[com.twitter.finatra.http.filters.CommonFilters]].
*
* Usage: To use, create a logger (with your preferred logging implementation) over this class
* which is configured to write to a specific file (usually named, `access.log`).
*
* @param logFormatter an instance of [[LogFormatter]] which specifies how log lines should be formatted.
* @tparam R - "Request" type param which must be a subtype of [[com.twitter.finagle.http.Request]].
*
* @see [[com.twitter.finatra.http.filters.CommonFilters]]
* @see [[com.twitter.finatra.http.modules.AccessLogModule]]
* @see [[com.twitter.finagle.filter.LogFormatter]]
* @see [[com.twitter.finagle.http.filter.CommonLogFormatter]]
*/
@Singleton
class AccessLoggingFilter[R <: Request] @Inject()(logFormatter: LogFormatter[R, Response])
extends SimpleFilter[R, Response]
Expand All @@ -21,7 +42,7 @@ class AccessLoggingFilter[R <: Request] @Inject()(logFormatter: LogFormatter[R,
service(request) onSuccess { response =>
info(logFormatter.format(request, response, elapsed()))
} onFailure { e =>
// should never get here since this filter is meant to be after the exception barrier
// should never get here since this filter is meant to be after the `ExceptionMappingFilter`
info(logFormatter.formatException(request, e, elapsed()))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ class HttpRouter @Inject()(
route.path.startsWith(FinatraAdminPrefix) || route.admin
}
assertAdminRoutes(adminRoutes)
RoutesByType(external = externalRoutes.toSeq, admin = adminRoutes.toSeq)
RoutesByType(external = externalRoutes, admin = adminRoutes)
}

// non-constant routes MUST start with /admin/finatra
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ class EmbeddedHttpServer(
*
* @param path - URI of the request
* @param params - a Seq of [[com.twitter.finagle.http.FormElement]] to send in the request
* @param multipart - if this form post is a multi-part request, true by default
* @param routeToAdminServer - force the request to the admin interface of the embedded server, false by default.
* @param headers - additional headers that should be passed with the request
* @param andExpect - expected [[com.twitter.finagle.http.Status]] value
Expand All @@ -937,6 +938,7 @@ class EmbeddedHttpServer(
def httpMultipartFormPost(
path: String,
params: Seq[FormElement],
multipart: Boolean = true,
routeToAdminServer: Boolean = false,
headers: Map[String, String] = Map.empty,
andExpect: Status = Status.Ok,
Expand All @@ -948,7 +950,7 @@ class EmbeddedHttpServer(
formPost(
path = path,
params = params,
multipart = true,
multipart = multipart,
routeToAdminServer = routeToAdminServer,
headers = headers,
andExpect = andExpect,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ class EmbeddedTwitterServer(
.hosts(host)
.hostConnectionLimit(75)
.retryPolicy(retryPolicy)
.reportTo(inMemoryStatsReceiver)
.reportTo(new InMemoryStatsReceiver)
.failFast(false)
.daemon(true)

Expand Down
2 changes: 1 addition & 1 deletion project/plugins.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ resolvers ++= Seq(
Resolver.sonatypeRepo("snapshots")
)

val releaseVersion = "17.11.0"
val releaseVersion = "17.12.0"

addSbtPlugin("com.twitter" % "scrooge-sbt-plugin" % releaseVersion)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.twitter.finatra.thrift.tests

import com.twitter.doeverything.thriftscala.{Answer, Question, DoEverything}
import com.twitter.doeverything.thriftscala.{Answer, DoEverything, Question}
import com.twitter.finatra.thrift.EmbeddedThriftServer
import com.twitter.finatra.thrift.tests.doeverything.DoEverythingThriftServer
import com.twitter.finatra.thrift.thriftscala.{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package com.twitter.finatra.thrift.tests

import com.twitter.doeverything.thriftscala.DoEverything
import com.twitter.doeverything.thriftscala.DoEverything.{Echo, MagicNum, Uppercase}
import com.twitter.finagle.ThriftMux
import com.twitter.finagle.param.Stats
import com.twitter.finagle.stats.NullStatsReceiver
import com.twitter.finatra.thrift.EmbeddedThriftServer
import com.twitter.finatra.thrift.tests.doeverything.DoEverythingThriftTwitterServer
import com.twitter.inject.server.PortUtils
import com.twitter.io.Buf
import com.twitter.scrooge.{Request, Response}
import com.twitter.util.Future

class DoEverythingThriftServerReqRepTest extends ReqRepServicePerEndpointTest {
private val server = new EmbeddedThriftServer(
twitterServer = new DoEverythingThriftTwitterServer(TestClientRequestHeaderKey)
)

private val muxCtxtsFilter = muxContextsFilter

private val reqRepServicePerEndpoint: DoEverything.ReqRepServicePerEndpoint =
ThriftMux.client
.filtered(muxCtxtsFilter.toFilter)
.configured(Stats(NullStatsReceiver))
.withClientId(TestClientId)
.reqRepServicePerEndpoint[DoEverything.ReqRepServicePerEndpoint](
PortUtils.loopbackAddressForPort(server.thriftPort),
"client123")

/* test that copy works for client */
private val filteredReqRepServicePerEndpoint: DoEverything.ReqRepServicePerEndpoint =
reqRepServicePerEndpoint
.withUppercase(
uppercase = specificMethodLoggingFilter.andThen(reqRepServicePerEndpoint.uppercase))
.filtered(globalLoggingFilter)

override def beforeAll(): Unit = {
server.start()
}

override def afterAll(): Unit = {
server.close()
}

override def afterEach(): Unit = {
muxCtxtsFilter.clear()
}

test("uppercase") {
val request: Request[Uppercase.Args] =
Request(
headers = Map(
TestClientRequestHeaderKey -> Seq(Buf.Utf8("bar")),
"com.twitter.client123.another.header" -> Seq(Buf.Utf8("foo")),
"com.twitter.finagle.Retries" -> Seq(Buf.Utf8( "5"))), // set a header that finagle already broadcast to ensure that we collect and don't blow up.
args = Uppercase.Args("Hi"))
val f: Future[Response[Uppercase.SuccessType]] =
reqRepServicePerEndpoint.uppercase(request)
val response = await(f)
response.value should equal("HI")
response.headers.isEmpty should be(false)
response.headers.toMap.size should be(1)
val buf = response.headers("com.twitter.doeverything.thriftscala.doeverything.uppercase").head
decodeBuf(buf) should equal("response")

muxCtxtsFilter.contexts.isEmpty should be(false)
muxCtxtsFilter.contexts.size should be(1)
val (keyBuf, valBuf) = muxCtxtsFilter.contexts.head
decodeBuf(keyBuf) should equal("com.twitter.doeverything.thriftscala.doeverything.uppercase")
decodeBuf(valBuf) should equal("response")
}

test("filtered uppercase") {
val request: Request[Uppercase.Args] =
Request(Uppercase.Args("Hi"))
.setHeader(TestClientRequestHeaderKey, "bar")
.setHeader("com.twitter.finagle.Retries", "5") // set a header that finagle already broadcast to ensure that we collect and don't blow up.
val f: Future[Response[Uppercase.SuccessType]] =
filteredReqRepServicePerEndpoint.uppercase(request)
val response = await(f)
response.value should equal("HI")
response.headers.isEmpty should be(false)
response.headers.toMap.size should be(1)
val buf = response.headers("com.twitter.doeverything.thriftscala.doeverything.uppercase").head
decodeBuf(buf) should equal("response")
}

test("echo") {
val request: Request[Echo.Args] =
Request(Echo.Args("Hello, World."))
.setHeader(TestClientRequestHeaderKey, "bar")
val f: Future[Response[Echo.SuccessType]] =
reqRepServicePerEndpoint.echo(request)
val response = await(f)
response.value should equal("Hello, World.")
response.headers.isEmpty should be(false)
response.headers.toMap.size should be(1)
val buf = response.headers("com.twitter.doeverything.thriftscala.doeverything.echo").head
decodeBuf(buf) should equal("response")
}

test("filtered echo") {
val request: Request[Echo.Args] =
Request(
headers = Map(
TestClientRequestHeaderKey -> Seq(Buf.Utf8("bar")),
"com.twitter.client123.another.header" -> Seq(Buf.Utf8("foo"))),
args = Echo.Args("Hello, World."))
val f: Future[Response[Echo.SuccessType]] =
filteredReqRepServicePerEndpoint.echo(request)
val response = await(f)
response.value should equal("Hello, World.")
response.headers.isEmpty should be(false)
response.headers.toMap.size should be(1)
val buf = response.headers("com.twitter.doeverything.thriftscala.doeverything.echo").head
decodeBuf(buf) should equal("response")
}

test("magicNum[Future]") {
// use ServicePerEndpoint which has the 'muxCtxtsFilter' ThriftMux client filter
val reqRepMethodPerEndpoint: DoEverything[Future] =
ThriftMux.client
.reqRepMethodPerEndpoint(reqRepServicePerEndpoint)

val f: Future[String] =
reqRepMethodPerEndpoint.magicNum()
val response = await(f)
response should equal("42")

muxCtxtsFilter.contexts.isEmpty should be(false)
muxCtxtsFilter.contexts.size should be(1)
val (keyBuf, valBuf) = muxCtxtsFilter.contexts.head
decodeBuf(keyBuf) should equal("com.twitter.doeverything.thriftscala.doeverything.magicNum")
decodeBuf(valBuf) should equal("response")
}

test("methodPerEndpoint magicNum") {
val methodPerEndpoint: DoEverything.MethodPerEndpoint =
ThriftMux.client
.filtered(muxCtxtsFilter.toFilter)
.newIface[DoEverything.MethodPerEndpoint](
PortUtils.loopbackAddressForPort(server.thriftPort),
"client123")

val f: Future[MagicNum.SuccessType] =
methodPerEndpoint.magicNum()
val response = await(f)
response should equal("42")

muxCtxtsFilter.contexts.isEmpty should be(false)
muxCtxtsFilter.contexts.size should be(1)
val (keyBuf, valBuf) = muxCtxtsFilter.contexts.head
decodeBuf(keyBuf) should equal("com.twitter.doeverything.thriftscala.doeverything.magicNum")
decodeBuf(valBuf) should equal("response")
}
}
Loading

0 comments on commit 0ceb0c2

Please sign in to comment.