Clean up changes

clientapi/routing/memberships.go

@@ -17,6 +17,7 @@ package routing
 import (
 	"encoding/json"
 	"net/http"
+	"regexp"
 
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/roomserver/api"
@@ -68,28 +69,43 @@ func GetMemberships(
 	_ *config.ClientAPI,
 	rsAPI api.RoomserverInternalAPI,
 ) util.JSONResponse {
-	// how would i unpack this nicely into
-	// type queryParams struct {
-	// 	membership string
-	// 	notMembership string
-	// }
 	membership := req.URL.Query().Get("membership")
 	notMembership := req.URL.Query().Get("not_membership")
-	membershipStatusFilter := []string{"join", "invite", "leave", "ban"}
+
-	if len(notMembership) > 0 {
+	regexpMembershipFilter, _ := regexp.Compile("join|invite|leave|ban")
-		for i, v := range membershipStatusFilter {
+	if membership != "" && !regexpMembershipFilter.MatchString(membership) {
-			if v == notMembership {
+		return util.JSONResponse{
-				membershipStatusFilter = append(membershipStatusFilter[:i], membershipStatusFilter[i+1:]...)
+			Code: http.StatusBadRequest,
 		}
 	}
-	} else if len(membership) > 0 {
+	if notMembership != "" && !regexpMembershipFilter.MatchString(notMembership) {
-		membershipStatusFilter = []string{membership}
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+		}
+	}
+
+	membershipFilter := []string{"join", "invite", "leave", "ban"}
+
+	if notMembership != "" {
+		if membership != "" && membership != notMembership {
+			for idx, val := range membershipFilter {
+				if val == notMembership {
+					membershipFilter = append(membershipFilter[:idx], membershipFilter[idx+1:]...)
+					break
+				}
+			}
+		}
+		// If membership and not_membership are both specified and they are the same,
+		// then we do no filtering at all because they create an OR conditional
 	} else {
-		membershipStatusFilter = []string{}
+		if membership != "" {
+			membershipFilter = []string{membership}
 		}
+	}
+
 	queryReq := api.QueryMembershipsForRoomRequest{
 		JoinedOnly:       joinedOnly,
-		MembershipStatusFilter: membershipStatusFilter,
+		MembershipFilter: membershipFilter,
 		RoomID:           roomID,
 		Sender:           device.UserID,
 	}

roomserver/api/query.go

@@ -149,7 +149,9 @@ type QueryMembershipForUserResponse struct {
 type QueryMembershipsForRoomRequest struct {
 	// If true, only returns the membership events of "join" membership
 	JoinedOnly bool `json:"joined_only"`
-	MembershipStatusFilter []string `json:"membership_status_filter"`
+	// The kinds of membership to filter for - returns the membership
+	// events with the appropriate filter (ie. join, invite, leave, ban)
+	MembershipFilter []string `json:"membership_filter"`
 	// ID of the room to fetch memberships from
 	RoomID string `json:"room_id"`
 	// Optional - ID of the user sending the request, for checking if the

roomserver/internal/helpers/helpers.go

@@ -59,7 +59,7 @@ func IsServerCurrentlyInRoom(ctx context.Context, db storage.Database, serverNam
 		return false, fmt.Errorf("unknown room %s", roomID)
 	}
 
-	eventNIDs, err := db.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, []string{"join"}, false)
+	eventNIDs, err := db.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, true, []string{}, false)
 	if err != nil {
 		return false, err
 	}

roomserver/internal/input/input_events.go

@@ -256,7 +256,7 @@ func (r *Inputer) calculateAndSetState(
 		stateAtEvent.Overwrite = true
 		var joinEventNIDs []types.EventNID
 		// Request join memberships only for local users only.
-		if joinEventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, roomInfo.RoomNID, []string{"join"}, true); err == nil {
+		if joinEventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, roomInfo.RoomNID, true, []string{}, true); err == nil {
 			// If we have no local users that are joined to the room then any state about
 			// the room that we have is quite possibly out of date. Therefore in that case
 			// we should overwrite it rather than merge it.

roomserver/internal/perform/perform_backfill.go

@@ -537,7 +537,7 @@ func joinEventsFromHistoryVisibility(
 	if err != nil {
 		return nil, err
 	}
-	joinEventNIDs, err := db.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, []string{"join"}, false)
+	joinEventNIDs, err := db.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, true, []string{}, false)
 	if err != nil {
 		return nil, err
 	}

roomserver/internal/query/query.go

@@ -251,7 +251,7 @@ func (r *Queryer) QueryMembershipsForRoom(
 	if request.Sender == "" {
 		var events []types.Event
 		var eventNIDs []types.EventNID
-		eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.MembershipStatusFilter, false)
+		eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.JoinedOnly, request.MembershipFilter, false)
 		if err != nil {
 			return fmt.Errorf("r.DB.GetMembershipEventNIDsForRoom: %w", err)
 		}
@@ -286,7 +286,7 @@ func (r *Queryer) QueryMembershipsForRoom(
 	var stateEntries []types.StateEntry
 	if stillInRoom {
 		var eventNIDs []types.EventNID
-		eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.MembershipStatusFilter, false)
+		eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.JoinedOnly, request.MembershipFilter, false)
 		if err != nil {
 			return err
 		}
@@ -328,7 +328,7 @@ func (r *Queryer) QueryServerJoinedToRoom(
 	}
 	response.RoomExists = true
 
-	eventNIDs, err := r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, []string{"join"}, false)
+	eventNIDs, err := r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, true, []string{}, false)
 	if err != nil {
 		return fmt.Errorf("r.DB.GetMembershipEventNIDsForRoom: %w", err)
 	}

roomserver/storage/interface.go

@@ -131,7 +131,7 @@ type Database interface {
 	// been members of a given room. Only lookup events of "join" membership if
 	// joinOnly is set to true.
 	// Returns an error if there was a problem talking to the database.
-	GetMembershipEventNIDsForRoom(ctx context.Context, roomNID types.RoomNID, membershipStatusFilter []string, localOnly bool) ([]types.EventNID, error)
+	GetMembershipEventNIDsForRoom(ctx context.Context, roomNID types.RoomNID, joinOnly bool, membershipFilter []string, localOnly bool) ([]types.EventNID, error)
 	// EventsFromIDs looks up the Events for a list of event IDs. Does not error if event was
 	// not found.
 	// Returns an error if the retrieval went wrong.

roomserver/storage/shared/storage.go

@@ -343,13 +343,20 @@ func (d *Database) GetMembership(ctx context.Context, roomNID types.RoomNID, req
 }
 
 func (d *Database) GetMembershipEventNIDsForRoom(
-	ctx context.Context, roomNID types.RoomNID, membershipStatusFilter []string, localOnly bool,
+	ctx context.Context, roomNID types.RoomNID, joinOnly bool, membershipFilter []string, localOnly bool,
 ) ([]types.EventNID, error) {
+	if joinOnly {
+		return d.MembershipTable.SelectMembershipsFromRoomAndMembership(
+			ctx, roomNID, tables.MembershipStateJoin, localOnly,
+		)
+	}
+
+	if len(membershipFilter) > 0 {
 		var eventNIDs []types.EventNID
 		var filteredEventNIDs []types.EventNID
 		var err error
-	if len(membershipStatusFilter) > 0 {
+		leaveOrBan := false
-		for _, filter := range membershipStatusFilter {
+		for _, filter := range membershipFilter {
 			if filter == "join" {
 				filteredEventNIDs, err = d.MembershipTable.SelectMembershipsFromRoomAndMembership(
 					ctx, roomNID, tables.MembershipStateJoin, localOnly,
@@ -358,7 +365,8 @@ func (d *Database) GetMembershipEventNIDsForRoom(
 					return eventNIDs, err
 				}
 				eventNIDs = append(eventNIDs, filteredEventNIDs...)
-			} else if filter == "invite" {
+			}
+			if filter == "invite" {
 				filteredEventNIDs, err = d.MembershipTable.SelectMembershipsFromRoomAndMembership(
 					ctx, roomNID, tables.MembershipStateInvite, localOnly,
 				)
@@ -366,7 +374,8 @@ func (d *Database) GetMembershipEventNIDsForRoom(
 					return eventNIDs, err
 				}
 				eventNIDs = append(eventNIDs, filteredEventNIDs...)
-			} else {
+			}
+			if (filter == "ban" || filter == "leave") && !leaveOrBan {
 				filteredEventNIDs, err = d.MembershipTable.SelectMembershipsFromRoomAndMembership(
 					ctx, roomNID, tables.MembershipStateLeaveOrBan, localOnly,
 				)
@@ -374,6 +383,7 @@ func (d *Database) GetMembershipEventNIDsForRoom(
 					return eventNIDs, err
 				}
 				eventNIDs = append(eventNIDs, filteredEventNIDs...)
+				leaveOrBan = true
 			}
 		}
 		return eventNIDs, err

sytest-whitelist

@@ -520,3 +520,4 @@ Inviting an AS-hosted user asks the AS server
 Can generate a openid access_token that can be exchanged for information about a user
 Invalid openid access tokens are rejected
 Requests to userinfo without access tokens are rejected
+Can get rooms/{roomId}/members at a given point