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

Fix racy test issues (#24971)

Просмотр исходного кода
Этот коммит содержится в:
Ben Schumacher
2023-11-06 12:26:17 +01:00
коммит произвёл GitHub
родитель 366d1613b7
Коммит 486e836b83
36 изменённых файлов: 191 добавлений и 172 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

4
server/channels/app/app_iface.go
Просмотреть файл

@@ -553,7 +553,7 @@ type AppIface interface {
DoEmojisPermissionsMigration()
DoGuestRolesCreationMigration()
DoLocalRequest(c request.CTX, rawURL string, body []byte) (*http.Response, *model.AppError)
DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) *model.AppError
DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) (*model.Session, *model.AppError)
DoPostAction(c request.CTX, postID, actionId, userID, selectedOption string) (string, *model.AppError)
DoPostActionWithCookie(c request.CTX, postID, actionId, userID, selectedOption string, cookie *model.PostActionCookie) (string, *model.AppError)
DoSystemConsoleRolesCreationMigration()
@@ -687,7 +687,7 @@ type AppIface interface {
GetLatestVersion(latestVersionUrl string) (*model.GithubReleaseInfo, *model.AppError)
GetLogs(c request.CTX, page, perPage int) ([]string, *model.AppError)
GetLogsSkipSend(page, perPage int, logFilter *model.LogFilter) ([]string, *model.AppError)
GetMemberCountsByGroup(ctx context.Context, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError)
GetMemberCountsByGroup(rctx request.CTX, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError)
GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string
GetMultipleEmojiByName(c request.CTX, names []string) ([]*model.Emoji, *model.AppError)
GetNewUsersForTeamPage(teamID string, page, perPage int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError)

12
server/channels/app/channel.go
Просмотреть файл

@@ -1666,7 +1666,9 @@ func (a *App) AddChannelMember(c request.CTX, userID string, channel *model.Chan
}
} else {
a.Srv().Go(func() {
a.PostAddToChannelMessage(c, userRequestor, user, channel, opts.PostRootID)
if err := a.PostAddToChannelMessage(c, userRequestor, user, channel, opts.PostRootID); err != nil {
c.Logger().Error("Failed to post AddToChannel message", mlog.Err(err))
}
})
}
@@ -2363,7 +2365,9 @@ func (a *App) LeaveChannel(c request.CTX, channelID string, userID string) *mode
}
a.Srv().Go(func() {
a.postLeaveChannelMessage(c, user, channel)
if err := a.postLeaveChannelMessage(c, user, channel); err != nil {
c.Logger().Error("Failed to post LeaveChannel message", mlog.Err(err))
}
})
return nil
@@ -3458,8 +3462,8 @@ func (a *App) ClearChannelMembersCache(c request.CTX, channelID string) error {
return nil
}
func (a *App) GetMemberCountsByGroup(ctx context.Context, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError) {
channelMemberCounts, err := a.Srv().Store().Channel().GetMemberCountsByGroup(ctx, channelID, includeTimezones)
func (a *App) GetMemberCountsByGroup(rctx request.CTX, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError) {
channelMemberCounts, err := a.Srv().Store().Channel().GetMemberCountsByGroup(rctx.Context(), channelID, includeTimezones)
if err != nil {
return nil, model.NewAppError("GetMemberCountsByGroup", "app.channel.get_member_count.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}

2
server/channels/app/channel_test.go
Просмотреть файл

@@ -2152,7 +2152,7 @@ func TestGetMemberCountsByGroup(t *testing.T) {
mockChannelStore.On("GetMemberCountsByGroup", context.Background(), "channelID", true).Return(cmc, nil)
mockStore.On("Channel").Return(&mockChannelStore)
mockStore.On("GetDBSchemaVersion").Return(1, nil)
resp, err := th.App.GetMemberCountsByGroup(context.Background(), "channelID", true)
resp, err := th.App.GetMemberCountsByGroup(th.Context, "channelID", true)
require.Nil(t, err)
require.ElementsMatch(t, cmc, resp)
}

7
server/channels/app/compliance.go
Просмотреть файл

@@ -34,7 +34,7 @@ func (a *App) SaveComplianceReport(rctx request.CTX, job *model.Compliance) (*mo
job.Type = model.ComplianceTypeAdhoc
rctx.SetLogger(rctx.Logger().With(job.LoggerFields()...))
rctx = rctx.WithLogger(rctx.Logger().With(job.LoggerFields()...))
job, err := a.Srv().Store().Compliance().Save(job)
if err != nil {
@@ -48,11 +48,10 @@ func (a *App) SaveComplianceReport(rctx request.CTX, job *model.Compliance) (*mo
}
jCopy := job.DeepCopy()
crctx := rctx.Clone()
a.Srv().Go(func() {
err := a.Compliance().RunComplianceJob(crctx, jCopy)
err := a.Compliance().RunComplianceJob(rctx, jCopy)
if err != nil {
crctx.Logger().Warn("Error running compliance job", mlog.Err(err))
rctx.Logger().Warn("Error running compliance job", mlog.Err(err))
}
})

8
server/channels/app/context.go
Просмотреть файл

@@ -4,16 +4,14 @@
package app
import (
"context"
"github.com/mattermost/mattermost/server/public/plugin"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/mattermost/mattermost/server/v8/channels/store/sqlstore"
)
// WithMaster adds the context value that master DB should be selected for this request.
func WithMaster(ctx context.Context) context.Context {
return sqlstore.WithMaster(ctx)
// RequestContextWithMaster adds the context value that master DB should be selected for this request.
func RequestContextWithMaster(c request.CTX) request.CTX {
return sqlstore.RequestContextWithMaster(c)
}
func pluginContext(c request.CTX) *plugin.Context {

10
server/channels/app/file.go
Просмотреть файл

@@ -796,11 +796,10 @@ func (a *App) UploadFileX(c request.CTX, channelID, name string, input io.Reader
if *a.Config().FileSettings.ExtractContent {
infoCopy := *t.fileinfo
crctx := c.Clone()
a.Srv().GoBuffered(func() {
err := a.ExtractContentFromFileInfo(crctx, &infoCopy)
err := a.ExtractContentFromFileInfo(c, &infoCopy)
if err != nil {
crctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
c.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
}
})
}
@@ -1048,11 +1047,10 @@ func (a *App) DoUploadFileExpectModification(c request.CTX, now time.Time, rawTe
if *a.Config().FileSettings.ExtractContent {
infoCopy := *info
crctx := c.Clone()
a.Srv().GoBuffered(func() {
err := a.ExtractContentFromFileInfo(crctx, &infoCopy)
err := a.ExtractContentFromFileInfo(c, &infoCopy)
if err != nil {
crctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
c.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
}
})
}

1
server/channels/app/helper_test.go
Просмотреть файл

@@ -99,7 +99,6 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo
IncludeCacheLayer: includeCacheLayer,
ConfigStore: configStore,
}
th.Context.SetLogger(testLogger)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.MaxUsersPerTeam = 50 })
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.RateLimitSettings.Enable = false })

2
server/channels/app/import.go
Просмотреть файл

@@ -268,7 +268,7 @@ func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader
linesChan = make(chan imports.LineImportWorkerData, workers)
for i := 0; i < workers; i++ {
wg.Add(1)
go a.bulkImportWorker(c.Clone(), dryRun, &wg, linesChan, errorsChan)
go a.bulkImportWorker(c, dryRun, &wg, linesChan, errorsChan)
}
}

12
server/channels/app/login.go
Просмотреть файл

@@ -156,7 +156,7 @@ func (a *App) GetUserForLogin(c request.CTX, id, loginId string) (*model.User, *
return nil, model.NewAppError("GetUserForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusBadRequest)
}
func (a *App) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) *model.AppError {
func (a *App) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) (*model.Session, *model.AppError) {
var rejectionReason string
pluginContext := pluginContext(c)
a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
@@ -165,7 +165,7 @@ func (a *App) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, use
}, plugin.UserWillLogInID)
if rejectionReason != "" {
return model.NewAppError("DoLogin", "Login rejected by plugin: "+rejectionReason, nil, "", http.StatusBadRequest)
return nil, model.NewAppError("DoLogin", "Login rejected by plugin: "+rejectionReason, nil, "", http.StatusBadRequest)
}
session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceID, IsOAuth: false, Props: map[string]string{
@@ -181,7 +181,7 @@ func (a *App) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, use
// A special case where we logout of all other sessions with the same Id
if err := a.RevokeSessionsForDeviceId(c, user.Id, deviceID, ""); err != nil {
err.StatusCode = http.StatusInternalServerError
return err
return nil, err
}
} else if isMobile {
a.ch.srv.platform.SetSessionExpireInHours(session, *a.Config().ServiceSettings.SessionLengthMobileInHours)
@@ -210,12 +210,12 @@ func (a *App) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, use
var err *model.AppError
if session, err = a.CreateSession(c, session); err != nil {
err.StatusCode = http.StatusInternalServerError
return err
return nil, err
}
w.Header().Set(model.HeaderToken, session.Token)
c.SetSession(session)
c = c.WithSession(session)
if a.Srv().License() != nil && *a.Srv().License().Features.LDAP && a.Ldap() != nil {
userVal := *user
sessionVal := *session
@@ -231,7 +231,7 @@ func (a *App) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, use
}, plugin.UserHasLoggedInID)
})
return nil
return session, nil
}
func (a *App) AttachCloudSessionCookie(c request.CTX, w http.ResponseWriter, r *http.Request) {

14
server/channels/app/opentracing/opentracing_layer.go
Просмотреть файл

@@ -3828,7 +3828,7 @@ func (a *OpenTracingAppLayer) DoLocalRequest(c request.CTX, rawURL string, body
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile bool, isOAuthUser bool, isSaml bool) *model.AppError {
func (a *OpenTracingAppLayer) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile bool, isOAuthUser bool, isSaml bool) (*model.Session, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DoLogin")
@@ -3840,14 +3840,14 @@ func (a *OpenTracingAppLayer) DoLogin(c request.CTX, w http.ResponseWriter, r *h
}()
defer span.Finish()
resultVar0 := a.app.DoLogin(c, w, r, user, deviceID, isMobile, isOAuthUser, isSaml)
resultVar0, resultVar1 := a.app.DoLogin(c, w, r, user, deviceID, isMobile, isOAuthUser, isSaml)
if resultVar0 != nil {
span.LogFields(spanlog.Error(resultVar0))
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))
ext.Error.Set(span, true)
}
return resultVar0
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) DoPermissionsMigrations() error {
@@ -7320,7 +7320,7 @@ func (a *OpenTracingAppLayer) GetMarketplacePlugins(filter *model.MarketplacePlu
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetMemberCountsByGroup(ctx context.Context, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError) {
func (a *OpenTracingAppLayer) GetMemberCountsByGroup(rctx request.CTX, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetMemberCountsByGroup")
@@ -7332,7 +7332,7 @@ func (a *OpenTracingAppLayer) GetMemberCountsByGroup(ctx context.Context, channe
}()
defer span.Finish()
resultVar0, resultVar1 := a.app.GetMemberCountsByGroup(ctx, channelID, includeTimezones)
resultVar0, resultVar1 := a.app.GetMemberCountsByGroup(rctx, channelID, includeTimezones)
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))

2
server/channels/app/plugin_api.go
Просмотреть файл

@@ -1263,7 +1263,7 @@ func (api *PluginAPI) UploadData(us *model.UploadSession, rd io.Reader) (*model.
func (api *PluginAPI) GetUploadSession(uploadID string) (*model.UploadSession, error) {
// We want to fetch from master DB to avoid a potential read-after-write on the plugin side.
api.ctx.SetContext(WithMaster(api.ctx.Context()))
api.ctx = api.ctx.With(RequestContextWithMaster)
fi, err := api.app.GetUploadSession(api.ctx, uploadID)
if err != nil {
return nil, err

11
server/channels/app/plugin_hooks_test.go
Просмотреть файл

@@ -727,9 +727,10 @@ func TestUserWillLogIn_Blocked(t *testing.T) {
r := &http.Request{}
w := httptest.NewRecorder()
err = th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
session, err := th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
assert.Contains(t, err.Id, "Login rejected by plugin", "Expected Login rejected by plugin, got %s", err.Id)
assert.Nil(t, session)
}
func TestUserWillLogInIn_Passed(t *testing.T) {
@@ -766,10 +767,11 @@ func TestUserWillLogInIn_Passed(t *testing.T) {
r := &http.Request{}
w := httptest.NewRecorder()
err = th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
session, err := th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
assert.Nil(t, err, "Expected nil, got %s", err)
assert.Equal(t, th.Context.Session().UserId, th.BasicUser.Id)
require.NotNil(t, session)
assert.Equal(t, session.UserId, th.BasicUser.Id)
}
func TestUserHasLoggedIn(t *testing.T) {
@@ -807,9 +809,10 @@ func TestUserHasLoggedIn(t *testing.T) {
r := &http.Request{}
w := httptest.NewRecorder()
err = th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
session, err := th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
assert.Nil(t, err, "Expected nil, got %s", err)
assert.NotNil(t, session)
time.Sleep(2 * time.Second)

6
server/channels/app/post.go
Просмотреть файл

@@ -532,7 +532,7 @@ func (a *App) handlePostEvents(c request.CTX, post *model.Post, user *model.User
a.Srv().Go(func() {
_, err := a.SendAutoResponseIfNecessary(c, channel, user, post)
if err != nil {
mlog.Error("Failed to send auto response", mlog.String("user_id", user.Id), mlog.String("post_id", post.Id), mlog.Err(err))
c.Logger().Error("Failed to send auto response", mlog.String("user_id", user.Id), mlog.String("post_id", post.Id), mlog.Err(err))
}
})
}
@@ -540,7 +540,7 @@ func (a *App) handlePostEvents(c request.CTX, post *model.Post, user *model.User
if triggerWebhooks {
a.Srv().Go(func() {
if err := a.handleWebhookEvents(c, post, team, channel, user); err != nil {
mlog.Error(err.Error())
c.Logger().Error("Failed to handle webhook event", mlog.Err(err))
}
})
}
@@ -1378,7 +1378,7 @@ func (a *App) DeletePost(c request.CTX, postID, deleteByID string) (*model.Post,
a.Srv().Go(func() {
if err = a.RemoveNotifications(c, post, channel); err != nil {
a.Log().Error("DeletePost failed to delete notification", mlog.Err(err))
c.Logger().Error("DeletePost failed to delete notification", mlog.Err(err))
}
})

8
server/channels/app/team_test.go
Просмотреть файл

@@ -19,6 +19,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/mattermost/mattermost/server/v8/channels/app/email"
emailmocks "github.com/mattermost/mattermost/server/v8/channels/app/email/mocks"
"github.com/mattermost/mattermost/server/v8/channels/app/teams"
@@ -1027,7 +1028,12 @@ func TestLeaveTeamPanic(t *testing.T) {
mockLicenseStore.On("Get", "").Return(&model.LicenseRecord{}, nil)
mockTeamStore := mocks.TeamStore{}
mockTeamStore.On("GetMember", sqlstore.RequestContextWithMaster(th.Context), "myteam", "userID").Return(&model.TeamMember{TeamId: "myteam", UserId: "userID"}, nil)
mockTeamStore.On("GetMember", mock.AnythingOfType("*request.Context"), "myteam", "userID").Return(&model.TeamMember{TeamId: "myteam", UserId: "userID"}, nil).Run(func(args mock.Arguments) {
c, ok := args[0].(request.CTX)
require.True(t, ok)
sqlstore.HasMaster(c.Context())
})
mockTeamStore.On("UpdateMember", mock.Anything).Return(nil, errors.New("repro error")) // This is the line that triggers the error
mockStore.On("Channel").Return(&mockChannelStore)

7
server/channels/app/upload.go
Просмотреть файл

@@ -203,7 +203,7 @@ func (a *App) UploadData(c request.CTX, us *model.UploadSession, rd io.Reader) (
}()
// fetch the session from store to check for inconsistencies.
c.SetContext(WithMaster(c.Context()))
c = c.With(RequestContextWithMaster)
if storedSession, err := a.GetUploadSession(c, us.Id); err != nil {
return nil, err
} else if us.FileOffset != storedSession.FileOffset {
@@ -318,11 +318,10 @@ func (a *App) UploadData(c request.CTX, us *model.UploadSession, rd io.Reader) (
if *a.Config().FileSettings.ExtractContent {
infoCopy := *info
crctx := c.Clone()
a.Srv().Go(func() {
err := a.ExtractContentFromFileInfo(crctx, &infoCopy)
err := a.ExtractContentFromFileInfo(c, &infoCopy)
if err != nil {
crctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
c.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
}
})
}