[MM-36318] Set bot profile image and icon only if they have changed (#18008)

* Set bot profile image and icon only if they have changed

* Move equality check to the app layer

* Use ioutil.ReadAll

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

api4/user_test.go

@@ -3265,6 +3265,17 @@ func TestSetProfileImage(t *testing.T) {
 
 	ruser, appErr := th.App.GetUser(user.Id)
 	require.Nil(t, appErr)
+	assert.True(t, buser.LastPictureUpdate == ruser.LastPictureUpdate, "Same picture should not have updated")
+
+	data2, err := testutils.ReadTestFile("testjpg.jpg")
+	require.NoError(t, err)
+
+	_, resp = th.SystemAdminClient.SetProfileImage(user.Id, data2)
+	CheckNoError(t, resp)
+
+	ruser, appErr = th.App.GetUser(user.Id)
+	require.Nil(t, appErr)
+
 	assert.True(t, buser.LastPictureUpdate < ruser.LastPictureUpdate, "Picture should have updated for user")
 
 	info := &model.FileInfo{Path: "users/" + user.Id + "/profile.png"}

app/bot.go

@@ -4,12 +4,15 @@
 package app
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"mime/multipart"
 	"net/http"
+	"path/filepath"
 
 	"github.com/mattermost/mattermost-server/v6/app/imaging"
 	"github.com/mattermost/mattermost-server/v6/app/request"
@@ -592,9 +595,18 @@ func (a *App) SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppEr
 		return model.NewAppError("SetBotIconImage", "api.bot.set_bot_icon_image.parse.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
 
-	// Set icon
 	file.Seek(0, 0)
-	if _, err = a.WriteFile(file, getBotIconPath(botUserId)); err != nil {
+	data, readErr := ioutil.ReadAll(file)
+	if readErr != nil {
+		return model.NewAppError("SetBotIconImage", "api.bot.set_bot_icon_image.read.app_error", nil, readErr.Error(), http.StatusInternalServerError)
+	}
+
+	if storedData, readFileErr := a.ReadFile(getBotIconPath(botUserId)); readFileErr == nil && bytes.Equal(storedData, data) {
+		return nil
+	}
+
+	// Set icon
+	if _, err = a.WriteFile(bytes.NewReader(data), getBotIconPath(botUserId)); err != nil {
 		return model.NewAppError("SetBotIconImage", "api.bot.set_bot_icon_image.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
@@ -666,5 +678,5 @@ func (a *App) GetBotIconImage(botUserId string) ([]byte, *model.AppError) {
 }
 
 func getBotIconPath(botUserId string) string {
-	return fmt.Sprintf("bots/%v/icon.svg", botUserId)
+	return filepath.Join("bots", botUserId, "icon.svg")
 }

app/plugin_api.go

@@ -661,8 +661,7 @@ func (api *PluginAPI) GetProfileImage(userID string) ([]byte, *model.AppError) {
 }
 
 func (api *PluginAPI) SetProfileImage(userID string, data []byte) *model.AppError {
-	_, err := api.app.GetUser(userID)
+	if _, err := api.app.GetUser(userID); err != nil {
-	if err != nil {
 		return err
 	}
 
@@ -949,10 +948,6 @@ func (api *PluginAPI) GetBotIconImage(userID string) ([]byte, *model.AppError) {
 }
 
 func (api *PluginAPI) SetBotIconImage(userID string, data []byte) *model.AppError {
-	if _, err := api.app.GetBot(userID, true); err != nil {
-		return err
-	}
-
 	return api.app.SetBotIconImage(userID, bytes.NewReader(data))
 }
 

app/user.go

@@ -12,6 +12,7 @@ import (
 	"io"
 	"mime/multipart"
 	"net/http"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -715,8 +716,7 @@ func (a *App) SetDefaultProfileImage(user *model.User) *model.AppError {
 		return appErr
 	}
 
-	path := "users/" + user.Id + "/profile.png"
+	path := getProfileImagePath(user.Id)
-
 	if _, err := a.WriteFile(bytes.NewReader(img), path); err != nil {
 		return err
 	}
@@ -787,7 +787,11 @@ func (a *App) SetProfileImageFromFile(userID string, file io.Reader) *model.AppE
 	if err != nil {
 		return err
 	}
-	path := "users/" + userID + "/profile.png"
+
+	path := getProfileImagePath(userID)
+	if storedData, err := a.ReadFile(path); err == nil && bytes.Equal(storedData, buf.Bytes()) {
+		return nil
+	}
 
 	if _, err := a.WriteFile(buf, path); err != nil {
 		return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.upload_profile.app_error", nil, err.Error(), http.StatusInternalServerError)
@@ -2310,3 +2314,7 @@ func (a *App) UpdateThreadReadForUser(userID, teamID, threadID string, timestamp
 	a.Publish(message)
 	return thread, nil
 }
+
+func getProfileImagePath(userID string) string {
+	return filepath.Join("users", userID, "profile.png")
+}

i18n/en.json

@@ -211,6 +211,10 @@
     "id": "api.bot.set_bot_icon_image.parse.app_error",
     "translation": "Could not parse multipart form."
   },
+  {
+    "id": "api.bot.set_bot_icon_image.read.app_error",
+    "translation": "Could not read image data."
+  },
   {
     "id": "api.bot.set_bot_icon_image.too_large.app_error",
     "translation": "Unable to upload icon image. File is too large."