MM-15287: Migrate commandstore.save to sync by default (#10738)

* MM-15287: Migrate commandstore.save to sync by default

* MM-15287: Minor change to follow code consistency

app/command.go

@@ -466,12 +466,7 @@ func (a *App) CreateCommand(cmd *model.Command) (*model.Command, *model.AppError
 		}
 	}
 
-	result := <-a.Srv.Store.Command().Save(cmd)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-
-	return result.Data.(*model.Command), nil
+	return a.Srv.Store.Command().Save(cmd)
 }
 
 func (a *App) GetCommand(commandId string) (*model.Command, *model.AppError) {

store/sqlstore/command_store.go

@@ -44,24 +44,21 @@ func (s SqlCommandStore) CreateIndexesIfNotExists() {
 	s.CreateIndexIfNotExists("idx_command_delete_at", "Commands", "DeleteAt")
 }
 
-func (s SqlCommandStore) Save(command *model.Command) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		if len(command.Id) > 0 {
-			result.Err = model.NewAppError("SqlCommandStore.Save", "store.sql_command.save.saving_overwrite.app_error", nil, "id="+command.Id, http.StatusBadRequest)
-			return
-		}
+func (s SqlCommandStore) Save(command *model.Command) (*model.Command, *model.AppError) {
+	if len(command.Id) > 0 {
+		return nil, model.NewAppError("SqlCommandStore.Save", "store.sql_command.save.saving_overwrite.app_error", nil, "id="+command.Id, http.StatusBadRequest)
+	}
 
-		command.PreSave()
-		if result.Err = command.IsValid(); result.Err != nil {
-			return
-		}
+	command.PreSave()
+	if err := command.IsValid(); err != nil {
+		return nil, err
+	}
 
-		if err := s.GetMaster().Insert(command); err != nil {
-			result.Err = model.NewAppError("SqlCommandStore.Save", "store.sql_command.save.saving.app_error", nil, "id="+command.Id+", "+err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = command
-		}
-	})
+	if err := s.GetMaster().Insert(command); err != nil {
+		return nil, model.NewAppError("SqlCommandStore.Save", "store.sql_command.save.saving.app_error", nil, "id="+command.Id+", "+err.Error(), http.StatusInternalServerError)
+	}
+
+	return command, nil
 }
 
 func (s SqlCommandStore) Get(id string) store.StoreChannel {

store/store.go

@@ -409,7 +409,7 @@ type WebhookStore interface {
 }
 
 type CommandStore interface {
-	Save(webhook *model.Command) StoreChannel
+	Save(webhook *model.Command) (*model.Command, *model.AppError)
 	Get(id string) StoreChannel
 	GetByTeam(teamId string) ([]*model.Command, *model.AppError)
 	GetByTrigger(teamId string, trigger string) StoreChannel

store/storetest/command_store.go

@@ -30,11 +30,11 @@ func testCommandStoreSave(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	if err := (<-ss.Command().Save(&o1)).Err; err != nil {
+	if _, err := ss.Command().Save(&o1); err != nil {
 		t.Fatal("couldn't save item", err)
 	}
 
-	if err := (<-ss.Command().Save(&o1)).Err; err == nil {
+	if _, err := ss.Command().Save(&o1); err == nil {
 		t.Fatal("shouldn't be able to update from save")
 	}
 }
@@ -47,7 +47,10 @@ func testCommandStoreGet(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
 		t.Fatal(r1.Err)
@@ -70,7 +73,10 @@ func testCommandStoreGetByTeam(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1, err := ss.Command().GetByTeam(o1.TeamId); err != nil {
 		t.Fatal(err)
@@ -104,8 +110,14 @@ func testCommandStoreGetByTrigger(t *testing.T, ss store.Store) {
 	o2.URL = "http://nowhere.com/"
 	o2.Trigger = "trigger1"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
-	_ = (<-ss.Command().Save(o2)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = ss.Command().Save(o2)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1 := <-ss.Command().GetByTrigger(o1.TeamId, o1.Trigger); r1.Err != nil {
 		t.Fatal(r1.Err)
@@ -130,7 +142,10 @@ func testCommandStoreDelete(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
 		t.Fatal(r1.Err)
@@ -158,7 +173,10 @@ func testCommandStoreDeleteByTeam(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
 		t.Fatal(r1.Err)
@@ -186,7 +204,10 @@ func testCommandStoreDeleteByUser(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
 		t.Fatal(r1.Err)
@@ -214,7 +235,10 @@ func testCommandStoreUpdate(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	o1.Token = model.NewId()
 
@@ -237,7 +261,10 @@ func testCommandCount(t *testing.T, ss store.Store) {
 	o1.URL = "http://nowhere.com/"
 	o1.Trigger = "trigger"
 
-	o1 = (<-ss.Command().Save(o1)).Data.(*model.Command)
+	o1, err := ss.Command().Save(o1)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if r1 := <-ss.Command().AnalyticsCommandCount(""); r1.Err != nil {
 		t.Fatal(r1.Err)

store/storetest/mocks/CommandStore.go

@@ -135,19 +135,28 @@ func (_m *CommandStore) PermanentDeleteByUser(userId string) *model.AppError {
 }
 
 // Save provides a mock function with given fields: webhook
-func (_m *CommandStore) Save(webhook *model.Command) store.StoreChannel {
+func (_m *CommandStore) Save(webhook *model.Command) (*model.Command, *model.AppError) {
 	ret := _m.Called(webhook)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(*model.Command) store.StoreChannel); ok {
+	var r0 *model.Command
+	if rf, ok := ret.Get(0).(func(*model.Command) *model.Command); ok {
 		r0 = rf(webhook)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.Command)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(*model.Command) *model.AppError); ok {
+		r1 = rf(webhook)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // Update provides a mock function with given fields: hook