roomserver/alias: Minor fixes, consolidate test cases

Signed-off-by: Serra Allgood <serra@allgood.dev>

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
 			}

roomserver/alias/alias_test.go

@@ -29,7 +29,7 @@ type MockRoomserverAliasAPIDatabase struct {
 	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) {
+	for _, tc := range tt {
-		aliasAPI := setup("found", "error")
+		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 tc.wantError {
 				if err == nil {
-			t.Fatalf("Got no error; wanted error from DB")
+					t.Fatalf("Got no error; wanted error from %s", tc.errorMsg)
-		} else if !strings.Contains(err.Error(), "GetRoomIDForAlias") {
+				} else if !strings.Contains(err.Error(), tc.errorMsg) {
-			t.Errorf("Got %s; wanted error from GetRoomIDForAlias", err)
+					t.Fatalf("Got %s; wanted error from %s", err, tc.errorMsg)
 				}
-	})
+			} else if err != nil {
-
-	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)
-		}
+			} else if args.response.RoomID != tc.want {
-		if args.response.RoomID != "" {
+				t.Errorf("Got '%s'; wanted '%s'", args.response.RoomID, tc.want)
-			t.Errorf("response.RoomID should have been empty")
 			}
 		})
+	}
 }