MM-15288: Migrate CommandStore.Get to sync by default (#10739)

* MM-15288: Migrate Command.Get to sync by default

* addressing review comments, updating status code of err returned by Get Method
This commit is contained in:
Puneeth Reddy
2019-05-06 09:12:41 -07:00
committed by Jesús Espino
parent fc15eda37f
commit 2d3fb4f426
7 changed files with 50 additions and 47 deletions

View File

@@ -474,13 +474,13 @@ func (a *App) GetCommand(commandId string) (*model.Command, *model.AppError) {
return nil, model.NewAppError("GetCommand", "api.command.disabled.app_error", nil, "", http.StatusNotImplemented)
}
result := <-a.Srv.Store.Command().Get(commandId)
if result.Err != nil {
result.Err.StatusCode = http.StatusNotFound
return nil, result.Err
cmd, err := a.Srv.Store.Command().Get(commandId)
if err != nil {
err.StatusCode = http.StatusNotFound
return nil, err
}
return result.Data.(*model.Command), nil
return cmd, nil
}
func (a *App) UpdateCommand(oldCmd, updatedCmd *model.Command) (*model.Command, *model.AppError) {

View File

@@ -686,11 +686,9 @@ func (a *App) HandleCommandWebhook(hookId string, response *model.CommandRespons
hook = result.Data.(*model.CommandWebhook)
}
var cmd *model.Command
if result := <-a.Srv.Store.Command().Get(hook.CommandId); result.Err != nil {
return model.NewAppError("HandleCommandWebhook", "web.command_webhook.command.app_error", nil, "err="+result.Err.Message, http.StatusBadRequest)
} else {
cmd = result.Data.(*model.Command)
cmd, err := a.Srv.Store.Command().Get(hook.CommandId)
if err != nil {
return model.NewAppError("HandleCommandWebhook", "web.command_webhook.command.app_error", nil, "err="+err.Message, http.StatusBadRequest)
}
args := &model.CommandArgs{
@@ -705,6 +703,6 @@ func (a *App) HandleCommandWebhook(hookId string, response *model.CommandRespons
return model.NewAppError("HandleCommandWebhook", "web.command_webhook.invalid.app_error", nil, "err="+result.Err.Message, result.Err.StatusCode)
}
_, err := a.HandleCommandResponse(cmd, args, response, false)
_, err = a.HandleCommandResponse(cmd, args, response, false)
return err
}

View File

@@ -55,9 +55,7 @@ func getCommandFromCommandArg(a *app.App, commandArg string) *model.Command {
}
if command == nil {
if result := <-a.Srv.Store.Command().Get(commandPart); result.Err == nil {
command = result.Data.(*model.Command)
}
command, _ = a.Srv.Store.Command().Get(commandPart)
}
return command

View File

@@ -61,16 +61,14 @@ func (s SqlCommandStore) Save(command *model.Command) (*model.Command, *model.Ap
return command, nil
}
func (s SqlCommandStore) Get(id string) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
var command model.Command
func (s SqlCommandStore) Get(id string) (*model.Command, *model.AppError) {
var command model.Command
if err := s.GetReplica().SelectOne(&command, "SELECT * FROM Commands WHERE Id = :Id AND DeleteAt = 0", map[string]interface{}{"Id": id}); err != nil {
result.Err = model.NewAppError("SqlCommandStore.Get", "store.sql_command.save.get.app_error", nil, "id="+id+", err="+err.Error(), http.StatusInternalServerError)
}
if err := s.GetReplica().SelectOne(&command, "SELECT * FROM Commands WHERE Id = :Id AND DeleteAt = 0", map[string]interface{}{"Id": id}); err != nil {
return nil, model.NewAppError("SqlCommandStore.Get", "store.sql_command.save.get.app_error", nil, "id="+id+", err="+err.Error(), http.StatusInternalServerError)
}
result.Data = &command
})
return &command, nil
}
func (s SqlCommandStore) GetByTeam(teamId string) ([]*model.Command, *model.AppError) {

View File

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

View File

@@ -52,15 +52,15 @@ func testCommandStoreGet(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
t.Fatal(r1.Err)
if r1, err := ss.Command().Get(o1.Id); err != nil {
t.Fatal(err)
} else {
if r1.Data.(*model.Command).CreateAt != o1.CreateAt {
if r1.CreateAt != o1.CreateAt {
t.Fatal("invalid returned command")
}
}
if err := (<-ss.Command().Get("123")).Err; err == nil {
if _, err := ss.Command().Get("123"); err == nil {
t.Fatal("Missing id should have failed")
}
}
@@ -150,10 +150,10 @@ func testCommandStoreDelete(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
t.Fatal(r1.Err)
if r1, err := ss.Command().Get(o1.Id); err != nil {
t.Fatal(err)
} else {
if r1.Data.(*model.Command).CreateAt != o1.CreateAt {
if r1.CreateAt != o1.CreateAt {
t.Fatal("invalid returned command")
}
}
@@ -162,8 +162,8 @@ func testCommandStoreDelete(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r3 := (<-ss.Command().Get(o1.Id)); r3.Err == nil {
t.Log(r3.Data)
if r3, err := ss.Command().Get(o1.Id); err == nil {
t.Log(r3)
t.Fatal("Missing id should have failed")
}
}
@@ -181,10 +181,10 @@ func testCommandStoreDeleteByTeam(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
t.Fatal(r1.Err)
if r1, err := ss.Command().Get(o1.Id); err != nil {
t.Fatal(err)
} else {
if r1.Data.(*model.Command).CreateAt != o1.CreateAt {
if r1.CreateAt != o1.CreateAt {
t.Fatal("invalid returned command")
}
}
@@ -193,8 +193,8 @@ func testCommandStoreDeleteByTeam(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r3 := (<-ss.Command().Get(o1.Id)); r3.Err == nil {
t.Log(r3.Data)
if r3, err := ss.Command().Get(o1.Id); err == nil {
t.Log(r3)
t.Fatal("Missing id should have failed")
}
}
@@ -212,10 +212,10 @@ func testCommandStoreDeleteByUser(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r1 := <-ss.Command().Get(o1.Id); r1.Err != nil {
t.Fatal(r1.Err)
if r1, err := ss.Command().Get(o1.Id); err != nil {
t.Fatal(err)
} else {
if r1.Data.(*model.Command).CreateAt != o1.CreateAt {
if r1.CreateAt != o1.CreateAt {
t.Fatal("invalid returned command")
}
}
@@ -224,8 +224,8 @@ func testCommandStoreDeleteByUser(t *testing.T, ss store.Store) {
t.Fatal(err)
}
if r3 := (<-ss.Command().Get(o1.Id)); r3.Err == nil {
t.Log(r3.Data)
if r3, err := ss.Command().Get(o1.Id); err == nil {
t.Log(r3)
t.Fatal("Missing id should have failed")
}
}

View File

@@ -53,19 +53,28 @@ func (_m *CommandStore) Delete(commandId string, time int64) *model.AppError {
}
// Get provides a mock function with given fields: id
func (_m *CommandStore) Get(id string) store.StoreChannel {
func (_m *CommandStore) Get(id string) (*model.Command, *model.AppError) {
ret := _m.Called(id)
var r0 store.StoreChannel
if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
var r0 *model.Command
if rf, ok := ret.Get(0).(func(string) *model.Command); ok {
r0 = rf(id)
} 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(string) *model.AppError); ok {
r1 = rf(id)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
}
return r0, r1
}
// GetByTeam provides a mock function with given fields: teamId