From 17092e7e48e0c478b171d1f47c969b6e3149e653 Mon Sep 17 00:00:00 2001 From: Fede Date: Thu, 18 Apr 2019 14:03:59 +0200 Subject: [PATCH] [MM-15117] Migrate "WebHook.UpdateIncoming" to Sync by default (#10637) * SyncStore: Migrate Webhooks.UpdateIncoming method to Sync * Changes requested by @jespino --- app/webhook.go | 10 +++++----- cmd/mattermost/commands/channel.go | 2 +- store/sqlstore/webhook_store.go | 15 ++++++--------- store/store.go | 2 +- store/storetest/mocks/WebhookStore.go | 19 ++++++++++++++----- store/storetest/webhook_store.go | 18 +++++++++--------- 6 files changed, 36 insertions(+), 30 deletions(-) diff --git a/app/webhook.go b/app/webhook.go index 3938fd1f6c..d0c102b7d8 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -353,12 +353,12 @@ func (a *App) UpdateIncomingWebhook(oldHook, updatedHook *model.IncomingWebhook) updatedHook.TeamId = oldHook.TeamId updatedHook.DeleteAt = oldHook.DeleteAt - if result := <-a.Srv.Store.Webhook().UpdateIncoming(updatedHook); result.Err != nil { - return nil, result.Err - } else { - a.InvalidateCacheForWebhook(oldHook.Id) - return result.Data.(*model.IncomingWebhook), nil + newWebhook, err := a.Srv.Store.Webhook().UpdateIncoming(updatedHook) + if err != nil { + return nil, err } + a.InvalidateCacheForWebhook(oldHook.Id) + return newWebhook, nil } func (a *App) DeleteIncomingWebhook(hookId string) *model.AppError { diff --git a/cmd/mattermost/commands/channel.go b/cmd/mattermost/commands/channel.go index dc47a3b000..e9961ea3bf 100644 --- a/cmd/mattermost/commands/channel.go +++ b/cmd/mattermost/commands/channel.go @@ -408,7 +408,7 @@ func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *mod for _, webhook := range incomingWebhooks { if webhook.ChannelId == channel.Id { webhook.TeamId = team.Id - if result := <-a.Srv.Store.Webhook().UpdateIncoming(webhook); result.Err != nil { + if _, err := a.Srv.Store.Webhook().UpdateIncoming(webhook); err != nil { CommandPrintErrorln("Failed to move incoming webhook '" + webhook.Id + "' to new team.") } } diff --git a/store/sqlstore/webhook_store.go b/store/sqlstore/webhook_store.go index c4ad26eb12..7fad2a7b53 100644 --- a/store/sqlstore/webhook_store.go +++ b/store/sqlstore/webhook_store.go @@ -109,16 +109,13 @@ func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) store.Stor }) } -func (s SqlWebhookStore) UpdateIncoming(hook *model.IncomingWebhook) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - hook.UpdateAt = model.GetMillis() +func (s SqlWebhookStore) UpdateIncoming(hook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) { + hook.UpdateAt = model.GetMillis() - if _, err := s.GetMaster().Update(hook); err != nil { - result.Err = model.NewAppError("SqlWebhookStore.UpdateIncoming", "store.sql_webhooks.update_incoming.app_error", nil, "id="+hook.Id+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = hook - } - }) + if _, err := s.GetMaster().Update(hook); err != nil { + return nil, model.NewAppError("SqlWebhookStore.UpdateIncoming", "store.sql_webhooks.update_incoming.app_error", nil, "id="+hook.Id+", "+err.Error(), http.StatusInternalServerError) + } + return hook, nil } func (s SqlWebhookStore) GetIncoming(id string, allowFromCache bool) (*model.IncomingWebhook, *model.AppError) { diff --git a/store/store.go b/store/store.go index 73555371b8..13b2499c16 100644 --- a/store/store.go +++ b/store/store.go @@ -381,7 +381,7 @@ type WebhookStore interface { GetIncoming(id string, allowFromCache bool) (*model.IncomingWebhook, *model.AppError) GetIncomingList(offset, limit int) StoreChannel GetIncomingByTeam(teamId string, offset, limit int) StoreChannel - UpdateIncoming(webhook *model.IncomingWebhook) StoreChannel + UpdateIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) GetIncomingByChannel(channelId string) StoreChannel DeleteIncoming(webhookId string, time int64) StoreChannel PermanentDeleteIncomingByChannel(channelId string) StoreChannel diff --git a/store/storetest/mocks/WebhookStore.go b/store/storetest/mocks/WebhookStore.go index 0be4836fd3..72e8d44fd9 100644 --- a/store/storetest/mocks/WebhookStore.go +++ b/store/storetest/mocks/WebhookStore.go @@ -321,19 +321,28 @@ func (_m *WebhookStore) SaveOutgoing(webhook *model.OutgoingWebhook) store.Store } // UpdateIncoming provides a mock function with given fields: webhook -func (_m *WebhookStore) UpdateIncoming(webhook *model.IncomingWebhook) store.StoreChannel { +func (_m *WebhookStore) UpdateIncoming(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 } // UpdateOutgoing provides a mock function with given fields: hook diff --git a/store/storetest/webhook_store.go b/store/storetest/webhook_store.go index 486a389290..ec49f52416 100644 --- a/store/storetest/webhook_store.go +++ b/store/storetest/webhook_store.go @@ -56,17 +56,17 @@ func testWebhookStoreUpdateIncoming(t *testing.T, ss store.Store) { o1.DisplayName = "TestHook" time.Sleep(10 * time.Millisecond) - if result := (<-ss.Webhook().UpdateIncoming(o1)); result.Err != nil { - t.Fatal("updation of incoming hook failed", result.Err) - } else { - if result.Data.(*model.IncomingWebhook).UpdateAt == previousUpdatedAt { - t.Fatal("should have updated the UpdatedAt of the hook") - } + webhook, err := ss.Webhook().UpdateIncoming(o1) + require.Nil(t, err) - if result.Data.(*model.IncomingWebhook).DisplayName != "TestHook" { - t.Fatal("display name is not updated") - } + if webhook.UpdateAt == previousUpdatedAt { + t.Fatal("should have updated the UpdatedAt of the hook") } + + if webhook.DisplayName != "TestHook" { + t.Fatal("display name is not updated") + } + } func testWebhookStoreGetIncoming(t *testing.T, ss store.Store) {