From 85ee99a1b24cd605d8cfa157088bc1919a20f6d2 Mon Sep 17 00:00:00 2001
From: Henry <111202+cryptix@users.noreply.github.com>
Date: Fri, 21 May 2021 15:52:04 +0200
Subject: [PATCH] remove room.announce and room.leave from muxrpc (#223)

* remove room.announce and room.leave from muxrpc

those were confusion and implied some use beyond just opening
tunnel.endpoints/room.attendants (which implicitly does what
tunnel.announce does).

updates #89
---
 muxrpc/handlers/tunnel/server/attendants.go | 12 +++++++++---
 muxrpc/handlers/tunnel/server/plugin.go     |  3 ---
 muxrpc/handlers/tunnel/server/state.go      |  8 ++++----
 muxrpc/test/go/attendants_test.go           | 13 -------------
 muxrpc/test/go/roomstate_test.go            |  8 ++------
 muxrpc/test/go/utils_test.go                | 11 ++++++++---
 roomsrv/manifest.go                         |  2 --
 7 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/muxrpc/handlers/tunnel/server/attendants.go b/muxrpc/handlers/tunnel/server/attendants.go
index 83b8090..4ea13e6 100644
--- a/muxrpc/handlers/tunnel/server/attendants.go
+++ b/muxrpc/handlers/tunnel/server/attendants.go
@@ -28,9 +28,6 @@ type AttendantsInitialState struct {
 }
 
 func (h *Handler) attendants(ctx context.Context, req *muxrpc.Request, snk *muxrpc.ByteSink) error {
-	// for future updates
-	toPeer := newAttendantsEncoder(snk)
-	h.state.RegisterAttendantsUpdates(toPeer)
 
 	// get public key from the calling peer
 	peer, err := network.GetFeedRefFromAddr(req.RemoteAddr())
@@ -49,6 +46,11 @@ func (h *Handler) attendants(ctx context.Context, req *muxrpc.Request, snk *muxr
 			return fmt.Errorf("external user are not allowed to enumerate members")
 		}
 	}
+
+	// add peer to the state
+	h.state.AddEndpoint(*peer, req.Endpoint())
+
+	// send the current state
 	err = json.NewEncoder(snk).Encode(AttendantsInitialState{
 		Type: "state",
 		IDs:  h.state.ListAsRefs(),
@@ -57,6 +59,10 @@ func (h *Handler) attendants(ctx context.Context, req *muxrpc.Request, snk *muxr
 		return err
 	}
 
+	// register for future updates
+	toPeer := newAttendantsEncoder(snk)
+	h.state.RegisterAttendantsUpdates(toPeer)
+
 	return nil
 }
 
diff --git a/muxrpc/handlers/tunnel/server/plugin.go b/muxrpc/handlers/tunnel/server/plugin.go
index ef3f721..84a3069 100644
--- a/muxrpc/handlers/tunnel/server/plugin.go
+++ b/muxrpc/handlers/tunnel/server/plugin.go
@@ -56,9 +56,6 @@ func (h *Handler) RegisterRoom(mux typemux.HandlerMux) {
 	mux.RegisterAsync(append(namespace, "metadata"), typemux.AsyncFunc(h.metadata))
 	mux.RegisterAsync(append(namespace, "ping"), typemux.AsyncFunc(h.ping))
 
-	mux.RegisterAsync(append(namespace, "announce"), typemux.AsyncFunc(h.announce))
-	mux.RegisterAsync(append(namespace, "leave"), typemux.AsyncFunc(h.leave))
-
 	mux.RegisterSource(append(namespace, "attendants"), typemux.SourceFunc(h.attendants))
 
 	mux.RegisterDuplex(append(namespace, "connect"), connectHandler{
diff --git a/muxrpc/handlers/tunnel/server/state.go b/muxrpc/handlers/tunnel/server/state.go
index de017ca..1979c86 100644
--- a/muxrpc/handlers/tunnel/server/state.go
+++ b/muxrpc/handlers/tunnel/server/state.go
@@ -105,10 +105,6 @@ func (h *Handler) leave(_ context.Context, req *muxrpc.Request) (interface{}, er
 }
 
 func (h *Handler) endpoints(ctx context.Context, req *muxrpc.Request, snk *muxrpc.ByteSink) error {
-	toPeer := newEndpointsForwarder(snk)
-
-	// for future updates
-	h.state.RegisterLegacyEndpoints(toPeer)
 
 	// get public key from the calling peer
 	peer, err := network.GetFeedRefFromAddr(req.RemoteAddr())
@@ -131,6 +127,10 @@ func (h *Handler) endpoints(ctx context.Context, req *muxrpc.Request, snk *muxrp
 		}
 	}
 
+	// for future updates
+	toPeer := newEndpointsForwarder(snk)
+	h.state.RegisterLegacyEndpoints(toPeer)
+
 	// add the peer to the room state if they arent already
 	h.state.AlreadyAdded(*peer, req.Endpoint())
 
diff --git a/muxrpc/test/go/attendants_test.go b/muxrpc/test/go/attendants_test.go
index ace0e09..ba8acce 100644
--- a/muxrpc/test/go/attendants_test.go
+++ b/muxrpc/test/go/attendants_test.go
@@ -43,11 +43,6 @@ func TestRoomAttendants(t *testing.T) {
 
 	// start with carl
 	// ===============
-	var ok bool
-	err := carl.Async(ctx, &ok, muxrpc.TypeJSON, muxrpc.Method{"room", "announce"})
-	r.NoError(err)
-	a.True(ok, "announce should be fine")
-
 	carlsSource, err := carl.Source(ctx, muxrpc.TypeJSON, muxrpc.Method{"room", "attendants"})
 	r.NoError(err)
 	t.Log("carl opened attendants")
@@ -68,10 +63,6 @@ func TestRoomAttendants(t *testing.T) {
 
 	// let alf join the room
 	// =====================
-	err = alf.Async(ctx, &ok, muxrpc.TypeJSON, muxrpc.Method{"room", "announce"})
-	r.NoError(err)
-	a.True(ok, "announce should be fine")
-
 	alfsSource, err := alf.Source(ctx, muxrpc.TypeJSON, muxrpc.Method{"room", "attendants"})
 	r.NoError(err)
 
@@ -96,10 +87,6 @@ func TestRoomAttendants(t *testing.T) {
 	a.True(seen, "carl saw alf")
 
 	// let bre join the room
-	err = bre.Async(ctx, &ok, muxrpc.TypeJSON, muxrpc.Method{"room", "announce"})
-	r.NoError(err)
-	a.True(ok, "announce should be fine")
-
 	bresSource, err := bre.Source(ctx, muxrpc.TypeJSON, muxrpc.Method{"room", "attendants"})
 	r.NoError(err)
 
diff --git a/muxrpc/test/go/roomstate_test.go b/muxrpc/test/go/roomstate_test.go
index 6878810..46e4c5d 100644
--- a/muxrpc/test/go/roomstate_test.go
+++ b/muxrpc/test/go/roomstate_test.go
@@ -40,13 +40,8 @@ func TestStaleMembers(t *testing.T) {
 	srh := ts.makeTestClient("srh") // https://en.wikipedia.org/wiki/Sarah
 
 	// announce srh so that tal could connect
-	var ok bool
-	err := srh.Async(ctx, &ok, muxrpc.TypeJSON, muxrpc.Method{"room", "announce"})
-	r.NoError(err)
-	r.True(ok)
 
 	_, has := ts.srv.StateManager.Has(srh.feed)
-	r.True(has, "srh should be connected")
 
 	// shut down srh
 	srh.Terminate()
@@ -82,7 +77,8 @@ func TestStaleMembers(t *testing.T) {
 	time.Sleep(1 * time.Second) // let server respond
 
 	// announce srh so that tal can connect
-	err = srh.Async(ctx, &ok, muxrpc.TypeJSON, muxrpc.Method{"room", "announce"})
+	var ok bool
+	err = srh.Async(ctx, &ok, muxrpc.TypeJSON, muxrpc.Method{"tunnel", "announce"})
 	r.NoError(err)
 	r.True(ok)
 	t.Log("announced srh again")
diff --git a/muxrpc/test/go/utils_test.go b/muxrpc/test/go/utils_test.go
index 68ff548..d824878 100644
--- a/muxrpc/test/go/utils_test.go
+++ b/muxrpc/test/go/utils_test.go
@@ -149,9 +149,13 @@ type testClient struct {
 	mockedHandler *muxrpc.FakeHandler
 }
 
+var clientNo = 0
+
 func (ts *testSession) makeTestClient(name string) testClient {
 	r := require.New(ts.t)
 
+	clientNo++
+
 	// create a fresh keypairs for the clients
 	client, has := ts.clientKeys[name]
 	if !has {
@@ -186,9 +190,10 @@ func (ts *testSession) makeTestClient(name string) testClient {
 	authedConn, err := netwrap.Dial(tcpAddr, clientSHS.ConnWrapper(ts.srv.Whoami().PubKey()))
 	r.NoError(err)
 
-	// testPath := filepath.Join("testrun", ts.t.Name())
-	// debugConn := debug.Dump(filepath.Join(testPath, "client-"+name), authedConn)
-	pkr := muxrpc.NewPacker(authedConn)
+	testPath := filepath.Join("testrun", ts.t.Name())
+	dbgPath := filepath.Join(testPath, fmt.Sprintf("client-%d-%s", clientNo, name))
+	dbgConn := debug.Dump(dbgPath, authedConn)
+	pkr := muxrpc.NewPacker(dbgConn)
 
 	var muxMock = new(muxrpc.FakeHandler)
 	wsEndpoint := muxrpc.Handle(pkr, muxMock,
diff --git a/roomsrv/manifest.go b/roomsrv/manifest.go
index f56f664..833c345 100644
--- a/roomsrv/manifest.go
+++ b/roomsrv/manifest.go
@@ -40,8 +40,6 @@ const manifest manifestHandler = `
 		"registerAlias": "async",
 		"revokeAlias": "async",
 
-		"announce": "sync",
-		"leave": "sync",
 		"connect": "duplex",
 		"attendants": "source",
 		"metadata": "async",