diff --git a/app/emoji.go b/app/emoji.go index 5550d15c93..e1a0ecf83b 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -14,12 +14,14 @@ import ( "io" "mime/multipart" "net/http" + "path" "image/color/palette" "github.com/disintegration/imaging" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" ) const ( @@ -53,7 +55,7 @@ func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartIma return nil, model.NewAppError("createEmoji", "api.emoji.create.other_user.app_error", nil, "", http.StatusForbidden) } - if existingEmoji, err := a.Srv.Store.Emoji().GetByName(emoji.Name); err == nil && existingEmoji != nil { + if existingEmoji, err := a.Srv.Store.Emoji().GetByName(emoji.Name, true); err == nil && existingEmoji != nil { return nil, model.NewAppError("createEmoji", "api.emoji.create.duplicate.app_error", nil, "", http.StatusBadRequest) } @@ -156,7 +158,7 @@ func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *mode } func (a *App) DeleteEmoji(emoji *model.Emoji) *model.AppError { - if err := a.Srv.Store.Emoji().Delete(emoji.Id, model.GetMillis()); err != nil { + if err := a.Srv.Store.Emoji().Delete(emoji, model.GetMillis()); err != nil { return err } @@ -186,7 +188,7 @@ func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - return a.Srv.Store.Emoji().GetByName(emojiName) + return a.Srv.Store.Emoji().GetByName(emojiName, true) } func (a *App) GetMultipleEmojiByName(names []string) ([]*model.Emoji, *model.AppError) { @@ -224,6 +226,22 @@ func (a *App) SearchEmoji(name string, prefixOnly bool, limit int) ([]*model.Emo return a.Srv.Store.Emoji().Search(name, prefixOnly, limit) } +// GetEmojiStaticUrl returns a relative static URL for system default emojis, +// and the API route for custom ones. Errors if not found or if custom and deleted. +func (a *App) GetEmojiStaticUrl(emojiName string) (string, *model.AppError) { + subPath, _ := utils.GetSubpathFromConfig(a.Config()) + + if id, found := model.GetSystemEmojiId(emojiName); found { + return path.Join(subPath, "/static/emoji", id+".png"), nil + } + + if emoji, err := a.Srv.Store.Emoji().GetByName(emojiName, true); err == nil { + return path.Join(subPath, "/api/v4/emoji", emoji.Id, "image"), nil + } else { + return "", err + } +} + func resizeEmojiGif(gifImg *gif.GIF) *gif.GIF { // Create a new RGBA image to hold the incremental frames. firstFrame := gifImg.Image[0].Bounds() diff --git a/app/import_functions.go b/app/import_functions.go index 6f1752305d..00e3f2a422 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1287,7 +1287,7 @@ func (a *App) ImportEmoji(data *EmojiImportData, dryRun bool) *model.AppError { var emoji *model.Emoji - emoji, appError := a.Srv.Store.Emoji().GetByName(*data.Name) + emoji, appError := a.Srv.Store.Emoji().GetByName(*data.Name, true) if appError != nil && appError.StatusCode != http.StatusNotFound { return appError } diff --git a/app/import_functions_test.go b/app/import_functions_test.go index 2a5fdea019..0103c76195 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -2647,7 +2647,7 @@ func TestImportImportEmoji(t *testing.T) { err := th.App.ImportEmoji(&data, true) assert.NotNil(t, err, "Invalid emoji should have failed dry run") - emoji, err := th.App.Srv.Store.Emoji().GetByName(*data.Name) + emoji, err := th.App.Srv.Store.Emoji().GetByName(*data.Name, true) assert.Nil(t, emoji, "Emoji should not have been imported") assert.NotNil(t, err) @@ -2667,7 +2667,7 @@ func TestImportImportEmoji(t *testing.T) { err = th.App.ImportEmoji(&data, false) assert.Nil(t, err, "Valid emoji should have succeeded apply mode") - emoji, err = th.App.Srv.Store.Emoji().GetByName(*data.Name) + emoji, err = th.App.Srv.Store.Emoji().GetByName(*data.Name, true) assert.NotNil(t, emoji, "Emoji should have been imported") assert.Nil(t, err, "Emoji should have been imported without any error") diff --git a/app/post_metadata.go b/app/post_metadata.go index 94d44e266d..bef2d0ae18 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -55,12 +55,37 @@ func (a *App) PreparePostListForClient(originalList *model.PostList) *model.Post return list } +// OverrideIconURLIfEmoji changes the post icon override URL prop, if it has an emoji icon, +// so that it points to the URL (relative) of the emoji - static if emoji is default, /api if custom. +func (a *App) OverrideIconURLIfEmoji(post *model.Post) { + prop, ok := post.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + if !ok || prop == nil { + return + } + emojiName := prop.(string) + delete(post.Props, model.POST_PROPS_OVERRIDE_ICON_EMOJI) + + if !*a.Config().ServiceSettings.EnablePostIconOverride || emojiName == "" { + return + } + + if emojiUrl, err := a.GetEmojiStaticUrl(emojiName); err == nil { + post.AddProp(model.POST_PROPS_OVERRIDE_ICON_URL, emojiUrl) + } else { + mlog.Warn("Failed to retrieve URL for overriden profile icon (emoji)", mlog.String("emojiName", emojiName), mlog.Err(err)) + } + + return +} + func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isEditPost bool) *model.Post { post := originalPost.Clone() // Proxy image links before constructing metadata so that requests go through the proxy post = a.PostWithProxyAddedToImageURLs(post) + a.OverrideIconURLIfEmoji(post) + post.Metadata = &model.PostMetadata{} // Emojis and reaction counts diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index f14ed1e159..6177fa2f6d 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -235,6 +235,49 @@ func TestPreparePostForClient(t *testing.T) { }) }) + t.Run("emojis overriding profile icon", func(t *testing.T) { + th := setup() + defer th.TearDown() + + prepare := func(override bool, url, emoji string) *model.Post { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnablePostIconOverride = override + }) + + post, err := th.App.CreatePost(&model.Post{ + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Test", + }, th.BasicChannel, false) + + require.Nil(t, err) + + post.AddProp(model.POST_PROPS_OVERRIDE_ICON_URL, url) + post.AddProp(model.POST_PROPS_OVERRIDE_ICON_EMOJI, emoji) + + return th.App.PreparePostForClient(post, false, false) + } + + emoji := "basketball" + url := "http://host.com/image.png" + overridenUrl := "/static/emoji/1f3c0.png" + + t.Run("does not override icon URL", func(t *testing.T) { + clientPost := prepare(false, url, emoji) + + s, _ := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + assert.EqualValues(t, url, s) + }) + + t.Run("overrides icon URL", func(t *testing.T) { + clientPost := prepare(true, url, emoji) + + s, _ := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + assert.EqualValues(t, overridenUrl, s) + }) + + }) + t.Run("markdown image dimensions", func(t *testing.T) { th := setup() defer th.TearDown() diff --git a/app/webhook.go b/app/webhook.go index cab582fa5a..8129fa7b11 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -138,7 +138,7 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model. if *a.Config().ServiceSettings.EnablePostIconOverride && hook.IconURL != "" && webhookResp.IconURL == "" { webhookResp.IconURL = hook.IconURL } - if _, err := a.CreateWebhookPost(hook.CreatorId, channel, text, webhookResp.Username, webhookResp.IconURL, webhookResp.Props, webhookResp.Type, postRootId); err != nil { + if _, err := a.CreateWebhookPost(hook.CreatorId, channel, text, webhookResp.Username, webhookResp.IconURL, "", webhookResp.Props, webhookResp.Type, postRootId); err != nil { mlog.Error(fmt.Sprintf("Failed to create response post, err=%v", err)) } } @@ -245,7 +245,7 @@ func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. return splits, nil } -func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, overrideUsername, overrideIconUrl string, props model.StringInterface, postType string, postRootId string) (*model.Post, *model.AppError) { +func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, overrideUsername, overrideIconUrl, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string) (*model.Post, *model.AppError) { // parse links into Markdown format linkWithTextRegex := regexp.MustCompile(`<([^\n<\|>]+)\|([^\n>]+)>`) text = linkWithTextRegex.ReplaceAllString(text, "[${2}](${1})") @@ -274,6 +274,9 @@ func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, ove if len(overrideIconUrl) != 0 { post.AddProp("override_icon_url", overrideIconUrl) } + if len(overrideIconEmoji) != 0 { + post.AddProp("override_icon_emoji", overrideIconEmoji) + } } if len(props) > 0 { @@ -660,7 +663,7 @@ func (a *App) HandleIncomingWebhook(hookId string, req *model.IncomingWebhookReq overrideIconUrl = req.IconURL } - _, err := a.CreateWebhookPost(hook.UserId, channel, text, overrideUsername, overrideIconUrl, req.Props, webhookType, "") + _, err := a.CreateWebhookPost(hook.UserId, channel, text, overrideUsername, overrideIconUrl, req.IconEmoji, req.Props, webhookType, "") return err } diff --git a/app/webhook_test.go b/app/webhook_test.go index fe86eebc59..1f354ddeb2 100644 --- a/app/webhook_test.go +++ b/app/webhook_test.go @@ -300,7 +300,7 @@ func TestCreateWebhookPost(t *testing.T) { } defer th.App.DeleteIncomingWebhook(hook.Id) - post, err := th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", model.StringInterface{ + post, err := th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", model.StringInterface{ "attachments": []*model.SlackAttachment{ { Text: "text", @@ -319,13 +319,13 @@ func TestCreateWebhookPost(t *testing.T) { } } - _, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", nil, model.POST_SYSTEM_GENERIC, "") + _, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", nil, model.POST_SYSTEM_GENERIC, "") if err == nil { t.Fatal("should have failed - bad post type") } expectedText := "`<>|<>|`" - post, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, expectedText, "user", "http://iconurl", model.StringInterface{ + post, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, expectedText, "user", "http://iconurl", "", model.StringInterface{ "attachments": []*model.SlackAttachment{ { Text: "text", @@ -339,7 +339,7 @@ func TestCreateWebhookPost(t *testing.T) { assert.Equal(t, expectedText, post.Message) expectedText = "< | \n|\n>" - post, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, expectedText, "user", "http://iconurl", model.StringInterface{ + post, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, expectedText, "user", "http://iconurl", "", model.StringInterface{ "attachments": []*model.SlackAttachment{ { Text: "text", @@ -369,7 +369,7 @@ Date: Thu Mar 1 19:46:48 2018 +0300 test | 3 +++ 1 file changed, 3 insertions(+)` - post, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, expectedText, "user", "http://iconurl", model.StringInterface{ + post, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, expectedText, "user", "http://iconurl", "", model.StringInterface{ "attachments": []*model.SlackAttachment{ { Text: "text", diff --git a/model/emoji.go b/model/emoji.go index afe61493d7..c2ef25afa1 100644 --- a/model/emoji.go +++ b/model/emoji.go @@ -31,6 +31,11 @@ func inSystemEmoji(emojiName string) bool { return ok } +func GetSystemEmojiId(emojiName string) (string, bool) { + id, found := SystemEmojis[emojiName] + return id, found +} + func (emoji *Emoji) IsValid() *AppError { if len(emoji.Id) != 26 { return NewAppError("Emoji.IsValid", "model.emoji.id.app_error", nil, "", http.StatusBadRequest) diff --git a/model/incoming_webhook.go b/model/incoming_webhook.go index 3856d22ff3..8806a73f6a 100644 --- a/model/incoming_webhook.go +++ b/model/incoming_webhook.go @@ -38,6 +38,7 @@ type IncomingWebhookRequest struct { Props StringInterface `json:"props"` Attachments []*SlackAttachment `json:"attachments"` Type string `json:"type"` + IconEmoji string `json:"icon_emoji"` } func (o *IncomingWebhook) ToJson() string { diff --git a/model/post.go b/model/post.go index f0ee28728d..727296915d 100644 --- a/model/post.go +++ b/model/post.go @@ -50,8 +50,11 @@ const ( POST_CUSTOM_TYPE_PREFIX = "custom_" POST_ME = "me" PROPS_ADD_CHANNEL_MEMBER = "add_channel_member" - POST_PROPS_ADDED_USER_ID = "addedUserId" - POST_PROPS_DELETE_BY = "deleteBy" + + POST_PROPS_ADDED_USER_ID = "addedUserId" + POST_PROPS_DELETE_BY = "deleteBy" + POST_PROPS_OVERRIDE_ICON_URL = "override_icon_url" + POST_PROPS_OVERRIDE_ICON_EMOJI = "override_icon_emoji" ) type Post struct { diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index 67791734b0..8489454068 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -19,7 +19,8 @@ const ( EMOJI_CACHE_SEC = 1800 // 30 mins ) -var emojiCache *utils.Cache = utils.NewLru(EMOJI_CACHE_SIZE) +var emojiCacheById = utils.NewLru(EMOJI_CACHE_SIZE) +var emojiIdCacheByName = utils.NewLru(EMOJI_CACHE_SIZE) type SqlEmojiStore struct { SqlStore @@ -60,67 +61,32 @@ func (es SqlEmojiStore) Save(emoji *model.Emoji) (*model.Emoji, *model.AppError) if err := es.GetMaster().Insert(emoji); err != nil { return nil, model.NewAppError("SqlEmojiStore.Save", "store.sql_emoji.save.app_error", nil, "id="+emoji.Id+", "+err.Error(), http.StatusInternalServerError) } + return emoji, nil } func (es SqlEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) { if allowFromCache { - if cacheItem, ok := emojiCache.Get(id); ok { - if es.metrics != nil { - es.metrics.IncrementMemCacheHitCounter("Emoji") - } - return cacheItem.(*model.Emoji), nil - } - if es.metrics != nil { - es.metrics.IncrementMemCacheMissCounter("Emoji") - } - } else { - if es.metrics != nil { - es.metrics.IncrementMemCacheMissCounter("Emoji") + if emoji, ok := es.getFromCacheById(id); ok { + return emoji, nil } } - var emoji *model.Emoji + return es.getBy("Id", id, allowFromCache) +} - if err := es.GetReplica().SelectOne(&emoji, - `SELECT - * - FROM - Emoji - WHERE - Id = :Id - AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil { - return nil, model.NewAppError("SqlEmojiStore.Get", "store.sql_emoji.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound) +func (es SqlEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) { + if id, ok := model.GetSystemEmojiId(name); ok { + return es.Get(id, allowFromCache) } if allowFromCache { - emojiCache.AddWithExpiresInSecs(id, emoji, EMOJI_CACHE_SEC) - } - - return emoji, nil -} - -func (es SqlEmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) { - - var emoji *model.Emoji - - if err := es.GetReplica().SelectOne(&emoji, - `SELECT - * - FROM - Emoji - WHERE - Name = :Name - AND DeleteAt = 0`, map[string]interface{}{"Name": name}); err != nil { - - if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusNotFound) + if emoji, ok := es.getFromCacheByName(name); ok { + return emoji, nil } - - return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusInternalServerError) } - return emoji, nil + return es.getBy("Name", name, allowFromCache) } func (es SqlEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, *model.AppError) { @@ -158,7 +124,7 @@ func (es SqlEmojiStore) GetList(offset, limit int, sort string) ([]*model.Emoji, return emoji, nil } -func (es SqlEmojiStore) Delete(id string, time int64) *model.AppError { +func (es SqlEmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError { if sqlResult, err := es.GetMaster().Exec( `UPDATE Emoji @@ -167,13 +133,14 @@ func (es SqlEmojiStore) Delete(id string, time int64) *model.AppError { UpdateAt = :UpdateAt WHERE Id = :Id - AND DeleteAt = 0`, map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": id}); err != nil { - return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.app_error", nil, "id="+id+", err="+err.Error(), http.StatusInternalServerError) + AND DeleteAt = 0`, map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": emoji.Id}); err != nil { + return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.app_error", nil, "id="+emoji.Id+", err="+err.Error(), http.StatusInternalServerError) } else if rows, _ := sqlResult.RowsAffected(); rows == 0 { - return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+id+", err="+err.Error(), http.StatusBadRequest) + return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+emoji.Id+", err="+err.Error(), http.StatusBadRequest) } - emojiCache.Remove(id) + es.removeFromCache(emoji) + return nil } @@ -201,3 +168,74 @@ func (es SqlEmojiStore) Search(name string, prefixOnly bool, limit int) ([]*mode } return emojis, nil } + +// getBy returns one active (not deleted) emoji, found by any one column (what/key). +func (es SqlEmojiStore) getBy(what string, key interface{}, addToCache bool) (*model.Emoji, *model.AppError) { + var emoji *model.Emoji + + err := es.GetReplica().SelectOne(&emoji, + `SELECT + * + FROM + Emoji + WHERE + `+what+` = :Key + AND DeleteAt = 0`, map[string]interface{}{"Key": key}) + if err != nil { + var status int + if err == sql.ErrNoRows { + status = http.StatusNotFound + } else { + status = http.StatusInternalServerError + } + return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get.app_error", nil, "key="+fmt.Sprintf("%v", key)+", "+err.Error(), status) + } + + if addToCache { + es.addToCache(emoji) + } + + return emoji, nil +} + +func (es SqlEmojiStore) addToCache(emoji *model.Emoji) { + emojiCacheById.AddWithExpiresInSecs(emoji.Id, emoji, EMOJI_CACHE_SEC) + emojiIdCacheByName.AddWithExpiresInSecs(emoji.Name, emoji.Id, EMOJI_CACHE_SEC) +} + +func (es SqlEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) { + if cacheItem, ok := emojiCacheById.Get(id); ok { + es.incrementMemCacheHitCounter("Emoji") + return cacheItem.(*model.Emoji), true + } + es.incrementMemCacheMissCounter("Emoji") + return nil, false +} + +func (es SqlEmojiStore) getFromCacheByName(name string) (*model.Emoji, bool) { + if id, ok := emojiIdCacheByName.Get(name); ok { + return es.getFromCacheById(id.(string)) + } + + es.incrementMemCacheMissCounter("Emoji") + return nil, false +} + +func (es SqlEmojiStore) incrementMemCacheHitCounter(cache string) { + if es.metrics == nil { + return + } + es.metrics.IncrementMemCacheHitCounter(cache) +} + +func (es SqlEmojiStore) incrementMemCacheMissCounter(cache string) { + if es.metrics == nil { + return + } + es.metrics.IncrementMemCacheMissCounter(cache) +} + +func (es SqlEmojiStore) removeFromCache(emoji *model.Emoji) { + emojiCacheById.Remove(emoji.Id) + emojiIdCacheByName.Remove(emoji.Name) +} diff --git a/store/store.go b/store/store.go index 21f5ff95fe..39ca5c5b5c 100644 --- a/store/store.go +++ b/store/store.go @@ -463,10 +463,10 @@ type TokenStore interface { type EmojiStore interface { Save(emoji *model.Emoji) (*model.Emoji, *model.AppError) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) - GetByName(name string) (*model.Emoji, *model.AppError) + GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) GetMultipleByName(names []string) ([]*model.Emoji, *model.AppError) GetList(offset, limit int, sort string) ([]*model.Emoji, *model.AppError) - Delete(id string, time int64) *model.AppError + Delete(emoji *model.Emoji, time int64) *model.AppError Search(name string, prefixOnly bool, limit int) ([]*model.Emoji, *model.AppError) } diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index 072d5eed89..c0adfd0867 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -21,6 +21,7 @@ func TestEmojiStore(t *testing.T, ss store.Store) { t.Run("EmojiGetMultipleByName", func(t *testing.T) { testEmojiGetMultipleByName(t, ss) }) t.Run("EmojiGetList", func(t *testing.T) { testEmojiGetList(t, ss) }) t.Run("EmojiSearch", func(t *testing.T) { testEmojiSearch(t, ss) }) + t.Run("EmojiCaching", func(t *testing.T) { testEmojiCaching(t, ss) }) } func testEmojiSaveDelete(t *testing.T, ss store.Store) { @@ -45,7 +46,7 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) { t.Fatal("shouldn't be able to save emoji with duplicate name") } - if err := ss.Emoji().Delete(emoji1.Id, time.Now().Unix()); err != nil { + if err := ss.Emoji().Delete(emoji1, time.Now().Unix()); err != nil { t.Fatal(err) } @@ -53,7 +54,7 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) { t.Fatal("should be able to save emoji with duplicate name now that original has been deleted", err) } - if err := ss.Emoji().Delete(emoji2.Id, time.Now().Unix()+1); err != nil { + if err := ss.Emoji().Delete(&emoji2, time.Now().Unix()+1); err != nil { t.Fatal(err) } } @@ -81,7 +82,7 @@ func testEmojiGet(t *testing.T, ss store.Store) { } defer func() { for _, emoji := range emojis { - err := ss.Emoji().Delete(emoji.Id, time.Now().Unix()) + err := ss.Emoji().Delete(&emoji, time.Now().Unix()) require.Nil(t, err) } }() @@ -97,12 +98,61 @@ func testEmojiGet(t *testing.T, ss store.Store) { t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err) } } +} - for _, emoji := range emojis { - if _, err := ss.Emoji().Get(emoji.Id, true); err != nil { - t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err) +func testEmojiCaching(t *testing.T, ss store.Store) { + emojis := make([]*model.Emoji, 3) + for i := range emojis { + emojis[i] = &model.Emoji{ + CreatorId: model.NewId(), + Name: model.NewId(), } } + + for _, emoji := range emojis { + _, err := ss.Emoji().Save(emoji) + require.Nil(t, err) + } + defer func() { + for _, emoji := range emojis { + err := ss.Emoji().Delete(emoji, time.Now().Unix()) + require.Nil(t, err) + } + }() + + var retrievedEmoji *model.Emoji + var cachedEmoji *model.Emoji + var err *model.AppError + + for _, emoji := range emojis { + cachedEmoji, err = ss.Emoji().Get(emoji.Id, true) + assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) + + retrievedEmoji, err = ss.Emoji().Get(emoji.Id, false) + if assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) { + assert.Falsef(t, retrievedEmoji == cachedEmoji, "should not be the same as cached with id %v", emoji.Id) + } + + retrievedEmoji, err = ss.Emoji().Get(emoji.Id, true) + if assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) { + assert.Truef(t, retrievedEmoji == cachedEmoji, "should be the cached emoji with id %v", emoji.Id) + } + + retrievedEmoji, err = ss.Emoji().GetByName(emoji.Name, false) + if assert.Nilf(t, err, "should be able to retrieve emoji with name %v", emoji.Name) { + assert.Falsef(t, retrievedEmoji == cachedEmoji, "should not be the same as cached with name %v", emoji.Name) + } + + retrievedEmoji, _ = ss.Emoji().GetByName(emoji.Name, true) + if assert.Nilf(t, err, "should be able to retrieve emoji with name %v", emoji.Name) { + assert.Truef(t, retrievedEmoji == cachedEmoji, "should be the cached emoji with name %v", emoji.Name) + } + } + + _, err = ss.Emoji().Get(model.NewId(), false) + assert.NotNilf(t, err, "should not retrieve emoji with unsaved ID") + _, err = ss.Emoji().GetByName(model.NewId(), false) + assert.NotNilf(t, err, "should not retrieve emoji with unsaved name") } func testEmojiGetByName(t *testing.T, ss store.Store) { @@ -128,13 +178,13 @@ func testEmojiGetByName(t *testing.T, ss store.Store) { } defer func() { for _, emoji := range emojis { - err := ss.Emoji().Delete(emoji.Id, time.Now().Unix()) + err := ss.Emoji().Delete(&emoji, time.Now().Unix()) require.Nil(t, err) } }() for _, emoji := range emojis { - if _, err := ss.Emoji().GetByName(emoji.Name); err != nil { + if _, err := ss.Emoji().GetByName(emoji.Name, true); err != nil { t.Fatalf("failed to get emoji with name %v: %v", emoji.Name, err) } } @@ -163,7 +213,7 @@ func testEmojiGetMultipleByName(t *testing.T, ss store.Store) { } defer func() { for _, emoji := range emojis { - err := ss.Emoji().Delete(emoji.Id, time.Now().Unix()) + err := ss.Emoji().Delete(&emoji, time.Now().Unix()) require.Nil(t, err) } }() @@ -224,7 +274,7 @@ func testEmojiGetList(t *testing.T, ss store.Store) { } defer func() { for _, emoji := range emojis { - err := ss.Emoji().Delete(emoji.Id, time.Now().Unix()) + err := ss.Emoji().Delete(&emoji, time.Now().Unix()) require.Nil(t, err) } }() @@ -290,7 +340,7 @@ func testEmojiSearch(t *testing.T, ss store.Store) { } defer func() { for _, emoji := range emojis { - err := ss.Emoji().Delete(emoji.Id, time.Now().Unix()) + err := ss.Emoji().Delete(&emoji, time.Now().Unix()) require.Nil(t, err) } }() diff --git a/store/storetest/mocks/EmojiStore.go b/store/storetest/mocks/EmojiStore.go index 2ce84b0570..32cc6a49af 100644 --- a/store/storetest/mocks/EmojiStore.go +++ b/store/storetest/mocks/EmojiStore.go @@ -12,13 +12,13 @@ type EmojiStore struct { mock.Mock } -// Delete provides a mock function with given fields: id, time -func (_m *EmojiStore) Delete(id string, time int64) *model.AppError { - ret := _m.Called(id, time) +// Delete provides a mock function with given fields: emoji, time +func (_m *EmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError { + ret := _m.Called(emoji, time) var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string, int64) *model.AppError); ok { - r0 = rf(id, time) + if rf, ok := ret.Get(0).(func(*model.Emoji, int64) *model.AppError); ok { + r0 = rf(emoji, time) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.AppError) @@ -53,13 +53,13 @@ func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model. return r0, r1 } -// GetByName provides a mock function with given fields: name -func (_m *EmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) { - ret := _m.Called(name) +// GetByName provides a mock function with given fields: name, allowFromCache +func (_m *EmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) { + ret := _m.Called(name, allowFromCache) var r0 *model.Emoji - if rf, ok := ret.Get(0).(func(string) *model.Emoji); ok { - r0 = rf(name) + if rf, ok := ret.Get(0).(func(string, bool) *model.Emoji); ok { + r0 = rf(name, allowFromCache) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Emoji) @@ -67,8 +67,8 @@ func (_m *EmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) { } var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { - r1 = rf(name) + if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok { + r1 = rf(name, allowFromCache) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError)