Skip to content

Commit

Permalink
scrooge-generator: Provide Implementation for ServicePerEndpoint.filt…
Browse files Browse the repository at this point in the history
…ered

Problem / Solution

Classes which extend `ServicePerEndpoint` are not provided with a working
implementation of its `filtered` method. This can be confusing for users
who attempt to apply a TypeAgnostic filter and find that it does't work
without overriding the `filtered` method to provide a concrete implementation.
Compounding the problem is that using `ServicePerEndpoint.apply` does return
an instance with a working `filtered` method. So, let's delegate the
implementation of the extension to the companion object.

JIRA Issues: CSL-8042

Differential Revision: https://phabricator.twitter.biz/D309920
  • Loading branch information
ryanoneill authored and jenkins committed May 6, 2019
1 parent 2aec9b1 commit e46b278
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
Unreleased
----------

* scrooge-generator: Extensions of (ReqRep)ServicePerEndpoint now provide a proper `filtered`
method by default. ``PHAB_ID=D309920``

19.4.0
------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftServ
* Prepends the given type-agnostic `Filter` to all of the `Services`
* and returns a copy of the `ServicePerEndpoint` now including the filter.
*/
def filtered(filter: _root_.com.twitter.finagle.Filter.TypeAgnostic): ServicePerEndpoint = this
def filtered(filter: _root_.com.twitter.finagle.Filter.TypeAgnostic): ServicePerEndpoint =
ServicePerEndpoint.apply(doGreatThings).filtered(filter)

/**
* Converts the `ServicePerEndpoint` to a `GeneratedThriftService`.
Expand All @@ -92,7 +93,8 @@ object GoldService extends _root_.com.twitter.finagle.thrift.GeneratedThriftServ
* Prepends the given type-agnostic `Filter` to all of the `Services`
* and returns a copy of the `ServicePerEndpoint` now including the filter.
*/
def filtered(filter: com.twitter.finagle.Filter.TypeAgnostic): ReqRepServicePerEndpoint = this
def filtered(filter: com.twitter.finagle.Filter.TypeAgnostic): ReqRepServicePerEndpoint =
ReqRepServicePerEndpoint.apply(doGreatThings).filtered(filter)

/**
* Converts the `ServicePerEndpoint` to a `GeneratedThriftService`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThrift
* Prepends the given type-agnostic `Filter` to all of the `Services`
* and returns a copy of the `ServicePerEndpoint` now including the filter.
*/
override def filtered(filter: _root_.com.twitter.finagle.Filter.TypeAgnostic): ServicePerEndpoint = this
override def filtered(filter: _root_.com.twitter.finagle.Filter.TypeAgnostic): ServicePerEndpoint =
ServicePerEndpoint.apply(moreCoolThings, doGreatThings).filtered(filter)

/**
* Converts the `ServicePerEndpoint` to a `GeneratedThriftService`.
Expand Down Expand Up @@ -94,7 +95,8 @@ object PlatinumService extends _root_.com.twitter.finagle.thrift.GeneratedThrift
* Prepends the given type-agnostic `Filter` to all of the `Services`
* and returns a copy of the `ServicePerEndpoint` now including the filter.
*/
override def filtered(filter: com.twitter.finagle.Filter.TypeAgnostic): ReqRepServicePerEndpoint = this
override def filtered(filter: com.twitter.finagle.Filter.TypeAgnostic): ReqRepServicePerEndpoint =
ReqRepServicePerEndpoint.apply(moreCoolThings, doGreatThings).filtered(filter)

/**
* Converts the `ServicePerEndpoint` to a `GeneratedThriftService`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ object {{ServiceName}} {{#withFinagle}}extends _root_.com.twitter.finagle.thrift
* Prepends the given type-agnostic `Filter` to all of the `Services`
* and returns a copy of the `ServicePerEndpoint` now including the filter.
*/
{{#parent}}override {{/parent}}def filtered(filter: _root_.com.twitter.finagle.Filter.TypeAgnostic): ServicePerEndpoint = this
{{#parent}}override {{/parent}}def filtered(filter: _root_.com.twitter.finagle.Filter.TypeAgnostic): ServicePerEndpoint =
ServicePerEndpoint.apply({{#inheritedFunctions}}{{dedupedFuncName}}{{/inheritedFunctions|, }}).filtered(filter)

/**
* Converts the `ServicePerEndpoint` to a `GeneratedThriftService`.
Expand Down Expand Up @@ -126,7 +127,8 @@ object {{ServiceName}} {{#withFinagle}}extends _root_.com.twitter.finagle.thrift
* Prepends the given type-agnostic `Filter` to all of the `Services`
* and returns a copy of the `ServicePerEndpoint` now including the filter.
*/
{{#parent}}override {{/parent}}def filtered(filter: com.twitter.finagle.Filter.TypeAgnostic): ReqRepServicePerEndpoint = this
{{#parent}}override {{/parent}}def filtered(filter: com.twitter.finagle.Filter.TypeAgnostic): ReqRepServicePerEndpoint =
ReqRepServicePerEndpoint.apply({{#inheritedFunctions}}{{dedupedFuncName}}{{/inheritedFunctions|, }}).filtered(filter)

/**
* Converts the `ServicePerEndpoint` to a `GeneratedThriftService`.
Expand Down

0 comments on commit e46b278

Please sign in to comment.