From 719d505b64f8083c2214fca7da207ce06858963c Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Fri, 10 Jul 2015 20:08:01 +0000 Subject: [PATCH] [finagle-core] Remove Namer trait from Dtab Problem -- We have multiple ways to bind a name via a Dtab: Dtab itself is a `Namer`, and `NameInterpreter` convert a Path to a bound name, taking the Dtab into account. Solution -- Make local dtab resolution a NameInterpreter. Result -- One mechanism for resolving a path to a bound name using the dtab. RB_ID=711681 TBR=true --- CHANGES | 5 ++ .../main/scala/com/twitter/finagle/Dtab.scala | 69 ++++++++----------- .../scala/com/twitter/finagle/NameTree.scala | 5 +- .../scala/com/twitter/finagle/Namer.scala | 51 ++++++++------ .../finagle/client/ClientRegistry.scala | 3 +- .../finagle/naming/DefaultInterpreter.scala | 24 +++++++ .../finagle/naming/NameInterpreter.scala | 10 ++- .../scala/com/twitter/finagle/DtabTest.scala | 41 ----------- .../com/twitter/finagle/ExceptionsTest.scala | 4 +- .../com/twitter/finagle/NameTreeTest.scala | 5 +- .../finagle/client/StackClientTest.scala | 23 +++++-- .../finagle/factory/BindingFactoryTest.scala | 15 ++-- .../naming/DefaultInterpreterTest.scala | 49 +++++++++++++ .../finagle/naming/NameInterpreterTest.scala | 4 +- 14 files changed, 176 insertions(+), 132 deletions(-) create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/naming/DefaultInterpreter.scala create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/naming/DefaultInterpreterTest.scala diff --git a/CHANGES b/CHANGES index 64a9fc37d2..88054e7668 100644 --- a/CHANGES +++ b/CHANGES @@ -32,6 +32,11 @@ Breaking API Changes HttpRequest, and converted the rest of the public API to not leak other Netty types (notably ChannelBuffer is replaced by Buf). ``RB_ID=695896`` + * finagle-core: Dtab does not implement the Namer interface anymore. Use + `c.t.f.naming.DefaultInterpreter` to bind a name via a Dtab. Support for Dtab entries starting + with /#/ has been removed. `c.t.f.Namer.bindAndEval` has been removed. Use + `c.t.f.Namer.resolve` instead. ``RB_ID=711681`` + 6.26.0 ~~~~~~~ diff --git a/finagle-core/src/main/scala/com/twitter/finagle/Dtab.scala b/finagle-core/src/main/scala/com/twitter/finagle/Dtab.scala index f22a294a76..bad5a60e0e 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/Dtab.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/Dtab.scala @@ -1,56 +1,46 @@ package com.twitter.finagle import com.twitter.app.Flaggable -import com.twitter.util.{Activity, Local, Var} +import com.twitter.util.Local import java.io.PrintWriter -import java.net.SocketAddress -import java.util.concurrent.atomic.AtomicReference import scala.collection.generic.CanBuildFrom import scala.collection.immutable.VectorBuilder import scala.collection.mutable.Builder -import scala.collection.mutable /** - * A Dtab--short for delegation table--comprises a sequence - * of delegation rules. Together, these describe how to bind a - * path to an Addr. + * A Dtab--short for delegation table--comprises a sequence of + * delegation rules. Together, these describe how to bind a + * [[com.twitter.finagle.Path]] to a set of + * [[com.twitter.finagle.Addr]]. [[com.twitter.finagle.naming.DefaultInterpreter]] + * implements the default binding stategy. */ case class Dtab(dentries0: IndexedSeq[Dentry]) - extends IndexedSeq[Dentry] with Namer { + extends IndexedSeq[Dentry] { + private lazy val dentries = dentries0.reverse def apply(i: Int): Dentry = dentries0(i) def length = dentries0.length override def isEmpty = length == 0 - private[this] object NamerPath { - def unapply(path: Path): Option[(Namer, Path)] = path match { - case Path.Utf8("#", kind, rest@_*) => Some((Namer.namerOfKind(kind), Path.Utf8(rest: _*))) - case _ => None - } - } - - private def lookup0(path: Path): NameTree[Path] = { - val matches = dentries collect { - case Dentry(prefix, dst) if path startsWith prefix => - val suff = path drop prefix.size - dst map { pfx => pfx ++ suff } + /** + * Lookup the given `path` with this dtab. + */ + def lookup(path: Path): NameTree[Name.Path] = { + val matches = dentries.collect { + case Dentry(prefix, dst) if path.startsWith(prefix) => + val suff = path.drop(prefix.size) + dst.map { pfx => Name.Path(pfx ++ suff) } } matches.size match { case 0 => NameTree.Neg - case 1 => matches(0) + case 1 => matches.head case _ => NameTree.Alt(matches:_*) } } - def lookup(path: Path): Activity[NameTree[Name]] = - path match { - case NamerPath(namer, rest) => namer.lookup(rest) - case _ => Activity.value(lookup0(path) map { path => Name(path) }) - } - /** * Construct a new Dtab with the given delegation * entry appended. @@ -113,7 +103,7 @@ case class Dtab(dentries0: IndexedSeq[Dentry]) * whose destination name trees have been simplified. The returned * Dtab is equivalent with respect to evaluation. * - * @todo dedup equivalent entries so that the only the last entry is retained + * @todo dedup equivalent entries so that only the last entry is retained * @todo collapse entries with common prefixes */ def simplified: Dtab = Dtab({ @@ -130,10 +120,9 @@ case class Dtab(dentries0: IndexedSeq[Dentry]) } /** - * Trait Dentry describes a delegation table entry. - * It always has a prefix, describing the paths to - * which the entry applies, and a bind method to - * bind the given path. + * Trait Dentry describes a delegation table entry. `prefix` describes + * the paths that the entry applies to. `dst` describes the resulting + * tree for this prefix on lookup. */ case class Dentry(prefix: Path, dst: NameTree[Path]) { def show = "%s=>%s".format(prefix.show, dst.show) @@ -147,7 +136,7 @@ object Dentry { * dentry ::= path '=>' tree * }}} * - * where the productions ``path`` and ``tree`` are from the grammar + * where the productions `path` and `tree` are from the grammar * documented in [[com.twitter.finagle.NameTree$ NameTree.read]]. */ def read(s: String): Dentry = NameTreeParsers.parseDentry(s) @@ -190,10 +179,10 @@ object Dtab { * every request in this process. It is generally set at process * startup, and not changed thereafter. */ - @volatile var base: Dtab = Dtab.read("/=>/#/com.twitter.finagle.namer.global") + @volatile var base: Dtab = empty /** - * Java API for ``base_=`` + * Java API for `base_=` */ def setBase(dtab: Dtab) { base = dtab } @@ -203,14 +192,14 @@ object Dtab { * The local, or "per-request", delegation table applies to the * current [[com.twitter.util.Local Local]] scope which is usually * defined on a per-request basis. Finagle uses the Dtab - * ``Dtab.base ++ Dtab.local`` to bind - * [[com.twitter.finagle.Name.Path Paths]]. + * `Dtab.base ++ Dtab.local` to bind [[com.twitter.finagle.Name.Path + * Paths]] via a [[com.twitter.finagle.naming.NameInterpreter]]. * * Local's scope is dictated by [[com.twitter.util.Local Local]]. * * The local dtab is serialized into outbound requests when * supported protocols are used. (Http, Thrift via TTwitter, Mux, - * and ThriftMux are among these.) The upshot is that ``local`` is + * and ThriftMux are among these.) The upshot is that `local` is * defined for the entire request graph, so that a local dtab * defined here will apply to downstream services as well. */ @@ -221,7 +210,7 @@ object Dtab { def local_=(dtab: Dtab) { l() = dtab } /** - * Java API for ``local_=`` + * Java API for `local_=` */ def setLocal(dtab: Dtab) { local = dtab } @@ -237,7 +226,7 @@ object Dtab { * dtab ::= dentry ';' dtab | dentry * }}} * - * where the production ``dentry`` is from the grammar documented in + * where the production `dentry` is from the grammar documented in * [[com.twitter.finagle.Dentry$ Dentry.read]] * */ diff --git a/finagle-core/src/main/scala/com/twitter/finagle/NameTree.scala b/finagle-core/src/main/scala/com/twitter/finagle/NameTree.scala index 4befea803f..022a016f2c 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/NameTree.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/NameTree.scala @@ -1,10 +1,7 @@ package com.twitter.finagle -import com.twitter.util.{Activity, Var} import com.twitter.finagle.util.Showable import scala.annotation.tailrec -import java.net.{InetSocketAddress, SocketAddress} -import scala.collection.breakOut /** * Name trees represent a composite T-typed name whose interpretation @@ -323,7 +320,7 @@ object NameTree { * Alt(Union(Leaf(Path(foo)),Leaf(Path(bar))),Leaf(Path(baz)),Empty) * }}} * - * The production ``path`` is documented at [[com.twitter.finagle.Path$ Path.read]]. + * The production `path` is documented at [[com.twitter.finagle.Path$ Path.read]]. * * @throws IllegalArgumentException when the string does not * represent a valid name tree. diff --git a/finagle-core/src/main/scala/com/twitter/finagle/Namer.scala b/finagle-core/src/main/scala/com/twitter/finagle/Namer.scala index a0e0ee787e..9039293650 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/Namer.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/Namer.scala @@ -28,13 +28,7 @@ trait Namer { self => * to 100 is allowed. */ def bind(tree: NameTree[Path]): Activity[NameTree[Name.Bound]] = - Namer.bind(this, tree) - - /** - * Bind, then evaluate the given NameTree with this Namer. The result - * is translated into a Var[Addr]. - */ - def bindAndEval(tree: NameTree[Path]): Var[Addr] = boundNameTreeToAddr(bind(tree)) + Namer.bind(this.lookup, tree) } private case class FailingNamer(exc: Throwable) extends Namer { @@ -128,12 +122,21 @@ object Namer { override def toString = "Namer.global" } + /** + * Resolve a path to an address set (taking `dtab` into account). + */ + def resolve(dtab: Dtab, path: Path): Var[Addr] = + NameInterpreter.bind(dtab, path).map(_.eval).run.flatMap { + case Activity.Ok(None) => Var.value(Addr.Neg) + case Activity.Ok(Some(names)) => Name.all(names).addr + case Activity.Pending => Var.value(Addr.Pending) + case Activity.Failed(exc) => Var.value(Addr.Failed(exc)) + } + /** * Resolve a path to an address set (taking [[Dtab.local]] into account). */ - def resolve(path: Path): Var[Addr] = { - boundNameTreeToAddr(NameInterpreter.bind(Dtab.base ++ Dtab.local, path)) - } + def resolve(path: Path): Var[Addr] = resolve(Dtab.base ++ Dtab.local, path) /** * Resolve a path to an address set (taking [[Dtab.local]] into account). @@ -156,18 +159,26 @@ object Namer { private val MaxDepth = 100 - private def bind(namer: Namer, tree: NameTree[Path]): Activity[NameTree[Name.Bound]] = - bind(namer, 0)(tree map { path => Name.Path(path) }) + /** + * Bind the given tree by recursively following paths and looking them + * up with the provided `lookup` function. A recursion depth of up to + * 100 is allowed. + */ + def bind( + lookup: Path => Activity[NameTree[Name]], + tree: NameTree[Path] + ): Activity[NameTree[Name.Bound]] = + bind(lookup, 0)(tree map { path => Name.Path(path) }) private[this] def bindUnion( - namer: Namer, + lookup: Path => Activity[NameTree[Name]], depth: Int, trees: Seq[Weighted[Name]] ): Activity[NameTree[Name.Bound]] = { val weightedTreeVars: Seq[Var[Activity.State[NameTree.Weighted[Name.Bound]]]] = trees.map { case Weighted(w, t) => - val treesAct: Activity[NameTree[Name.Bound]] = bind(namer, depth)(t) + val treesAct: Activity[NameTree[Name.Bound]] = bind(lookup, depth)(t) treesAct.map(Weighted(w, _)).run } @@ -193,12 +204,12 @@ object Namer { } // values of the returned activity are simplified and contain no Alt nodes - private def bind(namer: Namer, depth: Int)(tree: NameTree[Name]) + private def bind(lookup: Path => Activity[NameTree[Name]], depth: Int)(tree: NameTree[Name]) : Activity[NameTree[Name.Bound]] = if (depth > MaxDepth) Activity.exception(new IllegalArgumentException("Max recursion level reached.")) else tree match { - case Leaf(Name.Path(path)) => namer.lookup(path) flatMap bind(namer, depth+1) + case Leaf(Name.Path(path)) => lookup(path) flatMap bind(lookup, depth+1) case Leaf(bound@Name.Bound(_)) => Activity.value(Leaf(bound)) case Fail => Activity.value(Fail) @@ -206,17 +217,17 @@ object Namer { case Empty => Activity.value(Empty) case Union() => Activity.value(Neg) - case Union(Weighted(_, tree)) => bind(namer, depth)(tree) - case Union(trees@_*) => bindUnion(namer, depth, trees) + case Union(Weighted(_, tree)) => bind(lookup, depth)(tree) + case Union(trees@_*) => bindUnion(lookup, depth, trees) case Alt() => Activity.value(Neg) - case Alt(tree) => bind(namer, depth)(tree) + case Alt(tree) => bind(lookup, depth)(tree) case Alt(trees@_*) => def loop(trees: Seq[NameTree[Name]]): Activity[NameTree[Name.Bound]] = trees match { case Nil => Activity.value(Neg) case Seq(head, tail@_*) => - bind(namer, depth)(head) flatMap { + bind(lookup, depth)(head) flatMap { case Fail => Activity.value(Fail) case Neg => loop(tail) case head => Activity.value(head) diff --git a/finagle-core/src/main/scala/com/twitter/finagle/client/ClientRegistry.scala b/finagle-core/src/main/scala/com/twitter/finagle/client/ClientRegistry.scala index db54966516..df4831921b 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/client/ClientRegistry.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/client/ClientRegistry.scala @@ -72,12 +72,13 @@ private[finagle] object RegistryEntryLifecycle { next: Stack[ServiceFactory[Req, Rep]] ): Stack[ServiceFactory[Req, Rep]] = { val BindingFactory.Dest(dest) = params[BindingFactory.Dest] + val BindingFactory.BaseDtab(baseDtab) = params[BindingFactory.BaseDtab] // for the benefit of ClientRegistry.expAllRegisteredClientsResolved // which waits for these to become non-Pending val va = dest match { case Name.Bound(va) => va - case Name.Path(path) => Namer.resolve(path) + case Name.Path(path) => Namer.resolve(baseDtab(), path) } val shown = Showable.show(dest) diff --git a/finagle-core/src/main/scala/com/twitter/finagle/naming/DefaultInterpreter.scala b/finagle-core/src/main/scala/com/twitter/finagle/naming/DefaultInterpreter.scala new file mode 100644 index 0000000000..9fa9e4e842 --- /dev/null +++ b/finagle-core/src/main/scala/com/twitter/finagle/naming/DefaultInterpreter.scala @@ -0,0 +1,24 @@ +package com.twitter.finagle.naming + +import com.twitter.finagle._ +import com.twitter.util.Activity + +/** + * Interpret names against the Dtab, using the default evaluation + * strategy. Recursively look up and rewrite paths according to entries + * matching the path prefix in the Dtab. If a path does not match any + * Dtab entry prefix, the global Namer is invoked to resolve it. This is + * how paths starting with `/$/` are bound. + */ +object DefaultInterpreter extends NameInterpreter { + + override def bind(dtab: Dtab, path: Path): Activity[NameTree[Name.Bound]] = { + def lookup(path: Path): Activity[NameTree[Name]] = + dtab.lookup(path) match { + case NameTree.Neg => Namer.global.bind(NameTree.Leaf(path)) + case t => Activity.value(t) + } + + Namer.bind(lookup, NameTree.Leaf(path)) + } +} diff --git a/finagle-core/src/main/scala/com/twitter/finagle/naming/NameInterpreter.scala b/finagle-core/src/main/scala/com/twitter/finagle/naming/NameInterpreter.scala index d7348b8aea..b0b9a2a635 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/naming/NameInterpreter.scala +++ b/finagle-core/src/main/scala/com/twitter/finagle/naming/NameInterpreter.scala @@ -17,17 +17,15 @@ trait NameInterpreter { object NameInterpreter extends NameInterpreter { - /** The default interpreter. Bind `path` using `dtab` as a namer. */ - val default = new NameInterpreter { - override def bind(dtab: Dtab, path: Path) = dtab.bind(NameTree.Leaf(path)) - } - /** * The global interpreter that resolves all names in Finagle. * * Can be modified to provide a different mechanism for name resolution. */ - @volatile var global: NameInterpreter = default + @volatile var global: NameInterpreter = DefaultInterpreter + + /** Java API for setting the interpreter */ + def setGlobal(nameInterpreter: NameInterpreter) = { global = nameInterpreter } override def bind(dtab: Dtab, tree: Path): Activity[NameTree[Name.Bound]] = global.bind(dtab, tree) diff --git a/finagle-core/src/test/scala/com/twitter/finagle/DtabTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/DtabTest.scala index 4dabf5ceb2..bea3bb7deb 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/DtabTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/DtabTest.scala @@ -1,7 +1,5 @@ package com.twitter.finagle -import com.twitter.util.Activity -import java.net.InetSocketAddress import org.junit.runner.RunWith import org.scalatest.FunSuite import org.scalatest.junit.{AssertionsForJUnit, JUnitRunner} @@ -17,22 +15,6 @@ class DtabTest extends FunSuite with AssertionsForJUnit { else Some(left + "!=" + right) ) - test("Lookup all prefixes in reverse order") { - val dtab = Dtab.read("/foo/bar=>/xxx;/foo=>/yyy") - - assertEquiv( - dtab.lookup(Path.read("/foo/bar/baz")).sample(), - pathTree("/yyy/bar/baz | /xxx/baz")) - } - - test("Expand names") { - val dtab = Dtab.read("/foo/bar => /xxx|/yyy&/zzz") - - assertEquiv( - dtab.lookup(Path.read("/foo/bar/baz")).sample(), - pathTree("/xxx/baz | /yyy/baz & /zzz/baz")) - } - test("d1 ++ d2") { val d1 = Dtab.read("/foo => /bar") val d2 = Dtab.read("/foo=>/biz;/biz=>/$/inet/0/8080;/bar=>/$/inet/0/9090") @@ -43,17 +25,6 @@ class DtabTest extends FunSuite with AssertionsForJUnit { /biz=>/$/inet/0/8080; /bar=>/$/inet/0/9090 """)) - - def assertEval(dtab: Dtab, path: Path, expected: Name.Bound*) { - val dtab2 = Dtab.read("/=>/#/com.twitter.finagle.namer.global") ++ dtab - dtab2.bind(NameTree.Leaf(path)).sample().eval match { - case Some(actual) => assert(actual.map(_.addr.sample) === expected.map(_.addr.sample).toSet) - case _ => assert(false) - } - } - - assertEval(d1 ++ d2, Path.read("/foo"), Name.bound(new InetSocketAddress(8080))) - assertEval(d2 ++ d1, Path.read("/foo"), Name.bound(new InetSocketAddress(9090))) } test("d1 ++ Dtab.empty") { @@ -104,16 +75,4 @@ class DtabTest extends FunSuite with AssertionsForJUnit { } catch { case _: IllegalArgumentException => Dtab.empty } assert(dtab.length === 2) } - - test("/# path loads Namer class, rewrite is ignored") { - assert(Dtab.read("/# => /foo").lookup(Path.read("/#/com.twitter.finagle.dtabtest.TestNamer")).sample() - === NameTree.Leaf(Name.Path(Path.read("/plugh/xyzzy")))) - } -} - -package dtabtest { - class TestNamer extends Namer { - def lookup(path: Path) = Activity.value(NameTree.Leaf(Name.Path(Path.read("/plugh/xyzzy")))) - def enum(prefix: Path) = throw new UnsupportedOperationException - } } diff --git a/finagle-core/src/test/scala/com/twitter/finagle/ExceptionsTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/ExceptionsTest.scala index 2c2d87b2b7..fdb5d34cba 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/ExceptionsTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/ExceptionsTest.scala @@ -109,13 +109,13 @@ class ExceptionsTest extends FunSuite with MockitoSugar { val ex = new NoBrokersAvailableException( "/s/cool/story", Dtab.base, - Dtab.read("/=>/#/com.twitter.butt") + Dtab.read("/foo=>/$/com.twitter.butt") ) assert(ex.getMessage === "No hosts are available for /s/cool/story, " + s"Dtab.base=[${Dtab.base.show}], " + - "Dtab.local=[/=>/#/com.twitter.butt]" + "Dtab.local=[/foo=>/$/com.twitter.butt]" ) } } diff --git a/finagle-core/src/test/scala/com/twitter/finagle/NameTreeTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/NameTreeTest.scala index ffcf31a982..673f911dc0 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/NameTreeTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/NameTreeTest.scala @@ -1,5 +1,6 @@ package com.twitter.finagle +import com.twitter.finagle.naming.DefaultInterpreter import com.twitter.util.NonFatal import org.junit.runner.RunWith import org.scalatest.FunSuite @@ -76,12 +77,10 @@ class NameTreeTest extends FunSuite { /bar/foo => /foo/bar""") intercept[IllegalArgumentException] { - dtab.bind(NameTree.read("/foo/bar")).sample() + DefaultInterpreter.bind(dtab, Path.read("/foo/bar")).sample() } } - - test("NameTree.eval/simplified") { val cases = Seq[(String, Option[Set[String]])]( "~" -> None, diff --git a/finagle-core/src/test/scala/com/twitter/finagle/client/StackClientTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/client/StackClientTest.scala index 0c36e4ce91..999ced3762 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/client/StackClientTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/client/StackClientTest.scala @@ -4,6 +4,7 @@ import com.twitter.finagle.Stack.Module0 import com.twitter.finagle._ import com.twitter.finagle.factory.BindingFactory import com.twitter.finagle.loadbalancer.LoadBalancerFactory +import com.twitter.finagle.naming.{DefaultInterpreter, NameInterpreter} import com.twitter.finagle.service.FailFastFactory.FailFast import com.twitter.finagle.stats.InMemoryStatsReceiver import com.twitter.finagle.util.StackRegistry @@ -12,7 +13,7 @@ import com.twitter.util._ import java.net.InetSocketAddress import org.junit.runner.RunWith import org.scalatest.concurrent.{Eventually, IntegrationPatience} -import org.scalatest.FunSuite +import org.scalatest.{BeforeAndAfter, FunSuite} import org.scalatest.junit.{AssertionsForJUnit, JUnitRunner} @RunWith(classOf[JUnitRunner]) @@ -20,7 +21,8 @@ class StackClientTest extends FunSuite with StringClient with AssertionsForJUnit with Eventually - with IntegrationPatience { + with IntegrationPatience + with BeforeAndAfter { trait Ctx { val sr = new InMemoryStatsReceiver @@ -28,6 +30,10 @@ class StackClientTest extends FunSuite .configured(param.Stats(sr)) } + after { + NameInterpreter.global = DefaultInterpreter + } + test("client stats are scoped to label")(new Ctx { // use dest when no label is set client.newService("inet!localhost:8080") @@ -323,12 +329,17 @@ class StackClientTest extends FunSuite val addr1 = new InetSocketAddress(1729) val addr2 = new InetSocketAddress(1730) - // override name resolution to a Union of two addresses - val dtab = new Dtab(Dtab.base) { - override def lookup(path: Path): Activity[NameTree[Name]] = + val baseDtab = Dtab.read("/s=>/test") + + // override name resolution to a Union of two addresses, and check + // that the base dtab is properly passed in + NameInterpreter.global = new NameInterpreter { + override def bind(dtab: Dtab, path: Path): Activity[NameTree[Name.Bound]] = { + assert(dtab === baseDtab) Activity.value(NameTree.Union( NameTree.Weighted(1D, NameTree.Leaf(Name.bound(addr1))), NameTree.Weighted(1D, NameTree.Leaf(Name.bound(addr2))))) + } } val stack = StackClient.newStack[Unit, Unit] @@ -355,7 +366,7 @@ class StackClientTest extends FunSuite new FactoryToService(stack.make(Stack.Params.empty + FactoryToService.Enabled(true) + param.Stats(sr) + - BindingFactory.BaseDtab(() => dtab))) + BindingFactory.BaseDtab(() => baseDtab))) intercept[ChannelWriteException] { Await.result(service(())) diff --git a/finagle-core/src/test/scala/com/twitter/finagle/factory/BindingFactoryTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/factory/BindingFactoryTest.scala index 892034370c..a8befdc9e3 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/factory/BindingFactoryTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/factory/BindingFactoryTest.scala @@ -2,6 +2,7 @@ package com.twitter.finagle.factory import com.twitter.conversions.time._ import com.twitter.finagle._ +import com.twitter.finagle.naming.{DefaultInterpreter, NameInterpreter} import com.twitter.finagle.stack.nilStack import com.twitter.finagle.stats._ import com.twitter.finagle.util.Rng @@ -27,6 +28,7 @@ class BindingFactoryTest extends FunSuite with MockitoSugar with BeforeAndAfter after { Dtab.base = saveBase + NameInterpreter.global = DefaultInterpreter } def anonNamer() = new Namer { @@ -199,7 +201,7 @@ class BindingFactoryTest extends FunSuite with MockitoSugar with BeforeAndAfter expectTrace(Seq( "namer.path" -> "/foo/bar", - "namer.dtab.base" -> "/=>/#/com.twitter.finagle.namer.global;/test1010=>/$/inet/0/1010", + "namer.dtab.base" -> "/test1010=>/$/inet/0/1010", "namer.tree" -> "/$/inet/0/1010", "namer.name" -> "/$/inet/0/1010" )) @@ -208,16 +210,15 @@ class BindingFactoryTest extends FunSuite with MockitoSugar with BeforeAndAfter test("Trace on exception") (new Ctx { val exc = new RuntimeException - Dtab.base = new Dtab(Dtab.base) { - override def lookup(path: Path) = - Activity.exception(exc) + NameInterpreter.global = new NameInterpreter { + override def bind(dtab: Dtab, path: Path) = Activity.exception(exc) } assert(intercept[Failure](Await.result(factory())).isFlagged(Failure.Naming)) expectTrace(Seq( "namer.path" -> "/foo/bar", - "namer.dtab.base" -> "/=>/#/com.twitter.finagle.namer.global;/test1010=>/$/inet/0/1010", + "namer.dtab.base" -> "/test1010=>/$/inet/0/1010", "namer.failure" -> "java.lang.RuntimeException" )) }) @@ -229,7 +230,7 @@ class BindingFactoryTest extends FunSuite with MockitoSugar with BeforeAndAfter expectTrace(Seq( "namer.path" -> "/foo/bar", - "namer.dtab.base" -> "/=>/#/com.twitter.finagle.namer.global;/test1010=>/$/inet/0/1010", + "namer.dtab.base" -> "/test1010=>/$/inet/0/1010", "namer.tree" -> "~" )) }) @@ -258,7 +259,7 @@ class BindingFactoryTest extends FunSuite with MockitoSugar with BeforeAndAfter expectTrace(Seq( "namer.path" -> "/foo/bar", - "namer.dtab.base" -> "/=>/#/com.twitter.finagle.namer.global;/test1010=>/$/inet/0/1010", + "namer.dtab.base" -> "/test1010=>/$/inet/0/1010", "namer.tree" -> "/$/inet/0/1010", "namer.name" -> "/$/inet/0/1010" )) diff --git a/finagle-core/src/test/scala/com/twitter/finagle/naming/DefaultInterpreterTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/naming/DefaultInterpreterTest.scala new file mode 100644 index 0000000000..7bef21df71 --- /dev/null +++ b/finagle-core/src/test/scala/com/twitter/finagle/naming/DefaultInterpreterTest.scala @@ -0,0 +1,49 @@ +package com.twitter.finagle.naming + +import com.twitter.finagle._ +import com.twitter.util.Activity +import java.net.InetSocketAddress +import org.junit.runner.RunWith +import org.scalatest.junit.JUnitRunner +import org.scalatest.FunSuite + +@RunWith(classOf[JUnitRunner]) +class DefaultInterpreterTest extends FunSuite { + + def assertEval(dtab: Dtab, path: String, expected: Name.Bound*) { + DefaultInterpreter.bind(dtab, Path.read(path)).sample().eval match { + case Some(actual) => assert(actual.map(_.addr.sample) === expected.map(_.addr.sample).toSet) + case _ => assert(false) + } + } + + test("basic dtab evaluation") { + val dtab = Dtab.read("/foo=>/$/inet/0/8080") + assertEval(dtab, "/foo", Name.bound(new InetSocketAddress(8080))) + } + + test("with indirections") { + val dtab = Dtab.read("/foo=>/bar;/bar=>/$/inet/0/8080") + assertEval(dtab, "/foo", Name.bound(new InetSocketAddress(8080))) + } + + test("order of dtab evaluation") { + val d1 = Dtab.read("/foo=>/bar") + val d2 = Dtab.read("/foo=>/biz;/biz=>/$/inet/0/8080;/bar=>/$/inet/0/9090") + + assertEval(d1 ++ d2, "/foo", Name.bound(new InetSocketAddress(8080))) + assertEval(d2 ++ d1, "/foo", Name.bound(new InetSocketAddress(9090))) + } + + test("full example") { + val dtab = Dtab.read(""" + /foo => /bar; + /foo => 3 * /baz & 2 * /booz; + /baz => 3 * /$/inet/0/8080 & 2 * /$/inet/0/9090; + /booz => ~ + """) + + assertEval(dtab, "/foo", + Name.bound(new InetSocketAddress(8080)), Name.bound(new InetSocketAddress(9090))) + } +} diff --git a/finagle-core/src/test/scala/com/twitter/finagle/naming/NameInterpreterTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/naming/NameInterpreterTest.scala index 75dc4c68d3..67497489ec 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/naming/NameInterpreterTest.scala +++ b/finagle-core/src/test/scala/com/twitter/finagle/naming/NameInterpreterTest.scala @@ -9,11 +9,11 @@ import org.scalatest.{BeforeAndAfter, FunSuite} @RunWith(classOf[JUnitRunner]) class NameInterpreterTest extends FunSuite with BeforeAndAfter { - val dtab = Dtab.read("/=>/#/com.twitter.finagle.namer.global;/test=>/$/inet/some-host/1234") + val dtab = Dtab.read("/test=>/$/inet/some-host/1234") val name = Path.read("/test") after { - NameInterpreter.global = NameInterpreter.default + NameInterpreter.global = DefaultInterpreter } test("NameInterpreter uses dtab when interpreter is not set") {