ref: Remove v23.NewServer and rpc.DeprecatedServer from the public api..
MultiPart: 2/2
Change-Id: Ie77e973beeb7c5b4b002322cd08d8b3e4df9f585
diff --git a/runtime/factories/fake/rpc.go b/runtime/factories/fake/rpc.go
index 786b021..22f0bf0 100644
--- a/runtime/factories/fake/rpc.go
+++ b/runtime/factories/fake/rpc.go
@@ -26,10 +26,6 @@
return c
}
-func (r *Runtime) NewServer(ctx *context.T, opts ...rpc.ServerOpt) (rpc.DeprecatedServer, error) {
- defer apilog.LogCallf(ctx, "opts...=%v", opts)(ctx, "") // gologcop: DO NOT EDIT, MUST BE FIRST STATEMENT
- panic("unimplemented")
-}
func (r *Runtime) WithNewStreamManager(ctx *context.T) (*context.T, error) {
defer apilog.LogCall(ctx)(ctx) // gologcop: DO NOT EDIT, MUST BE FIRST STATEMENT
panic("unimplemented")
diff --git a/runtime/internal/rpc/full_test.go b/runtime/internal/rpc/full_test.go
index d42c249..cb24d94 100644
--- a/runtime/internal/rpc/full_test.go
+++ b/runtime/internal/rpc/full_test.go
@@ -78,7 +78,7 @@
c.Unlock()
}
-func testInternalNewServerWithPubsub(ctx *context.T, streamMgr stream.Manager, ns namespace.T, settingsPublisher *pubsub.Publisher, settingsStreamName string, opts ...rpc.ServerOpt) (rpc.DeprecatedServer, error) {
+func testInternalNewServerWithPubsub(ctx *context.T, streamMgr stream.Manager, ns namespace.T, settingsPublisher *pubsub.Publisher, settingsStreamName string, opts ...rpc.ServerOpt) (DeprecatedServer, error) {
client, err := InternalNewClient(streamMgr, ns)
if err != nil {
return nil, err
@@ -86,7 +86,7 @@
return InternalNewServer(ctx, streamMgr, ns, settingsPublisher, settingsStreamName, client, opts...)
}
-func testInternalNewServer(ctx *context.T, streamMgr stream.Manager, ns namespace.T, opts ...rpc.ServerOpt) (rpc.DeprecatedServer, error) {
+func testInternalNewServer(ctx *context.T, streamMgr stream.Manager, ns namespace.T, opts ...rpc.ServerOpt) (DeprecatedServer, error) {
return testInternalNewServerWithPubsub(ctx, streamMgr, ns, nil, "", opts...)
}
diff --git a/runtime/internal/rpc/resolve_test.go b/runtime/internal/rpc/resolve_test.go
index 8f325c7..be38047 100644
--- a/runtime/internal/rpc/resolve_test.go
+++ b/runtime/internal/rpc/resolve_test.go
@@ -96,6 +96,10 @@
return s.ExpectVar("MT_NAME")
}
+type fakeService struct{}
+
+func (f *fakeService) Foo(ctx *context.T, call rpc.ServerCall) error { return nil }
+
func TestResolveToEndpoint(t *testing.T) {
setupRuntime()
ctx, shutdown := v23.Init()
@@ -117,7 +121,7 @@
t.Fatalf("ns.Mount failed: %s", err)
}
- server, err := v23.NewServer(ctx)
+ _, server, err := v23.WithNewServer(ctx, "", &fakeService{}, nil)
if err != nil {
t.Fatalf("runtime.NewServer failed: %s", err)
}
diff --git a/runtime/internal/rpc/server.go b/runtime/internal/rpc/server.go
index f93c7f4..8fbd1f8 100644
--- a/runtime/internal/rpc/server.go
+++ b/runtime/internal/rpc/server.go
@@ -56,6 +56,74 @@
errNoListeners = reg(".errNoListeners", "failed to ceate any listeners{:3}")
)
+type DeprecatedServer interface {
+ // Listen creates a listening network endpoint for the Server
+ // as specified by its ListenSpec parameter. If any of the listen
+ // addresses passed in the ListenSpec are 'unspecified' (e.g. don't
+ // include a fixed address such as in ":0") and the ListenSpec includes
+ // a Publisher, then 'roaming' support will be enabled. In this mode
+ // the server will listen for changes in the network configuration
+ // using a Stream created on the supplied Publisher and change the
+ // set of Endpoints it publishes to the mount table accordingly.
+ // The set of expected Settings received over the Stream is defined
+ // by the New<setting>Functions above. The Publisher is ignored if
+ // all of the addresses are specified.
+ //
+ // Listen may be called multiple times, but it must be called before
+ // Serve or ServeDispatcher.
+ //
+ // Listen returns the set of endpoints that can be used to reach
+ // this server. A single listen address in the ListenSpec can lead
+ // to multiple such endpoints (e.g. :0 on a device with multiple interfaces
+ // or that is being proxied). In the case where multiple listen addresses
+ // are used it is not possible to tell which listen address supports which
+ // Endpoint. If there is need to associate endpoints with specific
+ // listen addresses then Listen should be called separately for each one.
+ //
+ // Any non-nil value of error can be converted to a verror.E. If
+ // error is nil and at least one address was supplied in the ListenSpec
+ // then ListenEndpoints will include at least one Endpoint.
+ Listen(spec rpc.ListenSpec) ([]naming.Endpoint, error)
+
+ // Serve associates object with name by publishing the address of this
+ // server with the mount table under the supplied name and using
+ // authorizer to authorize access to it. RPCs invoked on the supplied
+ // name will be delivered to methods implemented by the supplied object.
+ //
+ // Reflection is used to match requests to the object's method set. As
+ // a special-case, if the object implements the Invoker interface, the
+ // Invoker is used to invoke methods directly, without reflection.
+ //
+ // If name is an empty string, no attempt will made to publish that
+ // name to a mount table.
+ //
+ // It is an error to call Serve if ServeDispatcher has already been
+ // called. It is also an error to call Serve multiple times.
+ // It is considered an error to call Listen after Serve.
+ Serve(name string, object interface{}, auth security.Authorizer) error
+
+ // ServeDispatcher associates dispatcher with the portion of the mount
+ // table's name space for which name is a prefix, by publishing the
+ // address of this dispatcher with the mount table under the supplied
+ // name.
+ //
+ // If name is an empty string, no attempt will made to publish that name
+ // to a mount table.
+ //
+ // RPCs invoked on the supplied name will be delivered to the supplied
+ // Dispatcher's Lookup method which will in turn return the object
+ // and security.Authorizer used to serve the actual RPC call.
+ // If name is an empty string, no attempt will made to publish that
+ // name to a mount table.
+ //
+ // It is an error to call ServeDispatcher if Serve has already been
+ // called. It is also an error to call ServeDispatcher multiple times.
+ // It is considered an error to call Listen after ServeDispatcher.
+ ServeDispatcher(name string, disp rpc.Dispatcher) error
+
+ rpc.Server
+}
+
// state for each requested listen address
type listenState struct {
protocol, address string
@@ -172,7 +240,7 @@
return s.state == stopping || s.state == stopped
}
-var _ rpc.DeprecatedServer = (*server)(nil)
+var _ DeprecatedServer = (*server)(nil)
func InternalNewServer(
ctx *context.T,
@@ -181,7 +249,7 @@
settingsPublisher *pubsub.Publisher,
settingsName string,
client rpc.Client,
- opts ...rpc.ServerOpt) (rpc.DeprecatedServer, error) {
+ opts ...rpc.ServerOpt) (DeprecatedServer, error) {
ctx, cancel := context.WithRootCancel(ctx)
ctx, _ = vtrace.WithNewSpan(ctx, "NewServer")
statsPrefix := naming.Join("rpc", "server", "routing-id", streamMgr.RoutingID().String())
diff --git a/runtime/internal/rt/mgmt.go b/runtime/internal/rt/mgmt.go
index 28c25cf..6295bb6 100644
--- a/runtime/internal/rt/mgmt.go
+++ b/runtime/internal/rt/mgmt.go
@@ -9,7 +9,6 @@
"v.io/v23"
"v.io/v23/context"
- "v.io/v23/naming"
"v.io/v23/options"
"v.io/v23/rpc"
"v.io/v23/verror"
@@ -45,8 +44,7 @@
// successfully started.
return handle.SetReady()
}
- listenSpec, err := getListenSpec(ctx, handle)
- if err != nil {
+ if ctx, err = setListenSpec(ctx, rt, handle); err != nil {
return err
}
var serverOpts []rpc.ServerOpt
@@ -57,19 +55,11 @@
serverBlessing := rt.GetPrincipal(ctx).BlessingStore().ForPeer(parentPeerPattern)
serverOpts = append(serverOpts, options.ServerBlessings{Blessings: serverBlessing})
}
- server, err := rt.NewServer(ctx, serverOpts...)
+ _, server, err := rt.WithNewServer(ctx, "", v23.GetAppCycle(ctx).Remote(), nil, serverOpts...)
if err != nil {
return err
}
- eps, err := server.Listen(*listenSpec)
- if err != nil {
- return err
- }
- if err := server.Serve("", v23.GetAppCycle(ctx).Remote(), nil); err != nil {
- server.Stop()
- return err
- }
- err = rt.callbackToParent(ctx, parentName, naming.JoinAddressName(eps[0].String(), ""))
+ err = rt.callbackToParent(ctx, parentName, server.Status().Endpoints[0].Name())
if err != nil {
server.Stop()
return err
@@ -77,7 +67,7 @@
return handle.SetReady()
}
-func getListenSpec(ctx *context.T, handle *exec.ChildHandle) (*rpc.ListenSpec, error) {
+func setListenSpec(ctx *context.T, rt *Runtime, handle *exec.ChildHandle) (*context.T, error) {
protocol, err := handle.Config.Get(mgmt.ProtocolConfigKey)
if err != nil {
return nil, err
@@ -93,7 +83,7 @@
if address == "" {
return nil, verror.New(errConfigKeyNotSet, ctx, mgmt.AddressConfigKey)
}
- return &rpc.ListenSpec{Addrs: rpc.ListenAddrs{{protocol, address}}}, nil
+ return rt.WithListenSpec(ctx, rpc.ListenSpec{Addrs: rpc.ListenAddrs{{protocol, address}}}), nil
}
func (rt *Runtime) callbackToParent(ctx *context.T, parentName, myName string) error {
diff --git a/runtime/internal/rt/runtime.go b/runtime/internal/rt/runtime.go
index c4f358e..59d9ed6 100644
--- a/runtime/internal/rt/runtime.go
+++ b/runtime/internal/rt/runtime.go
@@ -260,8 +260,7 @@
return inaming.NewEndpoint(ep)
}
-func (r *Runtime) NewServer(ctx *context.T, opts ...rpc.ServerOpt) (rpc.DeprecatedServer, error) {
- defer apilog.LogCallf(ctx, "opts...=%v", opts)(ctx, "") // gologcop: DO NOT EDIT, MUST BE FIRST STATEMENT
+func (r *Runtime) newServer(ctx *context.T, opts ...rpc.ServerOpt) (irpc.DeprecatedServer, error) {
// Create a new RoutingID (and StreamManager) for each server.
sm, err := newStreamManager(ctx)
if err != nil {
@@ -616,7 +615,7 @@
}
return newctx, s, nil
}
- s, err := r.NewServer(ctx, opts...)
+ s, err := r.newServer(ctx, opts...)
if err != nil {
return ctx, nil, err
}
@@ -647,7 +646,7 @@
return newctx, s, nil
}
- s, err := r.NewServer(ctx, opts...)
+ s, err := r.newServer(ctx, opts...)
if err != nil {
return ctx, nil, err
}
diff --git a/runtime/internal/rt/runtime_test.go b/runtime/internal/rt/runtime_test.go
index fc70a76..da50a92 100644
--- a/runtime/internal/rt/runtime_test.go
+++ b/runtime/internal/rt/runtime_test.go
@@ -10,67 +10,51 @@
"v.io/v23"
"v.io/v23/context"
"v.io/v23/naming"
- "v.io/v23/options"
+ "v.io/v23/rpc"
- "v.io/x/ref/lib/flags"
- "v.io/x/ref/runtime/internal/rt"
+ _ "v.io/x/ref/runtime/factories/generic"
"v.io/x/ref/services/debug/debuglib"
+ "v.io/x/ref/test"
"v.io/x/ref/test/testutil"
)
-// initForTest creates a context for use in a test.
-func initForTest(t *testing.T) (*rt.Runtime, *context.T, v23.Shutdown) {
- ctx, cancel := context.RootContext()
- r, ctx, shutdown, err := rt.Init(ctx, nil, nil, nil, nil, "", flags.RuntimeFlags{}, nil)
- if err != nil {
- t.Fatal(err)
- }
- if ctx, err = r.WithPrincipal(ctx, testutil.NewPrincipal("test-blessing")); err != nil {
- t.Fatal(err)
- }
- return r, ctx, func() {
- cancel()
- shutdown()
- }
-}
+type fakeServer struct{}
-func TestNewServer(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+func (*fakeServer) Foo(ctx *context.T, call rpc.ServerCall) error { return nil }
+
+func TestWithNewServer(t *testing.T) {
+ ctx, shutdown := test.V23Init()
defer shutdown()
- // Use options.SecurityNone to avoid calling back into the
- // v23 runtime, which is not setup in these tests.
- // TODO(cnicolaou): this can be undone when the security agent
- // no longer uses rpc as its communication mechanism.
- if s, err := r.NewServer(ctx, options.SecurityNone); err != nil || s == nil {
+ if _, s, err := v23.WithNewServer(ctx, "", &fakeServer{}, nil); err != nil || s == nil {
t.Fatalf("Could not create server: %v", err)
}
}
func TestPrincipal(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+ ctx, shutdown := test.V23Init()
defer shutdown()
p2 := testutil.NewPrincipal()
- c2, err := r.WithPrincipal(ctx, p2)
+ c2, err := v23.WithPrincipal(ctx, p2)
if err != nil {
t.Fatalf("Could not attach principal: %v", err)
}
if !c2.Initialized() {
t.Fatal("Got uninitialized context.")
}
- if p2 != r.GetPrincipal(c2) {
+ if p2 != v23.GetPrincipal(c2) {
t.Fatal("The new principal should be attached to the context, but it isn't")
}
}
func TestClient(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+ ctx, shutdown := test.V23Init()
defer shutdown()
- orig := r.GetClient(ctx)
+ orig := v23.GetClient(ctx)
- c2, client, err := r.WithNewClient(ctx)
+ c2, client, err := v23.WithNewClient(ctx)
if err != nil || client == nil {
t.Fatalf("Could not create client: %v", err)
}
@@ -80,20 +64,20 @@
if client == orig {
t.Fatal("Should have replaced the client but didn't")
}
- if client != r.GetClient(c2) {
+ if client != v23.GetClient(c2) {
t.Fatal("The new client should be attached to the context, but it isn't")
}
}
func TestNamespace(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+ ctx, shutdown := test.V23Init()
defer shutdown()
- orig := r.GetNamespace(ctx)
+ orig := v23.GetNamespace(ctx)
orig.CacheCtl(naming.DisableCache(true))
newroots := []string{"/newroot1", "/newroot2"}
- c2, ns, err := r.WithNewNamespace(ctx, newroots...)
+ c2, ns, err := v23.WithNewNamespace(ctx, newroots...)
if err != nil || ns == nil {
t.Fatalf("Could not create namespace: %v", err)
}
@@ -103,7 +87,7 @@
if ns == orig {
t.Fatal("Should have replaced the namespace but didn't")
}
- if ns != r.GetNamespace(c2) {
+ if ns != v23.GetNamespace(c2) {
t.Fatal("The new namespace should be attached to the context, but it isn't")
}
newrootmap := map[string]bool{"/newroot1": true, "/newroot2": true}
@@ -122,30 +106,30 @@
}
func TestBackgroundContext(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+ ctx, shutdown := test.V23Init()
defer shutdown()
- bgctx := r.GetBackgroundContext(ctx)
+ bgctx := v23.GetBackgroundContext(ctx)
if bgctx == ctx {
t.Error("The background context should not be the same as the context")
}
- bgctx2 := r.GetBackgroundContext(bgctx)
+ bgctx2 := v23.GetBackgroundContext(bgctx)
if bgctx != bgctx2 {
t.Error("Calling GetBackgroundContext a second time should return the same context.")
}
}
func TestReservedNameDispatcher(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+ ctx, shutdown := test.V23Init()
defer shutdown()
- oldDebugDisp := r.GetReservedNameDispatcher(ctx)
+ oldDebugDisp := v23.GetReservedNameDispatcher(ctx)
newDebugDisp := debuglib.NewDispatcher(nil)
- nctx := r.WithReservedNameDispatcher(ctx, newDebugDisp)
- debugDisp := r.GetReservedNameDispatcher(nctx)
+ nctx := v23.WithReservedNameDispatcher(ctx, newDebugDisp)
+ debugDisp := v23.GetReservedNameDispatcher(nctx)
if debugDisp != newDebugDisp || debugDisp == oldDebugDisp {
t.Error("WithNewDebugDispatcher didn't update the context properly")
@@ -154,24 +138,24 @@
}
func TestFlowManager(t *testing.T) {
- r, ctx, shutdown := initForTest(t)
+ ctx, shutdown := test.V23Init()
defer shutdown()
- oldman := r.ExperimentalGetFlowManager(ctx)
+ oldman := v23.ExperimentalGetFlowManager(ctx)
if oldman == nil {
t.Error("ExperimentalGetFlowManager should have returned a non-nil value")
}
if rid := oldman.RoutingID(); rid != naming.NullRoutingID {
t.Errorf("Initial flow.Manager should have NullRoutingID, got %v", rid)
}
- newctx, newman, err := r.ExperimentalWithNewFlowManager(ctx)
+ newctx, newman, err := v23.ExperimentalWithNewFlowManager(ctx)
if err != nil || newman == nil || newman == oldman {
t.Fatalf("Could not create flow manager: %v", err)
}
if !newctx.Initialized() {
t.Fatal("Got uninitialized context.")
}
- man := r.ExperimentalGetFlowManager(newctx)
+ man := v23.ExperimentalGetFlowManager(newctx)
if man != newman || man == oldman {
t.Error("ExperimentalWithNewFlowManager didn't update the context properly")
}
