diff --git a/api4/file_test.go b/api4/file_test.go index a25c02d72a..7e3702c762 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -20,9 +20,9 @@ import ( "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/mattermost/mattermost-server/utils/testutils" + "github.com/stretchr/testify/require" ) var testDir = "" @@ -680,13 +680,8 @@ func TestUploadFiles(t *testing.T) { fmt.Sprintf("Wrong clientId returned, expected %v, got %v", tc.clientIds[i], fileResp.ClientIds[i])) } - var dbInfo *model.FileInfo - result := <-th.App.Srv.Store.FileInfo().Get(ri.Id) - if result.Err != nil { - t.Error(result.Err) - } else { - dbInfo = result.Data.(*model.FileInfo) - } + dbInfo, err := th.App.Srv.Store.FileInfo().Get(ri.Id) + require.Nil(t, err) checkEq(t, dbInfo.Id, ri.Id, "File id from response should match one stored in database") checkEq(t, dbInfo.CreatorId, tc.expectedCreatorId, "F ile should be assigned to user") checkEq(t, dbInfo.PostId, "", "File shouldn't have a post") @@ -957,7 +952,8 @@ func TestGetFileLink(t *testing.T) { CheckBadRequestStatus(t, resp) // Hacky way to assign file to a post (usually would be done by CreatePost call) - store.Must(th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id)) + err := th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id) + require.Nil(t, err) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = false }) _, resp = Client.GetFileLink(fileId) @@ -993,11 +989,9 @@ func TestGetFileLink(t *testing.T) { _, resp = th.SystemAdminClient.GetFileLink(fileId) CheckNoError(t, resp) - if result := <-th.App.Srv.Store.FileInfo().Get(fileId); result.Err != nil { - t.Fatal(result.Err) - } else { - th.cleanupTestFile(result.Data.(*model.FileInfo)) - } + fileInfo, err := th.App.Srv.Store.FileInfo().Get(fileId) + require.Nil(t, err) + th.cleanupTestFile(fileInfo) } func TestGetFilePreview(t *testing.T) { @@ -1139,10 +1133,11 @@ func TestGetPublicFile(t *testing.T) { } // Hacky way to assign file to a post (usually would be done by CreatePost call) - store.Must(th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id)) + err := th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id) + require.Nil(t, err) - result := <-th.App.Srv.Store.FileInfo().Get(fileId) - info := result.Data.(*model.FileInfo) + info, err := th.App.Srv.Store.FileInfo().Get(fileId) + require.Nil(t, err) link := th.App.GeneratePublicLink(Client.Url, info) // Wait a bit for files to ready @@ -1174,9 +1169,8 @@ func TestGetPublicFile(t *testing.T) { t.Fatal("should've failed to get image with public link after salt changed") } - if err := th.cleanupTestFile(store.Must(th.App.Srv.Store.FileInfo().Get(fileId)).(*model.FileInfo)); err != nil { - t.Fatal(err) - } - + fileInfo, err := th.App.Srv.Store.FileInfo().Get(fileId) + require.Nil(t, err) + require.Nil(t, th.cleanupTestFile(fileInfo)) th.cleanupTestFile(info) } diff --git a/app/file.go b/app/file.go index b15efa147d..73759b4012 100644 --- a/app/file.go +++ b/app/file.go @@ -30,7 +30,6 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" "github.com/mattermost/mattermost-server/services/filesstore" - "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils" ) @@ -276,14 +275,14 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo { if newPost := result.Data.(*model.PostList).Posts[post.Id]; len(newPost.Filenames) != len(post.Filenames) { // Another thread has already created FileInfos for this post, so just return those - result := <-a.Srv.Store.FileInfo().GetForPost(post.Id, true, false) - if result.Err != nil { + fileInfos, err := a.Srv.Store.FileInfo().GetForPost(post.Id, true, false) + if err != nil { mlog.Error(fmt.Sprintf("Unable to get FileInfos for migrated post, err=%v", result.Err), mlog.String("post_id", post.Id)) return []*model.FileInfo{} } mlog.Debug("Post already migrated to use FileInfos", mlog.String("post_id", post.Id)) - return result.Data.([]*model.FileInfo) + return fileInfos } mlog.Debug("Migrating post to use FileInfos", mlog.String("post_id", post.Id)) @@ -291,9 +290,9 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo { savedInfos := make([]*model.FileInfo, 0, len(infos)) fileIds := make([]string, 0, len(filenames)) for _, info := range infos { - if result := <-a.Srv.Store.FileInfo().Save(info); result.Err != nil { + if _, err := a.Srv.Store.FileInfo().Save(info); err != nil { mlog.Error( - fmt.Sprintf("Unable to save file info when migrating post to use FileInfos, err=%v", result.Err), + fmt.Sprintf("Unable to save file info when migrating post to use FileInfos, err=%v", err), mlog.String("post_id", post.Id), mlog.String("file_info_id", info.Id), mlog.String("file_info_path", info.Path), @@ -506,7 +505,7 @@ type uploadFileTask struct { // Testing: overrideable dependency functions pluginsEnvironment *plugin.Environment writeFile func(io.Reader, string) (int64, *model.AppError) - saveToDatabase func(*model.FileInfo) store.StoreChannel + saveToDatabase func(*model.FileInfo) (*model.FileInfo, *model.AppError) } func (t *uploadFileTask) init(a *App) { @@ -605,8 +604,8 @@ func (a *App) UploadFileX(channelId, name string, input io.Reader, return nil, aerr } - if result := <-t.saveToDatabase(t.fileinfo); result.Err != nil { - return nil, result.Err + if _, err := t.saveToDatabase(t.fileinfo); err != nil { + return nil, err } wg.Wait() @@ -915,8 +914,8 @@ func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, ra return nil, data, err } - if result := <-a.Srv.Store.FileInfo().Save(info); result.Err != nil { - return nil, data, result.Err + if _, err := a.Srv.Store.FileInfo().Save(info); err != nil { + return nil, data, err } return info, data, nil @@ -1059,11 +1058,7 @@ func (a *App) generatePreviewImage(img image.Image, previewPath string, width in } func (a *App) GetFileInfo(fileId string) (*model.FileInfo, *model.AppError) { - result := <-a.Srv.Store.FileInfo().Get(fileId) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.FileInfo), nil + return a.Srv.Store.FileInfo().Get(fileId) } func (a *App) GetFile(fileId string) ([]byte, *model.AppError) { @@ -1086,21 +1081,19 @@ func (a *App) CopyFileInfos(userId string, fileIds []string) ([]string, *model.A now := model.GetMillis() for _, fileId := range fileIds { - result := <-a.Srv.Store.FileInfo().Get(fileId) - - if result.Err != nil { - return nil, result.Err + fileInfo, err := a.Srv.Store.FileInfo().Get(fileId) + if err != nil { + return nil, err } - fileInfo := result.Data.(*model.FileInfo) fileInfo.Id = model.NewId() fileInfo.CreatorId = userId fileInfo.CreateAt = now fileInfo.UpdateAt = now fileInfo.PostId = "" - if result := <-a.Srv.Store.FileInfo().Save(fileInfo); result.Err != nil { - return newFileIds, result.Err + if _, err := a.Srv.Store.FileInfo().Save(fileInfo); err != nil { + return newFileIds, err } newFileIds = append(newFileIds, fileInfo.Id) diff --git a/app/file_bench_test.go b/app/file_bench_test.go index 1960b43db7..57fc96a9a5 100644 --- a/app/file_bench_test.go +++ b/app/file_bench_test.go @@ -91,7 +91,7 @@ func BenchmarkUploadFile(b *testing.B) { if err != nil { b.Fatal(err) } - <-th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) th.App.RemoveFile(info1.Path) }, @@ -110,7 +110,7 @@ func BenchmarkUploadFile(b *testing.B) { if aerr != nil { b.Fatal(aerr) } - <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) th.App.RemoveFile(info.Path) }, }, @@ -128,7 +128,7 @@ func BenchmarkUploadFile(b *testing.B) { if aerr != nil { b.Fatal(aerr) } - <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) th.App.RemoveFile(info.Path) }, }, @@ -143,7 +143,7 @@ func BenchmarkUploadFile(b *testing.B) { if err != nil { b.Fatal(err) } - <-th.App.Srv.Store.FileInfo().PermanentDelete(resp.FileInfos[0].Id) + th.App.Srv.Store.FileInfo().PermanentDelete(resp.FileInfos[0].Id) th.App.RemoveFile(resp.FileInfos[0].Path) }, }, @@ -160,7 +160,7 @@ func BenchmarkUploadFile(b *testing.B) { if aerr != nil { b.Fatal(aerr) } - <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) th.App.RemoveFile(info.Path) }, }, @@ -177,7 +177,7 @@ func BenchmarkUploadFile(b *testing.B) { if aerr != nil { b.Fatal(aerr) } - <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) th.App.RemoveFile(info.Path) }, }, diff --git a/app/file_test.go b/app/file_test.go index 4848ba0391..dc9321e054 100644 --- a/app/file_test.go +++ b/app/file_test.go @@ -55,7 +55,7 @@ func TestDoUploadFile(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) th.App.RemoveFile(info1.Path) }() } @@ -69,7 +69,7 @@ func TestDoUploadFile(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info2.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info2.Id) th.App.RemoveFile(info2.Path) }() } @@ -83,7 +83,7 @@ func TestDoUploadFile(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info3.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info3.Id) th.App.RemoveFile(info3.Path) }() } @@ -97,7 +97,7 @@ func TestDoUploadFile(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info4.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info4.Id) th.App.RemoveFile(info4.Path) }() } @@ -120,7 +120,7 @@ func TestUploadFile(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) th.App.RemoveFile(info1.Path) }() } @@ -199,7 +199,7 @@ func TestCopyFileInfos(t *testing.T) { info1, err := th.App.DoUploadFile(time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamId, channelId, userId, filename, data) require.Nil(t, err) defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) th.App.RemoveFile(info1.Path) }() @@ -209,7 +209,7 @@ func TestCopyFileInfos(t *testing.T) { info2, err := th.App.GetFileInfo(infoIds[0]) require.Nil(t, err) defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info2.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info2.Id) th.App.RemoveFile(info2.Path) }() diff --git a/app/import_functions.go b/app/import_functions.go index 2eb066b905..240c914437 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1064,8 +1064,8 @@ func (a *App) uploadAttachments(attachments *[]AttachmentImportData, post *model func (a *App) UpdateFileInfoWithPostId(post *model.Post) { for _, fileId := range post.FileIds { - if result := <-a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId); result.Err != nil { - mlog.Error(fmt.Sprintf("Error attaching files to post. postId=%v, fileIds=%v, message=%v", post.Id, post.FileIds, result.Err), mlog.String("post_id", post.Id)) + if err := a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId); err != nil { + mlog.Error(fmt.Sprintf("Error attaching files to post. postId=%v, fileIds=%v, message=%v", post.Id, post.FileIds, err), mlog.String("post_id", post.Id)) } } } diff --git a/app/import_test.go b/app/import_test.go index 1afa13f705..966315fb15 100644 --- a/app/import_test.go +++ b/app/import_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils/fileutils" @@ -250,12 +251,9 @@ func TestImportProcessImportDataFileVersionLine(t *testing.T) { } func GetAttachments(userId string, th *TestHelper, t *testing.T) []*model.FileInfo { - if result := <-th.App.Srv.Store.FileInfo().GetForUser(userId); result.Err != nil { - t.Fatal(result.Err.Error()) - } else { - return result.Data.([]*model.FileInfo) - } - return nil + fileInfos, err := th.App.Srv.Store.FileInfo().GetForUser(userId) + require.Nil(t, err) + return fileInfos } func AssertFileIdsInPost(files []*model.FileInfo, th *TestHelper, t *testing.T) { diff --git a/app/notification.go b/app/notification.go index 421464e083..21123f88b6 100644 --- a/app/notification.go +++ b/app/notification.go @@ -30,10 +30,14 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod pchan := a.Srv.Store.User().GetAllProfilesInChannel(channel.Id, true) cmnchan := a.Srv.Store.Channel().GetAllChannelMembersNotifyPropsForChannel(channel.Id, true) - var fchan store.StoreChannel - + var fchan chan store.StoreResult if len(post.FileIds) != 0 { - fchan = a.Srv.Store.FileInfo().GetForPost(post.Id, true, true) + fchan = make(chan store.StoreResult, 1) + go func() { + fileInfos, err := a.Srv.Store.FileInfo().GetForPost(post.Id, true, true) + fchan <- store.StoreResult{Data: fileInfos, Err: err} + close(fchan) + }() } result := <-pchan diff --git a/app/notification_email.go b/app/notification_email.go index c86a279f02..a6b5d548c8 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -290,11 +290,10 @@ func (a *App) GetMessageForNotification(post *model.Post, translateFunc i18n.Tra } // extract the filenames from their paths and determine what type of files are attached - result := <-a.Srv.Store.FileInfo().GetForPost(post.Id, true, true) - if result.Err != nil { - mlog.Warn(fmt.Sprintf("Encountered error when getting files for notification message, post_id=%v, err=%v", post.Id, result.Err), mlog.String("post_id", post.Id)) + infos, err := a.Srv.Store.FileInfo().GetForPost(post.Id, true, true) + if err != nil { + mlog.Warn(fmt.Sprintf("Encountered error when getting files for notification message, post_id=%v, err=%v", post.Id, err), mlog.String("post_id", post.Id)) } - infos := result.Data.([]*model.FileInfo) filenames := make([]string, len(infos)) onlyImages := true diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index 381743cb8d..03557dd640 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -291,7 +291,7 @@ func TestPluginAPIGetFile(t *testing.T) { info, err := th.App.DoUploadFile(uploadTime, th.BasicTeam.Id, th.BasicChannel.Id, th.BasicUser.Id, filename, fileData) require.Nil(t, err) defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info.Id) th.App.RemoveFile(info.Path) }() diff --git a/app/post.go b/app/post.go index d92c3aa540..26fd2789ee 100644 --- a/app/post.go +++ b/app/post.go @@ -302,9 +302,9 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo func (a *App) attachFilesToPost(post *model.Post) *model.AppError { var attachedIds []string for _, fileId := range post.FileIds { - result := <-a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId) - if result.Err != nil { - mlog.Warn("Failed to attach file to post", mlog.String("file_id", fileId), mlog.String("post_id", post.Id), mlog.Err(result.Err)) + err := a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId) + if err != nil { + mlog.Warn("Failed to attach file to post", mlog.String("file_id", fileId), mlog.String("post_id", post.Id), mlog.Err(err)) continue } @@ -776,8 +776,8 @@ func (a *App) DeletePostFiles(post *model.Post) { return } - if result := <-a.Srv.Store.FileInfo().DeleteForPost(post.Id); result.Err != nil { - mlog.Warn(fmt.Sprintf("Encountered error when deleting files for post, post_id=%v, err=%v", post.Id, result.Err), mlog.String("post_id", post.Id)) + if _, err := a.Srv.Store.FileInfo().DeleteForPost(post.Id); err != nil { + mlog.Warn(fmt.Sprintf("Encountered error when deleting files for post, post_id=%v, err=%v", post.Id, err), mlog.String("post_id", post.Id)) } } @@ -980,12 +980,7 @@ func (a *App) GetFileInfosForPostWithMigration(postId string) ([]*model.FileInfo } func (a *App) GetFileInfosForPost(postId string) ([]*model.FileInfo, *model.AppError) { - result := <-a.Srv.Store.FileInfo().GetForPost(postId, false, true) - if result.Err != nil { - return nil, result.Err - } - - return result.Data.([]*model.FileInfo), nil + return a.Srv.Store.FileInfo().GetForPost(postId, false, true) } func (a *App) PostWithProxyAddedToImageURLs(post *model.Post) *model.Post { diff --git a/app/post_test.go b/app/post_test.go index 40a59b07c8..aa9f317c55 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -191,19 +191,22 @@ func TestAttachFilesToPost(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - info1 := store.Must(th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ + info1, err := th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ CreatorId: th.BasicUser.Id, Path: "path.txt", - })).(*model.FileInfo) - info2 := store.Must(th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ + }) + require.Nil(t, err) + + info2, err := th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ CreatorId: th.BasicUser.Id, Path: "path.txt", - })).(*model.FileInfo) + }) + require.Nil(t, err) post := th.BasicPost post.FileIds = []string{info1.Id, info2.Id} - err := th.App.attachFilesToPost(post) + err = th.App.attachFilesToPost(post) assert.Nil(t, err) infos, err := th.App.GetFileInfosForPost(post.Id) @@ -215,20 +218,23 @@ func TestAttachFilesToPost(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - info1 := store.Must(th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ + info1, err := th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ CreatorId: th.BasicUser.Id, Path: "path.txt", PostId: model.NewId(), - })).(*model.FileInfo) - info2 := store.Must(th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ + }) + require.Nil(t, err) + + info2, err := th.App.Srv.Store.FileInfo().Save(&model.FileInfo{ CreatorId: th.BasicUser.Id, Path: "path.txt", - })).(*model.FileInfo) + }) + require.Nil(t, err) post := th.BasicPost post.FileIds = []string{info1.Id, info2.Id} - err := th.App.attachFilesToPost(post) + err = th.App.attachFilesToPost(post) assert.Nil(t, err) infos, err := th.App.GetFileInfosForPost(post.Id) @@ -623,7 +629,7 @@ func TestDeletePostWithFileAttachments(t *testing.T) { t.Fatal(err) } else { defer func() { - <-th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) + th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id) th.App.RemoveFile(info1.Path) }() } diff --git a/app/slackimport.go b/app/slackimport.go index ed9006695e..b95c8a3f39 100644 --- a/app/slackimport.go +++ b/app/slackimport.go @@ -747,8 +747,8 @@ func (a *App) OldImportPost(post *model.Post) string { firstPostId = post.Id } for _, fileId := range post.FileIds { - if result := <-a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId); result.Err != nil { - mlog.Error(fmt.Sprintf("Error attaching files to post. postId=%v, fileIds=%v, message=%v", post.Id, post.FileIds, result.Err), mlog.String("post_id", post.Id)) + if err := a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId); err != nil { + mlog.Error(fmt.Sprintf("Error attaching files to post. postId=%v, fileIds=%v, message=%v", post.Id, post.FileIds, err), mlog.String("post_id", post.Id)) } } post.FileIds = nil diff --git a/app/user.go b/app/user.go index 0e31237414..8ddc428e8b 100644 --- a/app/user.go +++ b/app/user.go @@ -1447,12 +1447,11 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError { return result.Err } - result := <-a.Srv.Store.FileInfo().GetForUser(user.Id) - if result.Err != nil { + infos, err := a.Srv.Store.FileInfo().GetForUser(user.Id) + if err != nil { mlog.Warn("Error getting file list for user from FileInfoStore") } - infos := result.Data.([]*model.FileInfo) for _, info := range infos { res, err := a.FileExists(info.Path) if err != nil { @@ -1480,8 +1479,8 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError { } } - if result := <-a.Srv.Store.FileInfo().PermanentDeleteByUser(user.Id); result.Err != nil { - return result.Err + if _, err := a.Srv.Store.FileInfo().PermanentDeleteByUser(user.Id); err != nil { + return err } if result := <-a.Srv.Store.User().PermanentDelete(user.Id); result.Err != nil { diff --git a/store/sqlstore/file_info_store.go b/store/sqlstore/file_info_store.go index 92cd8966ca..444181c82f 100644 --- a/store/sqlstore/file_info_store.go +++ b/store/sqlstore/file_info_store.go @@ -61,50 +61,42 @@ func (fs SqlFileInfoStore) CreateIndexesIfNotExists() { fs.CreateIndexIfNotExists("idx_fileinfo_postid_at", "FileInfo", "PostId") } -func (fs SqlFileInfoStore) Save(info *model.FileInfo) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - info.PreSave() - if result.Err = info.IsValid(); result.Err != nil { - return - } +func (fs SqlFileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, *model.AppError) { + info.PreSave() + if err := info.IsValid(); err != nil { + return nil, err + } - if err := fs.GetMaster().Insert(info); err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.Save", "store.sql_file_info.save.app_error", nil, err.Error(), http.StatusInternalServerError) - } else { - result.Data = info - } - }) + if err := fs.GetMaster().Insert(info); err != nil { + return nil, model.NewAppError("SqlFileInfoStore.Save", "store.sql_file_info.save.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return info, nil } -func (fs SqlFileInfoStore) Get(id string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - info := &model.FileInfo{} +func (fs SqlFileInfoStore) Get(id string) (*model.FileInfo, *model.AppError) { + info := &model.FileInfo{} - if err := fs.GetReplica().SelectOne(info, - `SELECT - * - FROM - FileInfo - WHERE - Id = :Id - AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil { - if err == sql.ErrNoRows { - result.Err = model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound) - } else { - result.Err = model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) - } - } else { - result.Data = info + if err := fs.GetReplica().SelectOne(info, + `SELECT + * + FROM + FileInfo + WHERE + Id = :Id + AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil { + if err == sql.ErrNoRows { + return nil, model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound) } - }) + return nil, model.NewAppError("SqlFileInfoStore.Get", "store.sql_file_info.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + } + return info, nil } -func (fs SqlFileInfoStore) GetByPath(path string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - info := &model.FileInfo{} +func (fs SqlFileInfoStore) GetByPath(path string) (*model.FileInfo, *model.AppError) { + info := &model.FileInfo{} - if err := fs.GetReplica().SelectOne(info, - `SELECT + if err := fs.GetReplica().SelectOne(info, + `SELECT * FROM FileInfo @@ -112,11 +104,9 @@ func (fs SqlFileInfoStore) GetByPath(path string) store.StoreChannel { Path = :Path AND DeleteAt = 0 LIMIT 1`, map[string]interface{}{"Path": path}); err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.GetByPath", "store.sql_file_info.get_by_path.app_error", nil, "path="+path+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = info - } - }) + return nil, model.NewAppError("SqlFileInfoStore.GetByPath", "store.sql_file_info.get_by_path.app_error", nil, "path="+path+", "+err.Error(), http.StatusInternalServerError) + } + return info, nil } func (fs SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) { @@ -126,37 +116,34 @@ func (fs SqlFileInfoStore) InvalidateFileInfosForPostCache(postId string) { } } -func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if allowFromCache { - if cacheItem, ok := fileInfoCache.Get(postId); ok { - if fs.metrics != nil { - fs.metrics.IncrementMemCacheHitCounter("File Info Cache") - } - - result.Data = cacheItem.([]*model.FileInfo) - return - } else { - if fs.metrics != nil { - fs.metrics.IncrementMemCacheMissCounter("File Info Cache") - } - } - } else { +func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) ([]*model.FileInfo, *model.AppError) { + if allowFromCache { + if cacheItem, ok := fileInfoCache.Get(postId); ok { if fs.metrics != nil { - fs.metrics.IncrementMemCacheMissCounter("File Info Cache") + fs.metrics.IncrementMemCacheHitCounter("File Info Cache") } + + return cacheItem.([]*model.FileInfo), nil } - - var infos []*model.FileInfo - - dbmap := fs.GetReplica() - - if readFromMaster { - dbmap = fs.GetMaster() + if fs.metrics != nil { + fs.metrics.IncrementMemCacheMissCounter("File Info Cache") } + } else { + if fs.metrics != nil { + fs.metrics.IncrementMemCacheMissCounter("File Info Cache") + } + } - if _, err := dbmap.Select(&infos, - `SELECT + var infos []*model.FileInfo + + dbmap := fs.GetReplica() + + if readFromMaster { + dbmap = fs.GetMaster() + } + + if _, err := dbmap.Select(&infos, + `SELECT * FROM FileInfo @@ -165,26 +152,23 @@ func (fs SqlFileInfoStore) GetForPost(postId string, readFromMaster bool, allowF AND DeleteAt = 0 ORDER BY CreateAt`, map[string]interface{}{"PostId": postId}); err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.GetForPost", - "store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error(), http.StatusInternalServerError) - } else { - if len(infos) > 0 { - fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC) - } + return nil, model.NewAppError("SqlFileInfoStore.GetForPost", + "store.sql_file_info.get_for_post.app_error", nil, "post_id="+postId+", "+err.Error(), http.StatusInternalServerError) + } + if len(infos) > 0 { + fileInfoCache.AddWithExpiresInSecs(postId, infos, FILE_INFO_CACHE_SEC) + } - result.Data = infos - } - }) + return infos, nil } -func (fs SqlFileInfoStore) GetForUser(userId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var infos []*model.FileInfo +func (fs SqlFileInfoStore) GetForUser(userId string) ([]*model.FileInfo, *model.AppError) { + var infos []*model.FileInfo - dbmap := fs.GetReplica() + dbmap := fs.GetReplica() - if _, err := dbmap.Select(&infos, - `SELECT + if _, err := dbmap.Select(&infos, + `SELECT * FROM FileInfo @@ -193,18 +177,15 @@ func (fs SqlFileInfoStore) GetForUser(userId string) store.StoreChannel { AND DeleteAt = 0 ORDER BY CreateAt`, map[string]interface{}{"CreatorId": userId}); err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.GetForPost", - "store.sql_file_info.get_for_user_id.app_error", nil, "creator_id="+userId+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = infos - } - }) + return nil, model.NewAppError("SqlFileInfoStore.GetForPost", + "store.sql_file_info.get_for_user_id.app_error", nil, "creator_id="+userId+", "+err.Error(), http.StatusInternalServerError) + } + return infos, nil } -func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - sqlResult, err := fs.GetMaster().Exec( - `UPDATE +func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) *model.AppError { + sqlResult, err := fs.GetMaster().Exec( + `UPDATE FileInfo SET PostId = :PostId @@ -212,94 +193,80 @@ func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) store. Id = :Id AND PostId = '' AND CreatorId = :CreatorId`, map[string]interface{}{"PostId": postId, "Id": fileId, "CreatorId": creatorId}) - if err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.AttachToPost", - "store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError) - return - } + if err != nil { + return model.NewAppError("SqlFileInfoStore.AttachToPost", + "store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError) + } - count, err := sqlResult.RowsAffected() - if err != nil { - // RowsAffected should never fail with the MySQL or Postgres drivers - result.Err = model.NewAppError("SqlFileInfoStore.AttachToPost", - "store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError) - } else if count == 0 { - // Could not attach the file to the post - result.Err = model.NewAppError("SqlFileInfoStore.AttachToPost", - "store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId, http.StatusBadRequest) - } - }) + count, err := sqlResult.RowsAffected() + if err != nil { + // RowsAffected should never fail with the MySQL or Postgres drivers + return model.NewAppError("SqlFileInfoStore.AttachToPost", + "store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError) + } else if count == 0 { + // Could not attach the file to the post + return model.NewAppError("SqlFileInfoStore.AttachToPost", + "store.sql_file_info.attach_to_post.app_error", nil, "post_id="+postId+", file_id="+fileId, http.StatusBadRequest) + } + return nil } -func (fs SqlFileInfoStore) DeleteForPost(postId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if _, err := fs.GetMaster().Exec( - `UPDATE +func (fs SqlFileInfoStore) DeleteForPost(postId string) (string, *model.AppError) { + if _, err := fs.GetMaster().Exec( + `UPDATE FileInfo SET DeleteAt = :DeleteAt WHERE PostId = :PostId`, map[string]interface{}{"DeleteAt": model.GetMillis(), "PostId": postId}); err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.DeleteForPost", - "store.sql_file_info.delete_for_post.app_error", nil, "post_id="+postId+", err="+err.Error(), http.StatusInternalServerError) - } else { - result.Data = postId - } - }) + return "", model.NewAppError("SqlFileInfoStore.DeleteForPost", + "store.sql_file_info.delete_for_post.app_error", nil, "post_id="+postId+", err="+err.Error(), http.StatusInternalServerError) + } + return postId, nil } -func (fs SqlFileInfoStore) PermanentDelete(fileId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if _, err := fs.GetMaster().Exec( - `DELETE FROM +func (fs SqlFileInfoStore) PermanentDelete(fileId string) *model.AppError { + if _, err := fs.GetMaster().Exec( + `DELETE FROM FileInfo WHERE Id = :FileId`, map[string]interface{}{"FileId": fileId}); err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.PermanentDelete", - "store.sql_file_info.permanent_delete.app_error", nil, "file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError) - } - }) + return model.NewAppError("SqlFileInfoStore.PermanentDelete", + "store.sql_file_info.permanent_delete.app_error", nil, "file_id="+fileId+", err="+err.Error(), http.StatusInternalServerError) + } + return nil } -func (s SqlFileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var query string - if s.DriverName() == "postgres" { - query = "DELETE from FileInfo WHERE Id = any (array (SELECT Id FROM FileInfo WHERE CreateAt < :EndTime LIMIT :Limit))" - } else { - query = "DELETE from FileInfo WHERE CreateAt < :EndTime LIMIT :Limit" - } +func (s SqlFileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) { + var query string + if s.DriverName() == "postgres" { + query = "DELETE from FileInfo WHERE Id = any (array (SELECT Id FROM FileInfo WHERE CreateAt < :EndTime LIMIT :Limit))" + } else { + query = "DELETE from FileInfo WHERE CreateAt < :EndTime LIMIT :Limit" + } - sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"EndTime": endTime, "Limit": limit}) - if err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) - } else { - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { - result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) - result.Data = int64(0) - } else { - result.Data = rowsAffected - } - } - }) + sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"EndTime": endTime, "Limit": limit}) + if err != nil { + return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) + } + rowsAffected, err1 := sqlResult.RowsAffected() + if err1 != nil { + return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteBatch", "store.sql_file_info.permanent_delete_batch.app_error", nil, ""+err.Error(), http.StatusInternalServerError) + } + return rowsAffected, nil } -func (s SqlFileInfoStore) PermanentDeleteByUser(userId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - query := "DELETE from FileInfo WHERE CreatorId = :CreatorId" +func (s SqlFileInfoStore) PermanentDeleteByUser(userId string) (int64, *model.AppError) { + query := "DELETE from FileInfo WHERE CreatorId = :CreatorId" - sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"CreatorId": userId}) - if err != nil { - result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError) - } else { - rowsAffected, err1 := sqlResult.RowsAffected() - if err1 != nil { - result.Err = model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError) - result.Data = int64(0) - } else { - result.Data = rowsAffected - } - } - }) + sqlResult, err := s.GetMaster().Exec(query, map[string]interface{}{"CreatorId": userId}) + if err != nil { + return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError) + } + + rowsAffected, err1 := sqlResult.RowsAffected() + if err1 != nil { + return 0, model.NewAppError("SqlFileInfoStore.PermanentDeleteByUser", "store.sql_file_info.PermanentDeleteByUser.app_error", nil, ""+err.Error(), http.StatusInternalServerError) + } + return rowsAffected, nil } diff --git a/store/store.go b/store/store.go index aafbc15d99..462f79c0ad 100644 --- a/store/store.go +++ b/store/store.go @@ -476,17 +476,17 @@ type StatusStore interface { } type FileInfoStore interface { - Save(info *model.FileInfo) StoreChannel - Get(id string) StoreChannel - GetByPath(path string) StoreChannel - GetForPost(postId string, readFromMaster bool, allowFromCache bool) StoreChannel - GetForUser(userId string) StoreChannel + Save(info *model.FileInfo) (*model.FileInfo, *model.AppError) + Get(id string) (*model.FileInfo, *model.AppError) + GetByPath(path string) (*model.FileInfo, *model.AppError) + GetForPost(postId string, readFromMaster bool, allowFromCache bool) ([]*model.FileInfo, *model.AppError) + GetForUser(userId string) ([]*model.FileInfo, *model.AppError) InvalidateFileInfosForPostCache(postId string) - AttachToPost(fileId string, postId string, creatorId string) StoreChannel - DeleteForPost(postId string) StoreChannel - PermanentDelete(fileId string) StoreChannel - PermanentDeleteBatch(endTime int64, limit int64) StoreChannel - PermanentDeleteByUser(userId string) StoreChannel + AttachToPost(fileId string, postId string, creatorId string) *model.AppError + DeleteForPost(postId string) (string, *model.AppError) + PermanentDelete(fileId string) *model.AppError + PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) + PermanentDeleteByUser(userId string) (int64, *model.AppError) ClearCaches() } diff --git a/store/storetest/file_info_store.go b/store/storetest/file_info_store.go index d6ed6982e8..c18a71b290 100644 --- a/store/storetest/file_info_store.go +++ b/store/storetest/file_info_store.go @@ -32,36 +32,30 @@ func testFileInfoSaveGet(t *testing.T, ss store.Store) { Path: "file.txt", } - if result := <-ss.FileInfo().Save(info); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.(*model.FileInfo); len(returned.Id) == 0 { - t.Fatal("should've assigned an id to FileInfo") - } else { - info = returned - } + info, err := ss.FileInfo().Save(info) + require.Nil(t, err) + require.NotEqual(t, len(info.Id), 0) + defer func() { - <-ss.FileInfo().PermanentDelete(info.Id) + ss.FileInfo().PermanentDelete(info.Id) }() - if result := <-ss.FileInfo().Get(info.Id); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.(*model.FileInfo); returned.Id != info.Id { - t.Log(info) - t.Log(returned) - t.Fatal("should've returned correct FileInfo") - } + rinfo, err := ss.FileInfo().Get(info.Id) + require.Nil(t, err) + require.Equal(t, info.Id, rinfo.Id) - info2 := store.Must(ss.FileInfo().Save(&model.FileInfo{ + info2, err := ss.FileInfo().Save(&model.FileInfo{ CreatorId: model.NewId(), Path: "file.txt", DeleteAt: 123, - })).(*model.FileInfo) + }) + require.Nil(t, err) + + _, err = ss.FileInfo().Get(info2.Id) + assert.NotNil(t, err) - if result := <-ss.FileInfo().Get(info2.Id); result.Err == nil { - t.Fatal("shouldn't have gotten deleted file") - } defer func() { - <-ss.FileInfo().PermanentDelete(info2.Id) + ss.FileInfo().PermanentDelete(info2.Id) }() } @@ -71,36 +65,29 @@ func testFileInfoSaveGetByPath(t *testing.T, ss store.Store) { Path: fmt.Sprintf("%v/file.txt", model.NewId()), } - if result := <-ss.FileInfo().Save(info); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.(*model.FileInfo); len(returned.Id) == 0 { - t.Fatal("should've assigned an id to FileInfo") - } else { - info = returned - } + info, err := ss.FileInfo().Save(info) + require.Nil(t, err) + assert.NotEqual(t, len(info.Id), 0) defer func() { - <-ss.FileInfo().PermanentDelete(info.Id) + ss.FileInfo().PermanentDelete(info.Id) }() - if result := <-ss.FileInfo().GetByPath(info.Path); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.(*model.FileInfo); returned.Id != info.Id { - t.Log(info) - t.Log(returned) - t.Fatal("should've returned correct FileInfo") - } + rinfo, err := ss.FileInfo().GetByPath(info.Path) + require.Nil(t, err) + assert.Equal(t, info.Id, rinfo.Id) - info2 := store.Must(ss.FileInfo().Save(&model.FileInfo{ + info2, err := ss.FileInfo().Save(&model.FileInfo{ CreatorId: model.NewId(), Path: "file.txt", DeleteAt: 123, - })).(*model.FileInfo) + }) + require.Nil(t, err) + + _, err = ss.FileInfo().GetByPath(info2.Id) + assert.NotNil(t, err) - if result := <-ss.FileInfo().GetByPath(info2.Id); result.Err == nil { - t.Fatal("shouldn't have gotten deleted file") - } defer func() { - <-ss.FileInfo().PermanentDelete(info2.Id) + ss.FileInfo().PermanentDelete(info2.Id) }() } @@ -133,29 +120,25 @@ func testFileInfoGetForPost(t *testing.T, ss store.Store) { } for i, info := range infos { - infos[i] = store.Must(ss.FileInfo().Save(info)).(*model.FileInfo) + newInfo, err := ss.FileInfo().Save(info) + require.Nil(t, err) + infos[i] = newInfo defer func(id string) { - <-ss.FileInfo().PermanentDelete(id) - }(infos[i].Id) + ss.FileInfo().PermanentDelete(id) + }(newInfo.Id) } - if result := <-ss.FileInfo().GetForPost(postId, true, false); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 { - t.Fatal("should've returned exactly 2 file infos") - } + postInfos, err := ss.FileInfo().GetForPost(postId, true, false) + require.Nil(t, err) + assert.Len(t, postInfos, 2) - if result := <-ss.FileInfo().GetForPost(postId, false, false); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 { - t.Fatal("should've returned exactly 2 file infos") - } + postInfos, err = ss.FileInfo().GetForPost(postId, false, false) + require.Nil(t, err) + assert.Len(t, postInfos, 2) - if result := <-ss.FileInfo().GetForPost(postId, true, true); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.FileInfo); len(returned) != 2 { - t.Fatal("should've returned exactly 2 file infos") - } + postInfos, err = ss.FileInfo().GetForPost(postId, true, true) + require.Nil(t, err) + assert.Len(t, postInfos, 2) } func testFileInfoGetForUser(t *testing.T, ss store.Store) { @@ -187,23 +170,21 @@ func testFileInfoGetForUser(t *testing.T, ss store.Store) { } for i, info := range infos { - infos[i] = store.Must(ss.FileInfo().Save(info)).(*model.FileInfo) + newInfo, err := ss.FileInfo().Save(info) + require.Nil(t, err) + infos[i] = newInfo defer func(id string) { - <-ss.FileInfo().PermanentDelete(id) - }(infos[i].Id) + ss.FileInfo().PermanentDelete(id) + }(newInfo.Id) } - if result := <-ss.FileInfo().GetForUser(userId); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.FileInfo); len(returned) != 3 { - t.Fatal("should've returned exactly 3 file infos") - } + userPosts, err := ss.FileInfo().GetForUser(userId) + require.Nil(t, err) + assert.Len(t, userPosts, 3) - if result := <-ss.FileInfo().GetForUser(userId2); result.Err != nil { - t.Fatal(result.Err) - } else if returned := result.Data.([]*model.FileInfo); len(returned) != 1 { - t.Fatal("should've returned exactly 1 file infos") - } + userPosts, err = ss.FileInfo().GetForUser(userId2) + require.Nil(t, err) + assert.Len(t, userPosts, 1) } func testFileInfoAttachToPost(t *testing.T, ss store.Store) { @@ -211,28 +192,28 @@ func testFileInfoAttachToPost(t *testing.T, ss store.Store) { userId := model.NewId() postId := model.NewId() - info1 := store.Must(ss.FileInfo().Save(&model.FileInfo{ + info1, err := ss.FileInfo().Save(&model.FileInfo{ CreatorId: userId, Path: "file.txt", - })).(*model.FileInfo) - info2 := store.Must(ss.FileInfo().Save(&model.FileInfo{ + }) + require.Nil(t, err) + info2, err := ss.FileInfo().Save(&model.FileInfo{ CreatorId: userId, Path: "file2.txt", - })).(*model.FileInfo) + }) + require.Nil(t, err) require.Equal(t, "", info1.PostId) require.Equal(t, "", info2.PostId) - result := <-ss.FileInfo().AttachToPost(info1.Id, postId, userId) - assert.Nil(t, result.Err) + err = ss.FileInfo().AttachToPost(info1.Id, postId, userId) + assert.Nil(t, err) - result = <-ss.FileInfo().AttachToPost(info2.Id, postId, userId) - assert.Nil(t, result.Err) + err = ss.FileInfo().AttachToPost(info2.Id, postId, userId) + assert.Nil(t, err) - result = <-ss.FileInfo().GetForPost(postId, true, false) - assert.Nil(t, result.Err) - - data := result.Data.([]*model.FileInfo) + data, err := ss.FileInfo().GetForPost(postId, true, false) + assert.Nil(t, err) assert.Len(t, data, 2) assert.True(t, data[0].Id == info1.Id || data[0].Id == info2.Id) @@ -243,34 +224,35 @@ func testFileInfoAttachToPost(t *testing.T, ss store.Store) { userId := model.NewId() postId := model.NewId() - info := store.Must(ss.FileInfo().Save(&model.FileInfo{ + info, err := ss.FileInfo().Save(&model.FileInfo{ CreatorId: userId, Path: "file.txt", - })).(*model.FileInfo) + }) + require.Nil(t, err) require.Equal(t, "", info.PostId) - result := <-ss.FileInfo().AttachToPost(info.Id, model.NewId(), userId) - assert.Nil(t, result.Err) + err = ss.FileInfo().AttachToPost(info.Id, model.NewId(), userId) + assert.Nil(t, err) - result = <-ss.FileInfo().AttachToPost(info.Id, postId, userId) - assert.NotNil(t, result.Err) + err = ss.FileInfo().AttachToPost(info.Id, postId, userId) + assert.NotNil(t, err) }) t.Run("should not attach files owned from a different user", func(t *testing.T) { userId := model.NewId() postId := model.NewId() - info := store.Must(ss.FileInfo().Save(&model.FileInfo{ + info, err := ss.FileInfo().Save(&model.FileInfo{ CreatorId: model.NewId(), Path: "file.txt", - })).(*model.FileInfo) + }) + require.Nil(t, err) require.Equal(t, "", info.PostId) - result := <-ss.FileInfo().AttachToPost(info.Id, postId, userId) - - assert.NotNil(t, result.Err) + err = ss.FileInfo().AttachToPost(info.Id, postId, userId) + assert.NotNil(t, err) }) } @@ -303,83 +285,84 @@ func testFileInfoDeleteForPost(t *testing.T, ss store.Store) { } for i, info := range infos { - infos[i] = store.Must(ss.FileInfo().Save(info)).(*model.FileInfo) + newInfo, err := ss.FileInfo().Save(info) + require.Nil(t, err) + infos[i] = newInfo defer func(id string) { - <-ss.FileInfo().PermanentDelete(id) - }(infos[i].Id) + ss.FileInfo().PermanentDelete(id) + }(newInfo.Id) } - if result := <-ss.FileInfo().DeleteForPost(postId); result.Err != nil { - t.Fatal(result.Err) - } + _, err := ss.FileInfo().DeleteForPost(postId) + require.Nil(t, err) - if infos := store.Must(ss.FileInfo().GetForPost(postId, true, false)).([]*model.FileInfo); len(infos) != 0 { - t.Fatal("shouldn't have returned any file infos") - } + infos, err = ss.FileInfo().GetForPost(postId, true, false) + require.Nil(t, err) + assert.Len(t, infos, 0) } func testFileInfoPermanentDelete(t *testing.T, ss store.Store) { - info := store.Must(ss.FileInfo().Save(&model.FileInfo{ + info, err := ss.FileInfo().Save(&model.FileInfo{ PostId: model.NewId(), CreatorId: model.NewId(), Path: "file.txt", - })).(*model.FileInfo) + }) + require.Nil(t, err) - if result := <-ss.FileInfo().PermanentDelete(info.Id); result.Err != nil { - t.Fatal(result.Err) - } + err = ss.FileInfo().PermanentDelete(info.Id) + require.Nil(t, err) } func testFileInfoPermanentDeleteBatch(t *testing.T, ss store.Store) { postId := model.NewId() - store.Must(ss.FileInfo().Save(&model.FileInfo{ + _, err := ss.FileInfo().Save(&model.FileInfo{ PostId: postId, CreatorId: model.NewId(), Path: "file.txt", CreateAt: 1000, - })) + }) + require.Nil(t, err) - store.Must(ss.FileInfo().Save(&model.FileInfo{ + _, err = ss.FileInfo().Save(&model.FileInfo{ PostId: postId, CreatorId: model.NewId(), Path: "file.txt", CreateAt: 1200, - })) + }) + require.Nil(t, err) - store.Must(ss.FileInfo().Save(&model.FileInfo{ + _, err = ss.FileInfo().Save(&model.FileInfo{ PostId: postId, CreatorId: model.NewId(), Path: "file.txt", CreateAt: 2000, - })) + }) + require.Nil(t, err) - if result := <-ss.FileInfo().GetForPost(postId, true, false); result.Err != nil { - t.Fatal(result.Err) - } else if len(result.Data.([]*model.FileInfo)) != 3 { - t.Fatal("Expected 3 fileInfos") - } + postFiles, err := ss.FileInfo().GetForPost(postId, true, false) + require.Nil(t, err) + assert.Len(t, postFiles, 3) - store.Must(ss.FileInfo().PermanentDeleteBatch(1500, 1000)) + _, err = ss.FileInfo().PermanentDeleteBatch(1500, 1000) + require.Nil(t, err) - if result := <-ss.FileInfo().GetForPost(postId, true, false); result.Err != nil { - t.Fatal(result.Err) - } else if len(result.Data.([]*model.FileInfo)) != 1 { - t.Fatal("Expected 3 fileInfos") - } + postFiles, err = ss.FileInfo().GetForPost(postId, true, false) + require.Nil(t, err) + assert.Len(t, postFiles, 1) } func testFileInfoPermanentDeleteByUser(t *testing.T, ss store.Store) { userId := model.NewId() postId := model.NewId() - store.Must(ss.FileInfo().Save(&model.FileInfo{ + _, err := ss.FileInfo().Save(&model.FileInfo{ PostId: postId, CreatorId: userId, Path: "file.txt", - })) + }) + require.Nil(t, err) - if result := <-ss.FileInfo().PermanentDeleteByUser(userId); result.Err != nil { - t.Fatal(result.Err) - } + _, err = ss.FileInfo().PermanentDeleteByUser(userId) + require.Nil(t, err) } diff --git a/store/storetest/mocks/FileInfoStore.go b/store/storetest/mocks/FileInfoStore.go index 22de7f4d0a..4c66f24a8a 100644 --- a/store/storetest/mocks/FileInfoStore.go +++ b/store/storetest/mocks/FileInfoStore.go @@ -6,7 +6,6 @@ package mocks import mock "github.com/stretchr/testify/mock" import model "github.com/mattermost/mattermost-server/model" -import store "github.com/mattermost/mattermost-server/store" // FileInfoStore is an autogenerated mock type for the FileInfoStore type type FileInfoStore struct { @@ -14,15 +13,15 @@ type FileInfoStore struct { } // AttachToPost provides a mock function with given fields: fileId, postId, creatorId -func (_m *FileInfoStore) AttachToPost(fileId string, postId string, creatorId string) store.StoreChannel { +func (_m *FileInfoStore) AttachToPost(fileId string, postId string, creatorId string) *model.AppError { ret := _m.Called(fileId, postId, creatorId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string, string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string, string, string) *model.AppError); ok { r0 = rf(fileId, postId, creatorId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } @@ -35,83 +34,126 @@ func (_m *FileInfoStore) ClearCaches() { } // DeleteForPost provides a mock function with given fields: postId -func (_m *FileInfoStore) DeleteForPost(postId string) store.StoreChannel { +func (_m *FileInfoStore) DeleteForPost(postId string) (string, *model.AppError) { ret := _m.Called(postId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 string + if rf, ok := ret.Get(0).(func(string) string); ok { r0 = rf(postId) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(string) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(postId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // Get provides a mock function with given fields: id -func (_m *FileInfoStore) Get(id string) store.StoreChannel { +func (_m *FileInfoStore) Get(id string) (*model.FileInfo, *model.AppError) { ret := _m.Called(id) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.FileInfo + if rf, ok := ret.Get(0).(func(string) *model.FileInfo); ok { r0 = rf(id) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.FileInfo) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(id) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetByPath provides a mock function with given fields: path -func (_m *FileInfoStore) GetByPath(path string) store.StoreChannel { +func (_m *FileInfoStore) GetByPath(path string) (*model.FileInfo, *model.AppError) { ret := _m.Called(path) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.FileInfo + if rf, ok := ret.Get(0).(func(string) *model.FileInfo); ok { r0 = rf(path) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.FileInfo) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(path) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetForPost provides a mock function with given fields: postId, readFromMaster, allowFromCache -func (_m *FileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) store.StoreChannel { +func (_m *FileInfoStore) GetForPost(postId string, readFromMaster bool, allowFromCache bool) ([]*model.FileInfo, *model.AppError) { ret := _m.Called(postId, readFromMaster, allowFromCache) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool, bool) store.StoreChannel); ok { + var r0 []*model.FileInfo + if rf, ok := ret.Get(0).(func(string, bool, bool) []*model.FileInfo); ok { r0 = rf(postId, readFromMaster, allowFromCache) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.FileInfo) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, bool, bool) *model.AppError); ok { + r1 = rf(postId, readFromMaster, allowFromCache) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetForUser provides a mock function with given fields: userId -func (_m *FileInfoStore) GetForUser(userId string) store.StoreChannel { +func (_m *FileInfoStore) GetForUser(userId string) ([]*model.FileInfo, *model.AppError) { ret := _m.Called(userId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 []*model.FileInfo + if rf, ok := ret.Get(0).(func(string) []*model.FileInfo); ok { r0 = rf(userId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).([]*model.FileInfo) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // InvalidateFileInfosForPostCache provides a mock function with given fields: postId @@ -120,15 +162,15 @@ func (_m *FileInfoStore) InvalidateFileInfosForPostCache(postId string) { } // PermanentDelete provides a mock function with given fields: fileId -func (_m *FileInfoStore) PermanentDelete(fileId string) store.StoreChannel { +func (_m *FileInfoStore) PermanentDelete(fileId string) *model.AppError { ret := _m.Called(fileId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { r0 = rf(fileId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } @@ -136,49 +178,72 @@ func (_m *FileInfoStore) PermanentDelete(fileId string) store.StoreChannel { } // PermanentDeleteBatch provides a mock function with given fields: endTime, limit -func (_m *FileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) store.StoreChannel { +func (_m *FileInfoStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) { ret := _m.Called(endTime, limit) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(int64, int64) store.StoreChannel); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(int64, int64) int64); ok { r0 = rf(endTime, limit) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(int64, int64) *model.AppError); ok { + r1 = rf(endTime, limit) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // PermanentDeleteByUser provides a mock function with given fields: userId -func (_m *FileInfoStore) PermanentDeleteByUser(userId string) store.StoreChannel { +func (_m *FileInfoStore) PermanentDeleteByUser(userId string) (int64, *model.AppError) { ret := _m.Called(userId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 int64 + if rf, ok := ret.Get(0).(func(string) int64); ok { r0 = rf(userId) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) } } - return r0 + return r0, r1 } // Save provides a mock function with given fields: info -func (_m *FileInfoStore) Save(info *model.FileInfo) store.StoreChannel { +func (_m *FileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, *model.AppError) { ret := _m.Called(info) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.FileInfo) store.StoreChannel); ok { + var r0 *model.FileInfo + if rf, ok := ret.Get(0).(func(*model.FileInfo) *model.FileInfo); ok { r0 = rf(info) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.FileInfo) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.FileInfo) *model.AppError); ok { + r1 = rf(info) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 }