diff --git a/app/webhook.go b/app/webhook.go index 7aa6cd2a33..e169af9d79 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -681,11 +681,9 @@ func (a *App) HandleCommandWebhook(hookId string, response *model.CommandRespons return model.NewAppError("HandleCommandWebhook", "web.command_webhook.parse.app_error", nil, "", http.StatusBadRequest) } - var hook *model.CommandWebhook - if result := <-a.Srv.Store.CommandWebhook().Get(hookId); result.Err != nil { - return model.NewAppError("HandleCommandWebhook", "web.command_webhook.invalid.app_error", nil, "err="+result.Err.Message, result.Err.StatusCode) - } else { - hook = result.Data.(*model.CommandWebhook) + hook, err := a.Srv.Store.CommandWebhook().Get(hookId) + if err != nil { + return model.NewAppError("HandleCommandWebhook", "web.command_webhook.invalid.app_error", nil, "err="+err.Message, err.StatusCode) } cmd, err := a.Srv.Store.Command().Get(hook.CommandId) diff --git a/store/sqlstore/command_webhook_store.go b/store/sqlstore/command_webhook_store.go index 123e527481..2fe28c0007 100644 --- a/store/sqlstore/command_webhook_store.go +++ b/store/sqlstore/command_webhook_store.go @@ -53,20 +53,20 @@ func (s SqlCommandWebhookStore) Save(webhook *model.CommandWebhook) (*model.Comm return webhook, nil } -func (s SqlCommandWebhookStore) Get(id string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var webhook model.CommandWebhook +func (s SqlCommandWebhookStore) Get(id string) (*model.CommandWebhook, *model.AppError) { + var webhook model.CommandWebhook - exptime := model.GetMillis() - model.COMMAND_WEBHOOK_LIFETIME - if err := s.GetReplica().SelectOne(&webhook, "SELECT * FROM CommandWebhooks WHERE Id = :Id AND CreateAt > :ExpTime", map[string]interface{}{"Id": id, "ExpTime": exptime}); err != nil { - result.Err = model.NewAppError("SqlCommandWebhookStore.Get", "store.sql_command_webhooks.get.app_error", nil, "id="+id+", err="+err.Error(), http.StatusInternalServerError) - if err == sql.ErrNoRows { - result.Err.StatusCode = http.StatusNotFound - } + exptime := model.GetMillis() - model.COMMAND_WEBHOOK_LIFETIME + var appErr *model.AppError + if err := s.GetReplica().SelectOne(&webhook, "SELECT * FROM CommandWebhooks WHERE Id = :Id AND CreateAt > :ExpTime", map[string]interface{}{"Id": id, "ExpTime": exptime}); err != nil { + appErr = model.NewAppError("SqlCommandWebhookStore.Get", "store.sql_command_webhooks.get.app_error", nil, "id="+id+", err="+err.Error(), http.StatusInternalServerError) + if err == sql.ErrNoRows { + appErr.StatusCode = http.StatusNotFound } + return nil, appErr + } - result.Data = &webhook - }) + return &webhook, nil } func (s SqlCommandWebhookStore) TryUse(id string, limit int) store.StoreChannel { diff --git a/store/store.go b/store/store.go index f7796b6525..d500bc6b6b 100644 --- a/store/store.go +++ b/store/store.go @@ -429,7 +429,7 @@ type CommandStore interface { type CommandWebhookStore interface { Save(webhook *model.CommandWebhook) (*model.CommandWebhook, *model.AppError) - Get(id string) StoreChannel + Get(id string) (*model.CommandWebhook, *model.AppError) TryUse(id string, limit int) StoreChannel Cleanup() } diff --git a/store/storetest/command_webhook_store.go b/store/storetest/command_webhook_store.go index 661d701843..8748aed597 100644 --- a/store/storetest/command_webhook_store.go +++ b/store/storetest/command_webhook_store.go @@ -28,15 +28,16 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { h1, err := cws.Save(h1) require.Nil(t, err) - if r1 := <-cws.Get(h1.Id); r1.Err != nil { - t.Fatal(r1.Err) + var r1 *model.CommandWebhook + if r1, err = cws.Get(h1.Id); err != nil { + t.Fatal(err) } else { - if *r1.Data.(*model.CommandWebhook) != *h1 { + if *r1 != *h1 { t.Fatal("invalid returned webhook") } } - if err = (<-cws.Get("123")).Err; err.StatusCode != http.StatusNotFound { + if _, err = cws.Get("123"); err.StatusCode != http.StatusNotFound { t.Fatal("Should have set the status as not found for missing id") } @@ -48,17 +49,17 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { h2, err = cws.Save(h2) require.Nil(t, err) - if err := (<-cws.Get(h2.Id)).Err; err == nil || err.StatusCode != http.StatusNotFound { + if _, err := cws.Get(h2.Id); err == nil || err.StatusCode != http.StatusNotFound { t.Fatal("Should have set the status as not found for expired webhook") } cws.Cleanup() - if err := (<-cws.Get(h1.Id)).Err; err != nil { + if _, err := cws.Get(h1.Id); err != nil { t.Fatal("Should have no error getting unexpired webhook") } - if err := (<-cws.Get(h2.Id)).Err; err.StatusCode != http.StatusNotFound { + if _, err := cws.Get(h2.Id); err.StatusCode != http.StatusNotFound { t.Fatal("Should have set the status as not found for expired webhook") } diff --git a/store/storetest/mocks/CommandWebhookStore.go b/store/storetest/mocks/CommandWebhookStore.go index d57e36f9b0..1c35cb4f09 100644 --- a/store/storetest/mocks/CommandWebhookStore.go +++ b/store/storetest/mocks/CommandWebhookStore.go @@ -19,19 +19,28 @@ func (_m *CommandWebhookStore) Cleanup() { } // Get provides a mock function with given fields: id -func (_m *CommandWebhookStore) Get(id string) store.StoreChannel { +func (_m *CommandWebhookStore) Get(id string) (*model.CommandWebhook, *model.AppError) { ret := _m.Called(id) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.CommandWebhook + if rf, ok := ret.Get(0).(func(string) *model.CommandWebhook); ok { r0 = rf(id) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.CommandWebhook) } } - 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 } // Save provides a mock function with given fields: webhook