remove request/response structs from SetRoomAlias

clientapi/routing/directory.go

@@ -206,13 +206,8 @@ func SetLocalAlias(
 		}
 	}
 
-	queryReq := roomserverAPI.SetRoomAliasRequest{
-		SenderID: senderID,
-		RoomID:   r.RoomID,
-		Alias:    alias,
-	}
-	var queryRes roomserverAPI.SetRoomAliasResponse
-	if err := rsAPI.SetRoomAlias(req.Context(), &queryReq, &queryRes); err != nil {
+	aliasAlreadyExists, err := rsAPI.SetRoomAlias(req.Context(), senderID, *roomID, alias)
+	if err != nil {
 		util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.SetRoomAlias failed")
 		return util.JSONResponse{
 			Code: http.StatusInternalServerError,
@@ -220,7 +215,7 @@ func SetLocalAlias(
 		}
 	}
 
-	if queryRes.AliasExists {
+	if aliasAlreadyExists {
 		return util.JSONResponse{
 			Code: http.StatusConflict,
 			JSON: spec.Unknown("The alias " + alias + " already exists."),

roomserver/api/alias.go

@@ -20,22 +20,6 @@ import (
 	"github.com/matrix-org/gomatrixserverlib/spec"
 )
 
-// SetRoomAliasRequest is a request to SetRoomAlias
-type SetRoomAliasRequest struct {
-	// ID of the user setting the alias
-	SenderID spec.SenderID `json:"sender"`
-	// New alias for the room
-	Alias string `json:"alias"`
-	// The room ID the alias is referring to
-	RoomID string `json:"room_id"`
-}
-
-// SetRoomAliasResponse is a response to SetRoomAlias
-type SetRoomAliasResponse struct {
-	// Does the alias already refer to a room?
-	AliasExists bool `json:"alias_exists"`
-}
-
 // GetRoomIDForAliasRequest is a request to GetRoomIDForAlias
 type GetRoomIDForAliasRequest struct {
 	// Alias we want to lookup

roomserver/api/api.go

@@ -237,7 +237,11 @@ type ClientRoomserverAPI interface {
 	PerformPublish(ctx context.Context, req *PerformPublishRequest) error
 	// PerformForget forgets a rooms history for a specific user
 	PerformForget(ctx context.Context, req *PerformForgetRequest, resp *PerformForgetResponse) error
-	SetRoomAlias(ctx context.Context, req *SetRoomAliasRequest, res *SetRoomAliasResponse) error
+	// Sets a room alias, as provided sender, pointing to the provided room ID.
+	//
+	// If err is nil, then the returned boolean indicates if the alias is already in use.
+	// If true, then the alias has not been set to the provided room, as it already in use.
+	SetRoomAlias(ctx context.Context, senderID spec.SenderID, roomID spec.RoomID, alias string) (aliasAlreadyExists bool, err error)
 	RemoveRoomAlias(ctx context.Context, req *RemoveRoomAliasRequest, res *RemoveRoomAliasResponse) error
 	SigningIdentityFor(ctx context.Context, roomID spec.RoomID, senderID spec.UserID) (fclient.SigningIdentity, error)
 }

roomserver/internal/alias.go

@@ -35,27 +35,27 @@ import (
 // SetRoomAlias implements alias.RoomserverInternalAPI
 func (r *RoomserverInternalAPI) SetRoomAlias(
 	ctx context.Context,
-	request *api.SetRoomAliasRequest,
-	response *api.SetRoomAliasResponse,
-) error {
+	senderID spec.SenderID,
+	roomID spec.RoomID,
+	alias string,
+) (aliasAlreadyUsed bool, err error) {
 	// Check if the alias isn't already referring to a room
-	roomID, err := r.DB.GetRoomIDForAlias(ctx, request.Alias)
+	existingRoomID, err := r.DB.GetRoomIDForAlias(ctx, alias)
 	if err != nil {
-		return err
+		return false, err
 	}
-	if len(roomID) > 0 {
+
+	if len(existingRoomID) > 0 {
 		// If the alias already exists, stop the process
-		response.AliasExists = true
-		return nil
+		return true, nil
 	}
-	response.AliasExists = false
 
 	// Save the new alias
-	if err := r.DB.SetRoomAlias(ctx, request.Alias, request.RoomID, string(request.SenderID)); err != nil {
-		return err
+	if err := r.DB.SetRoomAlias(ctx, alias, roomID.String(), string(senderID)); err != nil {
+		return false, err
 	}
 
-	return nil
+	return false, nil
 }
 
 // GetRoomIDForAlias implements alias.RoomserverInternalAPI

roomserver/internal/perform/perform_create_room.go

@@ -433,23 +433,16 @@ func (c *Creator) PerformCreateRoom(ctx context.Context, userID spec.UserID, roo
 	// from creating the room but still failing due to the alias having already
 	// been taken.
 	if roomAlias != "" {
-		aliasReq := api.SetRoomAliasRequest{
-			SenderID: senderID,
-			Alias:    roomAlias,
-			RoomID:   roomID.String(),
-		}
-
-		var aliasResp api.SetRoomAliasResponse
-		err = c.RSAPI.SetRoomAlias(ctx, &aliasReq, &aliasResp)
-		if err != nil {
-			util.GetLogger(ctx).WithError(err).Error("aliasAPI.SetRoomAlias failed")
+		aliasAlreadyExists, aliasErr := c.RSAPI.SetRoomAlias(ctx, senderID, roomID, roomAlias)
+		if aliasErr != nil {
+			util.GetLogger(ctx).WithError(aliasErr).Error("aliasAPI.SetRoomAlias failed")
 			return "", &util.JSONResponse{
 				Code: http.StatusInternalServerError,
 				JSON: spec.InternalServerError{},
 			}
 		}
 
-		if aliasResp.AliasExists {
+		if aliasAlreadyExists {
 			return "", &util.JSONResponse{
 				Code: http.StatusBadRequest,
 				JSON: spec.RoomInUse("Room alias already exists."),

roomserver/internal/perform/perform_upgrade.go

@@ -181,6 +181,12 @@ func moveLocalAliases(ctx context.Context,
 		return fmt.Errorf("Failed to get old room aliases: %w", err)
 	}
 
+	// TODO: this should be spec.RoomID further up the call stack
+	parsedNewRoomID, err := spec.NewRoomID(newRoomID)
+	if err != nil {
+		return err
+	}
+
 	for _, alias := range aliasRes.Aliases {
 		removeAliasReq := api.RemoveRoomAliasRequest{SenderID: senderID, Alias: alias}
 		removeAliasRes := api.RemoveRoomAliasResponse{}
@@ -188,10 +194,11 @@ func moveLocalAliases(ctx context.Context,
 			return fmt.Errorf("Failed to remove old room alias: %w", err)
 		}
 
-		setAliasReq := api.SetRoomAliasRequest{SenderID: senderID, Alias: alias, RoomID: newRoomID}
-		setAliasRes := api.SetRoomAliasResponse{}
-		if err = URSAPI.SetRoomAlias(ctx, &setAliasReq, &setAliasRes); err != nil {
+		aliasAlreadyExists, err := URSAPI.SetRoomAlias(ctx, senderID, *parsedNewRoomID, alias)
+		if err != nil {
 			return fmt.Errorf("Failed to set new room alias: %w", err)
+		} else if aliasAlreadyExists {
+			return fmt.Errorf("Failed to set new room alias: alias exists when it should have just been removed")
 		}
 	}
 	return nil

roomserver/roomserver_test.go

@@ -227,6 +227,11 @@ func TestPurgeRoom(t *testing.T) {
 	bob := test.NewUser(t)
 	room := test.NewRoom(t, alice, test.RoomPreset(test.PresetTrustedPrivateChat))
 
+	roomID, err := spec.NewRoomID(room.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	// Invite Bob
 	inviteEvent := room.CreateAndInsert(t, alice, spec.MRoomMember, map[string]interface{}{
 		"membership": "invite",
@@ -274,8 +279,7 @@ func TestPurgeRoom(t *testing.T) {
 		if !isPublished {
 			t.Fatalf("room should be published before purging")
 		}
-		aliasResp := &api.SetRoomAliasResponse{}
-		if err = rsAPI.SetRoomAlias(ctx, &api.SetRoomAliasRequest{RoomID: room.ID, Alias: "myalias", SenderID: spec.SenderID(alice.ID)}, aliasResp); err != nil {
+		if _, err = rsAPI.SetRoomAlias(ctx, spec.SenderID(alice.ID), *roomID, "myalias"); err != nil {
 			t.Fatal(err)
 		}
 		// check the alias is actually there
@@ -929,14 +933,17 @@ func TestUpgrade(t *testing.T) {
 			upgradeUser: alice.ID,
 			roomFunc: func(rsAPI api.RoomserverInternalAPI) string {
 				r := test.NewRoom(t, alice)
+				roomID, err := spec.NewRoomID(r.ID)
+				if err != nil {
+					t.Fatal(err)
+				}
 				if err := api.SendEvents(ctx, rsAPI, api.KindNew, r.Events(), "test", "test", "test", nil, false); err != nil {
 					t.Errorf("failed to send events: %v", err)
 				}
 
-				if err := rsAPI.SetRoomAlias(ctx, &api.SetRoomAliasRequest{
-					RoomID: r.ID,
-					Alias:  "#myroomalias:test",
-				}, &api.SetRoomAliasResponse{}); err != nil {
+				if _, err := rsAPI.SetRoomAlias(ctx, spec.SenderID(alice.ID),
+					*roomID,
+					"#myroomalias:test"); err != nil {
 					t.Fatal(err)
 				}