MM-27353 Mini image previews (#15376)

This commit is contained in:
Eli Yukelzon
2020-10-02 11:14:57 +03:00
committed by GitHub
parent 2f47cf5994
commit b8ceb610e6
12 changed files with 256 additions and 72 deletions

View File

@@ -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)))

View File

@@ -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
}

View File

@@ -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
}

View File

@@ -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,

View File

@@ -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")

View File

@@ -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)

View File

@@ -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{}

View File

@@ -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)
}

View File

@@ -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)

View File

@@ -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)
})
}

View File

@@ -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
}

View File

@@ -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()