Skip to content

Commit

Permalink
finagle-core: Add Address.ServiceFactory.apply Without Addr.Metadata
Browse files Browse the repository at this point in the history
Problem

It's awkward ergonomically that `Address.apply` that takes a
`ServiceFactory` doesn't require `Addr.Metadata`, but the more specific
`Address.ServiceFactory.apply` does require `Addr.Metadata`. It's a rare
scenario where a user of the API would be supplying `Addr.Metadata` and
so it would be nicer if the `Address.ServiceFactory.apply` method did
not require it.

Solution

Add a variant of `Address.ServiceFactory.apply` which does not require
`Addr.Metadata` and defaults that value to `Addr.Metadata.empty`.

Differential Revision: https://phabricator.twitter.biz/D605438
  • Loading branch information
ryanoneill authored and jenkins committed Jan 21, 2021
1 parent 56e904e commit 11971f0
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
Unreleased
----------

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``

21.1.0
------

Expand Down
6 changes: 6 additions & 0 deletions finagle-core/src/main/scala/com/twitter/finagle/Address.scala
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ object Address {
metadata: Addr.Metadata)
extends Address

object ServiceFactory {
def apply[Req, Rep](
factory: com.twitter.finagle.ServiceFactory[Req, Rep]
): ServiceFactory[Req, Rep] = ServiceFactory(factory, Addr.Metadata.empty)
}

/** Create a new [[Address]] with given [[java.net.InetSocketAddress]]. */
def apply(addr: InetSocketAddress): Address =
Address.Inet(addr, Addr.Metadata.empty)
Expand Down
34 changes: 34 additions & 0 deletions finagle-core/src/test/scala/com/twitter/finagle/AddressTest.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
package com.twitter.finagle

import com.twitter.util.Future
import java.net.{InetAddress, InetSocketAddress}
import org.scalatest.FunSuite

class AddressTest extends FunSuite {

def assertFactory[Req, Rep](address: Address, factory: ServiceFactory[Req, Rep]): Unit =
address match {
case Address.ServiceFactory(sf, _) => assert(sf == factory)
case _ => fail(s"$address is not an Address.ServiceFactory")
}

def assertMetadata(address: Address, metadata: Addr.Metadata): Unit =
address match {
case Address.ServiceFactory(_, md) => assert(md == metadata)
case _ => fail(s"$address is not an Address.ServiceFactory")
}

test("constructor with no host points to localhost") {
assert(Address(8080) == Address(new InetSocketAddress(InetAddress.getLoopbackAddress, 8080)))
}
Expand All @@ -20,6 +34,26 @@ class AddressTest extends FunSuite {
val cmp12 = ordering.compare(sorted(0), sorted(1))
val cmp23 = ordering.compare(sorted(1), sorted(2))
assert(cmp13 == 0 && cmp12 == 0 && cmp23 == 0 || cmp13 < 0 && (cmp12 < 0 || cmp23 < 0))
}

test("can be created via a ServiceFactory") {
val service: Service[String, String] = Service.mk(_ => Future.value("Whatever"))
val sf: ServiceFactory[String, String] = ServiceFactory.const(service)

// Without Metadata
val address1: Address = Address.ServiceFactory(sf)
assertFactory(address1, sf)
assertMetadata(address1, Addr.Metadata.empty)

// With Metadata
val metadata = Addr.Metadata("a" -> "b")
val address2: Address = Address.ServiceFactory(sf, metadata)
assertFactory(address2, sf)
assertMetadata(address2, metadata)

// Without Metadata via Address.apply
val address3: Address = Address(sf)
assertFactory(address3, sf)
assertMetadata(address3, Addr.Metadata.empty)
}
}

0 comments on commit 11971f0

Please sign in to comment.