IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

MM-61375: Update file handling for bookmarks (#30034)

Browse Source
This commit is contained in:
Caleb Roseland 2025-02-13 17:11:13 -06:00 committed by GitHub
parent 2182b1eaf9
commit 68c11e9ecb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 200 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
server/channels/app/channel_bookmark.go
View File

@ -55,7 +55,7 @@ func (a *App) UpdateChannelBookmark(c request.CTX, updateBookmark *model.Channel
isAnotherFile := updateBookmark.FileInfo != nil && updateBookmark.FileId != "" && updateBookmark.FileId != updateBookmark.FileInfo.Id
if isAnotherFile {
if fileAlreadyAttachedErr := a.Srv().Store().ChannelBookmark().ErrorIfBookmarkFileInfoAlreadyAttached(updateBookmark.FileId); fileAlreadyAttachedErr != nil {
if fileAlreadyAttachedErr := a.Srv().Store().ChannelBookmark().ErrorIfBookmarkFileInfoAlreadyAttached(updateBookmark.FileId, updateBookmark.ChannelId); fileAlreadyAttachedErr != nil {
return nil, model.NewAppError("UpdateChannelBookmark", "app.channel.bookmark.update.app_error", nil, "", http.StatusInternalServerError).Wrap(fileAlreadyAttachedErr)
}
}

108
server/channels/app/channel_bookmark_test.go
View File

@ -89,6 +89,7 @@ func TestUpdateBookmark(t *testing.T) {
var testUpdateAnotherFile = func(th *TestHelper, t *testing.T) {
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: th.BasicChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
@ -116,6 +117,7 @@ func TestUpdateBookmark(t *testing.T) {
file2 := &model.FileInfo{
Id: model.NewId(),
ChannelId: th.BasicChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
@ -144,6 +146,106 @@ func TestUpdateBookmark(t *testing.T) {
require.Nil(t, bookmarkResp)
}
var testUpdateInvalidFiles = func(th *TestHelper, t *testing.T, creatingUserId string, updatingUserId string) {
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: th.BasicChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
PreviewPath: "prevPath",
Name: "test file",
Extension: "png",
MimeType: "images/png",
Size: 873182,
Width: 3076,
Height: 2200,
HasPreviewImage: true,
}
_, err := th.App.Srv().Store().FileInfo().Save(th.Context, file)
assert.NoError(t, err)
defer func() {
err = th.App.Srv().Store().FileInfo().PermanentDelete(th.Context, file.Id)
assert.NoError(t, err)
}()
th.Context.Session().UserId = creatingUserId
bookmark := createBookmark("File to be updated", model.ChannelBookmarkFile, th.BasicChannel.Id, file.Id)
bookmarkToEdit, appErr := th.App.CreateChannelBookmark(th.Context, bookmark, "")
require.Nil(t, appErr)
require.NotNil(t, bookmarkToEdit)
otherChannel := th.CreateChannel(th.Context, th.BasicTeam)
createAt := time.Now().Add(-1 * time.Minute)
deleteAt := createAt.Add(1 * time.Second)
deletedFile := &model.FileInfo{
Id: model.NewId(),
ChannelId: th.BasicChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
PreviewPath: "prevPath",
Name: "test file",
Extension: "png",
MimeType: "images/png",
Size: 873182,
Width: 3076,
Height: 2200,
HasPreviewImage: true,
CreateAt: createAt.UnixMilli(),
UpdateAt: createAt.UnixMilli(),
DeleteAt: deleteAt.UnixMilli(),
}
_, err = th.App.Srv().Store().FileInfo().Save(th.Context, deletedFile)
assert.NoError(t, err)
defer func() {
err = th.App.Srv().Store().FileInfo().PermanentDelete(th.Context, deletedFile.Id)
assert.NoError(t, err)
}()
th.Context.Session().UserId = updatingUserId
updateBookmarkPending := bookmarkToEdit.Clone()
updateBookmarkPending.FileId = deletedFile.Id
bookmarkEdited, appErr := th.App.UpdateChannelBookmark(th.Context, updateBookmarkPending, "")
assert.NotNil(t, appErr)
require.Nil(t, bookmarkEdited)
anotherChannelFile := &model.FileInfo{
Id: model.NewId(),
ChannelId: otherChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
PreviewPath: "prevPath",
Name: "test file",
Extension: "png",
MimeType: "images/png",
Size: 873182,
Width: 3076,
Height: 2200,
HasPreviewImage: true,
}
_, err = th.App.Srv().Store().FileInfo().Save(th.Context, anotherChannelFile)
assert.NoError(t, err)
defer func() {
err = th.App.Srv().Store().FileInfo().PermanentDelete(th.Context, anotherChannelFile.Id)
require.NoError(t, err)
}()
updateBookmarkPending = bookmarkToEdit.Clone()
updateBookmarkPending.FileId = anotherChannelFile.Id
bookmarkEdited, appErr = th.App.UpdateChannelBookmark(th.Context, updateBookmarkPending, "")
assert.NotNil(t, appErr)
require.Nil(t, bookmarkEdited)
}
t.Run("same user update a channel bookmark", func(t *testing.T) {
bookmark1 := &model.ChannelBookmark{
ChannelId: th.BasicChannel.Id,
@ -166,6 +268,8 @@ func TestUpdateBookmark(t *testing.T) {
assert.Greater(t, response.Updated.UpdateAt, response.Updated.CreateAt)
testUpdateAnotherFile(th, t)
testUpdateInvalidFiles(th, t, th.BasicUser.Id, th.BasicUser.Id)
})
t.Run("another user update a channel bookmark", func(t *testing.T) {
@ -181,6 +285,8 @@ func TestUpdateBookmark(t *testing.T) {
assert.Equal(t, "New name", response.Deleted.DisplayName)
testUpdateAnotherFile(th, t)
testUpdateInvalidFiles(th, t, th.BasicUser.Id, th.BasicUser.Id)
})
t.Run("update an already deleted channel bookmark", func(t *testing.T) {
@ -265,6 +371,7 @@ func TestGetChannelBookmarks(t *testing.T) {
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: th.BasicChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
@ -346,6 +453,7 @@ func TestUpdateChannelBookmarkSortOrder(t *testing.T) {
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: th.BasicChannel.Id,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",

4
server/channels/store/retrylayer/retrylayer.go
View File

@ -3153,11 +3153,11 @@ func (s *RetryLayerChannelBookmarkStore) Delete(bookmarkID string, deleteFile bo
}
func (s *RetryLayerChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileID string) error {
func (s *RetryLayerChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileID string, channelID string) error {
tries := 0
for {
err := s.ChannelBookmarkStore.ErrorIfBookmarkFileInfoAlreadyAttached(fileID)
err := s.ChannelBookmarkStore.ErrorIfBookmarkFileInfoAlreadyAttached(fileID, channelID)
if err == nil {
return nil
}

8
server/channels/store/sqlstore/channel_bookmark_store.go
View File

@ -51,7 +51,7 @@ func bookmarkWithFileInfoSliceColumns() []string {
}
}
func (s *SqlChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileId string) error {
func (s *SqlChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileId string, channelId string) error {
existingQuery := s.getSubQueryBuilder().
Select("FileInfoId").
From("ChannelBookmarks").
@ -66,11 +66,13 @@ func (s *SqlChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileId
Where(sq.Or{
sq.Expr("Id IN (?)", existingQuery),
sq.And{
sq.Eq{"Id": fileId},
sq.Or{
sq.NotEq{"PostId": ""},
sq.NotEq{"CreatorId": model.BookmarkFileOwner},
sq.NotEq{"ChannelId": channelId},
sq.NotEq{"DeleteAt": 0},
},
sq.Eq{"Id": fileId},
},
})
@ -139,7 +141,7 @@ func (s *SqlChannelBookmarkStore) Save(bookmark *model.ChannelBookmark, increase
}
if bookmark.FileId != "" {
err = s.ErrorIfBookmarkFileInfoAlreadyAttached(bookmark.FileId)
err = s.ErrorIfBookmarkFileInfoAlreadyAttached(bookmark.FileId, bookmark.ChannelId)
if err != nil {
return nil, errors.Wrap(err, "unable_to_save_channel_bookmark")
}

2
server/channels/store/store.go
View File

@ -1059,7 +1059,7 @@ type PostPersistentNotificationStore interface {
DeleteByTeam(teamIds []string) error
}
type ChannelBookmarkStore interface {
ErrorIfBookmarkFileInfoAlreadyAttached(fileID string) error
ErrorIfBookmarkFileInfoAlreadyAttached(fileID string, channelID string) error
Get(Id string, includeDeleted bool) (b *model.ChannelBookmarkWithFileInfo, err error)
Save(bookmark *model.ChannelBookmark, increaseSortOrder bool) (b *model.ChannelBookmarkWithFileInfo, err error)
Update(bookmark *model.ChannelBookmark) error

76
server/channels/store/storetest/channel_bookmark.go
View File

@ -34,8 +34,12 @@ func TestChannelBookmarkStore(t *testing.T, rctx request.CTX, ss store.Store, s
func testSaveChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
channelID := model.NewId()
otherChannelID := model.NewId()
userID := model.NewId()
createAt := time.Now().Add(-1 * time.Minute)
deleteAt := createAt.Add(1 * time.Second)
bookmark1 := &model.ChannelBookmark{
ChannelId: channelID,
OwnerId: userID,
@ -47,6 +51,7 @@ func testSaveChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: channelID,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
@ -80,6 +85,7 @@ func testSaveChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
file2 := &model.FileInfo{
Id: model.NewId(),
ChannelId: channelID,
CreatorId: userID,
Path: "somepath",
ThumbnailPath: "thumbpath",
@ -102,6 +108,60 @@ func testSaveChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
Emoji: ":smile:",
}
deletedFile := &model.FileInfo{
Id: model.NewId(),
ChannelId: channelID,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
PreviewPath: "prevPath",
Name: "test file",
Extension: "png",
MimeType: "images/png",
Size: 873182,
Width: 3076,
Height: 2200,
HasPreviewImage: true,
CreateAt: createAt.UnixMilli(),
UpdateAt: createAt.UnixMilli(),
DeleteAt: deleteAt.UnixMilli(),
}
bookmarkFileDeleted := &model.ChannelBookmark{
ChannelId: channelID,
OwnerId: userID,
DisplayName: "file deleted",
FileId: deletedFile.Id,
Type: model.ChannelBookmarkFile,
Emoji: ":smile:",
}
// another channel
anotherChannelFile := &model.FileInfo{
Id: model.NewId(),
ChannelId: otherChannelID,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
PreviewPath: "prevPath",
Name: "test file",
Extension: "png",
MimeType: "images/png",
Size: 873182,
Width: 3076,
Height: 2200,
HasPreviewImage: true,
}
bookmarkFileAnotherChannel := &model.ChannelBookmark{
ChannelId: channelID,
OwnerId: userID,
DisplayName: "file another channel",
FileId: anotherChannelFile.Id,
Type: model.ChannelBookmarkFile,
Emoji: ":smile:",
}
_, err := ss.FileInfo().Save(rctx, file)
require.NoError(t, err)
defer ss.FileInfo().PermanentDelete(rctx, file.Id)
@ -113,6 +173,14 @@ func testSaveChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
err = ss.FileInfo().AttachToPost(rctx, file2.Id, model.NewId(), channelID, userID)
require.NoError(t, err)
_, err = ss.FileInfo().Save(rctx, deletedFile)
require.NoError(t, err)
defer ss.FileInfo().PermanentDelete(rctx, deletedFile.Id)
_, err = ss.FileInfo().Save(rctx, anotherChannelFile)
require.NoError(t, err)
defer ss.FileInfo().PermanentDelete(rctx, anotherChannelFile.Id)
t.Run("save bookmarks", func(t *testing.T) {
bookmarkResp, err := ss.ChannelBookmark().Save(bookmark1.Clone(), true)
assert.NoError(t, err)
@ -137,6 +205,12 @@ func testSaveChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
_, err = ss.ChannelBookmark().Save(bookmark4.Clone(), true)
assert.Error(t, err) // Error as the file is attached to a post
_, err = ss.ChannelBookmark().Save(bookmarkFileDeleted.Clone(), true)
assert.Error(t, err) // Error as the file is deleted
_, err = ss.ChannelBookmark().Save(bookmarkFileAnotherChannel.Clone(), true)
assert.Error(t, err) // Error as the file is from another channel
})
}
@ -204,6 +278,7 @@ func testUpdateSortOrderChannelBookmark(t *testing.T, rctx request.CTX, ss store
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: channelID,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",
@ -391,6 +466,7 @@ func testDeleteChannelBookmark(t *testing.T, rctx request.CTX, ss store.Store) {
file := &model.FileInfo{
Id: model.NewId(),
ChannelId: channelID,
CreatorId: model.BookmarkFileOwner,
Path: "somepath",
ThumbnailPath: "thumbpath",

10
server/channels/store/storetest/mocks/ChannelBookmarkStore.go
View File

@ -32,17 +32,17 @@ func (_m *ChannelBookmarkStore) Delete(bookmarkID string, deleteFile bool) error
return r0
}
// ErrorIfBookmarkFileInfoAlreadyAttached provides a mock function with given fields: fileID
func (_m *ChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileID string) error {
ret := _m.Called(fileID)
// ErrorIfBookmarkFileInfoAlreadyAttached provides a mock function with given fields: fileID, channelID
func (_m *ChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileID string, channelID string) error {
ret := _m.Called(fileID, channelID)
if len(ret) == 0 {
panic("no return value specified for ErrorIfBookmarkFileInfoAlreadyAttached")
}
var r0 error
if rf, ok := ret.Get(0).(func(string) error); ok {
r0 = rf(fileID)
if rf, ok := ret.Get(0).(func(string, string) error); ok {
r0 = rf(fileID, channelID)
} else {
r0 = ret.Error(0)
}

4
server/channels/store/timerlayer/timerlayer.go
View File

@ -2614,10 +2614,10 @@ func (s *TimerLayerChannelBookmarkStore) Delete(bookmarkID string, deleteFile bo
return err
}
func (s *TimerLayerChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileID string) error {
func (s *TimerLayerChannelBookmarkStore) ErrorIfBookmarkFileInfoAlreadyAttached(fileID string, channelID string) error {
start := time.Now()
err := s.ChannelBookmarkStore.ErrorIfBookmarkFileInfoAlreadyAttached(fileID)
err := s.ChannelBookmarkStore.ErrorIfBookmarkFileInfoAlreadyAttached(fileID, channelID)
elapsed := float64(time.Since(start)) / float64(time.Second)
if s.Root.Metrics != nil {