From 3205b9212d76f61301efae3c554bf9c3fbfc94c8 Mon Sep 17 00:00:00 2001 From: oliverpool <3864879+oliverpool@users.noreply.github.com> Date: Tue, 25 Aug 2020 22:04:35 +0200 Subject: [PATCH] [readability] use event.StateKeyEquals where relevant and minor for-loop refactoring (#1339) * use event.StateKeyEquals where relevant Signed-off-by: Olivier Charvin * use userID --- clientapi/routing/getevent.go | 23 +++++------ clientapi/routing/membership.go | 11 ++++-- clientapi/routing/sendtyping.go | 46 +++++----------------- currentstateserver/consumers/roomserver.go | 6 +-- federationapi/routing/join.go | 11 +++--- federationapi/routing/leave.go | 19 ++++----- roomserver/internal/perform_backfill.go | 2 +- roomserver/internal/perform_join.go | 9 +++-- syncapi/consumers/roomserver.go | 3 -- syncapi/storage/shared/syncserver.go | 14 +++---- 10 files changed, 60 insertions(+), 84 deletions(-) diff --git a/clientapi/routing/getevent.go b/clientapi/routing/getevent.go index c74509f0..18b96e1e 100644 --- a/clientapi/routing/getevent.go +++ b/clientapi/routing/getevent.go @@ -104,17 +104,18 @@ func GetEvent( } for _, stateEvent := range stateResp.StateEvents { - if stateEvent.StateKeyEquals(r.device.UserID) { - membership, err := stateEvent.Membership() - if err != nil { - util.GetLogger(req.Context()).WithError(err).Error("stateEvent.Membership failed") - return jsonerror.InternalServerError() - } - if membership == gomatrixserverlib.Join { - return util.JSONResponse{ - Code: http.StatusOK, - JSON: gomatrixserverlib.ToClientEvent(r.requestedEvent, gomatrixserverlib.FormatAll), - } + if !stateEvent.StateKeyEquals(device.UserID) { + continue + } + membership, err := stateEvent.Membership() + if err != nil { + util.GetLogger(req.Context()).WithError(err).Error("stateEvent.Membership failed") + return jsonerror.InternalServerError() + } + if membership == gomatrixserverlib.Join { + return util.JSONResponse{ + Code: http.StatusOK, + JSON: gomatrixserverlib.ToClientEvent(r.requestedEvent, gomatrixserverlib.FormatAll), } } } diff --git a/clientapi/routing/membership.go b/clientapi/routing/membership.go index 37fafa5a..5d635c01 100644 --- a/clientapi/routing/membership.go +++ b/clientapi/routing/membership.go @@ -388,15 +388,20 @@ func checkMemberInRoom(ctx context.Context, stateAPI currentstateAPI.CurrentStat e := jsonerror.InternalServerError() return &e } - ev, ok := membershipRes.StateEvents[tuple] - if !ok { + ev := membershipRes.StateEvents[tuple] + if ev == nil { return &util.JSONResponse{ Code: http.StatusForbidden, JSON: jsonerror.Forbidden("user does not belong to room"), } } membership, err := ev.Membership() - if err != nil || membership != "join" { + if err != nil { + util.GetLogger(ctx).WithError(err).Error("Member event isn't valid") + e := jsonerror.InternalServerError() + return &e + } + if membership != gomatrixserverlib.Join { return &util.JSONResponse{ Code: http.StatusForbidden, JSON: jsonerror.Forbidden("user does not belong to room"), diff --git a/clientapi/routing/sendtyping.go b/clientapi/routing/sendtyping.go index 54a82286..e4b5b7a3 100644 --- a/clientapi/routing/sendtyping.go +++ b/clientapi/routing/sendtyping.go @@ -21,7 +21,6 @@ import ( "github.com/matrix-org/dendrite/eduserver/api" userapi "github.com/matrix-org/dendrite/userapi/api" "github.com/matrix-org/dendrite/userapi/storage/accounts" - "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" ) @@ -46,46 +45,19 @@ func SendTyping( } // Verify that the user is a member of this room - tuple := gomatrixserverlib.StateKeyTuple{ - EventType: gomatrixserverlib.MRoomMember, - StateKey: userID, - } - var res currentstateAPI.QueryCurrentStateResponse - err := stateAPI.QueryCurrentState(req.Context(), ¤tstateAPI.QueryCurrentStateRequest{ - RoomID: roomID, - StateTuples: []gomatrixserverlib.StateKeyTuple{tuple}, - }, &res) - if err != nil { - util.GetLogger(req.Context()).WithError(err).Error("QueryCurrentState failed") - return jsonerror.InternalServerError() - } - ev := res.StateEvents[tuple] - if ev == nil { - return util.JSONResponse{ - Code: http.StatusForbidden, - JSON: jsonerror.Forbidden("User not in this room"), - } - } - membership, err := ev.Membership() - if err != nil { - util.GetLogger(req.Context()).WithError(err).Error("Member event isn't valid") - return jsonerror.InternalServerError() - } - if membership != gomatrixserverlib.Join { - return util.JSONResponse{ - Code: http.StatusForbidden, - JSON: jsonerror.Forbidden("User not in this room"), - } - } - - // parse the incoming http request - var r typingContentJSON - resErr := httputil.UnmarshalJSONRequest(req, &r) + resErr := checkMemberInRoom(req.Context(), stateAPI, userID, roomID) if resErr != nil { return *resErr } - if err = api.SendTyping( + // parse the incoming http request + var r typingContentJSON + resErr = httputil.UnmarshalJSONRequest(req, &r) + if resErr != nil { + return *resErr + } + + if err := api.SendTyping( req.Context(), eduAPI, userID, roomID, r.Typing, r.Timeout, ); err != nil { util.GetLogger(req.Context()).WithError(err).Error("eduProducer.Send failed") diff --git a/currentstateserver/consumers/roomserver.go b/currentstateserver/consumers/roomserver.go index 492cf621..23495b24 100644 --- a/currentstateserver/consumers/roomserver.go +++ b/currentstateserver/consumers/roomserver.go @@ -126,10 +126,8 @@ func (c *OutputRoomEventConsumer) Start() error { } func (c *OutputRoomEventConsumer) updateStateEvent(event gomatrixserverlib.HeaderedEvent) (gomatrixserverlib.HeaderedEvent, error) { - var stateKey string - if event.StateKey() == nil { - stateKey = "" - } else { + stateKey := "" + if event.StateKey() != nil { stateKey = *event.StateKey() } diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 4874f4d1..ffdadd52 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -165,7 +165,7 @@ func SendJoin( } // Check that a state key is provided. - if event.StateKey() == nil || (event.StateKey() != nil && *event.StateKey() == "") { + if event.StateKey() == nil || event.StateKeyEquals("") { return util.JSONResponse{ Code: http.StatusBadRequest, JSON: jsonerror.BadJSON( @@ -253,11 +253,12 @@ func SendJoin( // there isn't much point in sending another join event into the room. alreadyJoined := false for _, se := range stateAndAuthChainResponse.StateEvents { + if !se.StateKeyEquals(*event.StateKey()) { + continue + } if membership, merr := se.Membership(); merr == nil { - if se.StateKey() != nil && *se.StateKey() == *event.StateKey() { - alreadyJoined = (membership == "join") - break - } + alreadyJoined = (membership == gomatrixserverlib.Join) + break } } diff --git a/federationapi/routing/leave.go b/federationapi/routing/leave.go index d265886b..d2fbfc71 100644 --- a/federationapi/routing/leave.go +++ b/federationapi/routing/leave.go @@ -81,15 +81,16 @@ func MakeLeave( // event. This means that /send_leave will be a no-op, which helps // to reject invites multiple times - hopefully. for _, state := range queryRes.StateEvents { - if state.Type() == gomatrixserverlib.MRoomMember && state.StateKeyEquals(userID) { - if mem, merr := state.Membership(); merr == nil && mem == gomatrixserverlib.Leave { - return util.JSONResponse{ - Code: http.StatusOK, - JSON: map[string]interface{}{ - "room_version": event.RoomVersion, - "event": state, - }, - } + if !state.StateKeyEquals(userID) { + continue + } + if mem, merr := state.Membership(); merr == nil && mem == gomatrixserverlib.Leave { + return util.JSONResponse{ + Code: http.StatusOK, + JSON: map[string]interface{}{ + "room_version": event.RoomVersion, + "event": state, + }, } } } diff --git a/roomserver/internal/perform_backfill.go b/roomserver/internal/perform_backfill.go index 23ae9455..fb8fd7e8 100644 --- a/roomserver/internal/perform_backfill.go +++ b/roomserver/internal/perform_backfill.go @@ -104,7 +104,7 @@ func (b *backfillRequester) calculateNewStateIDs(targetEvent, prevEvent gomatrix continue } // The state IDs BEFORE the target event are the state IDs BEFORE the prev_event PLUS the prev_event itself - if ev.Type() == prevEvent.Type() && ev.StateKey() != nil && *ev.StateKey() == *prevEvent.StateKey() { + if ev.Type() == prevEvent.Type() && ev.StateKeyEquals(*prevEvent.StateKey()) { newStateIDs[i] = prevEvent.EventID() foundEvent = true break diff --git a/roomserver/internal/perform_join.go b/roomserver/internal/perform_join.go index 73ea008d..b92a6663 100644 --- a/roomserver/internal/perform_join.go +++ b/roomserver/internal/perform_join.go @@ -204,11 +204,12 @@ func (r *RoomserverInternalAPI) performJoinRoomByID( // a member of the room. alreadyJoined := false for _, se := range buildRes.StateEvents { + if !se.StateKeyEquals(userID) { + continue + } if membership, merr := se.Membership(); merr == nil { - if se.StateKey() != nil && *se.StateKey() == *event.StateKey() { - alreadyJoined = (membership == gomatrixserverlib.Join) - break - } + alreadyJoined = (membership == gomatrixserverlib.Join) + break } } diff --git a/syncapi/consumers/roomserver.go b/syncapi/consumers/roomserver.go index 1af58837..bf231d09 100644 --- a/syncapi/consumers/roomserver.go +++ b/syncapi/consumers/roomserver.go @@ -169,9 +169,6 @@ func (s *OutputRoomEventConsumer) onNewRoomEvent( } func (s *OutputRoomEventConsumer) notifyKeyChanges(ev *gomatrixserverlib.HeaderedEvent) { - if ev.Type() != gomatrixserverlib.MRoomMember || ev.StateKey() == nil { - return - } membership, err := ev.Membership() if err != nil { return diff --git a/syncapi/storage/shared/syncserver.go b/syncapi/storage/shared/syncserver.go index 4031dc74..ad0c1d99 100644 --- a/syncapi/storage/shared/syncserver.go +++ b/syncapi/storage/shared/syncserver.go @@ -1216,14 +1216,14 @@ func removeDuplicates(stateEvents, recentEvents []gomatrixserverlib.HeaderedEven // getMembershipFromEvent returns the value of content.membership iff the event is a state event // with type 'm.room.member' and state_key of userID. Otherwise, an empty string is returned. func getMembershipFromEvent(ev *gomatrixserverlib.Event, userID string) string { - if ev.Type() == "m.room.member" && ev.StateKeyEquals(userID) { - membership, err := ev.Membership() - if err != nil { - return "" - } - return membership + if ev.Type() != "m.room.member" || !ev.StateKeyEquals(userID) { + return "" } - return "" + membership, err := ev.Membership() + if err != nil { + return "" + } + return membership } type stateDelta struct {