From eaef1952b0ecf7f2361bb3fe317fc08a7bd7ce03 Mon Sep 17 00:00:00 2001 From: Siyuan Liu Date: Tue, 23 Apr 2019 01:39:59 -0700 Subject: [PATCH] [MM-15183] Migrate "WebHook.SaveOutgoing" to Sync (#10675) --- app/webhook.go | 9 +++++---- store/sqlstore/webhook_store.go | 29 ++++++++++++--------------- store/store.go | 2 +- store/storetest/mocks/WebhookStore.go | 19 +++++++++++++----- store/storetest/webhook_store.go | 24 +++++++++++----------- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/app/webhook.go b/app/webhook.go index bebe7338b2..fbd9bc8fe5 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -444,11 +444,12 @@ func (a *App) CreateOutgoingWebhook(hook *model.OutgoingWebhook) (*model.Outgoin } } - if result := <-a.Srv.Store.Webhook().SaveOutgoing(hook); result.Err != nil { - return nil, result.Err - } else { - return result.Data.(*model.OutgoingWebhook), nil + webhook, err := a.Srv.Store.Webhook().SaveOutgoing(hook) + if err != nil { + return nil, err } + + return webhook, nil } func (a *App) UpdateOutgoingWebhook(oldHook, updatedHook *model.OutgoingWebhook) (*model.OutgoingWebhook, *model.AppError) { diff --git a/store/sqlstore/webhook_store.go b/store/sqlstore/webhook_store.go index 7db4ecb33b..cf826df881 100644 --- a/store/sqlstore/webhook_store.go +++ b/store/sqlstore/webhook_store.go @@ -208,24 +208,21 @@ func (s SqlWebhookStore) GetIncomingByChannel(channelId string) ([]*model.Incomi return webhooks, nil } -func (s SqlWebhookStore) SaveOutgoing(webhook *model.OutgoingWebhook) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if len(webhook.Id) > 0 { - result.Err = model.NewAppError("SqlWebhookStore.SaveOutgoing", "store.sql_webhooks.save_outgoing.override.app_error", nil, "id="+webhook.Id, http.StatusBadRequest) - return - } +func (s SqlWebhookStore) SaveOutgoing(webhook *model.OutgoingWebhook) (*model.OutgoingWebhook, *model.AppError) { + if len(webhook.Id) > 0 { + return nil, model.NewAppError("SqlWebhookStore.SaveOutgoing", "store.sql_webhooks.save_outgoing.override.app_error", nil, "id="+webhook.Id, http.StatusBadRequest) + } - webhook.PreSave() - if result.Err = webhook.IsValid(); result.Err != nil { - return - } + webhook.PreSave() + if err := webhook.IsValid(); err != nil { + return nil, err + } - if err := s.GetMaster().Insert(webhook); err != nil { - result.Err = model.NewAppError("SqlWebhookStore.SaveOutgoing", "store.sql_webhooks.save_outgoing.app_error", nil, "id="+webhook.Id+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = webhook - } - }) + if err := s.GetMaster().Insert(webhook); err != nil { + return nil, model.NewAppError("SqlWebhookStore.SaveOutgoing", "store.sql_webhooks.save_outgoing.app_error", nil, "id="+webhook.Id+", "+err.Error(), http.StatusInternalServerError) + } + + return webhook, nil } func (s SqlWebhookStore) GetOutgoing(id string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 0c2e302a60..8d03cccb90 100644 --- a/store/store.go +++ b/store/store.go @@ -387,7 +387,7 @@ type WebhookStore interface { PermanentDeleteIncomingByChannel(channelId string) StoreChannel PermanentDeleteIncomingByUser(userId string) StoreChannel - SaveOutgoing(webhook *model.OutgoingWebhook) StoreChannel + SaveOutgoing(webhook *model.OutgoingWebhook) (*model.OutgoingWebhook, *model.AppError) GetOutgoing(id string) StoreChannel GetOutgoingList(offset, limit int) StoreChannel GetOutgoingByChannel(channelId string, offset, limit int) StoreChannel diff --git a/store/storetest/mocks/WebhookStore.go b/store/storetest/mocks/WebhookStore.go index 9e0821193d..5753d8db4e 100644 --- a/store/storetest/mocks/WebhookStore.go +++ b/store/storetest/mocks/WebhookStore.go @@ -323,19 +323,28 @@ func (_m *WebhookStore) SaveIncoming(webhook *model.IncomingWebhook) store.Store } // SaveOutgoing provides a mock function with given fields: webhook -func (_m *WebhookStore) SaveOutgoing(webhook *model.OutgoingWebhook) store.StoreChannel { +func (_m *WebhookStore) SaveOutgoing(webhook *model.OutgoingWebhook) (*model.OutgoingWebhook, *model.AppError) { ret := _m.Called(webhook) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.OutgoingWebhook) store.StoreChannel); ok { + var r0 *model.OutgoingWebhook + if rf, ok := ret.Get(0).(func(*model.OutgoingWebhook) *model.OutgoingWebhook); ok { r0 = rf(webhook) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.OutgoingWebhook) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.OutgoingWebhook) *model.AppError); ok { + r1 = rf(webhook) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // UpdateIncoming provides a mock function with given fields: webhook diff --git a/store/storetest/webhook_store.go b/store/storetest/webhook_store.go index f80c6b968e..fec1bda826 100644 --- a/store/storetest/webhook_store.go +++ b/store/storetest/webhook_store.go @@ -248,11 +248,11 @@ func testWebhookStoreSaveOutgoing(t *testing.T, ss store.Store) { o1.Username = "test-user-name" o1.IconURL = "http://nowhere.com/icon" - if err := (<-ss.Webhook().SaveOutgoing(&o1)).Err; err != nil { + if _, err := ss.Webhook().SaveOutgoing(&o1); err != nil { t.Fatal("couldn't save item", err) } - if err := (<-ss.Webhook().SaveOutgoing(&o1)).Err; err == nil { + if _, err := ss.Webhook().SaveOutgoing(&o1); err == nil { t.Fatal("shouldn't be able to update from save") } } @@ -266,7 +266,7 @@ func testWebhookStoreGetOutgoing(t *testing.T, ss store.Store) { o1.Username = "test-user-name" o1.IconURL = "http://nowhere.com/icon" - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) if r1 := <-ss.Webhook().GetOutgoing(o1.Id); r1.Err != nil { t.Fatal(r1.Err) @@ -288,7 +288,7 @@ func testWebhookStoreGetOutgoingList(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) o2 := &model.OutgoingWebhook{} o2.ChannelId = model.NewId() @@ -296,7 +296,7 @@ func testWebhookStoreGetOutgoingList(t *testing.T, ss store.Store) { o2.TeamId = model.NewId() o2.CallbackURLs = []string{"http://nowhere.com/"} - o2 = (<-ss.Webhook().SaveOutgoing(o2)).Data.(*model.OutgoingWebhook) + o2, _ = ss.Webhook().SaveOutgoing(o2) if r1 := <-ss.Webhook().GetOutgoingList(0, 1000); r1.Err != nil { t.Fatal(r1.Err) @@ -339,7 +339,7 @@ func testWebhookStoreGetOutgoingByChannel(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) if r1 := <-ss.Webhook().GetOutgoingByChannel(o1.ChannelId, 0, 100); r1.Err != nil { t.Fatal(r1.Err) @@ -365,7 +365,7 @@ func testWebhookStoreGetOutgoingByTeam(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) if r1 := <-ss.Webhook().GetOutgoingByTeam(o1.TeamId, 0, 100); r1.Err != nil { t.Fatal(r1.Err) @@ -391,7 +391,7 @@ func testWebhookStoreDeleteOutgoing(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) if r1 := <-ss.Webhook().GetOutgoing(o1.Id); r1.Err != nil { t.Fatal(r1.Err) @@ -418,7 +418,7 @@ func testWebhookStoreDeleteOutgoingByChannel(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) if r1 := <-ss.Webhook().GetOutgoing(o1.Id); r1.Err != nil { t.Fatal(r1.Err) @@ -445,7 +445,7 @@ func testWebhookStoreDeleteOutgoingByUser(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) if r1 := <-ss.Webhook().GetOutgoing(o1.Id); r1.Err != nil { t.Fatal(r1.Err) @@ -474,7 +474,7 @@ func testWebhookStoreUpdateOutgoing(t *testing.T, ss store.Store) { o1.Username = "test-user-name" o1.IconURL = "http://nowhere.com/icon" - o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + o1, _ = ss.Webhook().SaveOutgoing(o1) o1.Token = model.NewId() o1.Username = "another-test-user-name" @@ -508,7 +508,7 @@ func testWebhookStoreCountOutgoing(t *testing.T, ss store.Store) { o1.TeamId = model.NewId() o1.CallbackURLs = []string{"http://nowhere.com/"} - _ = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook) + ss.Webhook().SaveOutgoing(o1) if r := <-ss.Webhook().AnalyticsOutgoingCount(""); r.Err != nil { t.Fatal(r.Err)