From 53d0bfe35e0683430303825450366817ca9a2d65 Mon Sep 17 00:00:00 2001 From: Shobhit Gupta Date: Fri, 10 May 2019 07:57:08 -0700 Subject: [PATCH] [MM-15193] Migrate "Emoji.Get" to Sync by default (#10801) * Change emoji.Get to sync * Make emojistore.get sync * Update mocks * Fix build --- app/emoji.go | 12 ++---- store/sqlstore/emoji_store.go | 66 ++++++++++++++--------------- store/store.go | 2 +- store/storetest/emoji_store.go | 12 +++--- store/storetest/mocks/EmojiStore.go | 19 ++++++--- 5 files changed, 56 insertions(+), 55 deletions(-) diff --git a/app/emoji.go b/app/emoji.go index e81ad109d9..85d7030b8b 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -178,11 +178,7 @@ func (a *App) GetEmoji(emojiId string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - result := <-a.Srv.Store.Emoji().Get(emojiId, false) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.Emoji), nil + return a.Srv.Store.Emoji().Get(emojiId, false) } func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { @@ -214,9 +210,9 @@ func (a *App) GetMultipleEmojiByName(names []string) ([]*model.Emoji, *model.App } func (a *App) GetEmojiImage(emojiId string) ([]byte, string, *model.AppError) { - result := <-a.Srv.Store.Emoji().Get(emojiId, true) - if result.Err != nil { - return nil, "", result.Err + _, storeErr := a.Srv.Store.Emoji().Get(emojiId, true) + if storeErr != nil { + return nil, "", storeErr } img, appErr := a.ReadFile(getEmojiImagePath(emojiId)) diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index b480c21649..4470a3fb8a 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -66,45 +66,41 @@ func (es SqlEmojiStore) Save(emoji *model.Emoji) store.StoreChannel { }) } -func (es SqlEmojiStore) Get(id string, allowFromCache bool) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if allowFromCache { - if cacheItem, ok := emojiCache.Get(id); ok { - if es.metrics != nil { - es.metrics.IncrementMemCacheHitCounter("Emoji") - } - result.Data = cacheItem.(*model.Emoji) - return - } else { - if es.metrics != nil { - es.metrics.IncrementMemCacheMissCounter("Emoji") - } - } - } else { +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.IncrementMemCacheMissCounter("Emoji") + es.metrics.IncrementMemCacheHitCounter("Emoji") } + return cacheItem.(*model.Emoji), nil } - - var emoji *model.Emoji - - if err := es.GetReplica().SelectOne(&emoji, - `SELECT - * - FROM - Emoji - WHERE - Id = :Id - AND DeleteAt = 0`, map[string]interface{}{"Id": id}); err != nil { - result.Err = model.NewAppError("SqlEmojiStore.Get", "store.sql_emoji.get.app_error", nil, "id="+id+", "+err.Error(), http.StatusNotFound) - } else { - result.Data = emoji - - if allowFromCache { - emojiCache.AddWithExpiresInSecs(id, emoji, EMOJI_CACHE_SEC) - } + if es.metrics != nil { + es.metrics.IncrementMemCacheMissCounter("Emoji") } - }) + } else { + if es.metrics != nil { + es.metrics.IncrementMemCacheMissCounter("Emoji") + } + } + + var emoji *model.Emoji + + 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) + } + + if allowFromCache { + emojiCache.AddWithExpiresInSecs(id, emoji, EMOJI_CACHE_SEC) + } + + return emoji, nil } func (es SqlEmojiStore) GetByName(name string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index f3f39f80d7..2dd2919d9f 100644 --- a/store/store.go +++ b/store/store.go @@ -456,7 +456,7 @@ type TokenStore interface { type EmojiStore interface { Save(emoji *model.Emoji) StoreChannel - Get(id string, allowFromCache bool) StoreChannel + Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) GetByName(name string) StoreChannel GetMultipleByName(names []string) StoreChannel GetList(offset, limit int, sort string) StoreChannel diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index 087bdbecbb..6e3a034188 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -83,20 +83,20 @@ func testEmojiGet(t *testing.T, ss store.Store) { }() for _, emoji := range emojis { - if result := <-ss.Emoji().Get(emoji.Id, false); result.Err != nil { - t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, result.Err) + if _, err := ss.Emoji().Get(emoji.Id, false); err != nil { + t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err) } } for _, emoji := range emojis { - if result := <-ss.Emoji().Get(emoji.Id, true); result.Err != nil { - t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, result.Err) + if _, err := ss.Emoji().Get(emoji.Id, true); err != nil { + t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err) } } for _, emoji := range emojis { - if result := <-ss.Emoji().Get(emoji.Id, true); result.Err != nil { - t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, result.Err) + if _, err := ss.Emoji().Get(emoji.Id, true); err != nil { + t.Fatalf("failed to get emoji with id %v: %v", emoji.Id, err) } } } diff --git a/store/storetest/mocks/EmojiStore.go b/store/storetest/mocks/EmojiStore.go index 80b12cfe69..973d821902 100644 --- a/store/storetest/mocks/EmojiStore.go +++ b/store/storetest/mocks/EmojiStore.go @@ -30,19 +30,28 @@ func (_m *EmojiStore) Delete(id string, time int64) store.StoreChannel { } // Get provides a mock function with given fields: id, allowFromCache -func (_m *EmojiStore) Get(id string, allowFromCache bool) store.StoreChannel { +func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) { ret := _m.Called(id, allowFromCache) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok { + var r0 *model.Emoji + if rf, ok := ret.Get(0).(func(string, bool) *model.Emoji); ok { r0 = rf(id, allowFromCache) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.Emoji) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok { + r1 = rf(id, allowFromCache) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetByName provides a mock function with given fields: name