From 6299af0fa54a9cd658e0d7fb1e5552746830cebf Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Thu, 5 Jul 2018 09:08:49 -0400 Subject: Add ability to bulk import emoji (#9048) * Add ability to bulk import emoji * Improve error handling * Update test config --- app/import.go | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++ app/import_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++- i18n/en.json | 16 +++++++++++ model/emoji.go | 8 ++++-- model/emoji_test.go | 5 ---- 5 files changed, 178 insertions(+), 8 deletions(-) diff --git a/app/import.go b/app/import.go index baf936567..12353d562 100644 --- a/app/import.go +++ b/app/import.go @@ -33,6 +33,7 @@ type LineImportData struct { Post *PostImportData `json:"post"` DirectChannel *DirectChannelImportData `json:"direct_channel"` DirectPost *DirectPostImportData `json:"direct_post"` + Emoji *EmojiImportData `json:"emoji"` Version *int `json:"version"` } @@ -114,6 +115,11 @@ type UserChannelNotifyPropsImportData struct { MarkUnread *string `json:"mark_unread"` } +type EmojiImportData struct { + Name *string `json:"name"` + Image *string `json:"image"` +} + type ReactionImportData struct { User *string `json:"user"` CreateAt *int64 `json:"create_at"` @@ -337,6 +343,12 @@ func (a *App) ImportLine(line LineImportData, dryRun bool) *model.AppError { } else { return a.ImportDirectPost(line.DirectPost, dryRun) } + case line.Type == "emoji": + if line.Emoji == nil { + return model.NewAppError("BulkImport", "app.import.import_line.null_emoji.error", nil, "", http.StatusBadRequest) + } else { + return a.ImportEmoji(line.Emoji, dryRun) + } default: return model.NewAppError("BulkImport", "app.import.import_line.unknown_line_type.error", map[string]interface{}{"Type": line.Type}, "", http.StatusBadRequest) } @@ -1925,6 +1937,71 @@ func validateDirectPostImportData(data *DirectPostImportData, maxPostSize int) * return nil } +func (a *App) ImportEmoji(data *EmojiImportData, dryRun bool) *model.AppError { + if err := validateEmojiImportData(data); err != nil { + return err + } + + // If this is a Dry Run, do not continue any further. + if dryRun { + return nil + } + + var emoji *model.Emoji + var err *model.AppError + + emoji, err = a.GetEmojiByName(*data.Name) + if err != nil && err.StatusCode != http.StatusNotFound { + return err + } + + alreadyExists := emoji != nil + + if !alreadyExists { + emoji = &model.Emoji{ + Name: *data.Name, + } + emoji.PreSave() + } + + file, fileErr := os.Open(*data.Image) + if fileErr != nil { + return model.NewAppError("BulkImport", "app.import.emoji.bad_file.error", map[string]interface{}{"EmojiName": *data.Name}, "", http.StatusBadRequest) + } + + if _, err := a.WriteFile(file, getEmojiImagePath(emoji.Id)); err != nil { + return err + } + + if !alreadyExists { + if result := <-a.Srv.Store.Emoji().Save(emoji); result.Err != nil { + return result.Err + } + } + + return nil +} + +func validateEmojiImportData(data *EmojiImportData) *model.AppError { + if data == nil { + return model.NewAppError("BulkImport", "app.import.validate_emoji_import_data.empty.error", nil, "", http.StatusBadRequest) + } + + if data.Name == nil || len(*data.Name) == 0 { + return model.NewAppError("BulkImport", "app.import.validate_emoji_import_data.name_missing.error", nil, "", http.StatusBadRequest) + } + + if err := model.IsValidEmojiName(*data.Name); err != nil { + return err + } + + if data.Image == nil || len(*data.Image) == 0 { + return model.NewAppError("BulkImport", "app.import.validate_emoji_import_data.image_missing.error", nil, "", http.StatusBadRequest) + } + + return nil +} + // // -- Old SlackImport Functions -- // Import functions are sutible for entering posts and users into the database without diff --git a/app/import_test.go b/app/import_test.go index e7bc055a4..8a88937f9 100644 --- a/app/import_test.go +++ b/app/import_test.go @@ -3774,11 +3774,16 @@ func TestImportBulkImport(t *testing.T) { th := Setup() defer th.TearDown() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCustomEmoji = true }) + teamName := model.NewId() channelName := model.NewId() username := model.NewId() username2 := model.NewId() username3 := model.NewId() + emojiName := model.NewId() + testsDir, _ := utils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") // Run bulk import with a valid 1 of everything. data1 := `{"type": "version", "version": 1} @@ -3791,7 +3796,8 @@ func TestImportBulkImport(t *testing.T) { {"type": "direct_channel", "direct_channel": {"members": ["` + username + `", "` + username2 + `"]}} {"type": "direct_channel", "direct_channel": {"members": ["` + username + `", "` + username2 + `", "` + username3 + `"]}} {"type": "direct_post", "direct_post": {"channel_members": ["` + username + `", "` + username2 + `"], "user": "` + username + `", "message": "Hello Direct Channel", "create_at": 123456789013}} -{"type": "direct_post", "direct_post": {"channel_members": ["` + username + `", "` + username2 + `", "` + username3 + `"], "user": "` + username + `", "message": "Hello Group Channel", "create_at": 123456789014}}` +{"type": "direct_post", "direct_post": {"channel_members": ["` + username + `", "` + username2 + `", "` + username3 + `"], "user": "` + username + `", "message": "Hello Group Channel", "create_at": 123456789014}} +{"type": "emoji", "emoji": {"name": "` + emojiName + `", "image": "` + testImage + `"}}` if err, line := th.App.BulkImport(strings.NewReader(data1), false, 2); err != nil || line != 0 { t.Fatalf("BulkImport should have succeeded: %v, %v", err.Error(), line) @@ -3833,3 +3839,75 @@ func TestImportProcessImportDataFileVersionLine(t *testing.T) { t.Fatalf("Expected error on invalid version line.") } } + +func TestImportValidateEmojiImportData(t *testing.T) { + data := EmojiImportData{ + Name: ptrStr("parrot"), + Image: ptrStr("/path/to/image"), + } + + err := validateEmojiImportData(&data) + assert.Nil(t, err, "Validation should succeed") + + *data.Name = "smiley" + err = validateEmojiImportData(&data) + assert.NotNil(t, err) + + *data.Name = "" + err = validateEmojiImportData(&data) + assert.NotNil(t, err) + + *data.Name = "" + *data.Image = "" + err = validateEmojiImportData(&data) + assert.NotNil(t, err) + + *data.Image = "/path/to/image" + data.Name = nil + err = validateEmojiImportData(&data) + assert.NotNil(t, err) + + data.Name = ptrStr("parrot") + data.Image = nil + err = validateEmojiImportData(&data) + assert.NotNil(t, err) +} + +func TestImportImportEmoji(t *testing.T) { + th := Setup() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCustomEmoji = true }) + + testsDir, _ := utils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + + data := EmojiImportData{Name: ptrStr(model.NewId())} + err := th.App.ImportEmoji(&data, true) + assert.NotNil(t, err, "Invalid emoji should have failed dry run") + + result := <-th.App.Srv.Store.Emoji().GetByName(*data.Name) + assert.Nil(t, result.Data, "Emoji should not have been imported") + + data.Image = ptrStr(testImage) + err = th.App.ImportEmoji(&data, true) + assert.Nil(t, err, "Valid emoji should have passed dry run") + + data = EmojiImportData{Name: ptrStr(model.NewId())} + err = th.App.ImportEmoji(&data, false) + assert.NotNil(t, err, "Invalid emoji should have failed apply mode") + + data.Image = ptrStr("non-existent-file") + err = th.App.ImportEmoji(&data, false) + assert.NotNil(t, err, "Emoji with bad image file should have failed apply mode") + + data.Image = ptrStr(testImage) + err = th.App.ImportEmoji(&data, false) + assert.Nil(t, err, "Valid emoji should have succeeded apply mode") + + result = <-th.App.Srv.Store.Emoji().GetByName(*data.Name) + assert.NotNil(t, result.Data, "Emoji should have been imported") + + err = th.App.ImportEmoji(&data, false) + assert.Nil(t, err, "Second run should have succeeded apply mode") +} diff --git a/i18n/en.json b/i18n/en.json index 36bf8772b..58ed704a8 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2362,6 +2362,22 @@ "id": "app.cluster.404.app_error", "translation": "Cluster API endpoint not found." }, + { + "id": "app.import.emoji.bad_file.error", + "translation": "Error reading import emoji image file. Emoji with name: \"{{.EmojiName}}\"" + }, + { + "id": "app.import.validate_emoji_import_data.empty.error", + "translation": "Import emoji data empty." + }, + { + "id": "app.import.validate_emoji_import_data.name_missing.error", + "translation": "Import emoji name field missing or blank." + }, + { + "id": "app.import.validate_emoji_import_data.image_missing.error", + "translation": "Import emoji image field missing or blank." + }, { "id": "app.import.bulk_import.file_scan.error", "translation": "Error reading import data file." diff --git a/model/emoji.go b/model/emoji.go index 78a266386..f14af89df 100644 --- a/model/emoji.go +++ b/model/emoji.go @@ -41,11 +41,15 @@ func (emoji *Emoji) IsValid() *AppError { return NewAppError("Emoji.IsValid", "model.emoji.update_at.app_error", nil, "id="+emoji.Id, http.StatusBadRequest) } - if len(emoji.CreatorId) != 26 { + if len(emoji.CreatorId) > 26 { return NewAppError("Emoji.IsValid", "model.emoji.user_id.app_error", nil, "", http.StatusBadRequest) } - if len(emoji.Name) == 0 || len(emoji.Name) > EMOJI_NAME_MAX_LENGTH || !IsValidAlphaNumHyphenUnderscore(emoji.Name, false) || inSystemEmoji(emoji.Name) { + return IsValidEmojiName(emoji.Name) +} + +func IsValidEmojiName(name string) *AppError { + if len(name) == 0 || len(name) > EMOJI_NAME_MAX_LENGTH || !IsValidAlphaNumHyphenUnderscore(name, false) || inSystemEmoji(name) { return NewAppError("Emoji.IsValid", "model.emoji.name.app_error", nil, "", http.StatusBadRequest) } diff --git a/model/emoji_test.go b/model/emoji_test.go index 95abe37c6..50d741214 100644 --- a/model/emoji_test.go +++ b/model/emoji_test.go @@ -40,11 +40,6 @@ func TestEmojiIsValid(t *testing.T) { } emoji.UpdateAt = 1234 - emoji.CreatorId = strings.Repeat("1", 25) - if err := emoji.IsValid(); err == nil { - t.Fatal() - } - emoji.CreatorId = strings.Repeat("1", 27) if err := emoji.IsValid(); err == nil { t.Fatal() -- cgit v1.2.3-1-g7c22