Skip to content

Commit

Permalink
finagle-core: Add Name.bound Variant Which Takes a Service
Browse files Browse the repository at this point in the history
Problem

For testing Finagle clients, it's extremely useful to be able to pass it
a `Service` directly that the client can use instead of talking to a
server. This is possible because Finagle has a unified interface between
client and server where a server exports a `Service` and a client
imports it. Many long time Finagle users don't know that this is
possible though because to go from a `Service` to a `Name` (which is
necessary for `EndpointerStackClient.newService` et al.) requires
multiple hops from `Service` to `ServiceFactory` to `Address` to `Name`.

Solution

Provide an alternative to `Name.bound` which can take in a `Service`
directly and handles the necessary hops for the user.

Differential Revision: https://phabricator.twitter.biz/D605745
  • Loading branch information
ryanoneill authored and jenkins committed Jan 23, 2021
1 parent 11971f0 commit 1422ffd
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ New Features
* finagle-core: Added variant of `c.t.f.Address.ServiceFactory.apply` that does not require
specifying `c.t.f.Addr.Metadata` and defaults to `c.t.f.Addr.Metadata.empty`. ``PHAB_ID=D605438``

* finagle-core: Added variant of `c.t.f.Name.bound` which takes a `c.t.f.Service` as a parameter.
Tying a `Name` directly to a `Service` can be extremely useful for testing the functionality
of a Finagle client. ``PHAB_ID=D605745``

21.1.0
------

Expand Down
31 changes: 31 additions & 0 deletions finagle-core/src/main/scala/com/twitter/finagle/Name.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,33 @@ object Name {
def bound(addrs: Address*): Name.Bound =
Name.Bound(Var.value(Addr.Bound(addrs: _*)), addrs.toSet)

/**
* Create a pre-bound [[Address]] which points directly to the provided [[Service]].
*
* @note This method can be extremely useful in testing the functionality of a Finagle
* client without involving the network.
*
* {{{
* import com.twitter.conversions.DurationOps._
* import com.twitter.finagle.{Http, Name, Service}
* import com.twitter.finagle.http.{Request, Response, Status}
* import com.twitter.util.{Await, Future}
*
* val service: Service[Request, Response] = Service.mk { request =>
* val response = Response()
* response.status = Status.Ok
* response.contentString = "Hello"
* Future.value(response)
* }
* val name = Name.bound(service)
* val client = Http.client.newService(name, "hello-service-example")
* val result = Await.result(client(Request("/")), 1.second)
* result.contentString // "Hello"
* }}}
*/
def bound[Req, Rep](service: Service[Req, Rep]): Name.Bound =
bound(Address.ServiceFactory[Req, Rep](ServiceFactory.const(service)))

/**
* An always-empty name.
* @see [[Names.empty]] for Java compatibility.
Expand Down Expand Up @@ -206,6 +233,10 @@ object Names {
def bound(addrs: Address*): Name.Bound =
Name.bound(addrs: _*)

/** See [[Name.bound]] */
def bound[Req, Rep](service: Service[Req, Rep]): Name.Bound =
Name.bound(service)

/** See [[Name.empty]] */
def empty: Name.Bound =
Name.empty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import org.junit.Assert;
import org.junit.Test;

import com.twitter.util.Future;

public class NameCompilationTest {

@Test
Expand All @@ -23,6 +25,11 @@ public void testBound() {
Name.Bound boundMultiple = Names.bound(addr, addr);
}

@Test
public void testBoundService() {
Name.Bound bound = Names.bound(Service.constant(Future.value("Hello")));
}

@Test
public void testEmpty() {
Name.Bound bound = Names.empty();
Expand Down
28 changes: 27 additions & 1 deletion finagle-core/src/test/scala/com/twitter/finagle/NameTest.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.twitter.finagle

import com.twitter.util.{Witness, Var}
import com.twitter.util.{Future, Var, Witness}
import java.net.{InetSocketAddress, SocketAddress}
import org.scalatest.FunSuite

Expand Down Expand Up @@ -42,4 +42,30 @@ class NameTest extends FunSuite {
assert(Name.all(names) == Name.all(names))
assert(Name.all(names) != Name.all(names drop 1))
}

test("Name.bound can take a Service for testing purposes") {
val service: Service[String, String] = Service.mk(_ => Future.value("Hello"))
val name = Name.bound(service)

// Sample the Var[Addr]
val addr = name.addr.sample
// It should be an Addr.Bound
addr match {
case Addr.Bound(addrs, metadata) =>
// Metadata should be empty
assert(metadata == Addr.Metadata.empty)
// Addrs should be a set with a single item being
// the ServiceFactory for the provided Service
assert(addrs.size == 1)
addrs.head match {
case Address.ServiceFactory(_, _) =>
// This is good enough as we can't test the equality of the contained
// 'factory' with only the 'service'.
succeed
case _ =>
fail(s"Expected $addrs to be an Address.ServiceFactory")
}
case _ => fail(s"Expected $addr to be an Addr.Bound")
}
}
}

0 comments on commit 1422ffd

Please sign in to comment.