Return error as last value in BulkImport functions (#30575)

server/channels/app/export_test.go

@@ -206,7 +206,7 @@ func TestExportAllUsers(t *testing.T) {
 
 	th2 := Setup(t)
 	defer th2.TearDown()
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, err)
 	assert.EqualValues(t, 0, i)
 
@@ -258,7 +258,7 @@ func TestExportAllBots(t *testing.T) {
 
 	th2 := Setup(t)
 	defer th2.TearDown()
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	require.Nil(t, err)
 	assert.EqualValues(t, 0, i)
 
@@ -309,8 +309,7 @@ func TestExportDMChannel(t *testing.T) {
 		assert.Equal(t, 0, len(channels))
 
 		// import the exported channel
-		var i int
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
-		appErr, i = th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 		require.Nil(t, appErr)
 		assert.Equal(t, 0, i)
 
@@ -352,7 +351,7 @@ func TestExportDMChannel(t *testing.T) {
 		defer th2.TearDown()
 
 		// import the exported channel
-		appErr, _ = th2.App.BulkImport(th2.Context, &b, nil, true, 5)
+		_, appErr = th2.App.BulkImport(th2.Context, &b, nil, true, 5)
 		require.Nil(t, appErr)
 
 		channels, nErr = th2.App.Srv().Store().Channel().GetAllDirectChannelsForExportAfter(1000, "00000000", false)
@@ -388,7 +387,7 @@ func TestExportDMChannel(t *testing.T) {
 		defer th2.TearDown()
 
 		// import the exported channel
-		err, _ = th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+		_, err = th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 		require.Nil(t, err)
 
 		channels, nErr = th2.App.Srv().Store().Channel().GetAllDirectChannelsForExportAfter(1000, "00000000", false)
@@ -429,7 +428,7 @@ func TestExportDMChannelToSelf(t *testing.T) {
 	assert.Equal(t, 0, len(channels))
 
 	// import the exported channel
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, err)
 	assert.EqualValues(t, 0, i)
 
@@ -501,7 +500,7 @@ func TestExportGMandDMChannels(t *testing.T) {
 	assert.Equal(t, 0, len(channels))
 
 	// import the exported channel
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, err)
 	assert.Equal(t, 0, i)
 
@@ -584,7 +583,7 @@ func TestExportDMandGMPost(t *testing.T) {
 	assert.Equal(t, 0, len(posts))
 
 	// import the exported posts
-	appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, appErr)
 	assert.Equal(t, 0, i)
 
@@ -661,7 +660,7 @@ func TestExportPostWithProps(t *testing.T) {
 	assert.Len(t, posts, 0)
 
 	// import the exported posts
-	appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, appErr)
 	assert.Equal(t, 0, i)
 
@@ -700,7 +699,7 @@ func TestExportUserCustomStatus(t *testing.T) {
 	th2 := Setup(t)
 	defer th2.TearDown()
 
-	appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+	i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 	require.Nil(t, appErr)
 	assert.Equal(t, 0, i)
 
@@ -737,7 +736,7 @@ func TestExportDMPostWithSelf(t *testing.T) {
 	assert.Equal(t, 0, len(posts))
 
 	// import the exported posts
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, err)
 	assert.Equal(t, 0, i)
 
@@ -922,7 +921,7 @@ func TestExportFileWarnings(t *testing.T) {
 			jsonFile := extractImportFile(filepath.Join(testsDir, "import_test.zip"))
 			defer jsonFile.Close()
 
-			appErr, _ := th.App.BulkImportWithPath(th.Context, jsonFile, nil, false, true, 1, dir)
+			_, appErr := th.App.BulkImportWithPath(th.Context, jsonFile, nil, false, true, 1, dir)
 			require.Nil(t, appErr)
 
 			// delete one of the files
@@ -1014,7 +1013,7 @@ func TestBulkExport(t *testing.T) {
 	jsonFile := extractImportFile(filepath.Join(testsDir, "import_test.zip"))
 	defer jsonFile.Close()
 
-	appErr, _ := th.App.BulkImportWithPath(th.Context, jsonFile, nil, false, true, 1, dir)
+	_, appErr := th.App.BulkImportWithPath(th.Context, jsonFile, nil, false, true, 1, dir)
 	require.Nil(t, appErr)
 
 	exportFile, err := os.Create(filepath.Join(dir, "export.zip"))
@@ -1035,7 +1034,7 @@ func TestBulkExport(t *testing.T) {
 	jsonFile = extractImportFile(filepath.Join(dir, "export.zip"))
 	defer jsonFile.Close()
 
-	appErr, _ = th.App.BulkImportWithPath(th.Context, jsonFile, nil, false, true, 1, filepath.Join(dir, "data"))
+	_, appErr = th.App.BulkImportWithPath(th.Context, jsonFile, nil, false, true, 1, filepath.Join(dir, "data"))
 	require.Nil(t, appErr)
 }
 
@@ -1121,7 +1120,7 @@ func TestExportDeletedTeams(t *testing.T) {
 
 	th2 := Setup(t)
 	defer th2.TearDown()
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, err)
 	assert.Equal(t, 0, i)
 
@@ -1161,7 +1160,7 @@ func TestExportArchivedChannels(t *testing.T) {
 
 	th2 := Setup(t)
 	defer th2.TearDown()
-	err, i := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
+	i, err := th2.App.BulkImport(th2.Context, &b, nil, false, 5)
 	assert.Nil(t, err)
 	assert.Equal(t, 0, i)
 
@@ -1194,7 +1193,7 @@ func TestExportRoles(t *testing.T) {
 
 		th2 := Setup(t)
 		defer th2.TearDown()
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		assert.Nil(t, appErr)
 		assert.Equal(t, 0, i)
 
@@ -1225,7 +1224,7 @@ func TestExportRoles(t *testing.T) {
 
 		th2 := Setup(t)
 		defer th2.TearDown()
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		require.Nil(t, appErr)
 		require.Equal(t, 0, i)
 
@@ -1262,7 +1261,7 @@ func TestExportRoles(t *testing.T) {
 
 		th2 := Setup(t)
 		defer th2.TearDown()
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		require.Nil(t, appErr)
 		require.Equal(t, 0, i)
 
@@ -1308,7 +1307,7 @@ func TestExportSchemes(t *testing.T) {
 		err = th2.App.Srv().Store().System().Save(&model.System{Name: model.MigrationKeyAdvancedPermissionsPhase2, Value: "true"})
 		require.NoError(t, err)
 
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		require.Nil(t, appErr)
 		require.Equal(t, 0, i)
 
@@ -1348,7 +1347,7 @@ func TestExportSchemes(t *testing.T) {
 		err = th2.App.Srv().Store().System().Save(&model.System{Name: model.MigrationKeyAdvancedPermissionsPhase2, Value: "true"})
 		require.NoError(t, err)
 
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		require.Nil(t, appErr)
 		require.Equal(t, 0, i)
 
@@ -1412,7 +1411,7 @@ func TestExportSchemes(t *testing.T) {
 		require.Nil(t, appErr)
 		require.Len(t, roles, builtInRoles)
 
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		require.Nil(t, appErr)
 		require.Equal(t, 0, i)
 
@@ -1520,7 +1519,7 @@ func TestExportSchemes(t *testing.T) {
 		require.Nil(t, appErr)
 		require.Len(t, roles, builtInRoles)
 
-		appErr, i := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
+		i, appErr := th2.App.BulkImport(th2.Context, &b, nil, false, 1)
 		require.Nil(t, appErr)
 		require.Equal(t, 0, i)
 

server/channels/app/import.go

@@ -181,11 +181,11 @@ func (a *App) bulkImportWorker(c request.CTX, dryRun, extractContent bool, wg *s
 	}
 }
 
-func (a *App) BulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun bool, workers int) (*model.AppError, int) {
+func (a *App) BulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun bool, workers int) (int, *model.AppError) {
 	return a.bulkImport(c, jsonlReader, attachmentsReader, dryRun, true, workers, "")
 }
 
-func (a *App) BulkImportWithPath(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun, extractContent bool, workers int, importPath string) (*model.AppError, int) {
+func (a *App) BulkImportWithPath(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun, extractContent bool, workers int, importPath string) (int, *model.AppError) {
 	return a.bulkImport(c, jsonlReader, attachmentsReader, dryRun, extractContent, workers, importPath)
 }
 
@@ -193,7 +193,7 @@ func (a *App) BulkImportWithPath(c request.CTX, jsonlReader io.Reader, attachmen
 // not nil. If it is nil, it will look for attachments on the
 // filesystem in the locations specified by the JSONL file according
 // to the older behavior
-func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun, extractContent bool, workers int, importPath string) (*model.AppError, int) {
+func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun, extractContent bool, workers int, importPath string) (int, *model.AppError) {
 	scanner := bufio.NewScanner(jsonlReader)
 	buf := make([]byte, 0, 64*1024)
 	scanner.Buffer(buf, maxScanTokenSize)
@@ -224,7 +224,7 @@ func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader
 
 		var line imports.LineImportData
 		if err := json.Unmarshal(scanner.Bytes(), &line); err != nil {
-			return model.NewAppError("BulkImport", "app.import.bulk_import.json_decode.error", nil, "", http.StatusBadRequest).Wrap(err), lineNumber
+			return lineNumber, model.NewAppError("BulkImport", "app.import.bulk_import.json_decode.error", nil, "", http.StatusBadRequest).Wrap(err)
 		}
 
 		if err := processAttachments(c, &line, importPath, attachedFiles); err != nil {
@@ -234,11 +234,11 @@ func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader
 		if lineNumber == 1 {
 			importDataFileVersion, appErr := processImportDataFileVersionLine(line)
 			if appErr != nil {
-				return appErr, lineNumber
+				return lineNumber, appErr
 			}
 
 			if importDataFileVersion != 1 {
-				return model.NewAppError("BulkImport", "app.import.bulk_import.unsupported_version.error", nil, "", http.StatusBadRequest), lineNumber
+				return lineNumber, model.NewAppError("BulkImport", "app.import.bulk_import.unsupported_version.error", nil, "", http.StatusBadRequest)
 			}
 			lastLineType = line.Type
 			continue
@@ -261,7 +261,7 @@ func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader
 				if len(errorsChan) != 0 {
 					err := <-errorsChan
 					if stopOnError(c, err) {
-						return err.Error, err.LineNumber
+						return err.LineNumber, err.Error
 					}
 				}
 			}
@@ -288,7 +288,7 @@ func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader
 			if stopOnError(c, err) {
 				close(linesChan)
 				wg.Wait()
-				return err.Error, err.LineNumber
+				return err.LineNumber, err.Error
 			}
 		}
 	}
@@ -303,15 +303,15 @@ func (a *App) bulkImport(c request.CTX, jsonlReader io.Reader, attachmentsReader
 	if len(errorsChan) != 0 {
 		err := <-errorsChan
 		if stopOnError(c, err) {
-			return err.Error, err.LineNumber
+			return err.LineNumber, err.Error
 		}
 	}
 
 	if err := scanner.Err(); err != nil {
-		return model.NewAppError("BulkImport", "app.import.bulk_import.file_scan.error", nil, "", http.StatusInternalServerError).Wrap(err), 0
+		return 0, model.NewAppError("BulkImport", "app.import.bulk_import.file_scan.error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
-	return nil, 0
+	return 0, nil
 }
 
 func processImportDataFileVersionLine(line imports.LineImportData) (int, *model.AppError) {

server/channels/app/import_test.go

@@ -178,13 +178,13 @@ func TestImportBulkImport(t *testing.T) {
 {"type": "direct_post", "direct_post": {"channel_members": ["` + username + `", "` + username2 + `", "` + username3 + `"], "user": "` + username + `", "message": "Hello Group Channel", "create_at": 123456789015}}
 {"type": "emoji", "emoji": {"name": "` + emojiName + `", "image": "` + testImage + `"}}`
 
-	err, line := th.App.BulkImport(th.Context, strings.NewReader(data1), nil, false, 2)
+	line, err := th.App.BulkImport(th.Context, strings.NewReader(data1), nil, false, 2)
 	require.Nil(t, err, "BulkImport should have succeeded")
 	require.Equal(t, 0, line, "BulkImport line should be 0")
 
 	// Run bulk import using a string that contains a line with invalid json.
 	data2 := `{"type": "version", "version": 1`
-	err, line = th.App.BulkImport(th.Context, strings.NewReader(data2), nil, false, 2)
+	line, err = th.App.BulkImport(th.Context, strings.NewReader(data2), nil, false, 2)
 	require.NotNil(t, err, "Should have failed due to invalid JSON on line 1.")
 	require.Equal(t, 1, line, "Should have failed due to invalid JSON on line 1.")
 
@@ -193,7 +193,7 @@ func TestImportBulkImport(t *testing.T) {
 {"type": "channel", "channel": {"type": "O", "display_name": "xr6m6udffngark2uekvr3hoeny", "team": "` + teamName + `", "name": "` + channelName + `"}}
 {"type": "user", "user": {"username": "kufjgnkxkrhhfgbrip6qxkfsaa", "email": "kufjgnkxkrhhfgbrip6qxkfsaa@example.com"}}
 {"type": "user", "user": {"username": "bwshaim6qnc2ne7oqkd5b2s2rq", "email": "bwshaim6qnc2ne7oqkd5b2s2rq@example.com", "teams": [{"name": "` + teamName + `", "channels": [{"name": "` + channelName + `"}]}]}}`
-	err, line = th.App.BulkImport(th.Context, strings.NewReader(data3), nil, false, 2)
+	line, err = th.App.BulkImport(th.Context, strings.NewReader(data3), nil, false, 2)
 	require.NotNil(t, err, "Should have failed due to missing version line on line 1.")
 	require.Equal(t, 1, line, "Should have failed due to missing version line on line 1.")
 
@@ -205,7 +205,7 @@ func TestImportBulkImport(t *testing.T) {
 {"type": "channel", "channel": {"type": "O", "display_name": "xr6m6udffngark2uekvr3hoeny", "team": "` + teamName + `", "name": "` + channelName + `"}}
 {"type": "user", "user": {"username": "` + username + `", "email": "` + username + `@example.com", "teams": [{"name": "` + teamName + `","theme": "` + teamTheme1 + `", "channels": [{"name": "` + channelName + `"}]}]}}
 {"type": "post", "post": {"team": "` + teamName + `", "channel": "` + channelName + `", "user": "` + username + `", "message": "Hello World", "create_at": 123456789012}}`
-		err, line = th.App.BulkImport(th.Context, strings.NewReader(data4+"\r\n"+posts), nil, false, 2)
+		line, err = th.App.BulkImport(th.Context, strings.NewReader(data4+"\r\n"+posts), nil, false, 2)
 		require.Nil(t, err, "BulkImport should have succeeded")
 		require.Equal(t, 0, line, "BulkImport line should be 0")
 	})
@@ -213,7 +213,7 @@ func TestImportBulkImport(t *testing.T) {
 	t.Run("First item after version without type", func(t *testing.T) {
 		data := `{"type": "version", "version": 1}
 {"name": "custom-emoji-troll", "image": "bulkdata/emoji/trollolol.png"}`
-		err, line := th.App.BulkImport(th.Context, strings.NewReader(data), nil, false, 2)
+		line, err := th.App.BulkImport(th.Context, strings.NewReader(data), nil, false, 2)
 		require.NotNil(t, err, "Should have failed due to invalid type on line 2.")
 		require.Equal(t, 2, line, "Should have failed due to invalid type on line 2.")
 	})
@@ -227,7 +227,7 @@ func TestImportBulkImport(t *testing.T) {
 {"type": "direct_channel", "direct_channel": {"members": ["` + username + `", "` + username + `"]}}
 {"type": "direct_post", "direct_post": {"channel_members": ["` + username + `", "` + username + `"], "user": "` + username + `", "message": "Hello Direct Channel to myself", "create_at": 123456789014, "props":{"attachments":[{"id":0,"fallback":"[February 4th, 2020 2:46 PM] author: fallback","color":"D0D0D0","pretext":"","author_name":"author","author_link":"","title":"","title_link":"","text":"this post has props","fields":null,"image_url":"","thumb_url":"","footer":"Posted in #general","footer_icon":"","ts":"1580823992.000100"}]}}}`
 
-		err, line := th.App.BulkImport(th.Context, strings.NewReader(data6), nil, false, 2)
+		line, err := th.App.BulkImport(th.Context, strings.NewReader(data6), nil, false, 2)
 		require.Nil(t, err, "BulkImport should have succeeded")
 		require.Equal(t, 0, line, "BulkImport line should be 0")
 	})
@@ -489,7 +489,7 @@ func TestImportBulkImportWithAttachments(t *testing.T) {
 
 	th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.MaxUsersPerTeam = model.NewPointer(1000) })
 
-	appErr, _ := th.App.BulkImportWithPath(th.Context, jsonFile, importZipReader, false, true, 1, model.ExportDataDir)
+	_, appErr := th.App.BulkImportWithPath(th.Context, jsonFile, importZipReader, false, true, 1, model.ExportDataDir)
 	require.Nil(t, appErr)
 
 	adminUser, appErr := th.App.GetUserByUsername("sysadmin")

server/channels/jobs/import_process/worker.go

@@ -29,7 +29,7 @@ type AppIface interface {
 	FileExists(path string) (bool, *model.AppError)
 	FileSize(path string) (int64, *model.AppError)
 	FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError)
-	BulkImportWithPath(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun, extractContent bool, workers int, importPath string) (*model.AppError, int)
+	BulkImportWithPath(c request.CTX, jsonlReader io.Reader, attachmentsReader *zip.Reader, dryRun, extractContent bool, workers int, importPath string) (int, *model.AppError)
 	Log() *mlog.Logger
 }
 
@@ -125,7 +125,7 @@ func MakeWorker(jobServer *jobs.JobServer, app AppIface) *jobs.SimpleWorker {
 
 		extractContent := job.Data["extract_content"] == "true"
 		// do the actual import.
-		appErr, lineNumber := app.BulkImportWithPath(appContext, jsonFile, importZipReader, false, extractContent, runtime.NumCPU(), model.ExportDataDir)
+		lineNumber, appErr := app.BulkImportWithPath(appContext, jsonFile, importZipReader, false, extractContent, runtime.NumCPU(), model.ExportDataDir)
 		if appErr != nil {
 			job.Data["line_number"] = strconv.Itoa(lineNumber)
 			return appErr

server/cmd/mattermost/commands/import.go

@@ -156,7 +156,7 @@ func bulkImportCmdF(command *cobra.Command, args []string) error {
 
 	CommandPrettyPrintln("")
 
-	if err, lineNumber := a.BulkImportWithPath(rctx, fileReader, nil, true, !apply, workers, importPath); err != nil {
+	if lineNumber, err := a.BulkImportWithPath(rctx, fileReader, nil, true, !apply, workers, importPath); err != nil {
 		CommandPrintErrorln(err.Error())
 		if lineNumber != 0 {
 			CommandPrintErrorln(fmt.Sprintf("Error occurred on data file line %v", lineNumber))