[MM-16514] Migrate Token.GetByName to Sync by Default (#11355)

* [MM-16514] Migrate Token.GetByToken to Sync by default

* test: use testify

* fix: shadowing

api4/team_test.go

@@ -1436,9 +1436,8 @@ func TestAddTeamMember(t *testing.T) {
 		t.Fatal("team ids should have matched")
 	}
 
-	if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil {
+	_, err := th.App.Srv.Store.Token().GetByToken(token.Token)
-		t.Fatal("The token must be deleted after be used")
+	require.NotNil(t, err, "The token must be deleted after be used")
-	}
 
 	tm, resp = Client.AddTeamMemberFromInvite("junk", "")
 	CheckBadRequestStatus(t, resp)
@@ -1495,7 +1494,7 @@ func TestAddTeamMember(t *testing.T) {
 
 	// Set a team to group-constrained
 	team.GroupConstrained = model.NewBool(true)
-	_, err := th.App.UpdateTeam(team)
+	_, err = th.App.UpdateTeam(team)
 	require.Nil(t, err)
 
 	// Attempt to use a token on a group-constrained team

api4/user_test.go

@@ -108,13 +108,8 @@ func TestCreateUserWithToken(t *testing.T) {
 			t.Fatal("did not clear roles")
 		}
 		CheckUserSanitization(t, ruser)
-		if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil {
+		_, err := th.App.Srv.Store.Token().GetByToken(token.Token)
-			t.Fatal("The token must be deleted after be used")
+		require.NotNil(t, err, "The token must be deleted after being used")
-		}
-
-		if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil {
-			t.Fatal("The token must be deleted after be used")
-		}
 
 		if teams, err := th.App.GetTeamsForUser(ruser.Id); err != nil || len(teams) == 0 {
 			t.Fatal("The user must have teams")
@@ -215,9 +210,8 @@ func TestCreateUserWithToken(t *testing.T) {
 			t.Fatal("did not clear roles")
 		}
 		CheckUserSanitization(t, ruser)
-		if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil {
+		_, err := th.App.Srv.Store.Token().GetByToken(token.Token)
-			t.Fatal("The token must be deleted after be used")
+		require.NotNil(t, err, "The token must be deleted after be used")
-		}
 	})
 }
 
@@ -2239,7 +2233,8 @@ func TestResetPassword(t *testing.T) {
 		if !strings.ContainsAny(resultsMailbox[0].To[0], user.Email) {
 			t.Fatal("Wrong To recipient")
 		} else {
-			if resultsEmail, err := mailservice.GetMessageFromMailbox(user.Email, resultsMailbox[0].ID); err == nil {
+			var resultsEmail mailservice.JSONMessageInbucket
+			if resultsEmail, err = mailservice.GetMessageFromMailbox(user.Email, resultsMailbox[0].ID); err == nil {
 				loc := strings.Index(resultsEmail.Body.Text, "token=")
 				if loc == -1 {
 					t.Log(resultsEmail.Body.Text)
@@ -2250,13 +2245,9 @@ func TestResetPassword(t *testing.T) {
 			}
 		}
 	}
-	var recoveryToken *model.Token
+	recoveryToken, err := th.App.Srv.Store.Token().GetByToken(recoveryTokenString)
-	if result := <-th.App.Srv.Store.Token().GetByToken(recoveryTokenString); result.Err != nil {
+	require.Nil(t, err, "Recovery token not found (%s)", recoveryTokenString)
-		t.Log(recoveryTokenString)
+
-		t.Fatal(result.Err)
-	} else {
-		recoveryToken = result.Data.(*model.Token)
-	}
 	_, resp = th.Client.ResetPassword(recoveryToken.Token, "")
 	CheckBadRequestStatus(t, resp)
 	_, resp = th.Client.ResetPassword(recoveryToken.Token, "newp")
@@ -4180,7 +4171,6 @@ func TestLoginErrorMessage(t *testing.T) {
 	_, resp := th.Client.Logout()
 	CheckNoError(t, resp)
 
-
 	// Email and Username enabled
 	th.App.UpdateConfig(func(cfg *model.Config) {
 		*cfg.EmailSettings.EnableSignInWithEmail = true

app/oauth.go

@@ -601,11 +601,10 @@ func (a *App) CreateOAuthStateToken(extra string) (*model.Token, *model.AppError
 }
 
 func (a *App) GetOAuthStateToken(token string) (*model.Token, *model.AppError) {
-	result := <-a.Srv.Store.Token().GetByToken(token)
+	mToken, err := a.Srv.Store.Token().GetByToken(token)
-	if result.Err != nil {
+	if err != nil {
-		return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, result.Err.Error(), http.StatusBadRequest)
+		return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
-	mToken := result.Data.(*model.Token)
 
 	if mToken.Type != model.TOKEN_TYPE_OAUTH {
 		return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, "", http.StatusBadRequest)

app/team.go

@@ -398,12 +398,11 @@ func (a *App) AddUserToTeamByTeamId(teamId string, user *model.User) *model.AppE
 }
 
 func (a *App) AddUserToTeamByToken(userId string, tokenId string) (*model.Team, *model.AppError) {
-	result := <-a.Srv.Store.Token().GetByToken(tokenId)
+	token, err := a.Srv.Store.Token().GetByToken(tokenId)
-	if result.Err != nil {
+	if err != nil {
-		return nil, model.NewAppError("AddUserToTeamByToken", "api.user.create_user.signup_link_invalid.app_error", nil, result.Err.Error(), http.StatusBadRequest)
+		return nil, model.NewAppError("AddUserToTeamByToken", "api.user.create_user.signup_link_invalid.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
 
-	token := result.Data.(*model.Token)
 	if token.Type != TOKEN_TYPE_TEAM_INVITATION {
 		return nil, model.NewAppError("AddUserToTeamByToken", "api.user.create_user.signup_link_invalid.app_error", nil, "", http.StatusBadRequest)
 	}
@@ -429,7 +428,7 @@ func (a *App) AddUserToTeamByToken(userId string, tokenId string) (*model.Team,
 		close(uchan)
 	}()
 
-	result = <-tchan
+	result := <-tchan
 	if result.Err != nil {
 		return nil, result.Err
 	}
@@ -1154,12 +1153,11 @@ func (a *App) GetTeamIdFromQuery(query url.Values) (string, *model.AppError) {
 	inviteId := query.Get("id")
 
 	if len(tokenId) > 0 {
-		result := <-a.Srv.Store.Token().GetByToken(tokenId)
+		token, err := a.Srv.Store.Token().GetByToken(tokenId)
-		if result.Err != nil {
+		if err != nil {
 			return "", model.NewAppError("GetTeamIdFromQuery", "api.oauth.singup_with_oauth.invalid_link.app_error", nil, "", http.StatusBadRequest)
 		}
 
-		token := result.Data.(*model.Token)
 		if token.Type != TOKEN_TYPE_TEAM_INVITATION {
 			return "", model.NewAppError("GetTeamIdFromQuery", "api.oauth.singup_with_oauth.invalid_link.app_error", nil, "", http.StatusBadRequest)
 		}

app/team_test.go

@@ -259,9 +259,8 @@ func TestAddUserToTeamByToken(t *testing.T) {
 			t.Log(err)
 			t.Fatal("Should add user to the team")
 		}
-		if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil {
+		_, err := th.App.Srv.Store.Token().GetByToken(token.Token)
-			t.Fatal("The token must be deleted after be used")
+		require.NotNil(t, err, "The token must be deleted after be used")
-		}
 	})
 
 	t.Run("group-constrained team", func(t *testing.T) {

app/user.go

@@ -50,12 +50,11 @@ func (a *App) CreateUserWithToken(user *model.User, tokenId string) (*model.User
 		return nil, err
 	}
 
-	result := <-a.Srv.Store.Token().GetByToken(tokenId)
+	token, err := a.Srv.Store.Token().GetByToken(tokenId)
-	if result.Err != nil {
+	if err != nil {
-		return nil, model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, result.Err.Error(), http.StatusBadRequest)
+		return nil, model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
 
-	token := result.Data.(*model.Token)
 	if token.Type != TOKEN_TYPE_TEAM_INVITATION {
 		return nil, model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, "", http.StatusBadRequest)
 	}
@@ -1350,11 +1349,10 @@ func (a *App) CreatePasswordRecoveryToken(userId, email string) (*model.Token, *
 }
 
 func (a *App) GetPasswordRecoveryToken(token string) (*model.Token, *model.AppError) {
-	result := <-a.Srv.Store.Token().GetByToken(token)
+	rtoken, err := a.Srv.Store.Token().GetByToken(token)
-	if result.Err != nil {
+	if err != nil {
-		return nil, model.NewAppError("GetPasswordRecoveryToken", "api.user.reset_password.invalid_link.app_error", nil, result.Err.Error(), http.StatusBadRequest)
+		return nil, model.NewAppError("GetPasswordRecoveryToken", "api.user.reset_password.invalid_link.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
-	rtoken := result.Data.(*model.Token)
 	if rtoken.Type != TOKEN_TYPE_PASSWORD_RECOVERY {
 		return nil, model.NewAppError("GetPasswordRecoveryToken", "api.user.reset_password.broken_token.app_error", nil, "", http.StatusBadRequest)
 	}
@@ -1617,11 +1615,10 @@ func (a *App) CreateVerifyEmailToken(userId string, newEmail string) (*model.Tok
 }
 
 func (a *App) GetVerifyEmailToken(token string) (*model.Token, *model.AppError) {
-	result := <-a.Srv.Store.Token().GetByToken(token)
+	rtoken, err := a.Srv.Store.Token().GetByToken(token)
-	if result.Err != nil {
+	if err != nil {
-		return nil, model.NewAppError("GetVerifyEmailToken", "api.user.verify_email.bad_link.app_error", nil, result.Err.Error(), http.StatusBadRequest)
+		return nil, model.NewAppError("GetVerifyEmailToken", "api.user.verify_email.bad_link.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
-	rtoken := result.Data.(*model.Token)
 	if rtoken.Type != TOKEN_TYPE_VERIFY_EMAIL {
 		return nil, model.NewAppError("GetVerifyEmailToken", "api.user.verify_email.broken_token.app_error", nil, "", http.StatusBadRequest)
 	}

app/user_test.go

@@ -618,9 +618,8 @@ func TestCreateUserWithToken(t *testing.T) {
 		if newUser.Email != invitationEmail {
 			t.Fatal("The user email must be the invitation one")
 		}
-		if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil {
+		_, err = th.App.Srv.Store.Token().GetByToken(token.Token)
-			t.Fatal("The token must be deleted after be used")
+		require.NotNil(t, err, "The token must be deleted after be used")
-		}
 	})
 }
 

store/sqlstore/tokens_store.go

@@ -52,20 +52,18 @@ func (s SqlTokenStore) Delete(token string) store.StoreChannel {
 	})
 }
 
-func (s SqlTokenStore) GetByToken(tokenString string) store.StoreChannel {
+func (s SqlTokenStore) GetByToken(tokenString string) (*model.Token, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	token := &model.Token{}
-		token := model.Token{}
 
-		if err := s.GetReplica().SelectOne(&token, "SELECT * FROM Tokens WHERE Token = :Token", map[string]interface{}{"Token": tokenString}); err != nil {
+	if err := s.GetReplica().SelectOne(token, "SELECT * FROM Tokens WHERE Token = :Token", map[string]interface{}{"Token": tokenString}); err != nil {
 		if err == sql.ErrNoRows {
-				result.Err = model.NewAppError("SqlTokenStore.GetByToken", "store.sql_recover.get_by_code.app_error", nil, err.Error(), http.StatusBadRequest)
+			return nil, model.NewAppError("SqlTokenStore.GetByToken", "store.sql_recover.get_by_code.app_error", nil, err.Error(), http.StatusBadRequest)
-			} else {
-				result.Err = model.NewAppError("SqlTokenStore.GetByToken", "store.sql_recover.get_by_code.app_error", nil, err.Error(), http.StatusInternalServerError)
-			}
 		}
 
-		result.Data = &token
+		return nil, model.NewAppError("SqlTokenStore.GetByToken", "store.sql_recover.get_by_code.app_error", nil, err.Error(), http.StatusInternalServerError)
-	})
+	}
+
+	return token, nil
 }
 
 func (s SqlTokenStore) Cleanup() {

store/store.go

@@ -448,7 +448,7 @@ type LicenseStore interface {
 type TokenStore interface {
 	Save(recovery *model.Token) StoreChannel
 	Delete(token string) StoreChannel
-	GetByToken(token string) StoreChannel
+	GetByToken(token string) (*model.Token, *model.AppError)
 	Cleanup()
 	RemoveAllTokensByType(tokenType string) StoreChannel
 }

store/storetest/mocks/TokenStore.go

@@ -35,19 +35,28 @@ func (_m *TokenStore) Delete(token string) store.StoreChannel {
 }
 
 // GetByToken provides a mock function with given fields: token
-func (_m *TokenStore) GetByToken(token string) store.StoreChannel {
+func (_m *TokenStore) GetByToken(token string) (*model.Token, *model.AppError) {
 	ret := _m.Called(token)
 
-	var r0 store.StoreChannel
+	var r0 *model.Token
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string) *model.Token); ok {
 		r0 = rf(token)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.Token)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(token)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // RemoveAllTokensByType provides a mock function with given fields: tokenType