From 3d890f3df9523e537276e0c20a35af5e70348aaf Mon Sep 17 00:00:00 2001 From: Serra Allgood Date: Mon, 3 Jun 2019 08:04:36 -0700 Subject: [PATCH] roomserver/alias: Minor fixes, consolidate test cases Signed-off-by: Serra Allgood --- roomserver/alias/alias.go | 19 +---- roomserver/alias/alias_test.go | 131 ++++++++++++++++++--------------- 2 files changed, 73 insertions(+), 77 deletions(-) diff --git a/roomserver/alias/alias.go b/roomserver/alias/alias.go index 1b07f53eb..6a34aacdd 100644 --- a/roomserver/alias/alias.go +++ b/roomserver/alias/alias.go @@ -84,19 +84,6 @@ func (r *RoomserverAliasAPI) SetRoomAlias( return r.sendUpdatedAliasesEvent(context.TODO(), request.UserID, request.RoomID) } -func getRoomIDFromDB( - ctx context.Context, - db RoomserverAliasAPIDatabase, - request *roomserverAPI.GetRoomIDForAliasRequest, -) (string, error) { - roomID, err := db.GetRoomIDForAlias(ctx, request.Alias) - if err != nil { - return "", err - } - - return roomID, nil -} - // GetRoomIDForAlias implements alias.RoomserverAliasAPI func (r *RoomserverAliasAPI) GetRoomIDForAlias( ctx context.Context, @@ -104,13 +91,13 @@ func (r *RoomserverAliasAPI) GetRoomIDForAlias( response *roomserverAPI.GetRoomIDForAliasResponse, ) error { // Look up the room ID in the database - roomID, err := getRoomIDFromDB(ctx, r.DB, request) + roomID, err := r.DB.GetRoomIDForAlias(ctx, request.Alias) if err != nil { return err } if roomID == "" { - // No rooms found locally, try our application services by making a call to + // No room found locally, try our application services by making a call to // the appservice component aliasReq := appserviceAPI.RoomAliasExistsRequest{Alias: request.Alias} var aliasResp appserviceAPI.RoomAliasExistsResponse @@ -119,7 +106,7 @@ func (r *RoomserverAliasAPI) GetRoomIDForAlias( } if aliasResp.AliasExists { - roomID, err = getRoomIDFromDB(ctx, r.DB, request) + roomID, err = r.DB.GetRoomIDForAlias(ctx, request.Alias) if err != nil { return err } diff --git a/roomserver/alias/alias_test.go b/roomserver/alias/alias_test.go index 3ce8d89ef..4b9ca022d 100644 --- a/roomserver/alias/alias_test.go +++ b/roomserver/alias/alias_test.go @@ -25,11 +25,11 @@ import ( ) type MockRoomserverAliasAPIDatabase struct { - mode string - attempts int + mode string + attempts int } -// Those methods can be essentially noop +// These methods can be essentially noop func (db MockRoomserverAliasAPIDatabase) SetRoomAlias(ctx context.Context, alias string, roomID string) error { return nil } @@ -109,12 +109,61 @@ func TestGetRoomIDForAlias(t *testing.T) { request *roomserverAPI.GetRoomIDForAliasRequest response *roomserverAPI.GetRoomIDForAliasResponse } - args := arguments{ context.Background(), &roomserverAPI.GetRoomIDForAliasRequest{}, &roomserverAPI.GetRoomIDForAliasResponse{}, } + type testCase struct { + name string + dbMode string + queryMode string + wantError bool + errorMsg string + want string + } + tt := []testCase{ + { + "found local alias", + "found", + "error", + false, + "", + "123", + }, + { + "found appservice alias", + "emptyFound", + "found", + false, + "", + "123", + }, + { + "error returned from DB", + "error", + "", + true, + "GetRoomIDForAlias", + "", + }, + { + "error returned from appserviceAPI", + "empty", + "error", + true, + "RoomAliasExists", + "", + }, + { + "no errors but no alias", + "empty", + "empty", + false, + "", + "", + }, + } setup := func(dbMode, queryMode string) *RoomserverAliasAPI { mockAliasAPIDB := &MockRoomserverAliasAPIDatabase{dbMode, 0} @@ -126,62 +175,22 @@ func TestGetRoomIDForAlias(t *testing.T) { } } - t.Run("Found local alias", func(t *testing.T) { - aliasAPI := setup("found", "error") + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + aliasAPI := setup(tc.dbMode, tc.queryMode) - err := aliasAPI.GetRoomIDForAlias(args.ctx, args.request, args.response) - if err != nil { - t.Errorf("Got %s; wanted no error", err) - } - - if args.response.RoomID != "123" { - t.Errorf("Got %s; wanted 123", args.response.RoomID) - } - }) - - t.Run("found appservice alias", func(t *testing.T) { - aliasAPI := setup("emptyFound", "found") - - if err := aliasAPI.GetRoomIDForAlias(args.ctx, args.request, args.response); err != nil { - t.Fatalf("Got %s; wanted no error", err) - } - - if args.response.RoomID != "123" { - t.Errorf("Got %s; wanted 123", args.response.RoomID) - } - }) - - t.Run("error returned from DB", func(t *testing.T) { - aliasAPI := setup("error", "") - - err := aliasAPI.GetRoomIDForAlias(args.ctx, args.request, args.response) - if err == nil { - t.Fatalf("Got no error; wanted error from DB") - } else if !strings.Contains(err.Error(), "GetRoomIDForAlias") { - t.Errorf("Got %s; wanted error from GetRoomIDForAlias", err) - } - }) - - t.Run("error returned from appserviceAPI", func(t *testing.T) { - aliasAPI := setup("empty", "error") - - err := aliasAPI.GetRoomIDForAlias(args.ctx, args.request, args.response) - if err == nil { - t.Fatalf("Got no error; wanted error from appserviceAPI") - } else if !strings.Contains(err.Error(), "RoomAliasExists") { - t.Errorf("Got %s; wanted error from RoomAliasExists", err) - } - }) - - t.Run("no errors but no alias", func(t *testing.T) { - aliasAPI := setup("empty", "empty") - args.response.RoomID = "Should be empty" - - if err := aliasAPI.GetRoomIDForAlias(args.ctx, args.request, args.response); err != nil { - t.Fatalf("Got %s; wanted no error", err) - } - if args.response.RoomID != "" { - t.Errorf("response.RoomID should have been empty") - } - }) + err := aliasAPI.GetRoomIDForAlias(args.ctx, args.request, args.response) + if tc.wantError { + if err == nil { + t.Fatalf("Got no error; wanted error from %s", tc.errorMsg) + } else if !strings.Contains(err.Error(), tc.errorMsg) { + t.Fatalf("Got %s; wanted error from %s", err, tc.errorMsg) + } + } else if err != nil { + t.Fatalf("Got %s; wanted no error", err) + } else if args.response.RoomID != tc.want { + t.Errorf("Got '%s'; wanted '%s'", args.response.RoomID, tc.want) + } + }) + } }