Skip to content

Commit

Permalink
inject-thrift-client: Make Injector accessible in ThriftMethodBuilder…
Browse files Browse the repository at this point in the history
…ClientModule

Summary: Problem

There are cases where further configuration of the given c.t.finagle.thriftmux.MethodBuilder
or c.t.inject.thrift.ThriftMethodBuilderFactory could benefit from having
access to the Injector to allow for using bound instances from the object graph.

Solution

Update the c.t.inject.thrift.modules.ThriftMethodBuilderClientModule#configureMethodBuilder
and c.t.inject.thrift.modules.ThriftMethodBuilderClientModule#configureServicePerEndpoint
methods to accept a c.t.inject.Injector as an argument.

Result

Users can obtain objects from the object graph when further configuring the
underlying client.

Differential Revision: https://phabricator.twitter.biz/D155451
  • Loading branch information
rkstap authored and jenkins committed Apr 5, 2018
1 parent e3047fe commit ee16c1c
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 1 deletion.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,13 @@ All notable changes to this project will be documented in this file. Note that `

### Added

* inject-thrift-client: Update `configureServicePerEndpoint` and
`configureMethodBuilder` in `ThriftMethodBuilderClientModule` to also pass a
`c.t.inject.Injector` instance which allows users to use bound instances from
the object graph when providing further `thriftmux.MethodBuilder` or
`ThriftMethodBuilderFactory` configuration.
``PHAB_ID=D155451``

* inject-thrift-client: Update `configureThriftMuxClient` in `ThriftClientModuleTrait` to
also pass a `c.t.inject.Injector` instance which allows users to use bound instances
from the object graph when providing further `ThriftMux.client` configuration.
Expand Down
Expand Up @@ -76,11 +76,13 @@ abstract class ThriftMethodBuilderClientModule[ServicePerEndpoint <: Filterable[
* Note: any configuration here will be applied to all methods unless explicitly overridden. However,
* also note that filters are cumulative. Thus filters added here will be present in any final configuration.
*
* @param injector a [[com.twitter.inject.Injector]] instance
* @param methodBuilder the [[thriftmux.MethodBuilder]] to configure.
* @return a configured MethodBuilder which will be used as the starting point for any per-method
* configuration.
*/
protected def configureMethodBuilder(
injector: Injector,
methodBuilder: thriftmux.MethodBuilder
): thriftmux.MethodBuilder = methodBuilder

Expand All @@ -98,12 +100,14 @@ abstract class ThriftMethodBuilderClientModule[ServicePerEndpoint <: Filterable[
* Subclasses of this module MAY provide an implementation of `configureServicePerEndpoint` which
* specifies configuration of a `ServicePerEndpoint` interface per-method of the interface.
*
* @param injector a [[com.twitter.inject.Injector]] instance
* @param builder a [[ThriftMethodBuilderFactory]] for creating a [[com.twitter.inject.thrift.ThriftMethodBuilder]].
* @param servicePerEndpoint the [[ServicePerEndpoint]] to configure.
* @return a per-method filtered [[ServicePerEndpoint]]
* @see [[com.twitter.inject.thrift.ThriftMethodBuilder]]
*/
protected def configureServicePerEndpoint(
injector: Injector,
builder: ThriftMethodBuilderFactory[ServicePerEndpoint],
servicePerEndpoint: ServicePerEndpoint
): ServicePerEndpoint = servicePerEndpoint
Expand All @@ -127,10 +131,11 @@ abstract class ThriftMethodBuilderClientModule[ServicePerEndpoint <: Filterable[
createThriftMuxClient(injector, clientId, statsReceiver)

val methodBuilder =
configureMethodBuilder(thriftMuxClient.methodBuilder(dest))
configureMethodBuilder(injector, thriftMuxClient.methodBuilder(dest))

val configuredServicePerEndpoint =
configureServicePerEndpoint(
injector,
builder = new ThriftMethodBuilderFactory[ServicePerEndpoint](injector, methodBuilder),
servicePerEndpoint = methodBuilder.servicePerEndpoint[ServicePerEndpoint])

Expand Down
Expand Up @@ -5,6 +5,7 @@ import com.twitter.inject.exceptions.PossiblyRetryable
import com.twitter.inject.thrift.ThriftMethodBuilderFactory
import com.twitter.inject.thrift.integration.filters.MethodLoggingTypeAgnosticFilter
import com.twitter.inject.thrift.modules.{ThriftClientIdModule, ThriftMethodBuilderClientModule}
import com.twitter.inject.Injector
import com.twitter.serviceA.thriftscala.ServiceA
import com.twitter.serviceB.thriftscala.ServiceB

Expand All @@ -17,6 +18,7 @@ object ServiceBThriftMethodBuilderClientModule
override val label = "serviceB-thrift-client"

override protected def configureServicePerEndpoint(
injector: Injector,
builder: ThriftMethodBuilderFactory[ServiceB.ServicePerEndpoint],
servicePerEndpoint: ServiceB.ServicePerEndpoint
): ServiceB.ServicePerEndpoint = {
Expand Down
Expand Up @@ -8,6 +8,7 @@ import com.twitter.inject.exceptions.PossiblyRetryable
import com.twitter.inject.thrift.ThriftMethodBuilderFactory
import com.twitter.inject.thrift.integration.filters.{HiLoggingTypeAgnosticFilter, MethodLoggingTypeAgnosticFilter}
import com.twitter.inject.thrift.modules.{ThriftClientIdModule, ThriftMethodBuilderClientModule}
import com.twitter.inject.Injector
import com.twitter.util.tunable.Tunable
import com.twitter.util.{Duration, Return, Throw}
import scala.util.control.NonFatal
Expand All @@ -23,6 +24,7 @@ class GreeterReqRepThriftMethodBuilderClientModule(
override val label = "greeter-thrift-client"

override protected def configureServicePerEndpoint(
injector: Injector,
builder: ThriftMethodBuilderFactory[Greeter.ReqRepServicePerEndpoint],
servicePerEndpoint: Greeter.ReqRepServicePerEndpoint
): Greeter.ReqRepServicePerEndpoint = {
Expand Down
Expand Up @@ -5,6 +5,7 @@ import com.twitter.inject.exceptions.PossiblyRetryable
import com.twitter.inject.thrift.ThriftMethodBuilderFactory
import com.twitter.inject.thrift.integration.filters.{MethodLoggingTypeAgnosticFilter, SetTimesEchoTypeAgnosticFilter}
import com.twitter.inject.thrift.modules.{ThriftClientIdModule, ThriftMethodBuilderClientModule}
import com.twitter.inject.Injector
import com.twitter.test.thriftscala.EchoService

object EchoThriftMethodBuilderClientModule
Expand All @@ -16,6 +17,7 @@ object EchoThriftMethodBuilderClientModule
override val label = "echo-thrift-client"

override protected def configureServicePerEndpoint(
injector: Injector,
builder: ThriftMethodBuilderFactory[EchoService.ServicePerEndpoint],
servicePerEndpoint: EchoService.ServicePerEndpoint
): EchoService.ServicePerEndpoint = {
Expand Down
Expand Up @@ -8,6 +8,7 @@ import com.twitter.inject.exceptions.PossiblyRetryable
import com.twitter.inject.thrift.ThriftMethodBuilderFactory
import com.twitter.inject.thrift.integration.filters.{HiLoggingTypeAgnosticFilter, MethodLoggingTypeAgnosticFilter}
import com.twitter.inject.thrift.modules.{ThriftClientIdModule, ThriftMethodBuilderClientModule}
import com.twitter.inject.Injector
import com.twitter.util.{Return, Throw}
import scala.util.control.NonFatal

Expand All @@ -20,6 +21,7 @@ object GreeterThriftMethodBuilderClientModule
override val label = "greeter-thrift-client"

override protected def configureServicePerEndpoint(
injector: Injector,
builder: ThriftMethodBuilderFactory[Greeter.ServicePerEndpoint],
servicePerEndpoint: Greeter.ServicePerEndpoint
): Greeter.ServicePerEndpoint = {
Expand Down

0 comments on commit ee16c1c

Please sign in to comment.