From 7b164d4f822a5b04dbf496eb55e226ec3a952217 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 16 Aug 2023 16:17:52 +0200 Subject: [PATCH] Fix MM-53949 (#24237) * Fix MM-53948 * Fix tests * Fix missing layers --- server/channels/app/app_iface.go | 1 + server/channels/app/draft.go | 15 +++++++- server/channels/app/draft_test.go | 38 ++++++++++++++++++- server/channels/app/file.go | 16 +++++++- .../app/opentracing/opentracing_layer.go | 22 +++++++++++ 5 files changed, 89 insertions(+), 3 deletions(-) diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index aca017db9e..4e47444fb7 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -1161,6 +1161,7 @@ type AppIface interface { UpdateUserRolesWithUser(c request.CTX, user *model.User, newRoles string, sendWebSocketEvent bool) (*model.User, *model.AppError) UploadData(c request.CTX, us *model.UploadSession, rd io.Reader) (*model.FileInfo, *model.AppError) UploadEmojiImage(c request.CTX, id string, imageData *multipart.FileHeader) *model.AppError + UploadFileForUserAndTeam(c request.CTX, data []byte, channelID string, filename string, rawUserId string, rawTeamId string) (*model.FileInfo, *model.AppError) UpsertDraft(c *request.Context, draft *model.Draft, connectionID string) (*model.Draft, *model.AppError) UpsertGroupMember(groupID string, userID string) (*model.GroupMember, *model.AppError) UpsertGroupMembers(groupID string, userIDs []string) ([]*model.GroupMember, *model.AppError) diff --git a/server/channels/app/draft.go b/server/channels/app/draft.go index 14f5f19df8..ee17eac892 100644 --- a/server/channels/app/draft.go +++ b/server/channels/app/draft.go @@ -107,11 +107,24 @@ func (a *App) getFileInfosForDraft(draft *model.Draft) ([]*model.FileInfo, *mode return nil, nil } - fileInfos, err := a.Srv().Store().FileInfo().GetByIds(draft.FileIds) + allFileInfos, err := a.Srv().Store().FileInfo().GetByIds(draft.FileIds) if err != nil { return nil, model.NewAppError("GetFileInfosForDraft", "app.draft.get_for_draft.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + fileInfos := []*model.FileInfo{} + for _, fileInfo := range allFileInfos { + if fileInfo.PostId == "" && fileInfo.CreatorId == draft.UserId { + fileInfos = append(fileInfos, fileInfo) + } else { + mlog.Debug("Invalid file id in draft", mlog.String("file_id", fileInfo.Id), mlog.String("user_id", draft.UserId)) + } + } + + if len(fileInfos) == 0 { + return nil, nil + } + a.generateMiniPreviewForInfos(fileInfos) return fileInfos, nil diff --git a/server/channels/app/draft_test.go b/server/channels/app/draft_test.go index dc8893e7f3..97acfe586e 100644 --- a/server/channels/app/draft_test.go +++ b/server/channels/app/draft_test.go @@ -283,7 +283,7 @@ func TestGetDraftsForUser(t *testing.T) { sent, readFileErr := testutils.ReadTestFile("test.png") require.NoError(t, readFileErr) - fileResp, updateDraftErr := th.App.UploadFile(th.Context, sent, channel.Id, "test.png") + fileResp, updateDraftErr := th.App.UploadFileForUserAndTeam(th.Context, sent, channel.Id, "test.png", user.Id, "") assert.Nil(t, updateDraftErr) draftWithFiles := draft1 @@ -303,11 +303,47 @@ func TestGetDraftsForUser(t *testing.T) { assert.Equal(t, draftWithFiles.ChannelId, draftsWithFilesResp[0].ChannelId) assert.ElementsMatch(t, draftWithFiles.FileIds, draftsWithFilesResp[0].FileIds) + assert.Len(t, draftsWithFilesResp[0].Metadata.Files, 1) assert.Equal(t, fileResp.Name, draftsWithFilesResp[0].Metadata.Files[0].Name) assert.Len(t, draftsWithFilesResp, 2) }) + t.Run("get draft with invalid files", func(t *testing.T) { + // upload file + sent, readFileErr := testutils.ReadTestFile("test.png") + require.NoError(t, readFileErr) + + fileResp1, updateDraftErr := th.App.UploadFileForUserAndTeam(th.Context, sent, channel.Id, "test1.png", user.Id, "") + assert.Nil(t, updateDraftErr) + + fileResp2, updateDraftErr := th.App.UploadFileForUserAndTeam(th.Context, sent, channel.Id, "test2.png", th.BasicUser2.Id, "") + assert.Nil(t, updateDraftErr) + + draftWithFiles := draft1 + draftWithFiles.FileIds = []string{fileResp1.Id, fileResp2.Id} + + draftResp, updateDraftErr := th.App.UpsertDraft(th.Context, draft1, "") + assert.Nil(t, updateDraftErr) + + assert.Equal(t, draftWithFiles.Message, draftResp.Message) + assert.Equal(t, draftWithFiles.ChannelId, draftResp.ChannelId) + assert.ElementsMatch(t, draftWithFiles.FileIds, draftResp.FileIds) + + assert.Len(t, draftWithFiles.Metadata.Files, 1) + assert.Equal(t, fileResp1.Name, draftWithFiles.Metadata.Files[0].Name) + + draftsWithFilesResp, err := th.App.GetDraftsForUser(user.Id, th.BasicTeam.Id) + assert.Nil(t, err) + + assert.Equal(t, draftWithFiles.Message, draftsWithFilesResp[0].Message) + assert.Equal(t, draftWithFiles.ChannelId, draftsWithFilesResp[0].ChannelId) + assert.ElementsMatch(t, draftWithFiles.FileIds, draftsWithFilesResp[0].FileIds) + + assert.Len(t, draftsWithFilesResp[0].Metadata.Files, 1) + assert.Equal(t, fileResp1.Name, draftsWithFilesResp[0].Metadata.Files[0].Name) + }) + t.Run("get drafts feature flag", func(t *testing.T) { os.Setenv("MM_FEATUREFLAGS_GLOBALDRAFTS", "false") defer os.Unsetenv("MM_FEATUREFLAGS_GLOBALDRAFTS") diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 3a37b8b1e7..31cf068367 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -555,13 +555,27 @@ func GeneratePublicLinkHash(fileID, salt string) string { // UploadFile uploads a single file in form of a completely constructed byte array for a channel. func (a *App) UploadFile(c request.CTX, data []byte, channelID string, filename string) (*model.FileInfo, *model.AppError) { + return a.UploadFileForUserAndTeam(c, data, channelID, filename, "", "") +} + +func (a *App) UploadFileForUserAndTeam(c request.CTX, data []byte, channelID string, filename string, rawUserId string, rawTeamId string) (*model.FileInfo, *model.AppError) { _, err := a.GetChannel(c, channelID) if err != nil && channelID != "" { return nil, model.NewAppError("UploadFile", "api.file.upload_file.incorrect_channelId.app_error", map[string]any{"channelId": channelID}, "", http.StatusBadRequest) } - info, _, appError := a.DoUploadFileExpectModification(c, time.Now(), "noteam", channelID, "nouser", filename, data) + userId := rawUserId + if userId == "" { + userId = "nouser" + } + + teamId := rawTeamId + if teamId == "" { + teamId = "noteam" + } + + info, _, appError := a.DoUploadFileExpectModification(c, time.Now(), teamId, channelID, userId, filename, data) if appError != nil { return nil, appError } diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index 528379bd5a..08f6d12278 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -18373,6 +18373,28 @@ func (a *OpenTracingAppLayer) UploadFile(c request.CTX, data []byte, channelID s return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) UploadFileForUserAndTeam(c request.CTX, data []byte, channelID string, filename string, rawUserId string, rawTeamId string) (*model.FileInfo, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UploadFileForUserAndTeam") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.UploadFileForUserAndTeam(c, data, channelID, filename, rawUserId, rawTeamId) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) UploadFileX(c *request.Context, channelID string, name string, input io.Reader, opts ...func(*app.UploadFileTask)) (*model.FileInfo, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UploadFileX")