From 24621a22eda52dfef7c12c1a1630efce607d9364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Mon, 2 Nov 2020 15:43:48 +0100 Subject: [PATCH] Add the content field to FileInfo (#15749) * Add the content field to FileInfo * Fixing the upgrade code * Trying to fix the text-scheme * Fixing test-schema * Fixing test-schema * Moving the migration to the next version --- model/file_info.go | 6 +++-- store/opentracinglayer/opentracinglayer.go | 18 ++++++++++++++ store/retrylayer/retrylayer.go | 20 ++++++++++++++++ store/sqlstore/file_info_store.go | 28 ++++++++++++++++++++++ store/sqlstore/upgrade.go | 10 ++++++++ store/store.go | 1 + store/storetest/mocks/FileInfoStore.go | 14 +++++++++++ store/timerlayer/timerlayer.go | 16 +++++++++++++ 8 files changed, 111 insertions(+), 2 deletions(-) diff --git a/model/file_info.go b/model/file_info.go index 8879d7a40a..c622b8f2bd 100644 --- a/model/file_info.go +++ b/model/file_info.go @@ -6,8 +6,6 @@ package model import ( "bytes" "encoding/json" - "github.com/disintegration/imaging" - "github.com/mattermost/mattermost-server/v5/mlog" "image" "image/gif" "image/jpeg" @@ -16,6 +14,9 @@ import ( "net/http" "path/filepath" "strings" + + "github.com/disintegration/imaging" + "github.com/mattermost/mattermost-server/v5/mlog" ) const ( @@ -57,6 +58,7 @@ type FileInfo struct { 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 + Content string `json:"-"` } func (fi *FileInfo) ToJson() string { diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 6bf69bbfcf..9b3b6d9d6d 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) SetContent(fileId string, content string) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.SetContent") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.FileInfoStore.SetContent(fileId, content) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return 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") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index b20b524c92..4b98e79015 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -3336,6 +3336,26 @@ func (s *RetryLayerFileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, e } +func (s *RetryLayerFileInfoStore) SetContent(fileId string, content string) error { + + tries := 0 + for { + err := s.FileInfoStore.SetContent(fileId, content) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + } + +} + func (s *RetryLayerFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { tries := 0 diff --git a/store/sqlstore/file_info_store.go b/store/sqlstore/file_info_store.go index 7855c55056..884a17f7b7 100644 --- a/store/sqlstore/file_info_store.go +++ b/store/sqlstore/file_info_store.go @@ -38,6 +38,7 @@ func newSqlFileInfoStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface table.ColMap("ThumbnailPath").SetMaxSize(512) table.ColMap("PreviewPath").SetMaxSize(512) table.ColMap("Name").SetMaxSize(256) + table.ColMap("Content").SetMaxSize(0) table.ColMap("Extension").SetMaxSize(64) table.ColMap("MimeType").SetMaxSize(256) } @@ -272,6 +273,33 @@ func (fs SqlFileInfoStore) AttachToPost(fileId, postId, creatorId string) error return nil } +func (fs SqlFileInfoStore) SetContent(fileId, content string) error { + query := fs.getQueryBuilder(). + Update("FileInfo"). + Set("Content", content). + Where(sq.Eq{"Id": fileId}) + + queryString, args, err := query.ToSql() + if err != nil { + return errors.Wrap(err, "file_info_tosql") + } + + sqlResult, err := fs.GetMaster().Exec(queryString, args...) + if err != nil { + return errors.Wrapf(err, "failed to update FileInfo content with id=%s", fileId) + } + + count, err := sqlResult.RowsAffected() + if err != nil { + // RowsAffected should never fail with the MySQL or Postgres drivers + return errors.Wrap(err, "unable to retrieve rows affected") + } else if count == 0 { + // Could not attach the file to the post + return store.NewErrInvalidInput("FileInfo", "", fmt.Sprintf("<%s>", fileId)) + } + return nil +} + func (fs SqlFileInfoStore) DeleteForPost(postId string) (string, error) { if _, err := fs.GetMaster().Exec( `UPDATE diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index 40b4ac5c69..6c6a273e35 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -192,6 +192,7 @@ func upgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error upgradeDatabaseToVersion528(sqlStore) upgradeDatabaseToVersion5281(sqlStore) upgradeDatabaseToVersion529(sqlStore) + upgradeDatabaseToVersion530(sqlStore) return nil } @@ -865,6 +866,15 @@ func upgradeDatabaseToVersion5281(sqlStore SqlStore) { } } +func upgradeDatabaseToVersion530(sqlStore SqlStore) { + // if shouldPerformUpgrade(sqlStore, VERSION_5_29_0, VERSION_5_30_0) { + + sqlStore.CreateColumnIfNotExistsNoDefault("FileInfo", "Content", "longtext", "text") + + // saveSchemaVersion(sqlStore, VERSION_5_30_0) + // } +} + func precheckMigrationToVersion528(sqlStore SqlStore) error { teamsQuery, _, err := sqlStore.getQueryBuilder().Select(`COALESCE(SUM(CASE WHEN CHAR_LENGTH(SchemeId) > 26 THEN 1 diff --git a/store/store.go b/store/store.go index b9e2b4834b..d7c2a293eb 100644 --- a/store/store.go +++ b/store/store.go @@ -569,6 +569,7 @@ type FileInfoStore interface { PermanentDelete(fileId string) error PermanentDeleteBatch(endTime int64, limit int64) (int64, error) PermanentDeleteByUser(userId string) (int64, error) + SetContent(fileId, content string) error ClearCaches() } diff --git a/store/storetest/mocks/FileInfoStore.go b/store/storetest/mocks/FileInfoStore.go index f3ab546aba..9cbf13aeac 100644 --- a/store/storetest/mocks/FileInfoStore.go +++ b/store/storetest/mocks/FileInfoStore.go @@ -253,6 +253,20 @@ func (_m *FileInfoStore) Save(info *model.FileInfo) (*model.FileInfo, error) { return r0, r1 } +// SetContent provides a mock function with given fields: fileId, content +func (_m *FileInfoStore) SetContent(fileId string, content string) error { + ret := _m.Called(fileId, content) + + var r0 error + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(fileId, content) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Upsert provides a mock function with given fields: info func (_m *FileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { ret := _m.Called(info) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index 2e65b7183e..89d8de4a87 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) SetContent(fileId string, content string) error { + start := timemodule.Now() + + err := s.FileInfoStore.SetContent(fileId, content) + + 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.SetContent", success, elapsed) + } + return err +} + func (s *TimerLayerFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) { start := timemodule.Now()