From 1422ffd5b35cfe01374b90122b671b2c69043ad7 Mon Sep 17 00:00:00 2001 From: Ryan O'Neill Date: Fri, 22 Jan 2021 23:53:59 +0000 Subject: [PATCH] finagle-core: Add Name.bound Variant Which Takes a Service 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 --- CHANGELOG.rst | 4 +++ .../main/scala/com/twitter/finagle/Name.scala | 31 +++++++++++++++++++ .../twitter/finagle/NameCompilationTest.java | 7 +++++ .../scala/com/twitter/finagle/NameTest.scala | 28 ++++++++++++++++- 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 80d3ccab8a..f46387014b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -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 ------ diff --git a/finagle-core/src/main/scala/com/twitter/finagle/Name.scala b/finagle-core/src/main/scala/com/twitter/finagle/Name.scala index d46da54467..0107cf1892 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/Name.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/Name.scala @@ -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. @@ -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 diff --git a/finagle-core/src/test/java/com/twitter/finagle/NameCompilationTest.java b/finagle-core/src/test/java/com/twitter/finagle/NameCompilationTest.java index c170a7816d..ab87f5b032 100644 --- a/finagle-core/src/test/java/com/twitter/finagle/NameCompilationTest.java +++ b/finagle-core/src/test/java/com/twitter/finagle/NameCompilationTest.java @@ -5,6 +5,8 @@ import org.junit.Assert; import org.junit.Test; +import com.twitter.util.Future; + public class NameCompilationTest { @Test @@ -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(); diff --git a/finagle-core/src/test/scala/com/twitter/finagle/NameTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/NameTest.scala index 7054b7ffd9..2a2916c3ba 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/NameTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/NameTest.scala @@ -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 @@ -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") + } + } }