diff --git a/api4/file_test.go b/api4/file_test.go index 3cd3701471..9cb271aaa0 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -215,51 +215,57 @@ func TestUploadFiles(t *testing.T) { expectedImageThumbnailNames []string expectedImagePreviewNames []string expectedImageHasPreview []bool + expectedImageMiniPreview []bool setupConfig func(a *app.App) func(a *app.App) checkResponse func(t *testing.T, resp *model.Response) }{ // Upload a bunch of files, mixed images and non-images { - title: "Happy", - names: []string{"test.png", "testgif.gif", "testplugin.tar.gz", "test-search.md", "test.tiff"}, - expectedCreatorId: th.BasicUser.Id, + title: "Happy", + names: []string{"test.png", "testgif.gif", "testplugin.tar.gz", "test-search.md", "test.tiff"}, + expectedCreatorId: th.BasicUser.Id, + expectedImageMiniPreview: []bool{true, true, false, false, true}, }, // Upload a bunch of files, with clientIds { - title: "Happy client_ids", - names: []string{"test.png", "testgif.gif", "testplugin.tar.gz", "test-search.md", "test.tiff"}, - clientIds: []string{"1", "2", "3", "4", "5"}, - expectedCreatorId: th.BasicUser.Id, + title: "Happy client_ids", + names: []string{"test.png", "testgif.gif", "testplugin.tar.gz", "test-search.md", "test.tiff"}, + clientIds: []string{"1", "2", "3", "4", "5"}, + expectedImageMiniPreview: []bool{true, true, false, false, true}, + expectedCreatorId: th.BasicUser.Id, }, // Upload a bunch of images. testgif.gif is an animated GIF, // so it does not have HasPreviewImage set. { - title: "Happy images", - names: []string{"test.png", "testgif.gif"}, - expectImage: true, - expectedCreatorId: th.BasicUser.Id, - expectedImageWidths: []int{408, 118}, - expectedImageHeights: []int{336, 118}, - expectedImageHasPreview: []bool{true, false}, + title: "Happy images", + names: []string{"test.png", "testgif.gif"}, + expectImage: true, + expectedCreatorId: th.BasicUser.Id, + expectedImageWidths: []int{408, 118}, + expectedImageHeights: []int{336, 118}, + expectedImageHasPreview: []bool{true, false}, + expectedImageMiniPreview: []bool{true, true}, }, { - title: "Happy invalid image", - names: []string{"testgif.gif"}, - blobs: [][]byte{fileBytes(t, "test-search.md")}, - skipPayloadValidation: true, - expectedCreatorId: th.BasicUser.Id, + title: "Happy invalid image", + names: []string{"testgif.gif"}, + blobs: [][]byte{fileBytes(t, "test-search.md")}, + skipPayloadValidation: true, + expectedCreatorId: th.BasicUser.Id, + expectedImageMiniPreview: []bool{false}, }, // Simple POST, chunked encoding { - title: "Happy image chunked post", - skipMultipart: true, - useChunkedInSimplePost: true, - names: []string{"test.png"}, - expectImage: true, - expectedImageWidths: []int{408}, - expectedImageHeights: []int{336}, - expectedImageHasPreview: []bool{true}, - expectedCreatorId: th.BasicUser.Id, + title: "Happy image chunked post", + skipMultipart: true, + useChunkedInSimplePost: true, + names: []string{"test.png"}, + expectImage: true, + expectedImageWidths: []int{408}, + expectedImageHeights: []int{336}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + expectedImageMiniPreview: []bool{true}, }, // Image thumbnail and preview: size and orientation. Note that // the expected image dimensions remain the same regardless of the @@ -275,6 +281,7 @@ func TestUploadFiles(t *testing.T) { expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, + expectedImageMiniPreview: []bool{true}, }, { title: "Happy image thumbnail/preview 2", @@ -285,6 +292,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { @@ -296,6 +304,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { @@ -307,6 +316,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { @@ -318,6 +328,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { @@ -329,6 +340,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { @@ -340,6 +352,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { @@ -351,6 +364,7 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{2860}, expectedImageHeights: []int{1578}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, // TIFF preview test @@ -363,13 +377,15 @@ func TestUploadFiles(t *testing.T) { expectedImageWidths: []int{701}, expectedImageHeights: []int{701}, expectedImageHasPreview: []bool{true}, + expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, { - title: "Happy admin", - client: th.SystemAdminClient, - names: []string{"test.png"}, - expectedCreatorId: th.SystemAdminUser.Id, + title: "Happy admin", + client: th.SystemAdminClient, + names: []string{"test.png"}, + expectedImageMiniPreview: []bool{true}, + expectedCreatorId: th.SystemAdminUser.Id, }, { title: "Happy stream", @@ -384,7 +400,8 @@ func TestUploadFiles(t *testing.T) { a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = maxFileSize }) } }, - expectedCreatorId: th.BasicUser.Id, + expectedImageMiniPreview: []bool{false}, + expectedCreatorId: th.BasicUser.Id, }, // Error cases { @@ -397,19 +414,21 @@ func TestUploadFiles(t *testing.T) { { // on simple post this uploads the last file // successfully, without a ClientId - title: "Error too few client_ids", - skipSimplePost: true, - names: []string{"test.png", "testplugin.tar.gz", "test-search.md"}, - clientIds: []string{"1", "4"}, - skipSuccessValidation: true, - checkResponse: CheckBadRequestStatus, + title: "Error too few client_ids", + skipSimplePost: true, + names: []string{"test.png", "testplugin.tar.gz", "test-search.md"}, + clientIds: []string{"1", "4"}, + expectedImageMiniPreview: []bool{true, false, false}, + skipSuccessValidation: true, + checkResponse: CheckBadRequestStatus, }, { - title: "Error invalid channel_id", - channelId: "../../junk", - names: []string{"test.png"}, - skipSuccessValidation: true, - checkResponse: CheckBadRequestStatus, + title: "Error invalid channel_id", + channelId: "../../junk", + names: []string{"test.png"}, + expectedImageMiniPreview: []bool{true}, + skipSuccessValidation: true, + checkResponse: CheckBadRequestStatus, }, { title: "Error admin channel_id does not exist", @@ -456,12 +475,13 @@ func TestUploadFiles(t *testing.T) { }, // File too large (chunked, simple POST only, multipart would've been redundant with above) { - title: "File too large chunked", - useChunkedInSimplePost: true, - skipMultipart: true, - names: []string{"test.png"}, - skipSuccessValidation: true, - checkResponse: CheckRequestEntityTooLargeStatus, + title: "File too large chunked", + useChunkedInSimplePost: true, + skipMultipart: true, + names: []string{"test.png"}, + skipSuccessValidation: true, + checkResponse: CheckRequestEntityTooLargeStatus, + expectedImageMiniPreview: []bool{false}, setupConfig: func(a *app.App) func(a *app.App) { maxFileSize := *a.Config().FileSettings.MaxFileSize a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = 279590 }) @@ -560,6 +580,9 @@ func TestUploadFiles(t *testing.T) { assert.Equal(t, ri.Path, "", "File path should not be set on returned info") assert.Equal(t, ri.ThumbnailPath, "", "File thumbnail path should not be set on returned info") assert.Equal(t, ri.PreviewPath, "", "File preview path should not be set on returned info") + if len(tc.expectedImageMiniPreview) == len(fileResp.FileInfos) { + assert.Equal(t, ri.MiniPreview != nil, tc.expectedImageMiniPreview[i], "File: %s mini preview state unexpected", tc.names[i]) + } if len(tc.clientIds) > i { assert.True(t, len(fileResp.ClientIds) == len(tc.clientIds), fmt.Sprintf("Wrong number of clientIds returned, expected %v, got %v", len(tc.clientIds), len(fileResp.ClientIds))) diff --git a/app/file.go b/app/file.go index accd767516..98c45df7c0 100644 --- a/app/file.go +++ b/app/file.go @@ -633,7 +633,7 @@ func (a *App) UploadFileX(channelId, name string, input io.Reader, return t.fileinfo, aerr } - // Concurrently upload and update DB, and post-process the image. + // Concurrently post-process the image. wg := sync.WaitGroup{} if !t.Raw && t.fileinfo.IsImage() { @@ -649,6 +649,7 @@ func (a *App) UploadFileX(channelId, name string, input io.Reader, return nil, aerr } + wg.Wait() if _, err := t.saveToDatabase(t.fileinfo); err != nil { var appErr *model.AppError switch { @@ -659,8 +660,6 @@ func (a *App) UploadFileX(channelId, name string, input io.Reader, } } - wg.Wait() - return t.fileinfo, nil } @@ -843,7 +842,7 @@ func (t *UploadFileTask) postprocessImage() { h := decoded.Bounds().Dy() var wg sync.WaitGroup - wg.Add(2) + wg.Add(3) go func() { defer wg.Done() thumb := decoded @@ -865,6 +864,13 @@ func (t *UploadFileTask) postprocessImage() { } writeJPEG(preview, t.fileinfo.PreviewPath) }() + + go func() { + defer wg.Done() + if t.fileinfo.MiniPreview == nil { + t.fileinfo.MiniPreview = model.GenerateMiniPreviewImage(decoded) + } + }() wg.Wait() } @@ -1125,6 +1131,41 @@ func (a *App) generatePreviewImage(img image.Image, previewPath string, width in } } +// generateMiniPreview updates mini preview if needed +// will save fileinfo with the preview added +func (a *App) generateMiniPreview(fi *model.FileInfo) { + if fi.IsImage() && fi.MiniPreview == nil { + data, err := a.ReadFile(fi.Path) + if err != nil { + mlog.Error("error reading image file", mlog.Err(err)) + return + } + img, _, _ := prepareImage(data) + if img == nil { + return + } + fi.MiniPreview = model.GenerateMiniPreviewImage(img) + if _, appErr := a.Srv().Store.FileInfo().Upsert(fi); appErr != nil { + mlog.Error("creating mini preview failed", mlog.Err(appErr)) + } else { + a.Srv().Store.FileInfo().InvalidateFileInfosForPostCache(fi.PostId, false) + } + } +} + +func (a *App) generateMiniPreviewForInfos(fileInfos []*model.FileInfo) { + wg := new(sync.WaitGroup) + + wg.Add(len(fileInfos)) + for _, fileInfo := range fileInfos { + go func(fi *model.FileInfo) { + defer wg.Done() + a.generateMiniPreview(fi) + }(fileInfo) + } + wg.Wait() +} + func (a *App) GetFileInfo(fileId string) (*model.FileInfo, *model.AppError) { fileInfo, err := a.Srv().Store.FileInfo().Get(fileId) if err != nil { @@ -1137,6 +1178,7 @@ func (a *App) GetFileInfo(fileId string) (*model.FileInfo, *model.AppError) { } } + a.generateMiniPreview(fileInfo) return fileInfo, nil } @@ -1155,6 +1197,8 @@ func (a *App) GetFileInfos(page, perPage int, opt *model.GetFileInfosOptions) ([ } } + a.generateMiniPreviewForInfos(fileInfos) + return fileInfos, nil } diff --git a/app/post.go b/app/post.go index 641eff2ec7..a15b215264 100644 --- a/app/post.go +++ b/app/post.go @@ -1252,6 +1252,8 @@ func (a *App) GetFileInfosForPost(postId string, fromMaster bool) ([]*model.File return nil, model.NewAppError("GetFileInfosForPost", "app.file_info.get_for_post.app_error", nil, err.Error(), http.StatusInternalServerError) } + a.generateMiniPreviewForInfos(fileInfos) + return fileInfos, nil } diff --git a/model/file_info.go b/model/file_info.go index 4b71f5a8c0..8879d7a40a 100644 --- a/model/file_info.go +++ b/model/file_info.go @@ -4,9 +4,13 @@ package model import ( + "bytes" "encoding/json" + "github.com/disintegration/imaging" + "github.com/mattermost/mattermost-server/v5/mlog" "image" "image/gif" + "image/jpeg" "io" "mime" "net/http" @@ -36,22 +40,23 @@ type GetFileInfosOptions struct { } type FileInfo struct { - Id string `json:"id"` - CreatorId string `json:"user_id"` - PostId string `json:"post_id,omitempty"` - CreateAt int64 `json:"create_at"` - UpdateAt int64 `json:"update_at"` - DeleteAt int64 `json:"delete_at"` - Path string `json:"-"` // not sent back to the client - ThumbnailPath string `json:"-"` // not sent back to the client - PreviewPath string `json:"-"` // not sent back to the client - Name string `json:"name"` - Extension string `json:"extension"` - Size int64 `json:"size"` - MimeType string `json:"mime_type"` - Width int `json:"width,omitempty"` - Height int `json:"height,omitempty"` - HasPreviewImage bool `json:"has_preview_image,omitempty"` + Id string `json:"id"` + CreatorId string `json:"user_id"` + PostId string `json:"post_id,omitempty"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + DeleteAt int64 `json:"delete_at"` + Path string `json:"-"` // not sent back to the client + ThumbnailPath string `json:"-"` // not sent back to the client + PreviewPath string `json:"-"` // not sent back to the client + Name string `json:"name"` + Extension string `json:"extension"` + Size int64 `json:"size"` + MimeType string `json:"mime_type"` + Width int `json:"width,omitempty"` + Height int `json:"height,omitempty"` + HasPreviewImage bool `json:"has_preview_image,omitempty"` + MiniPreview *[]byte `json:"mini_preview"` // declared as *[]byte to avoid postgres/mysql differences in deserialization } func (fi *FileInfo) ToJson() string { @@ -150,6 +155,19 @@ func NewInfo(name string) *FileInfo { return info } +func GenerateMiniPreviewImage(img image.Image) *[]byte { + preview := imaging.Resize(img, 16, 16, imaging.Lanczos) + + buf := new(bytes.Buffer) + + if err := jpeg.Encode(buf, preview, &jpeg.Options{Quality: 90}); err != nil { + mlog.Error("Unable to encode image as mini preview jpg", mlog.Err(err)) + return nil + } + data := buf.Bytes() + return &data +} + func GetInfoForBytes(name string, data io.ReadSeeker, size int) (*FileInfo, *AppError) { info := &FileInfo{ Name: name, diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index cf12ab9499..55eaa86f6e 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -3121,6 +3121,24 @@ func (s *OpenTracingLayerFileInfoStore) Save(info *model.FileInfo) (*model.FileI return result, err } +func (s *OpenTracingLayerFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.Upsert") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.FileInfoStore.Upsert(info) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerGroupStore) AdminRoleGroupsForSyncableMember(userID string, syncableID string, syncableType model.GroupSyncableType) ([]string, *model.AppError) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.AdminRoleGroupsForSyncableMember") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index c896d5763e..ade64a8bd6 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -2916,6 +2916,26 @@ func (s *RetryLayerFileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, e } +func (s *RetryLayerFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { + + tries := 0 + for { + result, err := s.FileInfoStore.Upsert(info) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } + +} + func (s *RetryLayerGroupStore) AdminRoleGroupsForSyncableMember(userID string, syncableID string, syncableType model.GroupSyncableType) ([]string, *model.AppError) { return s.GroupStore.AdminRoleGroupsForSyncableMember(userID, syncableID, syncableType) diff --git a/store/sqlstore/file_info_store.go b/store/sqlstore/file_info_store.go index d3c629c64f..7855c55056 100644 --- a/store/sqlstore/file_info_store.go +++ b/store/sqlstore/file_info_store.go @@ -64,6 +64,24 @@ func (fs SqlFileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, error) { return info, nil } +func (fs SqlFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { + info.PreSave() + if err := info.IsValid(); err != nil { + return nil, err + } + + n, err := fs.GetMaster().Update(info) + if err != nil { + return nil, errors.Wrap(err, "failed to update FileInfo") + } + if n == 0 { + if err = fs.GetMaster().Insert(info); err != nil { + return nil, errors.Wrap(err, "failed to save FileInfo") + } + } + return info, nil +} + func (fs SqlFileInfoStore) Get(id string) (*model.FileInfo, error) { info := &model.FileInfo{} diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index f101516883..c8e92714e9 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -848,6 +848,7 @@ func upgradeDatabaseToVersion528(sqlStore SqlStore) { sqlStore.AlterColumnTypeIfExists("Teams", "SchemeId", "VARCHAR(26)", "VARCHAR(26)") sqlStore.AlterColumnTypeIfExists("IncomingWebhooks", "Username", "varchar(255)", "varchar(255)") sqlStore.AlterColumnTypeIfExists("IncomingWebhooks", "IconURL", "text", "varchar(1024)") + sqlStore.CreateColumnIfNotExistsNoDefault("FileInfo", "MiniPreview", "MEDIUMBLOB", "bytea") saveSchemaVersion(sqlStore, VERSION_5_28_0) } diff --git a/store/store.go b/store/store.go index 81e050da90..eeed3e8888 100644 --- a/store/store.go +++ b/store/store.go @@ -548,6 +548,7 @@ type StatusStore interface { type FileInfoStore interface { Save(info *model.FileInfo) (*model.FileInfo, error) + Upsert(info *model.FileInfo) (*model.FileInfo, error) Get(id string) (*model.FileInfo, error) GetByPath(path string) (*model.FileInfo, error) GetForPost(postId string, readFromMaster, includeDeleted, allowFromCache bool) ([]*model.FileInfo, error) diff --git a/store/storetest/file_info_store.go b/store/storetest/file_info_store.go index 005ad26306..53b7accad5 100644 --- a/store/storetest/file_info_store.go +++ b/store/storetest/file_info_store.go @@ -434,7 +434,7 @@ func testFileInfoAttachToPost(t *testing.T, ss store.Store) { expected := []*model.FileInfo{info1, info2} sort.Sort(byFileInfoId(expected)) sort.Sort(byFileInfoId(data)) - assert.Equal(t, expected, data) + assert.EqualValues(t, expected, data) }) t.Run("should not attach files to multiple posts", func(t *testing.T) { @@ -488,7 +488,7 @@ func testFileInfoAttachToPost(t *testing.T, ss store.Store) { data, err := ss.FileInfo().GetForPost(postId, true, false, false) require.Nil(t, err) info.PostId = postId - assert.Equal(t, []*model.FileInfo{info}, data) + assert.EqualValues(t, []*model.FileInfo{info}, data) }) } diff --git a/store/storetest/mocks/FileInfoStore.go b/store/storetest/mocks/FileInfoStore.go index b9db3c33b7..f3ab546aba 100644 --- a/store/storetest/mocks/FileInfoStore.go +++ b/store/storetest/mocks/FileInfoStore.go @@ -252,3 +252,26 @@ func (_m *FileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, error) { return r0, r1 } + +// Upsert provides a mock function with given fields: info +func (_m *FileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { + ret := _m.Called(info) + + 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).(*model.FileInfo) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.FileInfo) error); ok { + r1 = rf(info) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index da0cde93ec..d6fac5739b 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -2861,6 +2861,22 @@ func (s *TimerLayerFileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, e return result, err } +func (s *TimerLayerFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { + start := timemodule.Now() + + result, err := s.FileInfoStore.Upsert(info) + + elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("FileInfoStore.Upsert", success, elapsed) + } + return result, err +} + func (s *TimerLayerGroupStore) AdminRoleGroupsForSyncableMember(userID string, syncableID string, syncableType model.GroupSyncableType) ([]string, *model.AppError) { start := timemodule.Now()