From 58363d3455c9c8f5694fa7053a870508abbd775a Mon Sep 17 00:00:00 2001 From: Jesper Hansen Date: Wed, 29 May 2019 20:32:54 +0200 Subject: [PATCH] [MM-15194] Store: Migrate Emoji.Save to Sync as default #10642 (#10955) * Store: Migrate Emoji.Save to Sync as default #10642 * Store: use require.Nil helper function to assert nil --- app/emoji.go | 8 ++++---- app/helper_test.go | 8 ++++---- app/import_functions.go | 4 ++-- store/sqlstore/emoji_store.go | 21 +++++++++---------- store/store.go | 2 +- store/storetest/emoji_store.go | 31 +++++++++++++++++++---------- store/storetest/mocks/EmojiStore.go | 19 +++++++++++++----- 7 files changed, 55 insertions(+), 38 deletions(-) diff --git a/app/emoji.go b/app/emoji.go index 85d7030b8b..99015b3270 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -67,15 +67,15 @@ func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartIma return nil, err } - result := <-a.Srv.Store.Emoji().Save(emoji) - if result.Err != nil { - return nil, result.Err + emoji, err := a.Srv.Store.Emoji().Save(emoji) + if err != nil { + return nil, err } message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_EMOJI_ADDED, "", "", "", nil) message.Add("emoji", emoji.ToJson()) a.Publish(message) - return result.Data.(*model.Emoji), nil + return emoji, nil } func (a *App) GetEmojiList(page, perPage int, sort string) ([]*model.Emoji, *model.AppError) { diff --git a/app/helper_test.go b/app/helper_test.go index 4a24388ea5..5d5aa02872 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -387,17 +387,17 @@ func (me *TestHelper) CreateGroup() *model.Group { func (me *TestHelper) CreateEmoji() *model.Emoji { utils.DisableDebugLogForTest() - result := <-me.App.Srv.Store.Emoji().Save(&model.Emoji{ + emoji, err := me.App.Srv.Store.Emoji().Save(&model.Emoji{ CreatorId: me.BasicUser.Id, Name: model.NewRandomString(10), }) - if result.Err != nil { - panic(result.Err) + if err != nil { + panic(err) } utils.EnableDebugLogForTest() - return result.Data.(*model.Emoji) + return emoji } func (me *TestHelper) AddReactionToPost(post *model.Post, user *model.User, emojiName string) *model.Reaction { diff --git a/app/import_functions.go b/app/import_functions.go index 5401b435c7..0b59c41441 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1314,8 +1314,8 @@ func (a *App) ImportEmoji(data *EmojiImportData, dryRun bool) *model.AppError { } if !alreadyExists { - if result := <-a.Srv.Store.Emoji().Save(emoji); result.Err != nil { - return result.Err + if _, err := a.Srv.Store.Emoji().Save(emoji); err != nil { + return err } } diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index 4470a3fb8a..4309a4c9bd 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -51,19 +51,16 @@ func (es SqlEmojiStore) CreateIndexesIfNotExists() { es.CreateIndexIfNotExists("idx_emoji_name", "Emoji", "Name") } -func (es SqlEmojiStore) Save(emoji *model.Emoji) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - emoji.PreSave() - if result.Err = emoji.IsValid(); result.Err != nil { - return - } +func (es SqlEmojiStore) Save(emoji *model.Emoji) (*model.Emoji, *model.AppError) { + emoji.PreSave() + if err := emoji.IsValid(); err != nil { + return nil, err + } - if err := es.GetMaster().Insert(emoji); err != nil { - result.Err = model.NewAppError("SqlEmojiStore.Save", "store.sql_emoji.save.app_error", nil, "id="+emoji.Id+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = emoji - } - }) + 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) { diff --git a/store/store.go b/store/store.go index 7474d3b685..c0c190b0a0 100644 --- a/store/store.go +++ b/store/store.go @@ -454,7 +454,7 @@ type TokenStore interface { } type EmojiStore interface { - Save(emoji *model.Emoji) StoreChannel + Save(emoji *model.Emoji) (*model.Emoji, *model.AppError) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) GetByName(name string) StoreChannel GetMultipleByName(names []string) StoreChannel diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index 6e3a034188..c018df3456 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -11,6 +11,7 @@ import ( "github.com/mattermost/mattermost-server/store" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestEmojiStore(t *testing.T, ss store.Store) { @@ -28,8 +29,8 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) { Name: model.NewId(), } - if result := <-ss.Emoji().Save(emoji1); result.Err != nil { - t.Fatal(result.Err) + if _, err := ss.Emoji().Save(emoji1); err != nil { + t.Fatal(err) } if len(emoji1.Id) != 26 { @@ -40,7 +41,7 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) { CreatorId: model.NewId(), Name: emoji1.Name, } - if result := <-ss.Emoji().Save(&emoji2); result.Err == nil { + if _, err := ss.Emoji().Save(&emoji2); err == nil { t.Fatal("shouldn't be able to save emoji with duplicate name") } @@ -48,8 +49,8 @@ func testEmojiSaveDelete(t *testing.T, ss store.Store) { t.Fatal(result.Err) } - if result := <-ss.Emoji().Save(&emoji2); result.Err != nil { - t.Fatal("should be able to save emoji with duplicate name now that original has been deleted", result.Err) + if _, err := ss.Emoji().Save(&emoji2); err != nil { + t.Fatal("should be able to save emoji with duplicate name now that original has been deleted", err) } if result := <-ss.Emoji().Delete(emoji2.Id, time.Now().Unix()+1); result.Err != nil { @@ -74,7 +75,9 @@ func testEmojiGet(t *testing.T, ss store.Store) { } for i, emoji := range emojis { - emojis[i] = *store.Must(ss.Emoji().Save(&emoji)).(*model.Emoji) + data, err := ss.Emoji().Save(&emoji) + require.Nil(t, err) + emojis[i] = *data } defer func() { for _, emoji := range emojis { @@ -118,7 +121,9 @@ func testEmojiGetByName(t *testing.T, ss store.Store) { } for i, emoji := range emojis { - emojis[i] = *store.Must(ss.Emoji().Save(&emoji)).(*model.Emoji) + data, err := ss.Emoji().Save(&emoji) + require.Nil(t, err) + emojis[i] = *data } defer func() { for _, emoji := range emojis { @@ -150,7 +155,9 @@ func testEmojiGetMultipleByName(t *testing.T, ss store.Store) { } for i, emoji := range emojis { - emojis[i] = *store.Must(ss.Emoji().Save(&emoji)).(*model.Emoji) + data, err := ss.Emoji().Save(&emoji) + require.Nil(t, err) + emojis[i] = *data } defer func() { for _, emoji := range emojis { @@ -208,7 +215,9 @@ func testEmojiGetList(t *testing.T, ss store.Store) { } for i, emoji := range emojis { - emojis[i] = *store.Must(ss.Emoji().Save(&emoji)).(*model.Emoji) + data, err := ss.Emoji().Save(&emoji) + require.Nil(t, err) + emojis[i] = *data } defer func() { for _, emoji := range emojis { @@ -273,7 +282,9 @@ func testEmojiSearch(t *testing.T, ss store.Store) { } for i, emoji := range emojis { - emojis[i] = *store.Must(ss.Emoji().Save(&emoji)).(*model.Emoji) + data, err := ss.Emoji().Save(&emoji) + require.Nil(t, err) + emojis[i] = *data } defer func() { for _, emoji := range emojis { diff --git a/store/storetest/mocks/EmojiStore.go b/store/storetest/mocks/EmojiStore.go index 973d821902..75f9b9b6c2 100644 --- a/store/storetest/mocks/EmojiStore.go +++ b/store/storetest/mocks/EmojiStore.go @@ -103,19 +103,28 @@ func (_m *EmojiStore) GetMultipleByName(names []string) store.StoreChannel { } // Save provides a mock function with given fields: emoji -func (_m *EmojiStore) Save(emoji *model.Emoji) store.StoreChannel { +func (_m *EmojiStore) Save(emoji *model.Emoji) (*model.Emoji, *model.AppError) { ret := _m.Called(emoji) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.Emoji) store.StoreChannel); ok { + var r0 *model.Emoji + if rf, ok := ret.Get(0).(func(*model.Emoji) *model.Emoji); ok { r0 = rf(emoji) } 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(*model.Emoji) *model.AppError); ok { + r1 = rf(emoji) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // Search provides a mock function with given fields: name, prefixOnly, limit