Add user profile tests, refactor user API methods (#3030)

This adds tests for `/profile`.
Also, as a first change in this regard, refactors the methods defined on
the `UserInternalAPI` to not use structs as the request/response
parameters.

3 files changed, 42 insertions, 83 deletions

diff --git a/userapi/api/api.go b/userapi/api/api.go
index 19d48684..5fd7992c 100644
--- a/userapi/api/api.go
+++ b/userapi/api/api.go
@@ -58,7 +58,7 @@ type MediaUserAPI interface {
 type FederationUserAPI interface {
 	UploadDeviceKeysAPI
 	QueryOpenIDToken(ctx context.Context, req *QueryOpenIDTokenRequest, res *QueryOpenIDTokenResponse) error
-	QueryProfile(ctx context.Context, req *QueryProfileRequest, res *QueryProfileResponse) error
+	QueryProfile(ctx context.Context, userID string) (*authtypes.Profile, error)
 	QueryDevices(ctx context.Context, req *QueryDevicesRequest, res *QueryDevicesResponse) error
 	QueryKeys(ctx context.Context, req *QueryKeysRequest, res *QueryKeysResponse) error
 	QuerySignatures(ctx context.Context, req *QuerySignaturesRequest, res *QuerySignaturesResponse) error
@@ -83,9 +83,9 @@ type ClientUserAPI interface {
 	LoginTokenInternalAPI
 	UserLoginAPI
 	ClientKeyAPI
+	ProfileAPI
 	QueryNumericLocalpart(ctx context.Context, req *QueryNumericLocalpartRequest, res *QueryNumericLocalpartResponse) error
 	QueryDevices(ctx context.Context, req *QueryDevicesRequest, res *QueryDevicesResponse) error
-	QueryProfile(ctx context.Context, req *QueryProfileRequest, res *QueryProfileResponse) error
 	QueryAccountData(ctx context.Context, req *QueryAccountDataRequest, res *QueryAccountDataResponse) error
 	QueryPushers(ctx context.Context, req *QueryPushersRequest, res *QueryPushersResponse) error
 	QueryPushRules(ctx context.Context, req *QueryPushRulesRequest, res *QueryPushRulesResponse) error
@@ -100,8 +100,6 @@ type ClientUserAPI interface {
 	PerformPushRulesPut(ctx context.Context, req *PerformPushRulesPutRequest, res *struct{}) error
 	PerformAccountDeactivation(ctx context.Context, req *PerformAccountDeactivationRequest, res *PerformAccountDeactivationResponse) error
 	PerformOpenIDTokenCreation(ctx context.Context, req *PerformOpenIDTokenCreationRequest, res *PerformOpenIDTokenCreationResponse) error
-	SetAvatarURL(ctx context.Context, req *PerformSetAvatarURLRequest, res *PerformSetAvatarURLResponse) error
-	SetDisplayName(ctx context.Context, req *PerformUpdateDisplayNameRequest, res *PerformUpdateDisplayNameResponse) error
 	QueryNotifications(ctx context.Context, req *QueryNotificationsRequest, res *QueryNotificationsResponse) error
 	InputAccountData(ctx context.Context, req *InputAccountDataRequest, res *InputAccountDataResponse) error
 	PerformKeyBackup(ctx context.Context, req *PerformKeyBackupRequest, res *PerformKeyBackupResponse) error
@@ -113,6 +111,12 @@ type ClientUserAPI interface {
 	PerformSaveThreePIDAssociation(ctx context.Context, req *PerformSaveThreePIDAssociationRequest, res *struct{}) error
 }
 
+type ProfileAPI interface {
+	QueryProfile(ctx context.Context, userID string) (*authtypes.Profile, error)
+	SetAvatarURL(ctx context.Context, localpart string, serverName gomatrixserverlib.ServerName, avatarURL string) (*authtypes.Profile, bool, error)
+	SetDisplayName(ctx context.Context, localpart string, serverName gomatrixserverlib.ServerName, displayName string) (*authtypes.Profile, bool, error)
+}
+
 // custom api functions required by pinecone / p2p demos
 type QuerySearchProfilesAPI interface {
 	QuerySearchProfiles(ctx context.Context, req *QuerySearchProfilesRequest, res *QuerySearchProfilesResponse) error
@@ -290,22 +294,6 @@ type QueryDevicesResponse struct {
 	Devices    []Device
 }
 
-// QueryProfileRequest is the request for QueryProfile
-type QueryProfileRequest struct {
-	// The user ID to query
-	UserID string
-}
-
-// QueryProfileResponse is the response for QueryProfile
-type QueryProfileResponse struct {
-	// True if the user exists. Querying for a profile does not create them.
-	UserExists bool
-	// The current display name if set.
-	DisplayName string
-	// The current avatar URL if set.
-	AvatarURL string
-}
-
 // QuerySearchProfilesRequest is the request for QueryProfile
 type QuerySearchProfilesRequest struct {
 	// The search string to match
@@ -600,16 +588,6 @@ type Notification struct {
 	TS         gomatrixserverlib.Timestamp   `json:"ts"`          // Required.
 }
 
-type PerformSetAvatarURLRequest struct {
-	Localpart  string
-	ServerName gomatrixserverlib.ServerName
-	AvatarURL  string
-}
-type PerformSetAvatarURLResponse struct {
-	Profile *authtypes.Profile `json:"profile"`
-	Changed bool               `json:"changed"`
-}
-
 type QueryNumericLocalpartRequest struct {
 	ServerName gomatrixserverlib.ServerName
 }
@@ -638,17 +616,6 @@ type QueryAccountByPasswordResponse struct {
 	Exists  bool
 }
 
-type PerformUpdateDisplayNameRequest struct {
-	Localpart   string
-	ServerName  gomatrixserverlib.ServerName
-	DisplayName string
-}
-
-type PerformUpdateDisplayNameResponse struct {
-	Profile *authtypes.Profile `json:"profile"`
-	Changed bool               `json:"changed"`
-}
-
 type QueryLocalpartForThreePIDRequest struct {
 	ThreePID, Medium string
 }
diff --git a/userapi/internal/user_api.go b/userapi/internal/user_api.go
index 4049d13b..6dad91dd 100644
--- a/userapi/internal/user_api.go
+++ b/userapi/internal/user_api.go
@@ -23,6 +23,8 @@ import (
 	"strconv"
 	"time"
 
+	appserviceAPI "github.com/matrix-org/dendrite/appservice/api"
+	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	fedsenderapi "github.com/matrix-org/dendrite/federationapi/api"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
@@ -418,25 +420,26 @@ func (a *UserInternalAPI) PerformDeviceUpdate(ctx context.Context, req *api.Perf
 	return nil
 }
 
-func (a *UserInternalAPI) QueryProfile(ctx context.Context, req *api.QueryProfileRequest, res *api.QueryProfileResponse) error {
-	local, domain, err := gomatrixserverlib.SplitID('@', req.UserID)
+var (
+	ErrIsRemoteServer = errors.New("cannot query profile of remote users")
+)
+
+func (a *UserInternalAPI) QueryProfile(ctx context.Context, userID string) (*authtypes.Profile, error) {
+	local, domain, err := gomatrixserverlib.SplitID('@', userID)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	if !a.Config.Matrix.IsLocalServerName(domain) {
-		return fmt.Errorf("cannot query profile of remote users (server name %s)", domain)
+		return nil, ErrIsRemoteServer
 	}
 	prof, err := a.DB.GetProfileByLocalpart(ctx, local, domain)
 	if err != nil {
 		if err == sql.ErrNoRows {
-			return nil
+			return nil, appserviceAPI.ErrProfileNotExists
 		}
-		return err
+		return nil, err
 	}
-	res.UserExists = true
-	res.AvatarURL = prof.AvatarURL
-	res.DisplayName = prof.DisplayName
-	return nil
+	return prof, nil
 }
 
 func (a *UserInternalAPI) QuerySearchProfiles(ctx context.Context, req *api.QuerySearchProfilesRequest, res *api.QuerySearchProfilesResponse) error {
@@ -901,11 +904,8 @@ func (a *UserInternalAPI) QueryPushRules(ctx context.Context, req *api.QueryPush
 	return nil
 }
 
-func (a *UserInternalAPI) SetAvatarURL(ctx context.Context, req *api.PerformSetAvatarURLRequest, res *api.PerformSetAvatarURLResponse) error {
-	profile, changed, err := a.DB.SetAvatarURL(ctx, req.Localpart, req.ServerName, req.AvatarURL)
-	res.Profile = profile
-	res.Changed = changed
-	return err
+func (a *UserInternalAPI) SetAvatarURL(ctx context.Context, localpart string, serverName gomatrixserverlib.ServerName, avatarURL string) (*authtypes.Profile, bool, error) {
+	return a.DB.SetAvatarURL(ctx, localpart, serverName, avatarURL)
 }
 
 func (a *UserInternalAPI) QueryNumericLocalpart(ctx context.Context, req *api.QueryNumericLocalpartRequest, res *api.QueryNumericLocalpartResponse) error {
@@ -939,11 +939,8 @@ func (a *UserInternalAPI) QueryAccountByPassword(ctx context.Context, req *api.Q
 	}
 }
 
-func (a *UserInternalAPI) SetDisplayName(ctx context.Context, req *api.PerformUpdateDisplayNameRequest, res *api.PerformUpdateDisplayNameResponse) error {
-	profile, changed, err := a.DB.SetDisplayName(ctx, req.Localpart, req.ServerName, req.DisplayName)
-	res.Profile = profile
-	res.Changed = changed
-	return err
+func (a *UserInternalAPI) SetDisplayName(ctx context.Context, localpart string, serverName gomatrixserverlib.ServerName, displayName string) (*authtypes.Profile, bool, error) {
+	return a.DB.SetDisplayName(ctx, localpart, serverName, displayName)
 }
 
 func (a *UserInternalAPI) QueryLocalpartForThreePID(ctx context.Context, req *api.QueryLocalpartForThreePIDRequest, res *api.QueryLocalpartForThreePIDResponse) error {
diff --git a/userapi/userapi_test.go b/userapi/userapi_test.go
index 03e65635..e29246ec 100644
--- a/userapi/userapi_test.go
+++ b/userapi/userapi_test.go
@@ -22,6 +22,8 @@ import (
 	"testing"
 	"time"
 
+	api2 "github.com/matrix-org/dendrite/appservice/api"
+	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/userapi/producers"
 	"github.com/matrix-org/gomatrixserverlib"
@@ -111,33 +113,26 @@ func TestQueryProfile(t *testing.T) {
 	aliceDisplayName := "Alice"
 
 	testCases := []struct {
-		req     api.QueryProfileRequest
-		wantRes api.QueryProfileResponse
+		userID  string
+		wantRes *authtypes.Profile
 		wantErr error
 	}{
 		{
-			req: api.QueryProfileRequest{
-				UserID: fmt.Sprintf("@alice:%s", serverName),
-			},
-			wantRes: api.QueryProfileResponse{
-				UserExists:  true,
-				AvatarURL:   aliceAvatarURL,
+			userID: fmt.Sprintf("@alice:%s", serverName),
+			wantRes: &authtypes.Profile{
+				Localpart:   "alice",
 				DisplayName: aliceDisplayName,
+				AvatarURL:   aliceAvatarURL,
+				ServerName:  string(serverName),
 			},
 		},
 		{
-			req: api.QueryProfileRequest{
-				UserID: fmt.Sprintf("@bob:%s", serverName),
-			},
-			wantRes: api.QueryProfileResponse{
-				UserExists: false,
-			},
+			userID:  fmt.Sprintf("@bob:%s", serverName),
+			wantErr: api2.ErrProfileNotExists,
 		},
 		{
-			req: api.QueryProfileRequest{
-				UserID: "@alice:wrongdomain.com",
-			},
-			wantErr: fmt.Errorf("wrong domain"),
+			userID:  "@alice:wrongdomain.com",
+			wantErr: api2.ErrProfileNotExists,
 		},
 	}
 
@@ -147,14 +142,14 @@ func TestQueryProfile(t *testing.T) {
 			mode = "HTTP"
 		}
 		for _, tc := range testCases {
-			var gotRes api.QueryProfileResponse
-			gotErr := testAPI.QueryProfile(context.TODO(), &tc.req, &gotRes)
+
+			profile, gotErr := testAPI.QueryProfile(context.TODO(), tc.userID)
 			if tc.wantErr == nil && gotErr != nil || tc.wantErr != nil && gotErr == nil {
 				t.Errorf("QueryProfile %s error, got %s want %s", mode, gotErr, tc.wantErr)
 				continue
 			}
-			if !reflect.DeepEqual(tc.wantRes, gotRes) {
-				t.Errorf("QueryProfile %s response got %+v want %+v", mode, gotRes, tc.wantRes)
+			if !reflect.DeepEqual(tc.wantRes, profile) {
+				t.Errorf("QueryProfile %s response got %+v want %+v", mode, profile, tc.wantRes)
 			}
 		}
 	}