mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

MM-30882: Fix read-after-write issue for demoting user (#16911)

Просмотр исходного кода 
* MM-30882: Fix read-after-write issue for demoting user

In (*App).DemoteUserToGuest, we would demote a user, and then immediately
read it back to do future operations from the user. This reading back
of the user had the effect of sticking the old value into the cache
after which it would never be updated.

There was another issue along with this, which was when the invalidation
message would broadcast across the cluster, it would hit the cache invalidation
problem where an unrelated store call would miss the cache because
it was invalidated, and then again read from replica and stick the old value.

To fix all these, we return the new value directly from the store method
to avoid having the app to read it again.

And we add a map in the localcache layer which tracks invalidations made,
and then switch to use master if it's true.

The core change is fairly limited, but due to changing the store method signatures,
a lot of code needed to be updated to pass "context.Background". Therefore the PR
just "appears" to be big, but the main changes are limited to app/user.go,
sqlstore/user_store.go and user_layer.go

https://mattermost.atlassian.net/browse/MM-30882

```release-note
Fix an issue where demoting a user to guest would not take effect in
an environment with read replicas.
```

* Fix concurrent map access

* Fixing mistakes

* fix tests
Этот коммит содержится в:
Agniva De Sarker
2021-02-12 19:04:05 +05:30
коммит произвёл GitHub
родитель 49907d3081
Коммит 021c90f29f
38 изменённых файлов: 410 добавлений и 288 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

92
store/sqlstore/user_store.go
Просмотреть файл

@@ -4,6 +4,7 @@
package sqlstore
import (
"context"
"database/sql"
"encoding/json"
"fmt"
@@ -38,7 +39,7 @@ type SqlUserStore struct {
usersQuery sq.SelectBuilder
}
func (us SqlUserStore) ClearCaches() {}
func (us *SqlUserStore) ClearCaches() {}
func (us SqlUserStore) InvalidateProfileCacheForUser(userId string) {}
@@ -326,28 +327,41 @@ func (us SqlUserStore) UpdateMfaActive(userId string, active bool) error {
}
// GetMany returns a list of users for the provided list of ids
func (us SqlUserStore) GetMany(ids []string) ([]*model.User, error) {
func (us SqlUserStore) GetMany(ctx context.Context, ids []string) ([]*model.User, error) {
query := us.usersQuery.Where(sq.Eq{"Id": ids})
queryString, args, err := query.ToSql()
if err != nil {
return nil, errors.Wrap(err, "users_get_many_tosql")
}
var db *gorp.DbMap
if hasMaster(ctx) {
db = us.GetMaster()
} else {
db = us.GetReplica()
}
var users []*model.User
if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
if _, err := db.Select(&users, queryString, args...); err != nil {
return nil, errors.Wrap(err, "users_get_many_select")
}
return users, nil
}
func (us SqlUserStore) Get(id string) (*model.User, error) {
func (us SqlUserStore) Get(ctx context.Context, id string) (*model.User, error) {
query := us.usersQuery.Where("Id = ?", id)
queryString, args, err := query.ToSql()
if err != nil {
return nil, errors.Wrap(err, "users_get_tosql")
}
row := us.GetReplica().Db.QueryRow(queryString, args...)
var db *gorp.DbMap
if hasMaster(ctx) {
db = us.GetMaster()
} else {
db = us.GetReplica()
}
row := db.Db.QueryRow(queryString, args...)
var user model.User
var props, notifyProps, timezone []byte
@@ -703,10 +717,10 @@ func (us SqlUserStore) GetProfilesInChannelByStatus(options *model.UserGetOption
return users, nil
}
func (us SqlUserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, error) {
func (us SqlUserStore) GetAllProfilesInChannel(ctx context.Context, channelID string, allowFromCache bool) (map[string]*model.User, error) {
query := us.usersQuery.
Join("ChannelMembers cm ON ( cm.UserId = u.Id )").
Where("cm.ChannelId = ?", channelId).
Where("cm.ChannelId = ?", channelID).
Where("u.DeleteAt = 0").
OrderBy("u.Username ASC")
@@ -714,8 +728,15 @@ func (us SqlUserStore) GetAllProfilesInChannel(channelId string, allowFromCache
if err != nil {
return nil, errors.Wrap(err, "get_all_profiles_in_channel_tosql")
}
var db *gorp.DbMap
if hasMaster(ctx) {
db = us.GetMaster()
} else {
db = us.GetReplica()
}
var users []*model.User
rows, err := us.GetReplica().Db.Query(queryString, args...)
rows, err := db.Db.Query(queryString, args...)
if err != nil {
return nil, errors.Wrap(err, "failed to find Users")
}
@@ -914,7 +935,7 @@ func (us SqlUserStore) GetNewUsersForTeam(teamId string, offset, limit int, view
return users, nil
}
func (us SqlUserStore) GetProfileByIds(userIds []string, options *store.UserGetByIdsOpts, allowFromCache bool) ([]*model.User, error) {
func (us SqlUserStore) GetProfileByIds(ctx context.Context, userIds []string, options *store.UserGetByIdsOpts, allowFromCache bool) ([]*model.User, error) {
if options == nil {
options = &store.UserGetByIdsOpts{}
}
@@ -939,7 +960,14 @@ func (us SqlUserStore) GetProfileByIds(userIds []string, options *store.UserGetB
return nil, errors.Wrap(err, "get_profile_by_ids_tosql")
}
if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
var db *gorp.DbMap
if hasMaster(ctx) {
db = us.GetMaster()
} else {
db = us.GetReplica()
}
if _, err := db.Select(&users, queryString, args...); err != nil {
return nil, errors.Wrap(err, "failed to find Users")
}
@@ -1775,7 +1803,7 @@ func (us SqlUserStore) PromoteGuestToUser(userId string) error {
}
defer finalizeTransaction(transaction)
user, err := us.Get(userId)
user, err := us.Get(context.Background(), userId)
if err != nil {
return err
}
@@ -1837,76 +1865,80 @@ func (us SqlUserStore) PromoteGuestToUser(userId string) error {
return nil
}
func (us SqlUserStore) DemoteUserToGuest(userId string) error {
func (us SqlUserStore) DemoteUserToGuest(userID string) (*model.User, error) {
transaction, err := us.GetMaster().Begin()
if err != nil {
return errors.Wrap(err, "begin_transaction")
return nil, errors.Wrap(err, "begin_transaction")
}
defer finalizeTransaction(transaction)
user, err := us.Get(userId)
user, err := us.Get(context.Background(), userID)
if err != nil {
return err
return nil, err
}
roles := user.GetRoles()
newRoles := []string{}
for _, role := range roles {
if role == "system_user" {
newRoles = append(newRoles, "system_guest")
} else if role != "system_admin" {
if role == model.SYSTEM_USER_ROLE_ID {
newRoles = append(newRoles, model.SYSTEM_GUEST_ROLE_ID)
} else if role != model.SYSTEM_ADMIN_ROLE_ID {
newRoles = append(newRoles, role)
}
}
curTime := model.GetMillis()
newRolesDBStr := strings.Join(newRoles, " ")
query := us.getQueryBuilder().Update("Users").
Set("Roles", strings.Join(newRoles, " ")).
Set("Roles", newRolesDBStr).
Set("UpdateAt", curTime).
Where(sq.Eq{"Id": userId})
Where(sq.Eq{"Id": userID})
queryString, args, err := query.ToSql()
if err != nil {
return errors.Wrap(err, "demote_user_to_guest_tosql")
return nil, errors.Wrap(err, "demote_user_to_guest_tosql")
}
if _, err = transaction.Exec(queryString, args...); err != nil {
return errors.Wrapf(err, "failed to update User with userId=%s", userId)
return nil, errors.Wrapf(err, "failed to update User with userId=%s", userID)
}
user.Roles = newRolesDBStr
user.UpdateAt = curTime
query = us.getQueryBuilder().Update("ChannelMembers").
Set("SchemeUser", false).
Set("SchemeGuest", true).
Where(sq.Eq{"UserId": userId})
Where(sq.Eq{"UserId": userID})
queryString, args, err = query.ToSql()
if err != nil {
return errors.Wrap(err, "demote_user_to_guest_tosql")
return nil, errors.Wrap(err, "demote_user_to_guest_tosql")
}
if _, err = transaction.Exec(queryString, args...); err != nil {
return errors.Wrapf(err, "failed to update ChannelMembers with userId=%s", userId)
return nil, errors.Wrapf(err, "failed to update ChannelMembers with userId=%s", userID)
}
query = us.getQueryBuilder().Update("TeamMembers").
Set("SchemeUser", false).
Set("SchemeGuest", true).
Where(sq.Eq{"UserId": userId})
Where(sq.Eq{"UserId": userID})
queryString, args, err = query.ToSql()
if err != nil {
return errors.Wrap(err, "demote_user_to_guest_tosql")
return nil, errors.Wrap(err, "demote_user_to_guest_tosql")
}
if _, err := transaction.Exec(queryString, args...); err != nil {
return errors.Wrapf(err, "failed to update TeamMembers with userId=%s", userId)
return nil, errors.Wrapf(err, "failed to update TeamMembers with userId=%s", userID)
}
if err := transaction.Commit(); err != nil {
return errors.Wrap(err, "commit_transaction")
return nil, errors.Wrap(err, "commit_transaction")
}
return nil
return user, nil
}
func (us SqlUserStore) AutocompleteUsersInChannel(teamId, channelId, term string, options *model.UserSearchOptions) (*model.UserAutocompleteInChannel, error) {