From 27566f6a06317bf7c6e37991c7586ca27fb6846a Mon Sep 17 00:00:00 2001
From: Siyuan Liu <liusy182@gmail.com>
Date: Wed, 8 May 2019 13:29:42 -0700
Subject: [PATCH] [MM-13033] Continue bulk import on large image error (#10780)

* [MM-13033] Continue bulk import on large image error

* add tests, review comments

* Update app/import_functions.go

Co-Authored-By: liusy182 <liusy182@hotmail.com>
---
 app/import.go           | 27 ++++++++++++++++++++++-----
 app/import_functions.go |  7 +++----
 app/import_test.go      | 18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/app/import.go b/app/import.go
index 6ae76bc7e9..d86b239f2a 100644
--- a/app/import.go
+++ b/app/import.go
@@ -6,14 +6,25 @@ package app
 import (
 	"bufio"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"strings"
 	"sync"
 
+	"github.com/mattermost/mattermost-server/mlog"
+
 	"github.com/mattermost/mattermost-server/model"
 )
 
+func stopOnError(err LineImportWorkerError) bool {
+	if err.Error.Id == "api.file.upload_file.large_image.app_error" {
+		mlog.Warn(fmt.Sprintf("Large image import error: %s", err.Error.Error()))
+		return false
+	}
+	return true
+}
+
 func (a *App) bulkImportWorker(dryRun bool, wg *sync.WaitGroup, lines <-chan LineImportWorkerData, errors chan<- LineImportWorkerError) {
 	for line := range lines {
 		if err := a.ImportLine(line.LineImportData, dryRun); err != nil {
@@ -65,7 +76,9 @@ func (a *App) BulkImport(fileReader io.Reader, dryRun bool, workers int) (*model
 				// Check no errors occurred while waiting for the queue to empty.
 				if len(errorsChan) != 0 {
 					err := <-errorsChan
-					return err.Error, err.LineNumber
+					if stopOnError(err) {
+						return err.Error, err.LineNumber
+					}
 				}
 			}
 
@@ -81,9 +94,11 @@ func (a *App) BulkImport(fileReader io.Reader, dryRun bool, workers int) (*model
 		select {
 		case linesChan <- LineImportWorkerData{line, lineNumber}:
 		case err := <-errorsChan:
-			close(linesChan)
-			wg.Wait()
-			return err.Error, err.LineNumber
+			if stopOnError(err) {
+				close(linesChan)
+				wg.Wait()
+				return err.Error, err.LineNumber
+			}
 		}
 	}
 
@@ -94,7 +109,9 @@ func (a *App) BulkImport(fileReader io.Reader, dryRun bool, workers int) (*model
 	// Check no errors occurred while waiting for the queue to empty.
 	if len(errorsChan) != 0 {
 		err := <-errorsChan
-		return err.Error, err.LineNumber
+		if stopOnError(err) {
+			return err.Error, err.LineNumber
+		}
 	}
 
 	if err := scanner.Err(); err != nil {
diff --git a/app/import_functions.go b/app/import_functions.go
index fdbf7c5b82..cecdf67e33 100644
--- a/app/import_functions.go
+++ b/app/import_functions.go
@@ -909,7 +909,6 @@ func (a *App) ImportReply(data *ReplyImportData, post *model.Post, teamId string
 }
 
 func (a *App) ImportAttachment(data *AttachmentImportData, post *model.Post, teamId string, dryRun bool) (*model.FileInfo, *model.AppError) {
-	fileUploadError := model.NewAppError("BulkImport", "app.import.attachment.file_upload.error", map[string]interface{}{"FilePath": *data.Path}, "", http.StatusBadRequest)
 	file, err := os.Open(*data.Path)
 	if err != nil {
 		return nil, model.NewAppError("BulkImport", "app.import.attachment.bad_file.error", map[string]interface{}{"FilePath": *data.Path}, "", http.StatusBadRequest)
@@ -922,8 +921,8 @@ func (a *App) ImportAttachment(data *AttachmentImportData, post *model.Post, tea
 		fileInfo, err := a.DoUploadFile(timestamp, teamId, post.ChannelId, post.UserId, file.Name(), buf.Bytes())
 
 		if err != nil {
-			fmt.Print(err)
-			return nil, fileUploadError
+			mlog.Error(fmt.Sprintf("Failed to upload file: %s", err.Error()))
+			return nil, err
 		}
 
 		a.HandleImages([]string{fileInfo.PreviewPath}, []string{fileInfo.ThumbnailPath}, [][]byte{buf.Bytes()})
@@ -931,7 +930,7 @@ func (a *App) ImportAttachment(data *AttachmentImportData, post *model.Post, tea
 		mlog.Info(fmt.Sprintf("uploading file with name %s", file.Name()))
 		return fileInfo, nil
 	}
-	return nil, fileUploadError
+	return nil, model.NewAppError("BulkImport", "app.import.attachment.file_upload.error", map[string]interface{}{"FilePath": *data.Path}, "", http.StatusBadRequest)
 }
 
 func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError {
diff --git a/app/import_test.go b/app/import_test.go
index 21382b5679..6b1ece8468 100644
--- a/app/import_test.go
+++ b/app/import_test.go
@@ -4,6 +4,7 @@
 package app
 
 import (
+	"net/http"
 	"path/filepath"
 	"runtime/debug"
 	"strings"
@@ -159,6 +160,23 @@ func TestImportImportLine(t *testing.T) {
 	}
 }
 
+func TestStopOnError(t *testing.T) {
+	assert.True(t, stopOnError(LineImportWorkerError{
+		model.NewAppError("test", "app.import.attachment.bad_file.error", nil, "", http.StatusBadRequest),
+		1,
+	}))
+
+	assert.True(t, stopOnError(LineImportWorkerError{
+		model.NewAppError("test", "app.import.attachment.file_upload.error", nil, "", http.StatusBadRequest),
+		1,
+	}))
+
+	assert.False(t, stopOnError(LineImportWorkerError{
+		model.NewAppError("test", "api.file.upload_file.large_image.app_error", nil, "", http.StatusBadRequest),
+		1,
+	}))
+}
+
 func TestImportBulkImport(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()