MM-16872 - Extend Plugin API to set LHS bot icon (#11601)

* MM-16872 - Extend Plugin API to set LHS bot icon

* MM-16872 - Using ReadSeeker as opposed to Reader for reading svg image file

* MM-16872 - PR feedback

* MM-16872 - Using userId rather than bot.UserId

* MM-16872 - Minor stylistic changes

* MM-16872 - Removing DriverName check
This commit is contained in:
Ali Farooq
2019-07-11 12:00:12 -04:00
committed by GitHub
parent 76f4fccf8a
commit 5ed40a48c8
10 changed files with 388 additions and 57 deletions
+8 -22
View File
@@ -244,13 +244,15 @@ func getBotIconImage(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
user, err := c.App.GetUser(botUserId)
img, err := c.App.GetBotIconImage(botUserId)
if err != nil {
c.Err = err
return
}
if !user.IsBot {
c.Err = model.MakeBotNotFoundError(botUserId)
user, err := c.App.GetUser(botUserId)
if err != nil {
c.Err = err
return
}
@@ -259,19 +261,8 @@ func getBotIconImage(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
img, readFailed, err := c.App.GetBotIconImage(user.Id)
if err != nil {
c.Err = err
return
}
if readFailed {
w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%v, public", 5*60)) // 5 mins
} else {
w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%v, public", 24*60*60)) // 24 hrs
w.Header().Set(model.HEADER_ETAG_SERVER, etag)
}
w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%v, public", 24*60*60)) // 24 hrs
w.Header().Set(model.HEADER_ETAG_SERVER, etag)
w.Header().Set("Content-Type", "image/svg+xml")
w.Write(img)
}
@@ -290,11 +281,6 @@ func setBotIconImage(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if _, err := c.App.GetBot(botUserId, true); err != nil {
c.Err = model.MakeBotNotFoundError(botUserId)
return
}
if r.ContentLength > *c.App.Config().FileSettings.MaxFileSize {
c.Err = model.NewAppError("setBotIconImage", "api.bot.set_bot_icon_image.too_large.app_error", nil, "", http.StatusRequestEntityTooLarge)
return
@@ -318,7 +304,7 @@ func setBotIconImage(c *Context, w http.ResponseWriter, r *http.Request) {
}
imageData := imageArray[0]
if err := c.App.SetBotIconImage(botUserId, imageData); err != nil {
if err := c.App.SetBotIconImageFromMultiPartFile(botUserId, imageData); err != nil {
c.Err = err
return
}
+7 -1
View File
@@ -1155,7 +1155,13 @@ func TestSetBotIconImage(t *testing.T) {
_, resp = th.SystemAdminClient.SetBotIconImage(bot.UserId, goodData)
CheckNoError(t, resp)
info := &model.FileInfo{Path: "/bots/" + bot.UserId + "/icon.svg"}
fpath := fmt.Sprintf("/bots/%v/icon.svg", bot.UserId)
actualData, err := th.App.ReadFile(fpath)
require.Nil(t, err)
require.NotNil(t, actualData)
require.Equal(t, goodData, actualData)
info := &model.FileInfo{Path: fpath}
err = th.cleanupTestFile(info)
require.Nil(t, err)
}
+21 -14
View File
@@ -5,6 +5,7 @@ package app
import (
"fmt"
"io"
"mime/multipart"
"net/http"
@@ -197,19 +198,25 @@ func (a *App) ConvertUserToBot(user *model.User) (*model.Bot, *model.AppError) {
return a.Srv.Store.Bot().Save(model.BotFromUser(user))
}
// SetBotIconImage sets LHS icon for a bot.
func (a *App) SetBotIconImage(botUserId string, imageData *multipart.FileHeader) *model.AppError {
if len(*a.Config().FileSettings.DriverName) == 0 {
return model.NewAppError("SetBotIconImage", "api.bot.icon_image.storage.app_error", nil, "", http.StatusNotImplemented)
}
// SetBotIconImageFromMultiPartFile sets LHS icon for a bot.
func (a *App) SetBotIconImageFromMultiPartFile(botUserId string, imageData *multipart.FileHeader) *model.AppError {
file, err := imageData.Open()
if err != nil {
return model.NewAppError("SetBotIconImage", "api.bot.set_bot_icon_image.open.app_error", nil, err.Error(), http.StatusBadRequest)
}
defer file.Close()
if _, err = parseSVG(file); err != nil {
file.Seek(0, 0)
return a.SetBotIconImage(botUserId, file)
}
// SetBotIconImage sets LHS icon for a bot.
func (a *App) SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppError {
if _, err := a.GetBot(botUserId, true); err != nil {
return err
}
if _, err := parseSVG(file); err != nil {
return model.NewAppError("SetBotIconImage", "api.bot.set_bot_icon_image.parse.app_error", nil, err.Error(), http.StatusBadRequest)
}
@@ -229,8 +236,8 @@ func (a *App) SetBotIconImage(botUserId string, imageData *multipart.FileHeader)
// DeleteBotIconImage deletes LHS icon for a bot.
func (a *App) DeleteBotIconImage(botUserId string) *model.AppError {
if len(*a.Config().FileSettings.DriverName) == 0 {
return model.NewAppError("DeleteBotIconImage", "api.bot.icon_image.storage.app_error", nil, "", http.StatusNotImplemented)
if _, err := a.GetBot(botUserId, true); err != nil {
return err
}
// Delete icon
@@ -247,17 +254,17 @@ func (a *App) DeleteBotIconImage(botUserId string) *model.AppError {
}
// GetBotIconImage retrieves LHS icon for a bot.
func (a *App) GetBotIconImage(botUserId string) ([]byte, bool, *model.AppError) {
if len(*a.Config().FileSettings.DriverName) == 0 {
return nil, false, model.NewAppError("GetBotIconImage", "api.bot.icon_image.storage.app_error", nil, "", http.StatusNotImplemented)
func (a *App) GetBotIconImage(botUserId string) ([]byte, *model.AppError) {
if _, err := a.GetBot(botUserId, true); err != nil {
return nil, err
}
data, err := a.ReadFile(getBotIconPath(botUserId))
if err != nil {
return nil, false, model.NewAppError("GetBotIconImage", "api.bot.get_bot_icon_image.read.app_error", nil, err.Error(), http.StatusNotFound)
return nil, model.NewAppError("GetBotIconImage", "api.bot.get_bot_icon_image.read.app_error", nil, err.Error(), http.StatusNotFound)
}
return data, false, nil
return data, nil
}
func getBotIconPath(botUserId string) string {
+163
View File
@@ -5,6 +5,9 @@ package app
import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"
@@ -12,6 +15,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/utils/fileutils"
)
func TestCreateBot(t *testing.T) {
@@ -596,6 +600,165 @@ func TestConvertUserToBot(t *testing.T) {
})
}
func TestSetBotIconImage(t *testing.T) {
t.Run("invalid bot", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
path, _ := fileutils.FindDir("tests")
svgFile, fileErr := os.Open(filepath.Join(path, "test.svg"))
require.NoError(t, fileErr)
defer svgFile.Close()
err := th.App.SetBotIconImage("invalid_bot_id", svgFile)
require.NotNil(t, err)
})
t.Run("valid bot", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
// Set an icon image
path, _ := fileutils.FindDir("tests")
svgFile, fileErr := os.Open(filepath.Join(path, "test.svg"))
require.NoError(t, fileErr)
defer svgFile.Close()
expectedData, fileErr := ioutil.ReadAll(svgFile)
require.Nil(t, fileErr)
require.NotNil(t, expectedData)
bot, err := th.App.ConvertUserToBot(&model.User{
Username: "username",
Id: th.BasicUser.Id,
})
defer th.App.PermanentDeleteBot(bot.UserId)
fpath := fmt.Sprintf("/bots/%v/icon.svg", bot.UserId)
exists, err := th.App.FileExists(fpath)
require.Nil(t, err)
require.False(t, exists, "icon.svg shouldn't exist for the bot")
svgFile.Seek(0, 0)
err = th.App.SetBotIconImage(bot.UserId, svgFile)
require.Nil(t, err)
exists, err = th.App.FileExists(fpath)
require.Nil(t, err)
require.True(t, exists, "icon.svg should exist for the bot")
actualData, err := th.App.ReadFile(fpath)
require.Nil(t, err)
require.NotNil(t, actualData)
require.Equal(t, expectedData, actualData)
})
}
func TestGetBotIconImage(t *testing.T) {
t.Run("invalid bot", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
actualData, err := th.App.GetBotIconImage("invalid_bot_id")
require.NotNil(t, err)
require.Nil(t, actualData)
})
t.Run("valid bot", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
// Set an icon image
path, _ := fileutils.FindDir("tests")
svgFile, fileErr := os.Open(filepath.Join(path, "test.svg"))
require.NoError(t, fileErr)
defer svgFile.Close()
expectedData, fileErr := ioutil.ReadAll(svgFile)
require.Nil(t, fileErr)
require.NotNil(t, expectedData)
bot, err := th.App.ConvertUserToBot(&model.User{
Username: "username",
Id: th.BasicUser.Id,
})
defer th.App.PermanentDeleteBot(bot.UserId)
svgFile.Seek(0, 0)
fpath := fmt.Sprintf("/bots/%v/icon.svg", bot.UserId)
_, err = th.App.WriteFile(svgFile, fpath)
require.Nil(t, err)
actualBytes, err := th.App.GetBotIconImage(bot.UserId)
require.Nil(t, err)
require.NotNil(t, actualBytes)
actualData, err := th.App.ReadFile(fpath)
require.Nil(t, err)
require.NotNil(t, actualData)
require.Equal(t, expectedData, actualData)
})
}
func TestDeleteBotIconImage(t *testing.T) {
t.Run("invalid bot", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
err := th.App.DeleteBotIconImage("invalid_bot_id")
require.NotNil(t, err)
})
t.Run("valid bot", func(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
// Set an icon image
path, _ := fileutils.FindDir("tests")
svgFile, fileErr := os.Open(filepath.Join(path, "test.svg"))
require.NoError(t, fileErr)
defer svgFile.Close()
expectedData, fileErr := ioutil.ReadAll(svgFile)
require.Nil(t, fileErr)
require.NotNil(t, expectedData)
bot, err := th.App.ConvertUserToBot(&model.User{
Username: "username",
Id: th.BasicUser.Id,
})
defer th.App.PermanentDeleteBot(bot.UserId)
// Set icon
svgFile.Seek(0, 0)
err = th.App.SetBotIconImage(bot.UserId, svgFile)
require.Nil(t, err)
// Get icon
actualData, err := th.App.GetBotIconImage(bot.UserId)
require.Nil(t, err)
require.NotNil(t, actualData)
require.Equal(t, expectedData, actualData)
// Bot icon should exist
fpath := fmt.Sprintf("/bots/%v/icon.svg", bot.UserId)
exists, err := th.App.FileExists(fpath)
require.Nil(t, err)
require.True(t, exists, "icon.svg should exist for the bot")
// Delete icon
err = th.App.DeleteBotIconImage(bot.UserId)
require.Nil(t, err)
// Bot icon should not exist
exists, err = th.App.FileExists(fpath)
require.Nil(t, err)
require.False(t, exists, "icon.svg should be deleted for the bot")
})
}
func sToP(s string) *string {
return &s
}
+26 -12
View File
@@ -500,12 +500,7 @@ func (api *PluginAPI) SetProfileImage(userId string, data []byte) *model.AppErro
return err
}
fileReader := bytes.NewReader(data)
err = api.app.SetProfileImageFromFile(userId, fileReader)
if err != nil {
return err
}
return nil
return api.app.SetProfileImageFromFile(userId, bytes.NewReader(data))
}
func (api *PluginAPI) GetEmojiList(sortBy string, page, perPage int) ([]*model.Emoji, *model.AppError) {
@@ -580,12 +575,7 @@ func (api *PluginAPI) SetTeamIcon(teamId string, data []byte) *model.AppError {
return err
}
fileReader := bytes.NewReader(data)
err = api.app.SetTeamIconFromFile(team, fileReader)
if err != nil {
return err
}
return nil
return api.app.SetTeamIconFromFile(team, bytes.NewReader(data))
}
func (api *PluginAPI) OpenInteractiveDialog(dialog model.OpenDialogRequest) *model.AppError {
@@ -756,3 +746,27 @@ func (api *PluginAPI) UpdateBotActive(userId string, active bool) (*model.Bot, *
func (api *PluginAPI) PermanentDeleteBot(userId string) *model.AppError {
return api.app.PermanentDeleteBot(userId)
}
func (api *PluginAPI) GetBotIconImage(userId string) ([]byte, *model.AppError) {
if _, err := api.app.GetBot(userId, true); err != nil {
return nil, err
}
return api.app.GetBotIconImage(userId)
}
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))
}
func (api *PluginAPI) DeleteBotIconImage(userId string) *model.AppError {
if _, err := api.app.GetBot(userId, true); err != nil {
return err
}
return api.app.DeleteBotIconImage(userId)
}
-4
View File
@@ -143,10 +143,6 @@
"id": "api.bot.get_bot_icon_image.read.app_error",
"translation": "Unable to read icon image file"
},
{
"id": "api.bot.icon_image.storage.app_error",
"translation": "Image storage is not configured."
},
{
"id": "api.bot.set_bot_icon_image.app_error",
"translation": "Couldn't upload icon image"
+4 -4
View File
@@ -1497,7 +1497,7 @@ func (c *Client4) AssignBot(botUserId, newOwnerId string) (*Bot, *Response) {
return BotFromJson(r.Body), BuildResponse(r)
}
// SetBotIconImage sets icon image of the user.
// SetBotIconImage sets LHS bot icon image.
func (c *Client4) SetBotIconImage(botUserId string, data []byte) (bool, *Response) {
body := &bytes.Buffer{}
writer := multipart.NewWriter(body)
@@ -1538,7 +1538,7 @@ func (c *Client4) SetBotIconImage(botUserId string, data []byte) (bool, *Respons
return CheckStatusOK(rp), BuildResponse(rp)
}
// GetBotIconImage gets user's LHS icon image. Must be logged in.
// GetBotIconImage gets LHS bot icon image. Must be logged in.
func (c *Client4) GetBotIconImage(botUserId string) ([]byte, *Response) {
r, appErr := c.DoApiGet(c.GetBotRoute(botUserId)+"/icon", "")
if appErr != nil {
@@ -1553,7 +1553,7 @@ func (c *Client4) GetBotIconImage(botUserId string) ([]byte, *Response) {
return data, BuildResponse(r)
}
// DeleteBotIconImage deletes user's LHS icon image. Must be logged in.
// DeleteBotIconImage deletes LHS bot icon image. Must be logged in.
func (c *Client4) DeleteBotIconImage(botUserId string) (bool, *Response) {
r, appErr := c.DoApiDelete(c.GetBotRoute(botUserId) + "/icon")
if appErr != nil {
@@ -3586,7 +3586,7 @@ func (c *Client4) GetBrandImage() ([]byte, *Response) {
return data, BuildResponse(r)
}
// DeleteBrandImage delets the brand image for the system.
// DeleteBrandImage deletes the brand image for the system.
func (c *Client4) DeleteBrandImage() *Response {
r, err := c.DoApiDelete(c.GetBrandRoute() + "/image")
if err != nil {
+16
View File
@@ -569,6 +569,22 @@ type API interface {
//
// Minimum server version: 5.10
PermanentDeleteBot(botUserId string) *model.AppError
// GetBotIconImage gets LHS bot icon image.
//
// Minimum server version: 5.14
GetBotIconImage(botUserId string) ([]byte, *model.AppError)
// SetBotIconImage sets LHS bot icon image.
// Icon image must be SVG format, all other formats are rejected.
//
// Minimum server version: 5.14
SetBotIconImage(botUserId string, data []byte) *model.AppError
// DeleteBotIconImage deletes LHS bot icon image.
//
// Minimum server version: 5.14
DeleteBotIconImage(botUserId string) *model.AppError
}
var handshake = plugin.HandshakeConfig{
+86
View File
@@ -4100,3 +4100,89 @@ func (s *apiRPCServer) PermanentDeleteBot(args *Z_PermanentDeleteBotArgs, return
}
return nil
}
type Z_GetBotIconImageArgs struct {
A string
}
type Z_GetBotIconImageReturns struct {
A []byte
B *model.AppError
}
func (g *apiRPCClient) GetBotIconImage(botUserId string) ([]byte, *model.AppError) {
_args := &Z_GetBotIconImageArgs{botUserId}
_returns := &Z_GetBotIconImageReturns{}
if err := g.client.Call("Plugin.GetBotIconImage", _args, _returns); err != nil {
log.Printf("RPC call to GetBotIconImage API failed: %s", err.Error())
}
return _returns.A, _returns.B
}
func (s *apiRPCServer) GetBotIconImage(args *Z_GetBotIconImageArgs, returns *Z_GetBotIconImageReturns) error {
if hook, ok := s.impl.(interface {
GetBotIconImage(botUserId string) ([]byte, *model.AppError)
}); ok {
returns.A, returns.B = hook.GetBotIconImage(args.A)
} else {
return encodableError(fmt.Errorf("API GetBotIconImage called but not implemented."))
}
return nil
}
type Z_SetBotIconImageArgs struct {
A string
B []byte
}
type Z_SetBotIconImageReturns struct {
A *model.AppError
}
func (g *apiRPCClient) SetBotIconImage(botUserId string, data []byte) *model.AppError {
_args := &Z_SetBotIconImageArgs{botUserId, data}
_returns := &Z_SetBotIconImageReturns{}
if err := g.client.Call("Plugin.SetBotIconImage", _args, _returns); err != nil {
log.Printf("RPC call to SetBotIconImage API failed: %s", err.Error())
}
return _returns.A
}
func (s *apiRPCServer) SetBotIconImage(args *Z_SetBotIconImageArgs, returns *Z_SetBotIconImageReturns) error {
if hook, ok := s.impl.(interface {
SetBotIconImage(botUserId string, data []byte) *model.AppError
}); ok {
returns.A = hook.SetBotIconImage(args.A, args.B)
} else {
return encodableError(fmt.Errorf("API SetBotIconImage called but not implemented."))
}
return nil
}
type Z_DeleteBotIconImageArgs struct {
A string
}
type Z_DeleteBotIconImageReturns struct {
A *model.AppError
}
func (g *apiRPCClient) DeleteBotIconImage(botUserId string) *model.AppError {
_args := &Z_DeleteBotIconImageArgs{botUserId}
_returns := &Z_DeleteBotIconImageReturns{}
if err := g.client.Call("Plugin.DeleteBotIconImage", _args, _returns); err != nil {
log.Printf("RPC call to DeleteBotIconImage API failed: %s", err.Error())
}
return _returns.A
}
func (s *apiRPCServer) DeleteBotIconImage(args *Z_DeleteBotIconImageArgs, returns *Z_DeleteBotIconImageReturns) error {
if hook, ok := s.impl.(interface {
DeleteBotIconImage(botUserId string) *model.AppError
}); ok {
returns.A = hook.DeleteBotIconImage(args.A)
} else {
return encodableError(fmt.Errorf("API DeleteBotIconImage called but not implemented."))
}
return nil
}
+57
View File
@@ -262,6 +262,22 @@ func (_m *API) CreateUser(user *model.User) (*model.User, *model.AppError) {
return r0, r1
}
// DeleteBotIconImage provides a mock function with given fields: botUserId
func (_m *API) DeleteBotIconImage(botUserId string) *model.AppError {
ret := _m.Called(botUserId)
var r0 *model.AppError
if rf, ok := ret.Get(0).(func(string) *model.AppError); ok {
r0 = rf(botUserId)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.AppError)
}
}
return r0
}
// DeleteChannel provides a mock function with given fields: channelId
func (_m *API) DeleteChannel(channelId string) *model.AppError {
ret := _m.Called(channelId)
@@ -420,6 +436,31 @@ func (_m *API) GetBot(botUserId string, includeDeleted bool) (*model.Bot, *model
return r0, r1
}
// GetBotIconImage provides a mock function with given fields: botUserId
func (_m *API) GetBotIconImage(botUserId string) ([]byte, *model.AppError) {
ret := _m.Called(botUserId)
var r0 []byte
if rf, ok := ret.Get(0).(func(string) []byte); ok {
r0 = rf(botUserId)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]byte)
}
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
r1 = rf(botUserId)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
}
return r0, r1
}
// GetBots provides a mock function with given fields: options
func (_m *API) GetBots(options *model.BotGetOptions) ([]*model.Bot, *model.AppError) {
ret := _m.Called(options)
@@ -2358,6 +2399,22 @@ func (_m *API) SendMail(to string, subject string, htmlBody string) *model.AppEr
return r0
}
// SetBotIconImage provides a mock function with given fields: botUserId, data
func (_m *API) SetBotIconImage(botUserId string, data []byte) *model.AppError {
ret := _m.Called(botUserId, data)
var r0 *model.AppError
if rf, ok := ret.Get(0).(func(string, []byte) *model.AppError); ok {
r0 = rf(botUserId, data)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.AppError)
}
}
return r0
}
// SetProfileImage provides a mock function with given fields: userId, data
func (_m *API) SetProfileImage(userId string, data []byte) *model.AppError {
ret := _m.Called(userId, data)