From 105e8647f89ae8514ebbf1ba3052b1c9dba53669 Mon Sep 17 00:00:00 2001 From: Andres Orozco Date: Wed, 24 Apr 2019 04:30:41 -0400 Subject: [PATCH] MM-15116 Migrate WebHook.SaveIncoming Method to Sync by default (#10663) * SyncStore: migrate WebHooks.SaveIncoming method to Sync * MM-15116 Migrates the WebHook.SaveIncoming method to Sync by default * MM-15116 Migrate Webhook.SaveIncoming to Sync by default - fix minor typo * MM-15116 Migrate WebHook.SaveIncoming to sync - update test case --- app/webhook.go | 6 +-- store/sqlstore/webhook_store.go | 31 +++++++------ store/store.go | 2 +- store/storetest/mocks/WebhookStore.go | 19 +++++--- store/storetest/webhook_store.go | 63 ++++++++++++++++++++------- 5 files changed, 79 insertions(+), 42 deletions(-) diff --git a/app/webhook.go b/app/webhook.go index fbd9bc8fe5..893f665db5 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -323,11 +323,7 @@ func (a *App) CreateIncomingWebhookForChannel(creatorId string, channel *model.C return nil, model.NewAppError("CreateIncomingWebhookForChannel", "api.incoming_webhook.invalid_username.app_error", nil, "", http.StatusBadRequest) } - if result := <-a.Srv.Store.Webhook().SaveIncoming(hook); result.Err != nil { - return nil, result.Err - } else { - return result.Data.(*model.IncomingWebhook), nil - } + return a.Srv.Store.Webhook().SaveIncoming(hook) } func (a *App) UpdateIncomingWebhook(oldHook, updatedHook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) { diff --git a/store/sqlstore/webhook_store.go b/store/sqlstore/webhook_store.go index b7081ab7e3..b06c5360f1 100644 --- a/store/sqlstore/webhook_store.go +++ b/store/sqlstore/webhook_store.go @@ -88,24 +88,23 @@ func (s SqlWebhookStore) InvalidateWebhookCache(webhookId string) { } } -func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if len(webhook.Id) > 0 { - result.Err = model.NewAppError("SqlWebhookStore.SaveIncoming", "store.sql_webhooks.save_incoming.existing.app_error", nil, "id="+webhook.Id, http.StatusBadRequest) - return - } +func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) { - webhook.PreSave() - if result.Err = webhook.IsValid(); result.Err != nil { - return - } + if len(webhook.Id) > 0 { + return nil, model.NewAppError("SqlWebhookStore.SaveIncoming", "store.sql_webhooks.save_incoming.existing.app_error", nil, "id="+webhook.Id, http.StatusBadRequest) + } + + webhook.PreSave() + if err := webhook.IsValid(); err != nil { + return nil, err + } + + if err := s.GetMaster().Insert(webhook); err != nil { + return nil, model.NewAppError("SqlWebhookStore.SaveIncoming", "store.sql_webhooks.save_incoming.app_error", nil, "id="+webhook.Id+", "+err.Error(), http.StatusInternalServerError) + } + + return webhook, nil - if err := s.GetMaster().Insert(webhook); err != nil { - result.Err = model.NewAppError("SqlWebhookStore.SaveIncoming", "store.sql_webhooks.save_incoming.app_error", nil, "id="+webhook.Id+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = webhook - } - }) } func (s SqlWebhookStore) UpdateIncoming(hook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) { diff --git a/store/store.go b/store/store.go index 60c10f0f9e..392d08ecc4 100644 --- a/store/store.go +++ b/store/store.go @@ -377,7 +377,7 @@ type SystemStore interface { } type WebhookStore interface { - SaveIncoming(webhook *model.IncomingWebhook) StoreChannel + SaveIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) GetIncoming(id string, allowFromCache bool) (*model.IncomingWebhook, *model.AppError) GetIncomingList(offset, limit int) StoreChannel GetIncomingByTeam(teamId string, offset, limit int) ([]*model.IncomingWebhook, *model.AppError) diff --git a/store/storetest/mocks/WebhookStore.go b/store/storetest/mocks/WebhookStore.go index c9ebfb492a..c2c88fec25 100644 --- a/store/storetest/mocks/WebhookStore.go +++ b/store/storetest/mocks/WebhookStore.go @@ -307,19 +307,28 @@ func (_m *WebhookStore) PermanentDeleteOutgoingByUser(userId string) store.Store } // SaveIncoming provides a mock function with given fields: webhook -func (_m *WebhookStore) SaveIncoming(webhook *model.IncomingWebhook) store.StoreChannel { +func (_m *WebhookStore) SaveIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) { ret := _m.Called(webhook) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.IncomingWebhook) store.StoreChannel); ok { + var r0 *model.IncomingWebhook + if rf, ok := ret.Get(0).(func(*model.IncomingWebhook) *model.IncomingWebhook); ok { r0 = rf(webhook) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.IncomingWebhook) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.IncomingWebhook) *model.AppError); ok { + r1 = rf(webhook) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // SaveOutgoing provides a mock function with given fields: webhook diff --git a/store/storetest/webhook_store.go b/store/storetest/webhook_store.go index 1d6043bb01..21bf5d4386 100644 --- a/store/storetest/webhook_store.go +++ b/store/storetest/webhook_store.go @@ -38,18 +38,25 @@ func TestWebhookStore(t *testing.T, ss store.Store) { func testWebhookStoreSaveIncoming(t *testing.T, ss store.Store) { o1 := buildIncomingWebhook() - if err := (<-ss.Webhook().SaveIncoming(o1)).Err; err != nil { + if _, err := ss.Webhook().SaveIncoming(o1); err != nil { t.Fatal("couldn't save item", err) } - if err := (<-ss.Webhook().SaveIncoming(o1)).Err; err == nil { + if _, err := ss.Webhook().SaveIncoming(o1); err == nil { t.Fatal("shouldn't be able to update from save") } } func testWebhookStoreUpdateIncoming(t *testing.T, ss store.Store) { + + var err *model.AppError + o1 := buildIncomingWebhook() - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1, err = ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook", err) + } + previousUpdatedAt := o1.UpdateAt o1.DisplayName = "TestHook" @@ -69,8 +76,13 @@ func testWebhookStoreUpdateIncoming(t *testing.T, ss store.Store) { } func testWebhookStoreGetIncoming(t *testing.T, ss store.Store) { + var err *model.AppError + o1 := buildIncomingWebhook() - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1, err = ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook", err) + } webhook, err := ss.Webhook().GetIncoming(o1.Id, false) require.Nil(t, err) @@ -103,7 +115,11 @@ func testWebhookStoreGetIncomingList(t *testing.T, ss store.Store) { o1.UserId = model.NewId() o1.TeamId = model.NewId() - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + var err *model.AppError + o1, err = ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook", err) + } if r1 := <-ss.Webhook().GetIncomingList(0, 1000); r1.Err != nil { t.Fatal(r1.Err) @@ -130,9 +146,11 @@ func testWebhookStoreGetIncomingList(t *testing.T, ss store.Store) { } func testWebhookStoreGetIncomingByTeam(t *testing.T, ss store.Store) { - o1 := buildIncomingWebhook() + var err *model.AppError - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1 := buildIncomingWebhook() + o1, err = ss.Webhook().SaveIncoming(o1) + require.Nil(t, err) if hooks, err := ss.Webhook().GetIncomingByTeam(o1.TeamId, 0, 100); err != nil { t.Fatal(err) @@ -154,7 +172,10 @@ func testWebhookStoreGetIncomingByTeam(t *testing.T, ss store.Store) { func testWebhookStoreGetIncomingByChannel(t *testing.T, ss store.Store) { o1 := buildIncomingWebhook() - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1, err := ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook") + } webhooks, err := ss.Webhook().GetIncomingByChannel(o1.ChannelId) require.Nil(t, err) @@ -172,9 +193,13 @@ func testWebhookStoreGetIncomingByChannel(t *testing.T, ss store.Store) { } func testWebhookStoreDeleteIncoming(t *testing.T, ss store.Store) { - o1 := buildIncomingWebhook() + var err *model.AppError - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1 := buildIncomingWebhook() + o1, err = ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook", err) + } webhook, err := ss.Webhook().GetIncoming(o1.Id, true) require.Nil(t, err) @@ -191,9 +216,13 @@ func testWebhookStoreDeleteIncoming(t *testing.T, ss store.Store) { } func testWebhookStoreDeleteIncomingByChannel(t *testing.T, ss store.Store) { - o1 := buildIncomingWebhook() + var err *model.AppError - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1 := buildIncomingWebhook() + o1, err = ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook", err) + } webhook, err := ss.Webhook().GetIncoming(o1.Id, true) require.Nil(t, err) @@ -211,9 +240,13 @@ func testWebhookStoreDeleteIncomingByChannel(t *testing.T, ss store.Store) { } func testWebhookStoreDeleteIncomingByUser(t *testing.T, ss store.Store) { - o1 := buildIncomingWebhook() + var err *model.AppError - o1 = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + o1 := buildIncomingWebhook() + o1, err = ss.Webhook().SaveIncoming(o1) + if err != nil { + t.Fatal("unable to save webhook", err) + } webhook, err := ss.Webhook().GetIncoming(o1.Id, true) require.Nil(t, err) @@ -490,7 +523,7 @@ func testWebhookStoreCountIncoming(t *testing.T, ss store.Store) { o1.UserId = model.NewId() o1.TeamId = model.NewId() - _ = (<-ss.Webhook().SaveIncoming(o1)).Data.(*model.IncomingWebhook) + _, _ = ss.Webhook().SaveIncoming(o1) if r := <-ss.Webhook().AnalyticsIncomingCount(""); r.Err != nil { t.Fatal(r.Err)