Skip to content

Commit 8d768ca

Browse files
yufangongjenkins
yufangong
authored and
jenkins
committed
scrooge/finagle: Remove generated HKT, FutureIface and their friends
Problem FutureIface is deprecated. Scrooge is more powerful when used with finagle integration, and in that case, higher-kinded-types won't play its advantages. Scrooge generated code is growing which increases source compilation time, we should get rid of the unused parts. Solution Remove them from code-gen. In generated code, replace HKT/FutureIface with MethodPerEndpoint. Remove some deprecated builders in both scrooge-gen and finagle that require HKT as the parameter. Differential Revision: https://phabricator.twitter.biz/D747744
1 parent 1f87b5e commit 8d768ca

File tree

15 files changed

+150
-258
lines changed

15 files changed

+150
-258
lines changed

‎CHANGELOG.rst

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@ Unreleased
1010
Breaking API Changes
1111
~~~~~~~~~~~~~~~~~~~~
1212

13+
* scrooge-generator: Dropped the generic (higher-kinded-types) service interface in scala-gen,
14+
users are recommended to use YourService.MethodPerEndpoint, YourService.ServicePerEndpoint
15+
and YourService.ReqRepServicePerEndpoint to represent Thrift service endpoints. Note,
16+
`-finagle` option is required to generated finagle binding code. ``PHAB_ID=D747744``
17+
18+
* scrooge-generator: Removed YourService.FutureIface and YourService[Future] in scala-gen,
19+
use $YourService.MethodPerEndpoint instead. Correspondingly, YourService$FinagleService and
20+
related constructors taking MethodPerEndpoint as parameters. ``PHAB_ID=D747744``
21+
22+
* Scrooge-generator: Dropped ThriftServiceBuilder.build and MethodIfaceBuilder.newMethodIface.
23+
``PHAB_ID=D747744``
24+
1325
* scrooge-generator: Add reserved keywords to ThriftParser. If your field names match
1426
these keywords, you may need to modify them. This change should not affect backwards
1527
and forwards compatiblility if using binary protocol for serde. ``PHAB_ID=D707116``

‎scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/GoldService$FinagleClient.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ import org.apache.thrift.protocol._
1818
class GoldService$FinagleClient(
1919
val service: com.twitter.finagle.Service[ThriftClientRequest, Array[Byte]],
2020
val clientParam: RichClientParam)
21-
extends GoldService.MethodPerEndpoint
22-
with GoldService.FutureIface {
21+
extends GoldService.MethodPerEndpoint {
2322

2423
@deprecated("Use com.twitter.finagle.thrift.RichClientParam", "2017-08-16")
2524
def this(

‎scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/GoldService$FinagleService.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ import org.apache.thrift.transport.TMemoryInputTransport
2323

2424
@javax.annotation.Generated(value = Array("com.twitter.scrooge.Compiler"))
2525
class GoldService$FinagleService(
26-
iface: GoldService[Future],
26+
iface: GoldService.MethodPerEndpoint,
2727
serverParam: RichServerParam
2828
) extends com.twitter.finagle.Service[Array[Byte], Array[Byte]] {
2929
import GoldService._
3030

3131
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
3232
def this(
33-
iface: GoldService[Future],
33+
iface: GoldService.MethodPerEndpoint,
3434
protocolFactory: TProtocolFactory,
3535
stats: StatsReceiver = NullStatsReceiver,
3636
maxThriftBufferSize: Int = Thrift.param.maxThriftBufferSize,
@@ -39,15 +39,15 @@ class GoldService$FinagleService(
3939

4040
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
4141
def this(
42-
iface: GoldService[Future],
42+
iface: GoldService.MethodPerEndpoint,
4343
protocolFactory: TProtocolFactory,
4444
stats: StatsReceiver,
4545
maxThriftBufferSize: Int
4646
) = this(iface, protocolFactory, stats, maxThriftBufferSize, "GoldService")
4747

4848
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
4949
def this(
50-
iface: GoldService[Future],
50+
iface: GoldService.MethodPerEndpoint,
5151
protocolFactory: TProtocolFactory
5252
) = this(iface, protocolFactory, NullStatsReceiver, Thrift.param.maxThriftBufferSize)
5353

‎scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/GoldService.scala

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,6 @@ import scala.reflect.{ClassTag, classTag}
3434

3535

3636
@javax.annotation.Generated(value = Array("com.twitter.scrooge.Compiler"))
37-
trait GoldService[+MM[_]] extends _root_.com.twitter.finagle.thrift.ThriftService {
38-
/** Hello, I'm a comment. */
39-
def doGreatThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): MM[com.twitter.scrooge.test.gold.thriftscala.Response]
40-
41-
def noExceptionCall(request: com.twitter.scrooge.test.gold.thriftscala.Request): MM[com.twitter.scrooge.test.gold.thriftscala.Response]
42-
43-
/**
44-
* Used to close the underlying `Service`.
45-
* Not a user-defined API.
46-
*/
47-
def asClosable: _root_.com.twitter.util.Closable = _root_.com.twitter.util.Closable.nop
48-
}
49-
50-
5137
object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftService { self =>
5238

5339
val annotations: immutable$Map[String, String] = immutable$Map(
@@ -1367,12 +1353,16 @@ object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftServ
13671353
type noExceptionCall$result = NoExceptionCall.Result
13681354

13691355

1370-
trait MethodPerEndpoint
1371-
extends GoldService[Future] {
1356+
trait MethodPerEndpoint extends _root_.com.twitter.finagle.thrift.ThriftService {
13721357
/** Hello, I'm a comment. */
13731358
def doGreatThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[com.twitter.scrooge.test.gold.thriftscala.Response]
13741359

13751360
def noExceptionCall(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[com.twitter.scrooge.test.gold.thriftscala.Response]
1361+
/**
1362+
* Used to close the underlying `Service`.
1363+
* Not a user-defined API.
1364+
*/
1365+
def asClosable: _root_.com.twitter.util.Closable = _root_.com.twitter.util.Closable.nop
13761366
}
13771367

13781368
object MethodPerEndpoint {
@@ -1425,7 +1415,7 @@ object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftServ
14251415

14261416
@deprecated("Use MethodPerEndpoint", "2017-11-07")
14271417
class MethodIface(serviceIface: BaseServiceIface)
1428-
extends FutureIface {
1418+
extends MethodPerEndpoint {
14291419
def doGreatThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[com.twitter.scrooge.test.gold.thriftscala.Response] =
14301420
serviceIface.doGreatThings(self.DoGreatThings.Args(request))
14311421
def noExceptionCall(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[com.twitter.scrooge.test.gold.thriftscala.Response] =
@@ -1438,41 +1428,16 @@ object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftServ
14381428
MethodPerEndpoint(servicePerEndpoint)
14391429
}
14401430

1441-
@deprecated("Use MethodPerEndpointBuilder", "2018-01-12")
1442-
implicit object ThriftServiceBuilder
1443-
extends _root_.com.twitter.finagle.thrift.service.ThriftServiceBuilder[ServicePerEndpoint, GoldService[Future]] {
1444-
def build(servicePerEndpoint: ServicePerEndpoint): MethodPerEndpoint =
1445-
MethodPerEndpoint(servicePerEndpoint)
1446-
}
1447-
14481431
implicit object ReqRepMethodPerEndpointBuilder
14491432
extends _root_.com.twitter.finagle.thrift.service.ReqRepMethodPerEndpointBuilder[ReqRepServicePerEndpoint, MethodPerEndpoint] {
14501433
def methodPerEndpoint(servicePerEndpoint: ReqRepServicePerEndpoint): MethodPerEndpoint =
14511434
ReqRepMethodPerEndpoint(servicePerEndpoint)
14521435
}
14531436

1454-
@deprecated("Use MethodPerEndpointBuilder", "2017-11-07")
1455-
implicit object MethodIfaceBuilder
1456-
extends com.twitter.finagle.thrift.MethodIfaceBuilder[ServiceIface, GoldService[Future]] {
1457-
def newMethodIface(serviceIface: ServiceIface): MethodIface =
1458-
new MethodIface(serviceIface)
1459-
}
1460-
1461-
@deprecated("Use MethodPerEndpoint", "2017-11-07")
1462-
trait FutureIface
1463-
extends MethodPerEndpoint
1464-
with GoldService[Future] {
1465-
/** Hello, I'm a comment. */
1466-
def doGreatThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[com.twitter.scrooge.test.gold.thriftscala.Response]
1467-
1468-
def noExceptionCall(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[com.twitter.scrooge.test.gold.thriftscala.Response]
1469-
}
1470-
14711437
class FinagledClient(
14721438
service: com.twitter.finagle.Service[ThriftClientRequest, Array[Byte]],
14731439
clientParam: RichClientParam)
14741440
extends GoldService$FinagleClient(service, clientParam)
1475-
with FutureIface
14761441
with MethodPerEndpoint {
14771442

14781443
@deprecated("Use com.twitter.finagle.thrift.RichClientParam", "2017-08-16")
@@ -1509,13 +1474,13 @@ object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftServ
15091474
}
15101475

15111476
class FinagledService(
1512-
iface: GoldService[Future],
1477+
iface: MethodPerEndpoint,
15131478
serverParam: RichServerParam)
15141479
extends GoldService$FinagleService(iface, serverParam) {
15151480

15161481
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
15171482
def this(
1518-
iface: GoldService[Future],
1483+
iface: MethodPerEndpoint,
15191484
protocolFactory: org.apache.thrift.protocol.TProtocolFactory,
15201485
serviceName: String = "GoldService"
15211486
) = this(iface, RichServerParam(protocolFactory, serviceName))

‎scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/PlatinumService$FinagleClient.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ class PlatinumService$FinagleClient(
1818
override val service: com.twitter.finagle.Service[ThriftClientRequest, Array[Byte]],
1919
override val clientParam: RichClientParam)
2020
extends GoldService$FinagleClient(service, clientParam)
21-
with PlatinumService.MethodPerEndpoint
22-
with PlatinumService.FutureIface {
21+
with PlatinumService.MethodPerEndpoint {
2322

2423
@deprecated("Use com.twitter.finagle.thrift.RichClientParam", "2017-08-16")
2524
def this(

‎scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/PlatinumService$FinagleService.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ import org.apache.thrift.protocol._
2020

2121
@javax.annotation.Generated(value = Array("com.twitter.scrooge.Compiler"))
2222
class PlatinumService$FinagleService(
23-
iface: PlatinumService[Future],
23+
iface: PlatinumService.MethodPerEndpoint,
2424
serverParam: RichServerParam
2525
) extends GoldService$FinagleService(iface, serverParam) {
2626
import PlatinumService._
2727

2828
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
2929
def this(
30-
iface: PlatinumService[Future],
30+
iface: PlatinumService.MethodPerEndpoint,
3131
protocolFactory: TProtocolFactory,
3232
stats: StatsReceiver = NullStatsReceiver,
3333
maxThriftBufferSize: Int = Thrift.param.maxThriftBufferSize,
@@ -36,15 +36,15 @@ class PlatinumService$FinagleService(
3636

3737
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
3838
def this(
39-
iface: PlatinumService[Future],
39+
iface: PlatinumService.MethodPerEndpoint,
4040
protocolFactory: TProtocolFactory,
4141
stats: StatsReceiver,
4242
maxThriftBufferSize: Int
4343
) = this(iface, protocolFactory, stats, maxThriftBufferSize, "PlatinumService")
4444

4545
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
4646
def this(
47-
iface: PlatinumService[Future],
47+
iface: PlatinumService.MethodPerEndpoint,
4848
protocolFactory: TProtocolFactory
4949
) = this(iface, protocolFactory, NullStatsReceiver, Thrift.param.maxThriftBufferSize)
5050

‎scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/PlatinumService.scala

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,6 @@ import scala.reflect.{ClassTag, classTag}
3333

3434

3535
@javax.annotation.Generated(value = Array("com.twitter.scrooge.Compiler"))
36-
trait PlatinumService[+MM[_]] extends GoldService[MM] {
37-
38-
def moreCoolThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): MM[Int]
39-
40-
/**
41-
* Used to close the underlying `Service`.
42-
* Not a user-defined API.
43-
*/
44-
override def asClosable: _root_.com.twitter.util.Closable = _root_.com.twitter.util.Closable.nop
45-
}
46-
47-
4836
object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThriftService { self =>
4937

5038
val annotations: immutable$Map[String, String] = immutable$Map.empty
@@ -936,11 +924,14 @@ object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThrift
936924
type moreCoolThings$result = MoreCoolThings.Result
937925

938926

939-
trait MethodPerEndpoint
940-
extends com.twitter.scrooge.test.gold.thriftscala.GoldService.MethodPerEndpoint
941-
with PlatinumService[Future] {
927+
trait MethodPerEndpoint extends com.twitter.scrooge.test.gold.thriftscala.GoldService.MethodPerEndpoint {
942928

943929
def moreCoolThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[Int]
930+
/**
931+
* Used to close the underlying `Service`.
932+
* Not a user-defined API.
933+
*/
934+
override def asClosable: _root_.com.twitter.util.Closable = _root_.com.twitter.util.Closable.nop
944935
}
945936

946937
object MethodPerEndpoint {
@@ -989,7 +980,7 @@ object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThrift
989980
@deprecated("Use MethodPerEndpoint", "2017-11-07")
990981
class MethodIface(serviceIface: BaseServiceIface)
991982
extends com.twitter.scrooge.test.gold.thriftscala.GoldService.MethodIface(serviceIface)
992-
with FutureIface {
983+
with MethodPerEndpoint {
993984
def moreCoolThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[Int] =
994985
serviceIface.moreCoolThings(self.MoreCoolThings.Args(request))
995986
}
@@ -1000,40 +991,16 @@ object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThrift
1000991
MethodPerEndpoint(servicePerEndpoint)
1001992
}
1002993

1003-
@deprecated("Use MethodPerEndpointBuilder", "2018-01-12")
1004-
implicit object ThriftServiceBuilder
1005-
extends _root_.com.twitter.finagle.thrift.service.ThriftServiceBuilder[ServicePerEndpoint, PlatinumService[Future]] {
1006-
def build(servicePerEndpoint: ServicePerEndpoint): MethodPerEndpoint =
1007-
MethodPerEndpoint(servicePerEndpoint)
1008-
}
1009-
1010994
implicit object ReqRepMethodPerEndpointBuilder
1011995
extends _root_.com.twitter.finagle.thrift.service.ReqRepMethodPerEndpointBuilder[ReqRepServicePerEndpoint, MethodPerEndpoint] {
1012996
def methodPerEndpoint(servicePerEndpoint: ReqRepServicePerEndpoint): MethodPerEndpoint =
1013997
ReqRepMethodPerEndpoint(servicePerEndpoint)
1014998
}
1015999

1016-
@deprecated("Use MethodPerEndpointBuilder", "2017-11-07")
1017-
implicit object MethodIfaceBuilder
1018-
extends com.twitter.finagle.thrift.MethodIfaceBuilder[ServiceIface, PlatinumService[Future]] {
1019-
def newMethodIface(serviceIface: ServiceIface): MethodIface =
1020-
new MethodIface(serviceIface)
1021-
}
1022-
1023-
@deprecated("Use MethodPerEndpoint", "2017-11-07")
1024-
trait FutureIface
1025-
extends com.twitter.scrooge.test.gold.thriftscala.GoldService.FutureIface
1026-
with MethodPerEndpoint
1027-
with PlatinumService[Future] {
1028-
1029-
def moreCoolThings(request: com.twitter.scrooge.test.gold.thriftscala.Request): Future[Int]
1030-
}
1031-
10321000
class FinagledClient(
10331001
service: com.twitter.finagle.Service[ThriftClientRequest, Array[Byte]],
10341002
clientParam: RichClientParam)
10351003
extends PlatinumService$FinagleClient(service, clientParam)
1036-
with FutureIface
10371004
with MethodPerEndpoint {
10381005

10391006
@deprecated("Use com.twitter.finagle.thrift.RichClientParam", "2017-08-16")
@@ -1070,13 +1037,13 @@ object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThrift
10701037
}
10711038

10721039
class FinagledService(
1073-
iface: PlatinumService[Future],
1040+
iface: MethodPerEndpoint,
10741041
serverParam: RichServerParam)
10751042
extends PlatinumService$FinagleService(iface, serverParam) {
10761043

10771044
@deprecated("Use com.twitter.finagle.thrift.RichServerParam", "2017-08-16")
10781045
def this(
1079-
iface: PlatinumService[Future],
1046+
iface: MethodPerEndpoint,
10801047
protocolFactory: org.apache.thrift.protocol.TProtocolFactory,
10811048
serviceName: String = "PlatinumService"
10821049
) = this(iface, RichServerParam(protocolFactory, serviceName))

‎scrooge-generator-tests/src/test/scala/com/twitter/scrooge/backend/NamespaceSpec.scala

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
11
package com.twitter.scrooge.backend
22

3+
import com.twitter.conversions.DurationOps._
34
import com.twitter.scrooge.testutil.Spec
5+
import com.twitter.util.Await
6+
import com.twitter.util.Future
47

58
class NamespaceSpec extends Spec {
69
"Scala Generator" should {
710
import bar._
811
import com.fake._
912
"import from another namespace" in {
10-
val service: Restaurant[Some] = new Restaurant[Some] {
11-
def isOpen(whichDay: Weekday) = Some(whichDay != Weekday.Monday)
13+
val service: Restaurant.MethodPerEndpoint = new Restaurant.MethodPerEndpoint {
14+
def isOpen(whichDay: Weekday) = Future.value(whichDay != Weekday.Monday)
1215
def makeReservation(whichDay: Weekday, howMany: Int) =
13-
Some(if (whichDay == Weekday.Monday) 0 else howMany)
16+
Future.value(if (whichDay == Weekday.Monday) 0 else howMany)
1417
}
15-
service.makeReservation(Weekday.Monday, 2) must be(Some(0))
16-
service.makeReservation(Weekday.Tuesday, 2) must be(Some(2))
18+
Await.result(service.makeReservation(Weekday.Monday, 2), 5.seconds) must equal(0)
19+
Await.result(service.makeReservation(Weekday.Tuesday, 2), 5.seconds) must equal(2)
1720
}
1821
}
1922
}

‎scrooge-generator-tests/src/test/scala/com/twitter/scrooge/backend/NonFinagleSpec.scala

Lines changed: 0 additions & 44 deletions
This file was deleted.

0 commit comments

Comments
 (0)