From a0dec456c173de650bef03d3da97f0f721288fd1 Mon Sep 17 00:00:00 2001 From: Serra Allgood Date: Tue, 25 Jun 2019 05:43:18 -0700 Subject: [PATCH] rommserver/alias: Do not call appserviceAPI in GetRoomIDForAlias if local alias found #631 (#702) A conditional is added to wrap the call to appserviceAPI if a local alias is not found in the database. Fixes #631 Signed-off-by: Serra Allgood --- roomserver/alias/alias.go | 21 +++- roomserver/alias/alias_test.go | 196 +++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 6 deletions(-) create mode 100644 roomserver/alias/alias_test.go diff --git a/roomserver/alias/alias.go b/roomserver/alias/alias.go index 27279aad..6a34aacd 100644 --- a/roomserver/alias/alias.go +++ b/roomserver/alias/alias.go @@ -96,12 +96,21 @@ func (r *RoomserverAliasAPI) GetRoomIDForAlias( return err } - // No rooms found locally, try our application services by making a call to - // the appservice component - aliasReq := appserviceAPI.RoomAliasExistsRequest{Alias: request.Alias} - var aliasResp appserviceAPI.RoomAliasExistsResponse - if err = r.AppserviceAPI.RoomAliasExists(ctx, &aliasReq, &aliasResp); err != nil { - return err + if roomID == "" { + // 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 + if err = r.AppserviceAPI.RoomAliasExists(ctx, &aliasReq, &aliasResp); err != nil { + return err + } + + if aliasResp.AliasExists { + roomID, err = r.DB.GetRoomIDForAlias(ctx, request.Alias) + if err != nil { + return err + } + } } response.RoomID = roomID diff --git a/roomserver/alias/alias_test.go b/roomserver/alias/alias_test.go new file mode 100644 index 00000000..4b9ca022 --- /dev/null +++ b/roomserver/alias/alias_test.go @@ -0,0 +1,196 @@ +// Copyright 2019 Serra Allgood +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package alias + +import ( + "context" + "fmt" + "strings" + "testing" + + appserviceAPI "github.com/matrix-org/dendrite/appservice/api" + roomserverAPI "github.com/matrix-org/dendrite/roomserver/api" +) + +type MockRoomserverAliasAPIDatabase struct { + mode string + attempts int +} + +// These methods can be essentially noop +func (db MockRoomserverAliasAPIDatabase) SetRoomAlias(ctx context.Context, alias string, roomID string) error { + return nil +} + +func (db MockRoomserverAliasAPIDatabase) GetAliasesForRoomID(ctx context.Context, roomID string) ([]string, error) { + aliases := make([]string, 0) + return aliases, nil +} + +func (db MockRoomserverAliasAPIDatabase) RemoveRoomAlias(ctx context.Context, alias string) error { + return nil +} + +// This method needs to change depending on test case +func (db *MockRoomserverAliasAPIDatabase) GetRoomIDForAlias( + ctx context.Context, + alias string, +) (string, error) { + switch db.mode { + case "empty": + return "", nil + case "error": + return "", fmt.Errorf("found an error from GetRoomIDForAlias") + case "found": + return "123", nil + case "emptyFound": + switch db.attempts { + case 0: + db.attempts = 1 + return "", nil + case 1: + db.attempts = 0 + return "123", nil + default: + return "", nil + } + default: + return "", fmt.Errorf("unknown option used") + } +} + +type MockAppServiceQueryAPI struct { + mode string +} + +// This method can be noop +func (q MockAppServiceQueryAPI) UserIDExists( + ctx context.Context, + req *appserviceAPI.UserIDExistsRequest, + resp *appserviceAPI.UserIDExistsResponse, +) error { + return nil +} + +func (q MockAppServiceQueryAPI) RoomAliasExists( + ctx context.Context, + req *appserviceAPI.RoomAliasExistsRequest, + resp *appserviceAPI.RoomAliasExistsResponse, +) error { + switch q.mode { + case "error": + return fmt.Errorf("found an error from RoomAliasExists") + case "found": + resp.AliasExists = true + return nil + case "empty": + resp.AliasExists = false + return nil + default: + return fmt.Errorf("Unknown option used") + } +} + +func TestGetRoomIDForAlias(t *testing.T) { + type arguments struct { + ctx context.Context + 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} + mockAppServiceQueryAPI := MockAppServiceQueryAPI{queryMode} + + return &RoomserverAliasAPI{ + DB: mockAliasAPIDB, + AppserviceAPI: mockAppServiceQueryAPI, + } + } + + 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 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) + } + }) + } +}