From f5963308e020644ec2f23bdf26dcfed15637ccea Mon Sep 17 00:00:00 2001
From: Neil Alexander <neilalexander@users.noreply.github.com>
Date: Mon, 4 May 2020 12:30:47 +0100
Subject: [PATCH] Use Membership, don't try more servers than needed

---
 federationapi/routing/join.go       | 16 ++++++----------
 roomserver/internal/perform_join.go | 18 ++++++------------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go
index e851cb086..c5a974065 100644
--- a/federationapi/routing/join.go
+++ b/federationapi/routing/join.go
@@ -15,7 +15,6 @@
 package routing
 
 import (
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"time"
@@ -131,6 +130,9 @@ func MakeJoin(
 }
 
 // SendJoin implements the /send_join API
+// TODO: Is there a way to break this function up in a way that actually
+// makes sense?
+// nolint:gocyclo
 func SendJoin(
 	httpReq *http.Request,
 	request *gomatrixserverlib.FederationRequest,
@@ -247,16 +249,10 @@ func SendJoin(
 	// there isn't much point in sending another join event into the room.
 	alreadyJoined := false
 	for _, se := range stateAndAuthChainResponse.StateEvents {
-		if se.Type() == gomatrixserverlib.MRoomMember {
+		if membership, merr := se.Membership(); merr == nil {
 			if se.StateKey() != nil && *se.StateKey() == *event.StateKey() {
-				var content map[string]interface{}
-				if err = json.Unmarshal(se.Content(), &content); err != nil {
-					continue
-				}
-				if membership, ok := content["membership"]; ok {
-					alreadyJoined = (membership == "join")
-					break
-				}
+				alreadyJoined = (membership == "join")
+				break
 			}
 		}
 	}
diff --git a/roomserver/internal/perform_join.go b/roomserver/internal/perform_join.go
index 67f7d3942..4b75ddca4 100644
--- a/roomserver/internal/perform_join.go
+++ b/roomserver/internal/perform_join.go
@@ -2,7 +2,6 @@ package internal
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -52,7 +51,7 @@ func (r *RoomserverInternalAPI) performJoinRoomByAlias(
 	// doesn't then we'll need to try a federated join.
 	var roomID string
 	if domain != r.Cfg.Matrix.ServerName {
-		// The alias isn't owned by us, so we will eed to try joining using
+		// The alias isn't owned by us, so we will need to try joining using
 		// a remote server.
 		dirReq := fsAPI.PerformDirectoryLookupRequest{
 			RoomAlias:  req.RoomIDOrAlias, // the room alias to lookup
@@ -144,16 +143,10 @@ func (r *RoomserverInternalAPI) performJoinRoomByID(
 		// a member of the room.
 		alreadyJoined := false
 		for _, se := range buildRes.StateEvents {
-			if se.Type() == gomatrixserverlib.MRoomMember {
-				if se.StateKey() != nil && *se.StateKey() == userID {
-					var content map[string]interface{}
-					if err = json.Unmarshal(se.Content(), &content); err != nil {
-						continue
-					}
-					if membership, ok := content["membership"]; ok {
-						alreadyJoined = (membership == "join")
-						break
-					}
+			if membership, merr := se.Membership(); merr == nil {
+				if se.StateKey() != nil && *se.StateKey() == *event.StateKey() {
+					alreadyJoined = (membership == "join")
+					break
 				}
 			}
 		}
@@ -205,6 +198,7 @@ func (r *RoomserverInternalAPI) performJoinRoomByID(
 				continue
 			}
 			joined = true
+			break
 		}
 
 		// If we didn't successfully join the room using any of the supplied