Skip to content

Commit 9a0f2ea

Browse files
jenkinsjenkins
authored andcommitted
Merge commit 'c4241c0c80534dbc249e7e15770b7f68341128b4'
2 parents 53c540e + c4241c0 commit 9a0f2ea

14 files changed

Lines changed: 160 additions & 50 deletions

File tree

CHANGELOG.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ Note that ``RB_ID=#`` and ``PHAB_ID=#`` correspond to associated message in comm
77
Unreleased
88
----------
99

10+
19.5.1
11+
------
12+
Fixed
13+
~~~~~
14+
15+
* finatra: The added `c.t.finatra.http.RouteHint` was missing from the test-jar sources and has
16+
been added. ``PHAB_ID=D317282``
17+
1018
19.5.0
1119
------
1220

build.sbt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import scoverage.ScoverageKeys
44
concurrentRestrictions in Global += Tags.limit(Tags.Test, 1)
55

66
// All Twitter library releases are date versioned as YY.MM.patch
7-
val releaseVersion = "19.5.0"
7+
val releaseVersion = "19.5.1"
88

99
lazy val buildSettings = Seq(
1010
version := releaseVersion,
@@ -627,6 +627,7 @@ lazy val httpTestJarSources =
627627
"com/twitter/finatra/http/HttpMockResponses",
628628
"com/twitter/finatra/http/HttpTest",
629629
"com/twitter/finatra/http/JsonAwareEmbeddedHttpClient",
630+
"com/twitter/finatra/http/RouteHint",
630631
"com/twitter/finatra/http/StreamingJsonTestHelper")
631632
lazy val http = project
632633
.settings(projectSettings)

http/src/main/scala/com/twitter/finatra/http/internal/marshalling/CallbackConverter.scala

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,20 @@ package com.twitter.finatra.http.internal.marshalling
33
import com.twitter.concurrent.AsyncStream
44
import com.twitter.finagle.http._
55
import com.twitter.finatra.http.internal.marshalling.CallbackConverter.url
6-
import com.twitter.finatra.http.response.{ResponseBuilder, StreamingResponse}
7-
import com.twitter.finatra.http.streaming.{FromReader, StreamingRequest}
6+
import com.twitter.finatra.http.response.{
7+
ResponseBuilder,
8+
StreamingResponse => DeprecatedStreamingResponse
9+
}
10+
import com.twitter.finatra.http.streaming.{FromReader, StreamingRequest, StreamingResponse}
811
import com.twitter.finatra.json.FinatraObjectMapper
912
import com.twitter.finatra.json.internal.streaming.JsonStreamParser
1013
import com.twitter.inject.TypeUtils
1114
import com.twitter.io.{Buf, Reader}
1215
import com.twitter.util.{Future, FuturePool, Promise}
1316
import javax.inject.Inject
1417
import scala.concurrent.{ExecutionContext => ScalaExecutionContext, Future => ScalaFuture}
15-
import scala.util.{Failure, Success}
1618
import scala.reflect.runtime.universe._
19+
import scala.util.{Failure, Success}
1720

1821
private object CallbackConverter {
1922
val url =
@@ -53,7 +56,7 @@ private[http] class CallbackConverter @Inject()(
5356
manifestRequestType match {
5457
case request if request == manifest[Request] =>
5558
callback.asInstanceOf[Request => ResponseType]
56-
case streamingRequest if runtimeClassEqs[StreamingRequest[_, _]](streamingRequest) =>
59+
case streamingRequest if runtimeClassEqs[StreamingRequest[Any, _]](streamingRequest) =>
5760
val streamIdentity = streamingRequest.typeArguments.head
5861
val streamType = streamingRequest.typeArguments.last
5962
request: Request =>
@@ -170,7 +173,8 @@ private[http] class CallbackConverter @Inject()(
170173
private def isStreamingType(manifested: Manifest[_]): Boolean = {
171174
runtimeClassEqs[AsyncStream[_]](manifested) ||
172175
runtimeClassEqs[Reader[_]](manifested) ||
173-
runtimeClassEqs[StreamingResponse[_, _]](manifested)
176+
runtimeClassEqs[DeprecatedStreamingResponse[_, _]](manifested) ||
177+
runtimeClassEqs[StreamingResponse[Any, _]](manifested)
174178
}
175179

176180
/**
@@ -185,6 +189,7 @@ private[http] class CallbackConverter @Inject()(
185189
requestCallback: Request => ResponseType,
186190
manifested: Manifest[_]
187191
): Request => Future[Response] = {
192+
// If the requestType is stream of Buf, pass it through without adding json fmt
188193
val (jsonPrefix, jsonSeparator, jsonSuffix) = {
189194
val typeArgs = TypeUtils.asManifest[RequestType].typeArguments
190195
if (typeArgs.size == 1 && typeArgs.head.runtimeClass == classOf[Buf]) {
@@ -197,37 +202,51 @@ private[http] class CallbackConverter @Inject()(
197202
case asyncStreamBuf if asyncStreamBuf == manifest[AsyncStream[Buf]] =>
198203
request: Request =>
199204
val asyncStream = requestCallback(request).asInstanceOf[AsyncStream[Buf]]
200-
val streamingResponse =
201-
StreamingResponse.apply[Buf](toBuf = a => a)(asyncStream = asyncStream)
202-
streamingResponse.toFutureFinagleResponse
205+
val depStreamingResponse =
206+
DeprecatedStreamingResponse.apply[Buf](toBuf = a => a)(asyncStream = asyncStream)
207+
depStreamingResponse.toFutureFinagleResponse
203208
case asyncStream if runtimeClassEqs[AsyncStream[_]](asyncStream) =>
204209
request: Request =>
205210
val asyncStream = requestCallback(request).asInstanceOf[AsyncStream[_]]
206-
val streamingResponse =
207-
StreamingResponse.apply(
211+
val depStreamingResponse =
212+
DeprecatedStreamingResponse.apply(
208213
toBuf = mapper.writeValueAsBuf,
209214
prefix = jsonPrefix,
210215
separator = jsonSeparator,
211216
suffix = jsonSuffix)(asyncStream = asyncStream)
212-
streamingResponse.toFutureFinagleResponse
217+
depStreamingResponse.toFutureFinagleResponse
213218
case readerBuf if readerBuf == manifest[Reader[Buf]] =>
214219
request: Request =>
215220
val reader = requestCallback(request).asInstanceOf[Reader[Buf]]
216-
val streamingResponse =
217-
StreamingResponse.apply[Buf](toBuf = a => a)(asyncStream = Reader.toAsyncStream(reader))
218-
streamingResponse.toFutureFinagleResponse
221+
val depStreamingResponse =
222+
DeprecatedStreamingResponse.apply[Buf](toBuf = a => a)(
223+
asyncStream = Reader.toAsyncStream(reader))
224+
depStreamingResponse.toFutureFinagleResponse
219225
case reader if runtimeClassEqs[Reader[_]](reader) =>
220226
request: Request =>
221227
val reader = requestCallback(request).asInstanceOf[Reader[_]]
222-
val streamingResponse = StreamingResponse.apply(
228+
val depStreamingResponse = DeprecatedStreamingResponse.apply(
223229
toBuf = mapper.writeValueAsBuf,
224230
prefix = jsonPrefix,
225231
separator = jsonSeparator,
226232
suffix = jsonSuffix)(asyncStream = Reader.toAsyncStream(reader))
227-
streamingResponse.toFutureFinagleResponse
228-
case streamingResponse if runtimeClassEqs[StreamingResponse[_, _]](streamingResponse) =>
233+
depStreamingResponse.toFutureFinagleResponse
234+
case depStreamingResponse
235+
if runtimeClassEqs[DeprecatedStreamingResponse[_, _]](depStreamingResponse) =>
229236
request: Request =>
230-
requestCallback(request).asInstanceOf[StreamingResponse[_, _]].toFutureFinagleResponse
237+
requestCallback(request)
238+
.asInstanceOf[DeprecatedStreamingResponse[_, _]].toFutureFinagleResponse
239+
case streamingResponse if runtimeClassEqs[StreamingResponse[Any, _]](streamingResponse) =>
240+
request: Request =>
241+
streamingResponse.typeArguments.head match {
242+
case r: Reader[_] =>
243+
requestCallback(request).asInstanceOf[StreamingResponse[Reader, _]].toFutureResponse
244+
case as: AsyncStream[_] =>
245+
requestCallback(request)
246+
.asInstanceOf[StreamingResponse[AsyncStream, _]].toFutureResponse
247+
case _ =>
248+
requestCallback(request).asInstanceOf[StreamingResponse[Any, _]].toFutureResponse
249+
}
231250
}
232251
}
233252

http/src/main/scala/com/twitter/finatra/http/streaming/StreamingRequest.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import com.twitter.concurrent.AsyncStream
44
import com.twitter.finagle.http.Request
55
import com.twitter.finatra.json.internal.streaming.JsonStreamParser
66
import com.twitter.io.{Buf, Reader}
7+
import scala.language.higherKinds
78

89
private[http] object StreamingRequest {
910

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package com.twitter.finatra.http.streaming
22

33
import com.twitter.finagle.http.{Response, Status, Version}
44
import com.twitter.finatra.json.FinatraObjectMapper
5+
import com.twitter.io.Buf
56
import com.twitter.util.Future
7+
import scala.language.higherKinds
68

79
/**
810
* StreamingResponse is an abstraction over an output Primitive Stream - Reader or AsyncStream.
@@ -16,8 +18,9 @@ private[http] final case class StreamingResponse[F[_]: ToReader, A] private (
1618
mapper: FinatraObjectMapper,
1719
val stream: F[A]) {
1820

19-
private[this] val reader = implicitly[ToReader[F]].apply(stream).map { i =>
20-
mapper.writeValueAsBuf(i)
21+
private[this] val reader = implicitly[ToReader[F]].apply(stream).map {
22+
case str: String => Buf.Utf8(str)
23+
case any => mapper.writeValueAsBuf(any)
2124
}
2225

2326
/**

http/src/test/java/com/twitter/finatra/http/tests/streaming/StreamingResponseJavaTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public void serdeReaderofString() throws Exception {
8181
Response response = fromReader(reader);
8282
Future<Buf> fBuf = Readers.readAll(response.reader());
8383
String result = Buf.decodeString(Await.result(fBuf), StandardCharsets.UTF_8);
84-
Assert.assertEquals("\"first\"\"second\"\"third\"", result);
84+
Assert.assertEquals("firstsecondthird", result);
8585
}
8686

8787
@Test
@@ -122,7 +122,7 @@ public void serdeAsyncStreamOfString() throws Exception {
122122
Response response = fromStream(stream);
123123
Future<Buf> fBuf = Readers.readAll(response.reader());
124124
String result = Buf.decodeString(Await.result(fBuf), StandardCharsets.UTF_8);
125-
Assert.assertEquals("\"first\"\"second\"\"third\"", result);
125+
Assert.assertEquals("firstsecondthird", result);
126126
}
127127

128128
}

http/src/test/scala/com/twitter/finatra/http/EmbeddedHttpServer.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ import scala.collection.JavaConverters._
5151
* underlying server when testing with an injectable server. By default
5252
* an injectable server under test will have an [[com.twitter.finagle.stats.InMemoryStatsReceiver]]
5353
* implementation bound for the purpose of testing. In some cases, users may want to test using
54-
* a custom [[StatsReceiver]] implementation instead and can provide and instance
54+
* a custom [[StatsReceiver]] implementation instead and can provide an instance
5555
* to use here. For non-injectable servers this can be a shared reference
5656
* used in the server under test.
5757
*/

http/src/test/scala/com/twitter/finatra/http/tests/integration/tweetexample/main/controllers/TweetsController.scala

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ package com.twitter.finatra.http.tests.integration.tweetexample.main.controllers
33
import com.twitter.concurrent.AsyncStream
44
import com.twitter.finagle.http.{Fields, Request, Response, Status}
55
import com.twitter.finatra.http.Controller
6+
import com.twitter.finatra.http.response.{
7+
StreamingResponseUtils,
8+
StreamingResponse => DeprecatedStreamingResponse
9+
}
10+
import com.twitter.finatra.http.streaming.StreamingRequest
611
import com.twitter.finatra.http.tests.integration.tweetexample.main.domain.Tweet
712
import com.twitter.finatra.http.tests.integration.tweetexample.main.services.TweetsRepository
8-
import com.twitter.finatra.http.response.{StreamingResponse, StreamingResponseUtils}
9-
import com.twitter.finatra.http.streaming.StreamingRequest
1013
import com.twitter.io.{Buf, Reader}
1114
import com.twitter.util.{Duration, Future, Try}
1215
import java.nio.charset.StandardCharsets
@@ -16,8 +19,7 @@ import scala.collection.mutable
1619
class TweetsController @Inject()(
1720
tweetsRepository: TweetsRepository,
1821
onWriteLog: mutable.ArrayBuffer[String]
19-
)
20-
extends Controller {
22+
) extends Controller {
2123

2224
get("/tweets/hello") { request: Request =>
2325
"hello world"
@@ -47,17 +49,24 @@ class TweetsController @Inject()(
4749
bufs
4850
}
4951

50-
5152
get("/tweets/streaming_json") { request: Request =>
5253
tweetsRepository.getByIds(Reader.fromSeq(Seq(0, 1, 2, 3, 4, 5)))
5354
}
5455

5556
get("/tweets/streaming_custom_tobuf") { request: Request =>
56-
StreamingResponse(Buf.Utf8.apply) {
57+
DeprecatedStreamingResponse(Buf.Utf8.apply) {
5758
AsyncStream("A", "B", "C")
5859
}
5960
}
6061

62+
get("/tweets/streamingRep_with_asyncStream") { _: Request =>
63+
response.streaming(AsyncStream("A", "B", "C"))
64+
}
65+
66+
get("/tweets/streamingRep_with_reader") { _: Request =>
67+
response.streaming(Reader.fromSeq(Seq(1, 2, 3)))
68+
}
69+
6170
get("/tweets/streaming_with_transformer") { _: Request =>
6271
def lowercaseTransformer(as: AsyncStream[String]) = as.map(_.toLowerCase)
6372

@@ -67,20 +76,27 @@ class TweetsController @Inject()(
6776

6877
def onWrite(ignored: Unit, buf: Buf)(t: Try[Unit]): Unit = ()
6978

70-
StreamingResponse(
71-
transformer,
72-
Status.Ok,
73-
Map.empty,
74-
onWrite,
75-
() => (),
76-
Duration.Zero
77-
) {
79+
DeprecatedStreamingResponse(
80+
transformer,
81+
Status.Ok,
82+
Map.empty,
83+
onWrite,
84+
() => (),
85+
Duration.Zero
86+
) {
7887
AsyncStream("A", "B", "C")
7988
}
8089
}
8190

82-
get("/tweets/streaming_with_onWrite") { _: Request =>
91+
get("/tweets/streamingRep_with_transformer_asyncStream") { _: Request =>
92+
response.streaming(AsyncStream("A", "B", "C").map(_.toLowerCase))
93+
}
94+
95+
get("/tweets/streamingRep_with_transformer_reader") { _: Request =>
96+
response.streaming(Reader.fromSeq(Seq("A", "B", "C")).map(_.toLowerCase))
97+
}
8398

99+
get("/tweets/streaming_with_onWrite") { _: Request =>
84100
def serializeAndLowercase(as: AsyncStream[String]): AsyncStream[(String, Buf)] = {
85101
as.map(str => (str.toLowerCase, Buf.Utf8(str)))
86102
}
@@ -89,7 +105,7 @@ class TweetsController @Inject()(
89105
onWriteLog.append(lowerCased)
90106
}
91107

92-
StreamingResponse(
108+
DeprecatedStreamingResponse(
93109
serializeAndLowercase,
94110
Status.Ok,
95111
Map.empty,
@@ -108,7 +124,7 @@ class TweetsController @Inject()(
108124
Fields.Pragma -> "no-cache"
109125
)
110126

111-
StreamingResponse(Buf.Utf8.apply, Status.Created, headers) {
127+
DeprecatedStreamingResponse(Buf.Utf8.apply, Status.Created, headers) {
112128
AsyncStream("A", "B", "C")
113129
}
114130
}

http/src/test/scala/com/twitter/finatra/http/tests/integration/tweetexample/test/TweetsControllerIntegrationTest.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,32 @@ class TweetsControllerIntegrationTest extends FeatureTest {
197197
server.httpGet("/tweets/streaming_custom_tobuf", andExpect = Status.Ok, withBody = "ABC")
198198
}
199199

200+
test("get StreamingResponse with AsyncStream") {
201+
server.httpGet("/tweets/streamingRep_with_asyncStream", andExpect = Status.Ok, withBody = "ABC")
202+
}
203+
204+
test("get StreamingResponse with Reader") {
205+
server.httpGet("/tweets/streamingRep_with_reader", andExpect = Status.Ok, withBody = "123")
206+
}
207+
200208
test("get streaming with transformer") {
201209
server.httpGet("/tweets/streaming_with_transformer", andExpect = Status.Ok, withBody = "abc")
202210
}
203211

212+
test("get StreamingResponse with AsyncStream with transformer ") {
213+
server.httpGet(
214+
"/tweets/streamingRep_with_transformer_asyncStream",
215+
andExpect = Status.Ok,
216+
withBody = "abc")
217+
}
218+
219+
test("get StreamingResponse with Reader with transformer") {
220+
server.httpGet(
221+
"/tweets/streamingRep_with_transformer_reader",
222+
andExpect = Status.Ok,
223+
withBody = "abc")
224+
}
225+
204226
test("get streaming with onWrite") {
205227
server.httpGet("/tweets/streaming_with_onWrite", andExpect = Status.Ok, withBody = "ABC")
206228
assert(onWriteLog == Seq("a", "b", "c"))

0 commit comments

Comments
 (0)