From d300f4a6ad23c200f774892b23e33abde8d04d57 Mon Sep 17 00:00:00 2001 From: Pradeep Murugesan Date: Fri, 21 Jun 2019 12:19:57 +0100 Subject: [PATCH] made the emoji store getByName method sync (#11324) --- app/emoji.go | 8 ++----- app/import_functions.go | 10 +++----- app/import_functions_test.go | 10 ++++---- store/sqlstore/emoji_store.go | 37 +++++++++++++++-------------- store/store.go | 2 +- store/storetest/emoji_store.go | 4 ++-- store/storetest/mocks/EmojiStore.go | 19 +++++++++++---- 7 files changed, 47 insertions(+), 43 deletions(-) diff --git a/app/emoji.go b/app/emoji.go index a6681836f4..2e4e333f7a 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -53,7 +53,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 result := <-a.Srv.Store.Emoji().GetByName(emoji.Name); result.Err == nil && result.Data != nil { + if existingEmoji, err := a.Srv.Store.Emoji().GetByName(emoji.Name); err == nil && existingEmoji != nil { return nil, model.NewAppError("createEmoji", "api.emoji.create.duplicate.app_error", nil, "", http.StatusBadRequest) } @@ -190,11 +190,7 @@ func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - result := <-a.Srv.Store.Emoji().GetByName(emojiName) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.Emoji), nil + return a.Srv.Store.Emoji().GetByName(emojiName) } func (a *App) GetMultipleEmojiByName(names []string) ([]*model.Emoji, *model.AppError) { diff --git a/app/import_functions.go b/app/import_functions.go index c839ca514e..1ffa86a35f 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1280,13 +1280,9 @@ func (a *App) ImportEmoji(data *EmojiImportData, dryRun bool) *model.AppError { var emoji *model.Emoji - result := <-a.Srv.Store.Emoji().GetByName(*data.Name) - if result.Err != nil && result.Err.StatusCode != http.StatusNotFound { - return result.Err - } - - if result.Data != nil { - emoji = result.Data.(*model.Emoji) + emoji, appError := a.Srv.Store.Emoji().GetByName(*data.Name) + if appError != nil && appError.StatusCode != http.StatusNotFound { + return appError } alreadyExists := emoji != nil diff --git a/app/import_functions_test.go b/app/import_functions_test.go index fa877a1d4a..5c6d555f49 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -2681,8 +2681,9 @@ func TestImportImportEmoji(t *testing.T) { 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") + emoji, err := th.App.Srv.Store.Emoji().GetByName(*data.Name) + assert.Nil(t, emoji, "Emoji should not have been imported") + assert.NotNil(t, err) data.Image = ptrStr(testImage) err = th.App.ImportEmoji(&data, true) @@ -2700,8 +2701,9 @@ func TestImportImportEmoji(t *testing.T) { 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") + emoji, err = th.App.Srv.Store.Emoji().GetByName(*data.Name) + assert.NotNil(t, emoji, "Emoji should have been imported") + assert.Nil(t, err, "Emoji should have been imported without any error") err = th.App.ImportEmoji(&data, false) assert.Nil(t, err, "Second run should have succeeded apply mode") diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index 5fc97f46fc..10af2f7a6e 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -100,26 +100,27 @@ func (es SqlEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *mode return emoji, nil } -func (es SqlEmojiStore) GetByName(name string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var emoji *model.Emoji +func (es SqlEmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) { - if err := es.GetReplica().SelectOne(&emoji, - `SELECT - * - FROM - Emoji - WHERE - Name = :Name - AND DeleteAt = 0`, map[string]interface{}{"Name": name}); err != nil { - result.Err = model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusInternalServerError) - if err == sql.ErrNoRows { - result.Err.StatusCode = http.StatusNotFound - } - } else { - result.Data = emoji + 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) } - }) + + return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get_by_name.app_error", nil, "name="+name+", "+err.Error(), http.StatusInternalServerError) + } + + return emoji, nil } func (es SqlEmojiStore) GetMultipleByName(names []string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index cd922c77f8..f010964788 100644 --- a/store/store.go +++ b/store/store.go @@ -456,7 +456,7 @@ 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) StoreChannel + GetByName(name string) (*model.Emoji, *model.AppError) GetMultipleByName(names []string) StoreChannel GetList(offset, limit int, sort string) StoreChannel Delete(id string, time int64) *model.AppError diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index e6bba086a9..adcad43527 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -134,8 +134,8 @@ func testEmojiGetByName(t *testing.T, ss store.Store) { }() for _, emoji := range emojis { - if result := <-ss.Emoji().GetByName(emoji.Name); result.Err != nil { - t.Fatalf("failed to get emoji with name %v: %v", emoji.Name, result.Err) + if _, err := ss.Emoji().GetByName(emoji.Name); err != nil { + t.Fatalf("failed to get emoji with name %v: %v", emoji.Name, err) } } } diff --git a/store/storetest/mocks/EmojiStore.go b/store/storetest/mocks/EmojiStore.go index ace709fabe..736aa15f75 100644 --- a/store/storetest/mocks/EmojiStore.go +++ b/store/storetest/mocks/EmojiStore.go @@ -55,19 +55,28 @@ func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model. } // GetByName provides a mock function with given fields: name -func (_m *EmojiStore) GetByName(name string) store.StoreChannel { +func (_m *EmojiStore) GetByName(name string) (*model.Emoji, *model.AppError) { ret := _m.Called(name) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.Emoji + if rf, ok := ret.Get(0).(func(string) *model.Emoji); ok { r0 = rf(name) } 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) *model.AppError); ok { + r1 = rf(name) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetList provides a mock function with given fields: offset, limit, sort