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

@@ -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 a.Srv.Store.Webhook().SaveIncoming(hook)
-		return nil, result.Err
-	} else {
-		return result.Data.(*model.IncomingWebhook), nil
-	}
 }
 
 func (a *App) UpdateIncomingWebhook(oldHook, updatedHook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) {

store/sqlstore/webhook_store.go

@@ -88,24 +88,23 @@ func (s SqlWebhookStore) InvalidateWebhookCache(webhookId string) {
 	}
 }
 
-func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) store.StoreChannel {
+func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) {
-	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
-		}
 
-		webhook.PreSave()
+	if len(webhook.Id) > 0 {
-		if result.Err = webhook.IsValid(); result.Err != nil {
+		return nil, model.NewAppError("SqlWebhookStore.SaveIncoming", "store.sql_webhooks.save_incoming.existing.app_error", nil, "id="+webhook.Id, http.StatusBadRequest)
-			return
+	}
-		}
+
+	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) {

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)

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
+	var r0 *model.IncomingWebhook
-	if rf, ok := ret.Get(0).(func(*model.IncomingWebhook) store.StoreChannel); ok {
+	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

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)