From 50c7f1df1279c8e060b08673909c87cc1d42399a Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Thu, 6 Feb 2025 10:34:34 -0500 Subject: [PATCH] [MM-62876] Bulk export warnings (#30105) * err -> appErr * record warnings in job.Data, and output them to warnings.txt * tests --- server/channels/app/export.go | 76 +++++++++++++++++++----------- server/channels/app/export_test.go | 37 ++++++++++++++- 2 files changed, 83 insertions(+), 30 deletions(-) diff --git a/server/channels/app/export.go b/server/channels/app/export.go index 0258a5171e..0c3df35b5a 100644 --- a/server/channels/app/export.go +++ b/server/channels/app/export.go @@ -27,6 +27,8 @@ import ( "github.com/mattermost/mattermost/server/v8/platform/shared/filestore" ) +const warningsFilename = "warnings.txt" + // We use this map to identify the exportable preferences. // Here we link the preference category and name, to the name of the relevant field in the import struct. var exportablePreferences = map[imports.ComparablePreference]string{ @@ -145,62 +147,65 @@ func (a *App) BulkExport(ctx request.CTX, writer io.Writer, outPath string, job } ctx.Logger().Info("Bulk export: exporting teams") - teamNames, err := a.exportAllTeams(ctx, job, writer) - if err != nil { - return err + teamNames, appErr := a.exportAllTeams(ctx, job, writer) + if appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting channels") - if err = a.exportAllChannels(ctx, job, writer, teamNames, opts.IncludeArchivedChannels); err != nil { - return err + if appErr = a.exportAllChannels(ctx, job, writer, teamNames, opts.IncludeArchivedChannels); appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting users") - profilePictures, err := a.exportAllUsers(ctx, job, writer, opts.IncludeArchivedChannels, opts.IncludeProfilePictures) - if err != nil { - return err + profilePictures, appErr := a.exportAllUsers(ctx, job, writer, opts.IncludeArchivedChannels, opts.IncludeProfilePictures) + if appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting bots") - botPPs, err := a.exportAllBots(ctx, job, writer, opts.IncludeProfilePictures) - if err != nil { - return err + botPPs, appErr := a.exportAllBots(ctx, job, writer, opts.IncludeProfilePictures) + if appErr != nil { + return appErr } profilePictures = append(profilePictures, botPPs...) ctx.Logger().Info("Bulk export: exporting posts") - attachments, err := a.exportAllPosts(ctx, job, writer, opts.IncludeAttachments, opts.IncludeArchivedChannels) - if err != nil { - return err + attachments, appErr := a.exportAllPosts(ctx, job, writer, opts.IncludeAttachments, opts.IncludeArchivedChannels) + if appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting emoji") - emojiPaths, err := a.exportCustomEmoji(ctx, job, writer, outPath, "exported_emoji", !opts.CreateArchive) - if err != nil { - return err + emojiPaths, appErr := a.exportCustomEmoji(ctx, job, writer, outPath, "exported_emoji", !opts.CreateArchive) + if appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting direct channels") - if err = a.exportAllDirectChannels(ctx, job, writer, opts.IncludeArchivedChannels); err != nil { - return err + if appErr = a.exportAllDirectChannels(ctx, job, writer, opts.IncludeArchivedChannels); appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting direct posts") - directAttachments, err := a.exportAllDirectPosts(ctx, job, writer, opts.IncludeAttachments, opts.IncludeArchivedChannels) - if err != nil { - return err + directAttachments, appErr := a.exportAllDirectPosts(ctx, job, writer, opts.IncludeAttachments, opts.IncludeArchivedChannels) + if appErr != nil { + return appErr } if opts.IncludeAttachments { ctx.Logger().Info("Bulk export: exporting file attachments") - if err = a.exportAttachments(ctx, attachments, outPath, zipWr); err != nil { - return err + warnings, appErr := a.exportAttachments(ctx, attachments, outPath, zipWr) + if appErr != nil { + return appErr } ctx.Logger().Info("Bulk export: exporting direct file attachments") - if err = a.exportAttachments(ctx, directAttachments, outPath, zipWr); err != nil { - return err + newWarnings, appErr := a.exportAttachments(ctx, directAttachments, outPath, zipWr) + if appErr != nil { + return appErr } + warnings = append(warnings, newWarnings...) totalExportedEmojis := 0 emojisLen := len(emojiPaths) @@ -216,6 +221,18 @@ func (a *App) BulkExport(ctx request.CTX, writer io.Writer, outPath string, job } } + if len(warnings) > 0 { + warningsFile, _ := zipWr.Create(warningsFilename) + for _, warning := range warnings { + _, err := warningsFile.Write([]byte(warning + "\n")) + if err != nil { + return model.NewAppError("BulkExport", "app.export.zip_create.error", + nil, "err="+err.Error(), http.StatusInternalServerError) + } + } + updateJobProgress(ctx.Logger(), a.Srv().Store(), job, "num_warnings", len(warnings)) + } + updateJobProgress(ctx.Logger(), a.Srv().Store(), job, "attachments_exported", len(attachments)+len(directAttachments)+len(emojiPaths)) } @@ -232,12 +249,15 @@ func (a *App) BulkExport(ctx request.CTX, writer io.Writer, outPath string, job return nil } -func (a *App) exportAttachments(ctx request.CTX, attachments []imports.AttachmentImportData, outPath string, zipWr *zip.Writer) *model.AppError { +func (a *App) exportAttachments(ctx request.CTX, attachments []imports.AttachmentImportData, outPath string, + zipWr *zip.Writer) ([]string, *model.AppError) { totalExportedFiles := 0 attachmentsLen := len(attachments) + var warnings []string for _, attachment := range attachments { if err := a.exportFile(ctx, outPath, *attachment.Path, zipWr); err != nil { ctx.Logger().Warn("Unable to export file attachment", mlog.String("attachment_path", *attachment.Path), mlog.Err(err)) + warnings = append(warnings, fmt.Sprintf("Unable to export file attachment, attachment path: %s , error: %s", *attachment.Path, err.Error())) } else { totalExportedFiles++ } @@ -245,7 +265,7 @@ func (a *App) exportAttachments(ctx request.CTX, attachments []imports.Attachmen ctx.Logger().Info("Bulk export: exporting file attachments progress", mlog.Int("total_successfully_exported_files", totalExportedFiles), mlog.Int("total_files_to_export", attachmentsLen)) } } - return nil + return warnings, nil } func (a *App) exportWriteLine(w io.Writer, line *imports.LineImportData) *model.AppError { diff --git a/server/channels/app/export_test.go b/server/channels/app/export_test.go index e746ed6fe2..9bd58a0120 100644 --- a/server/channels/app/export_test.go +++ b/server/channels/app/export_test.go @@ -938,18 +938,51 @@ func TestExportFileWarnings(t *testing.T) { exportFile, err := os.Create(filepath.Join(dir, "export.zip")) require.NoError(t, err) - defer exportFile.Close() + + job, appErr := th.App.Srv().Jobs.CreateJob(th.Context, model.JobTypeExportProcess, nil) + require.Nil(t, appErr) + ok, appErr := th.App.Srv().Jobs.ClaimJob(job) + require.Nil(t, appErr) + require.True(t, ok) + job, appErr = th.App.Srv().Jobs.GetJob(th.Context, job.Id) + require.Nil(t, appErr) opts := model.BulkExportOpts{ IncludeAttachments: true, CreateArchive: true, } - appErr = th.App.BulkExport(th.Context, exportFile, dir, nil, opts) + appErr = th.App.BulkExport(th.Context, exportFile, dir, job, opts) // should not get an error for the missing file require.Nil(t, appErr) // should get a warning instead: testlib.AssertLog(t, buffer, mlog.LvlWarn.Name, "Unable to export file attachment") + + // should get info in the job data: + job, appErr = th.App.Srv().Jobs.GetJob(th.Context, job.Id) + require.Nil(t, appErr) + warnings, ok := job.Data["num_warnings"] + require.True(t, ok) + require.Equal(t, "1", warnings) + + exportFile.Close() + + // Verify warnings.txt exists in the zip and contains expected content + exportZipPath := filepath.Join(dir, "export.zip") + exportZipFile, err := os.Open(exportZipPath) + require.NoError(t, err) + defer exportZipFile.Close() + + info, err := exportZipFile.Stat() + require.NoError(t, err) + + paths, err := utils.UnzipToPath(exportZipFile, info.Size(), dir) + require.NoError(t, err) + require.Contains(t, paths, filepath.Join(dir, warningsFilename)) + + warningsContent, err := os.ReadFile(filepath.Join(dir, warningsFilename)) + require.NoError(t, err) + require.Contains(t, string(warningsContent), "Unable to export file attachment, attachment path:") }) } }